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

[Pierre Gondois]

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.