Re: [PATCH v6 0/4] amd_pmc: Delay s2idle suspend for some devices

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5378 bytes --]

On Thu, 11 Jun 2026, Daniel Gibson wrote:

> On some AMD Zen3 and Zen3+-based Lenovo IdeaPad laptops the keyboard and
> the lid switch stop working after the first suspend, until rebooted.
> 
> More specifically, they stop sending events when pressing a key or
> closing the lid - it's still possible to toggle the capslock- and
> numlock-LEDs with an external keyboard or read the lid state at
> /proc/acpi/button/lid/LID/state.
> 
> See also https://bugzilla.kernel.org/show_bug.cgi?id=221383
> 
> It appears that suspending and/or resuming gets the EC into a broken
> state. This problem doesn't happen on Windows and Mario Limonciello
> mentioned that the Windows kernel gives hardware and software some time
> before actually suspending (before activating HW DRIPS), while Linux
> (or the amd_pmc module) does that immediately, so it may be worth trying
> if calling msleep() in amd_pmc_s2idle_check() helps.
> 
> It turned out that sleeping for 2.5 seconds at that point indeed makes
> the problems mostly disappear. Sleeping for 1.5 seconds wasn't enough.
> 
> "Mostly" because it turned out that they still occur (on some but not
> all devices needing this patch) when using a wakeup timer (wakealarm).
> 
> I could build on an existing quirk[1] that also sleeps for 2.5 seconds
> under other circumstances; my first commit refactors that a bit so I
> can integrate my further changes in a cleaner way.
> 
> I found several reports of these or similar issues on the web, for
> different devices, so in a second commit I added a parameter to the
> kernel module that allows enabling or disabling this, which will make
> it easy for people whose devices aren't matched yet to test this quirk.
> 
> Thanks to Mario Limonciello for his support and to Sindre Henriksen for
> testing my patch and to Ilpo Järvinen and Hans de Goede for reviewing!
> 
> [1] https://lore.kernel.org/platform-driver-x86/20250414162446.3853194-1-superm1@kernel.org/
> 
> Changes in v6:
> - Rebased to review-ilpo-next branch (commit 2565a28cdcdc "platform/x86: ISST: Restore SST-PP control to all domains")

Thanks, this one applied cleanly to the review-ilpo-next branch.

--
 i.

> 
> Changes in v5 (https://lore.kernel.org/platform-driver-x86/20260609105756.2813669-1-daniel@gibson.sh/T/#u):
> - Re-add missing first commit ("Check for intermediate wakeup in function")
>   (sorry!)
> - Add Reviewed-By tags for Hans de Goede's review
> 
> Changes in v4 (https://lore.kernel.org/platform-driver-x86/20260606044758.2213401-1-daniel@gibson.sh/T/#u):
> - Don't log during intermediate wakes, which happen a lot when charging
>   those IdeaPads while they're suspended, so dmesg isn't spammed
> - Removed the documentation commits to make merging this less painful
>   (one of them referred to commits added here, so the IDs would have to
>    be fixed up while merging). I'll submit them separately.
> 
> Changes in v3 (https://lore.kernel.org/platform-driver-x86/20260512202645.1549111-1-daniel@gibson.sh/T/#u
>     and https://lore.kernel.org/platform-driver-x86/20260603031110.345815-1-daniel@gibson.sh/T/#u):
> - Rewrote commit messages of patch 1, 4 and 5 as requested in the review
> - Adjusted formatting of the other commit messages
> - Added another confirmed device (83MM) to the quirks list and mention
>   it in the commit message
> 
> Changes in v2 (https://lore.kernel.org/platform-driver-x86/20260509013105.816339-1-daniel@gibson.sh/t/#u):
> - Documented this in Documentation/arch/x86/amd-debugging.rst
> - Added example for reset register kernel message in same file
> - In amd_pmc_quirk_need_suspend_delay(), avoid dereferencing a NULL
>   pointer of devices not detected for any quirk - oops!
> - Mention that timed resumes may still cause those keyboard/lid issues
> - Various code changes requested or suggested in reviews of v1:
>   - Some formatting changes (commas behind non-terminating entries)
>   - Moved check for existing quirk (that OVP thing) into its own function
>     amd_pmc_intermediate_wakeup_need_delay() in pmc.c, so the checks of
>     the different quirks are separated more clearly.
>   - Added function amd_pmc_want_suspend_delay() in pmc.c handling
>     amd_pmc_quirk_need_suspend_delay() together with disable_workarounds
>     and delay_suspend and also logging about the reason for the delay,
>     also for cleaner separation.
>   - If delay_suspend=1 is used to force-enable the fix on hardware that
>     is not automatically detected as needing this fix, log message
>     encouraging the user to report their device, including the most
>     relevant DMI values that could be used for matching
> 
> v1: https://lore.kernel.org/platform-driver-x86/20260501032655.283789-1-daniel@gibson.sh/t/#u
> 
> 
> Daniel Gibson (4):
>   platform/x86/amd/pmc: Check for intermediate wakeup in function
>   platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops
>   platform/x86/amd/pmc: Add delay_suspend module parameter
>   platform/x86/amd/pmc: Don't log during intermediate wakeups
> 
>  drivers/platform/x86/amd/pmc/pmc-quirks.c | 39 +++++++++++
>  drivers/platform/x86/amd/pmc/pmc.c        | 83 ++++++++++++++++++++++-
>  drivers/platform/x86/amd/pmc/pmc.h        |  2 +
>  3 files changed, 121 insertions(+), 3 deletions(-)
> 
> 

-- 
 i.