[PATCH v5 0/8] inefficient OPPs

[PATCH v5 0/8] inefficient OPPs

[Vincent Donnefort]

Hi all,

Here's the new version for the inefficient OPPs. This patch-set is based on the
following series from Viresh:

  [PATCH V3 0/9] Add callback to register with energy model
  https://lore.kernel.org/linux-arm-msm/cover.1628742634.git.viresh.kumar@linaro.org/

The main changes are:

 1. The EM inefficiencies reading is now done in the .register_em callback,
    introduced by the patch-set above.

 2. Inefficiencies will be skipped for all governors declaring
    CPUFREQ_GOV_DYNAMIC_SWITCHING, no matter the relation.

A bit of context:

We (Power team in Arm) are working with an experimental kernel for the
Google's Pixel4 to evaluate and improve the current mainline performance
and energy consumption on a real life device with Android.

The SD855 SoC found in this phone has several OPPs that are inefficient.
I.e. despite a lower frequency, they have a greater cost. (That cost being
fmax * OPP power / OPP freq). This issue is twofold. First of course,
running a specific workload at an inefficient OPP is counterproductive
since it wastes wasting energy. But also, inefficient OPPs make a
performance domain less appealing for task placement than it really is.

We evaluated the change presented here by running 30 iterations of Android
PCMark "Work 2.0 Performance". While we did not see any statistically
significant performance impact, this change allowed to drastically improve
the idle time residency.


                           |   Running   |  WFI [1]  |    Idle   |
   ------------------------+-------------+-----------+-----------+
   Little cluster (4 CPUs) |    -0.35%   |   +0.35%  |   +0.79%  |
   ------------------------+-------------+-----------+-----------+
   Medium cluster (3 CPUs) |    -6.3%    |    -18%   |    +12%   |
   ------------------------+-------------+-----------+-----------+
   Big cluster    (1 CPU)  |    -6.4%    |    -6.5%  |    +2.8%  |
   ------------------------+-------------+-----------+-----------+

On the SD855, the inefficient OPPs are found on the little cluster. By
removing them from the Energy Model, we make the most efficient CPUs more
appealing for task placement, helping to reduce the running time for the
medium and big CPUs. Increasing idle time is crucial for this platform due
to the substantial energy cost differences among the clusters. Also,
despite not appearing in the statistics (the idle driver used here doesn't
report it), we can speculate that we also improve the cluster idle time.

[1] WFI: Wait for interrupt.

Changelog since v4:
  - Remove CPUFREQ_RELATION_E.
  - Skip inefficient OPPs for all governors with CPUFREQ_GOV_DYNAMIC_SWITCHING
  - Remove CPUFREQ_READ_ENERGY_MODEL in favor of the register_em callback.

Changelog since v3:
  - New freq-table relation CPUFREQ_RELATION_E.
  - New CPUFreq driver flag CPUFREQ_READ_ENERGY_MODEL.
  - EM flag to skip or not inefficiencies (driven by CPUFreq).
  - Fix infinite loop in set_freq_table_efficiencies().

Changelog since v2:
  - Add separated support for inefficiencies into CPUFreq.
  - Collect Reviewed-by for the first patch.

Changelog since v1:
  - Remove the Look-up table as the numbers weren't strong enough to

Vincent Donnefort (8):
  PM / EM: Fix inefficient states detection
  PM / EM: Mark inefficient states
  PM / EM: Extend em_perf_domain with a flag field
  PM / EM: Allow skipping inefficient states
  cpufreq: Add an interface to mark inefficient frequencies
  cpufreq: Skip inefficient frequencies
  cpufreq: Read inefficiencies from EM
  cpufreq: scmi: Read inefficiencies from EM

 drivers/cpufreq/cpufreq.c      | 13 ++++++
 drivers/cpufreq/freq_table.c   | 46 +++++++++++++++++++++
 drivers/cpufreq/scmi-cpufreq.c |  7 ++--
 include/linux/cpufreq.h        | 90 +++++++++++++++++++++++++++++++++++++++---
 include/linux/energy_model.h   | 68 ++++++++++++++++++++++++++-----
 kernel/power/energy_model.c    | 46 ++++++++++++---------
 6 files changed, 234 insertions(+), 36 deletions(-)

-- 
2.7.4

[PATCH v5 1/8] PM / EM: Fix inefficient states detection

