Re: [PATCH v6 2/6] cpufreq:amd-pstate: fix the nominal freq value set

[Gautham R. Shenoy]

On Thu, Feb 08, 2024 at 11:46:29AM +0800, Perry Yuan wrote:
> Address an untested error where the nominal_freq was returned in KHz
> instead of the correct MHz units, this oversight led to a wrong
> nominal_freq set and resued, it will cause the max frequency of core to
> be initialized with a wrong frequency value.

The _CPC object on my Zen2 EPYC server which has CPPC v3, I see :
            0x00000190,			//LowestFrequency 
            0x000007D0			//NominalFrequency
        })
    }
 
where 0x7D0 is 2000 Mhz. Thus the value returend by cppc_perf.nominal_freq is in Mhz.

In this patch, we are changing making amd_get_nominal_freq() return
2000 (Mhz) instead of 2000000 (Khz) as it was doing previously.

amd_get_min_freq(), amd_get_max_freq(),
amd_get_lowest_nonlinear_perf() -all these functions return the
frequency in Khz units.

These functions are used the initialize the value of
cpudata->max_freq, cpu_data->min_freq, .... all of which are in Khz
units.

In, amd_pstate_set_boost(), we have:
 
 
	if (state)
		policy->cpuinfo.max_freq = cpudata->max_freq;  <---- In Khz
	else
		policy->cpuinfo.max_freq = cpudata->nominal_freq; <--- Now in Mhz

Since policy->cpuinfo.max_freq is expected to be in Khz, isn't this
patch introducing an error ? Or perhaps this patch series is based on
another patchset ?

--
Thanks and Regards
gautham.

 
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: ec437d71db7 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
> Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> Signed-off-by: Perry Yuan <perry.yuan@xxxxxxx>
> ---
>  drivers/cpufreq/amd-pstate.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 08e112444c27..ac7faa98a450 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -640,8 +640,7 @@ static int amd_get_nominal_freq(struct amd_cpudata *cpudata)
>  	if (ret)
>  		return ret;
>  
> -	/* Switch to khz */
> -	return cppc_perf.nominal_freq * 1000;
> +	return cppc_perf.nominal_freq;
>  }
>  
>  static int amd_get_lowest_nonlinear_freq(struct amd_cpudata *cpudata)
> -- 
> 2.34.1
>