[PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Imre Deak]

commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
Author: Imre Deak <imre.deak@intel.com>
Date:   Thu Oct 23 19:23:26 2014 +0300

    drm/i915: add poweroff_late handler

introduced a regression on old platforms during hibernation. A workaround was
added in

commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
Author: Imre Deak <imre.deak@intel.com>
Date:   Mon Mar 2 13:04:41 2015 +0200

    drm/i915: gen4: work around hang during hibernation

using an explicit blacklist for the GENs/BIOS vendors where the issue was
reported. Later there we had reports of the same failure on platforms not on
this list.

To my best knowledge the correct thing to do is still to put the device to PCI
D3 state during hibernation, see [1] and [2] for the reasons. This also aligns
with our future plans to unify more the runtime and system suspend/resume
paths. Since an exact blacklist seems to be impractical (multiple GENs and
BIOS vendors are affected) apply the workaround on everything pre GEN6.

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-February/060710.html
[2] https://lkml.org/lkml/2015/6/22/274

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=95061
Reported-by: Ilya Tumaykin <itumaykin@gmail.com>
Reported-by: Dirk Griesbach <spamthis@freenet.de>
Reported-by: Pavel Machek <pavel@ucw.cz>
Reported-by: Mikko Rapeli <mikko.rapeli@iki.fi>
Reported-by: Paul Bolle <pebolle@tiscali.nl>
CC: stable@vger.kernel.org
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e44dc0d..1e675ff 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -664,15 +664,18 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
 
 	pci_disable_device(drm_dev->pdev);
 	/*
-	 * During hibernation on some GEN4 platforms the BIOS may try to access
+	 * During hibernation on some platforms the BIOS may try to access
 	 * the device even though it's already in D3 and hang the machine. So
 	 * leave the device in D0 on those platforms and hope the BIOS will
-	 * power down the device properly. Platforms where this was seen:
-	 * Lenovo Thinkpad X301, X61s
+	 * power down the device properly. The issue was seen on multiple old
+	 * GENs with different BIOS vendors, so having an explicit blacklist
+	 * is inpractical; apply the workaround on everything pre GEN6. The
+	 * platforms where the issue was seen:
+	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
+	 * Fujitsu FSC S7110
+	 * Acer Aspire 1830T
 	 */
-	if (!(hibernation &&
-	      drm_dev->pdev->subsystem_vendor == PCI_VENDOR_ID_LENOVO &&
-	      INTEL_INFO(dev_priv)->gen == 4))
+	if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
 		pci_set_power_state(drm_dev->pdev, PCI_D3hot);
 
 	return 0;
-- 
2.1.4

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Pavel Machek]

Hi!

> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Thu Oct 23 19:23:26 2014 +0300
> 
>     drm/i915: add poweroff_late handler
> 
> introduced a regression on old platforms during hibernation. A workaround was
> added in
> 
> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Mon Mar 2 13:04:41 2015 +0200
> 
>     drm/i915: gen4: work around hang during hibernation
> 
> using an explicit blacklist for the GENs/BIOS vendors where the issue was
> reported. Later there we had reports of the same failure on platforms not on
> this list.
> 
> To my best knowledge the correct thing to do is still to put the device to PCI
> D3 state during hibernation, see [1] and [2] for the reasons. This
> also aligns

Hmm, so the reasons according to you are:

> - ACPI mandates that the OSPM (the kernel in our case) puts all
>   devices
>   into D3 that are not wake-up sources (i915 is not) (Kudos to Ville
>   for
>   pointing this out)

Clearly, BIOS vendors did not read this, and pretty clearly Windows
do not follow the specs, either. That means that it is bad idea for us
to follow the specs, and trigger BIOS bugs. 

> - Embedded panels have a well defined shutdown sequence. We don't
>   have
>   any good reason to not follow this, in fact for some panels the
>   subsequent reinitialization could be problematic in case of a hard
>   power-off. (Thanks to Jani for this info)

Please cite concrete example. I have yet to see machine that would not
power up on forced power down. In fact, I argue that such machine
would be very broken, and that such machine does not exist. While we
have these real machines broken:

