[PATCH v6 0/8] CPUs capacity information for heterogeneous systems

[PATCH v6 0/8] CPUs capacity information for heterogeneous systems

[Juri Lelli]

Hi all,

version 6 of "CPUs capacity information for heterogeneous systems" patchset [1]
(please refer to previous postings to get some context).

A few changes from v5:

 - rebase on top of mainline (4.7-rc6)
 - documentation fixes
 - kcalloc() for raw_capacity array
 - pr_err() only for partial updates
 - cpu_scale_mutex to protect sysfs interface

Patches high level description:

 o 01/08 introduces documentation for the new optional DT binding
 o [02-06]/08 add cpu-capacity attribute to TC2, Juno and Juno r2 DTs and
   provide parsing of such information at boot time
 o [07-08]/08 introduce sysfs attribute

In case you would like to test this out, I pushed a branch here:

 git://linux-arm.org/linux-jl.git upstream/default_caps_v6

This branch contains additional patches, useful to better understand how CPU
capacity information is actually used by the scheduler.

These patches also form the basis for Morten/Dietmar's "Clean-ups and
asymmetric cpu capacity support" series [2] (and the EAS stack in general).

Best,

- Juri

[1] v1 - https://lkml.org/lkml/2015/11/23/391
    v2 - https://lkml.org/lkml/2016/1/8/417
    v3 - https://lkml.org/lkml/2016/2/3/405
    v4 - https://lkml.org/lkml/2016/3/18/350
    v5 - https://lkml.org/lkml/2016/6/15/291
[2] https://lkml.org/lkml/2016/6/22/614 

Juri Lelli (8):
  Documentation: arm: define DT cpu capacity-dmips-mhz bindings
  arm: parse cpu capacity-dmips-mhz from DT
  arm, dts: add TC2 cpu capacity-dmips-mhz information
  arm64: parse cpu capacity-dmips-mhz from DT
  arm64, dts: add Juno cpu capacity-dmips-mhz information
  arm64, dts: add Juno r2 cpu capacity-dmips-mhz information
  arm: add sysfs cpu_capacity attribute
  arm64: add sysfs cpu_capacity attribute

 .../devicetree/bindings/arm/cpu-capacity.txt       | 236 +++++++++++++++++++++
 Documentation/devicetree/bindings/arm/cpus.txt     |  10 +
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts         |   5 +
 arch/arm/kernel/topology.c                         | 218 ++++++++++++++++++-
 arch/arm64/boot/dts/arm/juno-r2.dts                |   6 +
 arch/arm64/boot/dts/arm/juno.dts                   |   6 +
 arch/arm64/kernel/topology.c                       | 218 +++++++++++++++++++
 7 files changed, 698 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/cpu-capacity.txt

-- 
2.7.0

[PATCH v6 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings

[Juri Lelli]

ARM systems may be configured to have cpus with different power/performance
characteristics within the same chip. In this case, additional information
has to be made available to the kernel (the scheduler in particular) for it
to be aware of such differences and take decisions accordingly.

Therefore, this patch aims at standardizing cpu capacities device tree
bindings for ARM platforms. Bindings define cpu capacity-dmips-mhz
parameter, to allow operating systems to retrieve such information from
the device tree and initialize related kernel structures, paving the way
for common code in the kernel to deal with heterogeneity.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: devicetree at vger.kernel.org
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v1:
 - removed section regarding capacity-scale
 - added information regarding normalization

Changes from v4:
 - binding changed to capacity-dmips-mhz
 - sections and changelod updated accordingly

Changes from v5:
 - addressed Mark and Vincent comments
---
 .../devicetree/bindings/arm/cpu-capacity.txt       | 236 +++++++++++++++++++++
 Documentation/devicetree/bindings/arm/cpus.txt     |  10 +
 2 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cpu-capacity.txt

diff --git a/Documentation/devicetree/bindings/arm/cpu-capacity.txt b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
new file mode 100644
index 000000000000..7809fbe0cdb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
@@ -0,0 +1,236 @@
+==========================================
+ARM CPUs capacity bindings
+==========================================
+
+==========================================
+1 - Introduction
+==========================================
+
+ARM systems may be configured to have cpus with different power/performance
+characteristics within the same chip. In this case, additional information has
+to be made available to the kernel for it to be aware of such differences and
+take decisions accordingly.
+
+==========================================
+2 - CPU capacity definition
+==========================================
+
+CPU capacity is a number that provides the scheduler information about CPUs
+heterogeneity. Such heterogeneity can come from micro-architectural differences
+(e.g., ARM big.LITTLE systems) or maximum frequency at which CPUs can run
+(e.g., SMP systems with multiple frequency domains). Heterogeneity in this
+context is about differing performance characteristics; this binding tries to
+capture a first-order approximation of the relative performance of CPUs.
+
+CPU capacities are obtained by running a suitable benchmark. This binding makes
+no guarantees on the validity or suitability of any particular benchmark, the
+final capacity should, however, be:
+
+* A "single-threaded" or CPU affine benchmark
+* Divided by the running frequency of the CPU executing the benchmark
+* Not subject to dynamic frequency scaling of the CPU
+
+For the time being we however advise usage of the Dhrystone benchmark. What
+above thus becomes:
+
+CPU capacities are obtained by running the Dhrystone benchmark on each CPU at
+max frequency (with caches enabled). The obtained DMIPS score is then divided
+by the frequency (in MHz) at which the benchmark has been run, so that
+DMIPS/MHz are obtained.  Such values are then normalized w.r.t. the highest
+score obtained in the system.
+
+==========================================
+3 - capacity-dmips-mhz
+==========================================
+
+capacity-dmips-mhz is an optional cpu node [1] property: u32 value
+representing CPU capacity expressed in normalized DMIPS/MHz. At boot time, the
+maximum frequency available to the cpu is then used to calculate the capacity
+value internally used by the kernel.
+
+capacity-dmips-mhz property is all-or-nothing: if it is specified for a cpu
+node, it has to be specified for every other cpu nodes, or the system will
+fall back to the default capacity value for every CPU. If cpufreq is not
+available, final capacities are calculated by directly using capacity-dmips-
+mhz values (normalized w.r.t. the highest value found while parsing the DT).
+
+===========================================
+4 - Examples
+===========================================
+
+Example 1 (ARM 64-bit, 6-cpu system, two clusters):
+capacities-dmips-mhz are scaled w.r.t. 1024 (cpu at 0 and cpu at 1)
+supposing cluster0 at max-freq=1100 and custer1 at max-freq=850,
+final capacities are 1024 for cluster0 and 446 for cluster1
+
+cpus {
+	#address-cells = <2>;
+	#size-cells = <0>;
+
+	cpu-map {
+		cluster0 {
+			core0 {
+				cpu = <&A57_0>;
+			};
+			core1 {
+				cpu = <&A57_1>;
+			};
+		};
+
+		cluster1 {
+			core0 {
+				cpu = <&A53_0>;
+			};
+			core1 {
+				cpu = <&A53_1>;
+			};
+			core2 {
+				cpu = <&A53_2>;
+			};
+			core3 {
+				cpu = <&A53_3>;
+			};
+		};
+	};
+
+	idle-states {
+		entry-method = "arm,psci";
+
+		CPU_SLEEP_0: cpu-sleep-0 {
+			compatible = "arm,idle-state";
+			arm,psci-suspend-param = <0x0010000>;
+			local-timer-stop;
+			entry-latency-us = <100>;
+			exit-latency-us = <250>;
+			min-residency-us = <150>;
+		};
+
+		CLUSTER_SLEEP_0: cluster-sleep-0 {
+			compatible = "arm,idle-state";
+			arm,psci-suspend-param = <0x1010000>;
+			local-timer-stop;
+			entry-latency-us = <800>;
+			exit-latency-us = <700>;
+			min-residency-us = <2500>;
+		};
+	};
+
+	A57_0: cpu at 0 {
+		compatible = "arm,cortex-a57","arm,armv8";
+		reg = <0x0 0x0>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A57_L2>;
+		clocks = <&scpi_dvfs 0>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <1024>;
+	};
+
+	A57_1: cpu at 1 {
+		compatible = "arm,cortex-a57","arm,armv8";
+		reg = <0x0 0x1>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A57_L2>;
+		clocks = <&scpi_dvfs 0>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <1024>;
+	};
+
+	A53_0: cpu at 100 {
+		compatible = "arm,cortex-a53","arm,armv8";
+		reg = <0x0 0x100>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A53_L2>;
+		clocks = <&scpi_dvfs 1>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <578>;
+	};
+
+	A53_1: cpu at 101 {
+		compatible = "arm,cortex-a53","arm,armv8";
+		reg = <0x0 0x101>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A53_L2>;
+		clocks = <&scpi_dvfs 1>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <578>;
+	};
+
+	A53_2: cpu at 102 {
+		compatible = "arm,cortex-a53","arm,armv8";
+		reg = <0x0 0x102>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A53_L2>;
+		clocks = <&scpi_dvfs 1>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <578>;
+	};
+
+	A53_3: cpu at 103 {
+		compatible = "arm,cortex-a53","arm,armv8";
+		reg = <0x0 0x103>;
+		device_type = "cpu";
+		enable-method = "psci";
+		next-level-cache = <&A53_L2>;
+		clocks = <&scpi_dvfs 1>;
+		cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+		capacity-dmips-mhz = <578>;
+	};
+
+	A57_L2: l2-cache0 {
+		compatible = "cache";
+	};
+
+	A53_L2: l2-cache1 {
+		compatible = "cache";
+	};
+};
+
+Example 2 (ARM 32-bit, 4-cpu system, two clusters,
+	   cpus 0,1 at 1GHz, cpus 2,3 at 500MHz):
+capacities-dmips-mhz are scaled w.r.t. 2 (cpu at 0 and cpu at 1), this means that first
+cpu at 0 and cpu at 1 are twice fast than cpu at 2 and cpu at 3 (at the same frequency)
+
+cpus {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	cpu0: cpu at 0 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0>;
+		capacity-dmips-mhz = <2>;
+	};
+
+	cpu1: cpu at 1 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <1>;
+		capacity-dmips-mhz = <2>;
+	};
+
+	cpu2: cpu at 2 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x100>;
+		capacity-dmips-mhz = <1>;
+	};
+
+	cpu3: cpu at 3 {
+		device_type = "cpu";
+		compatible = "arm,cortex-a15";
+		reg = <0x101>;
+		capacity-dmips-mhz = <1>;
+	};
+};
+
+===========================================
+5 - References
+===========================================
+
+[1] ARM Linux Kernel documentation - CPUs bindings
+    Documentation/devicetree/bindings/arm/cpus.txt
diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 3f0cbbb8395f..d79442d6fdf8 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -238,6 +238,14 @@ nodes to be present and contain the properties described below.
 			# List of phandles to idle state nodes supported
 			  by this cpu [3].
 
