The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Daniel Gibson <daniel@gibson.sh>
Cc: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	 Hans de Goede <hansg@kernel.org>,
	platform-driver-x86@vger.kernel.org,
	 LKML <linux-kernel@vger.kernel.org>,
	 Mario Limonciello <superm1@kernel.org>,
	 Hans de Goede <johannes.goede@oss.qualcomm.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v5 4/4] platform/x86/amd/pmc: Don't log during intermediate wakeups
Date: Thu, 11 Jun 2026 17:02:30 +0300 (EEST)	[thread overview]
Message-ID: <4bc20ca2-8544-e36e-70af-a19364e59eba@linux.intel.com> (raw)
In-Reply-To: <20260609105756.2813669-5-daniel@gibson.sh>

On Tue, 9 Jun 2026, Daniel Gibson wrote:

> 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.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221383
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Daniel Gibson <daniel@gibson.sh>
> Cc: stable@vger.kernel.org
> ---
>  drivers/platform/x86/amd/pmc/pmc.c | 39 ++++++++++++++++++++++++------
>  drivers/platform/x86/amd/pmc/pmc.h |  1 +
>  2 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 2d3d180c15d2..7d772ccd17a6 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -619,6 +619,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
> @@ -637,17 +651,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: platform-driver-x86@vger.kernel.org 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: platform-driver-x86@vger.kernel.org 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;
> @@ -660,6 +677,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);
>  
> @@ -699,6 +719,9 @@ static void amd_pmc_s2idle_check(void)
>  	rc = amd_stb_write(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)
> diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
> index f5257e47b8c4..8aa7073ed09f 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.h
> +++ b/drivers/platform/x86/amd/pmc/pmc.h
> @@ -114,6 +114,7 @@ struct amd_pmc_dev {
>  	struct dentry *dbgfs_dir;
>  	struct quirk_entry *quirks;
>  	bool disable_8042_wakeup;
> +	bool is_first_check_after_suspend;
>  	struct amd_mp2_dev *mp2;
>  	struct stb_arg stb_arg;
>  };
> 

Hi,

This fails to apply to the review-ilpo-next branch and I don't want to 
spend time at this point to figure it out so please send v6 which is 
based on the for-next or review-ilpo-next branch:

Applying: platform/x86/amd/pmc: Check for intermediate wakeup in function
Applying: platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops
Applying: platform/x86/amd/pmc: Add delay_suspend module parameter
Applying: platform/x86/amd/pmc: Don't log during intermediate wakeups
error: patch failed: drivers/platform/x86/amd/pmc/pmc.c:660
error: drivers/platform/x86/amd/pmc/pmc.c: patch does not apply
error: patch failed: drivers/platform/x86/amd/pmc/pmc.h:114
error: drivers/platform/x86/amd/pmc/pmc.h: patch does not apply
Patch failed at 0004 platform/x86/amd/pmc: Don't log during intermediate wakeups


-- 
 i.


  reply	other threads:[~2026-06-11 14:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 10:57 [PATCH v5 0/4] amd_pmc: Delay s2idle suspend for some devices Daniel Gibson
2026-06-09 10:57 ` [PATCH v5 1/4] platform/x86/amd/pmc: Check for intermediate wakeup in function Daniel Gibson
2026-06-09 10:57 ` [PATCH v5 2/4] platform/x86/amd/pmc: Delay suspend for some Lenovo Laptops Daniel Gibson
2026-06-09 11:46   ` Ilpo Järvinen
2026-06-09 12:07     ` Daniel Gibson
2026-06-09 14:40       ` Mario Limonciello
2026-06-09 15:06         ` Daniel Gibson
2026-06-09 15:36           ` Daniel Gibson
2026-06-09 15:47             ` Mario Limonciello
2026-06-10  8:21               ` Ilpo Järvinen
2026-06-10 14:51                 ` Mario Limonciello
2026-06-09 10:57 ` [PATCH v5 3/4] platform/x86/amd/pmc: Add delay_suspend module parameter Daniel Gibson
2026-06-09 10:57 ` [PATCH v5 4/4] platform/x86/amd/pmc: Don't log during intermediate wakeups Daniel Gibson
2026-06-11 14:02   ` Ilpo Järvinen [this message]
2026-06-11 14:20     ` Daniel Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4bc20ca2-8544-e36e-70af-a19364e59eba@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=daniel@gibson.sh \
    --cc=hansg@kernel.org \
    --cc=johannes.goede@oss.qualcomm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=superm1@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox