2024-01-04 11:19:48

by Neha Malcom Francis

[permalink] [raw]
Subject: [PATCH v2] arm64: dts: ti: k3-j721e: Add support for DFS in J721E A72

Add 2G, 1G, 500M and 250M as the supported frequencies for A72. This
enables support for Dynamic Frequency Scaling (DFS). Note that Dynamic
Voltage and Frequency Scaling (DVFS) is not supported on J7 devices.

J721E SoC has three different speed grade devices (see [1], 7.5
Operating Performance Points) which as of today are indiscernible in
software, users of a different speed grade device must manually change
the DTS to ensure their maximum speed frequency is supported.

To obtain clock-latency-ns, the maximum time was found to switch from/to
any frequency for a CPU and this value was rounded off and set.

[1] https://www.ti.com/lit/gpn/tda4vm

Signed-off-by: Neha Malcom Francis <[email protected]>
---
Test and boot logs:
https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2

Changes since v1:
https://lore.kernel.org/all/[email protected]/
- removed OPPs 1.5G and 750M as they introduced boot regression in
J721E-SK
- Nishanth
- indicated DVFS not supported in commit message
- moved critical data sheet info from below tear line to commit
message
- added opp-shared property
- added clock-latency-ns property

arch/arm64/boot/dts/ti/k3-j721e.dtsi | 31 ++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
index a200810df54a..5de6c70bd989 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
@@ -48,6 +48,9 @@ cpu0: cpu@0 {
d-cache-line-size = <64>;
d-cache-sets = <256>;
next-level-cache = <&L2_0>;
+ clocks = <&k3_clks 202 2>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
};

cpu1: cpu@1 {
@@ -62,6 +65,34 @@ cpu1: cpu@1 {
d-cache-line-size = <64>;
d-cache-sets = <256>;
next-level-cache = <&L2_0>;
+ clocks = <&k3_clks 203 0>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
+ };
+ };
+
+ cpu0_opp_table: opp-table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp6-2000000000 {
+ opp-hz = /bits/ 64 <2000000000>;
+ clock-latency-ns = <300000>;
+ };
+
+ opp4-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ clock-latency-ns = <300000>;
+ };
+
+ opp2-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ clock-latency-ns = <300000>;
+ };
+
+ opp1-250000000 {
+ opp-hz = /bits/ 64 <250000000>;
+ clock-latency-ns = <300000>;
};
};

--
2.34.1



2024-01-04 15:17:46

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: ti: k3-j721e: Add support for DFS in J721E A72

On 16:49-20240104, Neha Malcom Francis wrote:
> Add 2G, 1G, 500M and 250M as the supported frequencies for A72. This
> enables support for Dynamic Frequency Scaling (DFS). Note that Dynamic
> Voltage and Frequency Scaling (DVFS) is not supported on J7 devices.
>
> J721E SoC has three different speed grade devices (see [1], 7.5
> Operating Performance Points) which as of today are indiscernible in
> software, users of a different speed grade device must manually change
> the DTS to ensure their maximum speed frequency is supported.
>
> To obtain clock-latency-ns, the maximum time was found to switch from/to
> any frequency for a CPU and this value was rounded off and set.
>
> [1] https://www.ti.com/lit/gpn/tda4vm
>
> Signed-off-by: Neha Malcom Francis <[email protected]>
> ---
> Test and boot logs:
> https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2
>
> Changes since v1:
> https://lore.kernel.org/all/[email protected]/
> - removed OPPs 1.5G and 750M as they introduced boot regression in
> J721E-SK

I do not think this is the right approach precisely for the above
reason.

See my comment in V1: https://lore.kernel.org/all/20231214125130.zqtq6ioj4c533wha@elbow/

"
I am also concerned if the table should be separated out as a dtsi and
included at board.dts level to prevent downstream users going crazy..
"

I suspect there is no magic opp configuration that will work with all
downstream and board variations. instead of creating a trimmed down
non-datasheet tuples of OPP configuration, use the data sheet provided
OPP configurations into each dtsi and the boards can apply the dtsi
based on the type of sample they have.

I don't see any other scheme (overlays, maybe?).. but this approach is
broken and your note above proves why this approach is broken.

