diff mbox series

arm64: dts: rockchip: Add OPP voltage ranges to RK3399 OP1 SoC dtsi

Message ID 806d5e2a07ae0c81d9907bbe8bec4e3e1138b392.1730838347.git.dsimic@manjaro.org (mailing list archive)
State New
Headers show
Series arm64: dts: rockchip: Add OPP voltage ranges to RK3399 OP1 SoC dtsi | expand

Commit Message

Dragan Simic Nov. 5, 2024, 8:28 p.m. UTC
Add support for voltage ranges to the CPU, GPU and DMC OPPs defined in the
SoC dtsi for Rockchip OP1, as a variant of the Rockchip RK3399.  This may be
useful if there are any OP1-based boards whose associated voltage regulators
are unable to deliver the exact voltages; otherwise, it causes no functional
changes to the resulting OPP voltages at runtime.

These changes cannot cause stability issues or any kind of damage, because
it's perfectly safe to use the highest voltage from an OPP group for each OPP
in the same group.  The only possible negative effect of using higher voltages
is wasted energy in form of some additionally generated heat.

Reported-by: Quentin Schulz <quentin.schulz@cherry.de>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi | 52 ++++++++++----------
 1 file changed, 26 insertions(+), 26 deletions(-)

Comments

Alexey Charkov Nov. 6, 2024, 7:55 a.m. UTC | #1
Hi Dragan,

