All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] dt-bindings: Powerzone new bindings
@ 2021-11-26 18:14 Daniel Lezcano
  2021-11-26 18:14   ` Daniel Lezcano
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Arnd Bergmann, Rob Herring

The proposed bindings are describing a set of powerzones.

A power zone is the logical name for a component which is capable of
power capping and where we can measure the power consumption.

A power zone can aggregate several power zones in terms of power
measurement and power limitations. That allows to apply power
constraint to a group of components and let the system balance the
allocated power in order to comply with the constraint.

The ARM System Control and Management Interface (SCMI) can provide a
power zone description.

The powerzone semantic is also found on the Intel platform with the
RAPL register.

The Linux kernel powercap framework deals with the powerzones:

https://www.kernel.org/doc/html/latest/power/powercap/powercap.html

The powerzone can also represent a group of children powerzones, hence
the description can result on a hierarchy. Such hierarchy already
exists with the hardware or can be represented an computed from the
kernel.

The hierarchical description was initially proposed but not desired
given there are other descriptions like the power domain proposing
almost the same description.

https://lore.kernel.org/all/CAL_JsqLuLcHj7525tTUmh7pLqe7T2j6UcznyhV7joS8ipyb_VQ@mail.gmail.com/

The description gives the power constraint dependencies to apply on a
specific group of logically or physically aggregated devices. They do
not represent the physical location or the power domains of the SoC
even if the description could be similar.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
   V1: Initial post
   V2:
     - Added pattern properties and stick to powerzone-*
     - Added required property compatible and powerzone-cells
     - Added additionnal property
     - Added compatible
     - Renamed to 'powerzones'
     - Added missing powerzone-cells to the topmost node
     - Fixed errors reported by 'make DT_CHECKER_FLAGS=-m dt_binding_check'
---
 .../devicetree/bindings/power/powerzones.yaml | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/powerzones.yaml

diff --git a/Documentation/devicetree/bindings/power/powerzones.yaml b/Documentation/devicetree/bindings/power/powerzones.yaml
new file mode 100644
index 000000000000..6e63bbc750c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/powerzones.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/powerzones.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Power zones description
+
+maintainers:
+  - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+description: |+
+
+  A System on Chip contains a multitude of active components and each
+  of them is a source of heat. Even if a temperature sensor is not
+  present, a source of heat can be controlled by acting on the
+  consumed power via different techniques.
+
+  A powerzone describes a component or a group of components where we
+  can control the maximum power consumption. For instance, a group of
+  CPUs via the performance domain, a LCD screen via the brightness,
+  etc ...
+
+  Different components when they are used together can significantly
+  increase the overall temperature, so the description needs to
+  reflect this dependency in order to assign a power budget for a
+  group of powerzones.
+
+  This description is done via a hierarchy and the DT reflects it. It
+  does not represent the physical location or a topology, eg. on a
+  big.Little system, the little CPUs may not be represented as they do
+  not contribute significantly to the heat, however the GPU can be
+  tied with the big CPUs as they usually have a connection for
+  multimedia or game workloads.
+    
+properties:
+  $nodename:
+    const: powerzones
+
+  compatible:
+    const: powerzones
+
+patternProperties:
+  "^(powerzone)([@-].*)?$":
+    type: object
+    description:
+      A node representing a powerzone acting as an aggregator for all
+      its children powerzones.
+
+    properties:
+      "#powerzone-cells":
+        description:
+          Number of cells in powerzone specifier. Typically 0 for nodes
+          representing but it can be any number in the future to
+          describe parameters of the powerzone.
+
+      powerzones:
+        description:
+          A phandle to a parent powerzone. If no powerzone attribute is
+          set, the described powerzone is the topmost in the hierarchy.
+
+    required:
+      - "#powerzone-cells"
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    powerzones {
+
+      compatible = "powerzones";
+
+      #powerzone-cells = <0>;
+
+      SOC_PZ: powerzone-soc {
+        #powerzone-cells = <0>;
+      };
+
+      PKG_PZ: powerzone-pkg {
+        #powerzone-cells = <0>;
+        powerzones = <&SOC_PZ>;
+      };
+
+      GPU_PZ: powerzone-gpu {
+        #powerzone-cells = <0>;
+        powerzones = <&PKG_PZ>;
+      };
+    };
+
+  - |
+    A57_0: big@0 {
+      compatible = "arm,cortex-a57";
+      reg = <0x0 0x0>;
+      device_type = "cpu";
+      #powerzone-cells = <0>;
+      powerzones = <&PKG_PZ>;
+    };
+
+    A57_1: big@1 {
+      compatible = "arm,cortex-a57";
+      reg = <0x0 0x0>;
+      device_type = "cpu";
+      #powerzone-cells = <0>;
+      powerzones = <&PKG_PZ>;
+    };
+...
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
  2021-11-26 18:14   ` Daniel Lezcano
@ 2021-11-26 18:14   ` Daniel Lezcano
  2021-11-26 18:14 ` [PATCH v2 4/5] powercap/drivers/dtpm: Add CPU " Daniel Lezcano
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Rob Herring, Johan Jonker,
	Helen Koike, Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