> - Nishanth
> - indicated DVFS not supported in commit message
> - moved critical data sheet info from below tear line to commit
> message
> - added opp-shared property
> - added clock-latency-ns property
>
> arch/arm64/boot/dts/ti/k3-j721e.dtsi | 31 ++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> index a200810df54a..5de6c70bd989 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> @@ -48,6 +48,9 @@ cpu0: cpu@0 {
> d-cache-line-size = <64>;
> d-cache-sets = <256>;
> next-level-cache = <&L2_0>;
> + clocks = <&k3_clks 202 2>;
> + clock-names = "cpu";
> + operating-points-v2 = <&cpu0_opp_table>;
> };
>
> cpu1: cpu@1 {
> @@ -62,6 +65,34 @@ cpu1: cpu@1 {
> d-cache-line-size = <64>;
> d-cache-sets = <256>;
> next-level-cache = <&L2_0>;
> + clocks = <&k3_clks 203 0>;
> + clock-names = "cpu";
> + operating-points-v2 = <&cpu0_opp_table>;
> + };
> + };
> +
> + cpu0_opp_table: opp-table {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp6-2000000000 {
> + opp-hz = /bits/ 64 <2000000000>;
> + clock-latency-ns = <300000>;
> + };
> +
> + opp4-1000000000 {
> + opp-hz = /bits/ 64 <1000000000>;
> + clock-latency-ns = <300000>;
> + };
> +
> + opp2-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + clock-latency-ns = <300000>;
> + };
> +
> + opp1-250000000 {
> + opp-hz = /bits/ 64 <250000000>;
> + clock-latency-ns = <300000>;
> };
> };
>
> --
> 2.34.1
>

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2024-01-05 03:57:08

by Neha Malcom Francis

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: dts: ti: k3-j721e: Add support for DFS in J721E A72

Hi Nishanth,

On 04/01/24 20:46, Nishanth Menon wrote:
> On 16:49-20240104, Neha Malcom Francis wrote:
>> Add 2G, 1G, 500M and 250M as the supported frequencies for A72. This
>> enables support for Dynamic Frequency Scaling (DFS). Note that Dynamic
>> Voltage and Frequency Scaling (DVFS) is not supported on J7 devices.
>>
>> J721E SoC has three different speed grade devices (see [1], 7.5
>> Operating Performance Points) which as of today are indiscernible in
>> software, users of a different speed grade device must manually change
>> the DTS to ensure their maximum speed frequency is supported.
>>
>> To obtain clock-latency-ns, the maximum time was found to switch from/to
>> any frequency for a CPU and this value was rounded off and set.
>>
>> [1] https://www.ti.com/lit/gpn/tda4vm
>>
>> Signed-off-by: Neha Malcom Francis <[email protected]>
>> ---
>> Test and boot logs:
>> https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2
>>
>> Changes since v1:
>> https://lore.kernel.org/all/[email protected]/
>> - removed OPPs 1.5G and 750M as they introduced boot regression in
>> J721E-SK
>
> I do not think this is the right approach precisely for the above
> reason.
>
> See my comment in V1: https://lore.kernel.org/all/20231214125130.zqtq6ioj4c533wha@elbow/
>
> "
> I am also concerned if the table should be separated out as a dtsi and
> included at board.dts level to prevent downstream users going crazy..
> "
>
> I suspect there is no magic opp configuration that will work with all
> downstream and board variations. instead of creating a trimmed down
> non-datasheet tuples of OPP configuration, use the data sheet provided
> OPP configurations into each dtsi and the boards can apply the dtsi
> based on the type of sample they have.
>
> I don't see any other scheme (overlays, maybe?).. but this approach is
> broken and your note above proves why this approach is broken.
>

Understood... I believed OPPs were an SoC specific thing until this failure came
about and it makes sense to take it at the board level now. Will factor this in
for v3.

>> - Nishanth
>> - indicated DVFS not supported in commit message
>> - moved critical data sheet info from below tear line to commit
>> message
>> - added opp-shared property
>> - added clock-latency-ns property
>>
>> arch/arm64/boot/dts/ti/k3-j721e.dtsi | 31 ++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
>> index a200810df54a..5de6c70bd989 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j721e.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
>> @@ -48,6 +48,9 @@ cpu0: cpu@0 {
>> d-cache-line-size = <64>;
>> d-cache-sets = <256>;
>> next-level-cache = <&L2_0>;
>> + clocks = <&k3_clks 202 2>;
>> + clock-names = "cpu";
>> + operating-points-v2 = <&cpu0_opp_table>;
>> };
>>
>> cpu1: cpu@1 {
>> @@ -62,6 +65,34 @@ cpu1: cpu@1 {
>> d-cache-line-size = <64>;
>> d-cache-sets = <256>;
>> next-level-cache = <&L2_0>;
>> + clocks = <&k3_clks 203 0>;
>> + clock-names = "cpu";
>> + operating-points-v2 = <&cpu0_opp_table>;
>> + };
>> + };
>> +
>> + cpu0_opp_table: opp-table {
>> + compatible = "operating-points-v2";
>> + opp-shared;
>> +
>> + opp6-2000000000 {
>> + opp-hz = /bits/ 64 <2000000000>;
>> + clock-latency-ns = <300000>;
>> + };
>> +
>> + opp4-1000000000 {
>> + opp-hz = /bits/ 64 <1000000000>;
>> + clock-latency-ns = <300000>;
>> + };
>> +
>> + opp2-500000000 {
>> + opp-hz = /bits/ 64 <500000000>;
>> + clock-latency-ns = <300000>;
>> + };
>> +
>> + opp1-250000000 {
>> + opp-hz = /bits/ 64 <250000000>;
>> + clock-latency-ns = <300000>;
>> };
>> };
>>
>> --
>> 2.34.1
>>
>

--
Thanking You
Neha Malcom Francis