+	- capacity-dmips-mhz
+		Usage: Optional
+		Value type: <u32>
+		Definition:
+			# u32 value representing CPU capacity [3] in
+			  DMIPS/MHz, relative to highest capacity-dmips-mhz
+			  in the system.
+
 	- rockchip,pmu
 		Usage: optional for systems that have an "enable-method"
 		       property value of "rockchip,rk3066-smp"
@@ -461,3 +469,5 @@ cpus {
 [2] arm/msm/qcom,kpss-acc.txt
 [3] ARM Linux kernel documentation - idle states bindings
     Documentation/devicetree/bindings/arm/idle-states.txt
+[3] ARM Linux kernel documentation - cpu capacity bindings
+    Documentation/devicetree/bindings/arm/cpu-capacity.txt
-- 
2.7.0

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

With the introduction of cpu capacity-dmips-mhz bindings, CPU capacities
can now be calculated from values extracted from DT and information
coming from cpufreq. Add parsing of DT information at boot time, and
complement it with cpufreq information. We keep code that can produce
same information, based on different DT properties and hard-coded
values, as fall-back for backward compatibility.

Caveat: the information provided by this patch will start to be used in
the future. We need to #define arch_scale_cpu_capacity to something
provided in arch, so that scheduler's default implementation (which gets
used if arch_scale_cpu_capacity is not defined) is overwritten.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v1:
  - normalize w.r.t. highest capacity found in DT
  - bailout conditions (all-or-nothing)

Changes from v4:
  - parsing modified to reflect change in binding (capacity-dmips-mhz)

Changes from v5:
  - allocate raw_capacity array with kcalloc()
  - pr_err() only for partial capacity information
---
 arch/arm/kernel/topology.c | 145 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index ec279d161b32..b3094e6eb1f5 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -78,6 +78,134 @@ static unsigned long *__cpu_capacity;
 #define cpu_capacity(cpu)	__cpu_capacity[cpu]
 
 static unsigned long middle_capacity = 1;
