drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
There are 3 places at which the maximum CPU frequency may change,
store_no_turbo(), intel_pstate_update_limits() (when called by the
cpufreq core) and intel_pstate_notify_work() (when handling a HWP
change notification). Currently, cpuinfo.max_freq is only updated by
store_no_turbo(), although it principle it may be necessary to update
it at the other 2 places too.
Make all of them mutually consistent.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1153,18 +1153,32 @@ static void intel_pstate_update_policies
static void __intel_pstate_update_max_freq(struct cpudata *cpudata,
struct cpufreq_policy *policy)
{
+ intel_pstate_get_hwp_cap(cpudata);
+
policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
cpudata->pstate.max_freq : cpudata->pstate.turbo_freq;
+
refresh_frequency_limits(policy);
}
static void intel_pstate_update_limits(unsigned int cpu)
{
- mutex_lock(&intel_pstate_driver_lock);
+ struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu);
- cpufreq_update_policy(cpu);
+ if (!policy)
+ return;
- mutex_unlock(&intel_pstate_driver_lock);
+ __intel_pstate_update_max_freq(all_cpu_data[cpu], policy);
+
+ cpufreq_cpu_release(policy);
+}
+
+static void intel_pstate_update_limits_for_all(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ intel_pstate_update_limits(cpu);
}
/************************** sysfs begin ************************/
@@ -1311,7 +1325,7 @@ static ssize_t store_no_turbo(struct kob
mutex_unlock(&intel_pstate_limits_lock);
- intel_pstate_update_policies();
+ intel_pstate_update_limits_for_all();
arch_set_max_freq_ratio(no_turbo);
unlock_driver:
@@ -1595,7 +1609,6 @@ static void intel_pstate_notify_work(str
struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
if (policy) {
- intel_pstate_get_hwp_cap(cpudata);
__intel_pstate_update_max_freq(cpudata, policy);
cpufreq_cpu_release(policy);On Mon, 2024-03-25 at 18:06 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> There are 3 places at which the maximum CPU frequency may change,
> store_no_turbo(), intel_pstate_update_limits() (when called by the
> cpufreq core) and intel_pstate_notify_work() (when handling a HWP
> change notification). Currently, cpuinfo.max_freq is only updated by
> store_no_turbo(), although it principle it may be necessary to update
> it at the other 2 places too.
It also works for intel_pstate_notify_work() path as is without this
change.
To start with:
$ sudo rdmsr 0x771
6080c14
$ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
2000000
800000
0
Now trigger a max frequency change via SST. intel_pstate_notify_work()
called because guaranteed also changed. We didn't subscribe for max
change only (although we should).
Max changed from 2GHz to 1.9 GHz.
$ sudo rdmsr 0x771
6080e13
[labuser@gnr-bkc ~]$ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
1900000
800000
0
Now trigger SST to change to max frequency to 2GHz.
sudo rdmsr 0x771
6080c14
cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_*
2000000
800000
0
May be you mean something else.
Thanks,
Srinivas
>
> Make all of them mutually consistent.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1153,18 +1153,32 @@ static void intel_pstate_update_policies
> static void __intel_pstate_update_max_freq(struct cpudata *cpudata,
> struct cpufreq_policy
> *policy)
> {
> + intel_pstate_get_hwp_cap(cpudata);
> +
> policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
> cpudata->pstate.max_freq : cpudata-
> >pstate.turbo_freq;
> +
> refresh_frequency_limits(policy);
> }
>
> static void intel_pstate_update_limits(unsigned int cpu)
> {
> - mutex_lock(&intel_pstate_driver_lock);
> + struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu);
>
> - cpufreq_update_policy(cpu);
> + if (!policy)
> + return;
>
> - mutex_unlock(&intel_pstate_driver_lock);
> + __intel_pstate_update_max_freq(all_cpu_data[cpu], policy);
> +
> + cpufreq_cpu_release(policy);
> +}
> +
> +static void intel_pstate_update_limits_for_all(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu)
> + intel_pstate_update_limits(cpu);
> }
>
> /************************** sysfs begin ************************/
> @@ -1311,7 +1325,7 @@ static ssize_t store_no_turbo(struct kob
>
> mutex_unlock(&intel_pstate_limits_lock);
>
> - intel_pstate_update_policies();
> + intel_pstate_update_limits_for_all();
> arch_set_max_freq_ratio(no_turbo);
>
> unlock_driver:
> @@ -1595,7 +1609,6 @@ static void intel_pstate_notify_work(str
> struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata-
> >cpu);
>
> if (policy) {
> - intel_pstate_get_hwp_cap(cpudata);
> __intel_pstate_update_max_freq(cpudata, policy);
>
> cpufreq_cpu_release(policy);
>
>
>
On Wed, Mar 27, 2024 at 7:08 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > On Mon, 2024-03-25 at 18:06 +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > There are 3 places at which the maximum CPU frequency may change, > > store_no_turbo(), intel_pstate_update_limits() (when called by the > > cpufreq core) and intel_pstate_notify_work() (when handling a HWP > > change notification). Currently, cpuinfo.max_freq is only updated by > > store_no_turbo(), although it principle it may be necessary to update > > it at the other 2 places too. > > It also works for intel_pstate_notify_work() path as is without this > change. > > To start with: > > $ sudo rdmsr 0x771 > 6080c14 > $ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_* > 2000000 > 800000 > 0 > > Now trigger a max frequency change via SST. intel_pstate_notify_work() > called because guaranteed also changed. We didn't subscribe for max > change only (although we should). > > Max changed from 2GHz to 1.9 GHz. > > $ sudo rdmsr 0x771 > 6080e13 > [labuser@gnr-bkc ~]$ cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_* > 1900000 > 800000 > 0 > > Now trigger SST to change to max frequency to 2GHz. > > sudo rdmsr 0x771 > 6080c14 > > cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_* > 2000000 > 800000 > 0 > > May be you mean something else. No, I don't, and you are right. When I was writing the changelog, I somehow forgot that intel_pstate_notify_work() called __intel_pstate_update_max_freq(), even though the code changes in the patch obviously take that into account (I can't really explain what happened :-/). I'll fix the changelog. Cheers, Rafael
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
There are 3 places at which the maximum CPU frequency may change,
store_no_turbo(), intel_pstate_update_limits() (when called by the
cpufreq core) and intel_pstate_notify_work() (when handling a HWP
change notification). Currently, cpuinfo.max_freq is only updated by
store_no_turbo() and intel_pstate_notify_work(), although it principle
it may be necessary to update it in intel_pstate_update_limits() either.
Make all of them mutually consistent.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v1.1: Fix changelog (Srinivas).
---
drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1153,18 +1153,32 @@ static void intel_pstate_update_policies
static void __intel_pstate_update_max_freq(struct cpudata *cpudata,
struct cpufreq_policy *policy)
{
+ intel_pstate_get_hwp_cap(cpudata);
+
policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
cpudata->pstate.max_freq : cpudata->pstate.turbo_freq;
+
refresh_frequency_limits(policy);
}
static void intel_pstate_update_limits(unsigned int cpu)
{
- mutex_lock(&intel_pstate_driver_lock);
+ struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpu);
- cpufreq_update_policy(cpu);
+ if (!policy)
+ return;
- mutex_unlock(&intel_pstate_driver_lock);
+ __intel_pstate_update_max_freq(all_cpu_data[cpu], policy);
+
+ cpufreq_cpu_release(policy);
+}
+
+static void intel_pstate_update_limits_for_all(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ intel_pstate_update_limits(cpu);
}
/************************** sysfs begin ************************/
@@ -1311,7 +1325,7 @@ static ssize_t store_no_turbo(struct kob
mutex_unlock(&intel_pstate_limits_lock);
- intel_pstate_update_policies();
+ intel_pstate_update_limits_for_all();
arch_set_max_freq_ratio(no_turbo);
unlock_driver:
@@ -1595,7 +1609,6 @@ static void intel_pstate_notify_work(str
struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
if (policy) {
- intel_pstate_get_hwp_cap(cpudata);
__intel_pstate_update_max_freq(cpudata, policy);
cpufreq_cpu_release(policy);© 2016 - 2026 Red Hat, Inc.