| Message ID | 20221208111852.386731-8-perry.yuan@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement AMD Pstate EPP Driver | expand |
On Thu, Dec 08, 2022 at 07:18:46PM +0800, Yuan, Perry wrote: > From: Perry Yuan <Perry.Yuan@amd.com> > > 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 <Perry.Yuan@amd.com> > --- > 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 412accab7bda..ea9255bdc9ac 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -1273,6 +1273,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); The same comment, could you please double confirm whether the perfo_ctrls registers will be cleared while we execute a round of S3 suspend/resume? > + > + 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) > { > @@ -1309,6 +1347,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 >
[Public] > -----Original Message----- > From: Huang, Ray <Ray.Huang@amd.com> > Sent: Monday, December 12, 2022 03:05 > To: Yuan, Perry <Perry.Yuan@amd.com> > Cc: rafael.j.wysocki@intel.com; Limonciello, Mario > <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Sharma, Deepak > <Deepak.Sharma@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Huang, Shimmer > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, > Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>; > linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v7 07/13] cpufreq: amd-pstate: implement suspend and > resume callbacks > > On Thu, Dec 08, 2022 at 07:18:46PM +0800, Yuan, Perry wrote: > > From: Perry Yuan <Perry.Yuan@amd.com> > > > > 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 <Perry.Yuan@amd.com> > > --- > > 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 412accab7bda..ea9255bdc9ac 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -1273,6 +1273,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); > > The same comment, could you please double confirm whether the > perfo_ctrls > registers will be cleared while we execute a round of S3 suspend/resume? And if they are; identify what is clearing them. It might not be the same for s0i3 and S3. > > > + > > + 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) > > { > > @@ -1309,6 +1347,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 > >
[Public] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Monday, December 12, 2022 11:15 PM > To: Huang, Ray <Ray.Huang@amd.com>; Yuan, Perry <Perry.Yuan@amd.com> > Cc: rafael.j.wysocki@intel.com; viresh.kumar@linaro.org; Sharma, Deepak > <Deepak.Sharma@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Huang, Shimmer > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, > Li (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>; > linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v7 07/13] cpufreq: amd-pstate: implement suspend and > resume callbacks > > [Public] > > > > > -----Original Message----- > > From: Huang, Ray <Ray.Huang@amd.com> > > Sent: Monday, December 12, 2022 03:05 > > To: Yuan, Perry <Perry.Yuan@amd.com> > > Cc: rafael.j.wysocki@intel.com; Limonciello, Mario > > <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Sharma, > Deepak > > <Deepak.Sharma@amd.com>; Fontenot, Nathan > <Nathan.Fontenot@amd.com>; > > Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer > > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; > Meng, Li > > (Jassmine) <Li.Meng@amd.com>; Karny, Wyes <Wyes.Karny@amd.com>; > > linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v7 07/13] cpufreq: amd-pstate: implement suspend > > and resume callbacks > > > > On Thu, Dec 08, 2022 at 07:18:46PM +0800, Yuan, Perry wrote: > > > From: Perry Yuan <Perry.Yuan@amd.com> > > > > > > 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 <Perry.Yuan@amd.com> > > > --- > > > 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 412accab7bda..ea9255bdc9ac > > > 100644 > > > --- a/drivers/cpufreq/amd-pstate.c > > > +++ b/drivers/cpufreq/amd-pstate.c > > > @@ -1273,6 +1273,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); > > > > The same comment, could you please double confirm whether the > > perfo_ctrls registers will be cleared while we execute a round of S3 > > suspend/resume? PERF_CTL register will be always 0 if we do not load acpi_cpufreq driver after kernel booting. So suspend/resume will not change the PERF_CTL MSR as well. > > And if they are; identify what is clearing them. It might not be the same for > s0i3 and S3. I checked the PERF_CTRL with suspend/resume, offline/online test. The MSRs of all cores are not changed while amd-pstate driver loaded instead of acpi-cpufreq. > > > > > > + > > > + 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) > { @@ -1309,6 +1347,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 > > >
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 412accab7bda..ea9255bdc9ac 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -1273,6 +1273,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) { @@ -1309,6 +1347,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, };