| Message ID | 20241205222847.7889-11-mario.limonciello@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | amd-pstate 6.14 cleanups and improvements | expand |
Hi Mario,
kernel test robot noticed the following build warnings:
[auto build test WARNING on ab9e5b2eb56412cb8c63b46b935878d29205418e]
url: https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/cpufreq-amd-pstate-Add-trace-event-for-EPP-perf-updates/20241206-063920
base: ab9e5b2eb56412cb8c63b46b935878d29205418e
patch link: https://lore.kernel.org/r/20241205222847.7889-11-mario.limonciello%40amd.com
patch subject: [PATCH 10/15] cpufreq/amd-pstate: Move limit updating code
config: x86_64-buildonly-randconfig-001-20241206 (https://download.01.org/0day-ci/archive/20241206/202412061054.cJTnhP0n-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/20241206/202412061054.cJTnhP0n-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/202412061054.cJTnhP0n-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/cpufreq/amd-pstate.c: In function 'amd_pstate_update_min_max_limit':
>> drivers/cpufreq/amd-pstate.c:606:45: warning: variable 'lowest_perf' set but not used [-Wunused-but-set-variable]
606 | u32 max_limit_perf, min_limit_perf, lowest_perf, max_perf, max_freq;
| ^~~~~~~~~~~
vim +/lowest_perf +606 drivers/cpufreq/amd-pstate.c
ec437d71db77a18 Huang Rui 2021-12-24 603
febab20caebac95 Wyes Karny 2023-11-17 604 static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy)
febab20caebac95 Wyes Karny 2023-11-17 605 {
b623ceabb704d2d Mario Limonciello 2024-12-05 @606 u32 max_limit_perf, min_limit_perf, lowest_perf, max_perf, max_freq;
febab20caebac95 Wyes Karny 2023-11-17 607 struct amd_cpudata *cpudata = policy->driver_data;
febab20caebac95 Wyes Karny 2023-11-17 608
18d9b5227121389 Mario Limonciello 2024-10-12 609 max_perf = READ_ONCE(cpudata->highest_perf);
b623ceabb704d2d Mario Limonciello 2024-12-05 610 max_freq = READ_ONCE(cpudata->max_freq);
b623ceabb704d2d Mario Limonciello 2024-12-05 611 max_limit_perf = div_u64(policy->max * max_perf, max_freq);
b623ceabb704d2d Mario Limonciello 2024-12-05 612 min_limit_perf = div_u64(policy->min * max_perf, max_freq);
febab20caebac95 Wyes Karny 2023-11-17 613
8164f743326404f Meng Li 2024-02-27 614 lowest_perf = READ_ONCE(cpudata->lowest_perf);
8164f743326404f Meng Li 2024-02-27 615
6fbd2a2609d0033 Mario Limonciello 2024-12-05 616 if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
6fbd2a2609d0033 Mario Limonciello 2024-12-05 617 min_limit_perf = min(cpudata->nominal_perf, max_limit_perf);
8164f743326404f Meng Li 2024-02-27 618
febab20caebac95 Wyes Karny 2023-11-17 619 WRITE_ONCE(cpudata->max_limit_perf, max_limit_perf);
febab20caebac95 Wyes Karny 2023-11-17 620 WRITE_ONCE(cpudata->min_limit_perf, min_limit_perf);
febab20caebac95 Wyes Karny 2023-11-17 621 WRITE_ONCE(cpudata->max_limit_freq, policy->max);
febab20caebac95 Wyes Karny 2023-11-17 622 WRITE_ONCE(cpudata->min_limit_freq, policy->min);
febab20caebac95 Wyes Karny 2023-11-17 623
febab20caebac95 Wyes Karny 2023-11-17 624 return 0;
febab20caebac95 Wyes Karny 2023-11-17 625 }
febab20caebac95 Wyes Karny 2023-11-17 626
On Thu, Dec 05, 2024 at 04:28:42PM -0600, Mario Limonciello wrote: > The limit updating code in amd_pstate_epp_update_limit() should not > only apply to EPP updates. Move it to amd_pstate_update_min_max_limit() > so other callers can benefit as well. > > With this move it's not necessary to have clamp_t calls anymore because > the verify callback is called when setting limits. Nice catch!! The patch looks good to me. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 24 ++++-------------------- > 1 file changed, 4 insertions(+), 20 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 842e7e3c5eaf2..5ee53b45c1ca1 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -537,10 +537,6 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf, > u32 nominal_perf = READ_ONCE(cpudata->nominal_perf); > u64 value = prev; > > - min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf, > - cpudata->max_limit_perf); > - max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf, > - cpudata->max_limit_perf); > des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf); > > max_freq = READ_ONCE(cpudata->max_limit_freq); > @@ -616,11 +612,9 @@ static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy) > min_limit_perf = div_u64(policy->min * max_perf, max_freq); > > lowest_perf = READ_ONCE(cpudata->lowest_perf); > - if (min_limit_perf < lowest_perf) > - min_limit_perf = lowest_perf; > > - if (max_limit_perf < min_limit_perf) > - max_limit_perf = min_limit_perf; > + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) > + min_limit_perf = min(cpudata->nominal_perf, max_limit_perf); > > WRITE_ONCE(cpudata->max_limit_perf, max_limit_perf); > WRITE_ONCE(cpudata->min_limit_perf, min_limit_perf); > @@ -1562,28 +1556,18 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy) > static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy) > { > struct amd_cpudata *cpudata = policy->driver_data; > - u32 max_perf, min_perf; > u64 value; > s16 epp; > > - max_perf = READ_ONCE(cpudata->highest_perf); > - min_perf = READ_ONCE(cpudata->lowest_perf); > amd_pstate_update_min_max_limit(policy); > > - max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf, > - cpudata->max_limit_perf); > - min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf, > - cpudata->max_limit_perf); > value = READ_ONCE(cpudata->cppc_req_cached); > > - if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) > - min_perf = min(cpudata->nominal_perf, max_perf); > - > value &= ~(AMD_PSTATE_MAX_PERF_MASK | AMD_PSTATE_MIN_PERF_MASK | > AMD_PSTATE_DES_PERF_MASK); > - value |= FIELD_PREP(AMD_PSTATE_MAX_PERF_MASK, max_perf); > + value |= FIELD_PREP(AMD_PSTATE_MAX_PERF_MASK, cpudata->max_limit_perf); > value |= FIELD_PREP(AMD_PSTATE_DES_PERF_MASK, 0); > - value |= FIELD_PREP(AMD_PSTATE_MIN_PERF_MASK, min_perf); > + value |= FIELD_PREP(AMD_PSTATE_MIN_PERF_MASK, cpudata->min_limit_perf); > > /* Get BIOS pre-defined epp value */ > epp = amd_pstate_get_epp(cpudata, value); > -- > 2.43.0 >
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 842e7e3c5eaf2..5ee53b45c1ca1 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -537,10 +537,6 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf, u32 nominal_perf = READ_ONCE(cpudata->nominal_perf); u64 value = prev; - min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf, - cpudata->max_limit_perf); - max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf, - cpudata->max_limit_perf); des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf); max_freq = READ_ONCE(cpudata->max_limit_freq); @@ -616,11 +612,9 @@ static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy) min_limit_perf = div_u64(policy->min * max_perf, max_freq); lowest_perf = READ_ONCE(cpudata->lowest_perf); - if (min_limit_perf < lowest_perf) - min_limit_perf = lowest_perf; - if (max_limit_perf < min_limit_perf) - max_limit_perf = min_limit_perf; + if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) + min_limit_perf = min(cpudata->nominal_perf, max_limit_perf); WRITE_ONCE(cpudata->max_limit_perf, max_limit_perf); WRITE_ONCE(cpudata->min_limit_perf, min_limit_perf); @@ -1562,28 +1556,18 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy) static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy) { struct amd_cpudata *cpudata = policy->driver_data; - u32 max_perf, min_perf; u64 value; s16 epp; - max_perf = READ_ONCE(cpudata->highest_perf); - min_perf = READ_ONCE(cpudata->lowest_perf); amd_pstate_update_min_max_limit(policy); - max_perf = clamp_t(unsigned long, max_perf, cpudata->min_limit_perf, - cpudata->max_limit_perf); - min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf, - cpudata->max_limit_perf); value = READ_ONCE(cpudata->cppc_req_cached); - if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) - min_perf = min(cpudata->nominal_perf, max_perf); - value &= ~(AMD_PSTATE_MAX_PERF_MASK | AMD_PSTATE_MIN_PERF_MASK | AMD_PSTATE_DES_PERF_MASK); - value |= FIELD_PREP(AMD_PSTATE_MAX_PERF_MASK, max_perf); + value |= FIELD_PREP(AMD_PSTATE_MAX_PERF_MASK, cpudata->max_limit_perf); value |= FIELD_PREP(AMD_PSTATE_DES_PERF_MASK, 0); - value |= FIELD_PREP(AMD_PSTATE_MIN_PERF_MASK, min_perf); + value |= FIELD_PREP(AMD_PSTATE_MIN_PERF_MASK, cpudata->min_limit_perf); /* Get BIOS pre-defined epp value */ epp = amd_pstate_get_epp(cpudata, value);
The limit updating code in amd_pstate_epp_update_limit() should not only apply to EPP updates. Move it to amd_pstate_update_min_max_limit() so other callers can benefit as well. With this move it's not necessary to have clamp_t calls anymore because the verify callback is called when setting limits. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/cpufreq/amd-pstate.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)