| Message ID | 9269494.CDJkKcVGEf@kreacher (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | intel_pstate: Turbo disabled handling rework | expand |
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
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);