> +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> +	 * Fujitsu FSC S7110
> +	 * Acer Aspire 1830T

What makes you think that BIOS writers will do something different for
Gen6+ hardware? X301 is not that old.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Jani Nikula]

On Tue, 30 Jun 2015, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
>> Author: Imre Deak <imre.deak@intel.com>
>> Date:   Thu Oct 23 19:23:26 2014 +0300
>> 
>>     drm/i915: add poweroff_late handler
>> 
>> introduced a regression on old platforms during hibernation. A workaround was
>> added in
>> 
>> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
>> Author: Imre Deak <imre.deak@intel.com>
>> Date:   Mon Mar 2 13:04:41 2015 +0200
>> 
>>     drm/i915: gen4: work around hang during hibernation
>> 
>> using an explicit blacklist for the GENs/BIOS vendors where the issue was
>> reported. Later there we had reports of the same failure on platforms not on
>> this list.
>> 
>> To my best knowledge the correct thing to do is still to put the device to PCI
>> D3 state during hibernation, see [1] and [2] for the reasons. This
>> also aligns
>
> Hmm, so the reasons according to you are:
>
>> - ACPI mandates that the OSPM (the kernel in our case) puts all
>>   devices
>>   into D3 that are not wake-up sources (i915 is not) (Kudos to Ville
>>   for
>>   pointing this out)
>
> Clearly, BIOS vendors did not read this, and pretty clearly Windows
> do not follow the specs, either. That means that it is bad idea for us
> to follow the specs, and trigger BIOS bugs. 
>
>> - Embedded panels have a well defined shutdown sequence. We don't
>>   have
>>   any good reason to not follow this, in fact for some panels the
>>   subsequent reinitialization could be problematic in case of a hard
>>   power-off. (Thanks to Jani for this info)
>
> Please cite concrete example. I have yet to see machine that would not
> power up on forced power down. In fact, I argue that such machine
> would be very broken, and that such machine does not exist. While we
> have these real machines broken:

I was originally referring to reboots, which might not be applicable
here. Anyway, we have to go out of our way to handle that properly in
some cases:

commit 01527b3127997ef6370d5ad4fa25d96847fbf12a
Author: Clint Taylor <clinton.a.taylor@intel.com>
Date:   Mon Jul 7 13:01:46 2014 -0700

    drm/i915/vlv: T12 eDP panel timing enforcement during reboot

BR,
Jani.

>
>> +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
>> +	 * Fujitsu FSC S7110
>> +	 * Acer Aspire 1830T
>
> What makes you think that BIOS writers will do something different for
> Gen6+ hardware? X301 is not that old.
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Pavel Machek]

On Wed 2015-07-01 11:35:48, Jani Nikula wrote:
> On Tue, 30 Jun 2015, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
> >> Author: Imre Deak <imre.deak@intel.com>
> >> Date:   Thu Oct 23 19:23:26 2014 +0300
> >> 
> >>     drm/i915: add poweroff_late handler
> >> 
> >> introduced a regression on old platforms during hibernation. A workaround was
> >> added in
> >> 
> >> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
> >> Author: Imre Deak <imre.deak@intel.com>
> >> Date:   Mon Mar 2 13:04:41 2015 +0200
> >> 
> >>     drm/i915: gen4: work around hang during hibernation
> >> 
> >> using an explicit blacklist for the GENs/BIOS vendors where the issue was
> >> reported. Later there we had reports of the same failure on platforms not on
> >> this list.
> >> 
> >> To my best knowledge the correct thing to do is still to put the device to PCI
> >> D3 state during hibernation, see [1] and [2] for the reasons. This
> >> also aligns
> >
> > Hmm, so the reasons according to you are:
> >
> >> - ACPI mandates that the OSPM (the kernel in our case) puts all
> >>   devices
> >>   into D3 that are not wake-up sources (i915 is not) (Kudos to Ville
> >>   for
> >>   pointing this out)
> >
> > Clearly, BIOS vendors did not read this, and pretty clearly Windows
> > do not follow the specs, either. That means that it is bad idea for us
> > to follow the specs, and trigger BIOS bugs. 
> >
> >> - Embedded panels have a well defined shutdown sequence. We don't
> >>   have
> >>   any good reason to not follow this, in fact for some panels the
> >>   subsequent reinitialization could be problematic in case of a hard
> >>   power-off. (Thanks to Jani for this info)
> >
> > Please cite concrete example. I have yet to see machine that would not
> > power up on forced power down. In fact, I argue that such machine
> > would be very broken, and that such machine does not exist. While we
> > have these real machines broken:
> 
> I was originally referring to reboots, which might not be applicable
> here. Anyway, we have to go out of our way to handle that properly in
> some cases:
> 
> commit 01527b3127997ef6370d5ad4fa25d96847fbf12a
> Author: Clint Taylor <clinton.a.taylor@intel.com>
> Date:   Mon Jul 7 13:01:46 2014 -0700
> 
>     drm/i915/vlv: T12 eDP panel timing enforcement during reboot

