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@xxxxxxxxxxxxx> > Reported-by: Xaver Hugl <xaver.hugl@xxxxxxxxx> > 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@xxxxxxx> 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,
