[v5,7/8] cpufreq: Read inefficiencies from EM

The Energy Model has a 1:1 mapping between OPPs and performance states
(em_perf_state). If a CPUFreq driver registers an Energy Model,
inefficiencies found by the latter can be applied to CPUFreq.

This applies to all drivers using the generic callback
cpufreq_register_em_with_opp() for .register_em.

Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>

On 26-08-21, 10:54, Vincent Donnefort wrote:
> On Thu, Aug 26, 2021 at 03:16:49PM +0530, Viresh Kumar wrote:
> > On 26-08-21, 09:35, Vincent Donnefort wrote:
> > > The Energy Model has a 1:1 mapping between OPPs and performance states
> > > (em_perf_state). If a CPUFreq driver registers an Energy Model,
> > > inefficiencies found by the latter can be applied to CPUFreq.
> > > 
> > > This applies to all drivers using the generic callback
> > > cpufreq_register_em_with_opp() for .register_em.
> > > 
> > > Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
> > > 
> > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > > index 2554dd1ec09d..50bf38ea2539 100644
> > > --- a/include/linux/cpufreq.h
> > > +++ b/include/linux/cpufreq.h
> > > @@ -1104,9 +1104,38 @@ void cpufreq_generic_init(struct cpufreq_policy *policy,
> > >  		struct cpufreq_frequency_table *table,
> > >  		unsigned int transition_latency);
> > >  
> > > +static inline void
> > > +cpufreq_read_inefficiencies_from_em(struct cpufreq_policy *policy,
> > > +				    struct em_perf_domain *em_pd)
> > > +{
> > > +	struct em_perf_state *em_table;
> > > +	int i;
> > > +
> > > +	if (!em_pd)
> > > +		return;
> > > +
> > > +	/* Inefficiencies support needs a sorted table */
> > > +	if (!policy->freq_table ||
> > > +	    policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED)
> > > +		return;
> > > +
> > > +	em_table = em_pd->table;
> > > +
> > > +	for (i = 0; i < em_pd->nr_perf_states; i++) {
> > > +		if (!(em_table[i].flags & EM_PERF_STATE_INEFFICIENT))
> > > +			continue;
> > > +
> > > +		cpufreq_table_set_inefficient(policy,
> > > +					      em_table[i].frequency);
> > > +		em_pd->flags |= EM_PERF_DOMAIN_SKIP_INEFFICIENCIES;
> > > +	}
> > > +}
> > > +
> > >  static inline void cpufreq_register_em_with_opp(struct cpufreq_policy *policy)
> > >  {
> > > -	dev_pm_opp_of_register_em(get_cpu_device(policy->cpu),
> > > -				  policy->related_cpus);
> > > +	struct device *cpu_dev = get_cpu_device(policy->cpu);
> > > +
> > > +	dev_pm_opp_of_register_em(cpu_dev, policy->related_cpus);
> > > +	cpufreq_read_inefficiencies_from_em(policy, em_pd_get(cpu_dev));
> > 
> > This should happen from em_dev_register_perf_domain() instead of here.
> 
> Shall we then let em_dev_register_perf_domain() call
> cpufreq_table_update_efficiencies() too?

It should be better IMO, instead of hardcoding it in cpufreq_online(). This also
allows the same to be done irrespective of the initialization order of the
policy. i.e. some other framework can call it at a later point as well, not just
at the beginning.

> The complete interface for ineffiencies in CPUfreq would then be:
> 
>  1. Mark a frequency as inefficient with cpufreq_table_set_inefficient()
>  2. Update the table with cpufreq_table_update_efficiencies() 

Yeah, it will just provide the helpers.

On 26-08-21, 11:35, Vincent Donnefort wrote:
> But it looks like a weird back and forth between EM and CPUFreq:
> 
>  cpufreq driver -> register EM -> sets inefficiencies back into cpufreq.
> 
> It creates a dependency loop:  cpufreq -> EM -> cpufreq
> 
> While this version is more straightforward:
> 
>  cpufreq driver -> register EM
>                 -> apply inefficiencies from EM into cpufreq.

The problem is there there is no good place for cpufreq core to call
cpufreq_update_efficiencies(). It may look correct to call it right after the EM
callback is called in the current scenario, but it isn't.

As I said earlier, there can be other stuff later on which can set inefficient
frequencies, not just EM. So they better make these calls.

> Also, I'm not sure how em_dev_register_perf_domain() can access the cpufreq
> policy while the latter hasn't finished initialized.

It is initialized enough at this point so that you can call cpufreq_cpu_get() to
get the policy, that was the whole point of my series which added register_em().