So we now have single argument "ACPI specs says so". Unfortunately,
BIOSes can not read, and Windows does it the other way around...

What is special about gen6?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Ville Syrjälä]

On Tue, Jun 30, 2015 at 07:27:06PM +0200, Pavel Machek wrote:
> Hi!
> 
> > commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
> > Author: Imre Deak <imre.deak@intel.com>
> > Date:   Thu Oct 23 19:23:26 2014 +0300
> > 
> >     drm/i915: add poweroff_late handler
> > 
> > introduced a regression on old platforms during hibernation. A workaround was
> > added in
> > 
> > commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
> > Author: Imre Deak <imre.deak@intel.com>
> > Date:   Mon Mar 2 13:04:41 2015 +0200
> > 
> >     drm/i915: gen4: work around hang during hibernation
> > 
> > using an explicit blacklist for the GENs/BIOS vendors where the issue was
> > reported. Later there we had reports of the same failure on platforms not on
> > this list.
> > 
> > To my best knowledge the correct thing to do is still to put the device to PCI
> > D3 state during hibernation, see [1] and [2] for the reasons. This
> > also aligns
> 
> Hmm, so the reasons according to you are:
> 
> > - ACPI mandates that the OSPM (the kernel in our case) puts all
> >   devices
> >   into D3 that are not wake-up sources (i915 is not) (Kudos to Ville
> >   for
> >   pointing this out)
> 
> Clearly, BIOS vendors did not read this, and pretty clearly Windows
> do not follow the specs, either. That means that it is bad idea for us
> to follow the specs, and trigger BIOS bugs. 

It's not clear at all. From what I managed to find on the Internets,
Windows (at least recent versions) should put devices into D3.

> 
> > - Embedded panels have a well defined shutdown sequence. We don't
> >   have
> >   any good reason to not follow this, in fact for some panels the
> >   subsequent reinitialization could be problematic in case of a hard
> >   power-off. (Thanks to Jani for this info)
> 
> Please cite concrete example. I have yet to see machine that would not
> power up on forced power down. In fact, I argue that such machine
> would be very broken, and that such machine does not exist. While we
> have these real machines broken:
> 
> > +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> > +	 * Fujitsu FSC S7110
> > +	 * Acer Aspire 1830T
> 
> What makes you think that BIOS writers will do something different for
> Gen6+ hardware? X301 is not that old.

Thinkpad X1 Carbon (IVB) is perfectly happy with the D3, so clearly
something changed for Lenovo at least. And most machines (old and new)
have no problems whatsoever with the D3.

It might be interesting to see if hibernate even works in Windows
without some vendor driver on these machines.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Pavel Machek]

> > > - Embedded panels have a well defined shutdown sequence. We
        don't

