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

Mario Limonciello
2024-10-29T21:54:43Z

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);
>