Re: Suspend problems in 2.6.31-rc6

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2819 bytes --]

On 2009.08.19 01:22:16 +0200, Rafael J. Wysocki wrote:
> > 
> > I have narrowed this down.  The problem occurs in agp_intel_resume()
> > in drivers/char/agp/intel-agp.c.  Here's the beginning of the routine:
> > 
> > 
> > static int agp_intel_resume(struct pci_dev *pdev)
> > {
> > 	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
> > 	int ret_val;
> > 
> > 	pci_restore_state(pdev);
> > 
> > 	/* We should restore our graphics device's config space,
> > 	 * as host bridge (00:00) resumes before graphics device (02:00),
> > 	 * then our access to its pci space can work right.
> > 	 */
> > 	if (intel_private.pcidev)
> > 		pci_restore_state(intel_private.pcidev);
> > 
> > 
> > As the comment says, pdev is 0000:00:00.0 and intel_private.pcidev is
> > 0000:00:02.0.  So this is the resume routine for 00:00.0, and it calls
> > pci_restore_state() for 00:02.0 before that device has been resumed.
> > 
> > Note that there is no corresponding agp_intel_suspend() function.
> > 
> > Stuff stops appearing on the screen in the middle of the second
> > pci_restore_state() call.  That is, I see messages about "restoring
> > config space at offset..." for each of the two devices, but a
> > debugging message added at the end of pci_restore_state() doesn't
> > appear for the second call.
> > 
> > (Should these config space changes occur at all, given that I'm
> > testing with "echo devices >pm_test"?)
> > 
> > It's also worth noting that a debugging message added to the start of
> > pci_restore_state() shows that for both of these calls,
> > pdev->state_saved is false.  It's not clear whether that matters,
> > though, because the suspend and resume routines in the PCI core do so
> > many calls to pci_save_state() and pci_restore_state() on their own.
> > 
> > Commenting out the second call to pci_restore_state() apparently fixes 
> > the problem.  I have no idea whether it's the right thing to do, 
> > though.
> 
> It is.  The PCI core have restored the PCI config already anyway at the early
> resume stage.
> 
> I think the first pci_restore_state() could be dropped from there as well.
> 

Rafael, as I added this restore long time ago, pci core has changed in some way?
We had problem when reconfigure agp GTT space in resume, as we must be sure
gfx device's config space has been restored properly, and as 02:00 gfx device
restored later than host bridge 00:00 device, that came to my origin patch to
restore it earlier. So do you mean this order issue is gone now?

We also do proper pci save/restore in drm/i915 driver, which is the real pci
driver for our gfx device. I'm not sure if there could any conflict between them...

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]