[PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Dragan Simic]

Add optional support for voltage ranges to the GPU OPPs defined in the SoC
dtsi for RK356x.  These voltage ranges are useful for RK356x-based boards that
are designed to use the same power supply for the GPU and NPU portions of the
SoC, which is described further in the following documents from Rockchip:

  - Rockchip RK3566 Hardware Design Guide, version 1.1.0, page 37
  - Rockchip RK3568 Hardware Design Guide, version 1.2, page 78

The values for the exact GPU OPP voltages and the lower limits for the GPU
OPP voltage ranges differ from the values found in the vendor kernel source
(cf. downstream commit f8b9431ee38e ("arm64: dts: rockchip: rk3568: support
adjust opp-table by otp")). [1][2]  However, our values have served us well
so far, so let's keep them for now, until we actually start supporting the
CPU and GPU binning, together with the related voltage adjustments.

No functional changes are introduced, which was validated by decompiling and
comparing all affected dtb files before and after these changes.

[1] https://github.com/rockchip-linux/kernel/commit/f8b9431ee38ed561650be7092ab93f564598daa9
[2] https://raw.githubusercontent.com/rockchip-linux/kernel/f8b9431ee38ed561650be7092ab93f564598daa9/arch/arm64/boot/dts/rockchip/rk3568.dtsi

Suggested-by: Diederik de Haas <didi.debian@cknow.org>
Helped-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 38 ++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index d8543b5557ee..febda473dc38 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -1,5 +1,11 @@
 // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 /*
+ * The defined GPU OPPs optionally support voltage ranges, which are useful
+ * for RK356x-based boards that are designed to use the same power supply for
+ * the GPU and NPU portions of the SoC.  To use GPU OPPs with voltage ranges
+ * on such boards, define the RK356X_GPU_NPU_SHARED_REGULATOR macro in the
+ * descendant board dts(i) file, before including this file.
+ *
  * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
  */
 
@@ -193,6 +199,7 @@ scmi_clk: protocol@14 {
 	gpu_opp_table: opp-table-1 {
 		compatible = "operating-points-v2";
 
+#ifndef RK356X_GPU_NPU_SHARED_REGULATOR
 		opp-200000000 {
 			opp-hz = /bits/ 64 <200000000>;
 			opp-microvolt = <825000>;
@@ -222,6 +229,37 @@ opp-800000000 {
 			opp-hz = /bits/ 64 <800000000>;
 			opp-microvolt = <1000000>;
 		};
+#else
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <825000 825000 1000000>;
+		};
+
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <825000 825000 1000000>;
+		};
+
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <825000 825000 1000000>;
+		};
+
+		opp-600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <825000 825000 1000000>;
+		};
+
+		opp-700000000 {
+			opp-hz = /bits/ 64 <700000000>;
+			opp-microvolt = <900000 900000 1000000>;
+		};
+
+		opp-800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <1000000 1000000 1000000>;
+		};
+#endif /* RK356X_GPU_NPU_SHARED_REGULATOR */
 	};
 
 	hdmi_sound: hdmi-sound {

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Heiko Stübner]

Hi Dragan,

Am Samstag, 29. Juni 2024, 07:11:24 CEST schrieb Dragan Simic:

> +#ifndef RK356X_GPU_NPU_SHARED_REGULATOR

is there some reason for this duplicating of opps?

The regulator framework should pick the lowest supported voltage
anyway, so it seems you're just extending them upwards a bit.

So I really don't so why we'd need to sets here.

Also the voltage-range thing makes sense for non-gpu-npu-sharing
boards, when the supplying regulator does not fully support the
direct single-value voltage.

(rk3399-puma was such a case if I remember correctly)

So I really see no reason for this duplication.


Heiko

