[Public]
> > Changes from v1->v2:
> > * Update for Rafael's suggested changes:
> > - fix resume comment
> > - undo QoS at end of resume instead
> > - Use smaller number for QoS to allow C1 entry
> >
> > drivers/platform/x86/amd-pmc.c | 39 ++++++++++++++++++++++++++--
> ------
> > 1 file changed, 30 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-
> pmc.c
> > index 4c72ba68b315..248dc6735f5a 100644
> > --- a/drivers/platform/x86/amd-pmc.c
> > +++ b/drivers/platform/x86/amd-pmc.c
> > @@ -21,6 +21,7 @@
> > #include <linux/module.h>
> > #include <linux/pci.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_qos.h>
> > #include <linux/rtc.h>
> > #include <linux/suspend.h>
> > #include <linux/seq_file.h>
> > @@ -131,6 +132,7 @@ struct amd_pmc_dev {
> > struct device *dev;
> > struct pci_dev *rdev;
> > struct mutex lock; /* generic mutex lock */
> > + struct pm_qos_request amd_pmc_pm_qos_req;
> > #if IS_ENABLED(CONFIG_DEBUG_FS)
> > struct dentry *dbgfs_dir;
> > #endif /* CONFIG_DEBUG_FS */
> > @@ -521,6 +523,14 @@ static int amd_pmc_verify_czn_rtc(struct
> amd_pmc_dev *pdev, u32 *arg)
> > rc = rtc_alarm_irq_enable(rtc_device, 0);
> > dev_dbg(pdev->dev, "wakeup timer programmed for %lld
> seconds\n", duration);
> >
> > + /*
> > + * Prevent CPUs from getting into idle states while running the code
>
> This should be "into deep idle states" now.
>
>
> > + * below which is generally safe to run when none of the CPUs are in
> > + * idle states.
> > + */
> > + cpu_latency_qos_update_request(&pdev-
> >amd_pmc_pm_qos_req, 3);
>
> And I would define a symbol for the 3, possibly with a comment
> documenting it. Something like
> AMD_PMC_SUSPEND_MAX_IDLE_STATE_LATENCY
> or similar.
>
> With the above addressed, please feel free to add
>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> to this patch.
>
> And thanks for taking care of this!
Sure thing, will make those minor modifications and send up a v3 with your tag.
Really appreciate your help and suggestions here too.
>
>
> > + wake_up_all_idle_cpus();
> > +
> > return rc;
> > }
> >
> > @@ -538,24 +548,31 @@ static int __maybe_unused
> amd_pmc_suspend(struct device *dev)
> > /* Activate CZN specific RTC functionality */
> > if (pdev->cpu_id == AMD_CPU_ID_CZN) {
> > rc = amd_pmc_verify_czn_rtc(pdev, &arg);
> > - if (rc < 0)
> > - return rc;
> > + if (rc)
> > + goto fail;
> > }
> >
> > /* Dump the IdleMask before we send hint to SMU */
> > amd_pmc_idlemask_read(pdev, dev, NULL);
> > msg = amd_pmc_get_os_hint(pdev);
> > rc = amd_pmc_send_cmd(pdev, arg, NULL, msg, 0);
> > - if (rc)
> > + if (rc) {
> > dev_err(pdev->dev, "suspend failed\n");
> > + goto fail;
> > + }
> >
> > if (enable_stb)
> > rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF);
> > - if (rc) {
> > + if (rc) {
> > dev_err(pdev->dev, "error writing to STB\n");
> > - return rc;
> > + goto fail;
> > }
> >
> > + return 0;
> > +fail:
> > + if (pdev->cpu_id == AMD_CPU_ID_CZN)
> > + cpu_latency_qos_update_request(&pdev-
> >amd_pmc_pm_qos_req,
> > + PM_QOS_DEFAULT_VALUE);
> > return rc;
> > }
> >
> > @@ -579,12 +596,15 @@ static int __maybe_unused
> amd_pmc_resume(struct device *dev)
> > /* Write data incremented by 1 to distinguish in stb_read */
> > if (enable_stb)
> > rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF +
> 1);
> > - if (rc) {
> > + if (rc)
> > dev_err(pdev->dev, "error writing to STB\n");
> > - return rc;
> > - }
> >
> > - return 0;
> > + /* Restore the QoS request back to defaults if it was set */
> > + if (pdev->cpu_id == AMD_CPU_ID_CZN)
> > + cpu_latency_qos_update_request(&pdev-
> >amd_pmc_pm_qos_req,
> > + PM_QOS_DEFAULT_VALUE);
> > +
> > + return rc;
> > }
> >
> > static const struct dev_pm_ops amd_pmc_pm_ops = {
> > @@ -722,6 +742,7 @@ static int amd_pmc_probe(struct platform_device
> *pdev)
> > amd_pmc_get_smu_version(dev);
> > platform_set_drvdata(pdev, dev);
> > amd_pmc_dbgfs_register(dev);
> > + cpu_latency_qos_add_request(&dev->amd_pmc_pm_qos_req,
> PM_QOS_DEFAULT_VALUE);
> > return 0;
> >
> > err_pci_dev_put:
>
