On Thu, Aug 26, 2021 at 03:29:46PM +0530, Viresh Kumar wrote:

On 26-08-21, 10:54, Vincent Donnefort wrote:

quoted

On Thu, Aug 26, 2021 at 03:16:49PM +0530, Viresh Kumar wrote:

quoted

On 26-08-21, 09:35, Vincent Donnefort wrote:

quoted

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 <redacted>

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.

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.

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

quoted

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.

-- 
viresh