[Vincent Donnefort]

Currently, a debug message is printed if an inefficient state is detected
in the Energy Model. Unfortunately, it won't detect if the first state is
inefficient or if two successive states are. Fix this behavior.

Fixes: 27871f7a8a34 (PM: Introduce an Energy Model management framework)
Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Reviewed-by: Quentin Perret <qperret@google.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index a332ccd829e2..97e62469a6b3 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -107,8 +107,7 @@ static void em_debug_remove_pd(struct device *dev) {}
 static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
 				int nr_states, struct em_data_callback *cb)
 {
-	unsigned long opp_eff, prev_opp_eff = ULONG_MAX;
-	unsigned long power, freq, prev_freq = 0;
+	unsigned long power, freq, prev_freq = 0, prev_cost = ULONG_MAX;
 	struct em_perf_state *table;
 	int i, ret;
 	u64 fmax;
@@ -153,27 +152,21 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
 
 		table[i].power = power;
 		table[i].frequency = prev_freq = freq;
-
-		/*
-		 * The hertz/watts efficiency ratio should decrease as the
-		 * frequency grows on sane platforms. But this isn't always
-		 * true in practice so warn the user if a higher OPP is more
-		 * power efficient than a lower one.
-		 */
-		opp_eff = freq / power;
-		if (opp_eff >= prev_opp_eff)
-			dev_dbg(dev, "EM: hertz/watts ratio non-monotonically decreasing: em_perf_state %d >= em_perf_state%d\n",
-					i, i - 1);
-		prev_opp_eff = opp_eff;
 	}
 
 	/* Compute the cost of each performance state. */
 	fmax = (u64) table[nr_states - 1].frequency;
-	for (i = 0; i < nr_states; i++) {
+	for (i = nr_states - 1; i >= 0; i--) {
 		unsigned long power_res = em_scale_power(table[i].power);
 
 		table[i].cost = div64_u64(fmax * power_res,
 					  table[i].frequency);
+		if (table[i].cost >= prev_cost) {
+			dev_dbg(dev, "EM: OPP:%lu is inefficient\n",
+				table[i].frequency);
+		} else {
+			prev_cost = table[i].cost;
+		}
 	}
 
 	pd->table = table;
-- 
2.7.4

[PATCH v5 2/8] PM / EM: Mark inefficient states

[Vincent Donnefort]

Some SoCs, such as the sd855 have OPPs within the same performance domain,
whose cost is higher than others with a higher frequency. Even though
those OPPs are interesting from a cooling perspective, it makes no sense
to use them when the device can run at full capacity. Those OPPs handicap
the performance domain, when choosing the most energy-efficient CPU and
are wasting energy. They are inefficient.

Hence, add support for such OPPs to the Energy Model. The table can now
be read skipping inefficient performance states (and by extension,
inefficient OPPs).

Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index 1834752c5617..629f5f63a7d7 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -17,13 +17,25 @@
  *		device). It can be a total power: static and dynamic.
  * @cost:	The cost coefficient associated with this level, used during
  *		energy calculation. Equal to: power * max_frequency / frequency
+ * @flags:	see "em_perf_state flags" description below.
  */
 struct em_perf_state {
 	unsigned long frequency;
 	unsigned long power;
 	unsigned long cost;
+	unsigned long flags;
 };
 
+/*
+ * em_perf_state flags:
+ *
+ * EM_PERF_STATE_INEFFICIENT: The performance state is inefficient. There is
+ * in this em_perf_domain, another performance state with a higher frequency
+ * but a lower or equal power cost. Such inefficient states are ignored when
+ * using em_pd_get_efficient_*() functions.
+ */
+#define EM_PERF_STATE_INEFFICIENT BIT(0)
+
 /**
  * em_perf_domain - Performance domain
  * @table:		List of performance states, in ascending order
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 97e62469a6b3..6d8438347535 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -2,7 +2,7 @@
 /*
  * Energy Model of devices
  *
- * Copyright (c) 2018-2020, Arm ltd.
+ * Copyright (c) 2018-2021, Arm ltd.
  * Written by: Quentin Perret, Arm ltd.
  * Improvements provided by: Lukasz Luba, Arm ltd.
  */
