diff mbox series

[2/3] arm64: dts: ti: k3-j7200: Add the supported frequencies for A72

Message ID 20230809173905.1844132-3-a-nandan@ti.com (mailing list archive)
State New, archived
Headers show
Series Add support for thermal mitigation for K3 J7200 SoC | expand

Commit Message

Apurva Nandan Aug. 9, 2023, 5:39 p.m. UTC
From: Keerthy <j-keerthy@ti.com>

Add 750M, 1G, 1.5G & 2G as the supported frequencies for A72.
This enables support for Dynamic Frequency Scaling(DFS)

Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Apurva Nandan <a-nandan@ti.com>
---
 arch/arm64/boot/dts/ti/k3-j7200.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Nishanth Menon Aug. 9, 2023, 7:09 p.m. UTC | #1
On 23:09-20230809, Apurva Nandan wrote:
> From: Keerthy <j-keerthy@ti.com>
> 
> Add 750M, 1G, 1.5G & 2G as the supported frequencies for A72.
> This enables support for Dynamic Frequency Scaling(DFS)
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-j7200.dtsi | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> index ef73e6d7e858..7222c453096f 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
> @@ -48,6 +48,10 @@ 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>;
> +			#cooling-cells = <2>; /* min followed by max */
>  		};
>  
>  		cpu1: cpu@1 {
> @@ -62,6 +66,30 @@ 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>;
> +			#cooling-cells = <2>; /* min followed by max */
> +		};
> +	};
> +
> +	cpu0_opp_table: opp-table {
> +		compatible = "operating-points-v2";
> +
> +		opp4-2000000000 {
> +			opp-hz = /bits/ 64 <2000000000>;
> +		};
> +
> +		opp3-1500000000 {
> +			opp-hz = /bits/ 64 <1500000000>;
> +		};
> +
> +		opp2-1000000000 {
> +			opp-hz = /bits/ 64 <1000000000>;
> +		};
> +
> +		opp1-750000000 {
> +			opp-hz = /bits/ 64 <750000000>;
>  		};
>  	};
>  
> -- 
> 2.34.1
> 

Are you sure this is correct to enable all OPPs without efuse bit checks?

https://www.ti.com/lit/ds/symlink/dra821u-q1.pdf
7.5 Operating Performance Points
DRA821xC operates only upto 750MHz
DRA821xE at 1GHz
DRA821xL upto 1.5GHz and
DRA821xT upto 2GHz
Kumar, Udit Aug. 10, 2023, 11:53 a.m. UTC | #2
On 8/10/2023 12:39 AM, Nishanth Menon wrote:
> On 23:09-20230809, Apurva Nandan wrote:
>> From: Keerthy <j-keerthy@ti.com>
>>
>> Add 750M, 1G, 1.5G & 2G as the supported frequencies for A72.
>> This enables support for Dynamic Frequency Scaling(DFS)
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> Signed-off-by: Apurva Nandan <a-nandan@ti.com>
>> ---
>>   arch/arm64/boot/dts/ti/k3-j7200.dtsi | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
>> index ef73e6d7e858..7222c453096f 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
>> @@ -48,6 +48,10 @@ 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>;
>> +			#cooling-cells = <2>; /* min followed by max */
>>   		};
>>   
>>   		cpu1: cpu@1 {
>> @@ -62,6 +66,30 @@ 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>;
>> +			#cooling-cells = <2>; /* min followed by max */
>> +		};
>> +	};
>> +
>> +	cpu0_opp_table: opp-table {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp4-2000000000 {
>> +			opp-hz = /bits/ 64 <2000000000>;
>> +		};
>> +
>> +		opp3-1500000000 {
>> +			opp-hz = /bits/ 64 <1500000000>;
>> +		};
>> +
>> +		opp2-1000000000 {
>> +			opp-hz = /bits/ 64 <1000000000>;
>> +		};
>> +
>> +		opp1-750000000 {
>> +			opp-hz = /bits/ 64 <750000000>;
>>   		};
>>   	};
>>   
>> -- 
>> 2.34.1
>>
> Are you sure this is correct to enable all OPPs without efuse bit checks?
>
> https://www.ti.com/lit/ds/symlink/dra821u-q1.pdf
> 7.5 Operating Performance Points
> DRA821xC operates only upto 750MHz
> DRA821xE at 1GHz
> DRA821xL upto 1.5GHz and
> DRA821xT upto 2GHz

