Re: [PATCH v5 04/19] cpufreq/amd-pstate: Move perf values into a union

[Mario Limonciello <superm1@kernel.org>]

On 2/27/2025 06:59, kernel test robot wrote:
> Hi Mario,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on amd-pstate/bleeding-edge]
> [cannot apply to rafael-pm/linux-next rafael-pm/bleeding-edge tip/x86/core amd-pstate/linux-next linus/master v6.14-rc4 next-20250227]
> [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/Mario-Limonciello/cpufreq-amd-pstate-Invalidate-cppc_req_cached-during-suspend/20250226-155545
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git bleeding-edge
> patch link:    https://lore.kernel.org/r/20250226074934.1667721-5-superm1%40kernel.org
> patch subject: [PATCH v5 04/19] cpufreq/amd-pstate: Move perf values into a union
> config: i386-buildonly-randconfig-003-20250227 (https://download.01.org/0day-ci/archive/20250227/202502272001.nafS0qXq-lkp@intel.com/config)
> compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250227/202502272001.nafS0qXq-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/202502272001.nafS0qXq-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>     In file included from drivers/cpufreq/amd-pstate.c:51:
>     In file included from drivers/cpufreq/amd-pstate-trace.h:15:
>     In file included from include/linux/trace_events.h:6:
>     In file included from include/linux/ring_buffer.h:5:
>     In file included from include/linux/mm.h:2224:
>     include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>       504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>           |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>       505 |                            item];
>           |                            ~~~~
>     include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>       511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>           |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>       512 |                            NR_VM_NUMA_EVENT_ITEMS +
>           |                            ~~~~~~~~~~~~~~~~~~~~~~
>>> drivers/cpufreq/amd-pstate.c:914:41: warning: variable 'nominal_freq' is uninitialized when used here [-Wuninitialized]
>       914 |                 perf.lowest_perf = freq_to_perf(perf, nominal_freq, min_freq);
>           |                                                       ^~~~~~~~~~~~
>     drivers/cpufreq/amd-pstate.c:902:38: note: initialize the variable 'nominal_freq' to silence this warning
>       902 |         u32 min_freq, max_freq, nominal_freq, lowest_nonlinear_freq;
>           |                                             ^
>           |                                              = 0
>     3 warnings generated.
> 
> 
> vim +/nominal_freq +914 drivers/cpufreq/amd-pstate.c
> 
>     891	
>     892	/*
>     893	 * amd_pstate_init_freq: Initialize the nominal_freq and lowest_nonlinear_freq
>     894	 *			 for the @cpudata object.
>     895	 *
>     896	 * Requires: all perf members of @cpudata to be initialized.
>     897	 *
>     898	 * Returns 0 on success, non-zero value on failure.
>     899	 */
>     900	static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>     901	{
>     902		u32 min_freq, max_freq, nominal_freq, lowest_nonlinear_freq;
>     903		struct cppc_perf_caps cppc_perf;
>     904		union perf_cached perf;
>     905		int ret;
>     906	
>     907		ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
>     908		if (ret)
>     909			return ret;
>     910		perf = READ_ONCE(cpudata->perf);
>     911	
>     912		if (quirks && quirks->lowest_freq) {
>     913			min_freq = quirks->lowest_freq;
>   > 914			perf.lowest_perf = freq_to_perf(perf, nominal_freq, min_freq);
>     915			WRITE_ONCE(cpudata->perf, perf);
>     916		} else
>     917			min_freq = cppc_perf.lowest_freq;
>     918	
>     919		if (quirks && quirks->nominal_freq)
>     920			nominal_freq = quirks->nominal_freq;
>     921		else
>     922			nominal_freq = cppc_perf.nominal_freq;
>     923	
>     924		min_freq *= 1000;
>     925		nominal_freq *= 1000;
>     926	
>     927		WRITE_ONCE(cpudata->nominal_freq, nominal_freq);
>     928	
>     929		max_freq = perf_to_freq(perf, nominal_freq, perf.highest_perf);
>     930		lowest_nonlinear_freq = perf_to_freq(perf, nominal_freq, perf.lowest_nonlinear_perf);
>     931		WRITE_ONCE(cpudata->lowest_nonlinear_freq, lowest_nonlinear_freq);
>     932	
>     933		/**
>     934		 * Below values need to be initialized correctly, otherwise driver will fail to load
>     935		 * max_freq is calculated according to (nominal_freq * highest_perf)/nominal_perf
>     936		 * lowest_nonlinear_freq is a value between [min_freq, nominal_freq]
>     937		 * Check _CPC in ACPI table objects if any values are incorrect
>     938		 */
>     939		if (min_freq <= 0 || max_freq <= 0 || nominal_freq <= 0 || min_freq > max_freq) {
>     940			pr_err("min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n",
>     941				min_freq, max_freq, nominal_freq);
>     942			return -EINVAL;
>     943		}
>     944	
>     945		if (lowest_nonlinear_freq <= min_freq || lowest_nonlinear_freq > nominal_freq) {
>     946			pr_err("lowest_nonlinear_freq(%d) value is out of range [min_freq(%d), nominal_freq(%d)]\n",
>     947				lowest_nonlinear_freq, min_freq, nominal_freq);
>     948			return -EINVAL;
>     949		}
>     950	
>     951		return 0;
>     952	}
>     953	
> 

The series is getting close (I think just one more patch needing review).

So if no other feedback for the series needing other fixes I will squash 
this in to fix this issue when the series is merged.

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index bd8bcda4e6eb0..034ee40681b4c 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -915,6 +915,12 @@ static int amd_pstate_init_freq(struct amd_cpudata 
*cpudata)
                 return ret;
         perf = READ_ONCE(cpudata->perf);

+       if (quirks && quirks->nominal_freq)
+               nominal_freq = quirks->nominal_freq;
+       else
+               nominal_freq = cppc_perf.nominal_freq;
+       nominal_freq *= 1000;
+
         if (quirks && quirks->lowest_freq) {
                 min_freq = quirks->lowest_freq;
                 perf.lowest_perf = freq_to_perf(perf, nominal_freq, 
min_freq);
@@ -922,13 +928,7 @@ static int amd_pstate_init_freq(struct amd_cpudata 
*cpudata)
         } else
                 min_freq = cppc_perf.lowest_freq;

-       if (quirks && quirks->nominal_freq)
-               nominal_freq = quirks->nominal_freq;
-       else
-               nominal_freq = cppc_perf.nominal_freq;
-
         min_freq *= 1000;
-       nominal_freq *= 1000;

         WRITE_ONCE(cpudata->nominal_freq, nominal_freq);