On Wed, Nov 6, 2024 at 11:50 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Add support for voltage ranges to the CPU, GPU and DMC OPPs defined in the
> SoC dtsi for Rockchip OP1, as a variant of the Rockchip RK3399.  This may be
> useful if there are any OP1-based boards whose associated voltage regulators
> are unable to deliver the exact voltages; otherwise, it causes no functional
> changes to the resulting OPP voltages at runtime.
>
> These changes cannot cause stability issues or any kind of damage, because
> it's perfectly safe to use the highest voltage from an OPP group for each OPP
> in the same group.  The only possible negative effect of using higher voltages
> is wasted energy in form of some additionally generated heat.
>
> Reported-by: Quentin Schulz <quentin.schulz@cherry.de>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi | 52 ++++++++++----------
>  1 file changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
> index b24bff511513..aa79219471d3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
> @@ -12,125 +12,125 @@ cluster0_opp: opp-table-0 {
>
>                 opp00 {
>                         opp-hz = /bits/ 64 <408000000>;
> -                       opp-microvolt = <800000>;
> +                       opp-microvolt = <800000 800000 1150000>;
>                         clock-latency-ns = <40000>;
>                 };
>                 opp01 {
>                         opp-hz = /bits/ 64 <600000000>;
> -                       opp-microvolt = <825000>;
> +                       opp-microvolt = <825000 825000 1150000>;
>                 };
>                 opp02 {
>                         opp-hz = /bits/ 64 <816000000>;
> -                       opp-microvolt = <850000>;
> +                       opp-microvolt = <850000 850000 1150000>;
>                 };
>                 opp03 {
>                         opp-hz = /bits/ 64 <1008000000>;
> -                       opp-microvolt = <900000>;
> +                       opp-microvolt = <900000 900000 1150000>;
>                 };
>                 opp04 {
>                         opp-hz = /bits/ 64 <1200000000>;
> -                       opp-microvolt = <975000>;
> +                       opp-microvolt = <975000 975000 1150000>;
>                 };
>                 opp05 {
>                         opp-hz = /bits/ 64 <1416000000>;
> -                       opp-microvolt = <1100000>;
> +                       opp-microvolt = <1100000 1100000 1150000>;
>                 };
>                 opp06 {
>                         opp-hz = /bits/ 64 <1512000000>;
> -                       opp-microvolt = <1150000>;
> +                       opp-microvolt = <1150000 1150000 1150000>;
>                 };
>         };
>
>         cluster1_opp: opp-table-1 {
>                 compatible = "operating-points-v2";
>                 opp-shared;
>
>                 opp00 {
>                         opp-hz = /bits/ 64 <408000000>;
>                         opp-microvolt = <800000>;
> -                       clock-latency-ns = <40000>;
> +                       clock-latency-ns = <40000 40000 1250000>;

Did you mean to update opp-microvolt instead?

Speaking of which, this single opp having clock-latency-ns while none
of the others do (except the first one in cluster0) looks weird.

Best regards,
Alexey
Dragan Simic Nov. 6, 2024, 8:07 a.m. UTC | #2
Hello Alexey,

On 2024-11-06 08:55, Alexey Charkov wrote:
> On Wed, Nov 6, 2024 at 11:50 AM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> 
>> Add support for voltage ranges to the CPU, GPU and DMC OPPs defined in 
>> the
>> SoC dtsi for Rockchip OP1, as a variant of the Rockchip RK3399.  This 
>> may be
>> useful if there are any OP1-based boards whose associated voltage 
>> regulators
>> are unable to deliver the exact voltages; otherwise, it causes no 
>> functional
>> changes to the resulting OPP voltages at runtime.
>> 
>> These changes cannot cause stability issues or any kind of damage, 
>> because
>> it's perfectly safe to use the highest voltage from an OPP group for 
>> each OPP
>> in the same group.  The only possible negative effect of using higher 
>> voltages
>> is wasted energy in form of some additionally generated heat.
>> 
>> Reported-by: Quentin Schulz <quentin.schulz@cherry.de>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi | 52 
>> ++++++++++----------
>>  1 file changed, 26 insertions(+), 26 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi 
>> b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
>> index b24bff511513..aa79219471d3 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
>> @@ -12,125 +12,125 @@ cluster0_opp: opp-table-0 {
>> 
>>                 opp00 {
>>                         opp-hz = /bits/ 64 <408000000>;
>> -                       opp-microvolt = <800000>;
>> +                       opp-microvolt = <800000 800000 1150000>;
>>                         clock-latency-ns = <40000>;
>>                 };
>>                 opp01 {
>>                         opp-hz = /bits/ 64 <600000000>;
>> -                       opp-microvolt = <825000>;
>> +                       opp-microvolt = <825000 825000 1150000>;
>>                 };
>>                 opp02 {
>>                         opp-hz = /bits/ 64 <816000000>;
>> -                       opp-microvolt = <850000>;
>> +                       opp-microvolt = <850000 850000 1150000>;
>>                 };
>>                 opp03 {
>>                         opp-hz = /bits/ 64 <1008000000>;
>> -                       opp-microvolt = <900000>;
>> +                       opp-microvolt = <900000 900000 1150000>;
>>                 };
>>                 opp04 {
>>                         opp-hz = /bits/ 64 <1200000000>;
>> -                       opp-microvolt = <975000>;
>> +                       opp-microvolt = <975000 975000 1150000>;
>>                 };
>>                 opp05 {
>>                         opp-hz = /bits/ 64 <1416000000>;
>> -                       opp-microvolt = <1100000>;
>> +                       opp-microvolt = <1100000 1100000 1150000>;
>>                 };
>>                 opp06 {
>>                         opp-hz = /bits/ 64 <1512000000>;
>> -                       opp-microvolt = <1150000>;
>> +                       opp-microvolt = <1150000 1150000 1150000>;
>>                 };
>>         };
>> 
>>         cluster1_opp: opp-table-1 {
>>                 compatible = "operating-points-v2";
>>                 opp-shared;
>> 
>>                 opp00 {
>>                         opp-hz = /bits/ 64 <408000000>;
>>                         opp-microvolt = <800000>;
>> -                       clock-latency-ns = <40000>;
>> +                       clock-latency-ns = <40000 40000 1250000>;
> 
> Did you mean to update opp-microvolt instead?

Oops, look at that! :/  Thanks for the correction, I'll send the v2
with the fix.  I created some macro in vim that produced the voltage
range additions, and as you can see, I applied that macro more than
a few times, and sometimes a bit carelessly. :/

> Speaking of which, this single opp having clock-latency-ns while none
> of the others do (except the first one in cluster0) looks weird.

Yes, I also noticed that.  Though, the way the OPP driver works, only
one of the OPPs need to have clock-latency-ns defined for everything
to work as expected.

I did "sprinkle" the missing clock-latency-ns values in one of my
other patches, [*] using the values from the vendor kernel source,
but in this case no such values are available from the vendor, so
we cannot go with "inventing" them somehow, and we need to keep
relying on the above-described OPP driver's behavior.

[*] 
https://lore.kernel.org/linux-rockchip/2e1e100284b1edb470d6e7fde021a0f1779336c8.1728752527.git.dsimic@manjaro.org/
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
index b24bff511513..aa79219471d3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-op1.dtsi
@@ -12,125 +12,125 @@  cluster0_opp: opp-table-0 {
 
 		opp00 {
 			opp-hz = /bits/ 64 <408000000>;
-			opp-microvolt = <800000>;
+			opp-microvolt = <800000 800000 1150000>;
 			clock-latency-ns = <40000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <825000>;
+			opp-microvolt = <825000 825000 1150000>;
 		};
 		opp02 {
 			opp-hz = /bits/ 64 <816000000>;
-			opp-microvolt = <850000>;
+			opp-microvolt = <850000 850000 1150000>;
 		};
 		opp03 {
 			opp-hz = /bits/ 64 <1008000000>;
-			opp-microvolt = <900000>;
+			opp-microvolt = <900000 900000 1150000>;
 		};
 		opp04 {
 			opp-hz = /bits/ 64 <1200000000>;
-			opp-microvolt = <975000>;
+			opp-microvolt = <975000 975000 1150000>;
 		};
 		opp05 {
 			opp-hz = /bits/ 64 <1416000000>;
-			opp-microvolt = <1100000>;
+			opp-microvolt = <1100000 1100000 1150000>;
 		};
 		opp06 {
 			opp-hz = /bits/ 64 <1512000000>;
-			opp-microvolt = <1150000>;
+			opp-microvolt = <1150000 1150000 1150000>;
 		};
 	};
 
 	cluster1_opp: opp-table-1 {
 		compatible = "operating-points-v2";
 		opp-shared;
 
 		opp00 {
 			opp-hz = /bits/ 64 <408000000>;
 			opp-microvolt = <800000>;
-			clock-latency-ns = <40000>;
+			clock-latency-ns = <40000 40000 1250000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <800000>;
+			opp-microvolt = <800000 800000 1250000>;
 		};
 		opp02 {
 			opp-hz = /bits/ 64 <816000000>;
-			opp-microvolt = <825000>;
+			opp-microvolt = <825000 825000 1250000>;
 		};
 		opp03 {
 			opp-hz = /bits/ 64 <1008000000>;
-			opp-microvolt = <850000>;
+			opp-microvolt = <850000 850000 1250000>;
 		};
 		opp04 {
 			opp-hz = /bits/ 64 <1200000000>;
-			opp-microvolt = <900000>;
+			opp-microvolt = <900000 900000 1250000>;
 		};
 		opp05 {
 			opp-hz = /bits/ 64 <1416000000>;
-			opp-microvolt = <975000>;
+			opp-microvolt = <975000 975000 1250000>;
 		};
 		opp06 {
 			opp-hz = /bits/ 64 <1608000000>;
-			opp-microvolt = <1050000>;
+			opp-microvolt = <1050000 1050000 1250000>;
 		};
 		opp07 {
 			opp-hz = /bits/ 64 <1800000000>;
-			opp-microvolt = <1150000>;
+			opp-microvolt = <1150000 1150000 1250000>;
 		};
 		opp08 {
 			opp-hz = /bits/ 64 <2016000000>;
-			opp-microvolt = <1250000>;
+			opp-microvolt = <1250000 1250000 1250000>;
 		};
 	};
 
 	gpu_opp_table: opp-table-2 {
 		compatible = "operating-points-v2";
 
 		opp00 {
 			opp-hz = /bits/ 64 <200000000>;
-			opp-microvolt = <800000>;
+			opp-microvolt = <800000 800000 1075000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <297000000>;
-			opp-microvolt = <800000>;
+			opp-microvolt = <800000 800000 1075000>;
 		};
 		opp02 {
 			opp-hz = /bits/ 64 <400000000>;
-			opp-microvolt = <825000>;
+			opp-microvolt = <825000 825000 1075000>;
 		};
 		opp03 {
 			opp-hz = /bits/ 64 <500000000>;
-			opp-microvolt = <850000>;
+			opp-microvolt = <850000 850000 1075000>;
 		};
 		opp04 {
 			opp-hz = /bits/ 64 <600000000>;
-			opp-microvolt = <925000>;
+			opp-microvolt = <925000 925000 1075000>;
 		};
 		opp05 {
 			opp-hz = /bits/ 64 <800000000>;
-			opp-microvolt = <1075000>;
+			opp-microvolt = <1075000 1075000 1075000>;
 		};
 	};
 
 	dmc_opp_table: opp-table-3 {
 		compatible = "operating-points-v2";
 
 		opp00 {
 			opp-hz = /bits/ 64 <400000000>;
-			opp-microvolt = <900000>;
+			opp-microvolt = <900000 900000 925000>;
 		};
 		opp01 {
 			opp-hz = /bits/ 64 <666000000>;
-			opp-microvolt = <900000>;
+			opp-microvolt = <900000 900000 925000>;
 		};
 		opp02 {
 			opp-hz = /bits/ 64 <800000000>;
-			opp-microvolt = <900000>;
+			opp-microvolt = <900000 900000 925000>;
 		};
 		opp03 {
 			opp-hz = /bits/ 64 <928000000>;
-			opp-microvolt = <925000>;
+			opp-microvolt = <925000 925000 925000>;
 		};
 	};
 };