* [PATCH 0/2] amd_pmc: Delay s2idle suspend for some devices
@ 2026-05-01 3:26 Daniel Gibson
2026-05-01 3:26 ` [PATCH 1/2] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops Daniel Gibson
2026-05-01 3:26 ` [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument Daniel Gibson
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Gibson @ 2026-05-01 3:26 UTC (permalink / raw)
To: Shyam Sundar S K, Hans de Goede, Ilpo Järvinen,
platform-driver-x86, linux-kernel
Cc: Mario Limonciello, Daniel Gibson
On some AMD Zen3 and Zen3+-based Lenovo IdeaPad laptops the keyboard and
the lid switch stop working after the first suspend, until rebooted.
More specifically, they stop sending events when pressing a key or
closing the lid - it's still possible to toggle the capslock- and
numlock-LEDs with an external keyboard or read the lid state at
/proc/acpi/button/lid/LID/state.
See also https://bugzilla.kernel.org/show_bug.cgi?id=221383
It appears that suspending and/or resuming gets the EC into a broken
state. This problem doesn't happen on Windows and Mario Limonciello
mentioned that the Windows kernel gives hardware and software some time
before actually suspending (before activating HW DRIPS), while Linux
(or the amd_pmc module) does that immediately, so it may be worth trying
if calling msleep() in amd_pmc_s2idle_check() helps.
It turned out that sleeping for 2.5 seconds at that point indeed makes
the problems disappear. Sleeping for 1.5 seconds wasn't enough.
Luckily there was already a quirk that does exactly that under other
circumstances, so I could build on that:
https://lore.kernel.org/platform-driver-x86/20250414162446.3853194-1-superm1@kernel.org/
I enable this quirk for two lines of Lenovo Laptops that are known to
have this problem and for which the patch has been successfully tested.
I found several reports of these or similar issues on the web, for
different devices, so in a second commit I added a parameter to the
kernel module that allows enabling or disabling this, which will make
it easy for people whose devices aren't matched yet to test this quirk.
Thanks to Mario Limonciello for his support and to Sindre Henriksen
for testing my patch!
Daniel Gibson (2):
platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops
platform/x86/amd/pmc: Add delay_suspend module argument
drivers/platform/x86/amd/pmc/pmc-quirks.c | 36 +++++++++++++++++++++++
drivers/platform/x86/amd/pmc/pmc.c | 18 +++++++++++-
drivers/platform/x86/amd/pmc/pmc.h | 1 +
3 files changed, 54 insertions(+), 1 deletion(-)
--
2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops
2026-05-01 3:26 [PATCH 0/2] amd_pmc: Delay s2idle suspend for some devices Daniel Gibson
@ 2026-05-01 3:26 ` Daniel Gibson
2026-05-01 3:26 ` [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument Daniel Gibson
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Gibson @ 2026-05-01 3:26 UTC (permalink / raw)
To: Shyam Sundar S K, Hans de Goede, Ilpo Järvinen,
platform-driver-x86, linux-kernel
Cc: Mario Limonciello, Daniel Gibson, Sindre Henriksen
Some IdeaPad Slim 3 devices and similar with AMD CPUs have a
nonfunctional keyboard and lid switch after s2idle.
It helps to delay suspend by 2.5 seconds so the EC has some time
to do whatever it needs to get done before suspend.
This issue has been reported for many different devices, this patch
has been tested with the Zen3-based IdeaPad Slim 3 16ABR8 (82XR)
and the Zen3+-based IdeaPad Slim 3 14ARP10 (83K6), see
https://bugzilla.kernel.org/show_bug.cgi?id=221383
Reported-by: Sindre Henriksen <sindrehenriksen93@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221383
Tested-by: Sindre Henriksen <sindrehenriksen93@gmail.com>
Tested-by: Daniel Gibson <daniel@gibson.sh>
Suggested-by: Mario Limonciello (AMD) <superm1@kernel.org>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Daniel Gibson <daniel@gibson.sh>
---
drivers/platform/x86/amd/pmc/pmc-quirks.c | 36 +++++++++++++++++++++++
drivers/platform/x86/amd/pmc/pmc.c | 5 +++-
drivers/platform/x86/amd/pmc/pmc.h | 1 +
3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmc/pmc-quirks.c b/drivers/platform/x86/amd/pmc/pmc-quirks.c
index 24506e342943..cea30f68f8dc 100644
--- a/drivers/platform/x86/amd/pmc/pmc-quirks.c
+++ b/drivers/platform/x86/amd/pmc/pmc-quirks.c
@@ -18,6 +18,7 @@
struct quirk_entry {
u32 s2idle_bug_mmio;
bool spurious_8042;
+ bool need_suspend_delay;
};
static struct quirk_entry quirk_s2idle_bug = {
@@ -33,6 +34,10 @@ static struct quirk_entry quirk_s2idle_spurious_8042 = {
.spurious_8042 = true,
};
+static struct quirk_entry quirk_s2idle_need_suspend_delay = {
+ .need_suspend_delay = true
+};
+
static const struct dmi_system_id fwbug_list[] = {
{
.ident = "L14 Gen2 AMD",
@@ -203,6 +208,32 @@ static const struct dmi_system_id fwbug_list[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "82XQ"),
}
},
+ /*
+ * Some Lenovo Laptops (like different IdeaPad 3 Slims) need some
+ * me-time before sleeping or they get uncooperative after waking
+ * up and don't send events for keyboard and lid switch anymore.
+ * See https://bugzilla.kernel.org/show_bug.cgi?id=221383
+ */
+ {
+ .ident = "Zen3-based IdeaPad Slim and similar",
+ .driver_data = &quirk_s2idle_need_suspend_delay,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+ /*
+ * Note: there are also some Zen2-based 82X* devices that
+ * need different quirks, they're already handled above
+ */
+ DMI_MATCH(DMI_PRODUCT_NAME, "82X")
+ }
+ },
+ {
+ .ident = "Zen3+-based IdeaPad Slim and similar",
+ .driver_data = &quirk_s2idle_need_suspend_delay,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "83K")
+ }
+ },
/* https://bugzilla.kernel.org/show_bug.cgi?id=221273 */
{
.ident = "Thinkpad L14 Gen3",
@@ -356,6 +387,11 @@ void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev)
amd_pmc_skip_nvme_smi_handler(dev->quirks->s2idle_bug_mmio);
}
+bool amd_pmc_quirk_need_suspend_delay(struct amd_pmc_dev *dev)
+{
+ return dev->quirks->need_suspend_delay;
+}
+
void amd_pmc_quirks_init(struct amd_pmc_dev *dev)
{
const struct dmi_system_id *dmi_id;
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index cae3fcafd4d7..c604dc7207ed 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -634,10 +634,13 @@ static void amd_pmc_s2idle_check(void)
struct amd_pmc_dev *pdev = &pmc;
struct smu_metrics table;
int rc;
+ bool ec_needs_sleep = !disable_workarounds && amd_pmc_quirk_need_suspend_delay(pdev);
/* Avoid triggering OVP */
- if (!get_metrics_table(pdev, &table) && table.s0i3_last_entry_status)
+ if (ec_needs_sleep || (!get_metrics_table(pdev, &table) && table.s0i3_last_entry_status)) {
+ dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
msleep(2500);
+ }
/* Dump the IdleMask before we add to the STB */
amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
index fe3f53eb5955..f5257e47b8c4 100644
--- a/drivers/platform/x86/amd/pmc/pmc.h
+++ b/drivers/platform/x86/amd/pmc/pmc.h
@@ -147,6 +147,7 @@ enum amd_pmc_def {
};
void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev);
+bool amd_pmc_quirk_need_suspend_delay(struct amd_pmc_dev *dev);
void amd_pmc_quirks_init(struct amd_pmc_dev *dev);
void amd_mp2_stb_init(struct amd_pmc_dev *dev);
void amd_mp2_stb_deinit(struct amd_pmc_dev *dev);
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument
2026-05-01 3:26 [PATCH 0/2] amd_pmc: Delay s2idle suspend for some devices Daniel Gibson
2026-05-01 3:26 ` [PATCH 1/2] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops Daniel Gibson
@ 2026-05-01 3:26 ` Daniel Gibson
2026-05-04 14:37 ` Mario Limonciello
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Gibson @ 2026-05-01 3:26 UTC (permalink / raw)
To: Shyam Sundar S K, Hans de Goede, Ilpo Järvinen,
platform-driver-x86, linux-kernel
Cc: Mario Limonciello, Daniel Gibson
Enabling it delays suspend for 2.5 seconds which is known to help for
some AMD-based Lenovo Laptops that otherwise failed to send/receive
events for key presses or the lid switch after s2idle.
Apparently the EC needs to do some things in the background before
suspend or it gets into a bad state.
There are many reports of AMD-based laptops (mostly but not exclusively
IdeaPads) about similar issues on the web; this parameter gives
affected users an easy way to try out if their issues have the same
root cause and to work around them until their specific device is added
to the quirks list.
I added a note to the parameter description encouraging users to report
their device so it can be added to the quirks list, inspired by a
similar request in parameter descriptions of the ideapad-laptop module.
The module parameter can be set to "1" to explicitly enable it,
"0" to disable it even on devices that are assumed to be affected,
or -1 (the default) to enable it if the device is assumed to be affected
(according to fwbug_list[])
This is related to https://bugzilla.kernel.org/show_bug.cgi?id=221383
Signed-off-by: Daniel Gibson <daniel@gibson.sh>
Tested-by: Daniel Gibson <daniel@gibson.sh>
---
drivers/platform/x86/amd/pmc/pmc.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index c604dc7207ed..f76936036d1f 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -89,6 +89,11 @@ static bool disable_workarounds;
module_param(disable_workarounds, bool, 0644);
MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs");
+static int delay_suspend = -1;
+module_param(delay_suspend, int, 0644);
+MODULE_PARM_DESC(delay_suspend,
+ "Delays s2idle by 2.5 seconds to work around buggy ECs, often causing keyboard issues after suspend. 0: don't delay, 1: do delay, -1 (default): let amd_pmc decide. If you need this please report this to: platform-driver-x86@vger.kernel.org");
+
static struct amd_pmc_dev pmc;
static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
@@ -634,11 +639,19 @@ static void amd_pmc_s2idle_check(void)
struct amd_pmc_dev *pdev = &pmc;
struct smu_metrics table;
int rc;
- bool ec_needs_sleep = !disable_workarounds && amd_pmc_quirk_need_suspend_delay(pdev);
+ bool ec_needs_sleep;
+
+ if (delay_suspend < 0)
+ ec_needs_sleep = !disable_workarounds && amd_pmc_quirk_need_suspend_delay(pdev);
+ else
+ ec_needs_sleep = delay_suspend != 0;
/* Avoid triggering OVP */
if (ec_needs_sleep || (!get_metrics_table(pdev, &table) && table.s0i3_last_entry_status)) {
- dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
+ if (delay_suspend > 0)
+ dev_info(pdev->dev, "Delaying suspend by 2.5s because delay_suspend=1\n");
+ else
+ dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
msleep(2500);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument
2026-05-01 3:26 ` [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument Daniel Gibson
@ 2026-05-04 14:37 ` Mario Limonciello
2026-05-04 15:38 ` Daniel Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello @ 2026-05-04 14:37 UTC (permalink / raw)
To: Daniel Gibson, Shyam Sundar S K, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-kernel
On 4/30/26 22:26, Daniel Gibson wrote:
> [You don't often get email from daniel@gibson.sh. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Enabling it delays suspend for 2.5 seconds which is known to help for
> some AMD-based Lenovo Laptops that otherwise failed to send/receive
> events for key presses or the lid switch after s2idle.
> Apparently the EC needs to do some things in the background before
> suspend or it gets into a bad state.
>
> There are many reports of AMD-based laptops (mostly but not exclusively
> IdeaPads) about similar issues on the web; this parameter gives
> affected users an easy way to try out if their issues have the same
> root cause and to work around them until their specific device is added
> to the quirks list.
> I added a note to the parameter description encouraging users to report
> their device so it can be added to the quirks list, inspired by a
> similar request in parameter descriptions of the ideapad-laptop module.
>
> The module parameter can be set to "1" to explicitly enable it,
> "0" to disable it even on devices that are assumed to be affected,
> or -1 (the default) to enable it if the device is assumed to be affected
> (according to fwbug_list[])
>
> This is related to https://bugzilla.kernel.org/show_bug.cgi?id=221383
>
> Signed-off-by: Daniel Gibson <daniel@gibson.sh>
> Tested-by: Daniel Gibson <daniel@gibson.sh>
> ---
> drivers/platform/x86/amd/pmc/pmc.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index c604dc7207ed..f76936036d1f 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -89,6 +89,11 @@ static bool disable_workarounds;
> module_param(disable_workarounds, bool, 0644);
> MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs");
>
> +static int delay_suspend = -1;
> +module_param(delay_suspend, int, 0644);
> +MODULE_PARM_DESC(delay_suspend,
> + "Delays s2idle by 2.5 seconds to work around buggy ECs, often causing keyboard issues after suspend. 0: don't delay, 1: do delay, -1 (default): let amd_pmc decide. If you need this please report this to: platform-driver-x86@vger.kernel.org");
> +
> static struct amd_pmc_dev pmc;
Generally speaking; I don't like new module parameters.
Since this is purely for debugging purposes and is going to hopefully
lead to a new patch; how about if it's done in debugfs?
I have a couple reasons I say this.
1. If you make it "too easy" to make the kernel command line permanent
people won't report bugs and they never get fixed.
2. It needs to become ABI essentially forever, even if we found out some
day this was actually something we can fix with other kernel changes
instead.
But then the problem arguing in the direction of a module parameter is
discoverability of this debugging knob. And for that; I would suggest
to add a bullet here: https://docs.kernel.org/arch/x86/amd-debugging.html
Then when people report this problem we can point them to that document
indicating they can use it.
>
> static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
> @@ -634,11 +639,19 @@ static void amd_pmc_s2idle_check(void)
> struct amd_pmc_dev *pdev = &pmc;
> struct smu_metrics table;
> int rc;
> - bool ec_needs_sleep = !disable_workarounds && amd_pmc_quirk_need_suspend_delay(pdev);
> + bool ec_needs_sleep;
> +
> + if (delay_suspend < 0)
> + ec_needs_sleep = !disable_workarounds && amd_pmc_quirk_need_suspend_delay(pdev);
> + else
> + ec_needs_sleep = delay_suspend != 0;
>
> /* Avoid triggering OVP */
> if (ec_needs_sleep || (!get_metrics_table(pdev, &table) && table.s0i3_last_entry_status)) {
> - dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
> + if (delay_suspend > 0)
> + dev_info(pdev->dev, "Delaying suspend by 2.5s because delay_suspend=1\n");
Rather than telling them to report to platform-driver-x86@ in the module
parameter description I would suggest you should be putting it in this
message. I think that's what some other drivers do when they want
people to report bugs.
> + else
> + dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
> msleep(2500);
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument
2026-05-04 14:37 ` Mario Limonciello
@ 2026-05-04 15:38 ` Daniel Gibson
2026-05-04 16:58 ` Mario Limonciello
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Gibson @ 2026-05-04 15:38 UTC (permalink / raw)
To: Mario Limonciello, Shyam Sundar S K, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-kernel
On 04.05.26 16:37, Mario Limonciello wrote:
> On 4/30/26 22:26, Daniel Gibson wrote:
(...)
>> +static int delay_suspend = -1;
>> +module_param(delay_suspend, int, 0644);
>> +MODULE_PARM_DESC(delay_suspend,
>> + "Delays s2idle by 2.5 seconds to work around buggy
>> ECs, often causing keyboard issues after suspend. 0: don't delay, 1:
>> do delay, -1 (default): let amd_pmc decide. If you need this please
>> report this to: platform-driver-x86@vger.kernel.org");
>> +
>> static struct amd_pmc_dev pmc;
>
> Generally speaking; I don't like new module parameters.
>
> Since this is purely for debugging purposes and is going to hopefully
> lead to a new patch; how about if it's done in debugfs?
>
> I have a couple reasons I say this.
>
> 1. If you make it "too easy" to make the kernel command line permanent
> people won't report bugs and they never get fixed.
Sorry, I don't think this makes a difference.
Even people using a bleeding-edge distro like Arch Linux will have to
wait at least a month or so after reporting their device until a stable
kernel with that quirk is released and that kernel lands in their
distro. Much longer for users of more traditional distros.
No one wants to have a broken laptop for months if it can be avoided, so
they'd make that workaround permanent either way, it's just more
annoying to do when it's in debugfs.
If someone reports the bug it's because they think it's the right thing
to do (and maybe because the module parameter or dmesg message asked
them to), not because the workaround requires a systemd unit instead of
a kernel parameter.
>
> 2. It needs to become ABI essentially forever, even if we found out some
> day this was actually something we can fix with other kernel changes
> instead.
Does it? prefer_microsoft_dsm_guid was removed, did that break anything?
>
> But then the problem arguing in the direction of a module parameter is
> discoverability of this debugging knob. And for that; I would suggest
> to add a bullet here: https://docs.kernel.org/arch/x86/amd-debugging.html
>
> Then when people report this problem we can point them to that document
> indicating they can use it.
So far I've missed that document - it certainly looks like a good place
to document the issue and workaround.
Side-Note: "18.10. Random reboot issues" sounds interesting, I guess it
would be helpful to document how that value can be read or identified in
dmesg ;)
>
>>
>> static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int
>> reg_offset)
>> @@ -634,11 +639,19 @@ static void amd_pmc_s2idle_check(void)
>> struct amd_pmc_dev *pdev = &pmc;
>> struct smu_metrics table;
>> int rc;
>> - bool ec_needs_sleep = !disable_workarounds &&
>> amd_pmc_quirk_need_suspend_delay(pdev);
>> + bool ec_needs_sleep;
>> +
>> + if (delay_suspend < 0)
>> + ec_needs_sleep = !disable_workarounds &&
>> amd_pmc_quirk_need_suspend_delay(pdev);
>> + else
>> + ec_needs_sleep = delay_suspend != 0;
>>
>> /* Avoid triggering OVP */
>> if (ec_needs_sleep || (!get_metrics_table(pdev, &table) &&
>> table.s0i3_last_entry_status)) {
>> - dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid
>> platform bug\n");
>> + if (delay_suspend > 0)
>> + dev_info(pdev->dev, "Delaying suspend by 2.5s
>> because delay_suspend=1\n");
>
> Rather than telling them to report to platform-driver-x86@ in the module
> parameter description I would suggest you should be putting it in this
> message. I think that's what some other drivers do when they want
> people to report bugs.
Great idea!
>
>> + else
>> + dev_info(pdev->dev, "Delaying suspend by 2.5s
>> to avoid platform bug\n");
>> msleep(2500);
>> }
>>
>> --
>> 2.48.1
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument
2026-05-04 15:38 ` Daniel Gibson
@ 2026-05-04 16:58 ` Mario Limonciello
0 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2026-05-04 16:58 UTC (permalink / raw)
To: Daniel Gibson, Shyam Sundar S K, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-kernel
On 5/4/26 10:38, Daniel Gibson wrote:
> On 04.05.26 16:37, Mario Limonciello wrote:
>> On 4/30/26 22:26, Daniel Gibson wrote:
> (...)
>>> +static int delay_suspend = -1;
>>> +module_param(delay_suspend, int, 0644);
>>> +MODULE_PARM_DESC(delay_suspend,
>>> + "Delays s2idle by 2.5 seconds to work around buggy
>>> ECs, often causing keyboard issues after suspend. 0: don't delay, 1:
>>> do delay, -1 (default): let amd_pmc decide. If you need this please
>>> report this to: platform-driver-x86@vger.kernel.org");
>>> +
>>> static struct amd_pmc_dev pmc;
>>
>> Generally speaking; I don't like new module parameters.
>>
>> Since this is purely for debugging purposes and is going to hopefully
>> lead to a new patch; how about if it's done in debugfs?
>>
>> I have a couple reasons I say this.
>>
>> 1. If you make it "too easy" to make the kernel command line permanent
>> people won't report bugs and they never get fixed.
>
> Sorry, I don't think this makes a difference.
OK. I'd say up to the driver maintainers to decide which way to go.
> Even people using a bleeding-edge distro like Arch Linux will have to
> wait at least a month or so after reporting their device until a stable
> kernel with that quirk is released and that kernel lands in their
> distro. Much longer for users of more traditional distros.
>
> No one wants to have a broken laptop for months if it can be avoided, so
> they'd make that workaround permanent either way, it's just more
> annoying to do when it's in debugfs.
>
> If someone reports the bug it's because they think it's the right thing
> to do (and maybe because the module parameter or dmesg message asked
> them to), not because the workaround requires a systemd unit instead of
> a kernel parameter.
>
>>
>> 2. It needs to become ABI essentially forever, even if we found out some
>> day this was actually something we can fix with other kernel changes
>> instead.
>
> Does it? prefer_microsoft_dsm_guid was removed, did that break anything?
Very good point.
>
>>
>> But then the problem arguing in the direction of a module parameter is
>> discoverability of this debugging knob. And for that; I would suggest
>> to add a bullet here: https://docs.kernel.org/arch/x86/amd-debugging.html
>>
>> Then when people report this problem we can point them to that document
>> indicating they can use it.
>
> So far I've missed that document - it certainly looks like a good place
> to document the issue and workaround.
Yeah; whatever direction is agreed upon (module parameter or debugfs),
please update the document for a section about it.
>
> Side-Note: "18.10. Random reboot issues" sounds interesting, I guess it
> would be helpful to document how that value can be read or identified in
> dmesg ;)
"This information is read by the kernel at bootup and printed into the
syslog".
Do you mean actually outputting the prefix of the message in this document?
This is a sample message from my local machine.
x86/amd: Previous system reset reason [0x00080800]: software wrote 0x6
to reset control register 0xCF9
I have no qualms with this - feel free to send a patch if you're
touching it already anyways.
>
>>
>>>
>>> static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int
>>> reg_offset)
>>> @@ -634,11 +639,19 @@ static void amd_pmc_s2idle_check(void)
>>> struct amd_pmc_dev *pdev = &pmc;
>>> struct smu_metrics table;
>>> int rc;
>>> - bool ec_needs_sleep = !disable_workarounds &&
>>> amd_pmc_quirk_need_suspend_delay(pdev);
>>> + bool ec_needs_sleep;
>>> +
>>> + if (delay_suspend < 0)
>>> + ec_needs_sleep = !disable_workarounds &&
>>> amd_pmc_quirk_need_suspend_delay(pdev);
>>> + else
>>> + ec_needs_sleep = delay_suspend != 0;
>>>
>>> /* Avoid triggering OVP */
>>> if (ec_needs_sleep || (!get_metrics_table(pdev, &table) &&
>>> table.s0i3_last_entry_status)) {
>>> - dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid
>>> platform bug\n");
>>> + if (delay_suspend > 0)
>>> + dev_info(pdev->dev, "Delaying suspend by 2.5s
>>> because delay_suspend=1\n");
>>
>> Rather than telling them to report to platform-driver-x86@ in the module
>> parameter description I would suggest you should be putting it in this
>> message. I think that's what some other drivers do when they want
>> people to report bugs.
>
> Great idea!
>
>>
>>> + else
>>> + dev_info(pdev->dev, "Delaying suspend by 2.5s
>>> to avoid platform bug\n");
>>> msleep(2500);
>>> }
>>>
>>> --
>>> 2.48.1
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-04 16:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01 3:26 [PATCH 0/2] amd_pmc: Delay s2idle suspend for some devices Daniel Gibson
2026-05-01 3:26 ` [PATCH 1/2] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops Daniel Gibson
2026-05-01 3:26 ` [PATCH 2/2] platform/x86/amd/pmc: Add delay_suspend module argument Daniel Gibson
2026-05-04 14:37 ` Mario Limonciello
2026-05-04 15:38 ` Daniel Gibson
2026-05-04 16:58 ` Mario Limonciello
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox