lkml.org 
[lkml]   [2021]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/5] dt-bindings: Powerzone new bindings
On Wed, 24 Nov 2021 at 13:55, 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>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> .../devicetree/bindings/power/powerzones.yaml | 95 +++++++++++++++++++
> 1 file changed, 95 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..1ae3f82ae29c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/powerzones.yaml
> @@ -0,0 +1,95 @@
> +# 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
> +

Do we really need a top-node like this? Can't that be left as a
platform/soc specific thing instead? Along the lines of how the last
example below looks like? Maybe we can have both options? I guess Rob
will tell us.

Moreover, maybe we should put some constraints on the names of
subnodes (provider nodes) with a "patternProperties". Something along
the lines of below.

patternProperties:
"^(powerzone)([@-].*)?$":
type: object
description:
Each node represents a powerzone.

> + "#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.
> +
> + powerzone:

Maybe "powerzones" instead of "powerzone". Unless we believe that we
never need to allow multiple parent-zones for a child-zone.

> + description:
> + A phandle to a parent powerzone. If no powerzone attribute is set, the
> + described powerzone is the topmost in the hierarchy.
> +

We should probably state that the "#powerzone-cells" are required. Like below:

required:
- "#powerzone-cells"

Moreover, we probably need to allow additional properties? At least it
looks so from the last example below. Then:

additionalProperties: true

> +examples:
> + - |
> + powerzones {
> +
> + SOC_PZ: soc {
> + };

This looks odd to me.

Why do we need an empty node? If this is the topmost power-zone, it
should still have the #powerzone-cells specifier, I think.

> +
> + PKG_PZ: pkg {

As I stated above, I would prefer some kind of common pattern of the
subnode names. Maybe "pkg-powerzone"?

> + #powerzone-cells = <0>;
> + powerzone = <&SOC_PZ>;
> + };
> +
> + BIG_PZ: big {
> + #powerzone-cells = <0>;
> + powerzone = <&PKG_PZ>;
> + };
> +
> + GPU_PZ: gpu {
> + #powerzone-cells = <0>;
> + powerzone = <&PKG_PZ>;
> + };
> +
> + MULTIMEDIA_PZ: multimedia {
> + #powerzone-cells = <0>;
> + powerzone = <&SOC_PZ>;
> + };
> + };
> +
> + - |
> + A57_0: big@0 {
> + compatible = "arm,cortex-a57";
> + reg = <0x0 0x0>;
> + device_type = "cpu";
> + #powerzone-cells = <0>;
> + powerzone = <&BIG_PZ>;

Just to make sure I understand correctly. The big@0 node is a
powerzone provider too? Or did you mean to specify it as a consumer?

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

Kind regards
Uffe

\
 
 \ /
  Last update: 2021-11-24 15:56    [W:0.100 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site