> > >   have
> > >   any good reason to not follow this, in fact for some panels the
> > >   subsequent reinitialization could be problematic in case of a hard
> > >   power-off. (Thanks to Jani for this info)
> > 
> > Please cite concrete example. I have yet to see machine that would not
> > power up on forced power down. In fact, I argue that such machine
> > would be very broken, and that such machine does not exist. While we
> > have these real machines broken:
> > 
> > > +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> > > +	 * Fujitsu FSC S7110
> > > +	 * Acer Aspire 1830T
> > 
> > What makes you think that BIOS writers will do something different for
> > Gen6+ hardware? X301 is not that old.
> 
> Thinkpad X1 Carbon (IVB) is perfectly happy with the D3, so clearly
> something changed for Lenovo at least. And most machines (old and new)
> have no problems whatsoever with the D3.

Well, one machine being happy does not matter that much... as going to
D3 has no real benefits.

> It might be interesting to see if hibernate even works in Windows
> without some vendor driver on these machines.

I don't have windows nearby..
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Ville Syrjälä]

On Wed, Jul 01, 2015 at 11:51:27AM +0200, Pavel Machek wrote:
> > > > - Embedded panels have a well defined shutdown sequence. We
>         don't
> 
> > > >   have
> > > >   any good reason to not follow this, in fact for some panels the
> > > >   subsequent reinitialization could be problematic in case of a hard
> > > >   power-off. (Thanks to Jani for this info)
> > > 
> > > Please cite concrete example. I have yet to see machine that would not
> > > power up on forced power down. In fact, I argue that such machine
> > > would be very broken, and that such machine does not exist. While we
> > > have these real machines broken:
> > > 
> > > > +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> > > > +	 * Fujitsu FSC S7110
> > > > +	 * Acer Aspire 1830T
> > > 
> > > What makes you think that BIOS writers will do something different for
> > > Gen6+ hardware? X301 is not that old.
> > 
> > Thinkpad X1 Carbon (IVB) is perfectly happy with the D3, so clearly
> > something changed for Lenovo at least. And most machines (old and new)
> > have no problems whatsoever with the D3.
> 
>  Well, one machine being happy does not matter that much...

I said most machines, not one.

> as going to
> D3 has no real benefits.

Sure it does. Eventually we'll want to avoid resuming runtime suspended
devices when entering system suspend. For broken machines we'd need to
resume the GPU at that point.

> 
> > It might be interesting to see if hibernate even works in Windows
> > without some vendor driver on these machines.
> 
> I don't have windows nearby..

Me neither.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Pavel Machek]

On Wed 2015-07-01 13:53:31, Ville Syrj�l� wrote:
> On Wed, Jul 01, 2015 at 11:51:27AM +0200, Pavel Machek wrote:
> > > > > - Embedded panels have a well defined shutdown sequence. We
> >         don't
> > 
> > > > >   have
> > > > >   any good reason to not follow this, in fact for some panels the
> > > > >   subsequent reinitialization could be problematic in case of a hard
> > > > >   power-off. (Thanks to Jani for this info)
> > > > 
> > > > Please cite concrete example. I have yet to see machine that would not
> > > > power up on forced power down. In fact, I argue that such machine
> > > > would be very broken, and that such machine does not exist. While we
> > > > have these real machines broken:
> > > > 
> > > > > +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> > > > > +	 * Fujitsu FSC S7110
> > > > > +	 * Acer Aspire 1830T
> > > > 
> > > > What makes you think that BIOS writers will do something different for
> > > > Gen6+ hardware? X301 is not that old.
> > > 
> > > Thinkpad X1 Carbon (IVB) is perfectly happy with the D3, so clearly
> > > something changed for Lenovo at least. And most machines (old and new)
> > > have no problems whatsoever with the D3.
> > 
> >  Well, one machine being happy does not matter that much...
> 
> I said most machines, not one.
> 
> > as going to
> > D3 has no real benefits.
> 
> Sure it does. Eventually we'll want to avoid resuming runtime suspended
> devices when entering system suspend. For broken machines we'd need to
> resume the GPU at that point.

You want to optimize transition between suspend-to-RAM and
hibernation? No? I thought so.

So no benefits, 7 real, broken machines.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Jani Nikula]