>  		opp-200000000 {
>  			opp-hz = /bits/ 64 <200000000>;
>  			opp-microvolt = <825000>;
> @@ -222,6 +229,37 @@ opp-800000000 {
>  			opp-hz = /bits/ 64 <800000000>;
>  			opp-microvolt = <1000000>;
>  		};
> +#else
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> +
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> +
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> +
> +		opp-700000000 {
> +			opp-hz = /bits/ 64 <700000000>;
> +			opp-microvolt = <900000 900000 1000000>;
> +		};
> +
> +		opp-800000000 {
> +			opp-hz = /bits/ 64 <800000000>;
> +			opp-microvolt = <1000000 1000000 1000000>;
> +		};
> +#endif /* RK356X_GPU_NPU_SHARED_REGULATOR */
>  	};
>  
>  	hdmi_sound: hdmi-sound {
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Dragan Simic]

Hello Heiko,

On 2024-06-29 17:10, Heiko Stübner wrote:
> Am Samstag, 29. Juni 2024, 07:11:24 CEST schrieb Dragan Simic:
> 
>> +#ifndef RK356X_GPU_NPU_SHARED_REGULATOR
> 
> is there some reason for this duplicating of opps?
> 
> The regulator framework should pick the lowest supported voltage
> anyway, so it seems you're just extending them upwards a bit.
> 
> So I really don't so why we'd need to sets here.

The reason is improved strictness.  Having the exact GPU OPP voltages
required for the boards whose GPU regulators can provide those exact
voltages makes it possible to detect misconfigurations much easier,
just like it was the case with the board dts misconfiguration that
resulted in the recent DCDC_REG2 patch. [1]

If we had GPU OPP voltage ranges in place instead, the aforementioned
issue would probably remain undetected for some time.  It wouldn't be
the end of the world, :) of course, but the resulting increased power
consumption isn't one of the desired outcomes.

[1] 
https://lore.kernel.org/linux-rockchip/e70742ea2df432bf57b3f7de542d81ca22b0da2f.1716225483.git.dsimic@manjaro.org/

> Also the voltage-range thing makes sense for non-gpu-npu-sharing
> boards, when the supplying regulator does not fully support the
> direct single-value voltage.
> 
> (rk3399-puma was such a case if I remember correctly)
> 
> So I really see no reason for this duplication.

Perhaps we could rename the RK356X_GPU_NPU_SHARED_REGULATOR macro
accordingly in the v2, to RK356X_GPU_OPP_VOLTAGE_RANGES, for example,
with some additional explanations in the patch description and the
RK356x SoC dtsi file itself.

>>  		opp-200000000 {
>>  			opp-hz = /bits/ 64 <200000000>;
>>  			opp-microvolt = <825000>;
>> @@ -222,6 +229,37 @@ opp-800000000 {
>>  			opp-hz = /bits/ 64 <800000000>;
>>  			opp-microvolt = <1000000>;
>>  		};
>> +#else
>> +		opp-200000000 {
>> +			opp-hz = /bits/ 64 <200000000>;
>> +			opp-microvolt = <825000 825000 1000000>;
>> +		};
>> +
>> +		opp-300000000 {
>> +			opp-hz = /bits/ 64 <300000000>;
>> +			opp-microvolt = <825000 825000 1000000>;
>> +		};
>> +
>> +		opp-400000000 {
>> +			opp-hz = /bits/ 64 <400000000>;
>> +			opp-microvolt = <825000 825000 1000000>;
>> +		};
>> +
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <825000 825000 1000000>;
>> +		};
>> +
>> +		opp-700000000 {
>> +			opp-hz = /bits/ 64 <700000000>;
>> +			opp-microvolt = <900000 900000 1000000>;
>> +		};
>> +
>> +		opp-800000000 {
>> +			opp-hz = /bits/ 64 <800000000>;
>> +			opp-microvolt = <1000000 1000000 1000000>;
>> +		};
>> +#endif /* RK356X_GPU_NPU_SHARED_REGULATOR */
>>  	};
>> 
>>  	hdmi_sound: hdmi-sound {
>> 
> 
> 
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Dragan Simic]

