2022-07-09 14:23:48

by Yuan, Perry

[permalink] [raw]
Subject: [PATCH v2 07/14] 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..8a2b6ad9b8c0 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -265,6 +265,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
u64 prev = READ_ONCE(cpudata->cppc_req_cached);
u64 value = prev;

+ des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
value &= ~AMD_CPPC_MIN_PERF(~0L);
value |= AMD_CPPC_MIN_PERF(min_perf);

--
2.25.1


2022-07-11 19:45:11

by Nathan Fontenot

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

On 7/9/22 09:19, 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..8a2b6ad9b8c0 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -265,6 +265,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
> u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> u64 value = prev;
>
> + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);

You should also update amd_pstate_adjust_perf() to remove the clamp_t() call now
that it is done here.

-Nathan

> value &= ~AMD_CPPC_MIN_PERF(~0L);
> value |= AMD_CPPC_MIN_PERF(min_perf);
>

2022-07-12 09:38:52

by Yuan, Perry

[permalink] [raw]
Subject: RE: [PATCH v2 07/14] 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: Tuesday, July 12, 2022 3:34 AM
> To: Yuan, Perry <[email protected]>; [email protected];
> [email protected]; Huang, Ray <[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]>; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 07/14] cpufreq: amd_pstate: map desired perf into
> pstate scope for powersave governor
>
> On 7/9/22 09:19, 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..8a2b6ad9b8c0
> 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -265,6 +265,7 @@ static void amd_pstate_update(struct
> amd_cpudata *cpudata, u32 min_perf,
> > u64 prev = READ_ONCE(cpudata->cppc_req_cached);
> > u64 value = prev;
> >
> > + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
>
> You should also update amd_pstate_adjust_perf() to remove the clamp_t()
> call now that it is done here.
>
> -Nathan

I missed that, changed into V3.
Thank you!

Perry.
>
> > value &= ~AMD_CPPC_MIN_PERF(~0L);
> > value |= AMD_CPPC_MIN_PERF(min_perf);
> >