Re: [PATCH 3/3] pinctrl: amd: Add a quirk for Lenovo Ideapad 5

[Mario Limonciello]

On 8/29/2023 14:54, Hans de Goede wrote:
> Hi Mario,
>
> On 8/29/23 18:56, Mario Limonciello wrote:
> > Lenovo ideapad 5 doesn't use interrupts for GPIO 0, and so internally
> > debouncing with WinBlue debounce behavior means that the GPIO doesn't
> > clear until a separate GPIO is used (such as touchpad).
> >
> > Prefer to use legacy debouncing to avoid problems.
> >
> > Reported-by: Luca Pigliacampo <lucapgl2001@xxxxxxxxx>
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217833
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> I'm not happy to see yet another DMI quirk solution here.
>
> and I guess you're not happy with this either...

Yeah I was really hoping the first patch was enough for the issue.

If we can't come up with anything else we can potentially drop patches 2 
and 3. Even patch 1 alone will "significantly" improve the situation.

The other option I considered is to hardcode WinBlue debounce behavior 
"off" in Linux.

I don't think this is a good idea though though because we will likely 
trade bugs because the debounce values in the AML for systems using _AEI 
aren't actually used in Windows and might not have good values.

> Are we sure there is no other way? Did you check an acpidump
> for the laptop and specifically for its ACPI powerbutton handling?

I'm not sure there is another way or not, but yes there is an acpidump 
attached to the bug in case you or anyone else has some ideas.

> I would expect the ACPI powerbutton handler to somehow clear
> the bit, like how patch 1/3 clears it from the GPIO chip's
> own IRQ handler.
>
> I see that drivers/acpi/button.c does:
>
> static u32 acpi_button_event(void *data)
> {
>          acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_button_notify_run, data);
>          return ACPI_INTERRUPT_HANDLED;
> }
>
> So unless I'm misreading something here, there is some AML being
> executed on power-button events. So maybe there is something wrong
> with how Linux interprets that AML ?
The relevant ACPI spec section is here:

https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/04_ACPI_Hardware_Specification/ACPI_Hardware_Specification.html#control-method-power-button

I did look at the acpidump.  GPE 08 notifies \_SB.PWRB (a PNP0C0C 
device) with 0x2.  According to the spec this is specifically for 
letting the system know the power button is waking up the system from G1.

I don't see any notifications with data 0x80 in the AML.  So I'm 
wondering if the AML designer made an assumption that pressing the power 
button caused the system to suspend or turn off.  That's the Windows 
behavior, isn't it?

The spec even says:

"While the system is in the working state, a power button press is a 
user request to transition the system into either the sleeping (G1) or 
soft-off state (G2)."

I think it would be a really good data point whether this happens with 
Windows if possible too.  Then we can know if we're looking at a Linux 
bug or a platform bug.