On Wed, 01 Jul 2015, Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2015-07-01 13:53:31, Ville Syrjälä wrote:
>> On Wed, Jul 01, 2015 at 11:51:27AM +0200, Pavel Machek wrote:
>> > > > > - Embedded panels have a well defined shutdown sequence. We
>> >         don't
>> > 
>> > > > >   have
>> > > > >   any good reason to not follow this, in fact for some panels the
>> > > > >   subsequent reinitialization could be problematic in case of a hard
>> > > > >   power-off. (Thanks to Jani for this info)
>> > > > 
>> > > > Please cite concrete example. I have yet to see machine that would not
>> > > > power up on forced power down. In fact, I argue that such machine
>> > > > would be very broken, and that such machine does not exist. While we
>> > > > have these real machines broken:
>> > > > 
>> > > > > +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
>> > > > > +	 * Fujitsu FSC S7110
>> > > > > +	 * Acer Aspire 1830T
>> > > > 
>> > > > What makes you think that BIOS writers will do something different for
>> > > > Gen6+ hardware? X301 is not that old.
>> > > 
>> > > Thinkpad X1 Carbon (IVB) is perfectly happy with the D3, so clearly
>> > > something changed for Lenovo at least. And most machines (old and new)
>> > > have no problems whatsoever with the D3.
>> > 
>> >  Well, one machine being happy does not matter that much...
>> 
>> I said most machines, not one.
>> 
>> > as going to
>> > D3 has no real benefits.
>> 
>> Sure it does. Eventually we'll want to avoid resuming runtime suspended
>> devices when entering system suspend. For broken machines we'd need to
>> resume the GPU at that point.
>
> You want to optimize transition between suspend-to-RAM and
> hibernation? No? I thought so.
>
> So no benefits, 7 real, broken machines.

runtime suspended != suspend-to-RAM





> 									Pavel
>
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [Intel-gfx] [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[David Weinehall]

On Wed, Jul 01, 2015 at 02:35:24PM +0200, Pavel Machek wrote:
> On Wed 2015-07-01 13:53:31, Ville Syrj�l� wrote:
> > Sure it does. Eventually we'll want to avoid resuming runtime suspended
> > devices when entering system suspend. For broken machines we'd need to
> > resume the GPU at that point.
> 
> You want to optimize transition between suspend-to-RAM and
> hibernation? No? I thought so.
> 
> So no benefits, 7 real, broken machines.

Runtime suspend = things like S0ix and other forms of shallow sleep.
System suspend = suspend-to-RAM.

It's not about hibernate (though it'd of course nice if a transition
from suspend to hibernate because of low battery won't result in the
device consuming so much battery during the intermediate wakeup that
the device shuts down rather than hibernates).


Kind regards, David

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Mikko Rapeli]

On Tue, Jun 30, 2015 at 05:06:47PM +0300, Imre Deak wrote:
> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Thu Oct 23 19:23:26 2014 +0300
> 
>     drm/i915: add poweroff_late handler
> 
> introduced a regression on old platforms during hibernation. A workaround was
> added in
> 
> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Mon Mar 2 13:04:41 2015 +0200
> 
>     drm/i915: gen4: work around hang during hibernation
> 
> using an explicit blacklist for the GENs/BIOS vendors where the issue was
> reported. Later there we had reports of the same failure on platforms not on
> this list.
> 
> To my best knowledge the correct thing to do is still to put the device to PCI
> D3 state during hibernation, see [1] and [2] for the reasons. This also aligns
> with our future plans to unify more the runtime and system suspend/resume
> paths. Since an exact blacklist seems to be impractical (multiple GENs and
> BIOS vendors are affected) apply the workaround on everything pre GEN6.
> 
> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-February/060710.html
> [2] https://lkml.org/lkml/2015/6/22/274
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=95061
> Reported-by: Ilya Tumaykin <itumaykin@gmail.com>
> Reported-by: Dirk Griesbach <spamthis@freenet.de>
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Reported-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> Reported-by: Paul Bolle <pebolle@tiscali.nl>
> CC: stable@vger.kernel.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Tested-by: Mikko Rapeli <mikko.rapeli@iki.fi>

on top of v4.1.2.

Could this or any better alternative please be sent Linus and then to stable?

