[regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk

[regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk

[Alan Jenkins]

Hi

I found this regression on my EeePC 701 with modesetting enabled.  When 
I hibernate using s2disk, I can abort the hibernation by pressing the 
backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).

X resumes where it left off, but the problem is that the screen is 
frozen (except for the cursor).  The system still responds to input.  
E.g. if I hibernate from Konsole and type "find /" on resume, I don't 
see any result on the screen, but I can see the disk activity LED 
flashing madly.  I can also switch to text consoles, which work as normal.

The first bad commit in this scenario is v2.6.32-7504-gcbda12d: 
"drm/i915: implement new pm ops for i915".  I think the problem is that 
it (deliberately) turns "thaw" into a no-op.  That said, this "first 
bad" behaviour was somewhat different.  When I checked out this commit 
and tested it, it looked more like X was crashing and being repeatedly 
restarted.  The screen went black, and every few seconds it flickered, 
showing a text console for a fraction of a second and then going black 
again.

A few commits further on at v2.6.32-7518-ge3d8aff, I get the same 
behaviour as v2.6.33-rc6.  I didn't bother to find out exactly where 
this change happens.

On v2.6.32-7518-ge3d8aff, I was able to observe that aborting s2disk 
leaves the i915 interrupt [IRQ16] disabled:

Before s2disk:

$ cat /proc/interrupts
            CPU0                        
   0:       2030   IO-APIC-edge      timer
   1:        191   IO-APIC-edge      i8042
   8:          1   IO-APIC-edge      rtc0
   9:         51   IO-APIC-fasteoi   acpi
  12:        525   IO-APIC-edge      i8042
  14:          0   IO-APIC-edge      ata_piix
  15:       4954   IO-APIC-edge      ata_piix
  16:        123   IO-APIC-fasteoi   uhci_hcd:usb5, i915
  18:        134   IO-APIC-fasteoi   uhci_hcd:usb4, ath
  19:          0   IO-APIC-fasteoi   uhci_hcd:usb3     
  23:        345   IO-APIC-fasteoi   ehci_hcd:usb1, uhci_hcd:usb2
  27:        169   PCI-MSI-edge      HDA Intel
  28:        150   PCI-MSI-edge      eth0
 NMI:          0   Non-maskable interrupts
...

After aborting s2disk:

$ cat /proc/interrupts
            CPU0
   0:       3241   IO-APIC-edge      timer
   1:        325   IO-APIC-edge      i8042
   8:          1   IO-APIC-edge      rtc0
   9:         79   IO-APIC-fasteoi   acpi
  12:        525   IO-APIC-edge      i8042
  14:          0   IO-APIC-edge      ata_piix
  15:       5812   IO-APIC-edge      ata_piix
  16:        160   IO-APIC-fasteoi   uhci_hcd:usb5
  18:        231   IO-APIC-fasteoi   uhci_hcd:usb4, ath
  19:          0   IO-APIC-fasteoi   uhci_hcd:usb3
  23:        726   IO-APIC-fasteoi   ehci_hcd:usb1, uhci_hcd:usb2
  27:          0   PCI-MSI-edge      HDA Intel
  28:        102   PCI-MSI-edge      eth0
 NMI:          0   Non-maskable interrupts
...


Without looking at the code, my suggestion is that

a) It looks like "freeze" disables the interrupt, and "thaw" is no 
longer re-enabling it.

b) *If* this is the only problem, it could be fixed by not explicitly 
disabling the interrupt in the "freeze" callback.  In any case this 
would be the right thing to do, since the PCI core now takes 
responsibility for disabling IRQs over suspend (mentioned here: 
http://article.gmane.org/gmane.linux.kernel.pci/4696).

Regards
Alan

Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk

[Rafael J. Wysocki]

On Wednesday 03 February 2010, Alan Jenkins wrote:
> Hi
> 
> I found this regression on my EeePC 701 with modesetting enabled.  When 
> I hibernate using s2disk, I can abort the hibernation by pressing the 
> backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).

Yeah.

To be honest, I knew that's going to happen, but didn't have the time to take
care of it.

The problem is that i915 does literally _nothing_ in its .thaw() callback,
although it should at least reverse whatever .freeze() did to the hardware
(and memory allocations and so on), so that the adapter is functional
after creating the image.

Fixing this requires some thought, though, because at the moment .freeze()
thinks it's .suspend(), which is not the case as this report clearly shows.
So, in fact i915_pci_suspend() has to be split into the .freeze() part and
the poweroff part cleanly and that's not  so simple (at least to me).

Hopefully I'll have some time to work on this during the weekend.