On 2024-06-29 17:25, Dragan Simic wrote:
> On 2024-06-29 17:10, Heiko Stübner wrote:
>> Am Samstag, 29. Juni 2024, 07:11:24 CEST schrieb Dragan Simic:
>> 
>>> +#ifndef RK356X_GPU_NPU_SHARED_REGULATOR
>> 
>> is there some reason for this duplicating of opps?
>> 
>> The regulator framework should pick the lowest supported voltage
>> anyway, so it seems you're just extending them upwards a bit.
>> 
>> So I really don't so why we'd need to sets here.
> 
> The reason is improved strictness.  Having the exact GPU OPP voltages
> required for the boards whose GPU regulators can provide those exact
> voltages makes it possible to detect misconfigurations much easier,
> just like it was the case with the board dts misconfiguration that
> resulted in the recent DCDC_REG2 patch. [1]
> 
> If we had GPU OPP voltage ranges in place instead, the aforementioned
> issue would probably remain undetected for some time.  It wouldn't be
> the end of the world, :) of course, but the resulting increased power
> consumption isn't one of the desired outcomes.
> 
> [1] 
> https://lore.kernel.org/linux-rockchip/e70742ea2df432bf57b3f7de542d81ca22b0da2f.1716225483.git.dsimic@manjaro.org/

On second thought, after seeing that the RK3399 CPU and GPU OPPs
already specify voltage ranges, I think it would be better to drop
the distinction between the separate strict voltages and the voltage
ranges in this patch, and to add some additional debugging messages
to drivers/opp/of.c that would allow any misconfiguration issues to
be rather easily detected.

>> Also the voltage-range thing makes sense for non-gpu-npu-sharing
>> boards, when the supplying regulator does not fully support the
>> direct single-value voltage.
>> 
>> (rk3399-puma was such a case if I remember correctly)
>> 
>> So I really see no reason for this duplication.
> 
> Perhaps we could rename the RK356X_GPU_NPU_SHARED_REGULATOR macro
> accordingly in the v2, to RK356X_GPU_OPP_VOLTAGE_RANGES, for example,
> with some additional explanations in the patch description and the
> RK356x SoC dtsi file itself.
> 
>>>  		opp-200000000 {
>>>  			opp-hz = /bits/ 64 <200000000>;
>>>  			opp-microvolt = <825000>;
>>> @@ -222,6 +229,37 @@ opp-800000000 {
>>>  			opp-hz = /bits/ 64 <800000000>;
>>>  			opp-microvolt = <1000000>;
>>>  		};
>>> +#else
>>> +		opp-200000000 {
>>> +			opp-hz = /bits/ 64 <200000000>;
>>> +			opp-microvolt = <825000 825000 1000000>;
>>> +		};
>>> +
>>> +		opp-300000000 {
>>> +			opp-hz = /bits/ 64 <300000000>;
>>> +			opp-microvolt = <825000 825000 1000000>;
>>> +		};
>>> +
>>> +		opp-400000000 {
>>> +			opp-hz = /bits/ 64 <400000000>;
>>> +			opp-microvolt = <825000 825000 1000000>;
>>> +		};
>>> +
>>> +		opp-600000000 {
>>> +			opp-hz = /bits/ 64 <600000000>;
>>> +			opp-microvolt = <825000 825000 1000000>;
>>> +		};
>>> +
>>> +		opp-700000000 {
>>> +			opp-hz = /bits/ 64 <700000000>;
>>> +			opp-microvolt = <900000 900000 1000000>;
>>> +		};
>>> +
>>> +		opp-800000000 {
>>> +			opp-hz = /bits/ 64 <800000000>;
>>> +			opp-microvolt = <1000000 1000000 1000000>;
>>> +		};
>>> +#endif /* RK356X_GPU_NPU_SHARED_REGULATOR */
>>>  	};
>>> 
>>>  	hdmi_sound: hdmi-sound {
>>> 
>> 
>> 
>> 
>> 
>> 
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Heiko Stübner]

