2022-02-22 04:56:43

by Lukasz Luba

[permalink] [raw]
Subject: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

Add DT bindings for the Energy Model information.

Signed-off-by: Lukasz Luba <[email protected]>
---
.../bindings/power/energy-model.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/energy-model.yaml

diff --git a/Documentation/devicetree/bindings/power/energy-model.yaml b/Documentation/devicetree/bindings/power/energy-model.yaml
new file mode 100644
index 000000000000..804a9b324925
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/energy-model.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/energy-model.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Energy Model Bindings
+
+maintainers:
+ - Lukasz Luba <[email protected]>
+
+description: |+
+ Devices work at specific performance states (frequencies). The power which
+ is used at a given performance state is an important information. A framework
+ which maintains this information is Energy Model. This document defines
+ bindings for these Energy Model performance states applicable across wide
+ range of devices. For illustration purpose, this document uses GPU as a device.
+
+ This binding only supports frequency-power pairs.
+
+select: true
+
+properties:
+ operating-points:
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ items:
+ items:
+ - description: Frequency in kHz
+ - description: Power in uW
+
+
+additionalProperties: true
+examples:
+ {
+ gpu_energy_model: energy-model {
+ compatible = "energy-model";
+ energy-model-entries = <
+ 200000 300000
+ 297000 500000
+ 400000 800000
+ 500000 1400000
+ 600000 2000000
+ 800000 2800000
+ >;
+ };
+ };
+
+ &gpu {
+ energy-model = <&gpu_energy_model>;
+ };
+...
--
2.17.1


2022-02-22 05:33:12

by Viresh Kumar

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

On 21-02-22, 22:51, Lukasz Luba wrote:
> Add DT bindings for the Energy Model information.
>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> .../bindings/power/energy-model.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/energy-model.yaml
>
> diff --git a/Documentation/devicetree/bindings/power/energy-model.yaml b/Documentation/devicetree/bindings/power/energy-model.yaml
> new file mode 100644
> index 000000000000..804a9b324925
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/energy-model.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/energy-model.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Energy Model Bindings
> +
> +maintainers:
> + - Lukasz Luba <[email protected]>
> +
> +description: |+
> + Devices work at specific performance states (frequencies). The power which
> + is used at a given performance state is an important information. A framework
> + which maintains this information is Energy Model. This document defines
> + bindings for these Energy Model performance states applicable across wide
> + range of devices. For illustration purpose, this document uses GPU as a device.
> +
> + This binding only supports frequency-power pairs.
> +
> +select: true
> +
> +properties:
> + operating-points:
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> + items:
> + items:
> + - description: Frequency in kHz
> + - description: Power in uW
> +
> +
> +additionalProperties: true
> +examples:
> + {
> + gpu_energy_model: energy-model {
> + compatible = "energy-model";
> + energy-model-entries = <
> + 200000 300000
> + 297000 500000
> + 400000 800000
> + 500000 1400000
> + 600000 2000000
> + 800000 2800000
> + >;
> + };
> + };
> +
> + &gpu {
> + energy-model = <&gpu_energy_model>;
> + };

What about getting this from the OPP table instead? There is no point adding
similar tables for a device.

--
viresh

2022-02-22 08:10:14

by Lukasz Luba

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

Hi Viresh,

Thanks for having a look at it.

On 2/22/22 03:03, Viresh Kumar wrote:
> On 21-02-22, 22:51, Lukasz Luba wrote:
>> Add DT bindings for the Energy Model information.
>>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> .../bindings/power/energy-model.yaml | 51 +++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/power/energy-model.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/power/energy-model.yaml b/Documentation/devicetree/bindings/power/energy-model.yaml
>> new file mode 100644
>> index 000000000000..804a9b324925
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/energy-model.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/energy-model.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Energy Model Bindings
>> +
>> +maintainers:
>> + - Lukasz Luba <[email protected]>
>> +
>> +description: |+
>> + Devices work at specific performance states (frequencies). The power which
>> + is used at a given performance state is an important information. A framework
>> + which maintains this information is Energy Model. This document defines
>> + bindings for these Energy Model performance states applicable across wide
>> + range of devices. For illustration purpose, this document uses GPU as a device.
>> +
>> + This binding only supports frequency-power pairs.
>> +
>> +select: true
>> +
>> +properties:
>> + operating-points:
>> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
>> + items:
>> + items:
>> + - description: Frequency in kHz
>> + - description: Power in uW
>> +
>> +
>> +additionalProperties: true
>> +examples:
>> + {
>> + gpu_energy_model: energy-model {
>> + compatible = "energy-model";
>> + energy-model-entries = <
>> + 200000 300000
>> + 297000 500000
>> + 400000 800000
>> + 500000 1400000
>> + 600000 2000000
>> + 800000 2800000
>> + >;
>> + };
>> + };
>> +
>> + &gpu {
>> + energy-model = <&gpu_energy_model>;
>> + };
>
> What about getting this from the OPP table instead? There is no point adding
> similar tables for a device.
>

