Re: [PATCH v2 4/4] cpufreq: Use policy->min/max init as QoS request

[Pierre Gondois]

Hello Viresh,

On 5/20/26 12:03, Viresh Kumar wrote:
> On 11-05-26, 15:55, Pierre Gondois wrote:
> > @@ -1399,8 +1399,16 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
> >   
> >   static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
> >   {
> > +	unsigned int min_freq, max_freq;
> >   	int ret;
> >   
> > +	/* Use policy->min/max set by the driver as QoS requests. */
> > +	min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
> > +	if (policy->max)
> > +		max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
> > +	else
> > +		max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
> > +
> Why is this required to be done before setting policy->min/max ? And
> so I don't think patch 1/4 is required at all.
Sorry if I misunderstand, but if we do:
"""
/*
 * If the driver didn't set policy->min/max, set them as
 * they are used to clamp frequency requests.
 */
policy->min = policy->min ? policy->min : policy->cpuinfo.min_freq;
policy->max = policy->max ? policy->max : policy->cpuinfo.max_freq;


/* Use policy->min/max set by the driver as QoS requests. */
min_freq = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
if (policy->max)
  max_freq = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
else
  max_freq = FREQ_QOS_MAX_DEFAULT_VALUE;
"""

then drivers that don't set policy->min/max in their .init() callback
will end up with a QoS constraint of:
[cpuinfo.min_freq:cpuinfo.max_freq].

This would bring us to what the following patch tried to solve:
521223d8b3ec ("cpufreq: Fix initialization of min and max
frequency QoS requests")

------

About removing patch [1/4], Zhongqiu noted that policy->min/max should
be set before the CPUFREQ_CREATE_POLICY notifier [1].
I then thought it would be better to save policy->min/max values
that are meant to become QoS constraint:
- as close as possible to the cpufreq_driver->init() call
- in a separate function, to do all the QoS creation in a separate
  function.

[1]

https://lore.kernel.org/all/73fac9ca-451d-49f0-b9c7-5ef6bc0119bf@xxxxxxxxxxxxxxxx/


> >   	/*
> >   	 * If the driver didn't set policy->min/max, set them as
> >   	 * they are used to clamp frequency requests.
> > @@ -1418,12 +1426,12 @@ static int cpufreq_policy_init_qos(struct cpufreq_policy *policy)
> >   	}
> >   
> >   	ret = freq_qos_add_request(&policy->constraints, &policy->min_freq_req,
> > -				   FREQ_QOS_MIN, FREQ_QOS_MIN_DEFAULT_VALUE);
> > +				   FREQ_QOS_MIN, min_freq);
> >   	if (ret < 0)
> >   		return ret;
> >   
> >   	ret = freq_qos_add_request(&policy->constraints, &policy->max_freq_req,
> > -				   FREQ_QOS_MAX, FREQ_QOS_MAX_DEFAULT_VALUE);
> > +				   FREQ_QOS_MAX, max_freq);
> >   	if (ret < 0)
> >   		return ret;
> >   
> > --
> > 2.43.0