diff mbox series

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

Message ID 20240104111922.832040-1-n-francis@ti.com
State New
Headers
Series [v2] arm64: dts: ti: k3-j721e: Add support for DFS in J721E A72 |

Commit Message

Neha Malcom Francis Jan. 4, 2024, 11:19 a.m. UTC 
  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 <n-francis@ti.com>
---
Test and boot logs:
https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2

Changes since v1:
https://lore.kernel.org/all/20231214075637.176586-1-n-francis@ti.com/
- 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(+)

Comments

Nishanth Menon Jan. 4, 2024, 3:16 p.m. UTC | #1 
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 <n-francis@ti.com>
> ---
> Test and boot logs:
> https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2
> 
> Changes since v1:
> https://lore.kernel.org/all/20231214075637.176586-1-n-francis@ti.com/
> - 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
>
Neha Malcom Francis Jan. 5, 2024, 3:56 a.m. UTC | #2 
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 <n-francis@ti.com>
>> ---
>> Test and boot logs:
>> https://gist.github.com/nehamalcom/33608837ab5ad3332ff11a7fa7a602e2
>>
>> Changes since v1:
>> https://lore.kernel.org/all/20231214075637.176586-1-n-francis@ti.com/
>> - 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
>>
>
diff mbox series

Patch

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>;
 		};
 	};