From: Hans de Goede <hdegoede@redhat.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Cc: kai.heng.feng@canonical.com, xaver.hugl@gmail.com,
Mark Gross <markgross@kernel.org>,
platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] platform/x86: amd: pmc: Disable IRQ1 wakeup for RN/CZN
Date: Mon, 23 Jan 2023 14:55:36 +0100 [thread overview]
Message-ID: <74a63457-52fd-2f16-1b76-d3b3497b5a5d@redhat.com> (raw)
In-Reply-To: <20230120191519.15926-1-mario.limonciello@amd.com>
Hi,
On 1/20/23 20:15, Mario Limonciello wrote:
> By default when the system is configured for low power idle in the FADT
> the keyboard is set up as a wake source. This matches the behavior that
> Windows uses for Modern Standby as well.
>
> It has been reported that a variety of AMD based designs there are
> spurious wakeups are happening where two IRQ sources are active.
>
> For example:
> ```
> PM: Triggering wakeup from IRQ 9
> PM: Triggering wakeup from IRQ 1
> ```
>
> In these designs IRQ 9 is the ACPI SCI and IRQ 1 is the keyboard.
> One way to trigger this problem is to suspend the laptop and then unplug
> the AC adapter. The SOC will be in a hardware sleep state and plugging
> in the AC adapter returns control to the kernel's s2idle loop.
>
> Normally if just IRQ 9 was active the s2idle loop would advance any EC
> transactions and no other IRQ being active would cause the s2idle loop
> to put the SOC back into hardware sleep state.
>
> When this bug occurred IRQ 1 is also active even if no keyboard activity
> occurred. This causes the s2idle loop to break and the system to wake.
>
> This is a platform firmware bug triggering IRQ1 without keyboard activity.
> This occurs in Windows as well, but Windows will enter "SW DRIPS" and
> then with no activity enters back into "HW DRIPS" (hardware sleep state).
>
> This issue affects Renoir, Lucienne, Cezanne, and Barcelo platforms. It
> does not happen on newer systems such as Mendocino or Rembrandt.
>
> It's been fixed in newer platform firmware. To avoid triggering the bug
> on older systems check the SMU F/W version and adjust the policy at suspend
> time for s2idle wakeup from keyboard on these systems. A lot of thought
> and experimentation has been given around the timing of disabling IRQ1,
> and to make it work the "suspend" PM callback is restored.
>
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Reported-by: Xaver Hugl <xaver.hugl@gmail.com>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2115
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1951
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> drivers/platform/x86/amd/pmc.c | 50 ++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index 8d924986381be..be1b49824edbd 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -22,6 +22,7 @@
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/rtc.h>
> +#include <linux/serio.h>
> #include <linux/suspend.h>
> #include <linux/seq_file.h>
> #include <linux/uaccess.h>
> @@ -653,6 +654,33 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev *dev)
> return -EINVAL;
> }
>
> +static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev)
> +{
> + struct device *d;
> + int rc;
> +
> + if (!pdev->major) {
> + rc = amd_pmc_get_smu_version(pdev);
> + if (rc)
> + return rc;
> + }
> +
> + if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65))
> + return 0;
> +
> + d = bus_find_device_by_name(&serio_bus, NULL, "serio0");
> + if (!d)
> + return 0;
> + if (device_may_wakeup(d)) {
> + dev_info_once(d, "Disabling IRQ1 wakeup source to avoid platform firmware bug\n");
> + disable_irq_wake(1);
> + device_set_wakeup_enable(d, false);
> + }
> + put_device(d);
> +
> + return 0;
> +}
> +
> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
> {
> struct rtc_device *rtc_device;
> @@ -782,6 +810,25 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = {
> .check = amd_pmc_s2idle_check,
> .restore = amd_pmc_s2idle_restore,
> };
> +
> +static int __maybe_unused amd_pmc_suspend_handler(struct device *dev)
> +{
> + struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
> +
> + if (pdev->cpu_id == AMD_CPU_ID_CZN) {
> + int rc = amd_pmc_czn_wa_irq1(pdev);
> +
> + if (rc) {
> + dev_err(pdev->dev, "failed to adjust keyboard wakeup: %d\n", rc);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(amd_pmc_pm, amd_pmc_suspend_handler, NULL);
> +
> #endif
>
> static const struct pci_device_id pmc_pci_ids[] = {
> @@ -980,6 +1027,9 @@ static struct platform_driver amd_pmc_driver = {
> .name = "amd_pmc",
> .acpi_match_table = amd_pmc_acpi_ids,
> .dev_groups = pmc_groups,
> +#ifdef CONFIG_SUSPEND
> + .pm = &amd_pmc_pm,
> +#endif
> },
> .probe = amd_pmc_probe,
> .remove = amd_pmc_remove,
prev parent reply other threads:[~2023-01-23 13:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-20 19:15 [PATCH 1/2] platform/x86: amd: pmc: Disable IRQ1 wakeup for RN/CZN Mario Limonciello
2023-01-20 19:15 ` [PATCH 2/2] platform/x86/amd: pmc: Add a module parameter to disable workarounds Mario Limonciello
2023-01-23 13:55 ` Hans de Goede [this message]
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=74a63457-52fd-2f16-1b76-d3b3497b5a5d@redhat.com \
--to=hdegoede@redhat.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=kai.heng.feng@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=xaver.hugl@gmail.com \
/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;
as well as URLs for NNTP newsgroup(s).