PM: EM: Exclude boost frequencies from inefficiency ranking
Subsystems (5)
PATCH v2 1/1 PM: EM: Exclude boost frequencies from inefficiency ranking
em_compute_costs() ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.
3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.
Exclude the boost frequencies from the ranking in em_compute_costs() and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helper cpufreq_table_max_nonboost_freq(), as the boundary.
---
Changes in v2:
- Drop prev_boost and keep all boost states efficient by resetting prev_cost for them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.
Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost-separately-v1-1-238a6b2bb888@oss.qualcomm.com/
--- include/linux/cpufreq.h | 24 ++++++++++++++++++++++++ kernel/power/energy_model.c | 16 ++++++++++++++++ 2 files changed, 40 insertions(+)
include/linux/cpufreq.h +24 -0
@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +struct cpufreq_frequency_table *pos; +unsigned int max =0 ; + +if (!policy->freq_table) +return 0 ; + +/* The table may be unsorted, scan all of it for the highest one. */ +cpufreq_for_each_valid_entry (pos, policy->freq_table) { +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && pos->frequency > max) + max = pos->frequency; + } + +return max; +} +static inline int parse_perf_domain (int cpu,const char *list_name,const char *cell_name,struct of_phandle_args *args)
@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +return 0 ; +} +static inline int of_perf_domain_get_sharing_cpumask (int pcpu,const char *list_name,const char *cell_name,struct cpumask *cpumask,struct of_phandle_args *pargs)
kernel/power/energy_model.c +16 -0
@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) {unsigned long prev_cost =ULONG_MAX ; +struct cpufreq_policy *policy; +unsigned int max_nonboost =0 ;int i, ret;/* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device (dev))return 0 ; + policy =cpufreq_cpu_get (dev->id); +if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); +cpufreq_cpu_put (policy); + } +/* Compute the cost of each performance state. */ for (i = nr_states -1 ; i >=0 ; i--) {unsigned long power_res, cost; +bool boost = max_nonboost && table[i].frequency > max_nonboost;if ((flags &EM_PERF_DOMAIN_ARTIFICIAL ) && cb->get_cost) { ret = cb->get_cost (dev, table[i].frequency, &cost);
@@ -284,6 +293,13 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, table[i].flags =EM_PERF_STATE_INEFFICIENT ;dev_dbg (dev,"EM: OPP:%lu is inefficient\n" , table[i].frequency); + }else if (boost) { +/* +* Keep all boost states efficient. Resetting prev_cost +* also keeps the highest non-boost state efficient, +* then rank the rest on their own. +*/ + prev_cost =ULONG_MAX ; }else { prev_cost = table[i].cost; }
---
base-commit: f1ac44c32cba88ad65dcecd4a279661d14e93651
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
change-id: 20261029-em-rank-boost-separately-a7e7c7d78058
Best regards,
Signature · 1 line
Guo Chen <[email protected]>
Quoted 123 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost-separately-v1-1-238a6b2bb888@oss.qualcomm.com/
--- include/linux/cpufreq.h | 24 ++++++++++++++++++++++++ kernel/power/energy_model.c | 16 ++++++++++++++++ 2 files changed, 40 insertions(+)include/linux/cpufreq.h +24
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +struct cpufreq_frequency_table *pos; +unsigned int max =0 ; + +if (!policy->freq_table) +return 0 ; + +/* The table may be unsorted, scan all of it for the highest one. */ +cpufreq_for_each_valid_entry (pos, policy->freq_table) { +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && pos->frequency > max) + max = pos->frequency; + } + +return max; +} +static inline int parse_perf_domain (int cpu,const char *list_name,const char *cell_name,struct of_phandle_args *args)@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +return 0 ; +} +static inline int of_perf_domain_get_sharing_cpumask (int pcpu,const char *list_name,const char *cell_name,struct cpumask *cpumask,struct of_phandle_args *pargs)kernel/power/energy_model.c +16
-1@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) {unsigned long prev_cost =ULONG_MAX ; +struct cpufreq_policy *policy; +unsigned int max_nonboost =0 ;int i, ret;/* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device (dev))return 0 ; + policy =cpufreq_cpu_get (dev->id); +if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); +cpufreq_cpu_put (policy); + } +/* Compute the cost of each performance state. */ for (i = nr_states -1 ; i >=0 ; i--) {unsigned long power_res, cost; +bool boost = max_nonboost && table[i].frequency > max_nonboost;if ((flags &EM_PERF_DOMAIN_ARTIFICIAL ) && cb->get_cost) { ret = cb->get_cost (dev, table[i].frequency, &cost);@@ -284,6 +293,13 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, table[i].flags =EM_PERF_STATE_INEFFICIENT ;dev_dbg (dev,"EM: OPP:%lu is inefficient\n" , table[i].frequency); + }else if (boost) { +/* +* Keep all boost states efficient. Resetting prev_cost +* also keeps the highest non-boost state efficient, +* then rank the rest on their own. +*/ + prev_cost =ULONG_MAX ; }else { prev_cost = table[i].cost; } ---base-commit:
f1ac44c32cba88ad65dcecd4a279661d14e93651
base-tree:git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.gitmaster
change-id: 20261029-em-rank-boost-separately-a7e7c7d78058
Quoted 3 lines
Best regards,
--
Guo Chen <[email protected]>
LGTM,
Rafael J. Wysocki (Intel)
maintainerQuoted 12 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.
Quoted 7 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>
Please see Sashiko's feedback regarding this patch
and tell me what you think about it.
Thanks!
Quoted 108 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost-separately-v1-1-238a6b2bb888@oss.qualcomm.com/
--- include/linux/cpufreq.h | 24 ++++++++++++++++++++++++ kernel/power/energy_model.c | 16 ++++++++++++++++ 2 files changed, 40 insertions(+)include/linux/cpufreq.h +24
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +struct cpufreq_frequency_table *pos; +unsigned int max =0 ; + +if (!policy->freq_table) +return 0 ; + +/* The table may be unsorted, scan all of it for the highest one. */ +cpufreq_for_each_valid_entry (pos, policy->freq_table) { +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && pos->frequency > max) + max = pos->frequency; + } + +return max; +} +static inline int parse_perf_domain (int cpu,const char *list_name,const char *cell_name,struct of_phandle_args *args)@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +return 0 ; +} +static inline int of_perf_domain_get_sharing_cpumask (int pcpu,const char *list_name,const char *cell_name,struct cpumask *cpumask,struct of_phandle_args *pargs)kernel/power/energy_model.c +16
-1@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) {unsigned long prev_cost =ULONG_MAX ; +struct cpufreq_policy *policy; +unsigned int max_nonboost =0 ;int i, ret;/* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device (dev))return 0 ; + policy =cpufreq_cpu_get (dev->id); +if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); +cpufreq_cpu_put (policy); + } +/* Compute the cost of each performance state. */ for (i = nr_states -1 ; i >=0 ; i--) {unsigned long power_res, cost; +bool boost = max_nonboost && table[i].frequency > max_nonboost;if ((flags &EM_PERF_DOMAIN_ARTIFICIAL ) && cb->get_cost) { ret = cb->get_cost (dev, table[i].frequency, &cost);@@ -284,6 +293,13 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, table[i].flags =EM_PERF_STATE_INEFFICIENT ;dev_dbg (dev,"EM: OPP:%lu is inefficient\n" , table[i].frequency); + }else if (boost) { +/* +* Keep all boost states efficient. Resetting prev_cost +* also keeps the highest non-boost state efficient, +* then rank the rest on their own. +*/ + prev_cost =ULONG_MAX ; }else { prev_cost = table[i].cost; } ---base-commit:
f1ac44c32cba88ad65dcecd4a279661d14e93651
base-tree:git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.gitmaster
change-id: 20261029-em-rank-boost-separately-a7e7c7d78058Best regards,
--
Guo Chen <[email protected]>
Quoted 19 lines · Rafael J. Wysocki (Intel)
Quoted 11 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Quoted 7 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>
Quoted 7 lines
Please see Sashiko's feedback regarding this patch
and tell me what you think about it.
Thanks!
Thanks for the review.
I did consider this case. If no policy is found, max_nonboost stays 0 and the new logic is bypassed, i.e. the ranking falls back to the old behaviour, which looked like the least intrusive option. I also thought about warning and returning an error, but most of the in-tree callers of em_recalc_and_update() ignore the return value (e.g. em_adjust_new_capacity() and exynos_asv_update_opps()), so the update would just be dropped without anyone noticing.
Quoted 72 lines
Quoted 72 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost-separately-v1-1-238a6b2bb888@oss.qualcomm.com/
--- include/linux/cpufreq.h | 24 ++++++++++++++++++++++++ kernel/power/energy_model.c | 16 ++++++++++++++++ 2 files changed, 40 insertions(+)include/linux/cpufreq.h +24
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +struct cpufreq_frequency_table *pos; +unsigned int max =0 ; + +if (!policy->freq_table) +return 0 ; + +/* The table may be unsorted, scan all of it for the highest one. */ +cpufreq_for_each_valid_entry (pos, policy->freq_table) { +if (!(pos->flags &CPUFREQ_BOOST_FREQ ) && pos->frequency > max) + max = pos->frequency; + } + +return max; +} +static inline int parse_perf_domain (int cpu,const char *list_name,const char *cell_name,struct of_phandle_args *args)@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy,return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +return 0 ; +} +static inline int of_perf_domain_get_sharing_cpumask (int pcpu,const char *list_name,const char *cell_name,struct cpumask *cpumask,struct of_phandle_args *pargs)kernel/power/energy_model.c +2
-0@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) {unsigned long prev_cost =ULONG_MAX ; +struct cpufreq_policy *policy; +unsigned int max_nonboost =0 ;int i, ret;/* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device (dev))return 0 ;
Quoted 6 lines
kernel/power/energy_model.c static int em_compute_costs(struct device *dev, struct em_perf_state *table, · line 271
Quoted 6 lines
kernel/power/energy_model.c +6
-0+ policy =cpufreq_cpu_get (dev->id); +if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); +cpufreq_cpu_put (policy); + } +
The lookup can be made more robust though: on the update paths dev->id may be offline while other CPUs of the performance domain are online. In v3, if dev->id has no policy and the performance domain already exists, I will use an active CPU of the performance domain instead, as em_cpufreq_update_efficiencies() does, and only warn and fall back to the old behaviour if that fails as well.
I did not use the performance domain for every lookup because at registration time dev->em_pd is not assigned yet (it is set after em_create_perf_table() has returned):
policy = cpufreq_cpu_get(dev->id);
if (!policy && dev->em_pd) {
cpu = cpumask_first_and(em_span_cpus(dev->em_pd), cpu_active_mask); if (cpu < nr_cpu_ids)
policy = cpufreq_cpu_get(cpu);
}
if (policy) {
max_nonboost = cpufreq_table_max_nonboost_freq(policy); cpufreq_cpu_put(policy);
} else {
dev_warn(dev, "EM: no CPUFreq policy, boost states are " "not excluded from the ranking\n"); }
This makes it much less likely to miss the policy, but cannot rule it out:
the CPU can still go offline between picking it and calling cpufreq_cpu_get(). em_cpufreq_update_efficiencies() has the same limitation.
Would that work for you?
Thanks,
Guo
Quoted 30 lines
Quoted 30 lines
kernel/power/energy_model.c +8
-1/* Compute the cost of each performance state. */ for (i = nr_states -1 ; i >=0 ; i--) {unsigned long power_res, cost; +bool boost = max_nonboost && table[i].frequency > max_nonboost;if ((flags &EM_PERF_DOMAIN_ARTIFICIAL ) && cb->get_cost) { ret = cb->get_cost (dev, table[i].frequency, &cost);@@ -284,6 +293,13 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, table[i].flags =EM_PERF_STATE_INEFFICIENT ;dev_dbg (dev,"EM: OPP:%lu is inefficient\n" , table[i].frequency); + }else if (boost) { +/* +* Keep all boost states efficient. Resetting prev_cost +* also keeps the highest non-boost state efficient, +* then rank the rest on their own. +*/ + prev_cost =ULONG_MAX ; }else { prev_cost = table[i].cost; } ---base-commit:
f1ac44c32cba88ad65dcecd4a279661d14e93651
base-tree:git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.gitmaster
change-id: 20261029-em-rank-boost-separately-a7e7c7d78058Best regards,
--
Guo Chen <[email protected]>
Lukasz Luba
maintainerQuoted 149 lines · Guo Chen
Quoted 19 lines · Rafael J. Wysocki (Intel)
Quoted 11 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Quoted 7 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>Quoted 7 lines
Please see Sashiko's feedback regarding this patch
https://sashiko.dev/#/patchset/20261103-em-rank-boost-separately- v2-1-5a13d1b38d37%40oss.qualcomm.com
and tell me what you think about it.
Thanks!
Thanks for the review.
I did consider this case. If no policy is found,
max_nonbooststays 0 and the new logic is bypassed, i.e. the ranking falls back to the old behaviour, which looked like the least intrusive option. I also thought about warning and returning an error, but most of the in-tree callers ofem_recalc_and_update()ignore the return value (e.g.em_adjust_new_capacity()andexynos_asv_update_opps()), so the update would just be dropped without anyone noticing.Quoted 77 lines
Quoted 77 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost- [email protected]/
---
include/linux/cpufreq.h | 24 ++++++++++++++++++++++++
kernel/power/energy_model.c| 16 ++++++++++++++++2 files changed, 40 insertions(+)include/linux/cpufreq.h +10
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + struct cpufreq_frequency_table *pos; + unsigned int max = 0; + + if (!policy->freq_table) + return 0; + + /* The table may be unsorted, scan all of it for the highestone. */
+cpufreq_for_each_valid_entry(pos, policy->freq_table){ + if (!(pos->flags &CPUFREQ_BOOST_FREQ) && pos-Quoted 1 line
frequency > max)
+ max =
pos->frequency;
+ }include/linux/cpufreq.h +4
-0+ + return max; +} +static inline int
parse_perf_domain(int cpu, const char *list_name, const char *cell_name, struct of_phandle_args *args)include/linux/cpufreq.h +6
-0@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + return 0; +} + static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_name, const char *cell_name, struct cpumask *cpumask,struct
of_phandle_args*pargs)kernel/power/energy_model.c +2
-0@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev,struct em_perf_state *table, unsigned long flags) { unsigned long prev_cost = ULONG_MAX; + struct cpufreq_policy *policy; + unsigned int max_nonboost = 0; int i, ret; /* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device(dev)) return 0;Quoted 6 lines
kernel/power/energy_model.c static int em_compute_costs(struct device *dev, struct em_perf_state *table, · line 271
Quoted 6 lines
kernel/power/energy_model.c +6
-0+ policy = cpufreq_cpu_get(dev->id); + if (policy) { + max_nonboost = cpufreq_table_max_nonboost_freq(policy); + cpufreq_cpu_put(policy); + } +The lookup can be made more robust though: on the update paths
dev->idmay be offline while other CPUs of the performance domain are online. In v3, ifdev->idhas no policy and the performance domain already exists, I will use an active CPU of the performance domain instead, asem_cpufreq_update_efficiencies()does, and only warn and fall back to the old behaviour if that fails as well.I did not use the performance domain for every lookup because at registration time
dev->em_pdis not assigned yet (it is set afterem_create_perf_table()has returned):policy =
cpufreq_cpu_get(dev->id);
if (!policy &&dev->em_pd) {
cpu =cpumask_first_and(em_span_cpus(dev->em_pd), cpu_active_mask); if (cpu <nr_cpu_ids)
policy =cpufreq_cpu_get(cpu);
}if (policy) {
max_nonboost=cpufreq_table_max_nonboost_freq(policy);cpufreq_cpu_put(policy);
} else {
dev_warn(dev, "EM: no CPUFreq policy, boost states are " "not excluded from the ranking\n");
}
Quoted 6 lines
This makes it much less likely to miss the policy, but cannot rule it out:
the CPU can still go offline between picking it and callingcpufreq_cpu_get().em_cpufreq_update_efficiencies()has the same limitation.Would that work for you?
I also thought about that possible racy corner case, where CPU can go offline (or already be offline). Although, in current usage this should be OK, since we do this in boot phase and platforms are sane.
We can can add that step with 'cpu_active_mask' to play safe, but no hard promises it would be atomic.
The warning also would help, for bring-up phase, so engineers would notice it and address the racy hot-plug booting system when it's there.
Quoted 164 lines · Lukasz Luba
Quoted 150 lines · Guo Chen
Quoted 19 lines · Rafael J. Wysocki (Intel)
Quoted 11 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Quoted 7 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>Quoted 7 lines
Please see Sashiko's feedback regarding this patch
https://sashiko.dev/#/patchset/20261103-em-rank-boost-separately- v2-1-5a13d1b38d37%40oss.qualcomm.com
and tell me what you think about it.
Thanks!
Thanks for the review.
I did consider this case. If no policy is found,
max_nonbooststays 0 and the new logic is bypassed, i.e. the ranking falls back to the old behaviour, which looked like the least intrusive option. I also thought about warning and returning an error, but most of the in-tree callers ofem_recalc_and_update()ignore the return value (e.g.em_adjust_new_capacity()andexynos_asv_update_opps()), so the update would just be dropped without anyone noticing.Quoted 77 lines
Quoted 77 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost- [email protected]/
---
include/linux/cpufreq.h | 24 ++++++++++++++++++++++++
kernel/power/energy_model.c| 16 ++++++++++++++++2 files changed, 40 insertions(+)include/linux/cpufreq.h +10
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + struct cpufreq_frequency_table *pos; + unsigned int max = 0; + + if (!policy->freq_table) + return 0; + + /* The table may be unsorted, scan all of it for thehighest one. */
+cpufreq_for_each_valid_entry(pos, policy->freq_table){ + if (!(pos->flags &CPUFREQ_BOOST_FREQ) && pos-Quoted 1 line
frequency > max)
+ max =
pos->frequency;
+ }include/linux/cpufreq.h +4
-0+ + return max; +} +static inline int
parse_perf_domain(int cpu, const char *list_name, const char *cell_name, struct of_phandle_args *args)include/linux/cpufreq.h +6
-0@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + return 0; +} + static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_name, const char *cell_name, struct cpumask *cpumask,struct
of_phandle_args*pargs)kernel/power/energy_model.c +2
-0@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) { unsigned long prev_cost = ULONG_MAX; + struct cpufreq_policy *policy; + unsigned int max_nonboost = 0; int i, ret; /* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device(dev)) return 0;Quoted 7 lines
kernel/power/energy_model.c static int em_compute_costs(struct device *dev, struct em_perf_state *table, · line 271
Quoted 7 lines
kernel/power/energy_model.c +6
-0+ policy = cpufreq_cpu_get(dev->id); + if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); + cpufreq_cpu_put(policy); + } +The lookup can be made more robust though: on the update paths
dev->idmay be offline while other CPUs of the performance domain are online. In v3, ifdev->idhas no policy and the performance domain already exists, I will use an active CPU of the performance domain instead, asem_cpufreq_update_efficiencies()does, and only warn and fall back to the old behaviour if that fails as well.I did not use the performance domain for every lookup because at registration time
dev->em_pdis not assigned yet (it is set afterem_create_perf_table()has returned):policy =
cpufreq_cpu_get(dev->id);
if (!policy &&dev->em_pd) {
cpu =cpumask_first_and(em_span_cpus(dev->em_pd), cpu_active_mask); if (cpu <nr_cpu_ids)
policy =cpufreq_cpu_get(cpu);
}if (policy) {
max_nonboost=cpufreq_table_max_nonboost_freq(policy);cpufreq_cpu_put(policy);
} else {
dev_warn(dev, "EM: no CPUFreq policy, boost states are " "not excluded from the ranking\n"); }Quoted 6 lines
This makes it much less likely to miss the policy, but cannot rule it out:
the CPU can still go offline between picking it and callingcpufreq_cpu_get().em_cpufreq_update_efficiencies()has the same limitation.Would that work for you?
I also thought about that possible racy corner case, where CPU can go offline (or already be offline). Although, in current usage this should be OK, since we do this in boot phase and platforms are sane.
Quoted 5 lines
We can can add that step with '
cpu_active_mask' to play safe, but no hard promises it would be atomic.The warning also would help, for bring-up phase, so engineers would notice it and address the racy hot-plug booting system when it's there.
Thanks Lukasz,
I’ll add the 'cpu_active_mask' check and the warning in v3. As the overall approach is unchanged, I’ll keep your reviewed-by. Please let me know if you’d rather have it removed.
Thanks,
Guo
Lukasz Luba
maintainerQuoted 170 lines · Guo Chen
Quoted 168 lines · Lukasz Luba
Quoted 151 lines · Guo Chen
Quoted 19 lines · Rafael J. Wysocki (Intel)
Quoted 11 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Quoted 7 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>Quoted 7 lines
Please see Sashiko's feedback regarding this patch
https://sashiko.dev/#/patchset/20261103-em-rank-boost-separately- v2-1-5a13d1b38d37%40oss.qualcomm.com
and tell me what you think about it.
Thanks!
Thanks for the review.
I did consider this case. If no policy is found,
max_nonbooststays 0 and the new logic is bypassed, i.e. the ranking falls back to the old behaviour, which looked like the least intrusive option. I also thought about warning and returning an error, but most of the in-tree callers ofem_recalc_and_update()ignore the return value (e.g.em_adjust_new_capacity()andexynos_asv_update_opps()), so the update would just be dropped without anyone noticing.Quoted 77 lines
Quoted 77 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost- [email protected]/
---
include/linux/cpufreq.h | 24 ++++++++++++++++++++++++
kernel/power/energy_model.c| 16 ++++++++++++++++2 files changed, 40 insertions(+)include/linux/cpufreq.h +10
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + struct cpufreq_frequency_table *pos; + unsigned int max = 0; + + if (!policy->freq_table) + return 0; + + /* The table may be unsorted, scan all of it for thehighest one. */
+cpufreq_for_each_valid_entry(pos, policy->freq_table){ + if (!(pos->flags &CPUFREQ_BOOST_FREQ) && pos-Quoted 1 line
frequency > max)
+ max =
pos->frequency;
+ }include/linux/cpufreq.h +4
-0+ + return max; +} +static inline int
parse_perf_domain(int cpu, const char *list_name, const char *cell_name, struct of_phandle_args *args)include/linux/cpufreq.h +6
-0@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(structcpufreq_policy *policy, return -EINVAL; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ + return 0; +} + static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_name, const char *cell_name, struct cpumask *cpumask,struct
of_phandle_args*pargs)kernel/power/energy_model.c +2
-0@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) { unsigned long prev_cost = ULONG_MAX; + struct cpufreq_policy *policy; + unsigned int max_nonboost = 0; int i, ret; /* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device(dev)) return 0;Quoted 7 lines
kernel/power/energy_model.c static int em_compute_costs(struct device *dev, struct em_perf_state *table, · line 271
Quoted 7 lines
kernel/power/energy_model.c +6
-0+ policy = cpufreq_cpu_get(dev->id); + if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); + cpufreq_cpu_put(policy); + } +The lookup can be made more robust though: on the update paths dev-
Quoted 1 line
id may
be offline while other CPUs of the performance domain are online. In v3, if
dev->idhas no policy and the performance domain already exists, I will use an active CPU of the performance domain instead, asem_cpufreq_update_efficiencies()does, and only warn and fall back to the old behaviour if that fails as well.I did not use the performance domain for every lookup because at registration time
dev->em_pdis not assigned yet (it is set afterem_create_perf_table()has returned):policy =
cpufreq_cpu_get(dev->id);
if (!policy &&dev->em_pd) {
cpu =cpumask_first_and(em_span_cpus(dev->em_pd), cpu_active_mask); if (cpu <nr_cpu_ids)
policy =cpufreq_cpu_get(cpu);
}if (policy) {
max_nonboost=cpufreq_table_max_nonboost_freq(policy);cpufreq_cpu_put(policy);
} else {
dev_warn(dev, "EM: no CPUFreq policy, boost states are " "not excluded from the ranking\n"); }Quoted 6 lines
This makes it much less likely to miss the policy, but cannot rule it out:
the CPU can still go offline between picking it and callingcpufreq_cpu_get().em_cpufreq_update_efficiencies()has the same limitation.Would that work for you?
I also thought about that possible racy corner case, where CPU can go offline (or already be offline). Although, in current usage this should be OK, since we do this in boot phase and platforms are sane.
We can can add that step with '
cpu_active_mask' to play safe, but no hard promises it would be atomic.
Quoted 8 lines
Quoted 2 lines
The warning also would help, for bring-up phase, so engineers would notice it and address the racy hot-plug booting system when it's there.
Thanks Lukasz,
I’ll add the '
cpu_active_mask' check and the warning in v3. As the overall approach is unchanged, I’ll keep your reviewed-by. Please let me know if you’d rather have it removed.
That's fine, please continue.
Rafael J. Wysocki (Intel)
maintainerQuoted 190 lines · Lukasz Luba
Quoted 186 lines · Guo Chen
Quoted 183 lines · Lukasz Luba
Quoted 163 lines · Guo Chen
Quoted 21 lines · Rafael J. Wysocki (Intel)
Quoted 11 lines · Guo Chen
em_compute_costs()ranks all the frequencies of the table, boost ones included. A non-boost frequency can be marked inefficient only because a faster boost frequency has a lower or equal cost, e.g.3.9 GHz 4.0 GHz (cost C) 4.1 GHz (boost, cost C)
With boost disabled 4.1 GHz cannot be selected, but 4.0 GHz is still skipped by governors rounding down to efficient frequencies (e.g. ondemand with
CPUFREQ_RELATION_HE), which land on 3.9 GHz instead of the best selectable frequency. Several frequencies can be affected.Quoted 8 lines
Exclude the boost frequencies from the ranking in
em_compute_costs()and
keep them all efficient, using the highest non-boost frequency of the CPUFreq table, returned by the new helpercpufreq_table_max_nonboost_freq(), as the boundary.Fixes c8ed99533dbc ("PM: EM: Mark inefficient states")Signed-off-by Guo Chen <[email protected]>Quoted 8 lines
Please see Sashiko's feedback regarding this patch
https://sashiko.dev/#/patchset/20261103-em-rank-boost-separately-
v2-1-5a13d1b38d37%40oss.qualcomm.comand tell me what you think about it.
Thanks!
Thanks for the review.
I did consider this case. If no policy is found,
max_nonbooststays 0 and
the new logic is bypassed, i.e. the ranking falls back to the old behaviour, which looked like the least intrusive option. I also thought about warning and returning an error, but most of the in-tree callers ofem_recalc_and_update()ignore the return value (e.g.em_adjust_new_capacity()andexynos_asv_update_opps()), so the update would
just be dropped without anyone noticing.Quoted 81 lines
Quoted 81 lines
---
Changes in v2:
- Dropprev_boostand keep all boost states efficient by resettingprev_costfor them, as suggested by Lukasz. Boost states are no longer ranked against each other.
- Retitle the patch to match the new behaviour (was "Rank boost and non-boost frequencies separately") and reword the commit message.Link to v1: https://lore.kernel.org/all/20261030-em-rank-boost-
[email protected]/--- include/linux/cpufreq.h | 24 ++++++++++++++++++++++++ kernel/power/energy_model.c | 16 ++++++++++++++++ 2 files changed, 40 insertions(+)include/linux/cpufreq.h +10
-0@@ -1155,6 +1155,24 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy, return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +struct cpufreq_frequency_table *pos; +unsigned int max =0 ; + +if (!policy->freq_table) +return 0 ; + +/* The table may be unsorted, scan all of it for the highest one. */
include/linux/cpufreq.h +2
-0+ cpufreq_for_each_valid_entry(pos, policy->freq_table) { + if (!(pos->flags & CPUFREQ_BOOST_FREQ) && pos-Quoted 1 line
frequency > max)
include/linux/cpufreq.h +12
-0+ max = pos->frequency; + } + +return max; +} +static inline int parse_perf_domain (int cpu,const char *list_name,const char *cell_name,struct of_phandle_args *args)@@ -1219,6 +1237,12 @@ cpufreq_table_set_inefficient(struct cpufreq_policy *policy, return -EINVAL ; } +static inline unsigned int +cpufreq_table_max_nonboost_freq (struct cpufreq_policy *policy) +{ +return 0 ; +} + static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_name,const char
*cell_name, struct cpumask *cpumask,
structof_phandle_args*pargs)kernel/power/energy_model.c +2
-0@@ -255,15 +255,24 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table, unsigned long flags) {unsigned long prev_cost =ULONG_MAX ; +struct cpufreq_policy *policy; +unsigned int max_nonboost =0 ;int i, ret;/* This is needed only for CPUs and EAS skip other devices */ if (!_is_cpu_device (dev))return 0 ;Quoted 7 lines
kernel/power/energy_model.c static int em_compute_costs(struct device *dev, struct em_perf_state *table, · line 271
Quoted 7 lines
kernel/power/energy_model.c +6
-0+ policy =cpufreq_cpu_get (dev->id); +if (policy) { + max_nonboost =cpufreq_table_max_nonboost_freq (policy); +cpufreq_cpu_put (policy); + } +The lookup can be made more robust though: on the update paths dev-
Quoted 1 line
id may
be offline while other CPUs of the performance domain are online. In v3, if
dev->idhas no policy and the performance domain already exists, I will use
an active CPU of the performance domain instead, asem_cpufreq_update_efficiencies()does, and only warn and fall back to the
old behaviour if that fails as well.I did not use the performance domain for every lookup because at registration time
dev->em_pdis not assigned yet (it is set afterem_create_perf_table()has returned):policy =
cpufreq_cpu_get(dev->id);if (!policy && dev->em_pd) { cpu = cpumask_first_and(em_span_cpus(dev->em_pd), cpu_active_mask); if (cpu < nr_cpu_ids) policy = cpufreq_cpu_get(cpu); }if (policy) { max_nonboost = cpufreq_table_max_nonboost_freq(policy); cpufreq_cpu_put(policy);} else {
dev_warn(dev, "EM: no CPUFreq policy, boost states are "
"not excluded from the ranking\n");
}Quoted 7 lines
This makes it much less likely to miss the policy, but cannot rule it out:
the CPU can still go offline between picking it and callingcpufreq_cpu_get().em_cpufreq_update_efficiencies()has the same limitation.Would that work for you?
I also thought about that possible racy corner case, where CPU can go offline (or already be offline). Although, in current usage this should be OK, since we do this in boot phase and platforms are sane.
We can can add that step with '
cpu_active_mask' to play safe, but no hard promises it would be atomic.The warning also would help, for bring-up phase, so engineers would notice it and address the racy hot-plug booting system when it's there.
Quoted 7 lines
Quoted 5 lines
Thanks Lukasz,
I’ll add the '
cpu_active_mask' check and the warning in v3. As the overall approach is unchanged, I’ll keep your reviewed-by. Please let me know if you’d rather have it removed.That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
Lukasz Luba
maintainerHi Rafael,
Quoted 1 line · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 8 lines
Quoted 2 lines
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become CPUFREQ_INEFFICIENT_FREQ entries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);
Then ondemand check for CPUFREQ_RELATION_HE will skip them and pass to __cpufreq_driver_target() with that flags as well.
Quoted 6 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
Quoted 4 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Regards,
Lukasz
Rafael J. Wysocki (Intel)
maintainerQuoted 18 lines · Lukasz Luba
Hi Rafael,
Quoted 1 line · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 8 lines
Quoted 2 lines
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);
So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.
Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 10 lines
Quoted 6 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 10 lines
Quoted 4 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set CPUFREQ_INEFFICIENT_FREQ for boost frequencies. Wouldn't that work?
Lukasz Luba
maintainerQuoted 70 lines · Rafael J. Wysocki (Intel)
Quoted 18 lines · Lukasz Luba
Hi Rafael,
Quoted 1 line · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 8 lines
Quoted 2 lines
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 10 lines
Quoted 6 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 10 lines
Quoted 4 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Quoted 8 lines
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?
I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 79 lines · Lukasz Luba
Quoted 76 lines · Rafael J. Wysocki (Intel)
Quoted 18 lines · Lukasz Luba
Hi Rafael,
Quoted 1 line · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 8 lines
Quoted 2 lines
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 10 lines
Quoted 6 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 10 lines
Quoted 4 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
Quoted 10 lines
Quoted 2 lines
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing CPUFREQ_INEFFICIENT_FREQ for boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specific to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but if policy->max is capped at 4.0 GHz by scaling_max_freq, thermal or QoS, RELATION_HE for 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done in cpufreq_frequency_table_target() without touching the EM or looking at boost flags.
Quoted 1 line
Signature · 2 lines
Thx and BRs, Zhongqiu Han
Rafael J. Wysocki (Intel)
maintainerQuoted 94 lines · Zhongqiu Han
Quoted 81 lines · Lukasz Luba
Quoted 78 lines · Rafael J. Wysocki (Intel)
Quoted 19 lines · Lukasz Luba
Hi Rafael,
Quoted 2 lines · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 8 lines
Quoted 2 lines
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 10 lines
Quoted 6 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 10 lines
Quoted 4 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
Quoted 10 lines
Quoted 2 lines
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 13 lines
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing
CPUFREQ_INEFFICIENT_FREQfor boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specific to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but ifpolicy->maxis capped at 4.0 GHz byscaling_max_freq, thermal or QoS,RELATION_HEfor 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done incpufreq_frequency_table_target()without touching the EM or looking at boost flags.
Good point.
Quoted 64 lines · Rafael J. Wysocki (Intel)
Quoted 62 lines · Zhongqiu Han
Quoted 61 lines · Lukasz Luba
Quoted 59 lines · Rafael J. Wysocki (Intel)
Quoted 14 lines · Lukasz Luba
Hi Rafael,
Quoted 2 lines · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 6 lines
Quoted 1 line
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 8 lines
Quoted 5 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 8 lines
Quoted 3 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?
Got your point. Having cpufreq directly refuse to set CPUFREQ_INEFFICIENT_FREQ might be the simplest approach. Let me summarize my understanding. There are two cases where we can simply avoid setting the CPUFREQ_INEFFICIENT_FREQ flag:
1. boost frequencies.
2. the highest non-boost frequency.
However, there is another issue. If we refuse to set CPUFREQ_INEFFICIENT_FREQ for certain frequencies, I think the corresponding EM_PERF_STATE_INEFFICIENT flags in the Energy Model also need to be updated. Otherwise, the EM state would be inconsistent with the frequencies used for energy estimation in the EAS path.
I think we also need to update the EM_PERF_STATE_INEFFICIENT flags accordingly, something like the following:
for (i = 0; i < pd->nr_perf_states; i++) {
int ret;
if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT)) continue;
/*Return -EOPNOTSUPP if the rejection condition is met. */ ret = cpufreq_table_set_inefficient(policy, table[i].frequency); if (!ret)
found++;
else if (ret == -EOPNOTSUPP)
table[i].flags &= ~EM_PERF_STATE_INEFFICIENT;
}
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 13 lines
Quoted 5 lines
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing
CPUFREQ_INEFFICIENT_FREQfor boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specificQuoted 8 lines
to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but if
policy->maxis capped at 4.0 GHz byscaling_max_freq, thermal or QoS,RELATION_HEfor 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done incpufreq_frequency_table_target()without touching the EM or looking at boost flags.
@Zhongqiu: thanks for the example. I think the two cases are different in nature.
In the case of the changelog, the highest non-boost frequency of cluster is marked inefficient because of the boost frequency. A sustainable frequency is being affected by an unsustainable one, but it is not really inefficient: I don't think these two kinds of frequencies should be compared with each other in terms of energy. So I think this should be fixed at the source, by not marking such frequencies as inefficient.
The case you describe looks different, as it is dynamic: policy->max is changed at runtime by scaling_max_freq, thermal or QoS, which would mean ignoring the inefficient flag every time policy->max lands on an inefficient frequency. Whether this is really necessary seems to me to deserve a discussion. Thermal and QoS limits are there to keep the frequency from going above the cap. In your example (3.9 GHz, 4.0 GHz, 4.1 GHz), if the frequency is limited to 4.0 GHz at some point and ends up on 3.9 GHz because 4.0 GHz is inefficient, the original purpose of the limit, not going above the cap, is still honoured.
So I am not sure that handling the dynamic case is needed, and I would like to hear what you, Rafael and Lukasz think about it.
Thanks,
Guo
Quoted 1 line
Good point.
Lukasz Luba
maintainerQuoted 138 lines · Guo Chen
Quoted 64 lines · Rafael J. Wysocki (Intel)
Quoted 62 lines · Zhongqiu Han
Quoted 61 lines · Lukasz Luba
Quoted 59 lines · Rafael J. Wysocki (Intel)
Quoted 14 lines · Lukasz Luba
Hi Rafael,
Quoted 2 lines · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 6 lines
Quoted 1 line
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 8 lines
Quoted 5 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 8 lines
Quoted 3 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?Got your point. Having cpufreq directly refuse to set
CPUFREQ_INEFFICIENT_FREQmight be the simplest approach. Let me summarize my understanding. There are two cases where we can simply avoid setting theCPUFREQ_INEFFICIENT_FREQflag:
1. boost frequencies.
2. the highest non-boost frequency.However, there is another issue. If we refuse to set
CPUFREQ_INEFFICIENT_FREQfor certain frequencies, I think the correspondingEM_PERF_STATE_INEFFICIENTflags in the Energy Model also need to be updated. Otherwise, the EM state would be inconsistent with the frequencies used for energy estimation in the EAS path.I think we also need to update the
EM_PERF_STATE_INEFFICIENTflags accordingly, something like the following:for (i = 0; i <
pd->nr_perf_states; i++) {
int ret;if (!(table[i].flags &
EM_PERF_STATE_INEFFICIENT)) continue;/*Return
-EOPNOTSUPPif the rejection condition is met. */ ret =cpufreq_table_set_inefficient(policy, table[i].frequency); if (!ret)
found++;
else if (ret ==-EOPNOTSUPP)
table[i].flags &= ~EM_PERF_STATE_INEFFICIENT;
}Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 13 lines
Quoted 5 lines
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing
CPUFREQ_INEFFICIENT_FREQfor boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specificQuoted 8 lines
to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but if
policy->maxis capped at 4.0 GHz byscaling_max_freq, thermal or QoS,RELATION_HEfor 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done incpufreq_frequency_table_target()without touching the EM or looking at boost flags.@Zhongqiu: thanks for the example. I think the two cases are different in nature.
In the case of the changelog, the highest non-boost frequency of cluster is marked inefficient because of the boost frequency. A sustainable frequency is being affected by an unsustainable one, but it is not really inefficient: I don't think these two kinds of frequencies should be compared with each other in terms of energy. So I think this should be fixed at the source, by not marking such frequencies as inefficient.
Quoted 9 lines
The case you describe looks different, as it is dynamic:
policy->maxis changed at runtime byscaling_max_freq, thermal or QoS, which would mean ignoring the inefficient flag every timepolicy->maxlands on an inefficient frequency. Whether this is really necessary seems to me to deserve a discussion. Thermal and QoS limits are there to keep the frequency from going above the cap. In your example (3.9 GHz, 4.0 GHz, 4.1 GHz), if the frequency is limited to 4.0 GHz at some point and ends up on 3.9 GHz because 4.0 GHz is inefficient, the original purpose of the limit, not going above the cap, is still honoured.
There are platforms which have ~20 OPPs and the difference is ~100MHz. Those at the top frequency they are inefficient but exist for this thermal purpose. The throttled frequency can be more stable and predictable in time, thus also capacity delivered to randomly scheduled tasks (which might be unfair if the freq gets lower by a lot and some latency sensitive task got a spin at the same time). Therefore, we do care by making each OPP (at least) available...
Quoted 4 lines
So I am not sure that handling the dynamic case is needed, and I would like to hear what you, Rafael and Lukasz think about it.
This becomes even more convoluted now.
1. The only call to cpufreq_table_set_inefficient() is in the EM.
2. EM works with thermal and all governors, not only EAS and schedutil
3. CPUfreq fwk holds those 'inefficient' flags in generic, global way
4. Inefficient flags were introduces mainly for EAS, now impacts non-EAS setup (as patch states 4.0GHz is valid but not reachable).
Here is an example from mainline DT platform. The goal was to avoid the frequencies which have the same voltage but lower freq, e.g. those from 200MHz to 700MHz and pick 800MHz instead. Same for 900MHz and 1200MHz.
That was the main motivation back then and research we've made with a few platforms and EAS.
freq MHz opp-microvolt 2000 <1312500 1312500 1500000> 1900 <1262500 1262500 1500000> 1800 <1237500 1237500 1500000> 1700 <1250000 1250000 1500000> 1600 <1250000 1250000 1500000> 1500 <1100000 1100000 1500000> 1400 <1100000 1100000 1500000> 1300 <1100000 1100000 1500000> 1200 <1000000 1000000 1500000> 1100 <1000000 1000000 1500000> 1000 <1000000 1000000 1500000> 900 <1000000 1000000 1500000> 800 <900000 900000 1500000> 700 <900000 900000 1500000> 600 <900000 900000 1500000> 500 <900000 900000 1500000> 400 <900000 900000 1500000> 300 <900000 900000 1500000> 200 <900000 900000 1500000>
Therefore, I would suggest to tackle this is steps.
1st, let's go ahead with this fix which is for mainly EM. The EM confuses the cpufreq and then the clients like ondemand, IMHO. This can go maybe into the stable kernels, since it's small.
2nd, let's think about that 'boost' thing wider. There are already investigations how to handle correctly the CPU's capacity and to not confuse the task scheduler. This might create the right infrastructure.
Side note:
I personally don't like calling some frequencies 'boost', because such frequencies were there but not labeled that way and they were OK. In this example, 4.0GHz is non-boost freq, but 4.1GHz is 'boost'. At the same time 4.0GHz is 'inefficient', which can only happen when 4.1GHz has the same voltage. I have seen such platforms ~13years ago and the 'boost' flag wasn't needed to handle them properly.
Quoted 141 lines · Lukasz Luba
Quoted 138 lines · Guo Chen
Quoted 64 lines · Rafael J. Wysocki (Intel)
Quoted 62 lines · Zhongqiu Han
Quoted 61 lines · Lukasz Luba
Quoted 59 lines · Rafael J. Wysocki (Intel)
Quoted 14 lines · Lukasz Luba
Hi Rafael,
Quoted 2 lines · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 6 lines
Quoted 1 line
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 8 lines
Quoted 5 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 8 lines
Quoted 3 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?Got your point. Having cpufreq directly refuse to set
CPUFREQ_INEFFICIENT_FREQmight be the simplest approach. Let me summarize my understanding. There are two cases where we can simply avoid setting theCPUFREQ_INEFFICIENT_FREQflag:
1. boost frequencies.
2. the highest non-boost frequency.However, there is another issue. If we refuse to set
CPUFREQ_INEFFICIENT_FREQfor certain frequencies, I think the correspondingEM_PERF_STATE_INEFFICIENTflags in the Energy Model also need to be updated. Otherwise, the EM state would be inconsistent with the frequencies used for energy estimation in the EAS path.I think we also need to update the
EM_PERF_STATE_INEFFICIENTflags accordingly, something like the following:for (i = 0; i <
pd->nr_perf_states; i++) {
int ret;if (!(table[i].flags &
EM_PERF_STATE_INEFFICIENT)) continue;/*Return
-EOPNOTSUPPif the rejection condition is met. */ ret =cpufreq_table_set_inefficient(policy, table[i].frequency); if (!ret)
found++;
else if (ret ==-EOPNOTSUPP)
table[i].flags &= ~EM_PERF_STATE_INEFFICIENT;
}Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 13 lines
Quoted 5 lines
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing
CPUFREQ_INEFFICIENT_FREQfor boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specificQuoted 8 lines
to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but if
policy->maxis capped at 4.0 GHz byscaling_max_freq, thermal or QoS,RELATION_HEfor 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done incpufreq_frequency_table_target()without touching the EM or looking at boost flags.@Zhongqiu: thanks for the example. I think the two cases are different in nature.
In the case of the changelog, the highest non-boost frequency of cluster is marked inefficient because of the boost frequency. A sustainable frequency is being affected by an unsustainable one, but it is not really inefficient: I don't think these two kinds of frequencies should be compared with each other in terms of energy. So I think this should be fixed at the source, by not marking such frequencies as inefficient.
Quoted 9 lines
Quoted 9 lines
The case you describe looks different, as it is dynamic:
policy->maxis changed at runtime byscaling_max_freq, thermal or QoS, which would mean ignoring the inefficient flag every timepolicy->maxlands on an inefficient frequency. Whether this is really necessary seems to me to deserve a discussion. Thermal and QoS limits are there to keep the frequency from going above the cap. In your example (3.9 GHz, 4.0 GHz, 4.1 GHz), if the frequency is limited to 4.0 GHz at some point and ends up on 3.9 GHz because 4.0 GHz is inefficient, the original purpose of the limit, not going above the cap, is still honoured.
Hi Guo,
Boost can be switched on and off at runtime as well, so it is no less dynamic than thermal or QoS. And with boost off, 3.9 GHz does not go above the cap either, so if that were enough, the changelog case would not need fixing; in both cases the highest allowed frequency is never reached.
Quoted 8 lines
There are platforms which have ~20 OPPs and the difference is ~100MHz. Those at the top frequency they are inefficient but exist for this thermal purpose. The throttled frequency can be more stable and predictable in time, thus also capacity delivered to randomly scheduled tasks (which might be unfair if the freq gets lower by a lot and some latency sensitive task got a spin at the same time). Therefore, we do care by making each OPP (at least) available...
Agreed with Lukasz in this point.
Quoted 45 lines
Quoted 5 lines
So I am not sure that handling the dynamic case is needed, and I would like to hear what you, Rafael and Lukasz think about it.
This becomes even more convoluted now.
1. The only call tocpufreq_table_set_inefficient()is in the EM.
2. EM works with thermal and all governors, not only EAS and schedutil
3. CPUfreq fwk holds those 'inefficient' flags in generic, global way
4. Inefficient flags were introduces mainly for EAS, now impacts non-EAS setup (as patch states 4.0GHz is valid but not reachable).Here is an example from mainline DT platform. The goal was to avoid the frequencies which have the same voltage but lower freq, e.g. those from 200MHz to 700MHz and pick 800MHz instead. Same for 900MHz and 1200MHz.
That was the main motivation back then and research we've made with a few platforms and EAS.freq MHz opp-microvolt
2000 <1312500 1312500 1500000>
1900 <1262500 1262500 1500000>
1800 <1237500 1237500 1500000>
1700 <1250000 1250000 1500000>
1600 <1250000 1250000 1500000>
1500 <1100000 1100000 1500000>
1400 <1100000 1100000 1500000>
1300 <1100000 1100000 1500000>
1200 <1000000 1000000 1500000>
1100 <1000000 1000000 1500000>
1000 <1000000 1000000 1500000>
900 <1000000 1000000 1500000>
800 <900000 900000 1500000>
700 <900000 900000 1500000>
600 <900000 900000 1500000>
500 <900000 900000 1500000>
400 <900000 900000 1500000>
300 <900000 900000 1500000>
200 <900000 900000 1500000>Therefore, I would suggest to tackle this is steps.
Quoted 9 lines
1st, let's go ahead with this fix which is for mainly EM. The EM confuses the cpufreq and then the clients like ondemand, IMHO. This can go maybe into the stable kernels, since it's small.
2nd, let's think about that 'boost' thing wider. There are already investigations how to handle correctly the CPU's capacity and to not confuse the task scheduler. This might create the right infrastructure.
Hi Lukasz,
Thanks for the explanation and the DT example. Real platform data like this is very helpful for this discussion.
May I know wouldn't it be better to handle this in cpufreq alone, without changing the EM? The EM change may not help with thermal capping in general: your DT example has no boost OPPs, and if policy->max is capped at 1700 MHz, RELATION_HE for max seems to resolve to 1500 MHz, as 1600 and 1700 MHz are dominated by 1800 MHz.
When the target is at or above max, the highest frequency within the limits is wanted. If it is marked inefficient, the frequency that dominates it is above max and cannot be selected, so maybe it should not be skipped. That would cover boost off, scaling_max_freq, thermal and QoS, while targets below max are not affected.
Something like the below (compile-tested only). (Please let me know what you and Rafael and Guo think, or if I have overlooked anything if you would like)
diff
include/linux/cpufreq.h +23 -3
@@ -1077,6 +1077,7 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, {bool efficiencies = policy->efficiencies_available && (relation &CPUFREQ_RELATION_E ); +bool capped = false;int idx;/* cpufreq_table_index_unsorted() has no use for this flag anyway */
@@ -1085,6 +1086,16 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,if (unlikely (policy->freq_table_sorted ==CPUFREQ_TABLE_UNSORTED ))return cpufreq_table_index_unsorted (policy, target_freq, min, max, relation); + +/* +* A target at or above @max asks for the highest frequency within the +* limits. Whatever makes that frequency inefficient lies above @max and +* cannot be selected, so inefficient frequencies must not be skipped. +*/ +if (efficiencies && target_freq >= max) { + efficiencies = false; + capped = true; + } retry:switch (relation) {case CPUFREQ_RELATION_L :
@@ -1102,9 +1113,18 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, }/* Limit frequency index to honor min and max */ -if (!cpufreq_is_in_limits (policy, min, max, idx) && efficiencies) { - efficiencies = false; - goto retry; +if (!cpufreq_is_in_limits (policy, min, max, idx)) { +/* Fall back to the regular lookup over efficient frequencies */ +if (capped) { + capped = false; + efficiencies = true; + goto retry; + } + +if (efficiencies) { + efficiencies = false; + goto retry; + } }return idx;
Quoted 11 lines
Side note:
I personally don't like calling some frequencies 'boost', because such frequencies were there but not labeled that way and they were OK. In this example, 4.0GHz is non-boost freq, but 4.1GHz is 'boost'. At the same time 4.0GHz is 'inefficient', which can only happen when 4.1GHz has the same voltage. I have seen such platforms ~13years ago and the 'boost' flag wasn't needed to handle them properly.
Signature · 2 lines
Thx and BRs, Zhongqiu Han
Quoted 153 lines · Zhongqiu Han
Quoted 142 lines · Lukasz Luba
Quoted 139 lines · Guo Chen
Quoted 64 lines · Rafael J. Wysocki (Intel)
Quoted 62 lines · Zhongqiu Han
Quoted 61 lines · Lukasz Luba
Quoted 59 lines · Rafael J. Wysocki (Intel)
Quoted 14 lines · Lukasz Luba
Hi Rafael,
Quoted 2 lines · Rafael J. Wysocki (Intel)
On Tue, Nov 10, 2026 at 10:58 AM Lukasz Luba <[email protected]> wrote:
[snip]
Quoted 6 lines
Quoted 1 line
That's fine, please continue.
I have a couple of questions though.
First, why is the ondemand governor mentioned in the changelog? In the mainline, the ondemand governor cannot work with EAS, so it shouldn't matter for this issue, should it?
Quoted 8 lines
The ondemand is affected by the EM setup code and set 'inefficient' flags for some OPPs. The EM is also present for cooling devices and can work w/o EAS and schedutil, w/ just ondemand governor.
Because EM marks the OPPs kind of 'globally' as inefficient, then they become
CPUFREQ_INEFFICIENT_FREQentries in the CPUfreq fwk. In the EM code we have:cpufreq_table_set_inefficient(policy, table[i].frequency);So maybe this is the one that needs to be fixed? What about simply skipping boost frequencies in it?
Quoted 2 lines
Then ondemand check for
CPUFREQ_RELATION_HEwill skip them and pass to__cpufreq_driver_target()with that flags as well.Thanks for explaining and all of that needs to be said in the patch changelog.
Quoted 8 lines
Quoted 5 lines
Second, why is the issue regarded as a defect in the code and not in the energy model? Arguably, boost frequencies should be more costly than the non-boost ones. At least the idea behind the boost is making higher-cost OPPs temporarily available. If they are sustainable, treating them as boost OPPs is rather pointless.
IMHO it was a bug in the commit mentioned in this patch fix tag. It is affecting more than just EAS or EM, since the EM is working with all generic code and all cpufreq governors.
To be precise, the EM initialization may affect cpufreq in general because inefficient frequencies are used for CPU cooling. Fair enough, but I'm not talking about this.
I'm talking about a reason for giving boost frequencies lower or equal cost relative to non-boost frequencies in the energy model because this doesn't make sense to me TBH.
And I get that such energy models were there before the commit in question and now they are not working properly, but IMO they are broken, not that commit.
Quoted 8 lines
Quoted 3 lines
TBH, I'm too comfortable with looking at cpufreq frequency from within the EM code. If that needs to be done, something is amiss at the design level.
I think this edge case is special, as I described above.
I'm happy to hear your view on this EM influence to some global setting in cpufreq. Maybe it was too far to establish this 'inefficiency' flags in there for many usage?
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
There's nothing wrong with using the information from the energy model, when available, to amend cpufreq, but the patch goes the other way around: It tries to amend the energy model with information taken from cpufreq (in order to avoid making mistakes later when amending cpufreq with the energy model information).
I think that cpufreq should simply refuse to set
CPUFREQ_INEFFICIENT_FREQfor boost frequencies. Wouldn't that work?Got your point. Having cpufreq directly refuse to set
CPUFREQ_INEFFICIENT_FREQmight be the simplest approach. Let me summarize my understanding. There are two cases where we can simply avoid setting theCPUFREQ_INEFFICIENT_FREQflag:
1. boost frequencies.
2. the highest non-boost frequency.However, there is another issue. If we refuse to set
CPUFREQ_INEFFICIENT_FREQfor certain frequencies, I think the correspondingEM_PERF_STATE_INEFFICIENTflags in the Energy Model also need to be updated. Otherwise, the EM state would be inconsistent with the frequencies used for energy estimation in the EAS path.I think we also need to update the
EM_PERF_STATE_INEFFICIENTflags accordingly, something like the following:for (i = 0; i <
pd->nr_perf_states; i++) {
int ret;if (!(table[i].flags &
EM_PERF_STATE_INEFFICIENT)) continue;/*Return
-EOPNOTSUPPif the rejection condition is met. */ ret =cpufreq_table_set_inefficient(policy, table[i].frequency); if (!ret)
found++;
else if (ret ==-EOPNOTSUPP)
table[i].flags &= ~EM_PERF_STATE_INEFFICIENT;
}Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
I see your point now, about the direction. Yes, you're right.
Since we are taking the information about 'boost' from the cpufreq, then refusing to set OPPs inefficient in the first place makes more sense.
Quoted 13 lines
Quoted 5 lines
I also agree this should be fixed in cpufreq rather than by feeding cpufreq information into the EM. However, refusing
CPUFREQ_INEFFICIENT_FREQfor boost frequencies seems not help the example in the changelog, because the frequency marked as inefficient there is 4.0 GHz, which is not a boost one. The problem is not specificQuoted 8 lines
to boost either: with 3.9 GHz, 4.0 GHz, 4.1 GHz and a 4.2 GHz boost OPP at costs C - d, C, C and C + d, 4.0 GHz is rightly inefficient, but if
policy->maxis capped at 4.0 GHz byscaling_max_freq, thermal or QoS,RELATION_HEfor 4.0 GHz still resolves to 3.9 GHz. I think the lookup should not skip an inefficient frequency when the next efficient frequency above it is beyond the max limit, which can be done incpufreq_frequency_table_target()without touching the EM or looking at boost flags.@Zhongqiu: thanks for the example. I think the two cases are different in
nature.In the case of the changelog, the highest non-boost frequency of cluster is marked inefficient because of the boost frequency. A sustainable frequency is being affected by an unsustainable one, but it is not really inefficient: I don't think these two kinds of frequencies should be compared with each other in terms of energy. So I think this should be fixed at the source, by not marking such frequencies as inefficient.
Quoted 9 lines
Quoted 9 lines
The case you describe looks different, as it is dynamic:
policy->maxis changed at runtime byscaling_max_freq, thermal or QoS, which would mean ignoring the inefficient flag every timepolicy->maxlands on an inefficient frequency. Whether this is really necessary seems to me to deserve a discussion. Thermal and QoS limits are there to keep the frequency from going above the cap. In your example (3.9 GHz, 4.0 GHz, 4.1 GHz), if the frequency is limited to 4.0 GHz at some point and ends up on 3.9 GHz because 4.0 GHz is inefficient, the original purpose of the limit, not going above the cap, is still honoured.
Quoted 4 lines
Hi Guo,
Boost can be switched on and off at runtime as well, so it is no less dynamic than thermal or QoS. And with boost off, 3.9 GHz does not go above the cap either, so if that were enough, the changelog case would not need fixing; in both cases the highest allowed frequency is never reached.
Thanks, Zhongqiu, and Lukasz for the DT example.
You are right that boost can be switched at runtime like thermal or QoS, so the key point I see is not about being dynamic. It is that a boost frequency is not sustainable, and I don't think it makes sense to rank it against the sustainable ones in terms of energy.
I agree with Lukasz's suggestion to address this in two steps. For the first step, I think fixing the EM is necessary because the EM is intended to balance performance and energy cost. The flag needs to be corrected at the source because it is also used by the EAS path. If we leave the flag unchanged and only adjust the cpufreq path to select an appropriate frequency, even if that frequency is marked as inefficient, we would end up with inconsistent behavior between EAS and cpufreq.
For example, suppose 4.0 GHz is the highest non-boost frequency, but it is marked as inefficient due to the presence of a 4.1 GHz boost frequency. With only the cpufreq-side change you suggested, EAS would still never use 4.0 GHz for energy evaluation because it remains marked as inefficient in the EM.
So my understanding is that the first step Lukasz suggested is not only about fixing the current impact of the EM on cpufreq, but also about making the EM itself correct for the EAS path.
As for the separate issue where policy->max is set to an inefficient frequency by thermal/QoS constraints and that frequency then cannot be selected, I think that should be treated as a different problem.
What do you think?
Quoted 159 lines
Quoted 8 lines
There are platforms which have ~20 OPPs and the difference is ~100MHz. Those at the top frequency they are inefficient but exist for this thermal purpose. The throttled frequency can be more stable and predictable in time, thus also capacity delivered to randomly scheduled tasks (which might be unfair if the freq gets lower by a lot and some latency sensitive task got a spin at the same time). Therefore, we do care by making each OPP (at least) available...
Agreed with Lukasz in this point.
Quoted 45 lines
Quoted 5 lines
So I am not sure that handling the dynamic case is needed, and I would like to hear what you, Rafael and Lukasz think about it.
This becomes even more convoluted now.
1. The only call tocpufreq_table_set_inefficient()is in the EM.
2. EM works with thermal and all governors, not only EAS and schedutil
3. CPUfreq fwk holds those 'inefficient' flags in generic, global way
4. Inefficient flags were introduces mainly for EAS, now impacts non-EAS setup (as patch states 4.0GHz is valid but not reachable).Here is an example from mainline DT platform. The goal was to avoid the frequencies which have the same voltage but lower freq, e.g. those from 200MHz to 700MHz and pick 800MHz instead. Same for 900MHz and 1200MHz.
That was the main motivation back then and research we've made with a few platforms and EAS.freq MHz opp-microvolt
2000 <1312500 1312500 1500000>
1900 <1262500 1262500 1500000>
1800 <1237500 1237500 1500000>
1700 <1250000 1250000 1500000>
1600 <1250000 1250000 1500000>
1500 <1100000 1100000 1500000>
1400 <1100000 1100000 1500000>
1300 <1100000 1100000 1500000>
1200 <1000000 1000000 1500000>
1100 <1000000 1000000 1500000>
1000 <1000000 1000000 1500000>
900 <1000000 1000000 1500000>
800 <900000 900000 1500000>
700 <900000 900000 1500000>
600 <900000 900000 1500000>
500 <900000 900000 1500000>
400 <900000 900000 1500000>
300 <900000 900000 1500000>
200 <900000 900000 1500000>Therefore, I would suggest to tackle this is steps.
Quoted 9 lines
1st, let's go ahead with this fix which is for mainly EM. The EM confuses the cpufreq and then the clients like ondemand, IMHO. This can go maybe into the stable kernels, since it's small.
2nd, let's think about that 'boost' thing wider. There are already investigations how to handle correctly the CPU's capacity and to not confuse the task scheduler. This might create the right infrastructure.
Hi Lukasz,
Thanks for the explanation and the DT example. Real platform data like this is very helpful for this discussion.
May I know wouldn't it be better to handle this in cpufreq alone, without changing the EM? The EM change may not help with thermal capping in general: your DT example has no boost OPPs, and if
policy->maxis capped at 1700 MHz,RELATION_HEfor max seems to resolve to 1500 MHz, as 1600 and 1700 MHz are dominated by 1800 MHz.When the target is at or above max, the highest frequency within the limits is wanted. If it is marked inefficient, the frequency that dominates it is above max and cannot be selected, so maybe it should not be skipped. That would cover boost off,
scaling_max_freq, thermal and QoS, while targets below max are not affected.Something like the below (compile-tested only). (Please let me know what you and Rafael and Guo think, or if I have overlooked anything if you would like)
diff
include/linux/cpufreq.h +0
-0@@ -1077,6 +1077,7 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, {bool efficiencies =
policy->efficiencies_available&& (relation &CPUFREQ_RELATION_E); + bool capped = false;
int idx;/*
cpufreq_table_index_unsorted()has no use for this flag anyway */include/linux/cpufreq.h +3
-0@@ -1085,6 +1086,16 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, if (unlikely(policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED)) return cpufreq_table_index_unsorted(policy, target_freq, min, max, relation); + + /* +* A target at or above @max asks for the highest frequency within the
+ * limits. Whatever makes that frequency inefficient lies above @max and
+ * cannot be selected, so inefficient frequencies must not be skipped.
+ */
+ if (efficiencies &&target_freq>= max) {
+ efficiencies = false;
+ capped = true;
+ }
retry:
switch (relation) {
caseCPUFREQ_RELATION_L:include/linux/cpufreq.h +12
-3@@ -1102,9 +1113,18 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy, } /* Limit frequency index to honor min and max */ - if (!cpufreq_is_in_limits(policy, min, max, idx) && efficiencies) { - efficiencies = false; - goto retry; + if (!cpufreq_is_in_limits(policy, min, max, idx)) { + /* Fall back to the regular lookup over efficient frequencies */ + if (capped) { + capped = false; + efficiencies = true; + goto retry; + } + + if (efficiencies) { + efficiencies = false; + goto retry; + } }return idx;
Quoted 11 lines
Side note:
I personally don't like calling some frequencies 'boost', because such frequencies were there but not labeled that way and they were OK. In this example, 4.0GHz is non-boost freq, but 4.1GHz is 'boost'. At the same time 4.0GHz is 'inefficient', which can only happen when 4.1GHz has the same voltage. I have seen such platforms ~13years ago and the 'boost' flag wasn't needed to handle them properly.