I'm not sure if that would be flexible enough to meet the requirement:
power for each OPP might be different in one board vs. other board.

Power can be different because of static power, which might vary due to
different temperature that the SoC operates at a particular OPP. It can
be due to: better heat sink (or no at all like on dev board), bigger PCB
with fat copper layers, different location of hot devices (like PMIC) on
the PCB, missing some hot devices on the PCB (fast charger), case, etc.

The 'advanced' EM and this DT array allows to provide avg power from
measurements for a particular board and for each OPP independently.

AFAIK the OPP definition is more SoC specific. I'm particularly
interested in this SC7180 SoC and it's GPU power [1]. There is
also a nice OPP definition in that node.
As you can see there is one SoC file and quite a few boards next to
it. Some of them have a decent thermal design (inside Chromebook) but
some are 'just' dev boards. The power would be different for those
boards.

Similar issue would be e.g. for Rk3399 SoC (Chromebook, Pine64, IoT and
some dev boards).

IMO the OPP table might be to much hassle and heavy for those boards.

[1]
https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/qcom/sc7180.dtsi#L1953

2022-02-22 10:33:52

by Lukasz Luba

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings



On 2/22/22 09:45, Viresh Kumar wrote:
> On 22-02-22, 08:06, Lukasz Luba wrote:
>> I'm not sure if that would be flexible enough to meet the requirement:
>> power for each OPP might be different in one board vs. other board.
>
> Don't DT files overload values from board files all the time ? Why wouldn't the
> same apply for OPP table as well ?

In that SoC and family of the boards, there are no such examples.
It used to be popular in arm32 boards, but I'm not sure nowadays.

>
>> AFAIK the OPP definition is more SoC specific.
>
> This isn't about OPP definition as well, but just that if DT allows you to
> override or not. I think it will.
>

Redefining the whole OPP table, when the freq, voltage, interconnect,
and other old entries don't change isn't too messy?

As I said, I would prefer something lightweight, not redefining all
stuff from OPP in every board file.

2022-02-22 10:41:40

by Viresh Kumar

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

On 22-02-22, 10:03, Lukasz Luba wrote:
>
>
> On 2/22/22 09:45, Viresh Kumar wrote:
> > On 22-02-22, 08:06, Lukasz Luba wrote:
> > > I'm not sure if that would be flexible enough to meet the requirement:
> > > power for each OPP might be different in one board vs. other board.
> >
> > Don't DT files overload values from board files all the time ? Why wouldn't the
> > same apply for OPP table as well ?
>
> In that SoC and family of the boards, there are no such examples.

Here is one I think.

arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dts

> It used to be popular in arm32 boards, but I'm not sure nowadays.

I think it is still common, not with OPPs though.

> > > AFAIK the OPP definition is more SoC specific.
> >
> > This isn't about OPP definition as well, but just that if DT allows you to
> > override or not. I think it will.
> >
>
> Redefining the whole OPP table, when the freq, voltage, interconnect,
> and other old entries don't change isn't too messy?

I think you misunderstood what I said. The common part of the OPP table should
stay in the central .dtsi file. The dts files though, should just add the power
specific values to the existing OPP table.

> As I said, I would prefer something lightweight, not redefining all
> stuff from OPP in every board file.

--
viresh

2022-02-22 10:49:44

by Viresh Kumar

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

