2023-05-18 06:14:33

by Wyes Karny

Subject: [PATCH] cpufreq: amd-pstate: Update policy->cur for adjust perf

Driver should update policy->cur after updating the frequency.
Currently amd_pstate doesn't update policy->cur when `adjust_perf`
is used. Which causes /proc/cpuinfo to show wrong cpu frequency.
Fix this by updating policy->cur with correct frequency value in
adjust_perf function callback.

- Before the fix: (setting min freq to 1.5 MHz)

[root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
1 cpu MHz : 1777.016
1 cpu MHz : 1797.160
1 cpu MHz : 1797.270
189 cpu MHz : 400.000

- After the fix: (setting min freq to 1.5 MHz)

[root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
1 cpu MHz : 1753.353
1 cpu MHz : 1756.838
1 cpu MHz : 1776.466
1 cpu MHz : 1776.873
1 cpu MHz : 1777.308
1 cpu MHz : 1779.900
183 cpu MHz : 1805.231
1 cpu MHz : 1956.815
1 cpu MHz : 2246.203
1 cpu MHz : 2259.984

Fixes: 1d215f0319c2 ("cpufreq: amd-pstate: Add fast switch function for AMD P-State")

Signed-off-by: Wyes Karny <[email protected]>
---
drivers/cpufreq/amd-pstate.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5a3d4aa0f45a..736dab69ba1e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -479,12 +479,14 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
unsigned long capacity)
{
unsigned long max_perf, min_perf, des_perf,
- cap_perf, lowest_nonlinear_perf;
+ cap_perf, lowest_nonlinear_perf, max_freq;
struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
struct amd_cpudata *cpudata = policy->driver_data;
+ unsigned int target_freq;

cap_perf = READ_ONCE(cpudata->highest_perf);
lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
+ max_freq = READ_ONCE(cpudata->max_freq);

des_perf = cap_perf;
if (target_perf < capacity)
@@ -501,6 +503,10 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
if (max_perf < min_perf)
max_perf = min_perf;

+ des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
+ target_freq = div_u64(des_perf * max_freq, max_perf);
+ policy->cur = target_freq;
+
amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
policy->governor->flags);
cpufreq_cpu_put(policy);
--
2.34.1

2023-05-24 18:19:40

by Rafael J. Wysocki

Subject: Re: [PATCH] cpufreq: amd-pstate: Update policy->cur for adjust perf

On Thu, May 18, 2023 at 7:58 AM Wyes Karny <[email protected]> wrote:
>
> Driver should update policy->cur after updating the frequency.
> Currently amd_pstate doesn't update policy->cur when `adjust_perf`
> is used. Which causes /proc/cpuinfo to show wrong cpu frequency.
> Fix this by updating policy->cur with correct frequency value in
> adjust_perf function callback.
>
> - Before the fix: (setting min freq to 1.5 MHz)
>
> [root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
> 1 cpu MHz : 1777.016
> 1 cpu MHz : 1797.160
> 1 cpu MHz : 1797.270
> 189 cpu MHz : 400.000
>
> - After the fix: (setting min freq to 1.5 MHz)
>
> [root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
> 1 cpu MHz : 1753.353
> 1 cpu MHz : 1756.838
> 1 cpu MHz : 1776.466
> 1 cpu MHz : 1776.873
> 1 cpu MHz : 1777.308
> 1 cpu MHz : 1779.900
> 183 cpu MHz : 1805.231
> 1 cpu MHz : 1956.815
> 1 cpu MHz : 2246.203
> 1 cpu MHz : 2259.984
>
> Fixes: 1d215f0319c2 ("cpufreq: amd-pstate: Add fast switch function for AMD P-State")
>
> Signed-off-by: Wyes Karny <[email protected]>
> ---
> drivers/cpufreq/amd-pstate.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5a3d4aa0f45a..736dab69ba1e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -479,12 +479,14 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> unsigned long capacity)
> {
> unsigned long max_perf, min_perf, des_perf,
> - cap_perf, lowest_nonlinear_perf;
> + cap_perf, lowest_nonlinear_perf, max_freq;
> struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> struct amd_cpudata *cpudata = policy->driver_data;
> + unsigned int target_freq;
>
> cap_perf = READ_ONCE(cpudata->highest_perf);
> lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
> + max_freq = READ_ONCE(cpudata->max_freq);
>
> des_perf = cap_perf;
> if (target_perf < capacity)
> @@ -501,6 +503,10 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> if (max_perf < min_perf)
> max_perf = min_perf;
>
> + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> + target_freq = div_u64(des_perf * max_freq, max_perf);
> + policy->cur = target_freq;
> +
> amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
> policy->governor->flags);
> cpufreq_cpu_put(policy);
> --

Applied under an edited subject, thanks!

I think you'd like this to go into 6.4 and "stable", right?

2023-05-25 06:51:57

by Wyes Karny

Subject: Re: [PATCH] cpufreq: amd-pstate: Update policy->cur for adjust perf

Hi Rafael,

On 24 May 19:57, Rafael J. Wysocki wrote:
> On Thu, May 18, 2023 at 7:58 AM Wyes Karny <[email protected]> wrote:
> >
> > Driver should update policy->cur after updating the frequency.
> > Currently amd_pstate doesn't update policy->cur when `adjust_perf`
> > is used. Which causes /proc/cpuinfo to show wrong cpu frequency.
> > Fix this by updating policy->cur with correct frequency value in
> > adjust_perf function callback.
> >
> > - Before the fix: (setting min freq to 1.5 MHz)
> >
> > [root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
> > 1 cpu MHz : 1777.016
> > 1 cpu MHz : 1797.160
> > 1 cpu MHz : 1797.270
> > 189 cpu MHz : 400.000
> >
> > - After the fix: (setting min freq to 1.5 MHz)
> >
> > [root@amd]# cat /proc/cpuinfo | grep "cpu MHz" | sort | uniq --count
> > 1 cpu MHz : 1753.353
> > 1 cpu MHz : 1756.838
> > 1 cpu MHz : 1776.466
> > 1 cpu MHz : 1776.873
> > 1 cpu MHz : 1777.308
> > 1 cpu MHz : 1779.900
> > 183 cpu MHz : 1805.231
> > 1 cpu MHz : 1956.815
> > 1 cpu MHz : 2246.203
> > 1 cpu MHz : 2259.984
> >
> > Fixes: 1d215f0319c2 ("cpufreq: amd-pstate: Add fast switch function for AMD P-State")
> >
> > Signed-off-by: Wyes Karny <[email protected]>
> > ---
> > drivers/cpufreq/amd-pstate.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 5a3d4aa0f45a..736dab69ba1e 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -479,12 +479,14 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> > unsigned long capacity)
> > {
> > unsigned long max_perf, min_perf, des_perf,
> > - cap_perf, lowest_nonlinear_perf;
> > + cap_perf, lowest_nonlinear_perf, max_freq;
> > struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> > struct amd_cpudata *cpudata = policy->driver_data;
> > + unsigned int target_freq;
> >
> > cap_perf = READ_ONCE(cpudata->highest_perf);
> > lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
> > + max_freq = READ_ONCE(cpudata->max_freq);
> >
> > des_perf = cap_perf;
> > if (target_perf < capacity)
> > @@ -501,6 +503,10 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
> > if (max_perf < min_perf)
> > max_perf = min_perf;
> >
> > + des_perf = clamp_t(unsigned long, des_perf, min_perf, max_perf);
> > + target_freq = div_u64(des_perf * max_freq, max_perf);
> > + policy->cur = target_freq;
> > +
> > amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
> > policy->governor->flags);
> > cpufreq_cpu_put(policy);
> > --
>
> Applied under an edited subject, thanks!
>
> I think you'd like this to go into 6.4 and "stable", right?

Yes, please. I should have added stable tag.

Thanks & Regards,
Wyes