Incorrect resume from suspend on HP ENVY x360

Summary:

On my machine, when resuming from suspend, the system usually wakes up, but the power LED does not come back on, certain keys like brightness controls do not work, and the fan never spins up, even under load. Occasionally, I also get a crash on resume where the screen stays black, sometimes with the backlight turning on after a while for just a second.

Hardware description:

• Model: HP ENVY x360 13z-ay000
• CPU: Ryzen 4500U

System information:

• BIOS revision: F.20
• Distro: NixOS 21.11 (unstable)
• Kernel version: 5.14-rc7 (unmodified)

Attachments

• ACPI dump
• Log including successful and failed resume

CC @superm1

0001-ACPI-PM-s2idle-Adjust-methods-used-in-another-AMD-co.patch

Here is a patch on top of 5.14-rc7 you can try which will disable the Microsoft method explicitly.

While this patch helps, it is actually what seems to prevent resuming with the lid switch! I am going to work on ACPI debugging under Windows today to help figure this out.

Are you sure it's explicitly this patch that caused it, with vanilla 5.14-rc7 the lid switch works before that? If so, can you please check /sys/power/pm_wakeup_irq in the working case and check /proc/interrupts to what it matches to?

Actually nevermind that, turns out the machine never even enters suspend after closing the lid switch if I start in the broken resume state on vanilla rc7. Did not realize that, as in that situation there is no power LED to indicate anything.

OK. In that case, let's see if the other idea I had does help.

I wonder if there is some bios date cutoff for determining what method to use?

If so that's pretty risky, you're one BIOS spin for a security vulnerability away from breaking the logic. I think if this commit ends up being problematic and we have to revert it we should instead go into quirks.

I just had another thought - what if we're supposed to be calling both methods if they're present? That might explain the low level of objects in the Microsoft method. @rytone

Another thing to try is this:

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index fbdbef0ab552..052ea386de5d 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -456,7 +456,8 @@ int acpi_s2idle_prepare_late(void)
                                lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY,
                                lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-       } else if (acpi_s2idle_vendor_amd()) {
+       }
+       if (acpi_s2idle_vendor_amd()) {
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD,
                                lps0_dsm_func_mask, lps0_dsm_guid);
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD,
@@ -483,7 +484,8 @@ void acpi_s2idle_restore_early(void)
                                lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON,
                                lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
-       } else if (acpi_s2idle_vendor_amd()) {
+       }
+       if (acpi_s2idle_vendor_amd()) {
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD,
                                lps0_dsm_func_mask, lps0_dsm_guid);
                acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD,

That patch also does the trick. At this point I think we may need to find other machines to test these patches with since there are multiple approaches that could be taken. I also see another option of falling back to the AMD methods on a per-function basis, so on my machine the Microsoft function 7 would be used, but then the rest would use the AMD method. Hopefully that makes sense, as I don't really have any experience with this ACPI stuff.

I should also add that the other issues (cannot resume with lid switch, occasional crash on resume) still stand with that patch.

That patch also does the trick. At this point I think we may need to find other machines to test these patches with since there are multiple approaches that could be taken. I also see another option of falling back to the AMD methods on a per-function basis, so on my machine the Microsoft function 7 would be used, but then the rest would use the AMD method. Hopefully that makes sense, as I don't really have any experience with this ACPI stuff.

Per-function basis I can say is very likely not the right way to go. That's monkey patching and won't scale. It should be call AMD's method, Microsoft's method or both methods.

I agree, it will need some wider testing on the impacts which of these approaches makes more sense.

I should also add that the other issues (cannot resume with lid switch, occasional crash on resume) still stand with that patch.

If you turn off your lid for going into sleep and waking up, can you please check whether acpi_listen is seeing events? It seems that your EC should be notifying the lid for updates and it should be marked as a wake device.

acpi_listen does see lid events.

And I recall you said that reverting 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 also helped?

Yes, though I did not test if it caused any breakage with regards to plugging in AC power.

If you can better characterize that, it would be good to know. Something we can explore doing is create another driver that only loads on systems that need it and marks them to use EC for wakeup.

Just tested with 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 reverted again, and suspend/resume was unaffected by AC power in any way.

