| Message ID | 20210630194606.530-1-mario.limonciello@amd.com |
|---|---|
| State | Accepted |
| Commit | 7b167c4cb48ee3912f0068b9ea5ea4eacc1a5e36 |
| Headers | show |
| Series | ACPI: PM: Only mark EC GPE for wakeup on Intel systems | expand |
Am 30.06.21 um 21:46 schrieb Mario Limonciello: > When using s2idle on a variety of AMD notebook systems, they are > experiencing spurious events that the EC or SMU are in the wrong > state leading to a hard time waking up or higher than expected > power consumption. > > These events only occur when the EC GPE is inadvertently set as a wakeup > source. Originally the EC GPE was only set as a wakeup source when using > the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: > Set up EC GPE for system wakeup from drivers that need it") but during > testing a reporter discovered that this was not enough for their ASUS > Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. > Marking the EC GPE for wakeup universally resolved this for that > reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE > for system wakeup"). > > However this behavior has lead to a number of problems: > > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and > sometimes the power button event doesn't work. > * 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 > > As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7-8550U > that don't use intel-vbtn or intel-hid avoid these problems by only > universally marking the EC GPE wakesource on non-AMD systems. > I can suspend with lid and resume with lid, power and keyboard on an Asus Zenbook 14 UM425IA with 4500U so there are no regressions from using this patch at least. My laptop did not fail to resume all that frequently so I need to test for a bit longer to see if this patch improves my situation. Best regards, Julian
On Wed, Jun 30, 2021 at 9:46 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > When using s2idle on a variety of AMD notebook systems, they are > experiencing spurious events that the EC or SMU are in the wrong > state leading to a hard time waking up or higher than expected > power consumption. > > These events only occur when the EC GPE is inadvertently set as a wakeup > source. Originally the EC GPE was only set as a wakeup source when using > the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: > Set up EC GPE for system wakeup from drivers that need it") but during > testing a reporter discovered that this was not enough for their ASUS > Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. > Marking the EC GPE for wakeup universally resolved this for that > reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE > for system wakeup"). > > However this behavior has lead to a number of problems: > > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and > sometimes the power button event doesn't work. > * 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 > > As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7-8550U > that don't use intel-vbtn or intel-hid avoid these problems by only > universally marking the EC GPE wakesource on non-AMD systems. > > Link: https://patchwork.kernel.org/project/linux-pm/cover/5997740.FPbUVk04hV@kreacher/#22825489 > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1629 > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > Acked-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/acpi/x86/s2idle.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index 816bf2c34b7a..1c507804fb10 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -417,11 +417,15 @@ static int lps0_device_attach(struct acpi_device *adev, > mem_sleep_current = PM_SUSPEND_TO_IDLE; > > /* > - * 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. > + * 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. > */ > - acpi_ec_mark_gpe_for_wake(); > + if (!acpi_s2idle_vendor_amd()) > + acpi_ec_mark_gpe_for_wake(); > > return 0; > } > -- Applied as 5.14-rc1 material, thanks!
On 30.06.21 21:46, Mario Limonciello wrote: > When using s2idle on a variety of AMD notebook systems, they are > experiencing spurious events that the EC or SMU are in the wrong > state leading to a hard time waking up or higher than expected > power consumption. > > These events only occur when the EC GPE is inadvertently set as a wakeup > source. Originally the EC GPE was only set as a wakeup source when using > the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: > Set up EC GPE for system wakeup from drivers that need it") but during > testing a reporter discovered that this was not enough for their ASUS > Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. > Marking the EC GPE for wakeup universally resolved this for that > reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE > for system wakeup"). > > However this behavior has lead to a number of problems: > > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and > sometimes the power button event doesn't work. > * 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 > > As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7-8550U > that don't use intel-vbtn or intel-hid avoid these problems by only > universally marking the EC GPE wakesource on non-AMD systems. > > Link: https://patchwork.kernel.org/project/linux-pm/cover/5997740.FPbUVk04hV@kreacher/#22825489 > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1629 > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > Acked-by: Alex Deucher <alexander.deucher@amd.com> As this seems to fix quite some issues for the AMD systems, is there any reason why this is not tagged as fix for stable? Are there any plans for backporting? > --- > drivers/acpi/x86/s2idle.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c > index 816bf2c34b7a..1c507804fb10 100644 > --- a/drivers/acpi/x86/s2idle.c > +++ b/drivers/acpi/x86/s2idle.c > @@ -417,11 +417,15 @@ static int lps0_device_attach(struct acpi_device *adev, > mem_sleep_current = PM_SUSPEND_TO_IDLE; > > /* > - * 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. > + * 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. > */ > - acpi_ec_mark_gpe_for_wake(); > + if (!acpi_s2idle_vendor_amd()) > + acpi_ec_mark_gpe_for_wake(); > > return 0; > } >
On Fri, Jul 16, 2021 at 1:13 PM Frieder Schrempf <frieder@fris.de> wrote: > > On 30.06.21 21:46, Mario Limonciello wrote: > > When using s2idle on a variety of AMD notebook systems, they are > > experiencing spurious events that the EC or SMU are in the wrong > > state leading to a hard time waking up or higher than expected > > power consumption. > > > > These events only occur when the EC GPE is inadvertently set as a wakeup > > source. Originally the EC GPE was only set as a wakeup source when using > > the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: > > Set up EC GPE for system wakeup from drivers that need it") but during > > testing a reporter discovered that this was not enough for their ASUS > > Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. > > Marking the EC GPE for wakeup universally resolved this for that > > reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE > > for system wakeup"). > > > > However this behavior has lead to a number of problems: > > > > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and > > sometimes the power button event doesn't work. > > * 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 > > > > As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7-8550U > > that don't use intel-vbtn or intel-hid avoid these problems by only > > universally marking the EC GPE wakesource on non-AMD systems. > > > > Link: https://patchwork.kernel.org/project/linux-pm/cover/5997740.FPbUVk04hV@kreacher/#22825489 > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1629 > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > As this seems to fix quite some issues for the AMD systems, is there any > reason why this is not tagged as fix for stable? Are there any plans for > backporting? If you need it in stable kernels, please send a request to stable@vger.kernel.org to include it.
[Public] > > > These events only occur when the EC GPE is inadvertently set as a wakeup > > > source. Originally the EC GPE was only set as a wakeup source when using > > > the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: > > > Set up EC GPE for system wakeup from drivers that need it") but during > > > testing a reporter discovered that this was not enough for their ASUS > > > Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. > > > Marking the EC GPE for wakeup universally resolved this for that > > > reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE > > > for system wakeup"). > > > > > > However this behavior has lead to a number of problems: > > > > > > * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and > > > sometimes the power button event doesn't work. > > > * 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 > > > > > > As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7- > 8550U > > > that don't use intel-vbtn or intel-hid avoid these problems by only > > > universally marking the EC GPE wakesource on non-AMD systems. > > > > > > Link: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw > ork.kernel.org%2Fproject%2Flinux- > pm%2Fcover%2F5997740.FPbUVk04hV%40kreacher%2F%2322825489&dat > a=04%7C01%7Cmario.limonciello%40amd.com%7Cc2ced5bd6bbb4e62e9ac08d > 948525197%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63762034 > 0670859712%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oUsKS5mjAa > rc%2FhnnY%2FILZWWzUbvdBQHlH1MAcusHSIw%3D&reserved=0 > > > Link: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr > eedesktop.org%2Fdrm%2Famd%2F- > %2Fissues%2F1230&data=04%7C01%7Cmario.limonciello%40amd.com%7 > Cc2ced5bd6bbb4e62e9ac08d948525197%7C3dd8961fe4884e608e11a82d994e1 > 83d%7C0%7C0%7C637620340670859712%7CUnknown%7CTWFpbGZsb3d8eyJ > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > 1000&sdata=KKUiOSeAQEkFjN9X8y8k3sC3J3s48faaNLzklPO12as%3D& > reserved=0 > > > Link: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr > eedesktop.org%2Fdrm%2Famd%2F- > %2Fissues%2F1629&data=04%7C01%7Cmario.limonciello%40amd.com%7 > Cc2ced5bd6bbb4e62e9ac08d948525197%7C3dd8961fe4884e608e11a82d994e1 > 83d%7C0%7C0%7C637620340670859712%7CUnknown%7CTWFpbGZsb3d8eyJ > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > 1000&sdata=kDGTIkQMh%2FGG4OhssvukJ7xJ7Ld6j6bl1TXRvpS58%2Fk%3D > &reserved=0 > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > > Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > > As this seems to fix quite some issues for the AMD systems, is there any > > reason why this is not tagged as fix for stable? Are there any plans for > > backporting? > There's a number of other patches in other subsystems besides this one that are needed for successful S0i3 on AMD systems that land in 5.14 but are not stable candidates at this time. Perhaps after 5.14 is out and has been well tested it will make sense to send the whole remaining series back to stable. > If you need it in stable kernels, please send a request to > stable@vger.kernel.org to include it.
On 16.07.21 14:25, Limonciello, Mario wrote: > [Public] > >>>> These events only occur when the EC GPE is inadvertently set as a wakeup >>>> source. Originally the EC GPE was only set as a wakeup source when using >>>> the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: >>>> Set up EC GPE for system wakeup from drivers that need it") but during >>>> testing a reporter discovered that this was not enough for their ASUS >>>> Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. >>>> Marking the EC GPE for wakeup universally resolved this for that >>>> reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE >>>> for system wakeup"). >>>> >>>> However this behavior has lead to a number of problems: >>>> >>>> * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and >>>> sometimes the power button event doesn't work. >>>> * 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 >>>> >>>> As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7- >> 8550U >>>> that don't use intel-vbtn or intel-hid avoid these problems by only >>>> universally marking the EC GPE wakesource on non-AMD systems. >>>> >>>> Link: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw >> ork.kernel.org%2Fproject%2Flinux- >> pm%2Fcover%2F5997740.FPbUVk04hV%40kreacher%2F%2322825489&dat >> a=04%7C01%7Cmario.limonciello%40amd.com%7Cc2ced5bd6bbb4e62e9ac08d >> 948525197%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63762034 >> 0670859712%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi >> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oUsKS5mjAa >> rc%2FhnnY%2FILZWWzUbvdBQHlH1MAcusHSIw%3D&reserved=0 >>>> Link: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr >> eedesktop.org%2Fdrm%2Famd%2F- >> %2Fissues%2F1230&data=04%7C01%7Cmario.limonciello%40amd.com%7 >> Cc2ced5bd6bbb4e62e9ac08d948525197%7C3dd8961fe4884e608e11a82d994e1 >> 83d%7C0%7C0%7C637620340670859712%7CUnknown%7CTWFpbGZsb3d8eyJ >> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C >> 1000&sdata=KKUiOSeAQEkFjN9X8y8k3sC3J3s48faaNLzklPO12as%3D& >> reserved=0 >>>> Link: >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr >> eedesktop.org%2Fdrm%2Famd%2F- >> %2Fissues%2F1629&data=04%7C01%7Cmario.limonciello%40amd.com%7 >> Cc2ced5bd6bbb4e62e9ac08d948525197%7C3dd8961fe4884e608e11a82d994e1 >> 83d%7C0%7C0%7C637620340670859712%7CUnknown%7CTWFpbGZsb3d8eyJ >> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C >> 1000&sdata=kDGTIkQMh%2FGG4OhssvukJ7xJ7Ld6j6bl1TXRvpS58%2Fk%3D >> &reserved=0 >>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>>> Acked-by: Alex Deucher <alexander.deucher@amd.com> >>> >>> As this seems to fix quite some issues for the AMD systems, is there any >>> reason why this is not tagged as fix for stable? Are there any plans for >>> backporting? >> > > There's a number of other patches in other subsystems besides this one that are > needed for successful S0i3 on AMD systems that land in 5.14 but are not stable > candidates at this time. Perhaps after 5.14 is out and has been well tested it will > make sense to send the whole remaining series back to stable. Ok, thanks for clarifying! I guess I will just wait until the dust has settled.
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index 816bf2c34b7a..1c507804fb10 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -417,11 +417,15 @@ static int lps0_device_attach(struct acpi_device *adev, mem_sleep_current = PM_SUSPEND_TO_IDLE; /* - * 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. + * 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. */ - acpi_ec_mark_gpe_for_wake(); + if (!acpi_s2idle_vendor_amd()) + acpi_ec_mark_gpe_for_wake(); return 0; }
When using s2idle on a variety of AMD notebook systems, they are experiencing spurious events that the EC or SMU are in the wrong state leading to a hard time waking up or higher than expected power consumption. These events only occur when the EC GPE is inadvertently set as a wakeup source. Originally the EC GPE was only set as a wakeup source when using the intel-vbtn or intel-hid drivers in commit 10a08fd65ec1 ("ACPI: PM: Set up EC GPE for system wakeup from drivers that need it") but during testing a reporter discovered that this was not enough for their ASUS Zenbook UX430UNR/i7-8550U to wakeup by lid event or keypress. Marking the EC GPE for wakeup universally resolved this for that reporter in commit b90ff3554aa3 ("ACPI: PM: s2idle: Always set up EC GPE for system wakeup"). However this behavior has lead to a number of problems: * On both Lenovo T14 and P14s the keyboard wakeup doesn't work, and sometimes the power button event doesn't work. * 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 As there may be other Intel systems besides ASUS Zenbook UX430UNR/i7-8550U that don't use intel-vbtn or intel-hid avoid these problems by only universally marking the EC GPE wakesource on non-AMD systems. Link: https://patchwork.kernel.org/project/linux-pm/cover/5997740.FPbUVk04hV@kreacher/#22825489 Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230 Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1629 Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/acpi/x86/s2idle.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)