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