2010-02-17 18:47:13

by Pedro Ribeiro

[permalink] [raw]
Subject: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

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.

Reverting this commit fixes both issues.

My platform is Lenovo T400, G45 chipset.

lspci is attached, please let me know if you need more info.

Regards,
Pedro


Attachments:
lspci (13.73 kB)

2010-02-17 20:11:07

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

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.

Rafael

2010-02-17 20:52:17

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

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?

Rafael

2010-02-17 22:10:35

by Pedro Ribeiro

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

2010/2/17 Rafael J. Wysocki <[email protected]>:
> 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?
>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

The driver is modular.
And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
a difference.

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

Pedro

2010-02-17 22:20:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> 2010/2/17 Rafael J. Wysocki <[email protected]>:
> > 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 = {

2010-02-17 22:39:05

by Nigel Cunningham

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

Hi Rafael.

Rafael J. Wysocki wrote:
> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> 2010/2/17 Rafael J. Wysocki <[email protected]>:
>>> 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.

You're right. It shouldn't. I'm using exactly the same driver model
calls in exactly the same order, and not changing driver code at all.

Regards,

Nigel

2010-02-18 00:03:09

by Tino Keitel

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

On Wed, Feb 17, 2010 at 23:20:57 +0100, Rafael J. Wysocki wrote:

[...]

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

I got hangs at resume on my ThinkPad X61s with i965 graphics and
non-modular i915 driver. I got the hangs after suspend to RAM and
suspend to disk using TuxOnIce, but not every time. I reverted the
commit metioned by Pedro and wasn't able to reproduce the hang after
several suspend cycles.

I'll also try the attached patch.

Regards,
Tino

2010-02-18 00:29:50

by Pedro Ribeiro

[permalink] [raw]
Subject: Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

On 17 February 2010 22:20, Rafael J. Wysocki <[email protected]> wrote:
> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> 2010/2/17 Rafael J. Wysocki <[email protected]>:
>> > 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 = {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>

The patch fixes this issue for me.

Thanks for your help.

Pedro

2010-02-18 22:06:00

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

On Thursday 18 February 2010, Pedro Ribeiro wrote:
> On 17 February 2010 22:20, Rafael J. Wysocki <[email protected]> wrote:
> > On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> 2010/2/17 Rafael J. Wysocki <[email protected]>:
> >> > 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 = {
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
> The patch fixes this issue for me.
>
> Thanks for your help.

OK, thanks for testing, let's submit it appropriately.

Jesse, Eric, the appended patch fixes a very recent hibernate regression in
i915, please push it to Linus ASAP.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: i915 / PM: Fix hibernate regression caused by suspend/resume splitting

Commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
while aborting hibernation) attempted to fix a regression introduced
by commit cbda12d77ea590082edb6d30bd342a67ebc459e0 (drm/i915:
implement new pm ops for i915), but it went too far trying to split
the freeze/suspend and resume/thaw parts of the code. As a result,
it introduced another regression, which only is visible on some systems.

Fix the problem by merging i915_drm_suspend() with
i915_drm_freeze() and moving some code from i915_resume()
into i915_drm_thaw(), so that intel_opregion_free() and
intel_opregion_init() are also executed in the freeze and thaw code
paths, respectively.

Signed-off-by: Rafael J. Wysocki <[email protected]>
Reported-and-tested-by: Pedro Ribeiro <[email protected]>
---
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 = {

2010-02-19 06:39:39

by Tino Keitel

[permalink] [raw]
Subject: Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

On Thu, Feb 18, 2010 at 23:06:27 +0100, Rafael J. Wysocki wrote:

[...]

> Jesse, Eric, the appended patch fixes a very recent hibernate regression in
> i915, please push it to Linus ASAP.

FYI: I got no resume failure during normal usage after 9 suspend to RAM
and 8 suspend to disk cycles (using both suspend types alternately)
with the patch applied.

Regards,
Tino

2010-02-22 15:48:30

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting


Jesse, Eric, what's the status of this patch?

Should I take it directly (I'd really like an Ack) or is it queued up in
some tree waiting for me to pull? Or is there some reason not to have it,
despite it fixing things for some people?

Linus

2010-02-22 16:35:08

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

On Mon, 22 Feb 2010 07:47:53 -0800 (PST)
Linus Torvalds <[email protected]> wrote:

>
> Jesse, Eric, what's the status of this patch?
>
> Should I take it directly (I'd really like an Ack) or is it queued up in
> some tree waiting for me to pull? Or is there some reason not to have it,
> despite it fixing things for some people?

Hm, I thought Eric had applied that to his for-linus branch, but it
looks like that branch is empty now. Maybe Rafael posted an update
that Eric missed and so you just have a partial fix in your tree?

--
Jesse Barnes, Intel Open Source Technology Center