Rafael

Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On 2010.02.03 23:44:41 +0100, Rafael J. Wysocki wrote:
> On Wednesday 03 February 2010, Alan Jenkins wrote:
> > Hi
> > 
> > I found this regression on my EeePC 701 with modesetting enabled.  When 
> > I hibernate using s2disk, I can abort the hibernation by pressing the 
> > backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).
> 
> Yeah.
> 
> To be honest, I knew that's going to happen, but didn't have the time to take
> care of it.
> 
> The problem is that i915 does literally _nothing_ in its .thaw() callback,
> although it should at least reverse whatever .freeze() did to the hardware
> (and memory allocations and so on), so that the adapter is functional
> after creating the image.
> 
> Fixing this requires some thought, though, because at the moment .freeze()
> thinks it's .suspend(), which is not the case as this report clearly shows.
> So, in fact i915_pci_suspend() has to be split into the .freeze() part and
> the poweroff part cleanly and that's not  so simple (at least to me).
> 

Right, I think that'll be more clean, stuff in i915_save/restore_state() need
to be splited too, especially isolate stuff for mode setting and other device
state, as what my original purpose for this is to remove extra mode setting 
cycle in old behavior so not waste time for hibernate.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] i915 / PM: Fix crash while aborting hibernation (Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk)

[Rafael J. Wysocki]

On Thursday 04 February 2010, Zhenyu Wang wrote:
> On 2010.02.03 23:44:41 +0100, Rafael J. Wysocki wrote:
> > On Wednesday 03 February 2010, Alan Jenkins wrote:
> > > Hi
> > > 
> > > I found this regression on my EeePC 701 with modesetting enabled.  When 
> > > I hibernate using s2disk, I can abort the hibernation by pressing the 
> > > backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).
> > 
> > Yeah.
> > 
> > To be honest, I knew that's going to happen, but didn't have the time to take
> > care of it.
> > 
> > The problem is that i915 does literally _nothing_ in its .thaw() callback,
> > although it should at least reverse whatever .freeze() did to the hardware
> > (and memory allocations and so on), so that the adapter is functional
> > after creating the image.
> > 
> > Fixing this requires some thought, though, because at the moment .freeze()
> > thinks it's .suspend(), which is not the case as this report clearly shows.
> > So, in fact i915_pci_suspend() has to be split into the .freeze() part and
> > the poweroff part cleanly and that's not  so simple (at least to me).
> > 
> 
> Right, I think that'll be more clean, stuff in i915_save/restore_state() need
> to be splited too, especially isolate stuff for mode setting and other device
> state, as what my original purpose for this is to remove extra mode setting 
> cycle in old behavior so not waste time for hibernate.

We can't really do that, because we'll need to restore the saved state at the
resume-from-hibernation stage.

The appended patch fixes the issue for me, although it's been only tested
a little.  It sort of defeats the purpose of commit
cbda12d77ea590082edb6d30bd342a67ebc459e0, but I don't see any less invasive
way to fix this except maybe for reverting that commit entirely.

Note that the drm_irq_[un]install() thing may be unnecessary, but I wasn't sure
about that and surely wouldn't suggest doing that for 2.6.33.  Also it looks like
some things from the freeze and thaw parts may be moved to the "low-level"
suspend and resume parts, respectively, but that would require some
i915_gem_* surgery I was too scared to do.

Alan, please test, i915 guys, please review.

Rafael

---
Subject: i915 / PM: Fix crash while aborting hibernation
From: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>

Commit cbda12d77ea590082edb6d30bd342a67ebc459e0 (drm/i915: implement
new pm ops for i915) introduced the problem that if s2disk
hibernation is aborted, the system will crash, because
i915_pm_freeze() does nothing, while it should at least reverse some
operations carried out by i915_suspend().

Fix this issue by splitting the i915 suspend into a freeze part a
suspend part, where the latter is not executed before creating a
hibernation image, and the i915 resume into a "low-level" resume
part and a thaw part, where the former is not executed after the
image has been created.

Signed-off-by: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
---
 drivers/gpu/drm/i915/i915_drv.c |  168 ++++++++++++++++++++++++----------------
 1 file changed, 101 insertions(+), 67 deletions(-)

Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
@@ -175,78 +175,100 @@ const static struct pci_device_id pciidl
 MODULE_DEVICE_TABLE(pci, pciidlist);
 #endif
 
