Re: [PATCH 06/15] cpufreq: cppc: Set policy->boost_supported

[Viresh Kumar]

On 06-02-25, 11:58, zhenglifeng (A) wrote:
> On 2025/1/24 16:58, Viresh Kumar wrote:
> > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > index 7fa89b601d2a..08117fb9c1eb 100644
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -34,8 +34,6 @@
> >   */
> >  static LIST_HEAD(cpu_data_list);
> >  
> > -static bool boost_supported;
> > -
> >  static struct cpufreq_driver cppc_cpufreq_driver;
> >  
> >  #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
> > @@ -653,7 +651,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
> >  	 * is supported.
> >  	 */
> >  	if (caps->highest_perf > caps->nominal_perf)
> > -		boost_supported = true;
> > +		policy->boost_supported = true;
> >  
> >  	/* Set policy->cur to max now. The governors will adjust later. */
> >  	policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
> > @@ -791,11 +789,6 @@ static int cppc_cpufreq_set_boost(struct cpufreq_policy *policy, int state)
> >  	struct cppc_perf_caps *caps = &cpu_data->perf_caps;
> >  	int ret;
> >  
> > -	if (!boost_supported) {
> > -		pr_err("BOOST not supported by CPU or firmware\n");
> > -		return -EINVAL;
> > -	}
> > -
> >  	if (state)
> >  		policy->max = cppc_perf_to_khz(caps, caps->highest_perf);
> >  	else
> 
> I have a little question. With the old boost_supported flag as false, it
> will fail when you operate the global boost flag. But if you replace it
> with the per-policy boost_supported flag, the global boost_enabled flag can
> be set to true without any error, even no policy's boost_enabled flag
> changed. Is this interface behavior change OK?

Right, so earlier even if a single policy didn't support boost, the code disabled
boost for all of them. Or it was rather racy, as the last policy to be
initialized will decide if boost will be supported or not. This is surely
incorrect.

The global boost flag can be enabled disabled without worrying about any
individual policy. If the policy supports boost, it will follow the global boost
here, else it shouldn't take part in the change.

So yes, the new interface does the right thing here.

-- 
viresh