Re: Suspend problems in 2.6.31-rc6

Zhenyu Wang
2009-08-21T03:13:20Z

[-- 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 --]