-static int i915_suspend(struct drm_device *dev, pm_message_t state)
+static int i915_drm_freeze(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (!dev || !dev_priv) {
-		DRM_ERROR("dev: %p, dev_priv: %p\n", dev, dev_priv);
-		DRM_ERROR("DRM not initialized, aborting suspend.\n");
-		return -ENODEV;
-	}
-
-	if (state.event == PM_EVENT_PRETHAW)
-		return 0;
-
 	pci_save_state(dev->pdev);
 
 	/* If KMS is active, we do the leavevt stuff here */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		if (i915_gem_idle(dev))
+		int error = i915_gem_idle(dev);
+		if (error) {
 			dev_err(&dev->pdev->dev,
-				"GEM idle failed, resume may fail\n");
+				"GEM idle failed, resume might fail\n");
+			return error;
+		}
 		drm_irq_uninstall(dev);
 	}
 
 	i915_save_state(dev);
 
+	return 0;
+}
+
+static void i915_drm_suspend(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	intel_opregion_free(dev, 1);
 
+	/* Modeset on resume, not lid events */
+	dev_priv->modeset_on_lid = 0;
+}
+
+static int i915_suspend(struct drm_device *dev, pm_message_t state)
+{
+	int error;
+
+	if (!dev || !dev->dev_private) {
+		DRM_ERROR("dev: %p\n", dev);
+		DRM_ERROR("DRM not initialized, aborting suspend.\n");
+		return -ENODEV;
+	}
+
+	if (state.event == PM_EVENT_PRETHAW)
+		return 0;
+
+	error = i915_drm_freeze(dev);
+	if (error)
+		return error;
+
+	i915_drm_suspend(dev);
+
 	if (state.event == PM_EVENT_SUSPEND) {
 		/* Shut down the device */
 		pci_disable_device(dev->pdev);
 		pci_set_power_state(dev->pdev, PCI_D3hot);
 	}
 
-	/* Modeset on resume, not lid events */
-	dev_priv->modeset_on_lid = 0;
-
 	return 0;
 }
 
-static int i915_resume(struct drm_device *dev)
+static int i915_drm_thaw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret = 0;
-
-	if (pci_enable_device(dev->pdev))
-		return -1;
-	pci_set_master(dev->pdev);
-
-	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
+	int error = 0;
 
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
 		dev_priv->mm.suspended = 0;
 
-		ret = i915_gem_init_ringbuffer(dev);
-		if (ret != 0)
-			ret = -1;
+		error = i915_gem_init_ringbuffer(dev);
 		mutex_unlock(&dev->struct_mutex);
 
 		drm_irq_install(dev);
-	}
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+
 		/* Resume the modeset for every activated CRTC */
 		drm_helper_resume_force_mode(dev);
 	}
 
 	dev_priv->modeset_on_lid = 0;
 
-	return ret;
+	return error;
+}
+
+static int i915_resume(struct drm_device *dev)
+{
+	if (pci_enable_device(dev->pdev))
+		return -EIO;
+
+	pci_set_master(dev->pdev);
+
+	i915_restore_state(dev);
+
+	intel_opregion_init(dev, 1);
+
+	return i915_drm_thaw(dev);
 }
 
 /**
@@ -387,57 +409,69 @@ i915_pci_remove(struct pci_dev *pdev)
 	drm_put_dev(dev);
 }
 
-static int
-i915_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int i915_pm_suspend(struct device *dev)
 {
-	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	int error;
 
-	return i915_suspend(dev, state);
-}
+	if (!drm_dev || !drm_dev->dev_private) {
+		dev_err(dev, "DRM not initialized, aborting suspend.\n");
+		return -ENODEV;
+	}
 
-static int
-i915_pci_resume(struct pci_dev *pdev)
-{
-	struct drm_device *dev = pci_get_drvdata(pdev);
+	error = i915_drm_freeze(drm_dev);
+	if (error)
+		return error;
 
-	return i915_resume(dev);
-}
+	i915_drm_suspend(drm_dev);
 
-static int
-i915_pm_suspend(struct device *dev)
-{
-	return i915_pci_suspend(to_pci_dev(dev), PMSG_SUSPEND);
-}
+	pci_disable_device(pdev);
+	pci_set_power_state(pdev, PCI_D3hot);
 
-static int
-i915_pm_resume(struct device *dev)
-{
-	return i915_pci_resume(to_pci_dev(dev));
+	return 0;
 }
 
-static int
-i915_pm_freeze(struct device *dev)
+static int i915_pm_resume(struct device *dev)
 {
-	return i915_pci_suspend(to_pci_dev(dev), PMSG_FREEZE);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+	return i915_resume(drm_dev);
 }
 
-static int
-i915_pm_thaw(struct device *dev)
+static int i915_pm_freeze(struct device *dev)
 {
-	/* thaw during hibernate, do nothing! */
-	return 0;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+	if (!drm_dev || !drm_dev->dev_private) {
+		dev_err(dev, "DRM not initialized, aborting suspend.\n");
+		return -ENODEV;
+	}
+
+	return i915_drm_freeze(drm_dev);
 }
 
