Re: [PATCH v5 2/4] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops

[Mario Limonciello <superm1@kernel.org>]

On 6/10/26 03:21, Ilpo Järvinen wrote:
> On Tue, 9 Jun 2026, Mario Limonciello wrote:
>> On 6/9/26 10:36, Daniel Gibson wrote:
>>> On 09.06.26 17:06, Daniel Gibson wrote:
>>>> On 09.06.26 16:40, Mario Limonciello wrote:
>>>>> On 6/9/26 07:07, Daniel Gibson wrote:
>>>>>> On 09.06.26 13:46, Ilpo Järvinen wrote:
>>>>>>> On Tue, 9 Jun 2026, Daniel Gibson wrote:
>>>>>>>>         },
>>>>>>>> +    /* https://bugzilla.kernel.org/show_bug.cgi?id=221383 */
>>>>>>>> +    {
>>>>>>>> +        .ident = "Zen3-based IdeaPad Slim and similar",
>>>>>>>> +        .driver_data = &quirk_s2idle_need_suspend_delay,
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> One more question.
>>>>>>>
>>>>>>> Sashiko noted, that amd_pmc_quirks_init() can overwrite
>>>>>>> disable_8042_wakeup from the AMD_CPU_ID_CZN check when .driver_data
>>>>>>> provides quirks. Is it okay in this case to not have .spurious_8042?
>>>>>>>
>>>>>>
>>>>>> Good question.
>>>>>> So far I haven't had complaints that sounded like they'd be related to
>>>>>> this quirk not being active.
>>>>>>
>>>>>> (The only report about things not working as expected on detected
>>>>>> devices was something about "ACPI event storms" after resume:
>>>>>> https://github.com/DanielGibson/amd_pmc-ideapad/issues/3 - but no one
>>>>>> else with similar devices could reproduce that, so no idea what's
>>>>>> going
>>>>>> on there, and it doesn't sound like that IRQ1 issue)
>>>>>>
>>>>>> I can test if explicitly enabling .spurious_8042 in
>>>>>> quirk_s2idle_need_suspend_delay breaks anything on my device, if you
>>>>>> think that enabling it by default makes more sense?
>>>>>
>>>>> Famous last words - but we haven't had a need for spurious 8042 on
>>>>> recent hardware so I think this is unlikely to be a big problem.
>>>>
>>>> FWIW enabling .spurious_8042 didn't break anything on my machine, but
>>>> didn't improve anything either - only visible difference is that when
>>>> resuming by pressing a key without that quirk both IRQ1 and IRQ7 are
>>>> reported as having triggered the resume, and with the quirk only IRQ7 is
>>>> reported. But it didn't seem like IRQ1 triggers a resume when it
>>>> shouldn't.
>>>>
>>>> OTOH I have a the latest BIOS (from this year), so it's likely fixed
>>>> there - maybe people with older BIOS versions still need the
>>>> .spurious_8042 quirk?
>>>>
>>>> As these devices are relatively recent and still sold I hope that
>>>> everyone affected can get a new BIOS (which they should do either way).
>>>
>>> Anyway, overall I'd say that the patches can be merged as they are - the
>>> affected devices are known to have serious (-ly annoying) suspend
>>> issues, so it's unlikely that a currently matched devices has working
>>> suspend that breaks with them, so things at least shouldn't get any
>>> worse for their users?
>>>
>>> The patches have gotten some testing already on different devices (from
>>> my out-of-tree patched amd_pmc module on Github) and so far it looks
>>> like the spurious_8042 quirk isn't needed.
>>> If reports of needing both quirks turn up after all, that can still be
>>> easily added in a few lines of code (maybe even just one).
>>>
>>> Cheers,
>>> Daniel
>>
>> Even with all that testing; it's only on hardware with problems.
>> We don't want to have issues exposed by these patches for people that didn't
>> need the patches.
>>
>> So my 2c:
>> * It's "too risky" to pick up for 7.1 final
> 
> Definitely it won't be happening.
> 
> Also, Linus effectively only allows regression fixes during -rc phase
> anymore (he stated his policy change around -rc5 timeframe). I therefore
> moved even some of queued new HW support patches from the pdx86 fixes
> branch into for-next.
> 
>> * It's a "bit late" in the cycle for 7.2-rc1 (usually new content stops being
>> added around rc6).
> 
> I usually only stop accepting large series around that time, with some
> exceptions. (We've one such exception ATM because I was a week away and
> couldn't process patches, it would feel unfair to penalize other devs
> because of that so for the series which have been around for awhile, I
> still try to process them during this week, this one included).
> 
>> * This isn't "risky enough" to wait until 7.3 (basically after 7.2-rc1 merge
>> window is done)
>>
>> But this has been on the list for a while now, so I would say this makes sense
>> to put in for 7.2-rc1 and we should all make sure we test well once the RCs
>> are posted.
> 
> My plan is to take this into for-next during this cycle.
> 
> We can always take a timeout and revert it during -rc phase if problems
> appear because of it (and even after 7.2 release, if situation requires
> it).

Sounds good to me, thanks.