Re: Suspend problems in 2.6.31-rc6

Zhenyu Wang
2009-08-21T01:21:32Z

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

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...

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index 8c9d50d..1f3a299 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -2303,6 +2303,12 @@ static void __devexit agp_intel_remove(struct pci_dev *pdev)
}

#ifdef CONFIG_PM
+static int agp_intel_suspend(struct pci_dev *pdev)
+{
+ pci_save_state(dev);
+ pci_set_power_state(dev, 3);
+}
+
static int agp_intel_resume(struct pci_dev *pdev)
{
struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
@@ -2410,6 +2416,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 --]