add suspend and resume support for the AMD processors by amd_pstate_epp
driver instance.
When the CPPC is suspended, EPP driver will set EPP profile to 'power'
profile and set max/min perf to lowest perf value.
When resume happens, it will restore the MSR registers with
previous cached value.
Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 6ce9fca0a128..841b1e2383b8 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1282,6 +1282,44 @@ static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
return amd_pstate_cpu_offline(policy);
}
+static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+ int ret;
+
+ /* avoid suspending when EPP is not enabled */
+ if (!cppc_active)
+ return 0;
+
+ /* set this flag to avoid setting core offline*/
+ cpudata->suspended = true;
+
+ /* disable CPPC in lowlevel firmware */
+ ret = amd_pstate_enable(false);
+ if (ret)
+ pr_err("failed to suspend, return %d\n", ret);
+
+ return 0;
+}
+
+static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
+{
+ struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
+
+ if (cpudata->suspended) {
+ mutex_lock(&amd_pstate_limits_lock);
+
+ /* enable amd pstate from suspend state*/
+ amd_pstate_epp_reenable(cpudata);
+
+ mutex_unlock(&amd_pstate_limits_lock);
+
+ cpudata->suspended = false;
+ }
+
+ return 0;
+}
+
static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
struct cpufreq_policy_data *policy)
{
@@ -1318,6 +1356,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
.update_limits = amd_pstate_epp_update_limits,
.offline = amd_pstate_epp_cpu_offline,
.online = amd_pstate_epp_cpu_online,
+ .suspend = amd_pstate_epp_suspend,
+ .resume = amd_pstate_epp_resume,
.name = "amd_pstate_epp",
.attr = amd_pstate_epp_attr,
};
--
2.34.1
On 11/7/2022 11:57, Perry Yuan wrote:
> add suspend and resume support for the AMD processors by amd_pstate_epp
> driver instance.
>
> When the CPPC is suspended, EPP driver will set EPP profile to 'power'
> profile and set max/min perf to lowest perf value.
> When resume happens, it will restore the MSR registers with
> previous cached value.
>
> Signed-off-by: Perry Yuan <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
> ---
> drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 6ce9fca0a128..841b1e2383b8 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1282,6 +1282,44 @@ static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
> return amd_pstate_cpu_offline(policy);
> }
>
> +static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> + int ret;
> +
> + /* avoid suspending when EPP is not enabled */
> + if (!cppc_active)
> + return 0;
> +
> + /* set this flag to avoid setting core offline*/
> + cpudata->suspended = true;
> +
> + /* disable CPPC in lowlevel firmware */
> + ret = amd_pstate_enable(false);
> + if (ret)
> + pr_err("failed to suspend, return %d\n", ret);
> +
> + return 0;
> +}
> +
> +static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> +
> + if (cpudata->suspended) {
> + mutex_lock(&amd_pstate_limits_lock);
> +
> + /* enable amd pstate from suspend state*/
> + amd_pstate_epp_reenable(cpudata);
> +
> + mutex_unlock(&amd_pstate_limits_lock);
> +
> + cpudata->suspended = false;
> + }
> +
> + return 0;
> +}
> +
> static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
> struct cpufreq_policy_data *policy)
> {
> @@ -1318,6 +1356,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .update_limits = amd_pstate_epp_update_limits,
> .offline = amd_pstate_epp_cpu_offline,
> .online = amd_pstate_epp_cpu_online,
> + .suspend = amd_pstate_epp_suspend,
> + .resume = amd_pstate_epp_resume,
> .name = "amd_pstate_epp",
> .attr = amd_pstate_epp_attr,
> };
On 11/7/22 11:57, Perry Yuan wrote:
> add suspend and resume support for the AMD processors by amd_pstate_epp
> driver instance.
>
> When the CPPC is suspended, EPP driver will set EPP profile to 'power'
> profile and set max/min perf to lowest perf value.
> When resume happens, it will restore the MSR registers with
> previous cached value.
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> drivers/cpufreq/amd-pstate.c | 40 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 6ce9fca0a128..841b1e2383b8 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1282,6 +1282,44 @@ static int amd_pstate_epp_cpu_offline(struct cpufreq_policy *policy)
> return amd_pstate_cpu_offline(policy);
> }
>
> +static int amd_pstate_epp_suspend(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> + int ret;
> +
> + /* avoid suspending when EPP is not enabled */
> + if (!cppc_active)
> + return 0;
> +
> + /* set this flag to avoid setting core offline*/
> + cpudata->suspended = true;
Shouldn't we set this only after the amd_pstate_enable(false) call succeeds?
> +
> + /* disable CPPC in lowlevel firmware */
> + ret = amd_pstate_enable(false);
> + if (ret)
> + pr_err("failed to suspend, return %d\n", ret);
> +
> + return 0;
You could drop the checking of ret here and just return ret. The caller
cpufreq.c:cpufreq_suspend() already does a check of the return code and
prints an error message.
> +}
> +
> +static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
> +{
> + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> +
> + if (cpudata->suspended) {
> + mutex_lock(&amd_pstate_limits_lock);
> +
> + /* enable amd pstate from suspend state*/
> + amd_pstate_epp_reenable(cpudata);
This call can fail, we should check the return code.
-Nathan
> +
> + mutex_unlock(&amd_pstate_limits_lock);
> +
> + cpudata->suspended = false;
> + }
> +
> + return 0;
> +}
> +
> static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
> struct cpufreq_policy_data *policy)
> {
> @@ -1318,6 +1356,8 @@ static struct cpufreq_driver amd_pstate_epp_driver = {
> .update_limits = amd_pstate_epp_update_limits,
> .offline = amd_pstate_epp_cpu_offline,
> .online = amd_pstate_epp_cpu_online,
> + .suspend = amd_pstate_epp_suspend,
> + .resume = amd_pstate_epp_resume,
> .name = "amd_pstate_epp",
> .attr = amd_pstate_epp_attr,
> };
[AMD Official Use Only - General]
Hi Mario.
> -----Original Message-----
> From: Limonciello, Mario <[email protected]>
> Sent: Tuesday, November 8, 2022 2:18 AM
> To: Yuan, Perry <[email protected]>; [email protected]; Huang,
> Ray <[email protected]>; [email protected]
> Cc: Sharma, Deepak <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Huang, Shimmer
> <[email protected]>; Du, Xiaojian <[email protected]>; Meng,
> Li (Jassmine) <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v3 6/8] cpufreq: amd-pstate: implement suspend and
> resume callbacks
>
> On 11/7/2022 11:57, Perry Yuan wrote:
> > add suspend and resume support for the AMD processors by
> > amd_pstate_epp driver instance.
> >
> > When the CPPC is suspended, EPP driver will set EPP profile to 'power'
> > profile and set max/min perf to lowest perf value.
> > When resume happens, it will restore the MSR registers with previous
> > cached value.
> >
> > Signed-off-by: Perry Yuan <[email protected]>
>
> Reviewed-by: Mario Limonciello <[email protected]>
Thank you.
I pick up the RB tag in the V4.
Perry.
>
> > ---
> > drivers/cpufreq/amd-pstate.c | 40
> ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 6ce9fca0a128..841b1e2383b8
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -1282,6 +1282,44 @@ static int amd_pstate_epp_cpu_offline(struct
> cpufreq_policy *policy)
> > return amd_pstate_cpu_offline(policy);
> > }
> >
> > +static int amd_pstate_epp_suspend(struct cpufreq_policy *policy) {
> > + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> > + int ret;
> > +
> > + /* avoid suspending when EPP is not enabled */
> > + if (!cppc_active)
> > + return 0;
> > +
> > + /* set this flag to avoid setting core offline*/
> > + cpudata->suspended = true;
> > +
> > + /* disable CPPC in lowlevel firmware */
> > + ret = amd_pstate_enable(false);
> > + if (ret)
> > + pr_err("failed to suspend, return %d\n", ret);
> > +
> > + return 0;
> > +}
> > +
> > +static int amd_pstate_epp_resume(struct cpufreq_policy *policy) {
> > + struct amd_cpudata *cpudata = all_cpu_data[policy->cpu];
> > +
> > + if (cpudata->suspended) {
> > + mutex_lock(&amd_pstate_limits_lock);
> > +
> > + /* enable amd pstate from suspend state*/
> > + amd_pstate_epp_reenable(cpudata);
> > +
> > + mutex_unlock(&amd_pstate_limits_lock);
> > +
> > + cpudata->suspended = false;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static void amd_pstate_verify_cpu_policy(struct amd_cpudata *cpudata,
> > struct cpufreq_policy_data *policy)
> > {
> > @@ -1318,6 +1356,8 @@ static struct cpufreq_driver
> amd_pstate_epp_driver = {
> > .update_limits = amd_pstate_epp_update_limits,
> > .offline = amd_pstate_epp_cpu_offline,
> > .online = amd_pstate_epp_cpu_online,
> > + .suspend = amd_pstate_epp_suspend,
> > + .resume = amd_pstate_epp_resume,
> > .name = "amd_pstate_epp",
> > .attr = amd_pstate_epp_attr,
> > };