Looks, top SKUs is considered here .

After detecting which SKU we are running (I hope TRM should have this 
information- through efuse or some other register)

I think, we can follow two approaches.

1) have OPP table for each SKU and select based SKUs type or

2) Do run time fixup by u-boot based upon SKU type
Nishanth Menon Aug. 10, 2023, 12:53 p.m. UTC | #3
On 17:23-20230810, Kumar, Udit wrote:
[..]
> > > +		opp1-750000000 {
> > > +			opp-hz = /bits/ 64 <750000000>;
> > >   		};
> > >   	};
> > > -- 
> > > 2.34.1
> > > 
> > Are you sure this is correct to enable all OPPs without efuse bit checks?
> > 
> > https://www.ti.com/lit/ds/symlink/dra821u-q1.pdf
> > 7.5 Operating Performance Points
> > DRA821xC operates only upto 750MHz
> > DRA821xE at 1GHz
> > DRA821xL upto 1.5GHz and
> > DRA821xT upto 2GHz
> 
> Looks, top SKUs is considered here .
> 
> After detecting which SKU we are running (I hope TRM should have this
> information- through efuse or some other register)
> 
> I think, we can follow two approaches.

Both of these are wrong approaches.

> 
> 1) have OPP table for each SKU and select based SKUs type or

This proliferates cpu dtsi to make it hard to manage

> 
> 2) Do run time fixup by u-boot based upon SKU type

This wont work:

a) in u-boot's falcon boot mode and puts unrelated responsibility to
bootloader (u-boot is not the only bootloader in the party here).
b) Further, the reason for doing the opp detection in the kernel is
due to the severity of consequence of attempting to run a lower rated
chip at higher frequency - PoH (Power on Hours) or physical damage can
result.
c) Finally, in a virtualized environment: TISCI will get DM (Device
Manager) to arbitrate between the each of the VM's request, but if
the VM's are'nt self sufficient, we will have DM making wrong choices
resulting in (b) condition again.

This is the reason why drivers/cpufreq/ti-cpufreq.c exists and all SoCs
that have OPPs from TI is handled in the kernel itself.
Kumar, Udit Aug. 10, 2023, 2:19 p.m. UTC | #4
On 8/10/2023 6:23 PM, Nishanth Menon wrote:
> On 17:23-20230810, Kumar, Udit wrote:
> [..]
>>>> +		opp1-750000000 {
>>>> +			opp-hz = /bits/ 64 <750000000>;
>>>>    		};
>>>>    	};
>>>> -- 
>>>> 2.34.1
>>>>
>>> [..]
>>> This wont work:
>>>
>>> a) in u-boot's falcon boot mode and puts unrelated responsibility to
>>> bootloader (u-boot is not the only bootloader in the party here).
>>> b) Further, the reason for doing the opp detection in the kernel is
>>> due to the severity of consequence of attempting to run a lower rated
>>> chip at higher frequency - PoH (Power on Hours) or physical damage can
>>> result.
>>> c) Finally, in a virtualized environment: TISCI will get DM (Device
>>> Manager) to arbitrate between the each of the VM's request, but if
>>> the VM's are'nt self sufficient, we will have DM making wrong choices
>>> resulting in (b) condition again.
>>>
>>> This is the reason why drivers/cpufreq/ti-cpufreq.c exists and all SoCs
>>> that have OPPs from TI is handled in the kernel itself.

Thanks to pointing to this driver.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/ti/k3-j7200.dtsi b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
index ef73e6d7e858..7222c453096f 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200.dtsi
@@ -48,6 +48,10 @@  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>;
+			#cooling-cells = <2>; /* min followed by max */
 		};
 
 		cpu1: cpu@1 {
@@ -62,6 +66,30 @@  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>;
+			#cooling-cells = <2>; /* min followed by max */
+		};
+	};
+
+	cpu0_opp_table: opp-table {
+		compatible = "operating-points-v2";
+
+		opp4-2000000000 {
+			opp-hz = /bits/ 64 <2000000000>;
+		};
+
+		opp3-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+		};
+
+		opp2-1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+		};
+
+		opp1-750000000 {
+			opp-hz = /bits/ 64 <750000000>;
 		};
 	};