Received-SPF: pass (google.com: domain of linux-kernel+bounces-173190-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240508054703.3728337-1-perry.yuan@amd.com>
In-Reply-To: <20240508054703.3728337-1-perry.yuan@amd.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Wed, 8 May 2024 13:58:55 +0200
Message-ID: <CAJZ5v0iqEj0bx72Rqn--srv8Y+ievjZx6W8_kJksPzPVTLLqxQ@mail.gmail.com>
Subject: Re: [PATCH] cpufreq: amd-pstate: fix the highest frequency issue
 which limit performance
To: Perry Yuan <perry.yuan@amd.com>
Cc: rafael.j.wysocki@intel.com, Mario.Limonciello@amd.com, 
	viresh.kumar@linaro.org, Ray.Huang@amd.com, gautham.shenoy@amd.com, 
	Borislav.Petkov@amd.com, Alexander.Deucher@amd.com, Xinmei.Huang@amd.com, 
	Xiaojian.Du@amd.com, Li.Meng@amd.com, linux-pm@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 8, 2024 at 7:47=E2=80=AFAM Perry Yuan <perry.yuan@amd.com> wrot=
e:
>
> To address the performance drop issue, an optimization has been
> implemented. The incorrect highest performance value previously set by th=
e
> low-level power firmware for AMD CPUs with Family ID 0x19 and Model ID
> ranging from 0x70 to 0x7F series has been identified as the cause.
>
> To resolve this, a check has been implemented to accurately determine the
> CPU family and model ID. The correct highest performance value is now set
> and the performance drop caused by the incorrect highest performance valu=
e
> are eliminated.
>
> Before the fix, the highest frequency was set to 4200MHz, now it is set
> to 4971MHz which is correct.
>
> CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE    MAXMHZ   MINMHZ       MHZ
>   0    0      0    0 0:0:0:0          yes 4971.0000 400.0000  400.0000
>   1    0      0    0 0:0:0:0          yes 4971.0000 400.0000  400.0000
>   2    0      0    1 1:1:1:0          yes 4971.0000 400.0000 4865.8140
>   3    0      0    1 1:1:1:0          yes 4971.0000 400.0000  400.0000
>
> v1->v2:
>  * add test by flag from Gaha Bana

This should go into the section below the tags.

And if you send a v2 of a patch, please indicate that this is a v2 in
the subject.

> Fixes: f3a052391822 ("cpufreq: amd-pstate: Enable amd-pstate preferred co=
re support")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=3D218759
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Gaha Bana <gahabana@gmail.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 2db095867d03..6a342b0c0140 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -50,7 +50,8 @@
>
>  #define AMD_PSTATE_TRANSITION_LATENCY  20000
>  #define AMD_PSTATE_TRANSITION_DELAY    1000
> -#define AMD_PSTATE_PREFCORE_THRESHOLD  166
> +#define CPPC_HIGHEST_PERF_PERFORMANCE  196
> +#define CPPC_HIGHEST_PERF_DEFAULT      166
>
>  /*
>   * TODO: We need more time to fine tune processors with shared memory so=
lution
> @@ -326,6 +327,21 @@ static inline int amd_pstate_enable(bool enable)
>         return static_call(amd_pstate_enable)(enable);
>  }
>
> +static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
> +{
> +       struct cpuinfo_x86 *c =3D &cpu_data(0);
> +
> +       /*
> +        * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7=
f,
> +        * the highest performance level is set to 196.
> +        * https://bugzilla.kernel.org/show_bug.cgi?id=3D218759
> +        */
> +       if (c->x86 =3D=3D 0x19 && (c->x86_model >=3D 0x70 && c->x86_model=
 <=3D 0x7f))
> +               return CPPC_HIGHEST_PERF_PERFORMANCE;
> +
> +       return CPPC_HIGHEST_PERF_DEFAULT;
> +}
> +
>  static int pstate_init_perf(struct amd_cpudata *cpudata)
>  {
>         u64 cap1;
> @@ -342,7 +358,7 @@ static int pstate_init_perf(struct amd_cpudata *cpuda=
ta)
>          * the default max perf.
>          */
>         if (cpudata->hw_prefcore)
> -               highest_perf =3D AMD_PSTATE_PREFCORE_THRESHOLD;
> +               highest_perf =3D amd_pstate_highest_perf_set(cpudata);
>         else
>                 highest_perf =3D AMD_CPPC_HIGHEST_PERF(cap1);
>
> @@ -366,7 +382,7 @@ static int cppc_init_perf(struct amd_cpudata *cpudata=
)
>                 return ret;
>
>         if (cpudata->hw_prefcore)
> -               highest_perf =3D AMD_PSTATE_PREFCORE_THRESHOLD;
> +               highest_perf =3D amd_pstate_highest_perf_set(cpudata);
>         else
>                 highest_perf =3D cppc_perf.highest_perf;
>
> --

Applied as 6.10 material, thanks!