Add the powerzones description. This first step introduces the big,
the little and the gpu as a powerzone place.

Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
   V1: Initial post
   V2:
     - Move description in the SoC dtsi specific file
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d3cdf6f42a30..3c0dbc0cb2bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l1: cpu@1 {
@@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l2: cpu@2 {
@@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l3: cpu@3 {
@@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_b0: cpu@100 {
@@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -791,6 +803,17 @@ spi5: spi@ff200000 {
 		status = "disabled";
 	};
 
+	powerzones {
+
+		PKG_PZ: pkg {
+			#powerzone-cells = <0>;
+                        powerzone = <&SOC_PZ>;
+		};
+
+		SOC_PZ: soc {
+		};
+	};
+
 	thermal_zones: thermal-zones {
 		cpu_thermal: cpu-thermal {
 			polling-delay-passive = <100>;
@@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
 		clocks = <&cru ACLK_GPU>;
 		#cooling-cells = <2>;
 		power-domains = <&power RK3399_PD_GPU>;
+		#powerzone-cells = <0>;
+		powerzone = <&PKG_PZ>;
 		status = "disabled";
 	};
 
-- 
2.25.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-11-26 18:14   ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Rob Herring, Johan Jonker,
	Helen Koike, Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

Add the powerzones description. This first step introduces the big,
the little and the gpu as a powerzone place.

Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
   V1: Initial post
   V2:
     - Move description in the SoC dtsi specific file
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d3cdf6f42a30..3c0dbc0cb2bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l1: cpu@1 {
@@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l2: cpu@2 {
@@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l3: cpu@3 {
@@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_b0: cpu@100 {
@@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -791,6 +803,17 @@ spi5: spi@ff200000 {
 		status = "disabled";
 	};
 
+	powerzones {
+
+		PKG_PZ: pkg {
+			#powerzone-cells = <0>;
+                        powerzone = <&SOC_PZ>;
+		};
+
+		SOC_PZ: soc {
+		};
+	};
+
 	thermal_zones: thermal-zones {
 		cpu_thermal: cpu-thermal {
 			polling-delay-passive = <100>;
@@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
 		clocks = <&cru ACLK_GPU>;
 		#cooling-cells = <2>;
 		power-domains = <&power RK3399_PD_GPU>;
+		#powerzone-cells = <0>;
+		powerzone = <&PKG_PZ>;
 		status = "disabled";
 	};
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-11-26 18:14   ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Rob Herring, Johan Jonker,
	Helen Koike, Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

Add the powerzones description. This first step introduces the big,
the little and the gpu as a powerzone place.

Cc: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
   V1: Initial post
   V2:
     - Move description in the SoC dtsi specific file
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d3cdf6f42a30..3c0dbc0cb2bc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l1: cpu@1 {
@@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l2: cpu@2 {
@@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_l3: cpu@3 {
@@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <100>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 		};
 
 		cpu_b0: cpu@100 {
@@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
 			#cooling-cells = <2>; /* min followed by max */
 			dynamic-power-coefficient = <436>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
+			#powerzone-cells = <0>;
+			powerzone = <&PKG_PZ>;
 
 			thermal-idle {
 				#cooling-cells = <2>;
@@ -791,6 +803,17 @@ spi5: spi@ff200000 {
 		status = "disabled";
 	};
 
+	powerzones {
+
+		PKG_PZ: pkg {
+			#powerzone-cells = <0>;
+                        powerzone = <&SOC_PZ>;
+		};
+
+		SOC_PZ: soc {
+		};
+	};
+
 	thermal_zones: thermal-zones {
 		cpu_thermal: cpu-thermal {
 			polling-delay-passive = <100>;
@@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
 		clocks = <&cru ACLK_GPU>;
 		#cooling-cells = <2>;
 		power-domains = <&power RK3399_PD_GPU>;
+		#powerzone-cells = <0>;
+		powerzone = <&PKG_PZ>;
 		status = "disabled";
 	};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/5] powercap/drivers/dtpm: Add DT initialization support
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
  2021-11-26 18:14   ` Daniel Lezcano
@ 2021-11-26 18:14 ` Daniel Lezcano
  2021-11-26 18:14 ` [PATCH v2 4/5] powercap/drivers/dtpm: Add CPU " Daniel Lezcano
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Rafael J. Wysocki, Daniel Lezcano

The DTPM framework is available but without a way to configure it.

These changes add DT support to initialize the powercap hierarchy via
the powerzones description.

It acts in two steps. First it reads the powerzone dependencies and
build the DTPM hierarchy. Second, it search for all devices which
belongs to a powerzone and attach them to the hierarchy.

This approach makes the initialization self-encapsulated for the DTPM
components.

In order to ensure a nice self-encapsulation, the DTPM table
descriptors contains a couple of initialization functions, one to
setup the DTPM backend and one to initialize it up. With this
approach, the DTPM framework has a very few functions to export.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/powercap/Kconfig    |  1 +
 drivers/powercap/dtpm.c     | 95 +++++++++++++++++++++++++++++++++++--
 drivers/powercap/dtpm_cpu.c |  2 +-
 include/linux/dtpm.h        | 19 +++++++-
 4 files changed, 110 insertions(+), 7 deletions(-)

diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
index 8242e8c5ed77..b1ca339957e3 100644
--- a/drivers/powercap/Kconfig
+++ b/drivers/powercap/Kconfig
@@ -46,6 +46,7 @@ config IDLE_INJECT
 
 config DTPM
 	bool "Power capping for Dynamic Thermal Power Management (EXPERIMENTAL)"
+	depends on OF
 	help
 	  This enables support for the power capping for the dynamic
 	  thermal power management userspace engine.
diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index 0fe70687c198..ebf08c0f489c 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -23,6 +23,7 @@
 #include <linux/powercap.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 
 #define DTPM_POWER_LIMIT_FLAG 0
 
@@ -461,9 +462,69 @@ int dtpm_register(const char *name, struct dtpm *dtpm, struct dtpm *parent)
 	return 0;
 }
 
-static int __init init_dtpm(void)
+static int dtpm_for_each_child_of(struct device_node *root,
+				  struct device_node *np, struct dtpm *parent)
 {
+	struct device_node *child;
+	struct device_node *pz;
+	struct dtpm *dtpm;
+	int ret;
+
+	for_each_child_of_node(root, child) {
+
+		pz = of_parse_phandle(child, "powerzone", 0);
+		if (pz != np)
+			continue;
+
+		dtpm = kzalloc(sizeof(*dtpm), GFP_KERNEL);
+		if (!dtpm)
+			return -ENOMEM;
+
+		dtpm_init(dtpm, NULL);
+
+		ret = dtpm_register(child->name, dtpm, parent);
+		if (ret) {
+			pr_err("Failed to register dtpm node '%s'\n", child->name);
+			return ret;
+		}
+
+		dtpm_set_data(dtpm, child);
+
+		dtpm_for_each_child_of(root, child, dtpm);
+	}
+
+	return 0;
+}
+
+static int for_each_pz_dtpm(struct dtpm *dtpm, struct device_node *pz,
+			    struct device_node *np, dtpm_setup_t setup)
+{
+	struct dtpm *child;
+	int ret = 0;
+
+	if (dtpm_get_data(dtpm) == pz && setup) {
+		ret = setup(dtpm, np);
+		if (ret)
+			return ret;
+	}
+
+	list_for_each_entry(child, &dtpm->children, sibling)
+		ret |= for_each_pz_dtpm(child, pz, np, setup);
+
+	return ret;
+}
+
+static int dtpm_probe(void)
+{
+	struct device_node *np;
+	struct device_node *pz;
+
 	struct dtpm_descr *dtpm_descr;
+	int ret;
+
+	np = of_find_node_by_name(NULL, "powerzones");
+	if (!np)
+		return 0;
 
 	pct = powercap_register_control_type(NULL, "dtpm", NULL);
 	if (IS_ERR(pct)) {
@@ -471,9 +532,35 @@ static int __init init_dtpm(void)
 		return PTR_ERR(pct);
 	}
 
-	for_each_dtpm_table(dtpm_descr)
-		dtpm_descr->init();
+	ret = dtpm_for_each_child_of(np, NULL, NULL);
+	if (ret) {
+		pr_err("Failed to read powerzones hierarchy: %d\n", ret);
+		goto out_release;
+	}
 
+	for_each_node_with_property(np, "powerzone") {
+
+		pz = of_parse_phandle(np, "powerzone", 0);
+
+		of_node_put(np);
+		if (!pz)
+			continue;
+
+		for_each_dtpm_table(dtpm_descr)
+			for_each_pz_dtpm(root, pz, np, dtpm_descr->setup);
+
+		of_node_put(pz);
+	}
+
+	for_each_dtpm_table(dtpm_descr)
+		if (dtpm_descr->init)
+			dtpm_descr->init();
+	
 	return 0;
+
+out_release:
+	powercap_unregister_control_type(pct);
+
+	return ret;
 }
-late_initcall(init_dtpm);
+late_initcall(dtpm_probe);
diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
index b740866b228d..6bffb44c75aa 100644
--- a/drivers/powercap/dtpm_cpu.c
+++ b/drivers/powercap/dtpm_cpu.c
@@ -269,4 +269,4 @@ static int __init dtpm_cpu_init(void)
 	return 0;
 }
 
-DTPM_DECLARE(dtpm_cpu, dtpm_cpu_init);
+DTPM_DECLARE(dtpm_cpu, dtpm_cpu_init, NULL);
diff --git a/include/linux/dtpm.h b/include/linux/dtpm.h
index d37e5d06a357..7328682f24c9 100644
--- a/include/linux/dtpm.h
+++ b/include/linux/dtpm.h
@@ -32,23 +32,28 @@ struct dtpm_ops {
 	void (*release)(struct dtpm *);
 };
 
+struct device_node;
+
 typedef int (*dtpm_init_t)(void);
+typedef int (*dtpm_setup_t)(struct dtpm *, struct device_node *);
 
 struct dtpm_descr {
 	dtpm_init_t init;
+	dtpm_setup_t setup;
 };
 
 /* Init section thermal table */
 extern struct dtpm_descr __dtpm_table[];
 extern struct dtpm_descr __dtpm_table_end[];
 
-#define DTPM_TABLE_ENTRY(name, __init)				\
+#define DTPM_TABLE_ENTRY(name, __init, __setup)			\
 	static struct dtpm_descr __dtpm_table_entry_##name	\
 	__used __section("__dtpm_table") = {			\
 		.init = __init,					\
+		.setup = __setup,				\
 	}
 
-#define DTPM_DECLARE(name, init)	DTPM_TABLE_ENTRY(name, init)
+#define DTPM_DECLARE(name, init, setup)	DTPM_TABLE_ENTRY(name, init, setup)
 
 #define for_each_dtpm_table(__dtpm)	\
 	for (__dtpm = __dtpm_table;	\
@@ -60,6 +65,16 @@ static inline struct dtpm *to_dtpm(struct powercap_zone *zone)
 	return container_of(zone, struct dtpm, zone);
 }
 
+static inline void dtpm_set_data(struct dtpm *dtpm, void *data)
+{
+	powercap_set_zone_data(&dtpm->zone, data);
+}
+
+static inline void *dtpm_get_data(struct dtpm *dtpm)
+{
+	return powercap_get_zone_data(&dtpm->zone);
+}
+
 int dtpm_update_power(struct dtpm *dtpm);
 
 int dtpm_release_zone(struct powercap_zone *pcz);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/5] powercap/drivers/dtpm: Add CPU DT initialization support
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
  2021-11-26 18:14   ` Daniel Lezcano
  2021-11-26 18:14 ` [PATCH v2 3/5] powercap/drivers/dtpm: Add DT initialization support Daniel Lezcano
@ 2021-11-26 18:14 ` Daniel Lezcano
  2021-11-26 18:14 ` [PATCH v2 5/5] powercap/drivers/dtpm: Add dtpm devfreq with energy model support Daniel Lezcano
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Daniel Lezcano, Rafael J. Wysocki

Based on the previous DT changes in the core code, use the 'setup'
callback to initialize the CPU DTPM backend.

Code is reorganized to stick to the DTPM table description. No
functional changes.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/powercap/dtpm_cpu.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c
index 6bffb44c75aa..64cec0770803 100644
--- a/drivers/powercap/dtpm_cpu.c
+++ b/drivers/powercap/dtpm_cpu.c
@@ -21,6 +21,7 @@
 #include <linux/cpuhotplug.h>
 #include <linux/dtpm.h>
 #include <linux/energy_model.h>
+#include <linux/of.h>
 #include <linux/pm_qos.h>
 #include <linux/slab.h>
 #include <linux/units.h>
@@ -176,6 +177,17 @@ static int cpuhp_dtpm_cpu_offline(unsigned int cpu)
 }
 
 static int cpuhp_dtpm_cpu_online(unsigned int cpu)
+{
+	struct dtpm_cpu *dtpm_cpu;
+
+	dtpm_cpu = per_cpu(dtpm_per_cpu, cpu);
+	if (dtpm_cpu)
+		return dtpm_update_power(&dtpm_cpu->dtpm);
+
+	return 0;
+}
+
+static int __dtpm_cpu_setup(int cpu, struct dtpm *parent)
 {
 	struct dtpm_cpu *dtpm_cpu;
 	struct cpufreq_policy *policy;
@@ -183,6 +195,10 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
 	char name[CPUFREQ_NAME_LEN];
 	int ret = -ENOMEM;
 
+	dtpm_cpu = per_cpu(dtpm_per_cpu, cpu);
+	if (dtpm_cpu)
+		return 0;
+
 	policy = cpufreq_cpu_get(cpu);
 	if (!policy)
 		return 0;
@@ -191,10 +207,6 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
 	if (!pd)
 		return -EINVAL;
 
-	dtpm_cpu = per_cpu(dtpm_per_cpu, cpu);
-	if (dtpm_cpu)
-		return dtpm_update_power(&dtpm_cpu->dtpm);
-
 	dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL);
 	if (!dtpm_cpu)
 		return -ENOMEM;
@@ -207,7 +219,7 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
 
 	snprintf(name, sizeof(name), "cpu%d-cpufreq", dtpm_cpu->cpu);
 
-	ret = dtpm_register(name, &dtpm_cpu->dtpm, NULL);
+	ret = dtpm_register(name, &dtpm_cpu->dtpm, parent);
 	if (ret)
 		goto out_kfree_dtpm_cpu;
 
@@ -231,6 +243,17 @@ static int cpuhp_dtpm_cpu_online(unsigned int cpu)
 	return ret;
 }
 
+static int __init dtpm_cpu_setup(struct dtpm *dtpm, struct device_node *np)
+{
+	int cpu;
+
+	cpu = of_cpu_node_to_id(np);
+	if (cpu < 0)
+		return 0;
+
+	return __dtpm_cpu_setup(cpu, dtpm);
+}
+
 static int __init dtpm_cpu_init(void)
 {
 	int ret;
@@ -269,4 +292,4 @@ static int __init dtpm_cpu_init(void)
 	return 0;
 }
 
-DTPM_DECLARE(dtpm_cpu, dtpm_cpu_init, NULL);
+DTPM_DECLARE(dtpm_cpu, dtpm_cpu_init, dtpm_cpu_setup);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/5] powercap/drivers/dtpm: Add dtpm devfreq with energy model support
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
                   ` (2 preceding siblings ...)
  2021-11-26 18:14 ` [PATCH v2 4/5] powercap/drivers/dtpm: Add CPU " Daniel Lezcano
@ 2021-11-26 18:14 ` Daniel Lezcano
  2021-11-30 16:48 ` [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
  2021-12-01  9:23 ` Ulf Hansson
  5 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-26 18:14 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: robh, arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Chanwoo Choi, Kyungmin Park, MyungJoo Ham,
	Rafael J. Wysocki, Daniel Lezcano

Currently the dtpm supports the CPUs via cpufreq and the energy
model. This change provides the same for the device which supports
devfreq.

Each device supporting devfreq and having an energy model will be
added to the hierarchy if the corresponding powerzone is described in
the DT.

The concept is the same as the cpufreq DTPM support: the QoS is used
to aggregate the requests and the energy model gives the value of the
instantaneous power consumption ponderated by the load of the device.

Cc: Chanwoo Choi <cwchoi00@gmail.com>
Cc: Lukasz Luba <lukasz.luba@arm.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
   V1: Initial post
   V2:
     - Fixed missing prototype warning reported by lkp@
---
 drivers/powercap/Kconfig        |   7 ++
 drivers/powercap/Makefile       |   1 +
 drivers/powercap/dtpm_devfreq.c | 201 ++++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+)
 create mode 100644 drivers/powercap/dtpm_devfreq.c

diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
index b1ca339957e3..515e3ceb3393 100644
--- a/drivers/powercap/Kconfig
+++ b/drivers/powercap/Kconfig
@@ -57,4 +57,11 @@ config DTPM_CPU
 	help
 	  This enables support for CPU power limitation based on
 	  energy model.
+
+config DTPM_DEVFREQ
+	bool "Add device power capping based on the energy model"
+	depends on DTPM && ENERGY_MODEL
+	help
+	  This enables support for device power limitation based on
+	  energy model.
 endif
diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
index fabcf388a8d3..494617cdad88 100644
--- a/drivers/powercap/Makefile
+++ b/drivers/powercap/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_DTPM) += dtpm.o
 obj-$(CONFIG_DTPM_CPU) += dtpm_cpu.o
+obj-$(CONFIG_DTPM_DEVFREQ) += dtpm_devfreq.o
 obj-$(CONFIG_POWERCAP)	+= powercap_sys.o
 obj-$(CONFIG_INTEL_RAPL_CORE) += intel_rapl_common.o
 obj-$(CONFIG_INTEL_RAPL) += intel_rapl_msr.o
diff --git a/drivers/powercap/dtpm_devfreq.c b/drivers/powercap/dtpm_devfreq.c
new file mode 100644
index 000000000000..a1273eb54e80
--- /dev/null
+++ b/drivers/powercap/dtpm_devfreq.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2021 Linaro Limited
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ * The devfreq device combined with the energy model and the load can
+ * give an estimation of the power consumption as well as limiting the
+ * power.
+ *
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cpumask.h>
+#include <linux/devfreq.h>
+#include <linux/dtpm.h>
+#include <linux/energy_model.h>
+#include <linux/of.h>
+#include <linux/pm_qos.h>
+#include <linux/slab.h>
+#include <linux/units.h>
+
+struct dtpm_devfreq {
+	struct dtpm dtpm;
+	struct dev_pm_qos_request qos_req;
+	struct devfreq *devfreq;
+};
+
+static struct dtpm_devfreq *to_dtpm_devfreq(struct dtpm *dtpm)
+{
+	return container_of(dtpm, struct dtpm_devfreq, dtpm);
+}
+
+static int update_pd_power_uw(struct dtpm *dtpm)
+{
+	struct dtpm_devfreq *dtpm_devfreq = to_dtpm_devfreq(dtpm);
+	struct devfreq *devfreq = dtpm_devfreq->devfreq;
+	struct device *dev = devfreq->dev.parent;
+	struct em_perf_domain *pd = em_pd_get(dev);
+
+	dtpm->power_min = pd->table[0].power;
+	dtpm->power_min *= MICROWATT_PER_MILLIWATT;
+
+	dtpm->power_max = pd->table[pd->nr_perf_states - 1].power;
+	dtpm->power_max *= MICROWATT_PER_MILLIWATT;
+
+	return 0;
+}
+
+static u64 set_pd_power_limit(struct dtpm *dtpm, u64 power_limit)
+{
+	struct dtpm_devfreq *dtpm_devfreq = to_dtpm_devfreq(dtpm);
+	struct devfreq *devfreq = dtpm_devfreq->devfreq;
+	struct device *dev = devfreq->dev.parent;
+	struct em_perf_domain *pd = em_pd_get(dev);
+	unsigned long freq;
+	u64 power;
+	int i;
+
+	for (i = 0; i < pd->nr_perf_states; i++) {
+
+		power = pd->table[i].power * MICROWATT_PER_MILLIWATT;
+		if (power > power_limit)
+			break;
+	}
+
+	freq = pd->table[i - 1].frequency;
+
+	dev_pm_qos_update_request(&dtpm_devfreq->qos_req, freq);
+
+	power_limit = pd->table[i - 1].power * MICROWATT_PER_MILLIWATT;
+
+	return power_limit;
+}
+
+static void _normalize_load(struct devfreq_dev_status *status)
+{
+	if (status->total_time > 0xfffff) {
+		status->total_time >>= 10;
+		status->busy_time >>= 10;
+	}
+
+	status->busy_time <<= 10;
+	status->busy_time /= status->total_time ? : 1;
+
+	status->busy_time = status->busy_time ? : 1;
+	status->total_time = 1024;
+}
+
+static u64 get_pd_power_uw(struct dtpm *dtpm)
+{
+	struct dtpm_devfreq *dtpm_devfreq = to_dtpm_devfreq(dtpm);
+	struct devfreq *devfreq = dtpm_devfreq->devfreq;
+	struct device *dev = devfreq->dev.parent;
+	struct em_perf_domain *pd = em_pd_get(dev);
+	struct devfreq_dev_status status;
+	unsigned long freq;
+	u64 power;
+	int i;
+
+	mutex_lock(&devfreq->lock);
+	status = devfreq->last_status;
+	mutex_unlock(&devfreq->lock);
+
+	freq = DIV_ROUND_UP(status.current_frequency, HZ_PER_KHZ);
+	_normalize_load(&status);
+
+	for (i = 0; i < pd->nr_perf_states; i++) {
+
+		if (pd->table[i].frequency < freq)
+			continue;
+
+		power = pd->table[i].power * MICROWATT_PER_MILLIWATT;
+		power *= status.busy_time;
+		power >>= 10;
+
+		return power;
+	}
+
+	return 0;
+}
+
+static void pd_release(struct dtpm *dtpm)
+{
+	struct dtpm_devfreq *dtpm_devfreq = to_dtpm_devfreq(dtpm);
+
+	if (dev_pm_qos_request_active(&dtpm_devfreq->qos_req))
+		dev_pm_qos_remove_request(&dtpm_devfreq->qos_req);
+
+	kfree(dtpm_devfreq);
+}
+
+static struct dtpm_ops dtpm_ops = {
+	.set_power_uw = set_pd_power_limit,
+	.get_power_uw = get_pd_power_uw,
+	.update_power_uw = update_pd_power_uw,
+	.release = pd_release,
+};
+
+static int __dtpm_devfreq_setup(struct devfreq *devfreq, struct dtpm *parent)
+{
+	struct device *dev = devfreq->dev.parent;
+	struct dtpm_devfreq *dtpm_devfreq;
+	struct em_perf_domain *pd;
+	int ret = -ENOMEM;
+
+	pd = em_pd_get(dev);
+	if (!pd) {
+		ret = dev_pm_opp_of_register_em(dev, NULL);
+		if (ret) {
+			pr_err("No energy model available for '%s'\n", dev_name(dev));
+			return -EINVAL;
+		}
+	}
+
+	dtpm_devfreq = kzalloc(sizeof(*dtpm_devfreq), GFP_KERNEL);
+	if (!dtpm_devfreq)
+		return -ENOMEM;
+
+	dtpm_init(&dtpm_devfreq->dtpm, &dtpm_ops);
+
+	dtpm_devfreq->devfreq = devfreq;
+
+	ret = dtpm_register(dev_name(dev), &dtpm_devfreq->dtpm, parent);
+	if (ret) {
+		pr_err("Failed to register '%s': %d\n", dev_name(dev), ret);
+		goto out_dtpm_devfreq;
+	}
+
+	ret = dev_pm_qos_add_request(dev, &dtpm_devfreq->qos_req,
+				     DEV_PM_QOS_MAX_FREQUENCY,
+				     PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
+	if (ret) {
+		pr_err("Failed to add QoS request: %d\n", ret);
+		goto out_dtpm_unregister;
+	}
+
+	dtpm_update_power(&dtpm_devfreq->dtpm);
+
+	return 0;
+
+out_dtpm_unregister:
+	dtpm_unregister(&dtpm_devfreq->dtpm);
+out_dtpm_devfreq:
+	kfree(dtpm_devfreq);
+
+	return ret;
+}
+
+static int __init dtpm_devfreq_setup(struct dtpm *dtpm, struct device_node *np)
+{
+	struct devfreq *devfreq;
+
+	devfreq = devfreq_get_devfreq_by_node(np);
+	if (IS_ERR(devfreq))
+		return 0;
+
+	return __dtpm_devfreq_setup(devfreq, dtpm);
+}
+
+DTPM_DECLARE(dtpm_dev, NULL, dtpm_devfreq_setup);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/5] dt-bindings: Powerzone new bindings
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
                   ` (3 preceding siblings ...)
  2021-11-26 18:14 ` [PATCH v2 5/5] powercap/drivers/dtpm: Add dtpm devfreq with energy model support Daniel Lezcano
@ 2021-11-30 16:48 ` Daniel Lezcano
  2021-12-01  9:23 ` Ulf Hansson
  5 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-11-30 16:48 UTC (permalink / raw)
  To: robh, arnd, ulf.hansson, Rob Herring
  Cc: heiko, rjw, devicetree, linux-kernel, linux-pm, lukasz.luba,
	Arnd Bergmann


Hi,

I believe I took into account all the comments, is this bindings fine?

Thanks
   -- D.

On 26/11/2021 19:14, Daniel Lezcano wrote:
> The proposed bindings are describing a set of powerzones.
> 
> A power zone is the logical name for a component which is capable of
> power capping and where we can measure the power consumption.
> 
> A power zone can aggregate several power zones in terms of power
> measurement and power limitations. That allows to apply power
> constraint to a group of components and let the system balance the
> allocated power in order to comply with the constraint.
> 
> The ARM System Control and Management Interface (SCMI) can provide a
> power zone description.
> 
> The powerzone semantic is also found on the Intel platform with the
> RAPL register.
> 
> The Linux kernel powercap framework deals with the powerzones:
> 
> https://www.kernel.org/doc/html/latest/power/powercap/powercap.html
> 
> The powerzone can also represent a group of children powerzones, hence
> the description can result on a hierarchy. Such hierarchy already
> exists with the hardware or can be represented an computed from the
> kernel.
> 
> The hierarchical description was initially proposed but not desired
> given there are other descriptions like the power domain proposing
> almost the same description.
> 
> https://lore.kernel.org/all/CAL_JsqLuLcHj7525tTUmh7pLqe7T2j6UcznyhV7joS8ipyb_VQ@mail.gmail.com/
> 
> The description gives the power constraint dependencies to apply on a
> specific group of logically or physically aggregated devices. They do
> not represent the physical location or the power domains of the SoC
> even if the description could be similar.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>    V1: Initial post
>    V2:
>      - Added pattern properties and stick to powerzone-*
>      - Added required property compatible and powerzone-cells
>      - Added additionnal property
>      - Added compatible
>      - Renamed to 'powerzones'
>      - Added missing powerzone-cells to the topmost node
>      - Fixed errors reported by 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> ---
>  .../devicetree/bindings/power/powerzones.yaml | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/powerzones.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/powerzones.yaml b/Documentation/devicetree/bindings/power/powerzones.yaml
> new file mode 100644
> index 000000000000..6e63bbc750c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/powerzones.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/powerzones.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Power zones description
> +
> +maintainers:
> +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> +
> +description: |+
> +
> +  A System on Chip contains a multitude of active components and each
> +  of them is a source of heat. Even if a temperature sensor is not
> +  present, a source of heat can be controlled by acting on the
> +  consumed power via different techniques.
> +
> +  A powerzone describes a component or a group of components where we
> +  can control the maximum power consumption. For instance, a group of
> +  CPUs via the performance domain, a LCD screen via the brightness,
> +  etc ...
> +
> +  Different components when they are used together can significantly
> +  increase the overall temperature, so the description needs to
> +  reflect this dependency in order to assign a power budget for a
> +  group of powerzones.
> +
> +  This description is done via a hierarchy and the DT reflects it. It
> +  does not represent the physical location or a topology, eg. on a
> +  big.Little system, the little CPUs may not be represented as they do
> +  not contribute significantly to the heat, however the GPU can be
> +  tied with the big CPUs as they usually have a connection for
> +  multimedia or game workloads.
> +    
> +properties:
> +  $nodename:
> +    const: powerzones
> +
> +  compatible:
> +    const: powerzones
> +
> +patternProperties:
> +  "^(powerzone)([@-].*)?$":
> +    type: object
> +    description:
> +      A node representing a powerzone acting as an aggregator for all
> +      its children powerzones.
> +
> +    properties:
> +      "#powerzone-cells":
> +        description:
> +          Number of cells in powerzone specifier. Typically 0 for nodes
> +          representing but it can be any number in the future to
> +          describe parameters of the powerzone.
> +
> +      powerzones:
> +        description:
> +          A phandle to a parent powerzone. If no powerzone attribute is
> +          set, the described powerzone is the topmost in the hierarchy.
> +
> +    required:
> +      - "#powerzone-cells"
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    powerzones {
> +
> +      compatible = "powerzones";
> +
> +      #powerzone-cells = <0>;
> +
> +      SOC_PZ: powerzone-soc {
> +        #powerzone-cells = <0>;
> +      };
> +
> +      PKG_PZ: powerzone-pkg {
> +        #powerzone-cells = <0>;
> +        powerzones = <&SOC_PZ>;
> +      };
> +
> +      GPU_PZ: powerzone-gpu {
> +        #powerzone-cells = <0>;
> +        powerzones = <&PKG_PZ>;
> +      };
> +    };
> +
> +  - |
> +    A57_0: big@0 {
> +      compatible = "arm,cortex-a57";
> +      reg = <0x0 0x0>;
> +      device_type = "cpu";
> +      #powerzone-cells = <0>;
> +      powerzones = <&PKG_PZ>;
> +    };
> +
> +    A57_1: big@1 {
> +      compatible = "arm,cortex-a57";
> +      reg = <0x0 0x0>;
> +      device_type = "cpu";
> +      #powerzone-cells = <0>;
> +      powerzones = <&PKG_PZ>;
> +    };
> +...
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/5] dt-bindings: Powerzone new bindings
  2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
                   ` (4 preceding siblings ...)
  2021-11-30 16:48 ` [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
@ 2021-12-01  9:23 ` Ulf Hansson
  2021-12-01 14:43   ` Ulf Hansson
  5 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2021-12-01  9:23 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: robh, arnd, heiko, rjw, devicetree, linux-kernel, linux-pm,
	lukasz.luba, Arnd Bergmann, Rob Herring

On Fri, 26 Nov 2021 at 19:15, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> The proposed bindings are describing a set of powerzones.
>
> A power zone is the logical name for a component which is capable of
> power capping and where we can measure the power consumption.
>
> A power zone can aggregate several power zones in terms of power
> measurement and power limitations. That allows to apply power
> constraint to a group of components and let the system balance the
> allocated power in order to comply with the constraint.
>
> The ARM System Control and Management Interface (SCMI) can provide a
> power zone description.
>
> The powerzone semantic is also found on the Intel platform with the
> RAPL register.
>
> The Linux kernel powercap framework deals with the powerzones:
>
> https://www.kernel.org/doc/html/latest/power/powercap/powercap.html
>
> The powerzone can also represent a group of children powerzones, hence
> the description can result on a hierarchy. Such hierarchy already
> exists with the hardware or can be represented an computed from the
> kernel.
>
> The hierarchical description was initially proposed but not desired
> given there are other descriptions like the power domain proposing
> almost the same description.
>
> https://lore.kernel.org/all/CAL_JsqLuLcHj7525tTUmh7pLqe7T2j6UcznyhV7joS8ipyb_VQ@mail.gmail.com/
>
> The description gives the power constraint dependencies to apply on a
> specific group of logically or physically aggregated devices. They do
> not represent the physical location or the power domains of the SoC
> even if the description could be similar.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>    V1: Initial post
>    V2:
>      - Added pattern properties and stick to powerzone-*
>      - Added required property compatible and powerzone-cells
>      - Added additionnal property
>      - Added compatible
>      - Renamed to 'powerzones'
>      - Added missing powerzone-cells to the topmost node
>      - Fixed errors reported by 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> ---
>  .../devicetree/bindings/power/powerzones.yaml | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/powerzones.yaml
>
> diff --git a/Documentation/devicetree/bindings/power/powerzones.yaml b/Documentation/devicetree/bindings/power/powerzones.yaml
> new file mode 100644
> index 000000000000..6e63bbc750c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/powerzones.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/powerzones.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Power zones description
> +
> +maintainers:
> +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> +
> +description: |+
> +
> +  A System on Chip contains a multitude of active components and each
> +  of them is a source of heat. Even if a temperature sensor is not
> +  present, a source of heat can be controlled by acting on the
> +  consumed power via different techniques.
> +
> +  A powerzone describes a component or a group of components where we
> +  can control the maximum power consumption. For instance, a group of
> +  CPUs via the performance domain, a LCD screen via the brightness,
> +  etc ...
> +
> +  Different components when they are used together can significantly
> +  increase the overall temperature, so the description needs to
> +  reflect this dependency in order to assign a power budget for a
> +  group of powerzones.
> +
> +  This description is done via a hierarchy and the DT reflects it. It
> +  does not represent the physical location or a topology, eg. on a
> +  big.Little system, the little CPUs may not be represented as they do
> +  not contribute significantly to the heat, however the GPU can be
> +  tied with the big CPUs as they usually have a connection for
> +  multimedia or game workloads.
> +
> +properties:
> +  $nodename:
> +    const: powerzones
> +
> +  compatible:
> +    const: powerzones

This looks odd. Why do we need const compatible string? Shouldn't this
be allowed to be an SoC-powerzone specific compatible?

> +
> +patternProperties:
> +  "^(powerzone)([@-].*)?$":
> +    type: object
> +    description:
> +      A node representing a powerzone acting as an aggregator for all
> +      its children powerzones.
> +
> +    properties:
> +      "#powerzone-cells":
> +        description:
> +          Number of cells in powerzone specifier. Typically 0 for nodes
> +          representing but it can be any number in the future to
> +          describe parameters of the powerzone.
> +
> +      powerzones:
> +        description:
> +          A phandle to a parent powerzone. If no powerzone attribute is
> +          set, the described powerzone is the topmost in the hierarchy.
> +
> +    required:
> +      - "#powerzone-cells"
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    powerzones {
> +
> +      compatible = "powerzones";
> +
> +      #powerzone-cells = <0>;
> +
> +      SOC_PZ: powerzone-soc {
> +        #powerzone-cells = <0>;
> +      };
> +
> +      PKG_PZ: powerzone-pkg {
> +        #powerzone-cells = <0>;
> +        powerzones = <&SOC_PZ>;
> +      };
> +
> +      GPU_PZ: powerzone-gpu {
> +        #powerzone-cells = <0>;
> +        powerzones = <&PKG_PZ>;
> +      };
> +    };
> +
> +  - |
> +    A57_0: big@0 {
> +      compatible = "arm,cortex-a57";
> +      reg = <0x0 0x0>;
> +      device_type = "cpu";
> +      #powerzone-cells = <0>;
> +      powerzones = <&PKG_PZ>;
> +    };

I think we discussed this in the earlier version too...

The above example describes a powerzone provider, but it doesn't
really conform to the binding. That's because the binding states that
powerzone providers should be inside a top-level "powerzone {" node.

I am wondering if we really need the toplevel "powerzone" node.

> +
> +    A57_1: big@1 {
> +      compatible = "arm,cortex-a57";
> +      reg = <0x0 0x0>;
> +      device_type = "cpu";
> +      #powerzone-cells = <0>;
> +      powerzones = <&PKG_PZ>;
> +    };
> +...
> --
> 2.25.1
>

No further comments from my side.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/5] dt-bindings: Powerzone new bindings
  2021-12-01  9:23 ` Ulf Hansson
@ 2021-12-01 14:43   ` Ulf Hansson
  0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2021-12-01 14:43 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: robh, arnd, heiko, rjw, devicetree, linux-kernel, linux-pm,
	lukasz.luba, Arnd Bergmann, Rob Herring

On Wed, 1 Dec 2021 at 10:23, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 26 Nov 2021 at 19:15, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >
> > The proposed bindings are describing a set of powerzones.
> >
> > A power zone is the logical name for a component which is capable of
> > power capping and where we can measure the power consumption.
> >
> > A power zone can aggregate several power zones in terms of power
> > measurement and power limitations. That allows to apply power
> > constraint to a group of components and let the system balance the
> > allocated power in order to comply with the constraint.
> >
> > The ARM System Control and Management Interface (SCMI) can provide a
> > power zone description.
> >
> > The powerzone semantic is also found on the Intel platform with the
> > RAPL register.
> >
> > The Linux kernel powercap framework deals with the powerzones:
> >
> > https://www.kernel.org/doc/html/latest/power/powercap/powercap.html
> >
> > The powerzone can also represent a group of children powerzones, hence
> > the description can result on a hierarchy. Such hierarchy already
> > exists with the hardware or can be represented an computed from the
> > kernel.
> >
> > The hierarchical description was initially proposed but not desired
> > given there are other descriptions like the power domain proposing
> > almost the same description.
> >
> > https://lore.kernel.org/all/CAL_JsqLuLcHj7525tTUmh7pLqe7T2j6UcznyhV7joS8ipyb_VQ@mail.gmail.com/
> >
> > The description gives the power constraint dependencies to apply on a
> > specific group of logically or physically aggregated devices. They do
> > not represent the physical location or the power domains of the SoC
> > even if the description could be similar.
> >
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> >    V1: Initial post
> >    V2:
> >      - Added pattern properties and stick to powerzone-*
> >      - Added required property compatible and powerzone-cells
> >      - Added additionnal property
> >      - Added compatible
> >      - Renamed to 'powerzones'
> >      - Added missing powerzone-cells to the topmost node
> >      - Fixed errors reported by 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > ---
> >  .../devicetree/bindings/power/powerzones.yaml | 109 ++++++++++++++++++
> >  1 file changed, 109 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/powerzones.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/power/powerzones.yaml b/Documentation/devicetree/bindings/power/powerzones.yaml
> > new file mode 100644
> > index 000000000000..6e63bbc750c6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/powerzones.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/powerzones.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Power zones description
> > +
> > +maintainers:
> > +  - Daniel Lezcano <daniel.lezcano@linaro.org>
> > +
> > +description: |+
> > +
> > +  A System on Chip contains a multitude of active components and each
> > +  of them is a source of heat. Even if a temperature sensor is not
> > +  present, a source of heat can be controlled by acting on the
> > +  consumed power via different techniques.
> > +
> > +  A powerzone describes a component or a group of components where we
> > +  can control the maximum power consumption. For instance, a group of
> > +  CPUs via the performance domain, a LCD screen via the brightness,
> > +  etc ...
> > +
> > +  Different components when they are used together can significantly
> > +  increase the overall temperature, so the description needs to
> > +  reflect this dependency in order to assign a power budget for a
> > +  group of powerzones.
> > +
> > +  This description is done via a hierarchy and the DT reflects it. It
> > +  does not represent the physical location or a topology, eg. on a
> > +  big.Little system, the little CPUs may not be represented as they do
> > +  not contribute significantly to the heat, however the GPU can be
> > +  tied with the big CPUs as they usually have a connection for
> > +  multimedia or game workloads.
> > +
> > +properties:
> > +  $nodename:
> > +    const: powerzones
> > +
> > +  compatible:
> > +    const: powerzones
>
> This looks odd. Why do we need const compatible string? Shouldn't this
> be allowed to be an SoC-powerzone specific compatible?

Alright, after our recent discussions offlist, I believe the
compatible property should be entirely removed.

>
> > +
> > +patternProperties:
> > +  "^(powerzone)([@-].*)?$":
> > +    type: object
> > +    description:
> > +      A node representing a powerzone acting as an aggregator for all
> > +      its children powerzones.
> > +
> > +    properties:
> > +      "#powerzone-cells":
> > +        description:
> > +          Number of cells in powerzone specifier. Typically 0 for nodes
> > +          representing but it can be any number in the future to
> > +          describe parameters of the powerzone.
> > +
> > +      powerzones:
> > +        description:
> > +          A phandle to a parent powerzone. If no powerzone attribute is
> > +          set, the described powerzone is the topmost in the hierarchy.
> > +
> > +    required:
> > +      - "#powerzone-cells"
> > +
> > +required:
> > +  - compatible

This should be removed too, of course.

> > +
> > +additionalProperties: true

This should be set to "false", I think. There is no need for any
additional properties besides those that are being part of the binding
above.

> > +
> > +examples:
> > +  - |
> > +    powerzones {
> > +
> > +      compatible = "powerzones";
> > +
> > +      #powerzone-cells = <0>;

This toplevel "powerzones" node, should neither contain a compatible
nor a #powerzone-cells. Please drop this.

Instead we only need to describe the topology by using child nodes, as
in the example below.

> > +
> > +      SOC_PZ: powerzone-soc {
> > +        #powerzone-cells = <0>;
> > +      };
> > +
> > +      PKG_PZ: powerzone-pkg {
> > +        #powerzone-cells = <0>;
> > +        powerzones = <&SOC_PZ>;
> > +      };
> > +
> > +      GPU_PZ: powerzone-gpu {
> > +        #powerzone-cells = <0>;
> > +        powerzones = <&PKG_PZ>;
> > +      };
> > +    };
> > +
> > +  - |
> > +    A57_0: big@0 {
> > +      compatible = "arm,cortex-a57";
> > +      reg = <0x0 0x0>;
> > +      device_type = "cpu";
> > +      #powerzone-cells = <0>;
> > +      powerzones = <&PKG_PZ>;
> > +    };
>
> I think we discussed this in the earlier version too...
>
> The above example describes a powerzone provider, but it doesn't
> really conform to the binding. That's because the binding states that
> powerzone providers should be inside a top-level "powerzone {" node.

From our offlist discussion, it seems like the cpu nodes should not
have a #powerzone-cells. Instead, the powerzones property should be
sufficient, as it allows you to describe what powerzone(s) the cpu
belongs to, which is exactly what you need.

This also means that we need to extend the DT bindings for CPUs
(Documentation/devicetree/bindings/arm/cpus.yaml), to allow cpu nodes
to have a "powerzones" property. I believe we can do that separately,
on top of $subject patch, as cpus.yaml has "additionalProperties:
true".

>
> I am wondering if we really need the toplevel "powerzone" node.

Please ignore this comment. It has become clear to me that the
toplevel node serves a purpose.

>
> > +
> > +    A57_1: big@1 {
> > +      compatible = "arm,cortex-a57";
> > +      reg = <0x0 0x0>;
> > +      device_type = "cpu";
> > +      #powerzone-cells = <0>;

Ditto.

> > +      powerzones = <&PKG_PZ>;
> > +    };
> > +...
> > --
> > 2.25.1
> >
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
  2021-11-26 18:14   ` Daniel Lezcano
  (?)
@ 2021-12-07 18:41     ` Rob Herring
  -1 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2021-12-07 18:41 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

On Fri, Nov 26, 2021 at 12:15 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> Add the powerzones description. This first step introduces the big,
> the little and the gpu as a powerzone place.
>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>    V1: Initial post
>    V2:
>      - Move description in the SoC dtsi specific file
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index d3cdf6f42a30..3c0dbc0cb2bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l1: cpu@1 {
> @@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l2: cpu@2 {
> @@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l3: cpu@3 {
> @@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_b0: cpu@100 {
> @@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -791,6 +803,17 @@ spi5: spi@ff200000 {
>                 status = "disabled";
>         };
>
> +       powerzones {
> +
> +               PKG_PZ: pkg {
> +                       #powerzone-cells = <0>;
> +                        powerzone = <&SOC_PZ>;
> +               };
> +
> +               SOC_PZ: soc {
> +               };
> +       };
> +
>         thermal_zones: thermal-zones {
>                 cpu_thermal: cpu-thermal {
>                         polling-delay-passive = <100>;
> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>                 clocks = <&cru ACLK_GPU>;
>                 #cooling-cells = <2>;
>                 power-domains = <&power RK3399_PD_GPU>;
> +               #powerzone-cells = <0>;
> +               powerzone = <&PKG_PZ>;

Every CPU and the GPU are in the same powerzone. What is the point? Do
you really have to be told that CPUs and GPU are a source of heat and
might need to be limited?

Rob

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-12-07 18:41     ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2021-12-07 18:41 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

On Fri, Nov 26, 2021 at 12:15 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> Add the powerzones description. This first step introduces the big,
> the little and the gpu as a powerzone place.
>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>    V1: Initial post
>    V2:
>      - Move description in the SoC dtsi specific file
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index d3cdf6f42a30..3c0dbc0cb2bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l1: cpu@1 {
> @@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l2: cpu@2 {
> @@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l3: cpu@3 {
> @@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_b0: cpu@100 {
> @@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -791,6 +803,17 @@ spi5: spi@ff200000 {
>                 status = "disabled";
>         };
>
> +       powerzones {
> +
> +               PKG_PZ: pkg {
> +                       #powerzone-cells = <0>;
> +                        powerzone = <&SOC_PZ>;
> +               };
> +
> +               SOC_PZ: soc {
> +               };
> +       };
> +
>         thermal_zones: thermal-zones {
>                 cpu_thermal: cpu-thermal {
>                         polling-delay-passive = <100>;
> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>                 clocks = <&cru ACLK_GPU>;
>                 #cooling-cells = <2>;
>                 power-domains = <&power RK3399_PD_GPU>;
> +               #powerzone-cells = <0>;
> +               powerzone = <&PKG_PZ>;

Every CPU and the GPU are in the same powerzone. What is the point? Do
you really have to be told that CPUs and GPU are a source of heat and
might need to be limited?

Rob

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-12-07 18:41     ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2021-12-07 18:41 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

On Fri, Nov 26, 2021 at 12:15 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> Add the powerzones description. This first step introduces the big,
> the little and the gpu as a powerzone place.
>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>    V1: Initial post
>    V2:
>      - Move description in the SoC dtsi specific file
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index d3cdf6f42a30..3c0dbc0cb2bc 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -76,6 +76,8 @@ cpu_l0: cpu@0 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l1: cpu@1 {
> @@ -88,6 +90,8 @@ cpu_l1: cpu@1 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l2: cpu@2 {
> @@ -100,6 +104,8 @@ cpu_l2: cpu@2 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_l3: cpu@3 {
> @@ -112,6 +118,8 @@ cpu_l3: cpu@3 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <100>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>                 };
>
>                 cpu_b0: cpu@100 {
> @@ -124,6 +132,8 @@ cpu_b0: cpu@100 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -142,6 +152,8 @@ cpu_b1: cpu@101 {
>                         #cooling-cells = <2>; /* min followed by max */
>                         dynamic-power-coefficient = <436>;
>                         cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
> +                       #powerzone-cells = <0>;
> +                       powerzone = <&PKG_PZ>;
>
>                         thermal-idle {
>                                 #cooling-cells = <2>;
> @@ -791,6 +803,17 @@ spi5: spi@ff200000 {
>                 status = "disabled";
>         };
>
> +       powerzones {
> +
> +               PKG_PZ: pkg {
> +                       #powerzone-cells = <0>;
> +                        powerzone = <&SOC_PZ>;
> +               };
> +
> +               SOC_PZ: soc {
> +               };
> +       };
> +
>         thermal_zones: thermal-zones {
>                 cpu_thermal: cpu-thermal {
>                         polling-delay-passive = <100>;
> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>                 clocks = <&cru ACLK_GPU>;
>                 #cooling-cells = <2>;
>                 power-domains = <&power RK3399_PD_GPU>;
> +               #powerzone-cells = <0>;
> +               powerzone = <&PKG_PZ>;

Every CPU and the GPU are in the same powerzone. What is the point? Do
you really have to be told that CPUs and GPU are a source of heat and
might need to be limited?

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
  2021-12-07 18:41     ` Rob Herring
  (?)
@ 2021-12-08 16:16       ` Daniel Lezcano
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-12-08 16:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support


Hi Rob,

On 07/12/2021 19:41, Rob Herring wrote:

[ ... ]

>>         thermal_zones: thermal-zones {
>>                 cpu_thermal: cpu-thermal {
>>                         polling-delay-passive = <100>;
>> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>>                 clocks = <&cru ACLK_GPU>;
>>                 #cooling-cells = <2>;
>>                 power-domains = <&power RK3399_PD_GPU>;
>> +               #powerzone-cells = <0>;
>> +               powerzone = <&PKG_PZ>;
> 
> Every CPU and the GPU are in the same powerzone. What is the point? Do
> you really have to be told that CPUs and GPU are a source of heat and
> might need to be limited?

A powerzone ==> can read power && set power limit

Every CPU is a powerzone as well as the GPU.

They are all grouped under PKG_PZ.

That means we have:

 pkg
  |-- cpu0-3
  |
  |-- cpu4-7
  |
  `-- gpu

We can read the power consumption of cpu0-3, cpu4-7 or gpu and set their
power limit.

We can read the power consumption of pkg (which is the sum of the power
consumption of cpu0-3, cpu4-7 and gpu) and I can set the power limit
which will ensure powerof(cpu0-3 + cpu4-7 + gpu) <= powerof(pkg).



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-12-08 16:16       ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-12-08 16:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support


Hi Rob,

On 07/12/2021 19:41, Rob Herring wrote:

[ ... ]

>>         thermal_zones: thermal-zones {
>>                 cpu_thermal: cpu-thermal {
>>                         polling-delay-passive = <100>;
>> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>>                 clocks = <&cru ACLK_GPU>;
>>                 #cooling-cells = <2>;
>>                 power-domains = <&power RK3399_PD_GPU>;
>> +               #powerzone-cells = <0>;
>> +               powerzone = <&PKG_PZ>;
> 
> Every CPU and the GPU are in the same powerzone. What is the point? Do
> you really have to be told that CPUs and GPU are a source of heat and
> might need to be limited?

A powerzone ==> can read power && set power limit

Every CPU is a powerzone as well as the GPU.

They are all grouped under PKG_PZ.

That means we have:

 pkg
  |-- cpu0-3
  |
  |-- cpu4-7
  |
  `-- gpu

We can read the power consumption of cpu0-3, cpu4-7 or gpu and set their
power limit.

We can read the power consumption of pkg (which is the sum of the power
consumption of cpu0-3, cpu4-7 and gpu) and I can set the power limit
which will ensure powerof(cpu0-3 + cpu4-7 + gpu) <= powerof(pkg).



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960
@ 2021-12-08 16:16       ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2021-12-08 16:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: arnd, heiko, ulf.hansson, rjw, devicetree, linux-kernel,
	linux-pm, lukasz.luba, Robin Murphy, Johan Jonker, Helen Koike,
	Brian Norris, Shunqian Zheng, Elaine Zhang,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support


Hi Rob,

On 07/12/2021 19:41, Rob Herring wrote:

[ ... ]

>>         thermal_zones: thermal-zones {
>>                 cpu_thermal: cpu-thermal {
>>                         polling-delay-passive = <100>;
>> @@ -2027,6 +2050,8 @@ gpu: gpu@ff9a0000 {
>>                 clocks = <&cru ACLK_GPU>;
>>                 #cooling-cells = <2>;
>>                 power-domains = <&power RK3399_PD_GPU>;
>> +               #powerzone-cells = <0>;
>> +               powerzone = <&PKG_PZ>;
> 
> Every CPU and the GPU are in the same powerzone. What is the point? Do
> you really have to be told that CPUs and GPU are a source of heat and
> might need to be limited?

A powerzone ==> can read power && set power limit

Every CPU is a powerzone as well as the GPU.

They are all grouped under PKG_PZ.

That means we have:

 pkg
  |-- cpu0-3
  |
  |-- cpu4-7
  |
  `-- gpu

We can read the power consumption of cpu0-3, cpu4-7 or gpu and set their
power limit.

We can read the power consumption of pkg (which is the sum of the power
consumption of cpu0-3, cpu4-7 and gpu) and I can set the power limit
which will ensure powerof(cpu0-3 + cpu4-7 + gpu) <= powerof(pkg).



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-12-08 16:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26 18:14 [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
2021-11-26 18:14 ` [PATCH v2 2/5] arm64: dts: rockchip: Add powerzones definition for rock960 Daniel Lezcano
2021-11-26 18:14   ` Daniel Lezcano
2021-11-26 18:14   ` Daniel Lezcano
2021-12-07 18:41   ` Rob Herring
2021-12-07 18:41     ` Rob Herring
2021-12-07 18:41     ` Rob Herring
2021-12-08 16:16     ` Daniel Lezcano
2021-12-08 16:16       ` Daniel Lezcano
2021-12-08 16:16       ` Daniel Lezcano
2021-11-26 18:14 ` [PATCH v2 3/5] powercap/drivers/dtpm: Add DT initialization support Daniel Lezcano
2021-11-26 18:14 ` [PATCH v2 4/5] powercap/drivers/dtpm: Add CPU " Daniel Lezcano
2021-11-26 18:14 ` [PATCH v2 5/5] powercap/drivers/dtpm: Add dtpm devfreq with energy model support Daniel Lezcano
2021-11-30 16:48 ` [PATCH v2 1/5] dt-bindings: Powerzone new bindings Daniel Lezcano
2021-12-01  9:23 ` Ulf Hansson
2021-12-01 14:43   ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.