-Mikko

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e44dc0d..1e675ff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -664,15 +664,18 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>  
>  	pci_disable_device(drm_dev->pdev);
>  	/*
> -	 * During hibernation on some GEN4 platforms the BIOS may try to access
> +	 * During hibernation on some platforms the BIOS may try to access
>  	 * the device even though it's already in D3 and hang the machine. So
>  	 * leave the device in D0 on those platforms and hope the BIOS will
> -	 * power down the device properly. Platforms where this was seen:
> -	 * Lenovo Thinkpad X301, X61s
> +	 * power down the device properly. The issue was seen on multiple old
> +	 * GENs with different BIOS vendors, so having an explicit blacklist
> +	 * is inpractical; apply the workaround on everything pre GEN6. The
> +	 * platforms where the issue was seen:
> +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> +	 * Fujitsu FSC S7110
> +	 * Acer Aspire 1830T
>  	 */
> -	if (!(hibernation &&
> -	      drm_dev->pdev->subsystem_vendor == PCI_VENDOR_ID_LENOVO &&
> -	      INTEL_INFO(dev_priv)->gen == 4))
> +	if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
>  		pci_set_power_state(drm_dev->pdev, PCI_D3hot);
>  
>  	return 0;
> -- 
> 2.1.4
> 

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Mikko Rapeli]

Please, please merge this patch already. Without it hibernation poweroff
is broken for several users. There were some doubts raised over the
approach on lkml review but Imre as maintainer thinks this is the right
thing to do and users like me need this patch.

Tested again with Thinkpad T60 and kernel version 4.1.6.

Tested-by: Mikko Rapeli <mikko.rapeli@iki.fi>

-Mikko

On Tue, Jun 30, 2015 at 05:06:47PM +0300, Imre Deak wrote:
> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Thu Oct 23 19:23:26 2014 +0300
> 
>     drm/i915: add poweroff_late handler
> 
> introduced a regression on old platforms during hibernation. A workaround was
> added in
> 
> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
> Author: Imre Deak <imre.deak@intel.com>
> Date:   Mon Mar 2 13:04:41 2015 +0200
> 
>     drm/i915: gen4: work around hang during hibernation
> 
> using an explicit blacklist for the GENs/BIOS vendors where the issue was
> reported. Later there we had reports of the same failure on platforms not on
> this list.
> 
> To my best knowledge the correct thing to do is still to put the device to PCI
> D3 state during hibernation, see [1] and [2] for the reasons. This also aligns
> with our future plans to unify more the runtime and system suspend/resume
> paths. Since an exact blacklist seems to be impractical (multiple GENs and
> BIOS vendors are affected) apply the workaround on everything pre GEN6.
> 
> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-February/060710.html
> [2] https://lkml.org/lkml/2015/6/22/274
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=95061
> Reported-by: Ilya Tumaykin <itumaykin@gmail.com>
> Reported-by: Dirk Griesbach <spamthis@freenet.de>
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Reported-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> Reported-by: Paul Bolle <pebolle@tiscali.nl>
> CC: stable@vger.kernel.org
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e44dc0d..1e675ff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -664,15 +664,18 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>  
>  	pci_disable_device(drm_dev->pdev);
>  	/*
> -	 * During hibernation on some GEN4 platforms the BIOS may try to access
> +	 * During hibernation on some platforms the BIOS may try to access
>  	 * the device even though it's already in D3 and hang the machine. So
>  	 * leave the device in D0 on those platforms and hope the BIOS will
> -	 * power down the device properly. Platforms where this was seen:
> -	 * Lenovo Thinkpad X301, X61s
> +	 * power down the device properly. The issue was seen on multiple old
> +	 * GENs with different BIOS vendors, so having an explicit blacklist
> +	 * is inpractical; apply the workaround on everything pre GEN6. The
> +	 * platforms where the issue was seen:
> +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
> +	 * Fujitsu FSC S7110
> +	 * Acer Aspire 1830T
>  	 */
> -	if (!(hibernation &&
> -	      drm_dev->pdev->subsystem_vendor == PCI_VENDOR_ID_LENOVO &&
> -	      INTEL_INFO(dev_priv)->gen == 4))
> +	if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
>  		pci_set_power_state(drm_dev->pdev, PCI_D3hot);
>  
>  	return 0;
> -- 
> 2.1.4
> 

