[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b6fc4e66-b35a-41ce-a633-db3d660b88a2@amd.com>
Date: Sat, 19 Apr 2025 13:03:14 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: "M. Bergo" <marcusbergo@...il.com>, mark.pearson@...ovo.com
Cc: linux-acpi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
rafael@...nel.org, lenb@...nel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ACPI: EC: Fix CPU frequency limitation on AMD platforms
after suspend/resume
On 4/19/2025 4:28 AM, M. Bergo wrote:
> From 881e57c87b9595c186c2ca7e6d35d0a52c1a10c2 Mon Sep 17 00:00:00 2001
> From: Marcus Bergo <marcusbergo@...il.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@...ovo.com>
> Signed-off-by: Marcus Bergo <marcusbergo@...il.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);
> }
Powered by blists - more mailing lists