Another HP Envy x360 13-ay0000 owner here, and my unit's CPU is 4700U. I can confirm that on Linux 5.14.0, when the diff that causes both methods to be called and the reversed patch of 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 are applied, the laptop wakes up normally (power LED lights up, fn keys work, fan can spin, etc.), and lid opening also works.

0001-ACPI-PM-s2idle-Run-both-AMD-and-Microsoft-methods-if.patch Attached next version of the patch. I'd like 3 things if @rytone and @Leo3418 can help. Test this new patch w/ reverting 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36.

1. Let me know if that works.
2. Let me know what wake source /sys/power/pm_wakeup_irq shows for the lid and share /proc/interrupts
3. Check if https://lore.kernel.org/linux-gpio/20210831120613.1514899-1-Basavaraj.Natikar@amd.com/T/#t instead of reverting 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 helps you for lid events as wake source perhaps?

The new patch for running both AMD and Microsoft methods does work. /sys/power/pm_wakeup_irq is 1 when waking from the lid, which corresponds to i8042. Here is the full listing from /proc/interrupts. Finally, the patch from https://lore.kernel.org/linux-gpio/20210831120613.1514899-1-Basavaraj.Natikar@amd.com/T/#t does not seem to have any effect, and lid wakeup does not work with it.

Is lid wakeup always 1? Or is it just the first time? I was expecting it to be 9 for a lid event if the GPIO wakeup doesn't do anything.

Just completed my testing and I had the same findings as @rytone. report.txt

Is lid wakeup always 1? Or is it just the first time? I was expecting it to be 9 for a lid event if the GPIO wakeup doesn't do anything.

On a single occasion, /sys/power/pm_wakeup_irq showed 9 to me, but right now I am not able to reproduce it.

Thanks guys. Is it 1 every other time?

Is it 1 every other time?

Not quite. It might be always 1, or it can be interweaving 1 and 9. I have reproduced 9 again but cannot determine the exact factor that affects the value of /sys/power/pm_wakeup_irq. I guess one possible factor is the duration of s2idle state, because 9's tend to occur when I wake up my laptop immediately after it goes to sleep, but if I leave the computer asleep for a minute or two, it can change to 1. And once it becomes 1, it does not necessarily go back to 9 even after I try to suspend and wake up the laptop again. To me, the value of /sys/power/pm_wakeup_irq seems undeterministic.

I tried reverting that commit on my hand with 5.14 final and that other fix you guys tested, but it definitely still causes problems on some other Lenovo laptops. My thought is maybe we can make hp-wmi cover wakeups for this. Can you please disable suspend on lid, and then check if this shows messages to the logs when you close/open it?

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 027a1467d009..0d26762cba1d 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -623,6 +623,7 @@ static void hp_wmi_notify(u32 value, void *context)
        case HPWMI_LOCK_SWITCH:
                break;
        case HPWMI_LID_SWITCH:
+               pr_info("Lid switch event found\n");
                break;
        case HPWMI_SCREEN_ROTATION:
                break;

and FYI I sent up that patch you guys tested, if you can please add your Tested-By: clauses: https://lore.kernel.org/linux-acpi/20210901142111.16891-1-mario.limonciello@amd.com/T/#u

A little off topic for this issue but I'm not sure where else to report it, these two patches:

• https://gitlab.freedesktop.org/drm/amd/uploads/a723968252764149d7c21178e04a27b8/0001-ACPI-PM-s2idle-Run-both-AMD-and-Microsoft-methods-if.patch
• https://lore.kernel.org/linux-gpio/20210831120613.1514899-1-Basavaraj.Natikar@amd.com/T/#t

work extremely well on my ASUS Zephyrus G15 (GA503QR.) The lid switch has been nonfunctional on this machine since the earlier commit disabling EC GPE wakeup on AMD. Wake source is consistently the pinctrl_amd interrupt when opening the lid, no issues with plugging/unplugging power causing spurious wakeups or other unexpected behavior.

If I can figure out how to work public-inbox before it's too late I'll add a Tested-By clause for the ACPI patch.

@smbruce that's great to hear! The ACPI PM one is already queued up in the linux-pm branch and should make 5.15. The GPIO one is still waiting for feedback. If you can respond with that clause there that will be great.

Can you please disable suspend on lid, and then check if this shows messages to the logs when you close/open it?

I applied the diff and built a kernel but did not see that message when I closed the lid with suspend on lid was disabled.