@@ -42,6 +42,7 @@ static void em_debug_create_ps(struct em_perf_state *ps, struct dentry *pd)
 	debugfs_create_ulong("frequency", 0444, d, &ps->frequency);
 	debugfs_create_ulong("power", 0444, d, &ps->power);
 	debugfs_create_ulong("cost", 0444, d, &ps->cost);
+	debugfs_create_ulong("inefficient", 0444, d, &ps->flags);
 }
 
 static int em_debug_cpus_show(struct seq_file *s, void *unused)
@@ -162,6 +163,7 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd,
 		table[i].cost = div64_u64(fmax * power_res,
 					  table[i].frequency);
 		if (table[i].cost >= prev_cost) {
+			table[i].flags = EM_PERF_STATE_INEFFICIENT;
 			dev_dbg(dev, "EM: OPP:%lu is inefficient\n",
 				table[i].frequency);
 		} else {
-- 
2.7.4

[PATCH v5 3/8] PM / EM: Extend em_perf_domain with a flag field

[Vincent Donnefort]

Merge the current "milliwatts" option into a "flag" field. This intends to
prepare the extension of this structure for inefficient states support in
the Energy Model.

Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index 629f5f63a7d7..ac2f7d0ab946 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -40,8 +40,7 @@ struct em_perf_state {
  * em_perf_domain - Performance domain
  * @table:		List of performance states, in ascending order
  * @nr_perf_states:	Number of performance states
- * @milliwatts:		Flag indicating the power values are in milli-Watts
- *			or some other scale.
+ * @flags:		See "em_perf_domain flags"
  * @cpus:		Cpumask covering the CPUs of the domain. It's here
  *			for performance reasons to avoid potential cache
  *			misses during energy calculations in the scheduler
@@ -56,10 +55,18 @@ struct em_perf_state {
 struct em_perf_domain {
 	struct em_perf_state *table;
 	int nr_perf_states;
-	int milliwatts;
+	unsigned long flags;
 	unsigned long cpus[];
 };
 
+/*
+ *  em_perf_domain flags:
+ *
+ *  EM_PERF_DOMAIN_MILLIWATTS: The power values are in milli-Watts or some
+ *  other scale.
+ */
+#define EM_PERF_DOMAIN_MILLIWATTS BIT(0)
+
 #define em_span_cpus(em) (to_cpumask((em)->cpus))
 
 #ifdef CONFIG_ENERGY_MODEL
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 6d8438347535..3a7d1573b214 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -56,7 +56,8 @@ DEFINE_SHOW_ATTRIBUTE(em_debug_cpus);
 static int em_debug_units_show(struct seq_file *s, void *unused)
 {
 	struct em_perf_domain *pd = s->private;
-	char *units = pd->milliwatts ? "milliWatts" : "bogoWatts";
+	char *units = (pd->flags & EM_PERF_DOMAIN_MILLIWATTS) ?
+		"milliWatts" : "bogoWatts";
 
 	seq_printf(s, "%s\n", units);
 
@@ -330,7 +331,8 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
 	if (ret)
 		goto unlock;
 
-	dev->em_pd->milliwatts = milliwatts;
+	if (milliwatts)
+		dev->em_pd->flags |= EM_PERF_DOMAIN_MILLIWATTS;
 
 	em_debug_create_pd(dev);
 	dev_info(dev, "EM: created perf domain\n");
-- 
2.7.4

[PATCH v5 4/8] PM / EM: Allow skipping inefficient states

[Vincent Donnefort]

The new performance domain flag EM_PERF_DOMAIN_SKIP_INEFFICIENCIES allows
to not take into account inefficient states when estimating energy
consumption. This intends to let the Energy Model know that CPUFreq itself
will skip inefficiencies and such states don't need to be part of the
estimation anymore.

Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index ac2f7d0ab946..15d41f09f009 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -64,8 +64,12 @@ struct em_perf_domain {
  *
  *  EM_PERF_DOMAIN_MILLIWATTS: The power values are in milli-Watts or some
  *  other scale.
+ *
+ *  EM_PERF_DOMAIN_SKIP_INEFFICIENCIES: Skip inefficient states when estimating
+ *  energy consumption.
  */
 #define EM_PERF_DOMAIN_MILLIWATTS BIT(0)
+#define EM_PERF_DOMAIN_SKIP_INEFFICIENCIES BIT(1)
 
 #define em_span_cpus(em) (to_cpumask((em)->cpus))
 
@@ -121,6 +125,37 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
 void em_dev_unregister_perf_domain(struct device *dev);
 
 /**
+ * em_pd_get_efficient_state() - Get an efficient performance state from the EM
+ * @pd   : Performance domain for which we want an efficient frequency
+ * @freq : Frequency to map with the EM
+ *
+ * It is called from the scheduler code quite frequently and as a consequence
+ * doesn't implement any check.
+ *
+ * Return: An efficient performance state, high enough to meet @freq
+ * requirement.
+ */
+static inline
+struct em_perf_state *em_pd_get_efficient_state(struct em_perf_domain *pd,
+						unsigned long freq)
+{
+	struct em_perf_state *ps;
+	int i;
+
+	for (i = 0; i < pd->nr_perf_states; i++) {
+		ps = &pd->table[i];
+		if (ps->frequency >= freq) {
+			if (pd->flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
+			    ps->flags & EM_PERF_STATE_INEFFICIENT)
+				continue;
+			break;
+		}
+	}
+
+	return ps;
+}
+
+/**
  * em_cpu_energy() - Estimates the energy consumed by the CPUs of a
 		performance domain
  * @pd		: performance domain for which energy has to be estimated
@@ -142,7 +177,7 @@ static inline unsigned long em_cpu_energy(struct em_perf_domain *pd,
 {
 	unsigned long freq, scale_cpu;
 	struct em_perf_state *ps;
-	int i, cpu;
+	int cpu;
 
 	if (!sum_util)
 		return 0;
@@ -167,11 +202,7 @@ static inline unsigned long em_cpu_energy(struct em_perf_domain *pd,
 	 * Find the lowest performance state of the Energy Model above the
 	 * requested frequency.
 	 */
-	for (i = 0; i < pd->nr_perf_states; i++) {
-		ps = &pd->table[i];
-		if (ps->frequency >= freq)
-			break;
-	}
+	ps = em_pd_get_efficient_state(pd, freq);
 
 	/*
 	 * The capacity of a CPU in the domain at the performance state (ps)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index 3a7d1573b214..d353ef29e37f 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -65,6 +65,17 @@ static int em_debug_units_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(em_debug_units);
 
+static int em_debug_skip_inefficiencies_show(struct seq_file *s, void *unused)
+{
+	struct em_perf_domain *pd = s->private;
+	int enabled = (pd->flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES) ? 1 : 0;
+
+	seq_printf(s, "%d\n", enabled);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(em_debug_skip_inefficiencies);
+
 static void em_debug_create_pd(struct device *dev)
 {
 	struct dentry *d;
@@ -78,6 +89,8 @@ static void em_debug_create_pd(struct device *dev)
 				    &em_debug_cpus_fops);
 
 	debugfs_create_file("units", 0444, d, dev->em_pd, &em_debug_units_fops);
+	debugfs_create_file("skip-inefficiencies", 0444, d, dev->em_pd,
+			    &em_debug_skip_inefficiencies_fops);
 
 	/* Create a sub-directory for each performance state */
 	for (i = 0; i < dev->em_pd->nr_perf_states; i++)
-- 
2.7.4

[PATCH v5 5/8] cpufreq: Add an interface to mark inefficient frequencies

[Vincent Donnefort]

Some SoCs such as the sd855 have OPPs within the same policy whose cost is
higher than others with a higher frequency. Those OPPs are inefficients
and it might be interesting for a governor to not use them.

Adding a flag, CPUFREQ_INEFFICIENT_FREQ, to mark such OPPs into the
frequency table, as well as a new cpufreq_frequency_table member
"efficient". This new member allows CPUFreq to resolve an inefficient
frequency to an efficient one.

Efficient frequencies point to themselves. The efficiency resolution must
check it doesn't break the policy maximum.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 7d5f170ecad1..64ff3836955d 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1504,6 +1504,8 @@ static int cpufreq_online(unsigned int cpu)
 		 */
 		if (cpufreq_driver->register_em)
 			cpufreq_driver->register_em(policy);
+
+		cpufreq_table_update_efficiencies(policy);
 	}
 
 	ret = cpufreq_init_policy(policy);
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 67e56cf638ef..c43320c57067 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -365,6 +365,52 @@ int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy)
 	return set_freq_table_sorted(policy);
 }
 
+void cpufreq_table_update_efficiencies(struct cpufreq_policy *policy)
+{
+	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
+	enum cpufreq_table_sorting sort = policy->freq_table_sorted;
+	int efficient, idx;
+
+	/* Not supported */
+	if (sort == CPUFREQ_TABLE_UNSORTED) {
+		cpufreq_for_each_entry_idx(pos, table, idx)
+			pos->efficient = idx;
+		return;
+	}
+
+	/* The highest frequency is always efficient */
+	cpufreq_for_each_entry_idx(pos, table, idx) {
+		if (pos->frequency == CPUFREQ_ENTRY_INVALID)
+			continue;
+
+		efficient = idx;
+
+		if (sort == CPUFREQ_TABLE_SORTED_DESCENDING)
+			break;
+	}
+
+	for (;;) {
+		pos = &table[idx];
+
+		if (pos->frequency != CPUFREQ_ENTRY_INVALID) {
+			if (pos->flags & CPUFREQ_INEFFICIENT_FREQ) {
+				pos->efficient = efficient;
+			} else {
+				pos->efficient = idx;
+				efficient = idx;
+			}
+		}
+
+		if (sort == CPUFREQ_TABLE_SORTED_ASCENDING) {
+			if (--idx < 0)
+				break;
+		} else {
+			if (table[++idx].frequency == CPUFREQ_TABLE_END)
+				break;
+		}
+	}
+}
+
 MODULE_AUTHOR("Dominik Brodowski <linux@brodo.de>");
 MODULE_DESCRIPTION("CPUfreq frequency table helpers");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index c65a1d7385f8..4659921173f7 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -664,13 +664,15 @@ struct governor_attr {
 #define CPUFREQ_ENTRY_INVALID	~0u
 #define CPUFREQ_TABLE_END	~1u
 /* Special Values of .flags field */
-#define CPUFREQ_BOOST_FREQ	(1 << 0)
+#define CPUFREQ_BOOST_FREQ	 (1 << 0)
+#define CPUFREQ_INEFFICIENT_FREQ (1 << 1)
 
 struct cpufreq_frequency_table {
 	unsigned int	flags;
 	unsigned int	driver_data; /* driver specific data, not used by core */
 	unsigned int	frequency; /* kHz - doesn't need to be in ascending
 				    * order */
+	unsigned int	efficient; /* idx of an efficient frequency */
 };
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
@@ -1003,6 +1005,20 @@ static inline int cpufreq_table_count_valid_entries(const struct cpufreq_policy
 
 	return count;
 }
+
+static inline void
+cpufreq_table_set_inefficient(const struct cpufreq_policy *policy,
+			      unsigned int frequency)
+{
+	struct cpufreq_frequency_table *pos;
+
+	cpufreq_for_each_valid_entry(pos, policy->freq_table) {
+		if (pos->frequency == frequency) {
+			pos->flags |= CPUFREQ_INEFFICIENT_FREQ;
+			break;
+		}
+	}
+}
 #else
 static inline int cpufreq_boost_trigger_state(int state)
 {
@@ -1022,6 +1038,10 @@ static inline bool policy_has_boost_freq(struct cpufreq_policy *policy)
 {
 	return false;
 }
+
+static inline int
+cpufreq_table_set_inefficient(const struct cpufreq_policy *policy,
+			      unsigned int frequency) {}
 #endif
 
 #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)
@@ -1049,6 +1069,7 @@ extern struct freq_attr cpufreq_freq_attr_scaling_available_freqs;
 extern struct freq_attr cpufreq_freq_attr_scaling_boost_freqs;
 extern struct freq_attr *cpufreq_generic_attr[];
 int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy);
+void cpufreq_table_update_efficiencies(struct cpufreq_policy *policy);
 
 unsigned int cpufreq_generic_get(unsigned int cpu);
 void cpufreq_generic_init(struct cpufreq_policy *policy,
-- 
2.7.4

[PATCH v5 6/8] cpufreq: Skip inefficient frequencies

[Vincent Donnefort]

CPUFreq governors that do DVFS (i.e. CPUFREQ_GOV_DYNAMIC_SWITCHING flag)
can skip frequencies marked as inefficient, as long as the efficient
frequency found meet the policy maximum requirement.

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

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 64ff3836955d..936b92a09785 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2299,6 +2299,7 @@ __weak struct cpufreq_governor *cpufreq_fallback_governor(void)
 
 static int cpufreq_init_governor(struct cpufreq_policy *policy)
 {
+	struct cpufreq_frequency_table *pos;
 	int ret;
 
 	/* Don't start any governor operations if we are entering suspend */
@@ -2340,6 +2341,16 @@ static int cpufreq_init_governor(struct cpufreq_policy *policy)
 
 	policy->strict_target = !!(policy->governor->flags & CPUFREQ_GOV_STRICT_TARGET);
 
+	policy->skip_inefficiencies = false;
+	if (policy->governor->flags & CPUFREQ_GOV_DYNAMIC_SWITCHING) {
+		cpufreq_for_each_valid_entry(pos, policy->freq_table) {
+			if (pos->flags & CPUFREQ_INEFFICIENT_FREQ) {
+				policy->skip_inefficiencies = true;
+				break;
+			}
+		}
+	}
+
 	return 0;
 }
 
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 4659921173f7..2554dd1ec09d 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -117,6 +117,13 @@ struct cpufreq_policy {
 	bool			strict_target;
 
 	/*
+	 * Set if the CPUFREQ_GOV_DYNAMIC_SWITCHING flag is set for the current
+	 * governor and if inefficient frequencies were found in the frequency
+	 * table.
+	 */
+	bool			skip_inefficiencies;
+
+	/*
 	 * Preferred average time interval between consecutive invocations of
 	 * the driver to set the frequency for this policy.  To be set by the
 	 * scaling driver (0, which is the default, means no preference).
@@ -971,25 +978,46 @@ static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
 		return cpufreq_table_find_index_dc(policy, target_freq);
 }
 
+static inline unsigned int
+cpufreq_frequency_find_efficient(struct cpufreq_policy *policy,
+				 unsigned int idx)
+{
+	struct cpufreq_frequency_table *table = policy->freq_table;
+	unsigned int efficient_idx = table[idx].efficient;
+
+	return table[efficient_idx].frequency <= policy->max ? efficient_idx :
+		idx;
+}
+
 static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
 						 unsigned int target_freq,
 						 unsigned int relation)
 {
+	int idx;
+
 	if (unlikely(policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED))
 		return cpufreq_table_index_unsorted(policy, target_freq,
 						    relation);
 
 	switch (relation) {
 	case CPUFREQ_RELATION_L:
-		return cpufreq_table_find_index_l(policy, target_freq);
+		idx = cpufreq_table_find_index_l(policy, target_freq);
+		break;
 	case CPUFREQ_RELATION_H:
-		return cpufreq_table_find_index_h(policy, target_freq);
+		idx = cpufreq_table_find_index_h(policy, target_freq);
+		break;
 	case CPUFREQ_RELATION_C:
-		return cpufreq_table_find_index_c(policy, target_freq);
+		idx = cpufreq_table_find_index_c(policy, target_freq);
+		break;
 	default:
 		WARN_ON_ONCE(1);
 		return 0;
 	}
+
+	if (policy->skip_inefficiencies)
+		idx = cpufreq_frequency_find_efficient(policy, idx);
+
+	return idx;
 }
 
 static inline int cpufreq_table_count_valid_entries(const struct cpufreq_policy *policy)
-- 
2.7.4

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

[Vincent Donnefort]

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));
 }
 #endif /* _LINUX_CPUFREQ_H */
-- 
2.7.4

[PATCH v5 8/8] cpufreq: scmi: Read inefficiencies from EM

[Vincent Donnefort]

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.

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

diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index 1e0cd4d165f0..f4fd634d2ecd 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -250,6 +250,7 @@ static void scmi_cpufreq_register_em(struct cpufreq_policy *policy)
 {
 	struct em_data_callback em_cb = EM_DATA_CB(scmi_get_cpu_power);
 	bool power_scale_mw = perf_ops->power_scale_mw_get(ph);
+	struct device *cpu_dev = get_cpu_device(policy->cpu);
 	struct scmi_data *priv = policy->driver_data;
 
 	/*
@@ -262,9 +263,9 @@ static void scmi_cpufreq_register_em(struct cpufreq_policy *policy)
 	if (!priv->nr_opp)
 		return;
 
-	em_dev_register_perf_domain(get_cpu_device(policy->cpu), priv->nr_opp,
-				    &em_cb, priv->opp_shared_cpus,
-				    power_scale_mw);
+	em_dev_register_perf_domain(cpu_dev, priv->nr_opp, &em_cb,
+				    priv->opp_shared_cpus, power_scale_mw);
+	cpufreq_read_inefficiencies_from_em(policy, em_pd_get(cpu_dev));
 }
 
 static struct cpufreq_driver scmi_cpufreq_driver = {
-- 
2.7.4

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

[Viresh Kumar]

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.

>  }
>  #endif /* _LINUX_CPUFREQ_H */
> -- 
> 2.7.4

-- 
viresh

Re: [PATCH v5 8/8] cpufreq: scmi: Read inefficiencies from EM

[Viresh Kumar]

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.
> 
> Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
> 
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 1e0cd4d165f0..f4fd634d2ecd 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -250,6 +250,7 @@ static void scmi_cpufreq_register_em(struct cpufreq_policy *policy)
>  {
>  	struct em_data_callback em_cb = EM_DATA_CB(scmi_get_cpu_power);
>  	bool power_scale_mw = perf_ops->power_scale_mw_get(ph);
> +	struct device *cpu_dev = get_cpu_device(policy->cpu);
>  	struct scmi_data *priv = policy->driver_data;
>  
>  	/*
> @@ -262,9 +263,9 @@ static void scmi_cpufreq_register_em(struct cpufreq_policy *policy)
>  	if (!priv->nr_opp)
>  		return;
>  
> -	em_dev_register_perf_domain(get_cpu_device(policy->cpu), priv->nr_opp,
> -				    &em_cb, priv->opp_shared_cpus,
> -				    power_scale_mw);
> +	em_dev_register_perf_domain(cpu_dev, priv->nr_opp, &em_cb,
> +				    priv->opp_shared_cpus, power_scale_mw);
> +	cpufreq_read_inefficiencies_from_em(policy, em_pd_get(cpu_dev));
>  }
>  
>  static struct cpufreq_driver scmi_cpufreq_driver = {

Same here.

-- 
viresh

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

[Vincent Donnefort]

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? 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() 

-- 
Vincent

> 
> >  }
> >  #endif /* _LINUX_CPUFREQ_H */
> > -- 
> > 2.7.4
> 
> -- 
> viresh

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

[Viresh Kumar]

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.

-- 
viresh

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

[Vincent Donnefort]

On Thu, Aug 26, 2021 at 03:29:46PM +0530, Viresh Kumar wrote:
> 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.

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.

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

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

[Viresh Kumar]

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().

-- 
viresh

Re: [PATCH v5 5/8] cpufreq: Add an interface to mark inefficient frequencies

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6285 bytes --]

Hi Vincent,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210825]
[cannot apply to pm/linux-next linus/master v5.14-rc7 v5.14-rc6 v5.14-rc5 v5.14-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincent-Donnefort/inefficient-OPPs/20210826-163705
base:    7636510f976d75b860848884169ba985c8f844d8
config: i386-randconfig-r025-20210826 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project ea08c4cd1c0869ec5024a8bb3f5cdf06ab03ae83)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ed31a82b6d4abef95e8c50df33f826175cb6b6de
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincent-Donnefort/inefficient-OPPs/20210826-163705
        git checkout ed31a82b6d4abef95e8c50df33f826175cb6b6de
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from kernel/time/tick-common.c:19:
   In file included from include/trace/events/power.h:8:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   1 error generated.
--
   In file included from drivers/acpi/processor_idle.c:23:
   In file included from include/acpi/processor.h:7:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   drivers/acpi/processor_idle.c:1137:12: warning: no previous prototype for function 'acpi_processor_ffh_lpi_probe' [-Wmissing-prototypes]
   int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
              ^
   drivers/acpi/processor_idle.c:1137:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
   ^
   static 
   drivers/acpi/processor_idle.c:1142:12: warning: no previous prototype for function 'acpi_processor_ffh_lpi_enter' [-Wmissing-prototypes]
   int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
              ^
   drivers/acpi/processor_idle.c:1142:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
   ^
   static 
   2 warnings and 1 error generated.
--
   In file included from kernel/sched/topology.c:5:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/topology.c:691:6: warning: variable 'numa_distance' set but not used [-Wunused-but-set-variable]
           int numa_distance = 0;
               ^
   1 warning and 1 error generated.
--
   In file included from kernel/sched/core.c:13:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/core.c:3217:6: warning: no previous prototype for function 'sched_set_stop_task' [-Wmissing-prototypes]
   void sched_set_stop_task(int cpu, struct task_struct *stop)
        ^
   kernel/sched/core.c:3217:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void sched_set_stop_task(int cpu, struct task_struct *stop)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from kernel/sched/fair.c:23:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/fair.c:10924:6: warning: no previous prototype for function 'task_vruntime_update' [-Wmissing-prototypes]
   void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fi)
        ^
   kernel/sched/fair.c:10924:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fi)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from kernel/sched/rt.c:6:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/rt.c:669:6: warning: no previous prototype for function 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
        ^
   kernel/sched/rt.c:669:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
   ^
   static 
   1 warning and 1 error generated.


vim +1044 include/linux/cpufreq.h

  1041	
  1042	static inline int
  1043	cpufreq_table_set_inefficient(const struct cpufreq_policy *policy,
> 1044				      unsigned int frequency) {}
  1045	#endif
  1046	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29522 bytes --]

Re: [PATCH v5 5/8] cpufreq: Add an interface to mark inefficient frequencies

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6409 bytes --]

Hi Vincent,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210825]
[cannot apply to pm/linux-next linus/master v5.14-rc7 v5.14-rc6 v5.14-rc5 v5.14-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincent-Donnefort/inefficient-OPPs/20210826-163705
base:    7636510f976d75b860848884169ba985c8f844d8
config: i386-randconfig-r025-20210826 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project ea08c4cd1c0869ec5024a8bb3f5cdf06ab03ae83)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ed31a82b6d4abef95e8c50df33f826175cb6b6de
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincent-Donnefort/inefficient-OPPs/20210826-163705
        git checkout ed31a82b6d4abef95e8c50df33f826175cb6b6de
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from kernel/time/tick-common.c:19:
   In file included from include/trace/events/power.h:8:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   1 error generated.
