diff mbox series

[v12,7/9] cpufreq: amd-pstate: fix the MSR highest perf will be reset issue while cpb boost off

Message ID df44ee42774efd7d1c656ed43ff3b35988e22e60.1718787627.git.perry.yuan@amd.com
State New
Headers show
Series AMD Pstate Driver Core Performance Boost | expand

Commit Message

Yuan, Perry June 19, 2024, 9:16 a.m. UTC 
From: Perry Yuan <Perry.Yuan@amd.com>

Select the min perf to fix the highest perf value while update pstate
CPPC request MSR register, here it needs to limit the max perf value when
CPU boost is disabled in case of that highest perf value in the MSR will be
reset to original highest perf value which cause the BOOST control
failed.

Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Gautham R. Shenoy June 21, 2024, 5:46 a.m. UTC | #1 
Perry Yuan <perry.yuan@amd.com> writes:

> From: Perry Yuan <Perry.Yuan@amd.com>
>
> Select the min perf to fix the highest perf value while update pstate
> CPPC request MSR register, here it needs to limit the max perf value when
> CPU boost is disabled in case of that highest perf value in the MSR will be
> reset to original highest perf value which cause the BOOST control
> failed.

Perhaps we could rephrase this as

"When Core Performance Boost is disabled by the user, the
CPPC_REQ.max_perf should not exceed the nominal_perf since by definition
the frequencies between nominal_perf and the highest_perf are in the
boost range. Fix this in amd_pstate_update()"

Also change the subject to

"cpufreq:amd-pstate: Cap the CPPC.max_perf to nominal_perf if CPB is off"

?
>
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> Acked-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Other than that,

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 299e52d4b17e..f2ccef089acc 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -524,6 +524,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
>  			      u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
>  {
>  	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> +	u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
>  	u64 value = prev;
>  
>  	min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
> @@ -543,6 +544,10 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
>  	value &= ~AMD_CPPC_DES_PERF(~0L);
>  	value |= AMD_CPPC_DES_PERF(des_perf);
>  
> +	/* limit the max perf when core performance boost feature is disabled */
> +	if (!amd_pstate_global_params.cpb_boost)
> +		max_perf = min_t(unsigned long, nominal_perf, max_perf);
> +
>  	value &= ~AMD_CPPC_MAX_PERF(~0L);
>  	value |= AMD_CPPC_MAX_PERF(max_perf);
>  
> -- 
> 2.34.1

--
Thanks and Regards
gautham.
Yuan, Perry June 21, 2024, 5:56 a.m. UTC | #2 
[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Shenoy, Gautham Ranjal <gautham.shenoy@amd.com>
> Sent: Friday, June 21, 2024 1:46 PM
> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com;
> Limonciello, Mario <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org
> Cc: Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v12 7/9] cpufreq: amd-pstate: fix the MSR highest perf
> will be reset issue while cpb boost off
>
> Perry Yuan <perry.yuan@amd.com> writes:
>
> > From: Perry Yuan <Perry.Yuan@amd.com>
> >
> > Select the min perf to fix the highest perf value while update pstate
> > CPPC request MSR register, here it needs to limit the max perf value
> > when CPU boost is disabled in case of that highest perf value in the
> > MSR will be reset to original highest perf value which cause the BOOST
> > control failed.
>
> Perhaps we could rephrase this as
>
> "When Core Performance Boost is disabled by the user, the
> CPPC_REQ.max_perf should not exceed the nominal_perf since by definition
> the frequencies between nominal_perf and the highest_perf are in the boost
> range. Fix this in amd_pstate_update()"
>
> Also change the subject to
>
> "cpufreq:amd-pstate: Cap the CPPC.max_perf to nominal_perf if CPB is off"

Sure, will update the tile in v13,  will send out the V13 today to catch the merge window.
Thanks for the review.

Perry.

>
> ?
> >
> > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> > Acked-by: Huang Rui <ray.huang@amd.com>
> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>
> Other than that,
>
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 299e52d4b17e..f2ccef089acc 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -524,6 +524,7 @@ static void amd_pstate_update(struct amd_cpudata
> *cpudata, u32 min_perf,
> >                           u32 des_perf, u32 max_perf, bool fast_switch, int
> gov_flags)
> > {
> >     u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> > +   u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
> >     u64 value = prev;
> >
> >     min_perf = clamp_t(unsigned long, min_perf, cpudata-
> >min_limit_perf,
> > @@ -543,6 +544,10 @@ static void amd_pstate_update(struct
> amd_cpudata *cpudata, u32 min_perf,
> >     value &= ~AMD_CPPC_DES_PERF(~0L);
> >     value |= AMD_CPPC_DES_PERF(des_perf);
> >
> > +   /* limit the max perf when core performance boost feature is
> disabled */
> > +   if (!amd_pstate_global_params.cpb_boost)
> > +           max_perf = min_t(unsigned long, nominal_perf, max_perf);
> > +
> >     value &= ~AMD_CPPC_MAX_PERF(~0L);
> >     value |= AMD_CPPC_MAX_PERF(max_perf);
> >
> > --
> > 2.34.1
>
> --
> Thanks and Regards
> gautham.
diff mbox series

Patch

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 299e52d4b17e..f2ccef089acc 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -524,6 +524,7 @@  static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 			      u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
 {
 	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
+	u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
 	u64 value = prev;
 
 	min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,
@@ -543,6 +544,10 @@  static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 	value &= ~AMD_CPPC_DES_PERF(~0L);
 	value |= AMD_CPPC_DES_PERF(des_perf);
 
+	/* limit the max perf when core performance boost feature is disabled */
+	if (!amd_pstate_global_params.cpb_boost)
+		max_perf = min_t(unsigned long, nominal_perf, max_perf);
+
 	value &= ~AMD_CPPC_MAX_PERF(~0L);
 	value |= AMD_CPPC_MAX_PERF(max_perf);