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

[Daniel Gibson <daniel@gibson.sh>]

On 11.06.26 19:27, Ilpo Järvinen wrote:
> 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.
> 

Great, thanks a lot for merging! :)

> --
>  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(-)
>>
>>
>