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

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

Commit Message

Neha Malcom Francis Dec. 14, 2023, 7:56 a.m. UTC
  Add 2G, 1.5G, 1G, 750M, 500M and 250M as the supported frequencies for
A72. This enables support for Dynamic Frequency Scaling (DFS).

Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
---
Boot logs:
https://gist.github.com/nehamalcom/e3c3d0446f0467e7fd28706f7ffaeea8

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.

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

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

Comments

Nishanth Menon Dec. 14, 2023, 12:51 p.m. UTC | #1
On 13:26-20231214, Neha Malcom Francis wrote:
> Add 2G, 1.5G, 1G, 750M, 500M and 250M as the supported frequencies for
> A72. This enables support for Dynamic Frequency Scaling (DFS).
> 

Just curious, since I picked up the PMIC support... can we do dvfs? if
not, please indicate that in the commit message.

> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> ---
> Boot logs:
> https://gist.github.com/nehamalcom/e3c3d0446f0467e7fd28706f7ffaeea8
> 
> 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.
> 
> [1] https://www.ti.com/lit/gpn/tda4vm
This is critical info in the commit message and in documentation of
source.

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..

Are you absolutely sure this has no detection logic that can be
implemented? Almost all TI K3 SoCs seem to have a standard scheme to
detect the speed grades till date. /me wonders what the heck happened
here..

> 
>  arch/arm64/boot/dts/ti/k3-j721e.dtsi | 35 ++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
> index a200810df54a..fe92879f5812 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,9 +65,41 @@ 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";
Don't you need opp-shared ?
> +
> +		opp6-2000000000 {
> +			opp-hz = /bits/ 64 <2000000000>;
> +		};
> +
> +		opp5-1500000000 {
> +			opp-hz = /bits/ 64 <1500000000>;
> +		};
> +
> +		opp4-1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +		};
> +
> +		opp3-750000000 {
> +			opp-hz = /bits/ 64 <750000000>;
> +		};
> +
> +		opp2-500000000 {
> +			opp-hz = /bits/ 64 <500000000>;
> +		};
> +
> +		opp1-250000000 {
> +			opp-hz = /bits/ 64 <250000000>;
Could you add clock-latency-ns ?
> +		};
> +
> +	};
> +
>  	L2_0: l2-cache0 {
>  		compatible = "cache";
>  		cache-level = <2>;
> -- 
> 2.34.1
>
  
Neha Malcom Francis Dec. 15, 2023, 4:35 a.m. UTC | #2
Hi Nishanth

On 14/12/23 18:21, Nishanth Menon wrote:
> On 13:26-20231214, Neha Malcom Francis wrote:
>> Add 2G, 1.5G, 1G, 750M, 500M and 250M as the supported frequencies for
>> A72. This enables support for Dynamic Frequency Scaling (DFS).
>>
> 
> Just curious, since I picked up the PMIC support... can we do dvfs? if
> not, please indicate that in the commit message.
> 

DVFS is not supported on J7 devices, I'll mention that in v2.

>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>> Boot logs:
>> https://gist.github.com/nehamalcom/e3c3d0446f0467e7fd28706f7ffaeea8
>>
>> 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.
>>
>> [1] https://www.ti.com/lit/gpn/tda4vm
> This is critical info in the commit message and in documentation of
> source.
> 

Will put that in the commit message.

> 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..
> 

Hm... could you elaborate on that more? I don't understand the reasoning of 
including this at a board level for a SoC problem.

> Are you absolutely sure this has no detection logic that can be
> implemented? Almost all TI K3 SoCs seem to have a standard scheme to
> detect the speed grades till date. /me wonders what the heck happened
> here..
> 

Going through the reference manual and data sheet I didn't find anything that 
could differentiate between speed grades, this was confirmed for J7200, I'll do 
the needful to confirm this for J721E as well before v2.

>>
>>   arch/arm64/boot/dts/ti/k3-j721e.dtsi | 35 ++++++++++++++++++++++++++++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
>> index a200810df54a..fe92879f5812 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,9 +65,41 @@ 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";
> Don't you need opp-shared ?

opp-shared would imply that the CPUs switch DFS states together... is that 
something we want?

>> +
>> +		opp6-2000000000 {
>> +			opp-hz = /bits/ 64 <2000000000>;
>> +		};
>> +
>> +		opp5-1500000000 {
>> +			opp-hz = /bits/ 64 <1500000000>;
>> +		};
>> +
>> +		opp4-1000000000 {
>> +			opp-hz = /bits/ 64 <1000000000>;
>> +		};
>> +
>> +		opp3-750000000 {
>> +			opp-hz = /bits/ 64 <750000000>;
>> +		};
>> +
>> +		opp2-500000000 {
>> +			opp-hz = /bits/ 64 <500000000>;
>> +		};
>> +
>> +		opp1-250000000 {
>> +			opp-hz = /bits/ 64 <250000000>;
> Could you add clock-latency-ns ?

Will add in v2.

>> +		};
>> +
>> +	};
>> +
>>   	L2_0: l2-cache0 {
>>   		compatible = "cache";
>>   		cache-level = <2>;
>> -- 
>> 2.34.1
>>
> 
> 

Thanks for reviewing!
  

Patch

diff --git a/arch/arm64/boot/dts/ti/k3-j721e.dtsi b/arch/arm64/boot/dts/ti/k3-j721e.dtsi
index a200810df54a..fe92879f5812 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,9 +65,41 @@  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";
+
+		opp6-2000000000 {
+			opp-hz = /bits/ 64 <2000000000>;
+		};
+
+		opp5-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+		};
+
+		opp4-1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+		};
+
+		opp3-750000000 {
+			opp-hz = /bits/ 64 <750000000>;
+		};
+
+		opp2-500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+		};
+
+		opp1-250000000 {
+			opp-hz = /bits/ 64 <250000000>;
+		};
+
+	};
+
 	L2_0: l2-cache0 {
 		compatible = "cache";
 		cache-level = <2>;