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

[Viresh Kumar <viresh.kumar@linaro.org>]

On 15-06-21, 18:15, Vincent Donnefort wrote:
> On Tue, Jun 15, 2021 at 03:47:06PM +0530, Viresh Kumar wrote:
> > 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.
> 
> In the current version of this patchset, any driver can mark inefficiencies
> without relying on the EM, just by adding the flag CPUFREQ_INEFFICIENT_FREQ in
> cpufreq_frequency_table.

Yeah, I wasn't really talking about cpufreq drivers but external
entities, like EM.

> Populating cpufreq_frequency_table from the EM in cpufreq was just an attempt to
> a less intrusive set of changes.
 
> > 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.
> > 
> > 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.
> > 
> > This is the sequence that is followed in cpufreq drivers today:
> > 
> > dev_pm_opp_of_cpumask_add_table();
> > dev_pm_opp_init_cpufreq_table();
> > dev_pm_opp_of_register_em();
> > 
> > What about changing this to:
> > 
> > dev_pm_opp_of_cpumask_add_table();
> > 
> > /* Mark OPPs are inefficient here */
> > dev_pm_opp_of_register_em();
> > 
> > /* This should automatically pick the right set */
> > dev_pm_opp_init_cpufreq_table();
> > 
> > Will this break anything ?
> 
> Probably not, but with this approach I'll have to modify all the cpufreq drivers
> that are registering the EM, which I tried to avoid as much as possible so far.

Hmm. You are right as well, but I just want to get the right API in
place which lives a longer life :)

> But if we sum-up:
> 
> 1. em_dev_register_perf_domain() find inefficiencies
> 
> 2. dev_pm_opp_of_register_em() apply EM inefficiencies into the OPP structures

I was looking to add a new API to the OPP core
(dev_pm_opp_mark_inefficient()) to mark an OPP inefficient. And then
get it called from em_create_perf_table().

But I now see that EM core rather has callbacks to call into and with
that I think you should rather add another callback
(.mark_inefficient()) in struct em_data_callback, to set inefficient
frequencies.

>    Note: scmi-cpufreq would need special treatment, as it doesn't rely
>    dev_pm_opp_of_register_em().

For both dev_pm_opp_of_register_em() and scmi case, you can then set
this callback to dev_pm_opp_mark_inefficient().

> 3. dev_pm_opp_init_cpufreq_table() marks the cpufreq table with the OPP
>    inefficiencies 

Yes.

> Guess it would ease the adoption by other OPP clients. However this patchset
> will clearly get bigger.
> 
> Would you agree with that?

Yes, it is fine.

-- 
viresh