--
   In file included from drivers/acpi/processor_idle.c:23:
   In file included from include/acpi/processor.h:7:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   drivers/acpi/processor_idle.c:1137:12: warning: no previous prototype for function 'acpi_processor_ffh_lpi_probe' [-Wmissing-prototypes]
   int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
              ^
   drivers/acpi/processor_idle.c:1137:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
   ^
   static 
   drivers/acpi/processor_idle.c:1142:12: warning: no previous prototype for function 'acpi_processor_ffh_lpi_enter' [-Wmissing-prototypes]
   int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
              ^
   drivers/acpi/processor_idle.c:1142:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
   ^
   static 
   2 warnings and 1 error generated.
--
   In file included from kernel/sched/topology.c:5:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/topology.c:691:6: warning: variable 'numa_distance' set but not used [-Wunused-but-set-variable]
           int numa_distance = 0;
               ^
   1 warning and 1 error generated.
--
   In file included from kernel/sched/core.c:13:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/core.c:3217:6: warning: no previous prototype for function 'sched_set_stop_task' [-Wmissing-prototypes]
   void sched_set_stop_task(int cpu, struct task_struct *stop)
        ^
   kernel/sched/core.c:3217:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void sched_set_stop_task(int cpu, struct task_struct *stop)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from kernel/sched/fair.c:23:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/fair.c:10924:6: warning: no previous prototype for function 'task_vruntime_update' [-Wmissing-prototypes]
   void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fi)
        ^
   kernel/sched/fair.c:10924:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fi)
   ^
   static 
   1 warning and 1 error generated.
--
   In file included from kernel/sched/rt.c:6:
   In file included from kernel/sched/sched.h:43:
>> include/linux/cpufreq.h:1044:35: error: non-void function does not return a value [-Werror,-Wreturn-type]
                                 unsigned int frequency) {}
                                                          ^
   kernel/sched/rt.c:669:6: warning: no previous prototype for function 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
        ^
   kernel/sched/rt.c:669:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
   ^
   static 
   1 warning and 1 error generated.


vim +1044 include/linux/cpufreq.h

  1041	
  1042	static inline int
  1043	cpufreq_table_set_inefficient(const struct cpufreq_policy *policy,
> 1044				      unsigned int frequency) {}
  1045	#endif
  1046	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29522 bytes --]