Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.

[Jesse Barnes <jesse.barnes@intel.com>]

On Thursday, February 21, 2008 5:13 pm Jesse Barnes wrote:
> On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
> > On Friday, 22 of February 2008, Linus Torvalds wrote:
> > > On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
> > > > -	if (state.event == PM_EVENT_SUSPEND) {
> > > > +	if (state.event == PM_EVENT_SUSPEND && !in_hibernation_power_off())
> > > > {
> > >
> > > I don't understand why hibernation just doesn't use a
> > > PM_EVENT_HIBERNATE, and be done with it?
> > >
> > > Why should it be called PM_EVENT_SUSPEND when it isn't?
> > >
> > > Adding some external global variables is absolutely the wrong way to
> > > fix this.
> > >
> > > It's not even like there are very many drivers who actually care about
> > > "state.event" anyway: a 'git grep' returns just 35 users in the whole
> > > tree, so if this was done this ugly way just to avoid double-chcking
> > > the other cases that compare against PM_EVENT_SUSPEND, then it really
> > > wasn't worth it.
> >
> > Please relax, we're debugging the thing right now and the patch doesn't
> > even seem to help on the other affected box.
>
> Actually, looks like I forgot to reboot between tests (just rmmod'd &
> modprobed i915), your patch actually does work.
>
> However, making new PM event messages might be a good thing anyway,
> assuming Linus takes it for 2.6.25, since it should make the migration to
> ->hibernate callbacks easier.

Rafael, I'd actually prefer these changes to the i915 driver.  One is to avoid 
the "green screen" problem and the other is to actually save state at 
hibernate time in case we don't do a POST coming out of S4 (probably not 
common but hey).

Jesse

Make sure hibernation works by not shutting down the video device during 
hibernation power off.  This is important because later stages of the 
hibernation cycle end up touching the video device, which may cause a hang if 
it was disabled early on.  Also make sure the restoration correctly restores 
the AR registers by flipping the ARX register into index mode before doing 
anything.

Depends on Rafael's patch which exports hibernation state to drivers.

Signed-off-by:  Jesse Barnes <jesse.barnes@intel.com>

diff --git a/drivers/char/drm/i915_drv.c b/drivers/char/drm/i915_drv.c
index 35758a6..5e73869 100644
--- a/drivers/char/drm/i915_drv.c
+++ b/drivers/char/drm/i915_drv.c
@@ -27,6 +27,7 @@
  *
  */
 
+#include <linux/suspend.h>
 #include "drmP.h"
 #include "drm.h"
 #include "i915_drm.h"
@@ -222,6 +223,7 @@ static void i915_restore_vga(struct drm_device *dev)
 			   dev_priv->saveGR[0x18]);
 
 	/* Attribute controller registers */
+	inb(st01);
 	for (i = 0; i < 20; i++)
 		i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
 	inb(st01); /* switch back to index mode */
@@ -364,8 +366,8 @@ static int i915_suspend(struct drm_device *dev)
 	i915_save_vga(dev);
 
 	/* Shut down the device */
-	pci_disable_device(dev->pdev);
-	pci_set_power_state(dev->pdev, PCI_D3hot);
+	if (!in_hibernation_power_off())
+		pci_set_power_state(dev->pdev, PCI_D3hot);
 
 	return 0;
 }