[PATCH v2] PM: EM: Exclude boost frequencies from inefficiency ranking

[PATCH v2] PM: EM: Exclude boost frequencies from inefficiency ranking

[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 helper
cpufreq_table_max_nonboost_freq(), as the boundary.

Fixes: c8ed99533dbc ("PM: EM: Mark inefficient states")
Signed-off-by: Guo Chen <guo.chen@oss.qualcomm.com>
---
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(+)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d3d0d9d02aa4..c93316141a11 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -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)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index d19a65461ae8..5b1d095fd376 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -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,
--  
Guo Chen <guo.chen@oss.qualcomm.com>

Re: [PATCH v2] PM: EM: Exclude boost frequencies from inefficiency ranking

[Lukasz Luba]

On 11/3/26 11:21, Guo Chen wrote:
> 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.
> 
> Fixes: c8ed99533dbc ("PM: EM: Mark inefficient states")
> Signed-off-by: Guo Chen <guo.chen@oss.qualcomm.com>
> ---
> 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(+)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index d3d0d9d02aa4..c93316141a11 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -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)
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index d19a65461ae8..5b1d095fd376 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -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,
> --
> Guo Chen <guo.chen@oss.qualcomm.com>


LGTM,

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>