2022-07-07 17:21:53

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH 07/12] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor

The patch will fix the invalid desired perf value for powersave
governor. This issue is found when testing on one AMD EPYC system, the
actual des_perf is smaller than the min_perf value, that is invalid
value. because the min_perf is the lowest_perf system can support in
idle state.

Signed-off-by: Perry Yuan <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 7c51f4125263..154eed849f38 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -317,6 +317,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
des_perf = DIV_ROUND_CLOSEST(target_freq * cap_perf,
cpudata->max_freq);

+ des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
cpufreq_freq_transition_begin(policy, &freqs);
amd_pstate_update(cpudata, min_perf, des_perf,
max_perf, false);
--
2.25.1


2022-07-07 20:08:01

by Nathan Fontenot

[permalink] [raw]
Subject: Re: [PATCH 07/12] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor

On 7/7/22 12:00, Perry Yuan wrote:
> The patch will fix the invalid desired perf value for powersave
> governor. This issue is found when testing on one AMD EPYC system, the
> actual des_perf is smaller than the min_perf value, that is invalid
> value. because the min_perf is the lowest_perf system can support in
> idle state.
>
> Signed-off-by: Perry Yuan <[email protected]>
> ---
> drivers/cpufreq/amd-pstate.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 7c51f4125263..154eed849f38 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -317,6 +317,7 @@ static int amd_pstate_target(struct cpufreq_policy *policy,
> des_perf = DIV_ROUND_CLOSEST(target_freq * cap_perf,
> cpudata->max_freq);
>
> + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> cpufreq_freq_transition_begin(policy, &freqs);
> amd_pstate_update(cpudata, min_perf, des_perf,
> max_perf, false);

The clamping of the desired perf value should be moved to amd_pstate_update(). The
only other caller of amd_pstate_update() is amd_pstate_adjust_perf() which already
clamps the desired perf value before making the call.

-Nathan

2022-07-09 09:22:55

by Yuan, Perry

[permalink] [raw]
Subject: RE: [PATCH 07/12] cpufreq: amd_pstate: map desired perf into pstate scope for powersave governor

[AMD Official Use Only - General]

Hi Nathan:

> -----Original Message-----
> From: Fontenot, Nathan <[email protected]>
> Sent: Friday, July 8, 2022 3:57 AM
> To: Yuan, Perry <[email protected]>; [email protected];
> [email protected]; Huang, Ray <[email protected]>; Rafael J.
> Wysocki <[email protected]>; [email protected]; linux-
> [email protected]
> Cc: Sharma, Deepak <[email protected]>; Limonciello, Mario
> <[email protected]>; Fontenot, Nathan
> <[email protected]>; Deucher, Alexander
> <[email protected]>; Su, Jinzhou (Joe) <[email protected]>;
> Huang, Shimmer <[email protected]>; Du, Xiaojian
> <[email protected]>; Meng, Li (Jassmine) <[email protected]>
> Subject: Re: [PATCH 07/12] cpufreq: amd_pstate: map desired perf into
> pstate scope for powersave governor
>
> On 7/7/22 12:00, Perry Yuan wrote:
> > The patch will fix the invalid desired perf value for powersave
> > governor. This issue is found when testing on one AMD EPYC system, the
> > actual des_perf is smaller than the min_perf value, that is invalid
> > value. because the min_perf is the lowest_perf system can support in
> > idle state.
> >
> > Signed-off-by: Perry Yuan <[email protected]>
> > ---
> > drivers/cpufreq/amd-pstate.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 7c51f4125263..154eed849f38
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -317,6 +317,7 @@ static int amd_pstate_target(struct cpufreq_policy
> *policy,
> > des_perf = DIV_ROUND_CLOSEST(target_freq * cap_perf,
> > cpudata->max_freq);
> >
> > + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> > cpufreq_freq_transition_begin(policy, &freqs);
> > amd_pstate_update(cpudata, min_perf, des_perf,
> > max_perf, false);
>
> The clamping of the desired perf value should be moved to
> amd_pstate_update(). The only other caller of amd_pstate_update() is
> amd_pstate_adjust_perf() which already clamps the desired perf value
> before making the call.
>
> -Nathan

Thanks for your suggestion.
Add this change into V2.

Perry.