Re: [PATCH] drm/i915: apply the PCI_D0/D3 hibernation workaround everywhere on pre GEN6

[Jani Nikula]

On Sat, 29 Aug 2015, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> Please, please merge this patch already. Without it hibernation poweroff
> is broken for several users. There were some doubts raised over the
> approach on lkml review but Imre as maintainer thinks this is the right
> thing to do and users like me need this patch.
>
> Tested again with Thinkpad T60 and kernel version 4.1.6.
>
> Tested-by: Mikko Rapeli <mikko.rapeli@iki.fi>

Pushed to drm-intel-next-fixes.

BR,
Jani.

>
> -Mikko
>
> On Tue, Jun 30, 2015 at 05:06:47PM +0300, Imre Deak wrote:
>> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d
>> Author: Imre Deak <imre.deak@intel.com>
>> Date:   Thu Oct 23 19:23:26 2014 +0300
>> 
>>     drm/i915: add poweroff_late handler
>> 
>> introduced a regression on old platforms during hibernation. A workaround was
>> added in
>> 
>> commit ab3be73fa7b43f4c3648ce29b5fd649ea54d3adb
>> Author: Imre Deak <imre.deak@intel.com>
>> Date:   Mon Mar 2 13:04:41 2015 +0200
>> 
>>     drm/i915: gen4: work around hang during hibernation
>> 
>> using an explicit blacklist for the GENs/BIOS vendors where the issue was
>> reported. Later there we had reports of the same failure on platforms not on
>> this list.
>> 
>> To my best knowledge the correct thing to do is still to put the device to PCI
>> D3 state during hibernation, see [1] and [2] for the reasons. This also aligns
>> with our future plans to unify more the runtime and system suspend/resume
>> paths. Since an exact blacklist seems to be impractical (multiple GENs and
>> BIOS vendors are affected) apply the workaround on everything pre GEN6.
>> 
>> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-February/060710.html
>> [2] https://lkml.org/lkml/2015/6/22/274
>> 
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=95061
>> Reported-by: Ilya Tumaykin <itumaykin@gmail.com>
>> Reported-by: Dirk Griesbach <spamthis@freenet.de>
>> Reported-by: Pavel Machek <pavel@ucw.cz>
>> Reported-by: Mikko Rapeli <mikko.rapeli@iki.fi>
>> Reported-by: Paul Bolle <pebolle@tiscali.nl>
>> CC: stable@vger.kernel.org
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index e44dc0d..1e675ff 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -664,15 +664,18 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
>>  
>>  	pci_disable_device(drm_dev->pdev);
>>  	/*
>> -	 * During hibernation on some GEN4 platforms the BIOS may try to access
>> +	 * During hibernation on some platforms the BIOS may try to access
>>  	 * the device even though it's already in D3 and hang the machine. So
>>  	 * leave the device in D0 on those platforms and hope the BIOS will
>> -	 * power down the device properly. Platforms where this was seen:
>> -	 * Lenovo Thinkpad X301, X61s
>> +	 * power down the device properly. The issue was seen on multiple old
>> +	 * GENs with different BIOS vendors, so having an explicit blacklist
>> +	 * is inpractical; apply the workaround on everything pre GEN6. The
>> +	 * platforms where the issue was seen:
>> +	 * Lenovo Thinkpad X301, X61s, X60, T60, X41
>> +	 * Fujitsu FSC S7110
>> +	 * Acer Aspire 1830T
>>  	 */
>> -	if (!(hibernation &&
>> -	      drm_dev->pdev->subsystem_vendor == PCI_VENDOR_ID_LENOVO &&
>> -	      INTEL_INFO(dev_priv)->gen == 4))
>> +	if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
>>  		pci_set_power_state(drm_dev->pdev, PCI_D3hot);
>>  
>>  	return 0;
>> -- 
>> 2.1.4
>> 

-- 
Jani Nikula, Intel Open Source Technology Center