-static int
-i915_pm_poweroff(struct device *dev)
+static int i915_pm_thaw(struct device *dev)
 {
-	return i915_pci_suspend(to_pci_dev(dev), PMSG_HIBERNATE);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+	return i915_drm_thaw(drm_dev);
 }
 
-static int
-i915_pm_restore(struct device *dev)
+static int i915_pm_poweroff(struct device *dev)
 {
-	return i915_pci_resume(to_pci_dev(dev));
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	int error;
+
+	error = i915_drm_freeze(drm_dev);
+	if (!error)
+		i915_drm_suspend(drm_dev);
+
+	return error;
 }
 
 const struct dev_pm_ops i915_pm_ops = {
@@ -446,7 +480,7 @@ const struct dev_pm_ops i915_pm_ops = {
      .freeze = i915_pm_freeze,
      .thaw = i915_pm_thaw,
      .poweroff = i915_pm_poweroff,
-     .restore = i915_pm_restore,
+     .restore = i915_pm_resume,
 };
 
 static struct vm_operations_struct i915_gem_vm_ops = {

Re: [PATCH] i915 / PM: Fix crash while aborting hibernation (Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk)

[Alan Jenkins]

Rafael J. Wysocki wrote:
> On Thursday 04 February 2010, Zhenyu Wang wrote:
>   
>> On 2010.02.03 23:44:41 +0100, Rafael J. Wysocki wrote:
>>     
>>> On Wednesday 03 February 2010, Alan Jenkins wrote:
>>>       
>>>> Hi
>>>>
>>>> I found this regression on my EeePC 701 with modesetting enabled.  When 
>>>> I hibernate using s2disk, I can abort the hibernation by pressing the 
>>>> backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).
>>>>         
>>> Yeah.
>>>
>>> To be honest, I knew that's going to happen, but didn't have the time to take
>>> care of it.
>>>
>>> The problem is that i915 does literally _nothing_ in its .thaw() callback,
>>> although it should at least reverse whatever .freeze() did to the hardware
>>> (and memory allocations and so on), so that the adapter is functional
>>> after creating the image.
>>>
>>> Fixing this requires some thought, though, because at the moment .freeze()
>>> thinks it's .suspend(), which is not the case as this report clearly shows.
>>> So, in fact i915_pci_suspend() has to be split into the .freeze() part and
>>> the poweroff part cleanly and that's not  so simple (at least to me).
>>>
>>>       
>> Right, I think that'll be more clean, stuff in i915_save/restore_state() need
>> to be splited too, especially isolate stuff for mode setting and other device
>> state, as what my original purpose for this is to remove extra mode setting 
>> cycle in old behavior so not waste time for hibernate.
>>     
>
> We can't really do that, because we'll need to restore the saved state at the
> resume-from-hibernation stage.
>
> The appended patch fixes the issue for me, although it's been only tested
> a little.  It sort of defeats the purpose of commit
> cbda12d77ea590082edb6d30bd342a67ebc459e0, but I don't see any less invasive
> way to fix this except maybe for reverting that commit entirely.
>
> Note that the drm_irq_[un]install() thing may be unnecessary, but I wasn't sure
> about that and surely wouldn't suggest doing that for 2.6.33.  Also it looks like
> some things from the freeze and thaw parts may be moved to the "low-level"
> suspend and resume parts, respectively, but that would require some
> i915_gem_* surgery I was too scared to do.
>
> Alan, please test, i915 guys, please review.
>
> Rafael
>   

The patch works very nicely on my eeepc.

Thanks
(and thanks again for all your hard work this cycle, and specifically 
for pointing me to the s2disk hang-fix)

Alan

Re: [PATCH] i915 / PM: Fix crash while aborting hibernation (Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk)

[Rafael J. Wysocki]

On Monday 08 February 2010, Alan Jenkins wrote:
> Rafael J. Wysocki wrote:
> > On Thursday 04 February 2010, Zhenyu Wang wrote:
> >   
> >> On 2010.02.03 23:44:41 +0100, Rafael J. Wysocki wrote:
> >>     
> >>> On Wednesday 03 February 2010, Alan Jenkins wrote:
> >>>       
> >>>> Hi
> >>>>
> >>>> I found this regression on my EeePC 701 with modesetting enabled.  When 
> >>>> I hibernate using s2disk, I can abort the hibernation by pressing the 
> >>>> backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).
> >>>>         
> >>> Yeah.
> >>>
> >>> To be honest, I knew that's going to happen, but didn't have the time to take
> >>> care of it.
> >>>
> >>> The problem is that i915 does literally _nothing_ in its .thaw() callback,
> >>> although it should at least reverse whatever .freeze() did to the hardware
> >>> (and memory allocations and so on), so that the adapter is functional
> >>> after creating the image.
> >>>
> >>> Fixing this requires some thought, though, because at the moment .freeze()
> >>> thinks it's .suspend(), which is not the case as this report clearly shows.
> >>> So, in fact i915_pci_suspend() has to be split into the .freeze() part and
> >>> the poweroff part cleanly and that's not  so simple (at least to me).
> >>>
> >>>       
> >> Right, I think that'll be more clean, stuff in i915_save/restore_state() need
> >> to be splited too, especially isolate stuff for mode setting and other device
> >> state, as what my original purpose for this is to remove extra mode setting 
> >> cycle in old behavior so not waste time for hibernate.
> >>     
> >
> > We can't really do that, because we'll need to restore the saved state at the
> > resume-from-hibernation stage.
> >
> > The appended patch fixes the issue for me, although it's been only tested
> > a little.  It sort of defeats the purpose of commit
> > cbda12d77ea590082edb6d30bd342a67ebc459e0, but I don't see any less invasive
> > way to fix this except maybe for reverting that commit entirely.
> >
> > Note that the drm_irq_[un]install() thing may be unnecessary, but I wasn't sure
> > about that and surely wouldn't suggest doing that for 2.6.33.  Also it looks like
> > some things from the freeze and thaw parts may be moved to the "low-level"
> > suspend and resume parts, respectively, but that would require some
> > i915_gem_* surgery I was too scared to do.
> >
> > Alan, please test, i915 guys, please review.
> >
> > Rafael
> >   
> 
> The patch works very nicely on my eeepc.

