Re: [PATCH 2/2] cpufreq: amd-pstate: Use amd_get_highest_perf() to lookup perf values

[Mario Limonciello]

On 6/27/2024 00:12, Gautham R.Shenoy wrote:
> Mario Limonciello <mario.limonciello@xxxxxxx> writes:
>
> > To keep consistency with amd-pstate and acpi-cpufreq behavior, use
> > amd_get_highest_perf() to find the highest perf value for a given
> > platform.
> >
> > This fixes the exact same problem as commit bf202e654bfa ("cpufreq:
> > amd-pstate: fix the highest frequency issue which limits performance")
> > from happening on acpi-cpufreq too.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > ---
> >   arch/x86/kernel/cpu/amd.c    | 16 +++++++++++++++-
> >   drivers/cpufreq/amd-pstate.c | 21 ++-------------------
> >   2 files changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > index 8b730193d79e..e69f640cc248 100644
> > --- a/arch/x86/kernel/cpu/amd.c
> > +++ b/arch/x86/kernel/cpu/amd.c
> > @@ -1218,7 +1218,21 @@ u32 amd_get_highest_perf(void)
> >   		}
> >   	}
>
>  From Patch 1,
>
> +#define CPPC_HIGHEST_PERF_MAX		255
> +#define CPPC_HIGHEST_PERF_PERFORMANCE	196
> +#define CPPC_HIGHEST_PERF_DEFAULT	166
> +
>
>
>
> > -	return CPPC_HIGHEST_PERF_MAX;
> > +	/*
> > +	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
> > +	 * the highest performance level is set to 196.
> > +	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
> > +	 */
> > +	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
> > +		switch (c->x86_model) {
> > +		case 0x70 ... 0x7f:
> > +			return CPPC_HIGHEST_PERF_PERFORMANCE;
> > +		default:
> > +			return CPPC_HIGHEST_PERF_DEFAULT;
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Should this be CPPC_HIGHEST_PERF_MAX ?
>
> Without this patchset, this function returns 255 on Genoa (0x10-0x1f)
> and Bergamo (0xa0-0xaf) systems. This patchset changes the return value
> to 166.
>
> The acpi-cpufreq driver computes the max frequency based on the
> boost-ratio, which is the ratio of the highest_perf (returned by this
> function) to the nominal_perf.
>
> So assuming a nominal_freq of 2000Mhz, nominal_perf of 159.
>
> Previously the max_perf = (2000*255/159) ~ 3200Mhz
> With this patch max_perf = (2000*166/159) ~ 2100Mhz.
>
> Am I missing something ?

Yeah; this is exactly what I'm worried about.

How does Bergamo handle amd-pstate?  It should probably explode there too.