Re: [PATCH v3 3/6] cpufreq: Add an interface to mark inefficient frequencies

[Quentin Perret <qperret@google.com>]

On Tuesday 15 Jun 2021 at 15:47:06 (+0530), Viresh Kumar wrote:
> On 15-06-21, 09:44, Vincent Donnefort wrote:
> > The cpufreq_policy is accessed through percpu data. I originally tried to get it
> > with cpufreq_cpu_get_raw(). But when we init the cpufreq driver (and by
> > extension when we call em_dev_register_perf_domain()), the percpu data isn't
> > updated yet.
> 
> Right.
> 
> > I guess the only way at that moment would be to propagate the cpufreq_policy
> > pointer through the functions parameters, which is a bit cumbersome, especially
> > as the EM can be used with devfreq as well and that em_dev_register_perf_domain()
> > can be called from dev_pm_opp_of_register_em()
> > 
> > Would we be ok with that?
> 
> No.
> 
> You only talked about freq_table earlier and that's all I checked :)
> 
> > I could use em_data_callback as well ... but that doesn't change the fact some
> > registration is going through dev_pm_opp_of_register_em() which has no knowledge
> > about the cpufreq_policy. Doesn't look a better approach.
> 
> The point is that I don't want cpufreq to carry this for users, we
> have EM today, tomorrow we may want to mark a frequency as inefficient
> from somewhere else. The call need to initiate from EM core.
> 
> And this isn't a cpufreq only thing, but is going to be generic along
> with other device types.
>
> This is exactly why I asked you earlier to play with OPP core for
> this. That is the central place for data for all such users.

The thing is, the very reason for the existence of PM_EM in the first
place is to NOT depend on PM_OPP which was deemed too Arm-specific. So
let's not create those dependencies now please.

> If this information is present at the OPP table (somehow), then we can
> just fix dev_pm_opp_init_cpufreq_table() to set this for cpufreq core
> as well.

Honnestly, I don't think PM_OPP should have anything to do with this,
for the reason mentionned above.

I don't really see the problem with cpufreq core querying the EM data
directly, we already have core subsystems doing that (the scheduler) so
why would that be a problem? If the only concern is for non-CPU devices,
all we'd need is a matching feature in devfreq core and everything
should be good no?

Thanks,
Quentin