Re: [PATCH] PM: EM: Rank boost and non-boost frequencies separately

[Guo Chen <guo.chen@oss.qualcomm.com>]

On 11/2/2026 11:28 PM, Lukasz Luba wrote:
>
>
> On 10/30/26 08:10, 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.
>>
>> Rank boost and non-boost frequencies separately in em_compute_costs(),
>> 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>
>> ---
>>   include/linux/cpufreq.h     | 24 ++++++++++++++++++++++++
>>   kernel/power/energy_model.c | 15 +++++++++++++++
>>   2 files changed, 39 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..253c53ca6a74 100644
>> --- a/kernel/power/energy_model.c
>> +++ b/kernel/power/energy_model.c
>> @@ -255,15 +255,30 @@ 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;
>> +    bool prev_boost = false;
>>       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 = table[i].frequency > max_nonboost;
>> +
>> +        if (boost != prev_boost) {
>
> We expect in EM fwk that the frequencies are sorted ascending
> and we return error during setup if the driver callback fails
> to provide that.
> Therefore, in this code we don't have to deal with random
> 'boost' labels with gaps between.
>
Hi Lukasz,

Thanks for the review.

You are right. The boundary is derived from the highest non-boost
frequency, so with an ascending table the boost states always form a
contiguous top segment, and prev_boost is redundant.

> Let's make all OPPs which are 'boost' equally possible
> to reach (at least in theory), so label them as
> 'efficient'. I think this is fair in 'boosting' case scenario
> and energy has not that importance in it.
>
> So simply something like, would be enough IMHO:
>
> if (boost)
>     prev_cost = ULONG_MAX;
>
Agreed, this is simpler and gives the same result for the non-boost
states. Boost states are no longer compared with each other, which is
also a sensible semantic. I will drop prev_boost and use the above in v2.

Thanks,
Guo
>
>> +            prev_boost = boost;
>> +            prev_cost = ULONG_MAX;
>> +        }
>>             if ((flags & EM_PERF_DOMAIN_ARTIFICIAL) && cb->get_cost) {
>>               ret = cb->get_cost(dev, table[i].frequency, &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>
>