Re: Suspend problems in 2.6.31-rc6

Jesse Barnes
2009-08-27T16:18:34Z

On Thu, 27 Aug 2009 10:51:47 -0400 (EDT)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Mon, 24 Aug 2009, Zhenyu Wang wrote:
>
> > yeah, Alex's patch is ok in general, and Alan's problem seems
> > specific on 845G. So the origin restore order issue for us seems
> > gone with pci layer early resume, we could remove the restore in
> > intel_agp and drm/i915, although Alex's patch makes them noop.
> >
> > How about this one? Alan, could you help to test this with drm/i915
> > module loaded?
> >
> > thanks.
> >
> > From 7ab123a67e3ab3f31051d95e74362ae711e0657e Mon Sep 17 00:00:00
> > 2001 From: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Date: Mon, 24 Aug 2009 10:29:07 +0800
> > Subject: [PATCH] agp/intel: remove restore in resume
> >
> > As earlier pci driver resume has already restored space for
> > host bridge and graphics device, don't need to restore it
> > again which might cause problem on some chips, e.g 845G tested
> > by Alan.
> >
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> > drivers/char/agp/intel-agp.c | 9 ---------
> > drivers/gpu/drm/i915/i915_drv.c | 1 -
> > 2 files changed, 0 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/char/agp/intel-agp.c
> > b/drivers/char/agp/intel-agp.c index 8c9d50d..ed43ab2 100644
> > --- a/drivers/char/agp/intel-agp.c
> > +++ b/drivers/char/agp/intel-agp.c
> > @@ -2308,15 +2308,6 @@ 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);
> > -
> > if (bridge->driver == &intel_generic_driver)
> > intel_configure();
> > else if (bridge->driver == &intel_850_driver)
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c index fc4b68a..645f298 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -95,7 +95,6 @@ static int i915_resume(struct drm_device *dev)
> > int ret = 0;
> >
> > pci_set_power_state(dev->pdev, PCI_D0);
> > - pci_restore_state(dev->pdev);
> > if (pci_enable_device(dev->pdev))
> > return -1;
> > pci_set_master(dev->pdev);
>
> I tested the following configurations (each with your patch applied):
>
> Without Alex's patch and without i915 loaded;
>
> With Alex's patch and without i915 loaded;
>
> With Alex's patch and with i915 loaded.
>
> They all worked. (Note that I normally run without drm/i915. The
> Xorg 2D-acceleration code is buggy on my hardware, so I don't use it.)

Could I convince you to file bugs for the problems you're seeing at
bugs.freedesktop.org? We don't have many 845G test machines but we
have fixed several problems on that chipset recently.

--
Jesse Barnes, Intel Open Source Technology Center