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

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

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

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

I'm not convinced testing pre rc1 really covers much when it comes to 
platform drivers, those few people testing linux-next, as valuable as they 
are, cannot really cover that much HW diversity.

-- 
 i.