2008-03-26 23:34:55

by Jesse Barnes

[permalink] [raw]
Subject: [PATCH] don't suspend/resume 8xx chips

Recent testing has turned up some bugs in the new Intel suspend/resume code
for old, 8xx chipsets. So for 2.6.25 it probably makes sense to apply this
patch, which should prevent the new code from getting called on those
chipsets. We should have this fixed soon, but not in time for 2.6.25
unfortunately. Note that this patch (along with the suspend/resume code in
general) could use more testing.

Signed-off-by: Jesse Barnes <[email protected]>

diff --git a/drivers/char/drm/i915_dma.c b/drivers/char/drm/i915_dma.c
index e9d6663..6964a28 100644
--- a/drivers/char/drm/i915_dma.c
+++ b/drivers/char/drm/i915_dma.c
@@ -762,6 +762,11 @@ int i915_driver_load(struct drm_device *dev, unsigned
long
unsigned long base, size;
int ret = 0, mmio_bar = IS_I9XX(dev) ? 0 : 1;

+ if (!IS_I9XX(dev)) {
+ dev->driver->suspend = NULL;
+ dev->driver->resume = NULL;
+ }
+
/* i915 has 4 more counters */
dev->counters += 4;
dev->types[6] = _DRM_STAT_IRQ;


2008-03-27 08:46:52

by Soren Hansen

[permalink] [raw]
Subject: Re: [PATCH] don't suspend/resume 8xx chips

On Wed, Mar 26, 2008 at 04:28:12PM -0700, Jesse Barnes wrote:
> Recent testing has turned up some bugs in the new Intel suspend/resume code
> for old, 8xx chipsets. So for 2.6.25 it probably makes sense to apply this
> patch, which should prevent the new code from getting called on those
> chipsets. We should have this fixed soon, but not in time for 2.6.25
> unfortunately. Note that this patch (along with the suspend/resume code in
> general) could use more testing.

I've tested this patch on two different laptops: A Uniwill 223 and an
IBM Thinkpad X40. Both have Intel 855 graphics. The former now suspends
and hibernates just fine (it fails without the patch). The latter works
both with and without the patch.

--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/


Attachments:
(No filename) (863.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2008-03-28 20:09:26

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] don't suspend/resume 8xx chips

On Wed 2008-03-26 16:28:12, Jesse Barnes wrote:
> Recent testing has turned up some bugs in the new Intel suspend/resume code
> for old, 8xx chipsets. So for 2.6.25 it probably makes sense to apply this
> patch, which should prevent the new code from getting called on those
> chipsets. We should have this fixed soon, but not in time for 2.6.25
> unfortunately. Note that this patch (along with the suspend/resume code in
> general) could use more testing.
>
> Signed-off-by: Jesse Barnes <[email protected]>
>
> diff --git a/drivers/char/drm/i915_dma.c b/drivers/char/drm/i915_dma.c
> index e9d6663..6964a28 100644
> --- a/drivers/char/drm/i915_dma.c
> +++ b/drivers/char/drm/i915_dma.c
> @@ -762,6 +762,11 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long
> unsigned long base, size;
> int ret = 0, mmio_bar = IS_I9XX(dev) ? 0 : 1;
>
> + if (!IS_I9XX(dev)) {
> + dev->driver->suspend = NULL;
> + dev->driver->resume = NULL;
> + }
> +

Are you sure your driver needs no state saving?

Maybe register suspend that printks, returns error?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-03-28 20:14:23

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH] don't suspend/resume 8xx chips

On Friday, March 28, 2008 7:40 am Pavel Machek wrote:
> On Wed 2008-03-26 16:28:12, Jesse Barnes wrote:
> > Recent testing has turned up some bugs in the new Intel suspend/resume
> > code for old, 8xx chipsets. So for 2.6.25 it probably makes sense to
> > apply this patch, which should prevent the new code from getting called
> > on those chipsets. We should have this fixed soon, but not in time for
> > 2.6.25 unfortunately. Note that this patch (along with the
> > suspend/resume code in general) could use more testing.
> >
> > Signed-off-by: Jesse Barnes <[email protected]>
> >
> > diff --git a/drivers/char/drm/i915_dma.c b/drivers/char/drm/i915_dma.c
> > index e9d6663..6964a28 100644
> > --- a/drivers/char/drm/i915_dma.c
> > +++ b/drivers/char/drm/i915_dma.c
> > @@ -762,6 +762,11 @@ int i915_driver_load(struct drm_device *dev,
> > unsigned long
> > unsigned long base, size;
> > int ret = 0, mmio_bar = IS_I9XX(dev) ? 0 : 1;
> >
> > + if (!IS_I9XX(dev)) {
> > + dev->driver->suspend = NULL;
> > + dev->driver->resume = NULL;
> > + }
> > +
>
> Are you sure your driver needs no state saving?
>
> Maybe register suspend that printks, returns error?

No, there's definitely state we'd like to save/restore, but 8xx chips are
tricky and we won't have them working before 2.6.25-final. This patch
preserves old behavior for 8xx chips and allows 9xx chips to properly survive
suspend/resume events, so I don't think a printk is necessary.

Jesse