Great, thanks for testing.

> Thanks
> (and thanks again for all your hard work this cycle, and specifically 
> for pointing me to the s2disk hang-fix)

You're welcome. :-)

Rafael

Re: [PATCH] i915 / PM: Fix crash while aborting hibernation (Re: [linux-pm] [regression] "drm/i915: implement new pm ops" disables irq on aborted s2disk)

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On Sun, 7 Feb 2010 21:48:24 +0100, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Thursday 04 February 2010, Zhenyu Wang wrote:
> > On 2010.02.03 23:44:41 +0100, Rafael J. Wysocki wrote:
> > > On Wednesday 03 February 2010, Alan Jenkins wrote:
> > > > Hi
> > > > 
> > > > I found this regression on my EeePC 701 with modesetting enabled.  When 
> > > > I hibernate using s2disk, I can abort the hibernation by pressing the 
> > > > backspace key.  Doing so breaks X on 2.6.32-rc6 (but not 2.6.32).
> > > 
> > > Yeah.
> > > 
> > > To be honest, I knew that's going to happen, but didn't have the time to take
> > > care of it.
> > > 
> > > The problem is that i915 does literally _nothing_ in its .thaw() callback,
> > > although it should at least reverse whatever .freeze() did to the hardware
> > > (and memory allocations and so on), so that the adapter is functional
> > > after creating the image.
> > > 
> > > Fixing this requires some thought, though, because at the moment .freeze()
> > > thinks it's .suspend(), which is not the case as this report clearly shows.
> > > So, in fact i915_pci_suspend() has to be split into the .freeze() part and
> > > the poweroff part cleanly and that's not  so simple (at least to me).
> > > 
> > 
> > Right, I think that'll be more clean, stuff in i915_save/restore_state() need
> > to be splited too, especially isolate stuff for mode setting and other device
> > state, as what my original purpose for this is to remove extra mode setting 
> > cycle in old behavior so not waste time for hibernate.
> 
> We can't really do that, because we'll need to restore the saved state at the
> resume-from-hibernation stage.
> 
> The appended patch fixes the issue for me, although it's been only tested
> a little.  It sort of defeats the purpose of commit
> cbda12d77ea590082edb6d30bd342a67ebc459e0, but I don't see any less invasive
> way to fix this except maybe for reverting that commit entirely.
> 
> Note that the drm_irq_[un]install() thing may be unnecessary, but I wasn't sure
> about that and surely wouldn't suggest doing that for 2.6.33.  Also it looks like
> some things from the freeze and thaw parts may be moved to the "low-level"
> suspend and resume parts, respectively, but that would require some
> i915_gem_* surgery I was too scared to do.
> 
> Alan, please test, i915 guys, please review.
> 
> Rafael

Applied to for-linus.  Thanks!

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]