platform/x86: amd-pmc: Set QOS during suspend on CZN w/ timer wakeup

commit 59348401ebed ("platform/x86: amd-pmc: Add special handling for
timer based S0i3 wakeup") adds support for using another platform timer
in lieu of the RTC which doesn't work properly on some systems. This path
was validated and worked well before submission. During the 5.16-rc1 merge
window other patches were merged that caused this to stop working properly.

When this feature was used with 5.16-rc1 or later some OEM laptops with the
matching firmware requirements from that commit would shutdown instead of
program a timer based wakeup.

This was bisected to commit 8d89835b0467 ("PM: suspend: Do not pause
cpuidle in the suspend-to-idle path"). This wasn't supposed to cause any
negative impacts and also tested well on both Intel and ARM platforms.
However this changed the semantics of when CPUs are allowed to be in the
deepest state. For the AMD systems in question it appears this causes a
firmware crash for timer based wakeup.

It's hypothesized to be caused by the `amd-pmc` driver sending `OS_HINT`
and all the CPUs going into a deep state while the timer is still being
programmed. It's likely a firmware bug, but to avoid it don't allow setting
CPUs into the deepest state while using CZN timer wakeup path.

If later it's discovered that this also occurs from "regular" suspends
without a timer as well or on other silicon, this may be later expanded to
run in the suspend path for more scenarios.

Cc: stable@vger.kernel.org # 5.16+
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Link: https://lore.kernel.org/linux-acpi/BL1PR12MB51570F5BD05980A0DCA1F3F4E23A9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#mee35f39c41a04b624700ab2621c795367f19c90e
Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
Fixes: 23f62d7ab25b ("PM: sleep: Pause cpuidle later and resume it earlier during system transitions")
Fixes: 59348401ebed ("platform/x86: amd-pmc: Add special handling for timer based S0i3 wakeup")
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Link: https://lore.kernel.org/r/20220223175237.6209-1-mario.limonciello@amd.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>

authored by

Mario Limonciello and committed by

Hans de Goede 4 years ago 32370191 07f5ed0e

+33 -9

1 changed file

expand all

drivers

platform

x86

amd-pmc.c

+33 -9

drivers/platform/x86/amd-pmc.c

··· 21 21 #include <linux/module.h> 22 22 #include <linux/pci.h> 23 23 #include <linux/platform_device.h> 24 + #include <linux/pm_qos.h> 24 25 #include <linux/rtc.h> 25 26 #include <linux/suspend.h> 26 27 #include <linux/seq_file.h> ··· 96 95 #define PMC_MSG_DELAY_MIN_US 50 97 96 #define RESPONSE_REGISTER_LOOP_MAX 20000 98 97 98 + /* QoS request for letting CPUs in idle states, but not the deepest */ 99 + #define AMD_PMC_MAX_IDLE_STATE_LATENCY 3 100 + 99 101 #define SOC_SUBSYSTEM_IP_MAX 12 100 102 #define DELAY_MIN_US 2000 101 103 #define DELAY_MAX_US 3000 ··· 153 149 struct device *dev; 154 150 struct pci_dev *rdev; 155 151 struct mutex lock; /* generic mutex lock */ 152 + struct pm_qos_request amd_pmc_pm_qos_req; 156 153 #if IS_ENABLED(CONFIG_DEBUG_FS) 157 154 struct dentry *dbgfs_dir; 158 155 #endif /* CONFIG_DEBUG_FS */ ··· 608 603 rc = rtc_alarm_irq_enable(rtc_device, 0); 609 604 dev_dbg(pdev->dev, "wakeup timer programmed for %lld seconds\n", duration); 610 605 606 + /* 607 + * Prevent CPUs from getting into deep idle states while sending OS_HINT 608 + * which is otherwise generally safe to send when at least one of the CPUs 609 + * is not in deep idle states. 610 + */ 611 + cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req, AMD_PMC_MAX_IDLE_STATE_LATENCY); 612 + wake_up_all_idle_cpus(); 613 + 611 614 return rc; 612 615 } 613 616 ··· 633 620 /* Activate CZN specific RTC functionality */ 634 621 if (pdev->cpu_id == AMD_CPU_ID_CZN) { 635 622 rc = amd_pmc_verify_czn_rtc(pdev, &arg); 636 - if (rc < 0) 637 - return rc; 623 + if (rc) 624 + goto fail; 638 625 } 639 626 640 627 /* Dump the IdleMask before we send hint to SMU */ 641 628 amd_pmc_idlemask_read(pdev, dev, NULL); 642 629 msg = amd_pmc_get_os_hint(pdev); 643 630 rc = amd_pmc_send_cmd(pdev, arg, NULL, msg, 0); 644 - if (rc) 631 + if (rc) { 645 632 dev_err(pdev->dev, "suspend failed\n"); 633 + goto fail; 634 + } 646 635 647 636 if (enable_stb) 648 637 rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF); 649 - if (rc) { 638 + if (rc) { 650 639 dev_err(pdev->dev, "error writing to STB\n"); 651 - return rc; 640 + goto fail; 652 641 } 653 642 643 + return 0; 644 + fail: 645 + if (pdev->cpu_id == AMD_CPU_ID_CZN) 646 + cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req, 647 + PM_QOS_DEFAULT_VALUE); 654 648 return rc; 655 649 } 656 650 ··· 681 661 /* Write data incremented by 1 to distinguish in stb_read */ 682 662 if (enable_stb) 683 663 rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1); 684 - if (rc) { 664 + if (rc) 685 665 dev_err(pdev->dev, "error writing to STB\n"); 686 - return rc; 687 - } 688 666 689 - return 0; 667 + /* Restore the QoS request back to defaults if it was set */ 668 + if (pdev->cpu_id == AMD_CPU_ID_CZN) 669 + cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req, 670 + PM_QOS_DEFAULT_VALUE); 671 + 672 + return rc; 690 673 } 691 674 692 675 static const struct dev_pm_ops amd_pmc_pm_ops = { ··· 861 838 amd_pmc_get_smu_version(dev); 862 839 platform_set_drvdata(pdev, dev); 863 840 amd_pmc_dbgfs_register(dev); 841 + cpu_latency_qos_add_request(&dev->amd_pmc_pm_qos_req, PM_QOS_DEFAULT_VALUE); 864 842 return 0; 865 843 866 844 err_pci_dev_put:

Configure Feed

Configure Feed