+static bool cap_from_dt = true;
+static u32 *raw_capacity;
+static bool cap_parsing_failed;
+static u32 capacity_scale;
+
+static int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+{
+	int ret = 1;
+	u32 cpu_capacity;
+
+	if (cap_parsing_failed)
+		return !ret;
+
+	ret = of_property_read_u32(cpu_node,
+				   "capacity-dmips-mhz",
+				   &cpu_capacity);
+	if (!ret) {
+		if (!raw_capacity) {
+			raw_capacity = kcalloc(num_possible_cpus(),
+					       sizeof(*raw_capacity),
+					       GFP_KERNEL);
+			if (!raw_capacity) {
+				pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
+				cap_parsing_failed = true;
+				return !ret;
+			}
+		}
+		capacity_scale = max(cpu_capacity, capacity_scale);
+		raw_capacity[cpu] = cpu_capacity;
+		pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
+			cpu_node->full_name, raw_capacity[cpu]);
+	} else {
+		if (raw_capacity) {
+			pr_err("cpu_capacity: missing %s raw capacity\n",
+				cpu_node->full_name);
+			pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
+		}
+		cap_parsing_failed = true;
+		kfree(raw_capacity);
+	}
+
+	return !ret;
+}
+
+static void normalize_cpu_capacity(void)
+{
+	u64 capacity;
+	int cpu;
+
+	if (!raw_capacity || cap_parsing_failed)
+		return;
+
+	pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+	for_each_possible_cpu(cpu) {
+		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
+			/ capacity_scale;
+		set_capacity_scale(cpu, capacity);
+		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
+			cpu, arch_scale_cpu_capacity(NULL, cpu));
+	}
+}
+
+#ifdef CONFIG_CPU_FREQ
+static cpumask_var_t cpus_to_visit;
+static bool cap_parsing_done;
+
+static int
+init_cpu_capacity_callback(struct notifier_block *nb,
+			   unsigned long val,
+			   void *data)
+{
+	struct cpufreq_policy *policy = data;
+	int cpu;
+
+	if (cap_parsing_failed || cap_parsing_done)
+		return 0;
+
+	switch (val) {
+	case CPUFREQ_NOTIFY:
+		pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
+				cpumask_pr_args(policy->related_cpus),
+				cpumask_pr_args(cpus_to_visit));
+		cpumask_andnot(cpus_to_visit,
+			       cpus_to_visit,
+			       policy->related_cpus);
+		for_each_cpu(cpu, policy->related_cpus) {
+			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+					    policy->max / 1000UL;
+			capacity_scale = max(raw_capacity[cpu], capacity_scale);
+		}
+		if (cpumask_empty(cpus_to_visit)) {
+			normalize_cpu_capacity();
+			kfree(raw_capacity);
+			pr_debug("cpu_capacity: parsing done\n");
+			cap_parsing_done = true;
+		}
+	}
+	return 0;
+}
+
+static struct notifier_block init_cpu_capacity_notifier = {
+	.notifier_call = init_cpu_capacity_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+	if (cap_parsing_failed)
+		return -EINVAL;
+
+	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
+		pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
+		return -ENOMEM;
+	}
+	cpumask_copy(cpus_to_visit, cpu_possible_mask);
+
+	return cpufreq_register_notifier(&init_cpu_capacity_notifier,
+					 CPUFREQ_POLICY_NOTIFIER);
+}
+core_initcall(register_cpufreq_notifier);
+#else
+static int __init free_raw_capacity(void)
+{
+	kfree(raw_capacity);
+
+	return 0;
+}
+core_initcall(free_raw_capacity);
+#endif
 
 /*
  * Iterate all CPUs' descriptor in DT and compute the efficiency
@@ -99,6 +227,12 @@ static void __init parse_dt_topology(void)
 	__cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
 				 GFP_NOWAIT);
 
+	cn = of_find_node_by_path("/cpus");
+	if (!cn) {
+		pr_err("No CPU information found in DT\n");
+		return;
+	}
+
 	for_each_possible_cpu(cpu) {
 		const u32 *rate;
 		int len;
@@ -110,6 +244,13 @@ static void __init parse_dt_topology(void)
 			continue;
 		}
 
+		if (parse_cpu_capacity(cn, cpu)) {
+			of_node_put(cn);
+			continue;
+		}
+
+		cap_from_dt = false;
+
 		for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
 			if (of_device_is_compatible(cn, cpu_eff->compatible))
 				break;
@@ -151,6 +292,8 @@ static void __init parse_dt_topology(void)
 		middle_capacity = ((max_capacity / 3)
 				>> (SCHED_CAPACITY_SHIFT-1)) + 1;
 
+	if (cap_from_dt && !cap_parsing_failed)
+		normalize_cpu_capacity();
 }
 
 /*
@@ -160,7 +303,7 @@ static void __init parse_dt_topology(void)
  */
 static void update_cpu_capacity(unsigned int cpu)
 {
-	if (!cpu_capacity(cpu))
+	if (!cpu_capacity(cpu) || cap_from_dt)
 		return;
 
 	set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
-- 
2.7.0

[PATCH v6 3/8] arm, dts: add TC2 cpu capacity-dmips-mhz information

[Juri Lelli]

Add TC2 cpu capacity binding information.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: devicetree at vger.kernel.org
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v1:
  - capacity-scale removed

Changes from v4:
  - binding changed to capacity-dmips-mhz
---
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index 0205c97efdef..45d08cc37b01 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
@@ -39,6 +39,7 @@
 			reg = <0>;
 			cci-control-port = <&cci_control1>;
 			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		cpu1: cpu at 1 {
@@ -47,6 +48,7 @@
 			reg = <1>;
 			cci-control-port = <&cci_control1>;
 			cpu-idle-states = <&CLUSTER_SLEEP_BIG>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		cpu2: cpu at 2 {
@@ -55,6 +57,7 @@
 			reg = <0x100>;
 			cci-control-port = <&cci_control2>;
 			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
+			capacity-dmips-mhz = <516>;
 		};
 
 		cpu3: cpu at 3 {
@@ -63,6 +66,7 @@
 			reg = <0x101>;
 			cci-control-port = <&cci_control2>;
 			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
+			capacity-dmips-mhz = <516>;
 		};
 
 		cpu4: cpu at 4 {
@@ -71,6 +75,7 @@
 			reg = <0x102>;
 			cci-control-port = <&cci_control2>;
 			cpu-idle-states = <&CLUSTER_SLEEP_LITTLE>;
+			capacity-dmips-mhz = <516>;
 		};
 
 		idle-states {
-- 
2.7.0

[PATCH v6 4/8] arm64: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

With the introduction of cpu capacity-dmips-mhz bindings, CPU capacities
can now be calculated from values extracted from DT and information
coming from cpufreq. Add parsing of DT information at boot time, and
complement it with cpufreq information. Also, store such information
using per CPU variables, as we do for arm.

Caveat: the information provided by this patch will start to be used in
the future. We need to #define arch_scale_cpu_capacity to something
provided in arch, so that scheduler's default implementation (which gets
used if arch_scale_cpu_capacity is not defined) is overwritten.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v1:
  - normalize w.r.t. highest capacity found in DT
  - bailout conditions (all-or-nothing)

Changes from v4:
  - parsing modified to reflect change in binding (capacity-dmips-mhz)

Changes from v5:
  - allocate raw_capacity array with kcalloc()
  - pr_err() only for partial capacity information
---
 arch/arm64/kernel/topology.c | 145 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 694f6deedbab..7c99600af48d 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,10 +19,152 @@
 #include <linux/nodemask.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/cpufreq.h>
 
 #include <asm/cputype.h>
 #include <asm/topology.h>
 
+static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+
+unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
+{
+	return per_cpu(cpu_scale, cpu);
+}
+
+static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
+{
+	per_cpu(cpu_scale, cpu) = capacity;
+}
+
+static u32 capacity_scale;
+static u32 *raw_capacity;
+static bool cap_parsing_failed;
+
+static void __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
+{
+	int ret;
+	u32 cpu_capacity;
+
+	if (cap_parsing_failed)
+		return;
+
+	ret = of_property_read_u32(cpu_node,
+				   "capacity-dmips-mhz",
+				   &cpu_capacity);
+	if (!ret) {
+		if (!raw_capacity) {
+			raw_capacity = kcalloc(num_possible_cpus(),
+					       sizeof(*raw_capacity),
+					       GFP_KERNEL);
+			if (!raw_capacity) {
+				pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
+				cap_parsing_failed = true;
+				return;
+			}
+		}
+		capacity_scale = max(cpu_capacity, capacity_scale);
+		raw_capacity[cpu] = cpu_capacity;
+		pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
+			cpu_node->full_name, raw_capacity[cpu]);
+	} else {
+		if (raw_capacity) {
+			pr_err("cpu_capacity: missing %s raw capacity\n",
+				cpu_node->full_name);
+			pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
+		}
+		cap_parsing_failed = true;
+		kfree(raw_capacity);
+	}
+}
+
+static void normalize_cpu_capacity(void)
+{
+	u64 capacity;
+	int cpu;
+
+	if (!raw_capacity || cap_parsing_failed)
+		return;
+
+	pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+	for_each_possible_cpu(cpu) {
+		pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
+			 cpu, raw_capacity[cpu]);
+		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
+			/ capacity_scale;
+		set_capacity_scale(cpu, capacity);
+		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
+			cpu, arch_scale_cpu_capacity(NULL, cpu));
+	}
+}
+
+#ifdef CONFIG_CPU_FREQ
+static cpumask_var_t cpus_to_visit;
+static bool cap_parsing_done;
+
+static int
+init_cpu_capacity_callback(struct notifier_block *nb,
+			   unsigned long val,
+			   void *data)
+{
+	struct cpufreq_policy *policy = data;
+	int cpu;
+
+	if (cap_parsing_failed || cap_parsing_done)
+		return 0;
+
+	switch (val) {
+	case CPUFREQ_NOTIFY:
+		pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
+				cpumask_pr_args(policy->related_cpus),
+				cpumask_pr_args(cpus_to_visit));
+		cpumask_andnot(cpus_to_visit,
+			       cpus_to_visit,
+			       policy->related_cpus);
+		for_each_cpu(cpu, policy->related_cpus) {
+			raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
+					    policy->max / 1000UL;
+			capacity_scale = max(raw_capacity[cpu], capacity_scale);
+		}
+		if (cpumask_empty(cpus_to_visit)) {
+			normalize_cpu_capacity();
+			kfree(raw_capacity);
+			pr_debug("cpu_capacity: parsing done\n");
+			cap_parsing_done = true;
+		}
+	}
+	return 0;
+}
+
+static struct notifier_block init_cpu_capacity_notifier = {
+	.notifier_call = init_cpu_capacity_callback,
+};
+
+static int __init register_cpufreq_notifier(void)
+{
+	if (cap_parsing_failed)
+		return -EINVAL;
+
+	if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
+		pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
+		return -ENOMEM;
+	}
+	cpumask_copy(cpus_to_visit, cpu_possible_mask);
+
+	return cpufreq_register_notifier(&init_cpu_capacity_notifier,
+					 CPUFREQ_POLICY_NOTIFIER);
+}
+core_initcall(register_cpufreq_notifier);
+#else
+static int __init free_raw_capacity(void)
+{
+	kfree(raw_capacity);
+
+	return 0;
+}
+core_initcall(free_raw_capacity);
+#endif
+
 static int __init get_cpu_for_node(struct device_node *node)
 {
 	struct device_node *cpu_node;
@@ -34,6 +176,7 @@ static int __init get_cpu_for_node(struct device_node *node)
 
 	for_each_possible_cpu(cpu) {
 		if (of_get_cpu_node(cpu, NULL) == cpu_node) {
+			parse_cpu_capacity(cpu_node, cpu);
 			of_node_put(cpu_node);
 			return cpu;
 		}
@@ -185,6 +328,8 @@ static int __init parse_dt_topology(void)
 	if (ret != 0)
 		goto out_map;
 
+	normalize_cpu_capacity();
+
 	/*
 	 * Check that all cores are in the topology; the SMP code will
 	 * only mark cores described in the DT as possible.
-- 
2.7.0

[PATCH v6 5/8] arm64, dts: add Juno cpu capacity-dmips-mhz information

[Juri Lelli]

Add Juno cpu capacity-dmips-mhz bindings information.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jon Medhurst <tixy@linaro.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: devicetree at vger.kernel.org
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v1:
  - capacity-scale removed

Changes from v4:
  - binding changed to capacity-dmips-mhz
---
 arch/arm64/boot/dts/arm/juno.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index dcfcf15a17f5..ad07815080e0 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -90,6 +90,7 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		A57_1: cpu at 1 {
@@ -100,6 +101,7 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		A53_0: cpu at 100 {
@@ -110,6 +112,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <578>;
 		};
 
 		A53_1: cpu at 101 {
@@ -120,6 +123,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <578>;
 		};
 
 		A53_2: cpu at 102 {
@@ -130,6 +134,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <578>;
 		};
 
 		A53_3: cpu at 103 {
@@ -140,6 +145,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <578>;
 		};
 
 		A57_L2: l2-cache0 {
-- 
2.7.0

[PATCH v6 6/8] arm64, dts: add Juno r2 cpu capacity-dmips-mhz information

[Juri Lelli]

Add Juno r2 cpu capacity-dmips-mhz bindings information.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jon Medhurst <tixy@linaro.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: devicetree at vger.kernel.org
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v4:
  - new patch since Juno r2 dt has been merged
---
 arch/arm64/boot/dts/arm/juno-r2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts
index 88ecd6182b67..823b614b9a40 100644
--- a/arch/arm64/boot/dts/arm/juno-r2.dts
+++ b/arch/arm64/boot/dts/arm/juno-r2.dts
@@ -90,6 +90,7 @@
 			next-level-cache = <&A72_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		A72_1: cpu at 1 {
@@ -100,6 +101,7 @@
 			next-level-cache = <&A72_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <1024>;
 		};
 
 		A53_0: cpu at 100 {
@@ -110,6 +112,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <485>;
 		};
 
 		A53_1: cpu at 101 {
@@ -120,6 +123,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <485>;
 		};
 
 		A53_2: cpu at 102 {
@@ -130,6 +134,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <485>;
 		};
 
 		A53_3: cpu at 103 {
@@ -140,6 +145,7 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			capacity-dmips-mhz = <485>;
 		};
 
 		A72_L2: l2-cache0 {
-- 
2.7.0

[PATCH v6 7/8] arm: add sysfs cpu_capacity attribute

[Juri Lelli]

Add a sysfs cpu_capacity attribute with which it is possible to read and
write (thus over-writing default values) CPUs capacity. This might be
useful in situations where values needs changing after boot.

The new attribute shows up as:

 /sys/devices/system/cpu/cpu*/cpu_capacity

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v5:
  - add mutex to protect cpu_scale (as pointed out by Morten off-line)
---
 arch/arm/kernel/topology.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index b3094e6eb1f5..55c6265d2d1f 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -41,6 +41,7 @@
  * updated during this sequence.
  */
 static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+static DEFINE_MUTEX(cpu_scale_mutex);
 
 unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
 {
@@ -52,6 +53,76 @@ static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
 	per_cpu(cpu_scale, cpu) = capacity;
 }
 
+#ifdef CONFIG_PROC_SYSCTL
+#include <asm/cpu.h>
+#include <linux/string.h>
+static ssize_t show_cpu_capacity(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	ssize_t rc;
+	int cpunum = cpu->dev.id;
+	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
+
+	rc = sprintf(buf, "%lu\n", capacity);
+
+	return rc;
+}
+
+static ssize_t store_cpu_capacity(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	int this_cpu = cpu->dev.id, i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (count) {
+		char *p = (char *) buf;
+
+		ret = kstrtoul(p, 0, &new_capacity);
+		if (ret)
+			return ret;
+		if (new_capacity > SCHED_CAPACITY_SCALE)
+			return -EINVAL;
+
+		mutex_lock(&cpu_scale_mutex);
+		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+			set_capacity_scale(i, new_capacity);
+		mutex_unlock(&cpu_scale_mutex);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(cpu_capacity,
+		   0644,
+		   show_cpu_capacity,
+		   store_cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+	int i;
+	struct device *cpu;
+
+	for_each_possible_cpu(i) {
+		cpu = get_cpu_device(i);
+		if (!cpu) {
+			pr_err("%s: too early to get CPU%d device!\n",
+			       __func__, i);
+			continue;
+		}
+		device_create_file(cpu, &dev_attr_cpu_capacity);
+	}
+
+	return 0;
+}
+late_initcall(register_cpu_capacity_sysctl);
+#endif
+
 #ifdef CONFIG_OF
 struct cpu_efficiency {
 	const char *compatible;
@@ -131,6 +202,7 @@ static void normalize_cpu_capacity(void)
 		return;
 
 	pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+	mutex_lock(&cpu_scale_mutex);
 	for_each_possible_cpu(cpu) {
 		capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
 			/ capacity_scale;
@@ -138,6 +210,7 @@ static void normalize_cpu_capacity(void)
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
 			cpu, arch_scale_cpu_capacity(NULL, cpu));
 	}
+	mutex_unlock(&cpu_scale_mutex);
 }
 
 #ifdef CONFIG_CPU_FREQ
-- 
2.7.0

[PATCH v6 8/8] arm64: add sysfs cpu_capacity attribute

[Juri Lelli]

Add a sysfs cpu_capacity attribute with which it is possible to read and
write (thus over-writing default values) CPUs capacity. This might be
useful in situations where values needs changing after boot.

The new attribute shows up as:

 /sys/devices/system/cpu/cpu*/cpu_capacity

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Juri Lelli <juri.lelli@arm.com>
---

Changes from v5:
  - add mutex to protect cpu_scale (as pointed out by Morten off-line)
---
 arch/arm64/kernel/topology.c | 73 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 7c99600af48d..770f05dd53cc 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -26,6 +26,7 @@
 #include <asm/topology.h>
 
 static DEFINE_PER_CPU(unsigned long, cpu_scale) = SCHED_CAPACITY_SCALE;
+static DEFINE_MUTEX(cpu_scale_mutex);
 
 unsigned long arch_scale_cpu_capacity(struct sched_domain *sd, int cpu)
 {
@@ -37,6 +38,76 @@ static void set_capacity_scale(unsigned int cpu, unsigned long capacity)
 	per_cpu(cpu_scale, cpu) = capacity;
 }
 
+#ifdef CONFIG_PROC_SYSCTL
+#include <asm/cpu.h>
+#include <linux/string.h>
+static ssize_t show_cpu_capacity(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	ssize_t rc;
+	int cpunum = cpu->dev.id;
+	unsigned long capacity = arch_scale_cpu_capacity(NULL, cpunum);
+
+	rc = sprintf(buf, "%lu\n", capacity);
+
+	return rc;
+}
+
+static ssize_t store_cpu_capacity(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t count)
+{
+	struct cpu *cpu = container_of(dev, struct cpu, dev);
+	int this_cpu = cpu->dev.id, i;
+	unsigned long new_capacity;
+	ssize_t ret;
+
+	if (count) {
+		char *p = (char *) buf;
+
+		ret = kstrtoul(p, 0, &new_capacity);
+		if (ret)
+			return ret;
+		if (new_capacity > SCHED_CAPACITY_SCALE)
+			return -EINVAL;
+
+		mutex_lock(&cpu_scale_mutex);
+		for_each_cpu(i, &cpu_topology[this_cpu].core_sibling)
+			set_capacity_scale(i, new_capacity);
+		mutex_unlock(&cpu_scale_mutex);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(cpu_capacity,
+		   0644,
+		   show_cpu_capacity,
+		   store_cpu_capacity);
+
+static int register_cpu_capacity_sysctl(void)
+{
+	int i;
+	struct device *cpu;
+
+	for_each_possible_cpu(i) {
+		cpu = get_cpu_device(i);
+		if (!cpu) {
+			pr_err("%s: too early to get CPU%d device!\n",
+			       __func__, i);
+			continue;
+		}
+		device_create_file(cpu, &dev_attr_cpu_capacity);
+	}
+
+	return 0;
+}
+late_initcall(register_cpu_capacity_sysctl);
+#endif
+
 static u32 capacity_scale;
 static u32 *raw_capacity;
 static bool cap_parsing_failed;
@@ -87,6 +158,7 @@ static void normalize_cpu_capacity(void)
 		return;
 
 	pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
+	mutex_lock(&cpu_scale_mutex);
 	for_each_possible_cpu(cpu) {
 		pr_debug("cpu_capacity: cpu=%d raw_capacity=%u\n",
 			 cpu, raw_capacity[cpu]);
@@ -96,6 +168,7 @@ static void normalize_cpu_capacity(void)
 		pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
 			cpu, arch_scale_cpu_capacity(NULL, cpu));
 	}
+	mutex_unlock(&cpu_scale_mutex);
 }
 
 #ifdef CONFIG_CPU_FREQ
-- 
2.7.0

[PATCH v6 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings

[Rob Herring]

On Tue, Jul 19, 2016 at 01:40:41PM +0100, Juri Lelli wrote:
> ARM systems may be configured to have cpus with different power/performance
> characteristics within the same chip. In this case, additional information
> has to be made available to the kernel (the scheduler in particular) for it
> to be aware of such differences and take decisions accordingly.
> 
> Therefore, this patch aims at standardizing cpu capacities device tree
> bindings for ARM platforms. Bindings define cpu capacity-dmips-mhz
> parameter, to allow operating systems to retrieve such information from
> the device tree and initialize related kernel structures, paving the way
> for common code in the kernel to deal with heterogeneity.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
> 
> Changes from v1:
>  - removed section regarding capacity-scale
>  - added information regarding normalization
> 
> Changes from v4:
>  - binding changed to capacity-dmips-mhz
>  - sections and changelod updated accordingly
> 
> Changes from v5:
>  - addressed Mark and Vincent comments
> ---
>  .../devicetree/bindings/arm/cpu-capacity.txt       | 236 +++++++++++++++++++++
>  Documentation/devicetree/bindings/arm/cpus.txt     |  10 +
>  2 files changed, 246 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/cpu-capacity.txt

I guess I'm okay with the scaled values, so:

Acked-by: Rob Herring <robh@kernel.org>

[...]

> +Example 2 (ARM 32-bit, 4-cpu system, two clusters,
> +	   cpus 0,1 at 1GHz, cpus 2,3 at 500MHz):
> +capacities-dmips-mhz are scaled w.r.t. 2 (cpu at 0 and cpu at 1), this means that first
> +cpu at 0 and cpu at 1 are twice fast than cpu at 2 and cpu at 3 (at the same frequency)

This example is a bit confusing with both the capacity and frequency 
being half. I also find it a bit unrealistic to have a 2x performance 
difference on the same micro arch. But it is all just an example...

Rob

[PATCH v6 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings

[Juri Lelli]

Hi Rob,

On 20/07/16 13:56, Rob Herring wrote:
> On Tue, Jul 19, 2016 at 01:40:41PM +0100, Juri Lelli wrote:
> > ARM systems may be configured to have cpus with different power/performance
> > characteristics within the same chip. In this case, additional information
> > has to be made available to the kernel (the scheduler in particular) for it
> > to be aware of such differences and take decisions accordingly.
> > 
> > Therefore, this patch aims at standardizing cpu capacities device tree
> > bindings for ARM platforms. Bindings define cpu capacity-dmips-mhz
> > parameter, to allow operating systems to retrieve such information from
> > the device tree and initialize related kernel structures, paving the way
> > for common code in the kernel to deal with heterogeneity.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: Kumar Gala <galak@codeaurora.org>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Olof Johansson <olof@lixom.net>
> > Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: devicetree at vger.kernel.org
> > Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> > ---
> > 
> > Changes from v1:
> >  - removed section regarding capacity-scale
> >  - added information regarding normalization
> > 
> > Changes from v4:
> >  - binding changed to capacity-dmips-mhz
> >  - sections and changelod updated accordingly
> > 
> > Changes from v5:
> >  - addressed Mark and Vincent comments
> > ---
> >  .../devicetree/bindings/arm/cpu-capacity.txt       | 236 +++++++++++++++++++++
> >  Documentation/devicetree/bindings/arm/cpus.txt     |  10 +
> >  2 files changed, 246 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/cpu-capacity.txt
> 
> I guess I'm okay with the scaled values, so:
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

Thanks!

> [...]
> 
> > +Example 2 (ARM 32-bit, 4-cpu system, two clusters,
> > +	   cpus 0,1 at 1GHz, cpus 2,3 at 500MHz):
> > +capacities-dmips-mhz are scaled w.r.t. 2 (cpu at 0 and cpu at 1), this means that first
> > +cpu at 0 and cpu at 1 are twice fast than cpu at 2 and cpu at 3 (at the same frequency)
> 
> This example is a bit confusing with both the capacity and frequency 
> being half. I also find it a bit unrealistic to have a 2x performance 
> difference on the same micro arch. But it is all just an example...
> 

Right, it's a fake example just to highlight how the values are used.

Best,

- Juri

[PATCH v6 0/8] CPUs capacity information for heterogeneous systems

[Peter Zijlstra]

On Tue, Jul 19, 2016 at 01:40:40PM +0100, Juri Lelli wrote:
> Patches high level description:
> 
>  o 01/08 introduces documentation for the new optional DT binding
>  o [02-06]/08 add cpu-capacity attribute to TC2, Juno and Juno r2 DTs and
>    provide parsing of such information at boot time
>  o [07-08]/08 introduce sysfs attribute
> 

So generally ok with me; there does seem to be an unfortunate
duplication of code between arm and arm64 but other than that it looks
fine.

[PATCH v6 4/8] arm64: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

Hi,

this patch needs the fix below, since platforms can boot without a
cpu-map.

On 19/07/16 13:40, Juri Lelli wrote:
[...]
> @@ -185,6 +328,8 @@ static int __init parse_dt_topology(void)
>  	if (ret != 0)
>  		goto out_map;
>  
> +	normalize_cpu_capacity();
> +
>  	/*
>  	 * Check that all cores are in the topology; the SMP code will
>  	 * only mark cores described in the DT as possible.
> -- 

--->8---
 arch/arm64/kernel/topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 56bbd6d41daf..288f511a5f29 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -394,8 +394,10 @@ static int __init parse_dt_topology(void)
 	 * cluster with restricted subnodes.
 	 */
 	map = of_get_child_by_name(cn, "cpu-map");
-	if (!map)
+	if (!map) {
+		cap_parsing_failed = true;
 		goto out;
+	}
 
 	ret = parse_cluster(map, 0);
 	if (ret != 0)
-- 
2.7.0

[PATCH v6 0/8] CPUs capacity information for heterogeneous systems

[Juri Lelli]

Hi,

On 08/08/16 18:49, Peter Zijlstra wrote:
> On Tue, Jul 19, 2016 at 01:40:40PM +0100, Juri Lelli wrote:
> > Patches high level description:
> > 
> >  o 01/08 introduces documentation for the new optional DT binding
> >  o [02-06]/08 add cpu-capacity attribute to TC2, Juno and Juno r2 DTs and
> >    provide parsing of such information at boot time
> >  o [07-08]/08 introduce sysfs attribute
> > 
> 
> So generally ok with me;

Thanks for reviewing.

> there does seem to be an unfortunate
> duplication of code between arm and arm64 but other than that it looks
> fine.
> 

Yes, I know. I'm failing to see how we can cleanly remove that
duplication, though (sysfs interface and callback functions being good
candidates for a common place).

Anyone any ideas?

Best,

- Juri

[PATCH v6 0/8] CPUs capacity information for heterogeneous systems

[Juri Lelli]

Hi,

On 19/07/16 13:40, Juri Lelli wrote:
> Hi all,
> 
> version 6 of "CPUs capacity information for heterogeneous systems" patchset [1]
> (please refer to previous postings to get some context).
> 

gentle ping to everybody participating in this discussion.

Rob acked the bindings and Peter seems fine with this set as well. Given
that no major problems with the current implementation where pointed out
in v5 (and the minor ones I should have addressed in v6), do people
think it might be time to merge this set? If you think the set is fine
it would be good to collect more ACKs, I guess; if not please provide
feedback so that I can address it before the next cycle.

Thanks!

Best,

- Juri

[PATCH v6 3/8] arm, dts: add TC2 cpu capacity-dmips-mhz information

[Sudeep Holla]

Hi Juri,

On 19/07/16 13:40, Juri Lelli wrote:
> Add TC2 cpu capacity binding information.
>

If you repost it,

s/binding//

> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Sudeep Holla <sudeep.holla@arm.com>
(assuming you take it via some other tree, let us know if it's
  OK to merge the DTS separately and you want us to pick them)

-- 
Regards,
Sudeep

[PATCH v6 5/8] arm64, dts: add Juno cpu capacity-dmips-mhz information

[Sudeep Holla]

On 19/07/16 13:40, Juri Lelli wrote:
> Add Juno cpu capacity-dmips-mhz bindings information.
>

s/bindings// (if you repost)

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Sudeep Holla <sudeep.holla@arm.com>
(again let us know if this needs to be picked by separately,
  generally preferred to avoid conflicts)

-- 
Regards,
Sudeep

[PATCH v6 6/8] arm64, dts: add Juno r2 cpu capacity-dmips-mhz information

[Sudeep Holla]

On 19/07/16 13:40, Juri Lelli wrote:
> Add Juno r2 cpu capacity-dmips-mhz bindings information.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

(reluctantly :), though I agree the bindings are approved to allow this,
  it seems odd to see capacity-dmips-mhz = <485> here and <578> on R0
  for the A53 CPUs)

-- 
Regards,
Sudeep

[PATCH v6 3/8] arm, dts: add TC2 cpu capacity-dmips-mhz information

[Juri Lelli]

Hi,

On 10/08/16 16:33, Sudeep Holla wrote:
> Hi Juri,
> 
> On 19/07/16 13:40, Juri Lelli wrote:
> >Add TC2 cpu capacity binding information.
> >
> 
> If you repost it,
> 
> s/binding//
> 

Yes, I think I'll need to repost, squashing the arm64 fix in the
appropriate patch. Thanks.

> >Cc: Liviu Dudau <liviu.dudau@arm.com>
> >Cc: Sudeep Holla <sudeep.holla@arm.com>
> 
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>

Thanks!

> (assuming you take it via some other tree, let us know if it's
>  OK to merge the DTS separately and you want us to pick them)
> 

As per off-line discussion with Russell, binding doc and arm bits should
go via his patch system. While arm64 bits could go separately via aarch64.

Does this make sense?

Best,

- Juri

[PATCH v6 3/8] arm, dts: add TC2 cpu capacity-dmips-mhz information

[Sudeep Holla]

On 10/08/16 16:43, Juri Lelli wrote:
> Hi,
>
> On 10/08/16 16:33, Sudeep Holla wrote:
>> Hi Juri,
>>
>> On 19/07/16 13:40, Juri Lelli wrote:
>>> Add TC2 cpu capacity binding information.
>>>
>>
>> If you repost it,
>>
>> s/binding//
>>
>
> Yes, I think I'll need to repost, squashing the arm64 fix in the
> appropriate patch. Thanks.
>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>
>> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Thanks!
>
>> (assuming you take it via some other tree, let us know if it's
>>  OK to merge the DTS separately and you want us to pick them)
>>
>
> As per off-line discussion with Russell, binding doc and arm bits should
> go via his patch system. While arm64 bits could go separately via aarch64.
>
> Does this make sense?

Yes, we can pick up the juno and tc2 dts changes and send it via
arm-soc. I have already acked them, so you can take it via other tree if
required.

-- 
Regards,
Sudeep

[PATCH v6 3/8] arm, dts: add TC2 cpu capacity-dmips-mhz information

[Juri Lelli]

On 10/08/16 16:52, Sudeep Holla wrote:
> 
> 
> On 10/08/16 16:43, Juri Lelli wrote:
> >Hi,
> >
> >On 10/08/16 16:33, Sudeep Holla wrote:
> >>Hi Juri,
> >>
> >>On 19/07/16 13:40, Juri Lelli wrote:
> >>>Add TC2 cpu capacity binding information.
> >>>
> >>
> >>If you repost it,
> >>
> >>s/binding//
> >>
> >
> >Yes, I think I'll need to repost, squashing the arm64 fix in the
> >appropriate patch. Thanks.
> >
> >>>Cc: Liviu Dudau <liviu.dudau@arm.com>
> >>>Cc: Sudeep Holla <sudeep.holla@arm.com>
> >>
> >>Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> >Thanks!
> >
> >>(assuming you take it via some other tree, let us know if it's
> >> OK to merge the DTS separately and you want us to pick them)
> >>
> >
> >As per off-line discussion with Russell, binding doc and arm bits should
> >go via his patch system. While arm64 bits could go separately via aarch64.
> >
> >Does this make sense?
> 
> Yes, we can pick up the juno and tc2 dts changes and send it via
> arm-soc.

If this works better for you it is fine by me! I'll only use Russell's
system for docs and arm parsing/sysfs implementation then.

Thanks,

- Juri

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Vincent Guittot]

Hi Juri,


On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
> With the introduction of cpu capacity-dmips-mhz bindings, CPU capacities
> can now be calculated from values extracted from DT and information
> coming from cpufreq. Add parsing of DT information at boot time, and
> complement it with cpufreq information. We keep code that can produce
> same information, based on different DT properties and hard-coded
> values, as fall-back for backward compatibility.
>
> Caveat: the information provided by this patch will start to be used in
> the future. We need to #define arch_scale_cpu_capacity to something
> provided in arch, so that scheduler's default implementation (which gets
> used if arch_scale_cpu_capacity is not defined) is overwritten.
>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>
> Changes from v1:
>   - normalize w.r.t. highest capacity found in DT
>   - bailout conditions (all-or-nothing)
>
> Changes from v4:
>   - parsing modified to reflect change in binding (capacity-dmips-mhz)
>
> Changes from v5:
>   - allocate raw_capacity array with kcalloc()
>   - pr_err() only for partial capacity information
> ---
>  arch/arm/kernel/topology.c | 145 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index ec279d161b32..b3094e6eb1f5 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -78,6 +78,134 @@ static unsigned long *__cpu_capacity;
>  #define cpu_capacity(cpu)      __cpu_capacity[cpu]
>
>  static unsigned long middle_capacity = 1;
> +static bool cap_from_dt = true;
> +static u32 *raw_capacity;
> +static bool cap_parsing_failed;
> +static u32 capacity_scale;
> +
> +static int __init parse_cpu_capacity(struct device_node *cpu_node, int cpu)
> +{
> +       int ret = 1;
> +       u32 cpu_capacity;
> +
> +       if (cap_parsing_failed)
> +               return !ret;
> +
> +       ret = of_property_read_u32(cpu_node,
> +                                  "capacity-dmips-mhz",
> +                                  &cpu_capacity);
> +       if (!ret) {
> +               if (!raw_capacity) {
> +                       raw_capacity = kcalloc(num_possible_cpus(),
> +                                              sizeof(*raw_capacity),
> +                                              GFP_KERNEL);
> +                       if (!raw_capacity) {
> +                               pr_err("cpu_capacity: failed to allocate memory for raw capacities\n");
> +                               cap_parsing_failed = true;
> +                               return !ret;
> +                       }
> +               }
> +               capacity_scale = max(cpu_capacity, capacity_scale);
> +               raw_capacity[cpu] = cpu_capacity;
> +               pr_debug("cpu_capacity: %s cpu_capacity=%u (raw)\n",
> +                       cpu_node->full_name, raw_capacity[cpu]);
> +       } else {
> +               if (raw_capacity) {
> +                       pr_err("cpu_capacity: missing %s raw capacity\n",
> +                               cpu_node->full_name);
> +                       pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n");
> +               }
> +               cap_parsing_failed = true;
> +               kfree(raw_capacity);
> +       }
> +
> +       return !ret;
> +}
> +
> +static void normalize_cpu_capacity(void)
> +{
> +       u64 capacity;
> +       int cpu;
> +
> +       if (!raw_capacity || cap_parsing_failed)
> +               return;
> +
> +       pr_debug("cpu_capacity: capacity_scale=%u\n", capacity_scale);
> +       for_each_possible_cpu(cpu) {
> +               capacity = (raw_capacity[cpu] << SCHED_CAPACITY_SHIFT)
> +                       / capacity_scale;
> +               set_capacity_scale(cpu, capacity);
> +               pr_debug("cpu_capacity: CPU%d cpu_capacity=%lu\n",
> +                       cpu, arch_scale_cpu_capacity(NULL, cpu));
> +       }
> +}
> +
> +#ifdef CONFIG_CPU_FREQ
> +static cpumask_var_t cpus_to_visit;
> +static bool cap_parsing_done;
> +
> +static int
> +init_cpu_capacity_callback(struct notifier_block *nb,
> +                          unsigned long val,
> +                          void *data)
> +{
> +       struct cpufreq_policy *policy = data;
> +       int cpu;
> +
> +       if (cap_parsing_failed || cap_parsing_done)
> +               return 0;
> +
> +       switch (val) {
> +       case CPUFREQ_NOTIFY:
> +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
> +                               cpumask_pr_args(policy->related_cpus),
> +                               cpumask_pr_args(cpus_to_visit));
> +               cpumask_andnot(cpus_to_visit,
> +                              cpus_to_visit,
> +                              policy->related_cpus);
> +               for_each_cpu(cpu, policy->related_cpus) {
> +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
> +                                           policy->max / 1000UL;

Should it be policy->cpuinfo.max_freq instead of policy->max ?

> +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
> +               }
> +               if (cpumask_empty(cpus_to_visit)) {
> +                       normalize_cpu_capacity();
> +                       kfree(raw_capacity);
> +                       pr_debug("cpu_capacity: parsing done\n");
> +                       cap_parsing_done = true;

ok so you do that once with the 1st governor that will be registered
for the CPU. Can't you unregister the notifier then ?

> +               }
> +       }
> +       return 0;
> +}
> +
> +static struct notifier_block init_cpu_capacity_notifier = {
> +       .notifier_call = init_cpu_capacity_callback,
> +};
> +
> +static int __init register_cpufreq_notifier(void)
> +{
> +       if (cap_parsing_failed)
> +               return -EINVAL;
> +
> +       if (!alloc_cpumask_var(&cpus_to_visit, GFP_KERNEL)) {
> +               pr_err("cpu_capacity: failed to allocate memory for cpus_to_visit\n");
> +               return -ENOMEM;
> +       }
> +       cpumask_copy(cpus_to_visit, cpu_possible_mask);
> +
> +       return cpufreq_register_notifier(&init_cpu_capacity_notifier,
> +                                        CPUFREQ_POLICY_NOTIFIER);
> +}
> +core_initcall(register_cpufreq_notifier);
> +#else
> +static int __init free_raw_capacity(void)
> +{
> +       kfree(raw_capacity);
> +
> +       return 0;
> +}
> +core_initcall(free_raw_capacity);
> +#endif
>
>  /*
>   * Iterate all CPUs' descriptor in DT and compute the efficiency
> @@ -99,6 +227,12 @@ static void __init parse_dt_topology(void)
>         __cpu_capacity = kcalloc(nr_cpu_ids, sizeof(*__cpu_capacity),
>                                  GFP_NOWAIT);
>
> +       cn = of_find_node_by_path("/cpus");
> +       if (!cn) {
> +               pr_err("No CPU information found in DT\n");
> +               return;
> +       }
> +
>         for_each_possible_cpu(cpu) {
>                 const u32 *rate;
>                 int len;
> @@ -110,6 +244,13 @@ static void __init parse_dt_topology(void)
>                         continue;
>                 }
>
> +               if (parse_cpu_capacity(cn, cpu)) {
> +                       of_node_put(cn);
> +                       continue;
> +               }
> +
> +               cap_from_dt = false;
> +
>                 for (cpu_eff = table_efficiency; cpu_eff->compatible; cpu_eff++)
>                         if (of_device_is_compatible(cn, cpu_eff->compatible))
>                                 break;
> @@ -151,6 +292,8 @@ static void __init parse_dt_topology(void)
>                 middle_capacity = ((max_capacity / 3)
>                                 >> (SCHED_CAPACITY_SHIFT-1)) + 1;
>
> +       if (cap_from_dt && !cap_parsing_failed)
> +               normalize_cpu_capacity();
>  }
>
>  /*
> @@ -160,7 +303,7 @@ static void __init parse_dt_topology(void)
>   */
>  static void update_cpu_capacity(unsigned int cpu)
>  {
> -       if (!cpu_capacity(cpu))
> +       if (!cpu_capacity(cpu) || cap_from_dt)
>                 return;
>
>         set_capacity_scale(cpu, cpu_capacity(cpu) / middle_capacity);
> --
> 2.7.0
>

[PATCH v6 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings

[Vincent Guittot]

On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
> ARM systems may be configured to have cpus with different power/performance
> characteristics within the same chip. In this case, additional information
> has to be made available to the kernel (the scheduler in particular) for it
> to be aware of such differences and take decisions accordingly.
>
> Therefore, this patch aims at standardizing cpu capacities device tree
> bindings for ARM platforms. Bindings define cpu capacity-dmips-mhz
> parameter, to allow operating systems to retrieve such information from
> the device tree and initialize related kernel structures, paving the way
> for common code in the kernel to deal with heterogeneity.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: devicetree at vger.kernel.org
> Signed-off-by: Juri Lelli <juri.lelli@arm.com>
> ---
>
> Changes from v1:
>  - removed section regarding capacity-scale
>  - added information regarding normalization
>
> Changes from v4:
>  - binding changed to capacity-dmips-mhz
>  - sections and changelod updated accordingly
>
> Changes from v5:
>  - addressed Mark and Vincent comments
> ---
>  .../devicetree/bindings/arm/cpu-capacity.txt       | 236 +++++++++++++++++++++
>  Documentation/devicetree/bindings/arm/cpus.txt     |  10 +
>  2 files changed, 246 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/cpu-capacity.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/cpu-capacity.txt b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
> new file mode 100644
> index 000000000000..7809fbe0cdb7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/cpu-capacity.txt
> @@ -0,0 +1,236 @@
> +==========================================
> +ARM CPUs capacity bindings
> +==========================================
> +
> +==========================================
> +1 - Introduction
> +==========================================
> +
> +ARM systems may be configured to have cpus with different power/performance
> +characteristics within the same chip. In this case, additional information has
> +to be made available to the kernel for it to be aware of such differences and
> +take decisions accordingly.
> +
> +==========================================
> +2 - CPU capacity definition
> +==========================================
> +
> +CPU capacity is a number that provides the scheduler information about CPUs
> +heterogeneity. Such heterogeneity can come from micro-architectural differences
> +(e.g., ARM big.LITTLE systems) or maximum frequency at which CPUs can run
> +(e.g., SMP systems with multiple frequency domains). Heterogeneity in this
> +context is about differing performance characteristics; this binding tries to
> +capture a first-order approximation of the relative performance of CPUs.
> +
> +CPU capacities are obtained by running a suitable benchmark. This binding makes
> +no guarantees on the validity or suitability of any particular benchmark, the
> +final capacity should, however, be:
> +
> +* A "single-threaded" or CPU affine benchmark
> +* Divided by the running frequency of the CPU executing the benchmark
> +* Not subject to dynamic frequency scaling of the CPU
> +
> +For the time being we however advise usage of the Dhrystone benchmark. What
> +above thus becomes:
> +
> +CPU capacities are obtained by running the Dhrystone benchmark on each CPU at
> +max frequency (with caches enabled). The obtained DMIPS score is then divided
> +by the frequency (in MHz) at which the benchmark has been run, so that
> +DMIPS/MHz are obtained.  Such values are then normalized w.r.t. the highest
> +score obtained in the system.
> +
> +==========================================
> +3 - capacity-dmips-mhz
> +==========================================
> +
> +capacity-dmips-mhz is an optional cpu node [1] property: u32 value
> +representing CPU capacity expressed in normalized DMIPS/MHz. At boot time, the
> +maximum frequency available to the cpu is then used to calculate the capacity
> +value internally used by the kernel.
> +
> +capacity-dmips-mhz property is all-or-nothing: if it is specified for a cpu
> +node, it has to be specified for every other cpu nodes, or the system will
> +fall back to the default capacity value for every CPU. If cpufreq is not
> +available, final capacities are calculated by directly using capacity-dmips-
> +mhz values (normalized w.r.t. the highest value found while parsing the DT).
> +
> +===========================================
> +4 - Examples
> +===========================================
> +
> +Example 1 (ARM 64-bit, 6-cpu system, two clusters):
> +capacities-dmips-mhz are scaled w.r.t. 1024 (cpu at 0 and cpu at 1)
> +supposing cluster0 at max-freq=1100 and custer1 at max-freq=850,
> +final capacities are 1024 for cluster0 and 446 for cluster1
> +
> +cpus {
> +       #address-cells = <2>;
> +       #size-cells = <0>;
> +
> +       cpu-map {
> +               cluster0 {
> +                       core0 {
> +                               cpu = <&A57_0>;
> +                       };
> +                       core1 {
> +                               cpu = <&A57_1>;
> +                       };
> +               };
> +
> +               cluster1 {
> +                       core0 {
> +                               cpu = <&A53_0>;
> +                       };
> +                       core1 {
> +                               cpu = <&A53_1>;
> +                       };
> +                       core2 {
> +                               cpu = <&A53_2>;
> +                       };
> +                       core3 {
> +                               cpu = <&A53_3>;
> +                       };
> +               };
> +       };
> +
> +       idle-states {
> +               entry-method = "arm,psci";
> +
> +               CPU_SLEEP_0: cpu-sleep-0 {
> +                       compatible = "arm,idle-state";
> +                       arm,psci-suspend-param = <0x0010000>;
> +                       local-timer-stop;
> +                       entry-latency-us = <100>;
> +                       exit-latency-us = <250>;
> +                       min-residency-us = <150>;
> +               };
> +
> +               CLUSTER_SLEEP_0: cluster-sleep-0 {
> +                       compatible = "arm,idle-state";
> +                       arm,psci-suspend-param = <0x1010000>;
> +                       local-timer-stop;
> +                       entry-latency-us = <800>;
> +                       exit-latency-us = <700>;
> +                       min-residency-us = <2500>;
> +               };
> +       };
> +
> +       A57_0: cpu at 0 {
> +               compatible = "arm,cortex-a57","arm,armv8";
> +               reg = <0x0 0x0>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A57_L2>;
> +               clocks = <&scpi_dvfs 0>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <1024>;
> +       };
> +
> +       A57_1: cpu at 1 {
> +               compatible = "arm,cortex-a57","arm,armv8";
> +               reg = <0x0 0x1>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A57_L2>;
> +               clocks = <&scpi_dvfs 0>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <1024>;
> +       };
> +
> +       A53_0: cpu at 100 {
> +               compatible = "arm,cortex-a53","arm,armv8";
> +               reg = <0x0 0x100>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A53_L2>;
> +               clocks = <&scpi_dvfs 1>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <578>;
> +       };
> +
> +       A53_1: cpu at 101 {
> +               compatible = "arm,cortex-a53","arm,armv8";
> +               reg = <0x0 0x101>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A53_L2>;
> +               clocks = <&scpi_dvfs 1>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <578>;
> +       };
> +
> +       A53_2: cpu at 102 {
> +               compatible = "arm,cortex-a53","arm,armv8";
> +               reg = <0x0 0x102>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A53_L2>;
> +               clocks = <&scpi_dvfs 1>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <578>;
> +       };
> +
> +       A53_3: cpu at 103 {
> +               compatible = "arm,cortex-a53","arm,armv8";
> +               reg = <0x0 0x103>;
> +               device_type = "cpu";
> +               enable-method = "psci";
> +               next-level-cache = <&A53_L2>;
> +               clocks = <&scpi_dvfs 1>;
> +               cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
> +               capacity-dmips-mhz = <578>;
> +       };
> +
> +       A57_L2: l2-cache0 {
> +               compatible = "cache";
> +       };
> +
> +       A53_L2: l2-cache1 {
> +               compatible = "cache";
> +       };
> +};
> +
> +Example 2 (ARM 32-bit, 4-cpu system, two clusters,
> +          cpus 0,1 at 1GHz, cpus 2,3 at 500MHz):
> +capacities-dmips-mhz are scaled w.r.t. 2 (cpu at 0 and cpu at 1), this means that first
> +cpu at 0 and cpu at 1 are twice fast than cpu at 2 and cpu at 3 (at the same frequency)
> +
> +cpus {
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +
> +       cpu0: cpu at 0 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0>;
> +               capacity-dmips-mhz = <2>;
> +       };
> +
> +       cpu1: cpu at 1 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <1>;
> +               capacity-dmips-mhz = <2>;
> +       };
> +
> +       cpu2: cpu at 2 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x100>;
> +               capacity-dmips-mhz = <1>;
> +       };
> +
> +       cpu3: cpu at 3 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x101>;
> +               capacity-dmips-mhz = <1>;
> +       };
> +};
> +
> +===========================================
> +5 - References
> +===========================================
> +
> +[1] ARM Linux Kernel documentation - CPUs bindings
> +    Documentation/devicetree/bindings/arm/cpus.txt
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 3f0cbbb8395f..d79442d6fdf8 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -238,6 +238,14 @@ nodes to be present and contain the properties described below.
>                         # List of phandles to idle state nodes supported
>                           by this cpu [3].
>
> +       - capacity-dmips-mhz
> +               Usage: Optional
> +               Value type: <u32>
> +               Definition:
> +                       # u32 value representing CPU capacity [3] in
> +                         DMIPS/MHz, relative to highest capacity-dmips-mhz
> +                         in the system.

