Re: [PATCH v2 3/3] cpufreq: Update set_boost callbacks to rely on boost_freq_req

[Pierre Gondois <pierre.gondois@arm.com>]

Hello Lifeng,

On 12/23/25 09:15, zhenglifeng (A) wrote:
> On 2025/12/18 0:22, Pierre Gondois wrote:
>> cpufreq_frequency_table_cpuinfo() can effectively update
>> policy->cpuinfo.max_freq, but directly setting policy->max should be wrong
>> as it bypasses the other QoS constraints on the maximal frequency.
>>
>> Updates to policy->max should go through the following call chain
>> to be sure all constraints/notifiers are respected/called.
>> freq_qos_update_request()
>> \-freq_qos_apply()
>>    \-pm_qos_update_target()
>>      \-blocking_notifier_call_chain()
>>        \-cpufreq_notifier_max()
>>          \-handle_update()
>>            \-refresh_frequency_limits()
>>              \-cpufreq_set_policy()
>>
>> FYIU, we should have:
>> - max_freq_req: the maximal frequency constraint as set by the user.
>>    It is updated whenever the user write to scaling_max_freq.
>> - boost_freq_req: the maximal frequency constraint as set by the
>>    driver. It is updated whenever boost is enabled/disabled.
>> - policy->cpuinfo.max_freq: the maximal frequency reachable by the driver.
>>    This value is used in cpufreq at various places to check frequencies
>>    are within valid boundaries.
>> - policy->max: the maximal frequency cpufreq can use. It is a resultant
>>    of all the QoS constraints received (from the user, boost, thermal).
>>    It should be updated whenever one of the QoS constraint is updated.
>>    It should never be set directly to avoid bypassing the QoS constraints.
>>
>> Whenever a cpufreq driver is initialized, policy->max is set, but the
>> value is overridden whenever the user writes to scaling_max_freq.
>> Thus we might think it should be replaced with a max_freq_req constraint.
>>
>> However if boost is enabled, the maximal frequency will be limited by
>> max_freq_req. So at init, cpufreq drivers should set boost_freq_req
>> instead (to policy->cpuinfo.max_freql).
>> That way, if boost is enabled, the maximal frequency available is the
>> boost frequency.
>>
>> ------
>>
>> Summary:
>> -
>> policy->max should never be set directly. It should only be set through
>> cpufreq_set_policy(). cpufreq_set_policy() might be called indirectly
>> after updating a QoS constraint using freq_qos_update_request().
>>
>> -
>> boost_freq_req should be set for all cpufreq drivers, with a default value
>> of policy->cpuinfo.max_freq. This represents the maximal frequency available
>> with/without boost.
>> Note: the name "boost_freq_req" might not be well chosen.
>>
>> -
>> Any update to policy->cpuinfo.max_freq should be followed by a call to
>> freq_qos_update_request(policy->boost_freq_req).
>> This will allow to update "policy->max" with the new boost frequency.
>>
> Hi Pierre,
>
> I now think we might not need to add a new QoS constraints. Calling
> refresh_frequency_limits() instead of freq_qos_update_request() when
> setting boost might solve your problem, since cpuinfo.max_freq is already
> used to limit policy->max in cpufreq_set_policy().
>
> What do you think?

In:
cpufreq_set_policy()
\-cpufreq_driver->verify(&new_data)
   \-cpufreq_verify_within_cpu_limits()

the requested min/max values are clamped wrt the cpuinfo.[min|max]_freq.
However this clamping happens after the QoS constraints have been
aggregated. This means that if a CPU has:
- min = 100.000 kHz
- max = 1.000.000 kHz
- boost = 1.200.000 kHz

With boost enabled, the user requests:
- scaling_min: 1.100.000
- scaling_max: 1.200.000

If boost is disabled, we will have:
policy->min == policy->max == 1.000.000
without notifying anybody.

Ideally I assume it would be better to prevent the user from disabling
boost without first asking to update the scaling_[min|max] frequencies,
or at least detecting this case and have a warning message.

It would be possible to detect this case in cpufreq_set_policy(),
but I m not sure it would be easy to act on it easily.

Please let me know if you prefer not adding the new qos constraint,
I ll try harder not to have it if yes.