Adventures in fixing suspend/resume on a HP x2 Detachable 10-p0XX
I got contacted by a user with a HP X2 10 p018wm 2-in-1 about the device waking up 10-60 seconds after suspend. I have access to a HP X2 10 p002nd myself which in essence is the same HW and I managed to reproduce the problem there. This is when the fun started:
1. There were a whole bunch of ACPI related errors in dmesg. It turns out that these affect almost all HP laptop models and we have a multiple bugs open for this. Debugging these pointed to the hp-wmi driver. I wrote 2 patches fixes 2 different kind of errors and submitted these upstream. Unfortunately this does not help with the suspend/resume issue, but it does fix all those errors people have been complaining about :)
2. I noticed some weird messages in dmesg with look like a PCI bus re-enumeration is started during suspend when suspending by closing the lid and then the re-enumeration continues after resume. This turns out to be triggered by this piece of buggy AML code which
is used for monitor hotplug notification on gfx state changes (the i915 driver ACPI opregion also tracks the lid state for some reason):
Method (GNOT, 2, NotSerialized)
{
...
CEVT = Arg0
CSTS = 0x03
If (((CHPD == Zero) && (Arg1 == Zero)))
{
If (((OSYS > 0x07D0) || (OSYS < 0x07D6)))
{
Notify (PCI0, Arg1)
}
Else
{
Notify (GFX0, Arg1)
}
}
...
}
Notice how "If (((OSYS > 0x07D0) || (OSYS < 0x07D6)))" is always true, the condition is broken the "||" clearly should have been a "&&" this is causing the code to send a hotplug notify to the PCI root instead of to the gfx card, triggering a re-enumeration. Doing a grep for this on my personal DSDT collection shows that 55 of the 93 DSDTs in my collection have this issue!
Luckily this can be easily fixed by setting CHPD to 1 in the i915 driver, which is something which we should do anyways according to the
opregion documentation. So I wrote a patch doing this and submitted it upstream. Unfortunately this also does not help with the suspend/resume issue.
3. So the actual spurious wakeups are caused by HP using an external embedded controller (EC) on the "legacy-free" platform which they use for these laptops. Since these are not designed to use an external EC they lack the standard interface for this, so HP has hooked the EC up over I2C and using an ACPI GPIO event handler as EC interrupt.
These devices use suspend2idle (s2idle) instead of good old firmware handled S3, so the EC stays active during suspend. It does some housekeeping work which involves a round-trip through the AML code every minute. Normally EC wakeups are ignored durin s2idle by some special handling in the kernel, but this is only done for ECs using the standardized ACPI EC interface, not for this bolted on the
side model. I've started a discussion on maybe extending our ACPI event handling to deal with this special case.
For now as a workaround I ended up writing 2 more patches to allow blacklisting wakeup by ACPI GPIO event handlers on select models. This breaks wakeup by opening the LID, the user needs to wake the laptop with the powerbutton. But at least the laptop will stay suspended now.
1. There were a whole bunch of ACPI related errors in dmesg. It turns out that these affect almost all HP laptop models and we have a multiple bugs open for this. Debugging these pointed to the hp-wmi driver. I wrote 2 patches fixes 2 different kind of errors and submitted these upstream. Unfortunately this does not help with the suspend/resume issue, but it does fix all those errors people have been complaining about :)
2. I noticed some weird messages in dmesg with look like a PCI bus re-enumeration is started during suspend when suspending by closing the lid and then the re-enumeration continues after resume. This turns out to be triggered by this piece of buggy AML code which
is used for monitor hotplug notification on gfx state changes (the i915 driver ACPI opregion also tracks the lid state for some reason):
Method (GNOT, 2, NotSerialized)
{
...
CEVT = Arg0
CSTS = 0x03
If (((CHPD == Zero) && (Arg1 == Zero)))
{
If (((OSYS > 0x07D0) || (OSYS < 0x07D6)))
{
Notify (PCI0, Arg1)
}
Else
{
Notify (GFX0, Arg1)
}
}
...
}
Notice how "If (((OSYS > 0x07D0) || (OSYS < 0x07D6)))" is always true, the condition is broken the "||" clearly should have been a "&&" this is causing the code to send a hotplug notify to the PCI root instead of to the gfx card, triggering a re-enumeration. Doing a grep for this on my personal DSDT collection shows that 55 of the 93 DSDTs in my collection have this issue!
Luckily this can be easily fixed by setting CHPD to 1 in the i915 driver, which is something which we should do anyways according to the
opregion documentation. So I wrote a patch doing this and submitted it upstream. Unfortunately this also does not help with the suspend/resume issue.
3. So the actual spurious wakeups are caused by HP using an external embedded controller (EC) on the "legacy-free" platform which they use for these laptops. Since these are not designed to use an external EC they lack the standard interface for this, so HP has hooked the EC up over I2C and using an ACPI GPIO event handler as EC interrupt.
These devices use suspend2idle (s2idle) instead of good old firmware handled S3, so the EC stays active during suspend. It does some housekeeping work which involves a round-trip through the AML code every minute. Normally EC wakeups are ignored durin s2idle by some special handling in the kernel, but this is only done for ECs using the standardized ACPI EC interface, not for this bolted on the
side model. I've started a discussion on maybe extending our ACPI event handling to deal with this special case.
For now as a workaround I ended up writing 2 more patches to allow blacklisting wakeup by ACPI GPIO event handlers on select models. This breaks wakeup by opening the LID, the user needs to wake the laptop with the powerbutton. But at least the laptop will stay suspended now.