| Message ID | 445f6320-698f-4d29-8556-665366668e4d@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: EC: Fix CPU frequency limitation on AMD platforms after suspend/resume | expand |
On 4/19/2025 4:28 AM, M. Bergo wrote: > From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 00:00:00 2001 > From: Marcus Bergo <marcusbergo@gmail.com> > Date: Sat, 19 Apr 2025 05:19:05 -0300 > Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD platforms > after > suspend/resume > > Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen 1, HP > EliteBook 845 G10) > experience a CPU frequency limitation issue where the processor gets > stuck at > approximately 544MHz after resuming from suspend when the power cord is > unplugged > during sleep. This issue makes the systems practically unusable until a > full > power cycle is performed. > > The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix > acpi_ec_dispatch_gpe()") which restored the behavior of clearing the GPE > in acpi_ec_dispatch_gpe() function to prevent GPE storms. While this fix is > necessary for most platforms to prevent excessive power consumption during > suspend-to-idle, it causes problems on certain AMD platforms by interfering > with the EC's ability to properly restore power management settings > after resume. > > This patch implements a targeted workaround that: > 1. Adds DMI-based detection for affected AMD platforms > 2. Adds a function to check if we're in suspend-to-idle mode > 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD platforms > specially: > - For affected AMD platforms during suspend-to-idle, it advances the > transaction without clearing the GPE status bit > - For all other platforms, it maintains the existing behavior of > clearing > the GPE status bit > > Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO 7840HS > and > confirmed that: > 1. Without the patch, the CPU frequency is limited to 544MHz after the > suspend/unplug/resume sequence > 2. With the patch applied, the CPU properly scales up to its maximum > frequency > (5.1GHz) after the same sequence > 3. No regressions were observed in other EC functionality (battery status, > keyboard backlight, etc.) > 4. Multiple suspend/resume cycles with different power states were tested > without issues > > The patch was also verified not to affect the behavior on Intel-based > systems, > ensuring that the GPE storm prevention remains effective where needed. > > Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 > Reported-by: Mark Pearson <mark.pearson@lenovo.com> > Signed-off-by: Marcus Bergo <marcusbergo@gmail.com> Great finding with this being a potential root cause of this behavior (at least from a Linux perspective). Although this helps, I'm not really a fan of the tech debt accumulated by needing to quirk this on a system by system basis as a bandage. At least for HP someone said that this commit happens to help them for the same issue you're describing: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d That was surprising to me, but it must be changing the timing of some of the code running in HP's EC. Since you happen to have a Lenovo system does it happen to help the Lenovo EC too? Mark, comments please? > --- > drivers/acpi/ec.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 3c5f34892734..f3698f3c100f 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -29,6 +29,7 @@ > #include <linux/acpi.h> > #include <linux/dmi.h> > #include <asm/io.h> > +#include <asm/processor.h> > #include "internal.h" > @@ -2139,6 +2140,59 @@ static bool acpi_ec_work_in_progress(struct > acpi_ec *ec) > return ec->events_in_progress + ec->queries_in_progress > 0; > } > +/* List of AMD platforms with CPU frequency issues after suspend/resume */ > +static const struct dmi_system_id acpi_ec_amd_freq_quirk[] = { > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), > + DMI_MATCH(DMI_PRODUCT_NAME, "P15v Gen 3"), > + }, > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), > + DMI_MATCH(DMI_PRODUCT_NAME, "P16v Gen 1"), > + }, > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "HP"), > + DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 845 14 inch G10 Notebook > PC"), > + }, > + }, > + { }, > +}; > + > +/* Check if we're in suspend-to-idle mode */ > +static bool pm_suspend_via_s2idle(void) > +{ > +#ifdef CONFIG_PM_SLEEP > + return pm_suspend_target_state == PM_SUSPEND_TO_IDLE; > +#else > + return false; > +#endif > +} > + > +/* Check if the system is an AMD platform with the frequency quirk */ > +static bool is_amd_freq_quirk_system(void) > +{ > + static int is_quirk_system = -1; > + > + if (is_quirk_system == -1) { > + /* Check if it's an AMD CPU */ > + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) { > + is_quirk_system = 0; > + } else { > + /* Check if it's in the DMI quirk list */ > + is_quirk_system = dmi_check_system(acpi_ec_amd_freq_quirk) ? 1 : 0; > + } > + } > + > + return is_quirk_system == 1; > +} > + > + > + > bool acpi_ec_dispatch_gpe(void) > { > bool work_in_progress = false; > @@ -2172,7 +2226,17 @@ bool acpi_ec_dispatch_gpe(void) > if (acpi_ec_gpe_status_set(first_ec)) { > pm_pr_dbg("ACPI EC GPE status set\n"); > - clear_gpe_and_advance_transaction(first_ec, false); > + /* > + * Special handling for AMD platforms with CPU frequency > issues > + * after suspend/resume. On these platforms, we need to > advance the > + * transaction but NOT clear the GPE status bit when in > suspend-to-idle > + * state to prevent CPU frequency limitation issues. > + */ > + if (is_amd_freq_quirk_system() && > pm_suspend_via_s2idle()) { > + advance_transaction(first_ec, false); > + } else { > + clear_gpe_and_advance_transaction(first_ec, false); > + } > work_in_progress = acpi_ec_work_in_progress(first_ec); > }
On 4/19/2025 1:03 PM, Mario Limonciello wrote: > On 4/19/2025 4:28 AM, M. Bergo wrote: >> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 00:00:00 2001 >> From: Marcus Bergo <marcusbergo@gmail.com> >> Date: Sat, 19 Apr 2025 05:19:05 -0300 >> Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD >> platforms after >> suspend/resume >> >> Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen 1, HP >> EliteBook 845 G10) >> experience a CPU frequency limitation issue where the processor gets >> stuck at >> approximately 544MHz after resuming from suspend when the power cord >> is unplugged >> during sleep. This issue makes the systems practically unusable until >> a full >> power cycle is performed. >> >> The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix >> acpi_ec_dispatch_gpe()") which restored the behavior of clearing the GPE >> in acpi_ec_dispatch_gpe() function to prevent GPE storms. While this >> fix is >> necessary for most platforms to prevent excessive power consumption >> during >> suspend-to-idle, it causes problems on certain AMD platforms by >> interfering >> with the EC's ability to properly restore power management settings >> after resume. >> >> This patch implements a targeted workaround that: >> 1. Adds DMI-based detection for affected AMD platforms >> 2. Adds a function to check if we're in suspend-to-idle mode >> 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD >> platforms specially: >> - For affected AMD platforms during suspend-to-idle, it advances the >> transaction without clearing the GPE status bit >> - For all other platforms, it maintains the existing behavior of >> clearing >> the GPE status bit >> >> Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO >> 7840HS and >> confirmed that: >> 1. Without the patch, the CPU frequency is limited to 544MHz after the >> suspend/unplug/resume sequence >> 2. With the patch applied, the CPU properly scales up to its maximum >> frequency >> (5.1GHz) after the same sequence >> 3. No regressions were observed in other EC functionality (battery >> status, >> keyboard backlight, etc.) >> 4. Multiple suspend/resume cycles with different power states were tested >> without issues >> >> The patch was also verified not to affect the behavior on Intel-based >> systems, >> ensuring that the GPE storm prevention remains effective where needed. >> >> Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 >> Reported-by: Mark Pearson <mark.pearson@lenovo.com> >> Signed-off-by: Marcus Bergo <marcusbergo@gmail.com> > > Great finding with this being a potential root cause of this behavior > (at least from a Linux perspective). > > Although this helps, I'm not really a fan of the tech debt accumulated > by needing to quirk this on a system by system basis as a bandage. > > At least for HP someone said that this commit happens to help them for > the same issue you're describing: > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers- > x86.git/commit/?h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d > > That was surprising to me, but it must be changing the timing of some of > the code running in HP's EC. Since you happen to have a Lenovo system > does it happen to help the Lenovo EC too? > > Mark, comments please? > Someone just reported that the timing delay patch helped their Lenovo system as well. Can you see if it helps you too?
It does make it work fine for me, I saw the clock/timing interference and this sane this problem for Lenovo as well. On 4/24/25 11:11 AM, Mario Limonciello wrote: > On 4/19/2025 1:03 PM, Mario Limonciello wrote: >> On 4/19/2025 4:28 AM, M. Bergo wrote: >>> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 00:00:00 2001 >>> From: Marcus Bergo <marcusbergo@gmail.com> >>> Date: Sat, 19 Apr 2025 05:19:05 -0300 >>> Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD >>> platforms after >>> suspend/resume >>> >>> Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen 1, HP >>> EliteBook 845 G10) >>> experience a CPU frequency limitation issue where the processor gets >>> stuck at >>> approximately 544MHz after resuming from suspend when the power cord >>> is unplugged >>> during sleep. This issue makes the systems practically unusable >>> until a full >>> power cycle is performed. >>> >>> The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix >>> acpi_ec_dispatch_gpe()") which restored the behavior of clearing the >>> GPE >>> in acpi_ec_dispatch_gpe() function to prevent GPE storms. While this >>> fix is >>> necessary for most platforms to prevent excessive power consumption >>> during >>> suspend-to-idle, it causes problems on certain AMD platforms by >>> interfering >>> with the EC's ability to properly restore power management settings >>> after resume. >>> >>> This patch implements a targeted workaround that: >>> 1. Adds DMI-based detection for affected AMD platforms >>> 2. Adds a function to check if we're in suspend-to-idle mode >>> 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD >>> platforms specially: >>> - For affected AMD platforms during suspend-to-idle, it advances >>> the >>> transaction without clearing the GPE status bit >>> - For all other platforms, it maintains the existing behavior of >>> clearing >>> the GPE status bit >>> >>> Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO >>> 7840HS and >>> confirmed that: >>> 1. Without the patch, the CPU frequency is limited to 544MHz after the >>> suspend/unplug/resume sequence >>> 2. With the patch applied, the CPU properly scales up to its maximum >>> frequency >>> (5.1GHz) after the same sequence >>> 3. No regressions were observed in other EC functionality (battery >>> status, >>> keyboard backlight, etc.) >>> 4. Multiple suspend/resume cycles with different power states were >>> tested >>> without issues >>> >>> The patch was also verified not to affect the behavior on >>> Intel-based systems, >>> ensuring that the GPE storm prevention remains effective where needed. >>> >>> Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 >>> Reported-by: Mark Pearson <mark.pearson@lenovo.com> >>> Signed-off-by: Marcus Bergo <marcusbergo@gmail.com> >> >> Great finding with this being a potential root cause of this behavior >> (at least from a Linux perspective). >> >> Although this helps, I'm not really a fan of the tech debt >> accumulated by needing to quirk this on a system by system basis as a >> bandage. >> >> At least for HP someone said that this commit happens to help them >> for the same issue you're describing: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers- >> x86.git/commit/?h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d >> >> That was surprising to me, but it must be changing the timing of some >> of the code running in HP's EC. Since you happen to have a Lenovo >> system does it happen to help the Lenovo EC too? >> >> Mark, comments please? >> > Someone just reported that the timing delay patch helped their Lenovo > system as well. Can you see if it helps you too?
Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen 1, HP
EliteBook 845 G10)
experience a CPU frequency limitation issue where the processor gets
stuck at
approximately 544MHz after resuming from suspend when the power cord is
unplugged
during sleep. This issue makes the systems practically unusable until a full
power cycle is performed.
The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix
acpi_ec_dispatch_gpe()") which restored the behavior of clearing the GPE
in acpi_ec_dispatch_gpe() function to prevent GPE storms. While this fix is
necessary for most platforms to prevent excessive power consumption during
suspend-to-idle, it causes problems on certain AMD platforms by interfering
with the EC's ability to properly restore power management settings
after resume.
This patch implements a targeted workaround that:
1. Adds DMI-based detection for affected AMD platforms
2. Adds a function to check if we're in suspend-to-idle mode
3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD platforms
specially:
- For affected AMD platforms during suspend-to-idle, it advances the
transaction without clearing the GPE status bit
- For all other platforms, it maintains the existing behavior of
clearing
the GPE status bit
Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO 7840HS and
confirmed that:
1. Without the patch, the CPU frequency is limited to 544MHz after the
suspend/unplug/resume sequence
2. With the patch applied, the CPU properly scales up to its maximum
frequency
(5.1GHz) after the same sequence
3. No regressions were observed in other EC functionality (battery status,
keyboard backlight, etc.)
4. Multiple suspend/resume cycles with different power states were tested
without issues
The patch was also verified not to affect the behavior on Intel-based
systems,
ensuring that the GPE storm prevention remains effective where needed.
Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()")
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557
Reported-by: Mark Pearson <mark.pearson@lenovo.com>
Signed-off-by: Marcus Bergo <marcusbergo@gmail.com>
---
drivers/acpi/ec.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 3c5f34892734..f3698f3c100f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -29,6 +29,7 @@
#include <linux/acpi.h>
#include <linux/dmi.h>
#include <asm/io.h>
+#include <asm/processor.h>
#include "internal.h"
@@ -2139,6 +2140,59 @@ static bool acpi_ec_work_in_progress(struct
acpi_ec *ec)
return ec->events_in_progress + ec->queries_in_progress > 0;
}
+/* List of AMD platforms with CPU frequency issues after suspend/resume */
+static const struct dmi_system_id acpi_ec_amd_freq_quirk[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "P15v Gen 3"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "P16v Gen 1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook 845 14 inch G10 Notebook
PC"),
+ },
+ },
+ { },
+};
+
+/* Check if we're in suspend-to-idle mode */
+static bool pm_suspend_via_s2idle(void)
+{
+#ifdef CONFIG_PM_SLEEP
+ return pm_suspend_target_state == PM_SUSPEND_TO_IDLE;
+#else
+ return false;
+#endif
+}
+
+/* Check if the system is an AMD platform with the frequency quirk */
+static bool is_amd_freq_quirk_system(void)
+{
+ static int is_quirk_system = -1;
+
+ if (is_quirk_system == -1) {
+ /* Check if it's an AMD CPU */
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) {
+ is_quirk_system = 0;
+ } else {
+ /* Check if it's in the DMI quirk list */
+ is_quirk_system = dmi_check_system(acpi_ec_amd_freq_quirk) ? 1 : 0;
+ }
+ }
+
+ return is_quirk_system == 1;
+}
+
+
+
bool acpi_ec_dispatch_gpe(void)
{
bool work_in_progress = false;
@@ -2172,7 +2226,17 @@ bool acpi_ec_dispatch_gpe(void)
if (acpi_ec_gpe_status_set(first_ec)) {
pm_pr_dbg("ACPI EC GPE status set\n");
- clear_gpe_and_advance_transaction(first_ec, false);
+ /*
+ * Special handling for AMD platforms with CPU frequency
issues
+ * after suspend/resume. On these platforms, we need to
advance the
+ * transaction but NOT clear the GPE status bit when in
suspend-to-idle
+ * state to prevent CPU frequency limitation issues.
+ */
+ if (is_amd_freq_quirk_system() && pm_suspend_via_s2idle()) {
+ advance_transaction(first_ec, false);
+ } else {
+ clear_gpe_and_advance_transaction(first_ec, false);
+ }
work_in_progress = acpi_ec_work_in_progress(first_ec);
}
On 4/27/2025 1:34 PM, M. Bergo wrote: > It does make it work fine for me, I saw the clock/timing interference > and this sane this problem for Lenovo as well. Huh? I think you have a typo; but I don't know what you actually meant. So you're saying the timing patch helps your system as well? Thanks, > > On 4/24/25 11:11 AM, Mario Limonciello wrote: >> On 4/19/2025 1:03 PM, Mario Limonciello wrote: >>> On 4/19/2025 4:28 AM, M. Bergo wrote: >>>> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 00:00:00 2001 >>>> From: Marcus Bergo <marcusbergo@gmail.com> >>>> Date: Sat, 19 Apr 2025 05:19:05 -0300 >>>> Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD >>>> platforms after >>>> suspend/resume >>>> >>>> Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen 1, HP >>>> EliteBook 845 G10) >>>> experience a CPU frequency limitation issue where the processor gets >>>> stuck at >>>> approximately 544MHz after resuming from suspend when the power cord >>>> is unplugged >>>> during sleep. This issue makes the systems practically unusable >>>> until a full >>>> power cycle is performed. >>>> >>>> The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix >>>> acpi_ec_dispatch_gpe()") which restored the behavior of clearing the >>>> GPE >>>> in acpi_ec_dispatch_gpe() function to prevent GPE storms. While this >>>> fix is >>>> necessary for most platforms to prevent excessive power consumption >>>> during >>>> suspend-to-idle, it causes problems on certain AMD platforms by >>>> interfering >>>> with the EC's ability to properly restore power management settings >>>> after resume. >>>> >>>> This patch implements a targeted workaround that: >>>> 1. Adds DMI-based detection for affected AMD platforms >>>> 2. Adds a function to check if we're in suspend-to-idle mode >>>> 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD >>>> platforms specially: >>>> - For affected AMD platforms during suspend-to-idle, it advances >>>> the >>>> transaction without clearing the GPE status bit >>>> - For all other platforms, it maintains the existing behavior of >>>> clearing >>>> the GPE status bit >>>> >>>> Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO >>>> 7840HS and >>>> confirmed that: >>>> 1. Without the patch, the CPU frequency is limited to 544MHz after the >>>> suspend/unplug/resume sequence >>>> 2. With the patch applied, the CPU properly scales up to its maximum >>>> frequency >>>> (5.1GHz) after the same sequence >>>> 3. No regressions were observed in other EC functionality (battery >>>> status, >>>> keyboard backlight, etc.) >>>> 4. Multiple suspend/resume cycles with different power states were >>>> tested >>>> without issues >>>> >>>> The patch was also verified not to affect the behavior on Intel- >>>> based systems, >>>> ensuring that the GPE storm prevention remains effective where needed. >>>> >>>> Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 >>>> Reported-by: Mark Pearson <mark.pearson@lenovo.com> >>>> Signed-off-by: Marcus Bergo <marcusbergo@gmail.com> >>> >>> Great finding with this being a potential root cause of this behavior >>> (at least from a Linux perspective). >>> >>> Although this helps, I'm not really a fan of the tech debt >>> accumulated by needing to quirk this on a system by system basis as a >>> bandage. >>> >>> At least for HP someone said that this commit happens to help them >>> for the same issue you're describing: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform- >>> drivers- x86.git/commit/? >>> h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d >>> >>> That was surprising to me, but it must be changing the timing of some >>> of the code running in HP's EC. Since you happen to have a Lenovo >>> system does it happen to help the Lenovo EC too? >>> >>> Mark, comments please? >>> >> Someone just reported that the timing delay patch helped their Lenovo >> system as well. Can you see if it helps you too?
On 4/28/2025 4:51 AM, Marcus Bergo wrote: > Yes, it does. > OK thanks for confirming. Considering your finding with this patch you've shared and knowing there is a timing dependency that delaying the next s2idle cycle helps I do wonder if we should keep exploring. Rafael, do you have thoughts here? Specifically do you think it's worth revisiting if b5539eb5ee70 was the correct move. > On Sun, Apr 27, 2025 at 11:06 PM Mario Limonciello > <mario.limonciello@amd.com <mailto:mario.limonciello@amd.com>> wrote: > > On 4/27/2025 1:34 PM, M. Bergo wrote: > > It does make it work fine for me, I saw the clock/timing > interference > > and this sane this problem for Lenovo as well. > > Huh? I think you have a typo; but I don't know what you actually meant. > > So you're saying the timing patch helps your system as well? > > Thanks, > > > > > On 4/24/25 11:11 AM, Mario Limonciello wrote: > >> On 4/19/2025 1:03 PM, Mario Limonciello wrote: > >>> On 4/19/2025 4:28 AM, M. Bergo wrote: > >>>> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 > 00:00:00 2001 > >>>> From: Marcus Bergo <marcusbergo@gmail.com > <mailto:marcusbergo@gmail.com>> > >>>> Date: Sat, 19 Apr 2025 05:19:05 -0300 > >>>> Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD > >>>> platforms after > >>>> suspend/resume > >>>> > >>>> Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen > 1, HP > >>>> EliteBook 845 G10) > >>>> experience a CPU frequency limitation issue where the > processor gets > >>>> stuck at > >>>> approximately 544MHz after resuming from suspend when the > power cord > >>>> is unplugged > >>>> during sleep. This issue makes the systems practically unusable > >>>> until a full > >>>> power cycle is performed. > >>>> > >>>> The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix > >>>> acpi_ec_dispatch_gpe()") which restored the behavior of > clearing the > >>>> GPE > >>>> in acpi_ec_dispatch_gpe() function to prevent GPE storms. > While this > >>>> fix is > >>>> necessary for most platforms to prevent excessive power > consumption > >>>> during > >>>> suspend-to-idle, it causes problems on certain AMD platforms by > >>>> interfering > >>>> with the EC's ability to properly restore power management > settings > >>>> after resume. > >>>> > >>>> This patch implements a targeted workaround that: > >>>> 1. Adds DMI-based detection for affected AMD platforms > >>>> 2. Adds a function to check if we're in suspend-to-idle mode > >>>> 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD > >>>> platforms specially: > >>>> - For affected AMD platforms during suspend-to-idle, it > advances > >>>> the > >>>> transaction without clearing the GPE status bit > >>>> - For all other platforms, it maintains the existing > behavior of > >>>> clearing > >>>> the GPE status bit > >>>> > >>>> Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO > >>>> 7840HS and > >>>> confirmed that: > >>>> 1. Without the patch, the CPU frequency is limited to 544MHz > after the > >>>> suspend/unplug/resume sequence > >>>> 2. With the patch applied, the CPU properly scales up to its > maximum > >>>> frequency > >>>> (5.1GHz) after the same sequence > >>>> 3. No regressions were observed in other EC functionality > (battery > >>>> status, > >>>> keyboard backlight, etc.) > >>>> 4. Multiple suspend/resume cycles with different power states > were > >>>> tested > >>>> without issues > >>>> > >>>> The patch was also verified not to affect the behavior on Intel- > >>>> based systems, > >>>> ensuring that the GPE storm prevention remains effective where > needed. > >>>> > >>>> Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") > >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 > <https://bugzilla.kernel.org/show_bug.cgi?id=218557> > >>>> Reported-by: Mark Pearson <mark.pearson@lenovo.com > <mailto:mark.pearson@lenovo.com>> > >>>> Signed-off-by: Marcus Bergo <marcusbergo@gmail.com > <mailto:marcusbergo@gmail.com>> > >>> > >>> Great finding with this being a potential root cause of this > behavior > >>> (at least from a Linux perspective). > >>> > >>> Although this helps, I'm not really a fan of the tech debt > >>> accumulated by needing to quirk this on a system by system > basis as a > >>> bandage. > >>> > >>> At least for HP someone said that this commit happens to help them > >>> for the same issue you're describing: > >>> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform- > <https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-> > >>> drivers- x86.git/commit/? > >>> h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d > >>> > >>> That was surprising to me, but it must be changing the timing > of some > >>> of the code running in HP's EC. Since you happen to have a Lenovo > >>> system does it happen to help the Lenovo EC too? > >>> > >>> Mark, comments please? > >>> > >> Someone just reported that the timing delay patch helped their > Lenovo > >> system as well. Can you see if it helps you too? > > > > -- > > > * > * > * > Marcus Bergo*
On Mon, Apr 28, 2025 at 8:23 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 4/28/2025 4:51 AM, Marcus Bergo wrote: > > Yes, it does. > > > > OK thanks for confirming. Considering your finding with this patch > you've shared and knowing there is a timing dependency that delaying the > next s2idle cycle helps I do wonder if we should keep exploring. > > Rafael, do you have thoughts here? Specifically do you think it's worth > revisiting if b5539eb5ee70 was the correct move. Well, it was done for a reason that is explained in its changelog. I think that the problem addressed by it is genuine, isn't it? > > On Sun, Apr 27, 2025 at 11:06 PM Mario Limonciello > > <mario.limonciello@amd.com <mailto:mario.limonciello@amd.com>> wrote: > > > > On 4/27/2025 1:34 PM, M. Bergo wrote: > > > It does make it work fine for me, I saw the clock/timing > > interference > > > and this sane this problem for Lenovo as well. > > > > Huh? I think you have a typo; but I don't know what you actually meant. > > > > So you're saying the timing patch helps your system as well? > > > > Thanks, > > > > > > > > On 4/24/25 11:11 AM, Mario Limonciello wrote: > > >> On 4/19/2025 1:03 PM, Mario Limonciello wrote: > > >>> On 4/19/2025 4:28 AM, M. Bergo wrote: > > >>>> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 > > 00:00:00 2001 > > >>>> From: Marcus Bergo <marcusbergo@gmail.com > > <mailto:marcusbergo@gmail.com>> > > >>>> Date: Sat, 19 Apr 2025 05:19:05 -0300 > > >>>> Subject: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD > > >>>> platforms after > > >>>> suspend/resume > > >>>> > > >>>> Several AMD-based laptop models (Lenovo P15v Gen 3, P16v Gen > > 1, HP > > >>>> EliteBook 845 G10) > > >>>> experience a CPU frequency limitation issue where the > > processor gets > > >>>> stuck at > > >>>> approximately 544MHz after resuming from suspend when the > > power cord > > >>>> is unplugged > > >>>> during sleep. This issue makes the systems practically unusable > > >>>> until a full > > >>>> power cycle is performed. > > >>>> > > >>>> The root cause was traced to commit b5539eb5ee70 ("ACPI: EC: Fix > > >>>> acpi_ec_dispatch_gpe()") which restored the behavior of > > clearing the > > >>>> GPE > > >>>> in acpi_ec_dispatch_gpe() function to prevent GPE storms. > > While this > > >>>> fix is > > >>>> necessary for most platforms to prevent excessive power > > consumption > > >>>> during > > >>>> suspend-to-idle, it causes problems on certain AMD platforms by > > >>>> interfering > > >>>> with the EC's ability to properly restore power management > > settings > > >>>> after resume. > > >>>> > > >>>> This patch implements a targeted workaround that: > > >>>> 1. Adds DMI-based detection for affected AMD platforms > > >>>> 2. Adds a function to check if we're in suspend-to-idle mode > > >>>> 3. Modifies the acpi_ec_dispatch_gpe() function to handle AMD > > >>>> platforms specially: > > >>>> - For affected AMD platforms during suspend-to-idle, it > > advances > > >>>> the > > >>>> transaction without clearing the GPE status bit > > >>>> - For all other platforms, it maintains the existing > > behavior of > > >>>> clearing > > >>>> the GPE status bit > > >>>> > > >>>> Testing was performed on a Lenovo P16v Gen 1 with AMD Ryzen 7 PRO > > >>>> 7840HS and > > >>>> confirmed that: > > >>>> 1. Without the patch, the CPU frequency is limited to 544MHz > > after the > > >>>> suspend/unplug/resume sequence > > >>>> 2. With the patch applied, the CPU properly scales up to its > > maximum > > >>>> frequency > > >>>> (5.1GHz) after the same sequence > > >>>> 3. No regressions were observed in other EC functionality > > (battery > > >>>> status, > > >>>> keyboard backlight, etc.) > > >>>> 4. Multiple suspend/resume cycles with different power states > > were > > >>>> tested > > >>>> without issues > > >>>> > > >>>> The patch was also verified not to affect the behavior on Intel- > > >>>> based systems, > > >>>> ensuring that the GPE storm prevention remains effective where > > needed. > > >>>> > > >>>> Fixes: b5539eb5ee70 ("ACPI: EC: Fix acpi_ec_dispatch_gpe()") > > >>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218557 > > <https://bugzilla.kernel.org/show_bug.cgi?id=218557> > > >>>> Reported-by: Mark Pearson <mark.pearson@lenovo.com > > <mailto:mark.pearson@lenovo.com>> > > >>>> Signed-off-by: Marcus Bergo <marcusbergo@gmail.com > > <mailto:marcusbergo@gmail.com>> > > >>> > > >>> Great finding with this being a potential root cause of this > > behavior > > >>> (at least from a Linux perspective). > > >>> > > >>> Although this helps, I'm not really a fan of the tech debt > > >>> accumulated by needing to quirk this on a system by system > > basis as a > > >>> bandage. > > >>> > > >>> At least for HP someone said that this commit happens to help them > > >>> for the same issue you're describing: > > >>> > > >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform- > > <https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-> > > >>> drivers- x86.git/commit/? > > >>> h=fixes&id=9f5595d5f03fd4dc640607a71e89a1daa68fd19d > > >>> > > >>> That was surprising to me, but it must be changing the timing > > of some > > >>> of the code running in HP's EC. Since you happen to have a Lenovo > > >>> system does it happen to help the Lenovo EC too? > > >>> > > >>> Mark, comments please? > > >>> > > >> Someone just reported that the timing delay patch helped their > > Lenovo > > >> system as well. Can you see if it helps you too? > > > > > > > > -- > > > > > > * > > * > > * > > Marcus Bergo* > >
On 4/28/2025 2:02 PM, Rafael J. Wysocki wrote: > On Mon, Apr 28, 2025 at 8:23 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> On 4/28/2025 4:51 AM, Marcus Bergo wrote: >>> Yes, it does. >>> >> >> OK thanks for confirming. Considering your finding with this patch >> you've shared and knowing there is a timing dependency that delaying the >> next s2idle cycle helps I do wonder if we should keep exploring. >> >> Rafael, do you have thoughts here? Specifically do you think it's worth >> revisiting if b5539eb5ee70 was the correct move. > > Well, it was done for a reason that is explained in its changelog. I > think that the problem addressed by it is genuine, isn't it? > I mean yes - of course. My inquiry was whether this should be the default behavior or if it should have been a quirked behavior. I don't have a good sense for the rest of the ecosystem what the impacts would really be at flipping it. Would it be worth adding a module parameter debug knob and survey what happens on a wide variety of machines?
On Mon, Apr 28, 2025 at 9:11 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 4/28/2025 2:02 PM, Rafael J. Wysocki wrote: > > On Mon, Apr 28, 2025 at 8:23 PM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> On 4/28/2025 4:51 AM, Marcus Bergo wrote: > >>> Yes, it does. > >>> > >> > >> OK thanks for confirming. Considering your finding with this patch > >> you've shared and knowing there is a timing dependency that delaying the > >> next s2idle cycle helps I do wonder if we should keep exploring. > >> > >> Rafael, do you have thoughts here? Specifically do you think it's worth > >> revisiting if b5539eb5ee70 was the correct move. > > > > Well, it was done for a reason that is explained in its changelog. I > > think that the problem addressed by it is genuine, isn't it? > > > I mean yes - of course. My inquiry was whether this should be the > default behavior or if it should have been a quirked behavior. I believe that it should be the default behavior because the EC GPE needs to be cleared after handling an EC event which effectively is what the suspend-to-idle code does. > I don't have a good sense for the rest of the ecosystem what the impacts > would really be at flipping it. Would it be worth adding a module > parameter debug knob and survey what happens on a wide variety of machines? Maybe, if you suspect that this might be a widespread issue.
On 4/28/2025 2:45 PM, Rafael J. Wysocki wrote: > On Mon, Apr 28, 2025 at 9:11 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> On 4/28/2025 2:02 PM, Rafael J. Wysocki wrote: >>> On Mon, Apr 28, 2025 at 8:23 PM Mario Limonciello >>> <mario.limonciello@amd.com> wrote: >>>> >>>> On 4/28/2025 4:51 AM, Marcus Bergo wrote: >>>>> Yes, it does. >>>>> >>>> >>>> OK thanks for confirming. Considering your finding with this patch >>>> you've shared and knowing there is a timing dependency that delaying the >>>> next s2idle cycle helps I do wonder if we should keep exploring. >>>> >>>> Rafael, do you have thoughts here? Specifically do you think it's worth >>>> revisiting if b5539eb5ee70 was the correct move. >>> >>> Well, it was done for a reason that is explained in its changelog. I >>> think that the problem addressed by it is genuine, isn't it? >>> >> I mean yes - of course. My inquiry was whether this should be the >> default behavior or if it should have been a quirked behavior. > > I believe that it should be the default behavior because the EC GPE > needs to be cleared after handling an EC event which effectively is > what the suspend-to-idle code does. > >> I don't have a good sense for the rest of the ecosystem what the impacts >> would really be at flipping it. Would it be worth adding a module >> parameter debug knob and survey what happens on a wide variety of machines? > > Maybe, if you suspect that this might be a widespread issue. Marcus, Before going down this path I have an important confirmation I need from you. With just /your/ patch in place did you see a message like this in your kernel log? amd_pmc AMDI000A:00: Last suspend didn't reach deepest state If so; your patch just papered over the real issue and blocked the system from getting into a deep state.
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 3c5f34892734..f3698f3c100f 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -29,6 +29,7 @@ #include <linux/acpi.h> #include <linux/dmi.h> #include <asm/io.h> +#include <asm/processor.h> #include "internal.h" @@ -2139,6 +2140,59 @@ static bool acpi_ec_work_in_progress(struct acpi_ec *ec)