FWIW, Acked-by Vincent Guittot <vincent.guittot@linaro.org>

> +
>         - rockchip,pmu
>                 Usage: optional for systems that have an "enable-method"
>                        property value of "rockchip,rk3066-smp"
> @@ -461,3 +469,5 @@ cpus {
>  [2] arm/msm/qcom,kpss-acc.txt
>  [3] ARM Linux kernel documentation - idle states bindings
>      Documentation/devicetree/bindings/arm/idle-states.txt
> +[3] ARM Linux kernel documentation - cpu capacity bindings
> +    Documentation/devicetree/bindings/arm/cpu-capacity.txt
> --
> 2.7.0
>

[PATCH v6 1/8] Documentation: arm: define DT cpu capacity-dmips-mhz bindings

[Juri Lelli]

On 16/08/16 11:02, Vincent Guittot wrote:
> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
> > ARM systems may be configured to have cpus with different power/performance
> > characteristics within the same chip. In this case, additional information
> > has to be made available to the kernel (the scheduler in particular) for it
> > to be aware of such differences and take decisions accordingly.
> >
> > Therefore, this patch aims at standardizing cpu capacities device tree
> > bindings for ARM platforms. Bindings define cpu capacity-dmips-mhz
> > parameter, to allow operating systems to retrieve such information from
> > the device tree and initialize related kernel structures, paving the way
> > for common code in the kernel to deal with heterogeneity.
> >

[...]

> 
> FWIW, Acked-by Vincent Guittot <vincent.guittot@linaro.org>
> 

Thanks!

Best,

- Juri

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

Hi Vincent,

On 16/08/16 10:20, Vincent Guittot wrote:
> Hi Juri,
> 
> 
> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:

[...]

> > +static int
> > +init_cpu_capacity_callback(struct notifier_block *nb,
> > +                          unsigned long val,
> > +                          void *data)
> > +{
> > +       struct cpufreq_policy *policy = data;
> > +       int cpu;
> > +
> > +       if (cap_parsing_failed || cap_parsing_done)
> > +               return 0;
> > +
> > +       switch (val) {
> > +       case CPUFREQ_NOTIFY:
> > +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
> > +                               cpumask_pr_args(policy->related_cpus),
> > +                               cpumask_pr_args(cpus_to_visit));
> > +               cpumask_andnot(cpus_to_visit,
> > +                              cpus_to_visit,
> > +                              policy->related_cpus);
> > +               for_each_cpu(cpu, policy->related_cpus) {
> > +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
> > +                                           policy->max / 1000UL;
> 
> Should it be policy->cpuinfo.max_freq instead of policy->max ?
> 

Right. I'll fix the arm64 bits as well.

> > +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
> > +               }
> > +               if (cpumask_empty(cpus_to_visit)) {
> > +                       normalize_cpu_capacity();
> > +                       kfree(raw_capacity);
> > +                       pr_debug("cpu_capacity: parsing done\n");
> > +                       cap_parsing_done = true;
> 
> ok so you do that once with the 1st governor that will be registered
> for the CPU. Can't you unregister the notifier then ?
> 

I tried, but the only place I could find to unregister it is from the
callback itself; and it is not possible to do so AFAIK. Suggestions?

Thanks for the review.

Best,

- Juri

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Vincent Guittot]

On 30 August 2016 at 18:28, Juri Lelli <juri.lelli@arm.com> wrote:
> Hi Vincent,
>
> On 16/08/16 10:20, Vincent Guittot wrote:
>> Hi Juri,
>>
>>
>> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
>
> [...]
>
>> > +static int
>> > +init_cpu_capacity_callback(struct notifier_block *nb,
>> > +                          unsigned long val,
>> > +                          void *data)
>> > +{
>> > +       struct cpufreq_policy *policy = data;
>> > +       int cpu;
>> > +
>> > +       if (cap_parsing_failed || cap_parsing_done)
>> > +               return 0;
>> > +
>> > +       switch (val) {
>> > +       case CPUFREQ_NOTIFY:
>> > +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
>> > +                               cpumask_pr_args(policy->related_cpus),
>> > +                               cpumask_pr_args(cpus_to_visit));
>> > +               cpumask_andnot(cpus_to_visit,
>> > +                              cpus_to_visit,
>> > +                              policy->related_cpus);
>> > +               for_each_cpu(cpu, policy->related_cpus) {
>> > +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
>> > +                                           policy->max / 1000UL;
>>
>> Should it be policy->cpuinfo.max_freq instead of policy->max ?
>>
>
> Right. I'll fix the arm64 bits as well.
>
>> > +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
>> > +               }
>> > +               if (cpumask_empty(cpus_to_visit)) {
>> > +                       normalize_cpu_capacity();
>> > +                       kfree(raw_capacity);
>> > +                       pr_debug("cpu_capacity: parsing done\n");
>> > +                       cap_parsing_done = true;
>>
>> ok so you do that once with the 1st governor that will be registered
>> for the CPU. Can't you unregister the notifier then ?
>>
>
> I tried, but the only place I could find to unregister it is from the
> callback itself; and it is not possible to do so AFAIK. Suggestions?

yes, you're right
Can't you queue a work to unregister your callback ?
That's may be overkill but it seems weird to keep this notifier that
will do nothing, registered for the whole life of the system

>
> Thanks for the review.
>
> Best,
>
> - Juri

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

On 31/08/16 10:14, Vincent Guittot wrote:
> On 30 August 2016 at 18:28, Juri Lelli <juri.lelli@arm.com> wrote:
> > Hi Vincent,
> >
> > On 16/08/16 10:20, Vincent Guittot wrote:
> >> Hi Juri,
> >>
> >>
> >> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
> >
> > [...]
> >
> >> > +static int
> >> > +init_cpu_capacity_callback(struct notifier_block *nb,
> >> > +                          unsigned long val,
> >> > +                          void *data)
> >> > +{
> >> > +       struct cpufreq_policy *policy = data;
> >> > +       int cpu;
> >> > +
> >> > +       if (cap_parsing_failed || cap_parsing_done)
> >> > +               return 0;
> >> > +
> >> > +       switch (val) {
> >> > +       case CPUFREQ_NOTIFY:
> >> > +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
> >> > +                               cpumask_pr_args(policy->related_cpus),
> >> > +                               cpumask_pr_args(cpus_to_visit));
> >> > +               cpumask_andnot(cpus_to_visit,
> >> > +                              cpus_to_visit,
> >> > +                              policy->related_cpus);
> >> > +               for_each_cpu(cpu, policy->related_cpus) {
> >> > +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
> >> > +                                           policy->max / 1000UL;
> >>
> >> Should it be policy->cpuinfo.max_freq instead of policy->max ?
> >>
> >
> > Right. I'll fix the arm64 bits as well.
> >
> >> > +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
> >> > +               }
> >> > +               if (cpumask_empty(cpus_to_visit)) {
> >> > +                       normalize_cpu_capacity();
> >> > +                       kfree(raw_capacity);
> >> > +                       pr_debug("cpu_capacity: parsing done\n");
> >> > +                       cap_parsing_done = true;
> >>
> >> ok so you do that once with the 1st governor that will be registered
> >> for the CPU. Can't you unregister the notifier then ?
> >>
> >
> > I tried, but the only place I could find to unregister it is from the
> > callback itself; and it is not possible to do so AFAIK. Suggestions?
> 
> yes, you're right
> Can't you queue a work to unregister your callback ?

You mean something like this? I guess I thought it would be much more
ugly. :)

If it looks OK, I'll add the same for arm64 and test it a bit more.

--->8---
 arch/arm/kernel/topology.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
index cbc57c287145..672ae22e2768 100644
--- a/arch/arm/kernel/topology.c
+++ b/arch/arm/kernel/topology.c
@@ -215,7 +215,8 @@ static void normalize_cpu_capacity(void)
 
 #ifdef CONFIG_CPU_FREQ
 static cpumask_var_t cpus_to_visit;
-static bool cap_parsing_done;
+static void parsing_done_workfn(struct work_struct *work);
+static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
 
 static int
 init_cpu_capacity_callback(struct notifier_block *nb,
@@ -225,7 +226,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 	struct cpufreq_policy *policy = data;
 	int cpu;
 
-	if (cap_parsing_failed || cap_parsing_done)
+	if (cap_parsing_failed)
 		return 0;
 
 	switch (val) {
@@ -245,7 +246,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
 			normalize_cpu_capacity();
 			kfree(raw_capacity);
 			pr_debug("cpu_capacity: parsing done\n");
-			cap_parsing_done = true;
+			schedule_work(&parsing_done_work);
 		}
 	}
 	return 0;
@@ -270,6 +271,13 @@ static int __init register_cpufreq_notifier(void)
 					 CPUFREQ_POLICY_NOTIFIER);
 }
 core_initcall(register_cpufreq_notifier);
+
+static void parsing_done_workfn(struct work_struct *work)
+{
+	cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
+					 CPUFREQ_POLICY_NOTIFIER);
+}
+
 #else
 static int __init free_raw_capacity(void)
 {

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Vincent Guittot]

On 31 August 2016 at 19:08, Juri Lelli <juri.lelli@arm.com> wrote:
> On 31/08/16 10:14, Vincent Guittot wrote:
>> On 30 August 2016 at 18:28, Juri Lelli <juri.lelli@arm.com> wrote:
>> > Hi Vincent,
>> >
>> > On 16/08/16 10:20, Vincent Guittot wrote:
>> >> Hi Juri,
>> >>
>> >>
>> >> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
>> >
>> > [...]
>> >
>> >> > +static int
>> >> > +init_cpu_capacity_callback(struct notifier_block *nb,
>> >> > +                          unsigned long val,
>> >> > +                          void *data)
>> >> > +{
>> >> > +       struct cpufreq_policy *policy = data;
>> >> > +       int cpu;
>> >> > +
>> >> > +       if (cap_parsing_failed || cap_parsing_done)
>> >> > +               return 0;
>> >> > +
>> >> > +       switch (val) {
>> >> > +       case CPUFREQ_NOTIFY:
>> >> > +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
>> >> > +                               cpumask_pr_args(policy->related_cpus),
>> >> > +                               cpumask_pr_args(cpus_to_visit));
>> >> > +               cpumask_andnot(cpus_to_visit,
>> >> > +                              cpus_to_visit,
>> >> > +                              policy->related_cpus);
>> >> > +               for_each_cpu(cpu, policy->related_cpus) {
>> >> > +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
>> >> > +                                           policy->max / 1000UL;
>> >>
>> >> Should it be policy->cpuinfo.max_freq instead of policy->max ?
>> >>
>> >
>> > Right. I'll fix the arm64 bits as well.
>> >
>> >> > +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
>> >> > +               }
>> >> > +               if (cpumask_empty(cpus_to_visit)) {
>> >> > +                       normalize_cpu_capacity();
>> >> > +                       kfree(raw_capacity);
>> >> > +                       pr_debug("cpu_capacity: parsing done\n");
>> >> > +                       cap_parsing_done = true;
>> >>
>> >> ok so you do that once with the 1st governor that will be registered
>> >> for the CPU. Can't you unregister the notifier then ?
>> >>
>> >
>> > I tried, but the only place I could find to unregister it is from the
>> > callback itself; and it is not possible to do so AFAIK. Suggestions?
>>
>> yes, you're right
>> Can't you queue a work to unregister your callback ?
>
> You mean something like this? I guess I thought it would be much more
> ugly. :)

Yes something like below.

>
> If it looks OK, I'll add the same for arm64 and test it a bit more.
>
> --->8---
>  arch/arm/kernel/topology.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index cbc57c287145..672ae22e2768 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -215,7 +215,8 @@ static void normalize_cpu_capacity(void)
>
>  #ifdef CONFIG_CPU_FREQ
>  static cpumask_var_t cpus_to_visit;
> -static bool cap_parsing_done;
> +static void parsing_done_workfn(struct work_struct *work);
> +static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
>
>  static int
>  init_cpu_capacity_callback(struct notifier_block *nb,
> @@ -225,7 +226,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
>         struct cpufreq_policy *policy = data;
>         int cpu;
>
> -       if (cap_parsing_failed || cap_parsing_done)
> +       if (cap_parsing_failed)

you probably need to keep cap_parsing_done to prevent spurious
notification until the work is scheduled

>                 return 0;
>
>         switch (val) {
> @@ -245,7 +246,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
>                         normalize_cpu_capacity();
>                         kfree(raw_capacity);
>                         pr_debug("cpu_capacity: parsing done\n");
> -                       cap_parsing_done = true;
> +                       schedule_work(&parsing_done_work);
>                 }
>         }
>         return 0;
> @@ -270,6 +271,13 @@ static int __init register_cpufreq_notifier(void)
>                                          CPUFREQ_POLICY_NOTIFIER);
>  }
>  core_initcall(register_cpufreq_notifier);
> +
> +static void parsing_done_workfn(struct work_struct *work)
> +{
> +       cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
> +                                        CPUFREQ_POLICY_NOTIFIER);
> +}
> +
>  #else
>  static int __init free_raw_capacity(void)
>  {

[PATCH v6 2/8] arm: parse cpu capacity-dmips-mhz from DT

[Juri Lelli]

On 01/09/16 10:22, Vincent Guittot wrote:
> On 31 August 2016 at 19:08, Juri Lelli <juri.lelli@arm.com> wrote:
> > On 31/08/16 10:14, Vincent Guittot wrote:
> >> On 30 August 2016 at 18:28, Juri Lelli <juri.lelli@arm.com> wrote:
> >> > Hi Vincent,
> >> >
> >> > On 16/08/16 10:20, Vincent Guittot wrote:
> >> >> Hi Juri,
> >> >>
> >> >>
> >> >> On 19 July 2016 at 14:40, Juri Lelli <juri.lelli@arm.com> wrote:
> >> >
> >> > [...]
> >> >
> >> >> > +static int
> >> >> > +init_cpu_capacity_callback(struct notifier_block *nb,
> >> >> > +                          unsigned long val,
> >> >> > +                          void *data)
> >> >> > +{
> >> >> > +       struct cpufreq_policy *policy = data;
> >> >> > +       int cpu;
> >> >> > +
> >> >> > +       if (cap_parsing_failed || cap_parsing_done)
> >> >> > +               return 0;
> >> >> > +
> >> >> > +       switch (val) {
> >> >> > +       case CPUFREQ_NOTIFY:
> >> >> > +               pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
> >> >> > +                               cpumask_pr_args(policy->related_cpus),
> >> >> > +                               cpumask_pr_args(cpus_to_visit));
> >> >> > +               cpumask_andnot(cpus_to_visit,
> >> >> > +                              cpus_to_visit,
> >> >> > +                              policy->related_cpus);
> >> >> > +               for_each_cpu(cpu, policy->related_cpus) {
> >> >> > +                       raw_capacity[cpu] = arch_scale_cpu_capacity(NULL, cpu) *
> >> >> > +                                           policy->max / 1000UL;
> >> >>
> >> >> Should it be policy->cpuinfo.max_freq instead of policy->max ?
> >> >>
> >> >
> >> > Right. I'll fix the arm64 bits as well.
> >> >
> >> >> > +                       capacity_scale = max(raw_capacity[cpu], capacity_scale);
> >> >> > +               }
> >> >> > +               if (cpumask_empty(cpus_to_visit)) {
> >> >> > +                       normalize_cpu_capacity();
> >> >> > +                       kfree(raw_capacity);
> >> >> > +                       pr_debug("cpu_capacity: parsing done\n");
> >> >> > +                       cap_parsing_done = true;
> >> >>
> >> >> ok so you do that once with the 1st governor that will be registered
> >> >> for the CPU. Can't you unregister the notifier then ?
> >> >>
> >> >
> >> > I tried, but the only place I could find to unregister it is from the
> >> > callback itself; and it is not possible to do so AFAIK. Suggestions?
> >>
> >> yes, you're right
> >> Can't you queue a work to unregister your callback ?
> >
> > You mean something like this? I guess I thought it would be much more
> > ugly. :)
> 
> Yes something like below.
> 
> >
> > If it looks OK, I'll add the same for arm64 and test it a bit more.
> >
> > --->8---
> >  arch/arm/kernel/topology.c | 14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> > index cbc57c287145..672ae22e2768 100644
> > --- a/arch/arm/kernel/topology.c
> > +++ b/arch/arm/kernel/topology.c
> > @@ -215,7 +215,8 @@ static void normalize_cpu_capacity(void)
> >
> >  #ifdef CONFIG_CPU_FREQ
> >  static cpumask_var_t cpus_to_visit;
> > -static bool cap_parsing_done;
> > +static void parsing_done_workfn(struct work_struct *work);
> > +static DECLARE_WORK(parsing_done_work, parsing_done_workfn);
> >
> >  static int
> >  init_cpu_capacity_callback(struct notifier_block *nb,
> > @@ -225,7 +226,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
> >         struct cpufreq_policy *policy = data;
> >         int cpu;
> >
> > -       if (cap_parsing_failed || cap_parsing_done)
> > +       if (cap_parsing_failed)
> 
> you probably need to keep cap_parsing_done to prevent spurious
> notification until the work is scheduled
> 

OK. Thanks.

Best,

- Juri