Skip to contentSkip to conversation

amd_pmc: Backport additional delay_suspend patches for linux-6.6.y

PATCH Applied Daniel Gibson 2 participants 5 messages Jul 22 – Jul 25

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover amd_pmc: Backport additional delay_suspend patches for linux-6.6.y

These commits belong to:
bc08f495c7b8 platform/x86/amd/pmc: Check for intermediate wakeup in function caa25575c3c2 platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops 0c9901b84b59 platform/x86/amd/pmc: Add delay_suspend module parameter

The "Don't log during intermediate wakeups" commit prevents the aforementioned commits from spamming dmesg during intermediate wakeups. The affected IdeaPad devices have *lots* of intermediate wakeups when charging while in s2idle.

The other commit is more cosmetical, but is also trivial and ensures that the log messages log the same on all kernel versions with these patches.

These commits are already merged in mainline and in the stable-rc branches for 6.12, 6.18 and 7.1.
In 6.6 they were missing because of merge conflicts. I resolved those. See also https://lore.kernel.org/stable/2026072230-volatile-citric-73fa@gregkh/

Daniel Gibson (2):
platform/x86/amd/pmc: Don't log during intermediate wakeups platform/x86/amd/pmc: Avoid logging "(null)" for DMI values

 drivers/platform/x86/amd/pmc/pmc.c | 39 ++++++++++++++++++++++++------
 drivers/platform/x86/amd/pmc/pmc.h |  1 +
 2 files changed, 32 insertions(+), 8 deletions(-)
Signature · 1 line
2.48.1
#1 lore raw

PATCH 1/2 platform/x86/amd/pmc: Don't log during intermediate wakeups

commit 037f0b03c663a247366673a807834389107995b7 upstream.

The ECs in the IdeaPads that need the delay_suspend quirk send lots of messages when charging, which not only causes intermediate wakeups when suspended, but also prevents the device from reaching the deepest suspend state.

Because of this amd_pmc_intermediate_wakeup_need_delay() returns false during intermediate wakeups and amd_pmc_want_suspend_delay() is called. So far it always logged its "Delaying suspend by 2.5s ..." messages then, which spams dmesg. This commit makes sure that those messages are only logged once per suspend.

---
 drivers/platform/x86/amd/pmc/pmc.c | 39 ++++++++++++++++++++++++------
 drivers/platform/x86/amd/pmc/pmc.h |  1 +
 2 files changed, 32 insertions(+), 8 deletions(-)
drivers/platform/x86/amd/pmc/pmc.c +31 -8
index d5b543f74a22..b09512368877 100644
@@ -811,6 +811,20 @@ static bool amd_pmc_intermediate_wakeup_need_delay(struct amd_pmc_dev *pdev)
 
 static bool amd_pmc_want_suspend_delay(struct amd_pmc_dev *pdev)
 {
+	/*
+	 * intermediate_wakeup implies that the machine didn't get to deepest sleep
+	 * state before - otherwise this function isn't called in amd_pmc_s2idle_check()
+	 * because amd_pmc_intermediate_wakeup_need_delay() returns true first.
+	 * On some IdeaPads that happens when charging, because the EC seems
+	 * to send lots of messages then that wake the machine.
+	 *
+	 * But even in that case, the sleep here is necessary (on those IdeaPads),
+	 * otherwise they wake up completely (resume) after a few seconds.
+	 * So this variable is only used to avoid spamming dmesg on each
+	 * intermediate wakeup.
+	 */
+	bool intermediate_wakeup = !pdev->is_first_check_after_suspend;
+
 	/*
 	 * Some Lenovo Laptops (like different IdeaPad 3 Slims) need some
 	 * me-time before sleeping or they get uncooperative after waking
@@ -829,17 +843,20 @@ static bool amd_pmc_want_suspend_delay(struct amd_pmc_dev *pdev)
 		 * disabled with disable_workarounds or delay_suspend=0
 		 */
 		if (delay_suspend == 1 || (delay_suspend == -1 && !disable_workarounds)) {
-			dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
+			if (!intermediate_wakeup)
+				dev_info(pdev->dev, "Delaying suspend by 2.5s to avoid platform bug\n");
 			return true;
 		}
-		dev_info(pdev->dev, "Not delaying suspend because of module parameter, even though your device is assumed to need it!\n");
+		if (!intermediate_wakeup)
+			dev_info(pdev->dev, "Not delaying suspend because of module parameter, even though your device is assumed to need it!\n");
 	} else if (delay_suspend == 1) {
-		dev_info(pdev->dev, "Delaying suspend by 2.5s because delay_suspend=1. If this solves problems on your machine, please report this whole line to: [email protected] so it can be automatically detected as affected in the future. System Vendor: \"%s\" Product Name: \"%s\" Product Family: \"%s\" Board Vendor: \"%s\" Board Name: \"%s\"\n",
-			 dmi_get_system_info(DMI_SYS_VENDOR),
-			 dmi_get_system_info(DMI_PRODUCT_NAME),
-			 dmi_get_system_info(DMI_PRODUCT_FAMILY),
-			 dmi_get_system_info(DMI_BOARD_VENDOR),
-			 dmi_get_system_info(DMI_BOARD_NAME));
+		if (!intermediate_wakeup)
+			dev_info(pdev->dev, "Delaying suspend by 2.5s because delay_suspend=1. If this solves problems on your machine, please report this whole line to: [email protected] so it can be automatically detected as affected in the future. System Vendor: \"%s\" Product Name: \"%s\" Product Family: \"%s\" Board Vendor: \"%s\" Board Name: \"%s\"\n",
+				 dmi_get_system_info(DMI_SYS_VENDOR),
+				 dmi_get_system_info(DMI_PRODUCT_NAME),
+				 dmi_get_system_info(DMI_PRODUCT_FAMILY),
+				 dmi_get_system_info(DMI_BOARD_VENDOR),
+				 dmi_get_system_info(DMI_BOARD_NAME));
 		return true;
 	}
 	return false;