OK. So unfortunately can't be a way to get the message through that means and this will require some more thoughts.

mentioned in commit agd5f/linux@aa9bb3d3

I've tried applying agd5f/linux@aa9bb3d3 on top of https://gitlab.com/cki-project/kernel-ark/-/tree/fedora-5.14 (at v5.14.1) and when resuming works, the LEDs and fan come back on as with my previous BIOS revision (F.16).

That being said, occasional resume failures (black screen; unresponsiveness, even to SysRq magic; but working LEDs) still occur. Was that supposed to be fixed by reverting https://gitlab.com/cki-project/kernel-ark/-/commit/7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 (which I'd forgotten) or is that still an open issue?

If so, is this the right issue to report a log of the resume failure?

P.S.: HP Envy x360 13-ay0278ng (4700U)

P.P.S.: acpi.ec_no_wakeup=1 still in my kernel cmdline

P.P.P.S.: Apparently, #1629 (closed) is the right place for this. I'm sorry for asking now; I guess I must've seen 1629, gotten it mixed up with 1691 and paid it no further mind.

I've tried applying agd5f/linux@aa9bb3d3 on top of https://gitlab.com/cki-project/kernel-ark/-/tree/fedora-5.14 (at v5.14.1) and when resuming works, the LEDs and fan come back on as with my previous BIOS revision (F.16).

Glad to hear no regressions from it.

That being said, occasional resume failures (black screen; unresponsiveness, even to SysRq magic; but working LEDs) still occur. Was that supposed to be fixed by reverting https://gitlab.com/cki-project/kernel-ark/-/commit/7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 (which I'd forgotten) or is that still an open issue?

Reverting that commit will help fix lid based wakeup for some designs, but it causes failure to resume on others.

If so, is this the right issue to report a log of the resume failure?

No, please keep that in #1629 (closed) or your own new issue if it's different.

P.S.: HP Envy x360 13-ay0278ng (4700U)

Interesting you didn't have same failure as @rytone. Maybe because you're on the older BIOS F.16 and they are on F.20.

P.P.S.: acpi.ec_no_wakeup=1 still in my kernel cmdline

You don't need this with 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 in place. It is the same thing as that commit.

P.P.P.S.: Apparently, #1629 (closed) is the right place for this. I'm sorry for asking now; I guess I must've seen 1629, gotten it mixed up with 1691 and paid it no further mind.

There is a hack on there that increases the time and retry cycles. If that helps you, it would be good to bring a discussion with the rest of the maintainers on platform-x86 mailing list to figure out what else is missing.

Glad to hear no regressions from it.

Interesting you didn't have same failure as @rytone. Maybe because you're on the older BIOS F.16 and they are on F.20.

I'm sorry for expressing it not clearly enough, but my previous revision was F.16 and resuming worked as expected (with the occasional 1629 hiccup) without agd5f/linux@aa9bb3d3. Upgrading to F.20 (which I'm now on) led me to this here issue and trying the aforementioned patch. I never tried the patch on F.16 because I didn't know about it before upgrading to F.20. I therefore can't make any statements about regressions that may come up on F.16.

I also can't discern what failure of @rytone's you mean; in this thread, I can only see them having issues with wake from lid-opening but that doesn't work for me either. I don't mind that, though, and someone in #1230 (closed) (maybe even you? I'm not opening that issue anymore, as it takes ages to load) told me that it was expected behaviour with acpi.ec_no_wakeup=1 anyway, so I didn't mention it.

You don't need this with 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 in place. It is the same thing as that commit.

I'll leave the kernel option in place, as I can't see any negative effects and also don't want to risk spurious wake-ups in case the commit gets reverted upstream without me noticing. This seems like a likely scenario, as you've mentioned it breaking resumes in some cases, so I'll stay on the safe side for now.

There is a hack on there that increases the time and retry cycles. If that helps you, it would be good to bring a discussion with the rest of the maintainers on platform-x86 mailing list to figure out what else is missing.

Neat, I'll apply that once I'm home and give feedback over there.

Thank you so much for your patience!