On 22-02-22, 08:06, Lukasz Luba wrote:
> I'm not sure if that would be flexible enough to meet the requirement:
> power for each OPP might be different in one board vs. other board.

Don't DT files overload values from board files all the time ? Why wouldn't the
same apply for OPP table as well ?

> AFAIK the OPP definition is more SoC specific.

This isn't about OPP definition as well, but just that if DT allows you to
override or not. I think it will.

--
viresh

2022-02-22 11:59:25

by Lukasz Luba

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings



On 2/22/22 10:12, Viresh Kumar wrote:
> On 22-02-22, 10:03, Lukasz Luba wrote:
>>
>>
>> On 2/22/22 09:45, Viresh Kumar wrote:
>>> On 22-02-22, 08:06, Lukasz Luba wrote:
>>>> I'm not sure if that would be flexible enough to meet the requirement:
>>>> power for each OPP might be different in one board vs. other board.
>>>
>>> Don't DT files overload values from board files all the time ? Why wouldn't the
>>> same apply for OPP table as well ?
>>
>> In that SoC and family of the boards, there are no such examples.
>
> Here is one I think.
>
> arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dts
>
>> It used to be popular in arm32 boards, but I'm not sure nowadays.
>
> I think it is still common, not with OPPs though.
>
>>>> AFAIK the OPP definition is more SoC specific.
>>>
>>> This isn't about OPP definition as well, but just that if DT allows you to
>>> override or not. I think it will.
>>>
>>
>> Redefining the whole OPP table, when the freq, voltage, interconnect,
>> and other old entries don't change isn't too messy?
>
> I think you misunderstood what I said. The common part of the OPP table should
> stay in the central .dtsi file. The dts files though, should just add the power
> specific values to the existing OPP table.
>

OK, I misunderstood that. If that is possible than it would
be great. I'm assuming you are taking about OPP v2. I can relax the
requirement that I need to provide this DT-EM for arm32, since they
have a legacy OPP v1.

So we might have an entry similar that interconnect for the
bandwidth, but for us it would be 'opp-power-uw'?

Let me have a look about some examples how that could be just
added/extended in the opp table but from board file.
If you have some handy link, I would be grateful.

2022-02-22 12:01:13

by Lukasz Luba

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings



On 2/22/22 11:15, Viresh Kumar wrote:
> On 22-02-22, 11:03, Lukasz Luba wrote:
>> OK, I misunderstood that. If that is possible than it would
>> be great. I'm assuming you are taking about OPP v2.
>
> Yes.
>
>> I can relax the
>> requirement that I need to provide this DT-EM for arm32, since they
>> have a legacy OPP v1.
>
> OPP V2 or V1 doesn't have anything to do with arm32/64. Anyone can implement the
> newer V2 version (basically whoever wants something more than a simple freq/volt
> pair). So arm32 SoC's that want to use the DT-EM thing, should migrate to
> opp-v2, we can't support that with opp-v1.
>
>> So we might have an entry similar that interconnect for the
>> bandwidth, but for us it would be 'opp-power-uw'?
>
> I will rather say similar to opp-microvolt or opp-microamp, so it better be
> opp-microwatt.
>
>> Let me have a look about some examples how that could be just
>> added/extended in the opp table but from board file.
>> If you have some handy link, I would be grateful.
>
> The file I provided earlier:
>
> arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dts
>
> This is updating opp-microvolt property of a single OPP node in the whole table.
> Pretty much like this only.
>

Thanks, I'll use that example for the v2

2022-02-22 12:04:20

by Viresh Kumar

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

On 22-02-22, 11:03, Lukasz Luba wrote:
> OK, I misunderstood that. If that is possible than it would
> be great. I'm assuming you are taking about OPP v2.

Yes.

> I can relax the
> requirement that I need to provide this DT-EM for arm32, since they
> have a legacy OPP v1.

OPP V2 or V1 doesn't have anything to do with arm32/64. Anyone can implement the
newer V2 version (basically whoever wants something more than a simple freq/volt
pair). So arm32 SoC's that want to use the DT-EM thing, should migrate to
opp-v2, we can't support that with opp-v1.

