| Message ID | 20241204144842.164178-4-Dhananjay.Ugwekar@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | cpufreq/amd-pstate: Reuse and refactor code | expand |
Hi Dhananjay,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.13-rc1 next-20241204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dhananjay-Ugwekar/cpufreq-amd-pstate-Convert-the-amd_pstate_get-set_epp-to-static-calls/20241204-225537
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20241204144842.164178-4-Dhananjay.Ugwekar%40amd.com
patch subject: [PATCH 3/5] cpufreq/amd-pstate: Refactor amd_pstate_epp_reenable() and amd_pstate_epp_offline()
config: x86_64-buildonly-randconfig-006-20241205 (https://download.01.org/0day-ci/archive/20241205/202412050615.ObPzrf34-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241205/202412050615.ObPzrf34-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412050615.ObPzrf34-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/cpufreq/amd-pstate.c: In function 'amd_pstate_epp_offline':
>> drivers/cpufreq/amd-pstate.c:1664:13: warning: variable 'value' set but not used [-Wunused-but-set-variable]
1664 | u64 value;
| ^~~~~
vim +/value +1664 drivers/cpufreq/amd-pstate.c
d4da12f8033a123 Perry Yuan 2023-01-31 1659
d4da12f8033a123 Perry Yuan 2023-01-31 1660 static void amd_pstate_epp_offline(struct cpufreq_policy *policy)
d4da12f8033a123 Perry Yuan 2023-01-31 1661 {
d4da12f8033a123 Perry Yuan 2023-01-31 1662 struct amd_cpudata *cpudata = policy->driver_data;
d4da12f8033a123 Perry Yuan 2023-01-31 1663 int min_perf;
d4da12f8033a123 Perry Yuan 2023-01-31 @1664 u64 value;
d4da12f8033a123 Perry Yuan 2023-01-31 1665
d4da12f8033a123 Perry Yuan 2023-01-31 1666 min_perf = READ_ONCE(cpudata->lowest_perf);
d4da12f8033a123 Perry Yuan 2023-01-31 1667 value = READ_ONCE(cpudata->cppc_req_cached);
d4da12f8033a123 Perry Yuan 2023-01-31 1668
d4da12f8033a123 Perry Yuan 2023-01-31 1669 mutex_lock(&amd_pstate_limits_lock);
d4da12f8033a123 Perry Yuan 2023-01-31 1670
33cc0b550fa3510 Dhananjay Ugwekar 2024-12-04 1671 amd_pstate_update_perf(cpudata, min_perf, 0, min_perf, false);
33cc0b550fa3510 Dhananjay Ugwekar 2024-12-04 1672 amd_pstate_set_epp(cpudata, AMD_CPPC_EPP_BALANCE_POWERSAVE);
33cc0b550fa3510 Dhananjay Ugwekar 2024-12-04 1673
d4da12f8033a123 Perry Yuan 2023-01-31 1674 mutex_unlock(&amd_pstate_limits_lock);
d4da12f8033a123 Perry Yuan 2023-01-31 1675 }
d4da12f8033a123 Perry Yuan 2023-01-31 1676
On Wed, Dec 04, 2024 at 02:48:40PM +0000, Dhananjay Ugwekar wrote: > Replace similar code chunks with amd_pstate_update_perf() and > amd_pstate_set_epp() function calls. > > Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 36 +++++++----------------------------- > 1 file changed, 7 insertions(+), 29 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index a1b2393cef22..a38be7727c9d 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -1630,25 +1630,17 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy) > > static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata) > { > - struct cppc_perf_ctrls perf_ctrls; > - u64 value, max_perf; > + u64 max_perf; > int ret; > > ret = amd_pstate_cppc_enable(true); > if (ret) > pr_err("failed to enable amd pstate during resume, return %d\n", ret); > > - value = READ_ONCE(cpudata->cppc_req_cached); > max_perf = READ_ONCE(cpudata->highest_perf); > > - if (cpu_feature_enabled(X86_FEATURE_CPPC)) { > - wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); > - } else { > - perf_ctrls.max_perf = max_perf; > - cppc_set_perf(cpudata->cpu, &perf_ctrls); > - perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(cpudata->epp_cached); > - cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); > - } > + amd_pstate_update_perf(cpudata, 0, 0, max_perf, false); > + amd_pstate_set_epp(cpudata, cpudata->epp_cached); This will cause two MSR writes on MSR based systems. But then, we don't expect the amd_pstate_epp_reenable() and amd_pstate_epp_offline() to be called often. Hence it should be ok. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > } > > static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy) > @@ -1668,7 +1660,6 @@ static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy) > static void amd_pstate_epp_offline(struct cpufreq_policy *policy) > { > struct amd_cpudata *cpudata = policy->driver_data; > - struct cppc_perf_ctrls perf_ctrls; > int min_perf; > u64 value; > > @@ -1676,23 +1667,10 @@ static void amd_pstate_epp_offline(struct cpufreq_policy *policy) > value = READ_ONCE(cpudata->cppc_req_cached); > > mutex_lock(&amd_pstate_limits_lock); > - if (cpu_feature_enabled(X86_FEATURE_CPPC)) { > - cpudata->epp_policy = CPUFREQ_POLICY_UNKNOWN; > - > - /* Set max perf same as min perf */ > - value &= ~AMD_CPPC_MAX_PERF(~0L); > - value |= AMD_CPPC_MAX_PERF(min_perf); > - value &= ~AMD_CPPC_MIN_PERF(~0L); > - value |= AMD_CPPC_MIN_PERF(min_perf); > - wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); > - } else { > - perf_ctrls.desired_perf = 0; > - perf_ctrls.min_perf = min_perf; > - perf_ctrls.max_perf = min_perf; > - cppc_set_perf(cpudata->cpu, &perf_ctrls); > - perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(HWP_EPP_BALANCE_POWERSAVE); > - cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); > - } > + > + amd_pstate_update_perf(cpudata, min_perf, 0, min_perf, false); > + amd_pstate_set_epp(cpudata, AMD_CPPC_EPP_BALANCE_POWERSAVE); > + > mutex_unlock(&amd_pstate_limits_lock); > } > > -- > 2.34.1 >
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index a1b2393cef22..a38be7727c9d 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -1630,25 +1630,17 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy) static void amd_pstate_epp_reenable(struct amd_cpudata *cpudata) { - struct cppc_perf_ctrls perf_ctrls; - u64 value, max_perf; + u64 max_perf; int ret; ret = amd_pstate_cppc_enable(true); if (ret) pr_err("failed to enable amd pstate during resume, return %d\n", ret); - value = READ_ONCE(cpudata->cppc_req_cached); max_perf = READ_ONCE(cpudata->highest_perf); - if (cpu_feature_enabled(X86_FEATURE_CPPC)) { - wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); - } else { - perf_ctrls.max_perf = max_perf; - cppc_set_perf(cpudata->cpu, &perf_ctrls); - perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(cpudata->epp_cached); - cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); - } + amd_pstate_update_perf(cpudata, 0, 0, max_perf, false); + amd_pstate_set_epp(cpudata, cpudata->epp_cached); } static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy) @@ -1668,7 +1660,6 @@ static int amd_pstate_epp_cpu_online(struct cpufreq_policy *policy) static void amd_pstate_epp_offline(struct cpufreq_policy *policy) { struct amd_cpudata *cpudata = policy->driver_data; - struct cppc_perf_ctrls perf_ctrls; int min_perf; u64 value; @@ -1676,23 +1667,10 @@ static void amd_pstate_epp_offline(struct cpufreq_policy *policy) value = READ_ONCE(cpudata->cppc_req_cached); mutex_lock(&amd_pstate_limits_lock); - if (cpu_feature_enabled(X86_FEATURE_CPPC)) { - cpudata->epp_policy = CPUFREQ_POLICY_UNKNOWN; - - /* Set max perf same as min perf */ - value &= ~AMD_CPPC_MAX_PERF(~0L); - value |= AMD_CPPC_MAX_PERF(min_perf); - value &= ~AMD_CPPC_MIN_PERF(~0L); - value |= AMD_CPPC_MIN_PERF(min_perf); - wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); - } else { - perf_ctrls.desired_perf = 0; - perf_ctrls.min_perf = min_perf; - perf_ctrls.max_perf = min_perf; - cppc_set_perf(cpudata->cpu, &perf_ctrls); - perf_ctrls.energy_perf = AMD_CPPC_ENERGY_PERF_PREF(HWP_EPP_BALANCE_POWERSAVE); - cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); - } + + amd_pstate_update_perf(cpudata, min_perf, 0, min_perf, false); + amd_pstate_set_epp(cpudata, AMD_CPPC_EPP_BALANCE_POWERSAVE); + mutex_unlock(&amd_pstate_limits_lock); }
Replace similar code chunks with amd_pstate_update_perf() and amd_pstate_set_epp() function calls. Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> --- drivers/cpufreq/amd-pstate.c | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-)