@@ -852,6 +869,9 @@ static void amd_pmc_s2idle_prepare(void)
 	u8 msg;
 	u32 arg = 1;
 
+	/* Reset this variable because this is a fresh suspend */
+	pdev->is_first_check_after_suspend = true;
+
 	/* Reset and Start SMU logging - to monitor the s0i3 stats */
 	amd_pmc_setup_smu_logging(pdev);
 
@@ -891,6 +911,9 @@ static void amd_pmc_s2idle_check(void)
 	rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_S2IDLE_CHECK);
 	if (rc)
 		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
+
+	/* remember that first check after suspend is done (until next prepare) */
+	pdev->is_first_check_after_suspend = false;
 }
 
 static int amd_pmc_dump_data(struct amd_pmc_dev *pdev)
drivers/platform/x86/amd/pmc/pmc.h +1 -0
index 5e7b8d5dc5d6..bea65810de5a 100644
@@ -37,6 +37,7 @@ struct amd_pmc_dev {
 	struct dentry *dbgfs_dir;
 	struct quirk_entry *quirks;
 	bool disable_8042_wakeup;
+	bool is_first_check_after_suspend;
 };
 
 void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev);
Signature · 1 line
2.48.1
#2 lore raw

PATCH 2/2 platform/x86/amd/pmc: Avoid logging "(null)" for DMI values

In reply to Daniel Gibson

commit a0738abd042f7406edd2175a819cf2e66388ed97 upstream.

dmi_get_system_info(...) can return NULL. Using that as %s arguments of dev_info() would log "(null)" (as part of a message like '... System Vendor: "(null)", Product Name: "(null)" ...'), which may be confusing for users.

Use Elvis operator to print "(Unknown)" instead.

---
 drivers/platform/x86/amd/pmc/pmc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/platform/x86/amd/pmc/pmc.c +5 -5
index b09512368877..bf6c7ca247e2 100644
@@ -852,11 +852,11 @@ static bool amd_pmc_want_suspend_delay(struct amd_pmc_dev *pdev)
 	} else if (delay_suspend == 1) {
 		if (!intermediate_wakeup)
 			dev_info(pdev->dev, "Delaying suspend by 2.5s because delay_suspend=1. If this solves problems on your machine, please report this whole line to: [email protected] so it can be automatically detected as affected in the future. System Vendor: \"%s\" Product Name: \"%s\" Product Family: \"%s\" Board Vendor: \"%s\" Board Name: \"%s\"\n",
-				 dmi_get_system_info(DMI_SYS_VENDOR),
-				 dmi_get_system_info(DMI_PRODUCT_NAME),
-				 dmi_get_system_info(DMI_PRODUCT_FAMILY),
-				 dmi_get_system_info(DMI_BOARD_VENDOR),
-				 dmi_get_system_info(DMI_BOARD_NAME));
+				 dmi_get_system_info(DMI_SYS_VENDOR) ?: "(Unknown)",
+				 dmi_get_system_info(DMI_PRODUCT_NAME) ?: "(Unknown)",
+				 dmi_get_system_info(DMI_PRODUCT_FAMILY) ?: "(Unknown)",
+				 dmi_get_system_info(DMI_BOARD_VENDOR) ?: "(Unknown)",
+				 dmi_get_system_info(DMI_BOARD_NAME) ?: "(Unknown)");
 		return true;
 	}
 	return false;
Signature · 1 line
2.48.1
maintainer Applied
#3 lore raw
In reply to Daniel Gibson
Quoted 3 lines

Daniel Gibson (2):
platform/x86/amd/pmc: Don't log during intermediate wakeups platform/x86/amd/pmc: Avoid logging "(null)" for DMI values

Queued the series for 6.6, thanks.

Signature · 2 lines
Thanks,
Sasha
#4 lore raw
Quoted 6 lines · Sasha Levin
Quoted 3 lines

Daniel Gibson (2):
platform/x86/amd/pmc: Don't log during intermediate wakeups platform/x86/amd/pmc: Avoid logging "(null)" for DMI values

Queued the series for 6.6, thanks.

Thank you very much! :)

Cheers,
Daniel