On Tue, Jun 15, 2021 at 10:32:11AM +0530, Viresh Kumar wrote:

On 14-06-21, 14:35, Vincent Donnefort wrote:

quoted

On Mon, Jun 14, 2021 at 12:58:35PM +0530, Viresh Kumar wrote:

quoted

On 04-06-21, 12:05, Vincent Donnefort wrote:

quoted

 int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
 {
 	int ret;

@@ -362,7 +409,13 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
 	if (ret)
 		return ret;
 
-	return set_freq_table_sorted(policy);
+	ret = set_freq_table_sorted(policy);
+	if (ret)
+		return ret;
+
+	set_freq_table_efficiencies(policy);
+
+	return ret;
 }

Lets provide a single callback from the cpufreq core to do all
settings related to efficient frequencies and call it from
em_dev_register_perf_domain() ?

Sadly we cannot do it in em_dev_register_perf_domain(). This function is called
from the cpufreq driver init, when the table isn't available yet.

I looked at all the users of em_dev_register_perf_domain() and each
one of them have the freq table ready at that point of time.

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.

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?

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.

quoted

quoted

So we only do them for the cpufreq drivers that register themselves
with EM.

Currently, only the EM would set those inefficient OPPs. But it also gives the
possibility for individual drivers to set them, even if they do not use the EM.

This is exactly why I want those parts of the kernel to call a
specific API to get this done. This shouldn't be done automatically by
the cpufreq core.

-- 
viresh