On Sat, Jan 29, 2022 at 8:22 AM Kai-Heng Feng
<kai.heng.feng@xxxxxxxxxxxxx> wrote:
>
> On Sat, Jan 29, 2022 at 4:34 AM Mario Limonciello
> <mario.limonciello@xxxxxxx> wrote:
> >
> > Testing on various upcoming OEM systems shows commit 7b167c4cb48e ("ACPI:
> > PM: Only mark EC GPE for wakeup on Intel systems") was short
> > sighted and the symptoms were indicative of other problems. Some OEMs
> > do have the dedicated GPIOs for the power button but also rely upon
> > an interrupt to the EC SCI to let the lid work.
> >
> > The original commit showed spurious activity on Lenovo systems:
> > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and
> > sometimes the power button event doesn't work.
> >
> > This was confirmed on my end at that time.
> >
> > However further development in the kernel showed that the issue was
> > actually the IRQ for the GPIO controller was also shared with the EC SCI.
> > This was actually fixed by commit 2d54067fcd23 ("pinctrl: amd: Fix
> > wakeups when IRQ is shared with SCI").
> >
> > The original commit also showed problems with AC adapter:
> > * On HP 635 G7 detaching or attaching AC during suspend will cause
> > the system not to wakeup
> > * On Asus vivobook to prevent detaching AC causing resume problems
> > * On Lenovo 14ARE05 to prevent detaching AC causing resume problems
> > * On HP ENVY x360 to prevent detaching AC causing resume problems
> >
> > Detaching AC adapter causing problems appears to have been a problem
> > because the EC SCI went off to notify the OS of the power adapter change
> > but the SCI was ignored and there was no other way to wake up this system
> > since GPIO controller wasn't properly enabled. The wakeups were fixed by
> > enabling the GPIO controller in commit acd47b9f28e5 ("pinctrl: amd: Handle
> > wake-up interrupt").
> >
> > I've confirmed on a variety of OEM notebooks with the following test
> > 1) echo 1 | sudo tee /sys/power/pm_debug_messages
> > 2) sudo systemctl suspend
> > 3) unplug AC adapter, make sure system is still asleep
> > 4) wake system from lid (which is provided by ACPI SCI on some of them)
> > 5) dmesg
> > a) see the EC GPE dispatched, timekeeping for X seconds (matching ~time
> > until AC adapter plug out)
> > b) see timekeeping for Y seconds until woke (matching ~time from AC
> > adapter until lid event)
> > 6) Look at /sys/kernel/debug/amd_pmc/s0ix_stats
> > "Time (in us) in S0i3" = X + Y - firmware processing time
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> Fixes LID wakeup on several AMD based HP ProBook and EliteBook.
> Tested-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
Applied as 5.17-rc material, thanks!
> > ---
> > drivers/acpi/x86/s2idle.c | 12 ++++--------
> > 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> > index a2f16d4ecbae..665a89e2c940 100644
> > --- a/drivers/acpi/x86/s2idle.c
> > +++ b/drivers/acpi/x86/s2idle.c
> > @@ -427,15 +427,11 @@ static int lps0_device_attach(struct acpi_device *adev,
> > mem_sleep_current = PM_SUSPEND_TO_IDLE;
> >
> > /*
> > - * Some Intel based LPS0 systems, like ASUS Zenbook UX430UNR/i7-8550U don't
> > - * use intel-hid or intel-vbtn but require the EC GPE to be enabled while
> > - * suspended for certain wakeup devices to work, so mark it as wakeup-capable.
> > - *
> > - * Only enable on !AMD as enabling this universally causes problems for a number
> > - * of AMD based systems.
> > + * Some LPS0 systems, like ASUS Zenbook UX430UNR/i7-8550U, require the
> > + * EC GPE to be enabled while suspended for certain wakeup devices to
> > + * work, so mark it as wakeup-capable.
> > */
> > - if (!acpi_s2idle_vendor_amd())
> > - acpi_ec_mark_gpe_for_wake();
> > + acpi_ec_mark_gpe_for_wake();
> >
> > return 0;
> > }
> > --
> > 2.25.1
> >
