Re: Suspend problems in 2.6.31-rc6

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

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

On 2009.08.21 09:21:32 +0800, Zhenyu Wang wrote:
> On 2009.08.20 11:17:45 -0400, Alan Stern wrote:
> > On Wed, 19 Aug 2009, Rafael J. Wysocki wrote:
> > 
> > > Usually, however, restoring the PCI config space twice doesn't hurt, although
> > > it evidently does hurt on your system.
> > > 
> > > Hm, there's a patch that should help here I think.  Can you please test
> > > this one: http://patchwork.kernel.org/patch/40062/ ?
> > 
> > Testing confirms that Alex's patch (pci: check saved state before
> > restore) fixes the resume problem.  IMO it should be merged before
> > 2.6.31 is released.
> > 
> 
> I can't understand why Alex's patch could fix for you, or because you've
> seen 'state_saved' is false in your testing, and Alex's patch just workaround
> state restore for you? I don't understand why 'state_saved' is false in your
> case, as pci layer should do save state for us by default, no?
> 
> Could you try with this one? And what's your gfx chip? I've tested rc6 on my
> 965GM yesterday, S3/S4 both seem fine...
> 

oops, here's right one for testing...

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 8c9d50d..5ed62c1 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -2303,6 +2303,13 @@ static void __devexit agp_intel_remove(struct pci_dev *pdev)
 }
 
 #ifdef CONFIG_PM
+static int agp_intel_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	pci_save_state(pdev);
+	pci_set_power_state(pdev, 3);
+	return 0;
+}
+
 static int agp_intel_resume(struct pci_dev *pdev)
 {
 	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
@@ -2410,6 +2417,7 @@ static struct pci_driver agp_intel_pci_driver = {
 	.remove		= __devexit_p(agp_intel_remove),
 #ifdef CONFIG_PM
 	.resume		= agp_intel_resume,
+	.suspend	= agp_intel_suspend,
 #endif
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index fc4b68a..f54880f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -95,7 +95,11 @@ static int i915_resume(struct drm_device *dev)
 	int ret = 0;
 
 	pci_set_power_state(dev->pdev, PCI_D0);
-	pci_restore_state(dev->pdev);
+	/* XXX as pci layer should have already early restore state
+	   for us, or intel_agp driver has taken care of reconfigure,
+	   we don't need to restore again here, which might corrupt
+	   reconfigured state...*/
+	/* pci_restore_state(dev->pdev); */
 	if (pci_enable_device(dev->pdev))
 		return -1;
 	pci_set_master(dev->pdev);
-- 
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 --]