I'm sorry for expressing it not clearly enough, but my previous revision was F.16 and resuming worked as expected (with the occasional 1629 hiccup) without agd5f/linux@aa9bb3d3. Upgrading to F.20 (which I'm now on) led me to this here issue and trying the aforementioned patch. I never tried the patch on F.16 because I didn't know about it before upgrading to F.20. I therefore can't make any statements about regressions that may come up on F.16.

Ahhh, that makes more sense and it's good to have the context that the behavior started with the BIOS upgrade on F.20.

I also can't discern what failure of @rytone's you mean; in this thread, I can only see them having issues with wake from lid-opening but that doesn't work for me either. I don't mind that, though, and someone in #1230 (closed) (maybe even you? I'm not opening that issue anymore, as it takes ages to load) told me that it was expected behaviour with acpi.ec_no_wakeup=1 anyway, so I didn't mention it.

The original failure that brought you hear and to try the fix that is queued for 5.15 I mean.

W/ 5.14 - besides lid not working, I'm curious do keyboard or mouse work for you for wakeup? Or only power button works?

I'll leave the kernel option in place, as I can't see any negative effects and also don't want to risk spurious wake-ups in case the commit gets reverted upstream without me noticing. This seems like a likely scenario, as you've mentioned it breaking resumes in some cases, so I'll stay on the safe side for now.

If it gets reverted upstream a lot of people are going to break. It's the lesser of two evils for now.

Neat, I'll apply that once I'm home and give feedback over there. Thank you so much for your patience!

Sure! Your note prompted me to have another look at all the numbers in that hack. I have an updated patch I attached there, please have a try with that.

The original failure that brought you hear and to try the fix that is queued for 5.15 I mean.

Ahh, I just Ctrl+F'ed for rytone and in the original issue description, the user handle isn't mentioned - only the clear name - so I didn't make the connection. Sorry about that. At least we were able to eventually work out that the problems started with the upgrade to F.20.

Since I've never tried F.20 with a 5.13 kernel: Is there anything to be gained from going back and trying to resume under a 5.13 kernel?

W/ 5.14 - besides lid not working, I'm curious do keyboard or mouse work for you for wakeup? Or only power button works?

Power button and keyboard work, but not the touchpad. Mind you, this is with acpi.ec_no_wakeup=1, but if I've understood correctly, this shouldn't matter as I haven't reverted 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36, right?

Sure! Your note prompted me to have another look at all the numbers in that hack. I have an updated patch I attached there, please have a try with that.

OK, I'm running agd5f/linux@aa9bb3d3 and your new https://gitlab.freedesktop.org/drm/amd/uploads/bbb081bfd043464ed2027b3ebba44fb8/0001-platform-x86-amd-pmc-Increase-the-response-register-.patch on top of fedora-5.14(.2) now and will give feedback over there either about a week from now or once I hit a resume failure, whatever comes first.

Since I've never tried F.20 with a 5.13 kernel: Is there anything to be gained from going back and trying to resume under a 5.13 kernel?

Not in my mind there isn't. A majority of the s2idle related fixes landed in 5.14.

Power button and keyboard work, but not the touchpad. Mind you, this is with acpi.ec_no_wakeup=1, but if I've understood correctly, this shouldn't matter as I haven't reverted 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36, right?

Interesting - a number of other HP laptops the power button and touchpad work, but not the keyboard. If you want the touchpad working for wakeup, I'd suggest we open a separate issue to try to dig into why that's not working. If you don't mind it not working, don't worry about it.

And yes, correct.

OK, I'm running agd5f/linux@aa9bb3d3 and your new https://gitlab.freedesktop.org/drm/amd/uploads/bbb081bfd043464ed2027b3ebba44fb8/0001-platform-x86-amd-pmc-Increase-the-response-register-.patch on top of fedora-5.14(.2) now and will give feedback over there either about a week from now or once I hit a resume failure, whatever comes first.

OK thanks.

mentioned in issue #1629 (closed)

@agd5f can we please close this one out. For those that come to it and want to know last status:

• This affects HP ENVY x360 with upgrades to BIOS F.20.
• The fix is upstream now as of 5.15-rc1: https://github.com/torvalds/linux/commit/fa209644a7124b3f4cf811ced55daef49ae39ac6
• It should be backported to 5.14.y in the near future: https://lore.kernel.org/linux-acpi/20210913223339.435347-10-sashal@kernel.org/T/

closed

mentioned in commit nouveau@abae6b3b