> So we might have an entry similar that interconnect for the
> bandwidth, but for us it would be 'opp-power-uw'?

I will rather say similar to opp-microvolt or opp-microamp, so it better be
opp-microwatt.

> Let me have a look about some examples how that could be just
> added/extended in the opp table but from board file.
> If you have some handy link, I would be grateful.

The file I provided earlier:

arch/arm64/boot/dts/freescale/imx8mq-librem5-r3.dts

This is updating opp-microvolt property of a single OPP node in the whole table.
Pretty much like this only.

--
viresh

2022-02-22 15:02:45

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

On Mon, 21 Feb 2022 22:51:30 +0000, Lukasz Luba wrote:
> Add DT bindings for the Energy Model information.
>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> .../bindings/power/energy-model.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/energy-model.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/power/energy-model.yaml:34:5: [warning] wrong indentation: expected 2 but found 4 (indentation)
./Documentation/devicetree/bindings/power/energy-model.yaml:35:8: [warning] wrong indentation: expected 6 but found 7 (indentation)
./Documentation/devicetree/bindings/power/energy-model.yaml:36:16: [warning] wrong indentation: expected 9 but found 15 (indentation)
./Documentation/devicetree/bindings/power/energy-model.yaml:35:39: [error] syntax error: expected ',' or '}', but got '{' (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/power/energy-model.example.dts'
Traceback (most recent call last):
File "/usr/local/bin/dt-extract-example", line 46, in <module>
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
node = self.composer.get_single_node()
File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.parser.ParserError: while parsing a flow mapping
in "<unicode string>", line 34, column 5
did not find expected ',' or '}'
in "<unicode string>", line 35, column 39
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/power/energy-model.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/power/energy-model.yaml: while parsing a flow mapping
in "<unicode string>", line 34, column 5
did not find expected ',' or '}'
in "<unicode string>", line 35, column 39
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/energy-model.yaml: ignoring, error parsing file
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1595776

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-02-22 15:52:02

by Lukasz Luba

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] dt-bindings: power: add Energy Model bindings

Hi Rob,


On 2/22/22 14:22, Rob Herring wrote:
> On Mon, 21 Feb 2022 22:51:30 +0000, Lukasz Luba wrote:
>> Add DT bindings for the Energy Model information.
>>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> .../bindings/power/energy-model.yaml | 51 +++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/power/energy-model.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/power/energy-model.yaml:34:5: [warning] wrong indentation: expected 2 but found 4 (indentation)
> ./Documentation/devicetree/bindings/power/energy-model.yaml:35:8: [warning] wrong indentation: expected 6 but found 7 (indentation)
> ./Documentation/devicetree/bindings/power/energy-model.yaml:36:16: [warning] wrong indentation: expected 9 but found 15 (indentation)
> ./Documentation/devicetree/bindings/power/energy-model.yaml:35:39: [error] syntax error: expected ',' or '}', but got '{' (syntax)
>
> dtschema/dtc warnings/errors:
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/power/energy-model.example.dts'
> Traceback (most recent call last):
> File "/usr/local/bin/dt-extract-example", line 46, in <module>
> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
> return constructor.get_single_data()
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
> node = self.composer.get_single_node()
> File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
> File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
> ruamel.yaml.parser.ParserError: while parsing a flow mapping
> in "<unicode string>", line 34, column 5
> did not find expected ',' or '}'
> in "<unicode string>", line 35, column 39
> make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/power/energy-model.example.dts] Error 1
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/power/energy-model.yaml: while parsing a flow mapping
> in "<unicode string>", line 34, column 5
> did not find expected ',' or '}'
> in "<unicode string>", line 35, column 39
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/energy-model.yaml: ignoring, error parsing file
> make: *** [Makefile:1398: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/1595776
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

This new dt bindings idea is abandoned. We would go for
a new entry in the OPP node: "opp-microwatt".
I've sent a v2.

BTW, I've tried to run this check, but for some reason my python failed
when I tried to upgrade the dtschema:

'Could not find a version that satisfies the requirement
jsonschema>=4.1.2 (from dtschema)'

It used to work. I'll probaby have to invest more time and figure
out why my pip3 fails.

Regards,
Lukasz