Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
> >> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> > Hi,
> >> >
> >> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> >> > while aborting hibernation) introduced two new issues which were not
> >> > present in 2.6.33-rc7:
> >> >
> >> > - every second resume from hibernate results in a blank screen
> >> > - the annoying flash at the end of atomic copy/restore during the
> >> > hibernate process is back (present in kernels < 2.6.33)
> >> >
> >> > The first issue is serious, the second is just an annoyance.
> >>
> >> The second one is an expected price of fixing the aborted hibernation
> >> regression.
> >>
> >> The first one shouldn't happen, though.
> >>
> >> I'll see if I can reproduce that locally.
> >
> > No, I can't.
> >
> > Is the driver compiled directly into the kernel or modular?
> 
> The driver is modular.
> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
> a difference.

It shouldn't in fact, although I'm not sure.

> However, every since I reverted that commit I've done 10 test
> hibernations and no hang so far.

First, please try if you can reproduce it with non-modular driver.

Second, please check if the appended patch helps.

Rafael

---
 drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 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
@@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 
 static int i915_drm_freeze(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	pci_save_state(dev->pdev);
 
 	/* If KMS is active, we do the leavevt stuff here */
@@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
 
 	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;
+
+	return 0;
 }
 
 static int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
 	if (error)
 		return error;
 
-	i915_drm_suspend(dev);
-
 	if (state.event == PM_EVENT_SUSPEND) {
 		/* Shut down the device */
 		pci_disable_device(dev->pdev);
@@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
+	i915_restore_state(dev);
+
+	intel_opregion_init(dev, 1);
+
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
@@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
 
 	pci_set_master(dev->pdev);
 
-	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
-
 	return i915_drm_thaw(dev);
 }
 
@@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
 	if (error)
 		return error;
 
-	i915_drm_suspend(drm_dev);
-
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
 
@@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
 {
 	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;
+	return i915_drm_freeze(drm_dev);
 }
 
 const struct dev_pm_ops i915_pm_ops = {