Re: [PATCH 6.1.y v2] cpufreq: amd-pstate: Enable CPU boost in passive and guided modes

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/29/2024 16:36, Nabil S. Alramli wrote:
> CPU frequency cannot be boosted when using the amd_pstate driver in
> passive or guided mode.
> 
> On a host that has an AMD EPYC 7662 processor, while running with
> amd-pstate configured for passive mode on full CPU load, the processor
> only reaches 2.0 GHz. On later kernels the CPU can reach 3.3GHz.
> 
> The CPU frequency is dependent on a setting called highest_perf which is
> the multiplier used to compute it. The highest_perf value comes from
> cppc_init_perf when the driver is built-in and from pstate_init_perf when
> it is a loaded module. Both of these calls have the following condition:
> 
> 	highest_perf = amd_get_highest_perf();
> 	if (highest_perf > __cppc_highest_perf_)
> 		highest_perf = __cppc_highest_perf;
> 
> Where again __cppc_highest_perf is either the return from
> cppc_get_perf_caps in the built-in case or AMD_CPPC_HIGHEST_PERF in the
> module case. Both of these functions actually return the nominal value,
> whereas the call to amd_get_highest_perf returns the correct boost value,
> so the condition tests true and highest_perf always ends up being the
> nominal value, therefore never having the ability to boost CPU frequency.
> 
> Since amd_get_highest_perf already returns the boost value, we have
> eliminated this check.
> 
> The issue was introduced in v6.1 via commit bedadcfb011f ("cpufreq:
> amd-pstate: Fix initial highest_perf value"), and exists in stable v6.1
> kernels. This has been fixed in v6.6.y and newer but due to refactoring that
> change isn't feasible to bring back to v6.1.y. Thus, v6.1 kernels are
> affected by this significant performance issue, and cannot be easily
> remediated.
> 
> Signed-off-by: Nabil S. Alramli <dev@nalramli.com>
> Reviewed-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Kyle Hubert <khubert@fastly.com>
> Fixes: bedadcfb011f ("cpufreq: amd-pstate: Fix initial highest_perf value")
> See-also: 1ec40a175a48 ("cpufreq: amd-pstate: Enable amd-pstate preferred core support")
> Cc: mario.limonciello@amd.com
> Cc: Perry.Yuan@amd.com
> Cc: li.meng@amd.com
> Cc: stable@vger.kernel.org # v6.1
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   v2:
>     - Omit cover letter
>     - Converted from RFC to PATCH
>     - Expand commit message based on feedback from Mario Limonciello
>     - Added Reviewed-by tags
>     - No functional/code changes
> 
>   rfc:
>   https://lore.kernel.org/lkml/20241025010527.491605-1-dev@nalramli.com/
> ---
>   drivers/cpufreq/amd-pstate.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 90dcf26f0973..c66086ae624a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -102,9 +102,7 @@ static int pstate_init_perf(struct amd_cpudata *cpudata)
>   	 *
>   	 * CPPC entry doesn't indicate the highest performance in some ASICs.
>   	 */
> -	highest_perf = amd_get_highest_perf();
> -	if (highest_perf > AMD_CPPC_HIGHEST_PERF(cap1))
> -		highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
> +	highest_perf = max(amd_get_highest_perf(), AMD_CPPC_HIGHEST_PERF(cap1));
>   
>   	WRITE_ONCE(cpudata->highest_perf, highest_perf);
>   
> @@ -124,9 +122,7 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
>   	if (ret)
>   		return ret;
>   
> -	highest_perf = amd_get_highest_perf();
> -	if (highest_perf > cppc_perf.highest_perf)
> -		highest_perf = cppc_perf.highest_perf;
> +	highest_perf = max(amd_get_highest_perf(), cppc_perf.highest_perf);
>   
>   	WRITE_ONCE(cpudata->highest_perf, highest_perf);
>