Am Samstag, 29. Juni 2024, 17:39:34 CEST schrieb Dragan Simic:
> On 2024-06-29 17:25, Dragan Simic wrote:
> > On 2024-06-29 17:10, Heiko Stübner wrote:
> >> Am Samstag, 29. Juni 2024, 07:11:24 CEST schrieb Dragan Simic:
> >> 
> >>> +#ifndef RK356X_GPU_NPU_SHARED_REGULATOR
> >> 
> >> is there some reason for this duplicating of opps?
> >> 
> >> The regulator framework should pick the lowest supported voltage
> >> anyway, so it seems you're just extending them upwards a bit.
> >> 
> >> So I really don't so why we'd need to sets here.
> > 
> > The reason is improved strictness.  Having the exact GPU OPP voltages
> > required for the boards whose GPU regulators can provide those exact
> > voltages makes it possible to detect misconfigurations much easier,
> > just like it was the case with the board dts misconfiguration that
> > resulted in the recent DCDC_REG2 patch. [1]
> > 
> > If we had GPU OPP voltage ranges in place instead, the aforementioned
> > issue would probably remain undetected for some time.  It wouldn't be
> > the end of the world, :) of course, but the resulting increased power
> > consumption isn't one of the desired outcomes.
> > 
> > [1] 
> > https://lore.kernel.org/linux-rockchip/e70742ea2df432bf57b3f7de542d81ca22b0da2f.1716225483.git.dsimic@manjaro.org/
> 
> On second thought, after seeing that the RK3399 CPU and GPU OPPs
> already specify voltage ranges, I think it would be better to drop
> the distinction between the separate strict voltages and the voltage
> ranges in this patch,

yes, that was what I was trying to say :-)

Also it makes the OPPs less cluttered. Also dt is firmware, I do expect
people to be reasonably knowledgeable if they mess around with their
boards OPPs ;-) .




_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH] arm64: dts: rockchip: Add optional GPU OPP voltage ranges to RK356x SoC dtsi

[Dragan Simic]

On 2024-06-29 18:18, Heiko Stübner wrote:
> Am Samstag, 29. Juni 2024, 17:39:34 CEST schrieb Dragan Simic:
>> On 2024-06-29 17:25, Dragan Simic wrote:
>> > On 2024-06-29 17:10, Heiko Stübner wrote:
>> >> Am Samstag, 29. Juni 2024, 07:11:24 CEST schrieb Dragan Simic:
>> >>
>> >>> +#ifndef RK356X_GPU_NPU_SHARED_REGULATOR
>> >>
>> >> is there some reason for this duplicating of opps?
>> >>
>> >> The regulator framework should pick the lowest supported voltage
>> >> anyway, so it seems you're just extending them upwards a bit.
>> >>
>> >> So I really don't so why we'd need to sets here.
>> >
>> > The reason is improved strictness.  Having the exact GPU OPP voltages
>> > required for the boards whose GPU regulators can provide those exact
>> > voltages makes it possible to detect misconfigurations much easier,
>> > just like it was the case with the board dts misconfiguration that
>> > resulted in the recent DCDC_REG2 patch. [1]
>> >
>> > If we had GPU OPP voltage ranges in place instead, the aforementioned
>> > issue would probably remain undetected for some time.  It wouldn't be
>> > the end of the world, :) of course, but the resulting increased power
>> > consumption isn't one of the desired outcomes.
>> >
>> > [1]
>> > https://lore.kernel.org/linux-rockchip/e70742ea2df432bf57b3f7de542d81ca22b0da2f.1716225483.git.dsimic@manjaro.org/
>> 
>> On second thought, after seeing that the RK3399 CPU and GPU OPPs
>> already specify voltage ranges, I think it would be better to drop
>> the distinction between the separate strict voltages and the voltage
>> ranges in this patch,
> 
> yes, that was what I was trying to say :-)
> 
> Also it makes the OPPs less cluttered. Also dt is firmware, I do expect
> people to be reasonably knowledgeable if they mess around with their
> boards OPPs ;-) .

Yes, but we still need new regulator/OPP debugging facilities
that should to be used while writing DTs for new boards and while
verifying already existing board DTs. :)

I'll prepare and send the v2 of this patch, and I'll also start
working on the new patch for those debugging facilities.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip