2024-05-27 05:12:10

by Dhananjay Ugwekar

Subject: [PATCH] cpufreq: amd-pstate: Fix the inconsistency in max frequency units

The nominal frequency in cpudata is maintained in MHz whereas all other
frequencies are in KHz. This means we have to convert nominal frequency
value to KHz before we do any interaction with other frequency values.

In amd_pstate_set_boost(), this conversion from MHz to KHz is missed,
fix that.

Tested on a AMD Zen4 EPYC server

Before:
$ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
2151
$ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
400000
$ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
2151
409422

After:
$ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
2151000
$ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
400000
$ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
2151000
1799527

Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
Signed-off-by: Dhananjay Ugwekar <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 1b7e82a0ad2e..cde3b91b4422 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -669,7 +669,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
if (state)
policy->cpuinfo.max_freq = cpudata->max_freq;
else
- policy->cpuinfo.max_freq = cpudata->nominal_freq;
+ policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;

policy->max = policy->cpuinfo.max_freq;

--
2.34.1

2024-05-27 15:36:55

by Mario Limonciello

Subject: Re: [PATCH] cpufreq: amd-pstate: Fix the inconsistency in max frequency units

On 5/27/2024 00:11, Dhananjay Ugwekar wrote:
> The nominal frequency in cpudata is maintained in MHz whereas all other
> frequencies are in KHz. This means we have to convert nominal frequency
> value to KHz before we do any interaction with other frequency values.
>
> In amd_pstate_set_boost(), this conversion from MHz to KHz is missed,
> fix that.
>
> Tested on a AMD Zen4 EPYC server
>
> Before:
> $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> 2151
> $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> 400000
> $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> 2151
> 409422
>
> After:
> $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> 2151000
> $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> 400000
> $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> 2151000
> 1799527
>

Cc: [email protected]

> Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
> Signed-off-by: Dhananjay Ugwekar <[email protected]>

Acked-by: Mario Limonciello <[email protected]>

> ---
> drivers/cpufreq/amd-pstate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1b7e82a0ad2e..cde3b91b4422 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -669,7 +669,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> if (state)
> policy->cpuinfo.max_freq = cpudata->max_freq;
> else
> - policy->cpuinfo.max_freq = cpudata->nominal_freq;
> + policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
>
> policy->max = policy->cpuinfo.max_freq;
>

2024-05-27 15:45:43

by Gautham R.Shenoy

Subject: Re: [PATCH] cpufreq: amd-pstate: Fix the inconsistency in max frequency units

On Mon, May 27, 2024 at 09:40:21AM -0500, Mario Limonciello wrote:
> On 5/27/2024 00:11, Dhananjay Ugwekar wrote:
> > The nominal frequency in cpudata is maintained in MHz whereas all other
> > frequencies are in KHz. This means we have to convert nominal frequency
> > value to KHz before we do any interaction with other frequency values.
> >
> > In amd_pstate_set_boost(), this conversion from MHz to KHz is missed,
> > fix that.
> >
> > Tested on a AMD Zen4 EPYC server
> >
> > Before:
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> > 2151
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> > 400000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> > 2151
> > 409422
> >
> > After:
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> > 2151000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> > 400000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> > 2151000
> > 1799527
> >
>
> Cc: [email protected]
>
> > Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
> > Signed-off-by: Dhananjay Ugwekar <[email protected]>
>
> Acked-by: Mario Limonciello <[email protected]>

Acked-by: Gautham R. Shenoy <[email protected]>

>
> > ---
> > drivers/cpufreq/amd-pstate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 1b7e82a0ad2e..cde3b91b4422 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -669,7 +669,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> > if (state)
> > policy->cpuinfo.max_freq = cpudata->max_freq;
> > else
> > - policy->cpuinfo.max_freq = cpudata->nominal_freq;
> > + policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
> > policy->max = policy->cpuinfo.max_freq;
>

2024-05-28 17:34:36

by Peter Jung

Subject: Re: [PATCH] cpufreq: amd-pstate: Fix the inconsistency in max frequency units

> > > Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD
P-State driver to support future processors")
> > > Signed-off-by: Dhananjay Ugwekar <[email protected]>
> >
> > Acked-by: Mario Limonciello <[email protected]>
>
> Acked-by: Gautham R. Shenoy <[email protected]>


Tested-by: Peter Jung <[email protected]>

Fixes also an introduced regression in amd-pstate=passive reporting
wrong frequency values.

Also, see[1]

[1]https://github.com/CachyOS/linux-cachyos/issues/253#issuecomment-2135659124

2024-05-28 20:11:48

by Rafael J. Wysocki

Subject: Re: [PATCH] cpufreq: amd-pstate: Fix the inconsistency in max frequency units

On Mon, May 27, 2024 at 4:40 PM Mario Limonciello
<[email protected]> wrote:
>
> On 5/27/2024 00:11, Dhananjay Ugwekar wrote:
> > The nominal frequency in cpudata is maintained in MHz whereas all other
> > frequencies are in KHz. This means we have to convert nominal frequency
> > value to KHz before we do any interaction with other frequency values.
> >
> > In amd_pstate_set_boost(), this conversion from MHz to KHz is missed,
> > fix that.
> >
> > Tested on a AMD Zen4 EPYC server
> >
> > Before:
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> > 2151
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> > 400000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> > 2151
> > 409422
> >
> > After:
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq | uniq
> > 2151000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/cpuinfo_min_freq | uniq
> > 400000
> > $ cat /sys/devices/system/cpu/cpufreq/policy*/scaling_cur_freq | uniq
> > 2151000
> > 1799527
> >
>
> Cc: [email protected]
>
> > Fixes: ec437d71db77 ("cpufreq: amd-pstate: Introduce a new AMD P-State driver to support future processors")
> > Signed-off-by: Dhananjay Ugwekar <[email protected]>
>
> Acked-by: Mario Limonciello <[email protected]>

Applied as 6.10-rc material, thanks!

> > ---
> > drivers/cpufreq/amd-pstate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 1b7e82a0ad2e..cde3b91b4422 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -669,7 +669,7 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
> > if (state)
> > policy->cpuinfo.max_freq = cpudata->max_freq;
> > else
> > - policy->cpuinfo.max_freq = cpudata->nominal_freq;
> > + policy->cpuinfo.max_freq = cpudata->nominal_freq * 1000;
> >
> > policy->max = policy->cpuinfo.max_freq;
> >
>