sched/cpufreq: fix schedutil's boost frequency handling
v3 cover sched/cpufreq: fix schedutil's boost frequency handling
Schedutil's ability to reach boost frequencies depends on two values being correct: policy max, which caps the resolved target frequency, and the per-CPU capacity frequency reference, which anchors the utilization-to-frequency mapping.
This series fixes a few gaps in how these values are maintained across boost transitions:
The per-CPU capacity frequency reference is set once at policy creation and never updated when boost is enabled afterwards, leaving schedutil unable to target boost frequencies even at full utilization. Introduce a helper cpufreq_frequency_table_max tp retrieve the highest value listed in the frequency table and use the higher of it and cpuinfo max_freq to seed capacity freq ref, letting it know the highest available frequency from policy creation.
The generic boost callback only raises cpuinfo max, never lowers it. Once boost is enabled, disabling it leaves cpuinfo max pinned at the boost ceiling, keeping policy max stuck there too. Track the max available non-boost frequency and use this in conjunction with the the highest frequency retrieved from the table with cpufreq_frequency_table_max to control boost frequencies when boost frequenies are available, allowing the frequency to drop back to non boost values on boost disable. In the absence of boost frequencies, the handling will fall back to using cpuinfo->max_freq, preserving the current behaviour.
Oleg Keri tested the v2 on Lenovo Yoga Slim 7x Gen 11 and provided Tested-by, but it is not picked up because of the implementation changes in v3.
Logs below for clear context:
Intermediate values from the time_in_state output and logs from bench runs are truncated for brevity.
Before fix
----------
\# cd /sys/devices/system/cpu/cpufreq/
\# ls
boost policy0 policy12 policy6
\# cat boost
0
\# cat policy6/scaling_max_freq
4454400
\# echo 1 > boost
\# cat policy6/scaling_max_freq
4723200
\# echo 0 > boost
\# cat policy6/scaling_max_freq
4723200
\# echo 1 > boost
\# cat policy6/stats/time_in_state
355200 36958
4454400 650
4588800 0
4723200 0
\#
After fix
---------
\# cd /sys/devices/system/cpu/cpufreq/
\# ls
boost policy0 policy12 policy6
\# cat boost
0
\# cat policy6/scaling_max_freq
4454400
\# echo 1 > boost
\# cat policy6/scaling_max_freq
4723200
\# echo 0 > policy6/boost
\# cat policy6/scaling_max_freq
4454400
\# cat policy6/boost
0
\# cat policy6/stats/time_in_state
355200 40147
4454400 79
4588800 0
4723200 0
\# echo 1 > policy6/boost
\# cat policy6/stats/time_in_state
355200 44834
4454400 93
4588800 25
4723200 569
To: Vincent Guittot <[email protected]>
To: Sudeep Holla <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Rafael J. Wysocki <[email protected]>
To: Danilo Krummrich <[email protected]>
To: Viresh Kumar <[email protected]>
To: Jie Zhan <[email protected]>
To: Pierre Gondois <[email protected]>
Changes in v3:
- dropped max_table_freq field and added cpuinfo_frequency_table_max function to retrieve max available freq instead
- changed the condition in policy_st_boost for not relying in cpuinfo max_freq to be whether boost frequencies are available, instead of whether a freq table is present.
- changed fixes tag for second patch to Fixes: db80ad776cd2 ("cpufreq: Remove driver default
policy->min/max init") - thanks to Zhongqiu Han for the checks and suggestions
- Link to v2: https://lore.kernel.org/r/20260908-schedutil-boost-frequency-handling-v2-0-25312a713699@oss.qualcomm.com
Changes in v2:
- introduced max_table_freq/max_base_freq fields in cpufreq policy struct to use for seeding capacity freq ref and handle frequency switching during boost cycling.
- does not try to update get_next_freq or drop the one way guard in cpufreq_frequency_table_cpuinfo anymore
- Link to v1: https://lore.kernel.org/all/[email protected]/
---
Ananthu C V (2):
arch_topology: seed capacity_freq_ref with highest available frequency
cpufreq: fix policy max not returning to non-boost freq on boost disable
drivers/base/arch_topology.c | 3 ++- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 16 ++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-)
---
base-commit: a8c591ed6b672915e0be57843f943a2a723aff40
change-id: 20260804-schedutil-boost-frequency-handling-8e6bf2387a4a
Best regards,
Signature · 1 line
Ananthu C V <[email protected]>
PATCH v3 1/2 arch_topology: seed capacity_freq_ref with highest available frequency
capacity_freq_ref, the per-CPU frequency-invariance reference used by schedutil, is seeded from policy->cpuinfo.max_freq at policy creation. If boost isn't enabled yet at boot, max_freq reflects only the non-boost ceiling, so capacity_freq_ref never learns about boost frequencies for the policy's lifetime. Enabling boost later raises policy->max, but capacity_freq_ref stays stale, leaving schedutil unable to scale utilization or target a frequency above the non-boost maximum.
Seed capacity_freq_ref with the highest available frequency regardless of whether boost is enabled. Runtime enforcement, still handled by policy->max, is unaffected.
--- drivers/base/arch_topology.c | 3 ++- include/linux/cpufreq.h | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-)
drivers/base/arch_topology.c +2 -1
@@ -404,7 +404,8 @@ init_cpu_capacity_callback(struct notifier_block *nb,cpumask_andnot (cpus_to_visit, cpus_to_visit, policy->related_cpus);for_each_cpu (cpu, policy->related_cpus) { -per_cpu (capacity_freq_ref, cpu) = policy->cpuinfo.max_freq; +per_cpu (capacity_freq_ref, cpu) =max (policy->cpuinfo.max_freq, +cpufreq_frequency_table_max (policy));freq_inv_set_max_ratio (cpu,per_cpu (capacity_freq_ref, cpu) *HZ_PER_KHZ ); }
include/linux/cpufreq.h +13 -0
@@ -1107,6 +1107,19 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,return idx; } +static inline unsigned int cpufreq_frequency_table_max (struct cpufreq_policy *policy) +{ +int idx; + +if (!policy->freq_table) +return policy->cpuinfo.max_freq; + + idx =cpufreq_frequency_table_target (policy,UINT_MAX ,0 ,UINT_MAX , +CPUFREQ_RELATION_H ); + +return policy->freq_table[idx].frequency; +} +static inline int cpufreq_table_count_valid_entries (const struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos;
Signature · 1 line
2.43.0
Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency
Quoted 45 lines · Ananthu C V
capacity_freq_ref, the per-CPU frequency-invariance reference used by schedutil, is seeded frompolicy->cpuinfo.max_freqat policy creation. If boost isn't enabled yet at boot,max_freqreflects only the non-boost ceiling, socapacity_freq_refnever learns about boost frequencies for the policy's lifetime. Enabling boost later raisespolicy->max, butcapacity_freq_refstays stale, leaving schedutil unable to scale utilization or target a frequency above the non-boost maximum.Seed
capacity_freq_refwith the highest available frequency regardless of whether boost is enabled. Runtime enforcement, still handled bypolicy->max, is unaffected.Suggested-by Vincent Guittot <[email protected]>Signed-off-by Ananthu C V <[email protected]>--- drivers/base/arch_topology.c | 3 ++- include/linux/cpufreq.h | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-)drivers/base/arch_topology.c +2
-1@@ -404,7 +404,8 @@ init_cpu_capacity_callback(struct notifier_block *nb,cpumask_andnot (cpus_to_visit, cpus_to_visit, policy->related_cpus);for_each_cpu (cpu, policy->related_cpus) { -per_cpu (capacity_freq_ref, cpu) = policy->cpuinfo.max_freq; +per_cpu (capacity_freq_ref, cpu) =max (policy->cpuinfo.max_freq, +cpufreq_frequency_table_max (policy));freq_inv_set_max_ratio (cpu,per_cpu (capacity_freq_ref, cpu) *HZ_PER_KHZ ); }include/linux/cpufreq.h +3
-0@@ -1107,6 +1107,19 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,return idx; } +static inline unsigned int cpufreq_frequency_table_max (struct cpufreq_policy *policy) +{ +int idx;
include/linux/cpufreq.h static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, · line 1122
Quoted 10 lines
include/linux/cpufreq.h +10
-0+ +if (!policy->freq_table) +return policy->cpuinfo.max_freq; + + idx =cpufreq_frequency_table_target (policy,UINT_MAX ,0 ,UINT_MAX , +CPUFREQ_RELATION_H ); + +return policy->freq_table[idx].frequency; +} +
The current implementation looks correct and works well. That said, we might be able to optimize it with a simpler implementation as shown below:
+/**
+ * cpufreq_frequency_table_max - Get the highest frequency in a policy's table
+ * @policy: the &struct cpufreq_policy to inspect
+ *
+ * Boost entries (%CPUFREQ_BOOST_FREQ) are taken into account regardless of
+ * whether or not boost is currently enabled.
+ *
+ * Return: the highest valid frequency in @policy->freq_table, or 0 if there
+ * is no frequency table.
+ */
+static inline unsigned int
+cpufreq_frequency_table_max(const struct cpufreq_policy *policy) +{
+ struct cpufreq_frequency_table *pos;
+ unsigned int max_freq = 0;
+
+ if (!policy->freq_table)
+ return 0;
+
+ cpufreq_for_each_valid_entry(pos, policy->freq_table)
+ max_freq = max(max_freq, pos->frequency);
+
+ return max_freq;
+}
+
include/linux/cpufreq.h static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, · line 1125
Quoted 4 lines
include/linux/cpufreq.h +0
-0static inline int cpufreq_table_count_valid_entries (const struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos;
Besides, could you comment patch v2 Dietmar's comments? It seems that patch 1/2 exposes a pre-existing ordering issue. On DT systems, cpu_scale is normalized only from the CPUFREQ_CREATE_POLICY notification of the last policy, so the cpufreq pressure of every other policy is computed in cpufreq_init_policy() from the raw, not yet normalized, capacity, and stays cached until the next cpufreq_set_policy() for that policy. So far this was harmless because capacity_freq_ref == policy->max at boot, so the pressure was 0 regardless of the capacity. With 1/2, capacity_freq_ref covers the boost frequencies while boost is disabled at boot, so policy->max < capacity_freq_ref becomes the common case and the stale value becomes visible (102 vs 79 on Juno R0, as reported by Dietmar). Perhaps the limits of all policies could be refreshed once from parsing_done_workfn(), after the capacities have been normalized?
drivers/base/arch_topology.c +18 -0
@@ -454,9 +454,27 @@ core_initcall(register_cpufreq_notifier);static void parsing_done_workfn (struct work_struct *work) { +int cpu; +cpufreq_unregister_notifier (&init_cpu_capacity_notifier,CPUFREQ_POLICY_NOTIFIER );free_cpumask_var (cpus_to_visit); + +/* +* The cpufreq pressure of all policies but the last one has been +* computed before topology_normalize_cpu_scale(), so refresh it now +* that cpu_scale is final. +*/ +for_each_possible_cpu (cpu) { + struct cpufreq_policy *policy __free(put_cpufreq_policy) = +cpufreq_cpu_get (cpu); + +if (!policy || cpu != policy->cpu) +continue ; + +guard (cpufreq_policy_write)(policy); +refresh_frequency_limits (policy); + }
Signature · 2 lines
Thx and BRs, Zhongqiu Han
Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency
Quoted 1 line · Zhongqiu Han
On 9/29/2026 7:38 PM, Ananthu C V wrote:
[...]
Quoted 32 lines
Besides, could you comment patch v2 Dietmar's comments? It seems that patch 1/2 exposes a pre-existing ordering issue. On DT systems,
cpu_scaleis normalized only from theCPUFREQ_CREATE_POLICYnotification of the last policy, so the cpufreq pressure of every other policy is computed incpufreq_init_policy()from the raw, not yet normalized, capacity, and stays cached until the nextcpufreq_set_policy()for that policy. So far this was harmless becausecapacity_freq_ref== policy-Quoted 1 line
max at boot, so the pressure was 0 regardless of the capacity. With
1/2,
capacity_freq_refcovers the boost frequencies while boost is disabled at boot, sopolicy->max<capacity_freq_refbecomes the common case and the stale value becomes visible (102 vs 79 on Juno R0, as reported by Dietmar). Perhaps the limits of all policies could be refreshed once fromparsing_done_workfn(), after the capacities have been normalized?drivers/base/arch_topology.c +8
-0@@ -454,9 +454,27 @@ core_initcall(register_cpufreq_notifier); static void parsing_done_workfn(struct work_struct *work) { + int cpu; + cpufreq_unregister_notifier(&init_cpu_capacity_notifier, CPUFREQ_POLICY_NOTIFIER); free_cpumask_var(cpus_to_visit); + + /* +* The cpufreq pressure of all policies but the last one has been +* computed before topology_normalize_cpu_scale(), so refresh it now +* that cpu_scale is final. +*/
drivers/base/arch_topology.c core_initcall(register_cpufreq_notifier); · line 477
Quoted 10 lines
drivers/base/arch_topology.c +10
-0+ for_each_possible_cpu(cpu) { + struct cpufreq_policy *policy __free(put_cpufreq_policy) = + cpufreq_cpu_get(cpu); + + if (!policy || cpu != policy->cpu) + continue; + + guard(cpufreq_policy_write)(policy); + refresh_frequency_limits(policy); + }
Yes, IMHO, something like this is needed.
[ 1.169158] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[] (1) [ 1.169209] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5] [ 1.169992] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000 [ 1.170203] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000 (2) [ 1.170835] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[] (1) [ 1.170872] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[1-2] [ 1.174516] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 [ 1.175261] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 (2) [ 1.187698] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[0,3-5] policy->max=700000 pressure=79 max_capacity=446 capped_freq=700000 max_freq=850000 (3) [ 1.187847] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 (3)
The work scheduled in (1) also still operates on raw capacity (2).
(3) shows your extension in parsing_done_workfn() work function which gets scheduled when all CPUs are initialised, so topology_normalize_cpu_scale() had been called.
Not sure whether we can call cpufreq_update_limits(cpu) there instead (maybe extended with a 'only do this for policy->cpu'?). So we won't have to expose cpufreq internals in /drivers/base/arch_topology.c.
Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency
Quoted 62 lines · Dietmar Eggemann
Quoted 1 line · Zhongqiu Han
On 9/29/2026 7:38 PM, Ananthu C V wrote:
[...]
Quoted 32 lines
Besides, could you comment patch v2 Dietmar's comments? It seems that patch 1/2 exposes a pre-existing ordering issue. On DT systems,
cpu_scaleis normalized only from theCPUFREQ_CREATE_POLICYnotification of the last policy, so the cpufreq pressure of every other policy is computed incpufreq_init_policy()from the raw, not yet normalized, capacity, and stays cached until the nextcpufreq_set_policy()for that policy. So far this was harmless becausecapacity_freq_ref== policy-Quoted 1 line
max at boot, so the pressure was 0 regardless of the capacity. With
1/2,
capacity_freq_refcovers the boost frequencies while boost is disabled at boot, sopolicy->max<capacity_freq_refbecomes the common case and the stale value becomes visible (102 vs 79 on Juno R0, as reported by Dietmar). Perhaps the limits of all policies could be refreshed once fromparsing_done_workfn(), after the capacities have been normalized?drivers/base/arch_topology.c +8
-0@@ -454,9 +454,27 @@ core_initcall(register_cpufreq_notifier); static void parsing_done_workfn(struct work_struct *work) { + int cpu; + cpufreq_unregister_notifier(&init_cpu_capacity_notifier, CPUFREQ_POLICY_NOTIFIER); free_cpumask_var(cpus_to_visit); + + /* +* The cpufreq pressure of all policies but the last one has been +* computed before topology_normalize_cpu_scale(), so refresh it now +* that cpu_scale is final. +*/ drivers/base/arch_topology.c core_initcall(register_cpufreq_notifier); · line 477
Quoted 10 lines
drivers/base/arch_topology.c +10
-0+ for_each_possible_cpu(cpu) { + struct cpufreq_policy *policy __free(put_cpufreq_policy) = + cpufreq_cpu_get(cpu); + + if (!policy || cpu != policy->cpu) + continue; + + guard(cpufreq_policy_write)(policy); + refresh_frequency_limits(policy); + }Yes, IMHO, something like this is needed.
[ 1.169158] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[] (1) [ 1.169209] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5] [ 1.169992] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000 [ 1.170203] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000 (2) [ 1.170835] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[] (1) [ 1.170872] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[1-2] [ 1.174516] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 [ 1.175261] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 (2) [ 1.187698] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[0,3-5] policy->max=700000 pressure=79 max_capacity=446 capped_freq=700000 max_freq=850000 (3) [ 1.187847] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000 (3)The work scheduled in (1) also still operates on raw capacity (2).
Quoted 7 lines
(3) shows your extension in
parsing_done_workfn()work function which gets scheduled when all CPUs are initialised, sotopology_normalize_cpu_scale()had been called.Not sure whether we can call
cpufreq_update_limits(cpu)there instead (maybe extended with a 'only do this forpolicy->cpu'?). So we won't have to expose cpufreq internals in /drivers/base/arch_topology.c.
Thanks for the test verification and the discussion.
Unless I'm missing something, it seems to me that cpufreq_update_limits() is mainly intended for platform limit changes. When a driver implements ->update_limits(), it is up to the driver whether the policy gets refreshed at all (it seems that amd-pstate only updates the preferred core ranking). Otherwise, it goes through cpufreq_policy_refresh(), which calls ->get() first and WARNs and skips the refresh if that returns 0, for example when the policy goes inactive under us.
In comparison, refresh_frequency_limits() seems to match what we need here more closely: it simply re-applies the limits and therefore recomputes the pressure, without going through any of the above.
However, calling refresh_frequency_limits() directly from drivers/base arch_topology.c does expose some cpufreq internals there. I wonder if it would make sense to add a wrapper around cpufreq_update_pressure() directly, something like:
drivers/base/arch_topology.c +34 -2
@@ -454,9 +454,19 @@ core_initcall(register_cpufreq_notifier);static void parsing_done_workfn (struct work_struct *work) { +int cpu; +cpufreq_unregister_notifier (&init_cpu_capacity_notifier,CPUFREQ_POLICY_NOTIFIER );free_cpumask_var (cpus_to_visit); + +/* +* The cpufreq pressure of all policies but the last one has been +* computed before topology_normalize_cpu_scale(), so update it now +* that cpu_scale is final. +*/ +for_each_possible_cpu (cpu) +cpufreq_refresh_pressure (cpu); } --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c
@@ -2611,6 +2611,27 @@ static void cpufreq_update_pressure(struct cpufreq_policy *policy)WRITE_ONCE (per_cpu (cpufreq_pressure, cpu), pressure); } +/** +* cpufreq_refresh_pressure() - Recompute cpufreq pressure for a CPU's policy. +* @cpu: CPU whose policy's cpufreq pressure is to be recomputed. +* +* To be called when arch_scale_cpu_capacity() of @cpu has changed while +* the limits of its policy have not. +* +* Takes the policy rwsem, so the caller must not hold it. +*/ +void cpufreq_refresh_pressure (unsigned int cpu) +{ + struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpu); +if (!policy) +return ; + +guard (cpufreq_policy_write)(policy); + +if (!policy_is_inactive (policy)) +cpufreq_update_pressure (policy); +} + --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h
@@ -251,6 +251,7 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, int io_busy);void refresh_frequency_limits (struct cpufreq_policy *policy);void cpufreq_update_policy (unsigned int cpu);void cpufreq_update_limits (unsigned int cpu); +void cpufreq_refresh_pressure (unsigned int cpu);bool have_governor_per_policy (void );bool cpufreq_supports_freq_invariance (void );struct kobject *get_governor_parent_kobj (struct cpufreq_policy *policy);
Signature · 2 lines
Thx and BRs, Zhongqiu Han
Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency
Hi Zhongiu and Dietmar,
Quoted 50 lines · Zhongqiu Han
Quoted 40 lines · Dietmar Eggemann
Quoted 1 line · Zhongqiu Han
On 9/29/2026 7:38 PM, Ananthu C V wrote:
[...]
Quoted 35 lines
Besides, could you comment patch v2 Dietmar's comments? It seems that patch 1/2 exposes a pre-existing ordering issue. On DT systems,
cpu_scaleis normalized only from theCPUFREQ_CREATE_POLICYnotification of the last policy, so the cpufreq pressure of every other policy is computed incpufreq_init_policy()from the raw, not yet normalized, capacity, and stays cached until the nextcpufreq_set_policy()for that policy. So far this was harmless becausecapacity_freq_ref== policy-Quoted 1 line
max at boot, so the pressure was 0 regardless of the capacity. With
1/2,
capacity_freq_refcovers the boost frequencies while boost is disabled at boot, sopolicy->max<capacity_freq_refbecomes the common case and the stale value becomes visible (102 vs 79 on Juno R0, as reported by Dietmar). Perhaps the limits of all policies could be refreshed once fromparsing_done_workfn(), after the capacities have been normalized?drivers/base/arch_topology.c +11
-0@@ -454,9 +454,27 @@ core_initcall(register_cpufreq_notifier); static void parsing_done_workfn(struct work_struct *work) { + int cpu; + cpufreq_unregister_notifier(&init_cpu_capacity_notifier, CPUFREQ_POLICY_NOTIFIER); free_cpumask_var(cpus_to_visit); + + /* + * The cpufreq pressure of all policies but the last one has been + * computed before topology_normalize_cpu_scale(), so refresh it now + * that cpu_scale is final. + */ + for_each_possible_cpu(cpu) { + struct cpufreq_policy *policy __free(put_cpufreq_policy) = + cpufreq_cpu_get(cpu);Quoted 8 lines
drivers/base/arch_topology.c core_initcall(register_cpufreq_notifier); · line 477
Quoted 7 lines
drivers/base/arch_topology.c +7
-0+ + if (!policy || cpu != policy->cpu) + continue; + + guard(cpufreq_policy_write)(policy); + refresh_frequency_limits(policy); + }Yes, IMHO, something like this is needed.
Quoted 1 line
Thanks for the test verification and the discussion.
Thanks to both of you for the discussion and suggestions. I am noticing similar pressure update related patterns on glymur as well, so the discovery is very valid, thanks for that. I will take in your suggestions and include patches to fix the pressure scenario in the next revision.
Quoted 3 lines
--
Thx and BRs,
Zhongqiu Han
Best,
Ananthu
Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency
In reply to Zhongqiu HanHi Zhongqiu,
Quoted 33 lines · Zhongqiu Han
The current implementation looks correct and works well. That said, we might be able to optimize it with a simpler implementation as shown below:
+/**
+ *cpufreq_frequency_table_max- Get the highest frequency in a policy's table
+ * @policy: the &structcpufreq_policyto inspect
+ *
+ * Boost entries (%CPUFREQ_BOOST_FREQ) are taken into account regardless of
+ * whether or not boost is currently enabled.
+ *
+ * Return: the highest valid frequency in @policy->freq_table, or 0 if there
+ * is no frequency table.
+ */(quoted diff) +15
-0+static inline unsigned int +cpufreq_frequency_table_max(const struct cpufreq_policy *policy) +{ + struct cpufreq_frequency_table *pos; + unsigned int max_freq = 0; + + if (!policy->freq_table) + return 0; + + cpufreq_for_each_valid_entry(pos, policy->freq_table) + max_freq = max(max_freq, pos->frequency); + + return max_freq; +} +
Quoted 3 lines
Quoted 3 lines
static inline int cpufreq_table_count_valid_entries(const struct cpufreq_policy *policy) { struct cpufreq_frequency_table *pos;
ack, I will try to further simply this with taking in your suggestions, thanks.
Quoted 2 lines
Thx and BRs,
Zhongqiu Han
Best,
Ananthu
PATCH v3 2/2 cpufreq: fix policy max not returning to non-boost freq on boost disable
In reply to Ananthu C VCommit 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known") introduced a guard for cpuinfo max updates to only increase, to preserve driver-set values above the freq table maximum. Later commit db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init") dropped the 'policy->max = max_freq' assignment from cpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only path for cpuinfo max and consequently policy max to be updated, causing both values to be stuck at boost frequency even when boost is disabled.
Unconditionally track the highest non-boost frequency (max_base_freq) in the freq table. If boost frequencies are available, use the highest frequency from the table/max_base_freq instead of cpuinfo->max_freq to control boost values, so the value can decrease again when boost is disabled.
--- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-)
drivers/cpufreq/cpufreq.c +15 -1
@@ -574,6 +574,8 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)static int policy_set_boost (struct cpufreq_policy *policy,bool enable) { +unsigned int max_freq; +unsigned int max_table_freq;int ret;if (policy->boost_enabled == enable)
@@ -587,7 +589,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)return ret; } - ret =freq_qos_update_request (&policy->boost_freq_req, policy->cpuinfo.max_freq); + max_table_freq =cpufreq_frequency_table_max (policy); + +if (policy_has_boost_freq (policy)) { + max_freq = enable ? max_table_freq : policy->cpuinfo.max_base_freq; + +if (!max_freq) +/* when the freq table contains only boost frequencies */ + max_freq = max_table_freq; + }else { + max_freq = policy->cpuinfo.max_freq; + } + + ret =freq_qos_update_request (&policy->boost_freq_req, max_freq);if (ret <0 ) { policy->boost_enabled = !policy->boost_enabled; cpufreq_driver->set_boost (policy, policy->boost_enabled);
drivers/cpufreq/freq_table.c +8 -1
@@ -14,7 +14,7 @@* FREQUENCY TABLE HELPERS * *********************************************************************/ -static bool policy_has_boost_freq (struct cpufreq_policy *policy) +bool policy_has_boost_freq (struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos, *table = policy->freq_table;
@@ -27,17 +27,22 @@ static bool policy_has_boost_freq(struct cpufreq_policy *policy)return false; } +EXPORT_SYMBOL_GPL (policy_has_boost_freq);int cpufreq_frequency_table_cpuinfo (struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos, *table = policy->freq_table;unsigned int min_freq = ~0 ;unsigned int max_freq =0 ; +unsigned int max_base_freq =0 ;unsigned int freq, i;cpufreq_for_each_valid_entry_idx (pos, table, i) { freq = pos->frequency; +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && freq > max_base_freq) + max_base_freq = freq; +if ((!cpufreq_boost_enabled () || !policy->boost_enabled) && (pos->flags &CPUFREQ_BOOST_FREQ ))continue ;
@@ -57,6 +62,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)if (policy->cpuinfo.max_freq < max_freq) policy->cpuinfo.max_freq = max_freq; + policy->cpuinfo.max_base_freq = max_base_freq; +if (min_freq == ~0 )return -EINVAL ;else
include/linux/cpufreq.h +3 -0
@@ -45,6 +45,7 @@ enum cpufreq_table_sorting {struct cpufreq_cpuinfo {unsigned int max_freq;unsigned int min_freq; +unsigned int max_base_freq;/* Highest non-boost frequency in the table */ /* in 10^(-9) s = nanoseconds */ unsigned int transition_latency;
@@ -791,6 +792,8 @@ struct cpufreq_frequency_table {int cpufreq_frequency_table_cpuinfo (struct cpufreq_policy *policy); +bool policy_has_boost_freq (struct cpufreq_policy *policy); +int cpufreq_frequency_table_verify (struct cpufreq_policy_data *policy);int cpufreq_generic_frequency_table_verify (struct cpufreq_policy_data *policy);
Signature · 1 line
2.43.0
Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable
Quoted 3 lines · Ananthu C V
Commit
538b0188da46("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known") introduced a guard for cpuinfo max updates to only increase, to preserve driver-set values above the freq table maximum.
I think as discussed in v2 the changelog should describe the impact of commit 6e39ba4e5a82 ("cpufreq: Add boost_freq_req QoS request"), since it is relevant to understanding this fix.
Quoted 6 lines
Later commit
db80ad776cd2("cpufreq: Remove driver default policy->min/max init") dropped the 'policy->max=max_freq' assignment fromcpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only path for cpuinfo max and consequently policy max to be updated, causing both values to be stuck at boost frequency even when boost is disabled.
Quoted 7 lines
Unconditionally track the highest non-boost frequency (
max_base_freq) in the freq table. If boost frequencies are available, use the highest frequency from the table/max_base_freq instead ofcpuinfo->max_freqto control boost values, so the value can decrease again when boost is disabled.Fixes db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init")
Patch 2/2 currently depends on a helper introduced by patch 1/2, which prevents it from being backported independently. Since 2/2 is an independent fix that should be backported to stable on its own, I think it should be placed before patch 1/2 in the series and kept self contained.
Quoted 91 lines
Signed-off-by Ananthu C V <[email protected]>--- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-)drivers/cpufreq/cpufreq.c +15
-1@@ -574,6 +574,8 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)static int policy_set_boost (struct cpufreq_policy *policy,bool enable) { +unsigned int max_freq; +unsigned int max_table_freq;int ret;if (policy->boost_enabled == enable)@@ -587,7 +589,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)return ret; } - ret =freq_qos_update_request (&policy->boost_freq_req, policy->cpuinfo.max_freq); + max_table_freq =cpufreq_frequency_table_max (policy); + +if (policy_has_boost_freq (policy)) { + max_freq = enable ? max_table_freq : policy->cpuinfo.max_base_freq; + +if (!max_freq) +/* when the freq table contains only boost frequencies */ + max_freq = max_table_freq; + }else { + max_freq = policy->cpuinfo.max_freq; + } + + ret =freq_qos_update_request (&policy->boost_freq_req, max_freq);if (ret <0 ) { policy->boost_enabled = !policy->boost_enabled; cpufreq_driver->set_boost (policy, policy->boost_enabled);drivers/cpufreq/freq_table.c +8
-1@@ -14,7 +14,7 @@* FREQUENCY TABLE HELPERS * *********************************************************************/ -static bool policy_has_boost_freq (struct cpufreq_policy *policy) +bool policy_has_boost_freq (struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos, *table = policy->freq_table;@@ -27,17 +27,22 @@ static bool policy_has_boost_freq(struct cpufreq_policy *policy)return false; } +EXPORT_SYMBOL_GPL (policy_has_boost_freq);int cpufreq_frequency_table_cpuinfo (struct cpufreq_policy *policy) {struct cpufreq_frequency_table *pos, *table = policy->freq_table;unsigned int min_freq = ~0 ;unsigned int max_freq =0 ; +unsigned int max_base_freq =0 ;unsigned int freq, i;cpufreq_for_each_valid_entry_idx (pos, table, i) { freq = pos->frequency; +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && freq > max_base_freq) + max_base_freq = freq; +if ((!cpufreq_boost_enabled () || !policy->boost_enabled) && (pos->flags &CPUFREQ_BOOST_FREQ ))continue ;@@ -57,6 +62,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)if (policy->cpuinfo.max_freq < max_freq) policy->cpuinfo.max_freq = max_freq; + policy->cpuinfo.max_base_freq = max_base_freq; +if (min_freq == ~0 )return -EINVAL ;else include/linux/cpufreq.h +0
-0
include/linux/cpufreq.h enum cpufreq_table_sorting { · line 48
Quoted 4 lines
include/linux/cpufreq.h +1
-0@@ -45,6 +45,7 @@ enum cpufreq_table_sorting {struct cpufreq_cpuinfo {unsigned int max_freq;unsigned int min_freq; +unsigned int max_base_freq;/* Highest non-boost frequency in the table */
To avoid introducing a new member that is only used by frequency-table drivers, please consider the following implementation instead:
If there are boost entries in the frequency table, the table is the only source of cpuinfo.max_freq, so let cpufreq_frequency_table_cpuinfo() set it unconditionally in that case.
drivers/cpufreq/freq_table.c +15 -9
@@ -33,14 +33,17 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)struct cpufreq_frequency_table *pos, *table = policy->freq_table;unsigned int min_freq = ~0 ;unsigned int max_freq =0 ; +bool has_boost = false;unsigned int freq, i;cpufreq_for_each_valid_entry_idx (pos, table, i) { freq = pos->frequency; -if ((!cpufreq_boost_enabled () || !policy->boost_enabled) - && (pos->flags &CPUFREQ_BOOST_FREQ )) -continue ; +if (pos->flags &CPUFREQ_BOOST_FREQ ) { + has_boost = true; +if (!cpufreq_boost_enabled () || !policy->boost_enabled) +continue ; + }pr_debug ("table entry %u: %u kHz\n" , i, freq);if (freq < min_freq)
@@ -50,17 +53,20 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy) } policy->cpuinfo.min_freq = min_freq; + +if (min_freq == ~0 ) +return -EINVAL ; +/* * If the driver has set its own cpuinfo.max_freq above max_freq, leave -* it as is. +* it as is. This does not apply to tables with boost entries, because +* the table is the only source of cpuinfo.max_freq then and it needs +* to go down again when boost is disabled. */ -if (policy->cpuinfo.max_freq < max_freq) +if (has_boost || policy->cpuinfo.max_freq < max_freq) policy->cpuinfo.max_freq = max_freq; -if (min_freq == ~0 ) -return -EINVAL ; -else -return 0 ; +return 0 ; }int cpufreq_frequency_table_verify (struct cpufreq_policy_data *policy)
Quoted 13 lines
include/linux/cpufreq.h +2
-0/* in 10^(-9) s = nanoseconds */ unsigned int transition_latency;@@ -791,6 +792,8 @@ struct cpufreq_frequency_table {int cpufreq_frequency_table_cpuinfo (struct cpufreq_policy *policy); +bool policy_has_boost_freq (struct cpufreq_policy *policy); +int cpufreq_frequency_table_verify (struct cpufreq_policy_data *policy);int cpufreq_generic_frequency_table_verify (struct cpufreq_policy_data *policy);
Signature · 2 lines
Thx and BRs, Zhongqiu Han
Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable
Hi Zhongqiu,
Quoted 5 lines · Zhongqiu Han
Patch 2/2 currently depends on a helper introduced by patch 1/2, which prevents it from being backported independently. Since 2/2 is an independent fix that should be backported to stable on its own, I think it should be placed before patch 1/2 in the series and kept self contained.
The idea was that currently patch 2/2 would serve no actual value without the fix in patch 1/2, and that is why it was the second one. But yes, we cannot fix 1/2 and leave 2/2 as is, which would turn the current benign issue to an actual problem, so the order should be probably reversed. I'll work on that.
Quoted 64 lines
To avoid introducing a new member that is only used by frequency-table drivers, please consider the following implementation instead:
If there are boost entries in the frequency table, the table is the only source of cpuinfo.max_freq, so let
cpufreq_frequency_table_cpuinfo()set it unconditionally in that case.drivers/cpufreq/freq_table.c +6
-3@@ -33,14 +33,17 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy)struct cpufreq_frequency_table *pos, *table = policy->freq_table;unsigned int min_freq = ~0 ;unsigned int max_freq =0 ; +bool has_boost = false;unsigned int freq, i;cpufreq_for_each_valid_entry_idx (pos, table, i) { freq = pos->frequency; -if ((!cpufreq_boost_enabled () || !policy->boost_enabled) - && (pos->flags &CPUFREQ_BOOST_FREQ )) -continue ; +if (pos->flags &CPUFREQ_BOOST_FREQ ) { + has_boost = true; +if (!cpufreq_boost_enabled () || !policy->boost_enabled) +continue ; + }pr_debug ("table entry %u: %u kHz\n" , i, freq);drivers/cpufreq/freq_table.c +9
-6if (freq < min_freq)@@ -50,17 +53,20 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy) } policy->cpuinfo.min_freq = min_freq; + +if (min_freq == ~0 ) +return -EINVAL ; +/* * If the driver has set its own cpuinfo.max_freq above max_freq, leave -* it as is. +* it as is. This does not apply to tables with boost entries, because +* the table is the only source of cpuinfo.max_freq then and it needs +* to go down again when boost is disabled. */ -if (policy->cpuinfo.max_freq < max_freq) +if (has_boost || policy->cpuinfo.max_freq < max_freq) policy->cpuinfo.max_freq = max_freq; -if (min_freq == ~0 ) -return -EINVAL ; -else -return 0 ; +return 0 ; }int
cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy)
Quoted 8 lines
Review struct cpufreq_frequency_table { · line 796
Quoted 8 lines
/* in 10^(-9) s = nanoseconds */
unsigned inttransition_latency;(quoted diff) +2
-0@@ -791,6 +792,8 @@ struct cpufreq_frequency_table { int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy); +bool policy_has_boost_freq(struct cpufreq_policy *policy); + int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy); int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
I wanted more discussion on this part regarding finer implementation details and hence went with a working version, and I think I asked in the last revision thread regarding what are people's thoughts on this added field. I am also not keen on adding more fields, so this is useful suggestion. If we are to continue this implementation route, I'll keep this in mind, thanks.
Quoted 2 lines
Thx and BRs,
Zhongqiu Han
Best,
Ananthu
Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable
In reply to Ananthu C VQuoted 44 lines · Ananthu C V
Commit
538b0188da46("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known") introduced a guard for cpuinfo max updates to only increase, to preserve driver-set values above the freq table maximum. Later commitdb80ad776cd2("cpufreq: Remove driver default policy->min/max init") dropped the 'policy->max=max_freq' assignment fromcpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only path for cpuinfo max and consequently policy max to be updated, causing both values to be stuck at boost frequency even when boost is disabled.Unconditionally track the highest non-boost frequency (
max_base_freq) in the freq table. If boost frequencies are available, use the highest frequency from the table/max_base_freq instead ofcpuinfo->max_freqto control boost values, so the value can decrease again when boost is disabled.Fixes db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init")Signed-off-by Ananthu C V <[email protected]>--- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-)drivers/cpufreq/cpufreq.c +5
-1@@ -574,6 +574,8 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)static int policy_set_boost (struct cpufreq_policy *policy,bool enable) { +unsigned int max_freq; +unsigned int max_table_freq;int ret;if (policy->boost_enabled == enable)@@ -587,7 +589,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)return ret; } - ret =freq_qos_update_request (&policy->boost_freq_req, policy->cpuinfo.max_freq); + max_table_freq =cpufreq_frequency_table_max (policy); + +if (policy_has_boost_freq (policy)) {
drivers/cpufreq/cpufreq.c static int policy_set_boost(struct cpufreq_policy *policy, bool enable) · line 604
Quoted 10 lines
drivers/cpufreq/cpufreq.c +10
-0+ max_freq = enable ? max_table_freq : policy->cpuinfo.max_base_freq; + +if (!max_freq) +/* when the freq table contains only boost frequencies */ + max_freq = max_table_freq; + }else { + max_freq = policy->cpuinfo.max_freq; + } + + ret =freq_qos_update_request (&policy->boost_freq_req, max_freq);
This looks pretty convoluted. You have to find the correct freq argument for freq_qos_update_request() for freq_table drivers but work around this for e.g. the CPPC driver.
cpufreq_driver->set_boost() of e.g. the freq_table scmi-cpufreq driver does not adjust policy->cpuinfo.max_freq for 'boost state = 0':
cpufreq_boost_set_sw() {
cpufreq_frequency_table_cpuinfo()
policy->cpuinfo.min_freq = min_freq
if (policy->cpuinfo.max_freq < max_freq) <-- (*)
policy->cpuinfo.max_freq = max_freq
}
whereas the one of the cppc_cpufreq driver does:
cppc_cpufreq_set_boost() {
if (state)
policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->highest_perf)
else
policy->cpuinfo.max_freq = cppc_perf_to_khz(caps, caps->nominal_perf)
}
(*) This was added by 538b0188da46 ("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known"). With this the
policy_set_boost(policy, enable) {
cpufreq_driver->set_boost(policy, enable)
freq_qos_update_request(..., policy->cpuinfo.max_freq)
}
[...]
Re: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable
Hi Dietmar,
Quoted 51 lines · Dietmar Eggemann
Quoted 50 lines · Ananthu C V
Commit
538b0188da46("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known") introduced a guard for cpuinfo max updates to only increase, to preserve driver-set values above the freq table maximum. Later commitdb80ad776cd2("cpufreq: Remove driver default policy->min/max init") dropped the 'policy->max=max_freq' assignment fromcpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only path for cpuinfo max and consequently policy max to be updated, causing both values to be stuck at boost frequency even when boost is disabled.Unconditionally track the highest non-boost frequency (
max_base_freq) in the freq table. If boost frequencies are available, use the highest frequency from the table/max_base_freq instead ofcpuinfo->max_freqto control boost values, so the value can decrease again when boost is disabled.Fixes db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init")Signed-off-by Ananthu C V <[email protected]>--- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-)drivers/cpufreq/cpufreq.c +11
-1@@ -574,6 +574,8 @@ static ssize_t show_local_boost(struct cpufreq_policy *policy, char *buf)static int policy_set_boost (struct cpufreq_policy *policy,bool enable) { +unsigned int max_freq; +unsigned int max_table_freq;int ret;if (policy->boost_enabled == enable)@@ -587,7 +589,19 @@ static int policy_set_boost(struct cpufreq_policy *policy, bool enable)return ret; } - ret =freq_qos_update_request (&policy->boost_freq_req, policy->cpuinfo.max_freq); + max_table_freq =cpufreq_frequency_table_max (policy); + +if (policy_has_boost_freq (policy)) { + max_freq = enable ? max_table_freq : policy->cpuinfo.max_base_freq; + +if (!max_freq) +/* when the freq table contains only boost frequencies */ + max_freq = max_table_freq; + }else {
Quoted 8 lines
drivers/cpufreq/cpufreq.c static int policy_set_boost(struct cpufreq_policy *policy, bool enable) · line 604
Quoted 4 lines
drivers/cpufreq/cpufreq.c +4
-0+ max_freq = policy->cpuinfo.max_freq; + } + + ret =freq_qos_update_request (&policy->boost_freq_req, max_freq);This looks pretty convoluted. You have to find the correct freq argument for
freq_qos_update_request()forfreq_tabledrivers but work around this for e.g. the CPPC driver.
We do need quite a bit of wrangling to account for every case, which is why I didn't like the idea of having to deal with all of this in core to begin with. My initial idea (revision 1) was to simply drop the one way guard from core and have the drivers deal with it in some way, but we wanted to keep it, I suppose. I am open for suggestions, but my guess is that if we have to keep implementing these fixes in core itself, it will be convoluted in one way or other.
Best,
Ananthu
Viresh Kumar
maintainerRe: [PATCH v3 2/2] cpufreq: fix policy max not returning to non-boost freq on boost disable
In reply to Ananthu C VHi,
Quoted 9 lines · Ananthu C V
Commit
538b0188da46("cpufreq: ACPI: Set cpuinfo.max_freq directly if max boost is known") introduced a guard for cpuinfo max updates to only increase, to preserve driver-set values above the freq table maximum. Later commitdb80ad776cd2("cpufreq: Remove driver default policy->min/max init") dropped the 'policy->max=max_freq' assignment fromcpufreq_frequency_table_cpuinfo, leaving that one-way-guard to be the only path for cpuinfo max and consequently policy max to be updated, causing both values to be stuck at boost frequency even when boost is disabled.
Quoted 5 lines
Unconditionally track the highest non-boost frequency (
max_base_freq) in the freq table. If boost frequencies are available, use the highest frequency from the table/max_base_freq instead ofcpuinfo->max_freqto control boost values, so the value can decrease again when boost is disabled.
If this is a fix, it should be submitted separately so we can get this merged independently.
Quoted 1 line
Fixes db80ad776cd2 ("cpufreq: Remove driver default policy->min/max init")
I am not able to understand the current problem though. I see from the sequence of events mentioned in your cover-letter that scaling_max_freq doesn't return back, but why ?
And does a simple revert of the above commit (just freq_table.c) fixes it ?
Quoted 6 lines
Signed-off-by Ananthu C V <[email protected]>--- drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- drivers/cpufreq/freq_table.c | 9 ++++++++- include/linux/cpufreq.h | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-)
Signature · 1 line
viresh
Tested on a Lenovo Yoga Slim 7x Gen 11 (Glymur): boost off 4032000, boost on 4723200, back to 4032000 when boost is disabled again.