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

Notes:
    Changes in v2:
      - Fixed one wrong application of a vim macro [1] that somehow slipped
        through the cracks, as pointed out by Alexey [2]
    
    Link to v1: https://lore.kernel.org/linux-rockchip/806d5e2a07ae0c81d9907bbe8bec4e3e1138b392.1730838347.git.dsimic@manjaro.org/T/#u
    
    [1] https://lore.kernel.org/linux-rockchip/0c237c49fae03bdad99be04053285ea2@manjaro.org/
    [2] https://lore.kernel.org/linux-rockchip/CABjd4Yyt6WiY5E5DbyjnboFvsTpp33dydkGMF7AwMB9m7bfX6A@mail.gmail.com/

 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..c4f4f1ff6117 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>;
+			opp-microvolt = <800000 800000 1250000>;
 			clock-latency-ns = <40000>;
 		};
 		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>;
 		};
 	};
 };

On Wed, 6 Nov 2024 09:33:08 +0100, Dragan Simic 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.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: rockchip: Add OPP voltage ranges to RK3399 OP1 SoC dtsi
      commit: 37e4ad9c795a725e7308089e9d59d094115138dc

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Hi Dragan,

On 11/6/24 9:33 AM, Dragan Simic 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>

Well, I merely highlighted that the voltage was different on OP1 
compared to RK3399 for the 600MHz OPP :)

So... If there's ONE SoC I'm pretty sure is working as expected it's the 
OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork 
(though yes, I believe all Gru CB are EoL since August 2023). In the 6.1 
kernel fork, there's also no range: 
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi

So not sure we need to handle theoretical cases here. Will let 
maintainers decide on that one. FWIW, there are two other OP1 devices, 
the RockPi4A+ and RockPi4B+ which do not change the OPP either.

Cheers,
Quentin

Hello Quentin,

On 2024-11-06 10:32, Quentin Schulz wrote:
> On 11/6/24 9:33 AM, Dragan Simic 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>
> 
> Well, I merely highlighted that the voltage was different on OP1
> compared to RK3399 for the 600MHz OPP :)

I just wanted to somehow acknowledge that you made me work on
this patch, so to speak. :)

> So... If there's ONE SoC I'm pretty sure is working as expected it's
> the OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork
> (though yes, I believe all Gru CB are EoL since August 2023). In the
> 6.1 kernel fork, there's also no range:
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> 
> So not sure we need to handle theoretical cases here. Will let
> maintainers decide on that one. FWIW, there are two other OP1 devices,
> the RockPi4A+ and RockPi4B+ which do not change the OPP either.

The thing is that adding the voltage ranges won't change anything
for the devices whose voltage regulators are capable of providing
the exact OPP voltages.  They'll still be set to the exact values
as before these changes, so there's no worry about breaking anything,
while we make the things a bit more consistent this way.

I also plan to implement and upstream the DT for TinkerBoard 2S,
which is based on the OP1, and its voltage regulator setup may
actually need these voltage ranges.

Am Mittwoch, 6. November 2024, 10:32:06 CET schrieb Quentin Schulz:
> Hi Dragan,
> 
> On 11/6/24 9:33 AM, Dragan Simic 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>
> 
> Well, I merely highlighted that the voltage was different on OP1 
> compared to RK3399 for the 600MHz OPP :)
> 
> So... If there's ONE SoC I'm pretty sure is working as expected it's the 
> OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork 
> (though yes, I believe all Gru CB are EoL since August 2023). In the 6.1 
> kernel fork, there's also no range: 
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi

yeah, this somehow goes quite a bit into the "stuff that doesn't need to
change" area. On the one hand it does make "some" sense to unify things
if we're using ranges everywhere else.

On the other hand, as Quentin noted below, all existing OP1 devices seem
to run just fine, and there won't be any more entering the kernel.

So what do we realisitically gain here, except hiding existing git-history
under another layer?

> So not sure we need to handle theoretical cases here. Will let 
> maintainers decide on that one. FWIW, there are two other OP1 devices, 
> the RockPi4A+ and RockPi4B+ which do not change the OPP either.


Heiko

Hello Heiko,

On 2024-11-06 10:41, Heiko Stübner wrote:
> Am Mittwoch, 6. November 2024, 10:32:06 CET schrieb Quentin Schulz:
>> On 11/6/24 9:33 AM, Dragan Simic 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>
>> 
>> Well, I merely highlighted that the voltage was different on OP1
>> compared to RK3399 for the 600MHz OPP :)
>> 
>> So... If there's ONE SoC I'm pretty sure is working as expected it's 
>> the
>> OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork
>> (though yes, I believe all Gru CB are EoL since August 2023). In the 
>> 6.1
>> kernel fork, there's also no range:
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> 
> yeah, this somehow goes quite a bit into the "stuff that doesn't need 
> to
> change" area. On the one hand it does make "some" sense to unify things
> if we're using ranges everywhere else.

I agree that it might be unneeded, but there's no possible harm, and
unifying the things may be seen as beneficial.

> On the other hand, as Quentin noted below, all existing OP1 devices 
> seem
> to run just fine, and there won't be any more entering the kernel.

Hmm, why can't we add more OP1-based devices?  As I mentioned in my
earlier response to Quentin, my plan is to implement the board dts
for OP1-based TinkerBoard 2S, so I'd like to know is there something
that might prevent that board dts from becoming merged?

> So what do we realisitically gain here, except hiding existing 
> git-history
> under another layer?

Sorry, I'm not sure what would become hidden by this patch?

>> So not sure we need to handle theoretical cases here. Will let
>> maintainers decide on that one. FWIW, there are two other OP1 devices,
>> the RockPi4A+ and RockPi4B+ which do not change the OPP either.

Am Mittwoch, 6. November 2024, 11:45:06 CET schrieb Dragan Simic:
> Hello Heiko,
> 
> On 2024-11-06 10:41, Heiko Stübner wrote:
> > Am Mittwoch, 6. November 2024, 10:32:06 CET schrieb Quentin Schulz:
> >> On 11/6/24 9:33 AM, Dragan Simic 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>
> >> 
> >> Well, I merely highlighted that the voltage was different on OP1
> >> compared to RK3399 for the 600MHz OPP :)
> >> 
> >> So... If there's ONE SoC I'm pretty sure is working as expected it's 
> >> the
> >> OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork
> >> (though yes, I believe all Gru CB are EoL since August 2023). In the 
> >> 6.1
> >> kernel fork, there's also no range:
> >> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> > 
> > yeah, this somehow goes quite a bit into the "stuff that doesn't need 
> > to
> > change" area. On the one hand it does make "some" sense to unify things
> > if we're using ranges everywhere else.
> 
> I agree that it might be unneeded, but there's no possible harm, and
> unifying the things may be seen as beneficial.
> 
> > On the other hand, as Quentin noted below, all existing OP1 devices 
> > seem
> > to run just fine, and there won't be any more entering the kernel.
> 
> Hmm, why can't we add more OP1-based devices?  As I mentioned in my
> earlier response to Quentin, my plan is to implement the board dts
> for OP1-based TinkerBoard 2S, so I'd like to know is there something
> that might prevent that board dts from becoming merged?
> 
> > So what do we realisitically gain here, except hiding existing 
> > git-history
> > under another layer?
> 
> Sorry, I'm not sure what would become hidden by this patch?

When you change a part of the file, a git blame points to you,
hiding the previous blame, so it makes traversing history a tiny
bit harder.

If you're actually doing the Tinkerboard and thus adding new things this
changes the whole judgement a bit too.
Like I was on the mindset-road of rk3399 is mostly done in terms of new
boards, so what's in the kernel now will at max get some new peripherals
but is in general already mostly working.

If we're still adding new rk3399 boards, it does make more sense to go
back and make the underlying parts nicer :-)


Heiko

On 2024-11-06 12:24, Heiko Stübner wrote:
> Am Mittwoch, 6. November 2024, 11:45:06 CET schrieb Dragan Simic:
>> On 2024-11-06 10:41, Heiko Stübner wrote:
>> > Am Mittwoch, 6. November 2024, 10:32:06 CET schrieb Quentin Schulz:
>> >> On 11/6/24 9:33 AM, Dragan Simic 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>
>> >>
>> >> Well, I merely highlighted that the voltage was different on OP1
>> >> compared to RK3399 for the 600MHz OPP :)
>> >>
>> >> So... If there's ONE SoC I'm pretty sure is working as expected it's
>> >> the
>> >> OP1 fitted on the Gru Chromebooks with the ChromiumOS kernel fork
>> >> (though yes, I believe all Gru CB are EoL since August 2023). In the
>> >> 6.1
>> >> kernel fork, there's also no range:
>> >> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-6.1/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
>> >
>> > yeah, this somehow goes quite a bit into the "stuff that doesn't need
>> > to
>> > change" area. On the one hand it does make "some" sense to unify things
>> > if we're using ranges everywhere else.
>> 
>> I agree that it might be unneeded, but there's no possible harm, and
>> unifying the things may be seen as beneficial.
>> 
>> > On the other hand, as Quentin noted below, all existing OP1 devices
>> > seem
>> > to run just fine, and there won't be any more entering the kernel.
>> 
>> Hmm, why can't we add more OP1-based devices?  As I mentioned in my
>> earlier response to Quentin, my plan is to implement the board dts
>> for OP1-based TinkerBoard 2S, so I'd like to know is there something
>> that might prevent that board dts from becoming merged?
>> 
>> > So what do we realisitically gain here, except hiding existing
>> > git-history
>> > under another layer?
>> 
>> Sorry, I'm not sure what would become hidden by this patch?
> 
> When you change a part of the file, a git blame points to you,
> hiding the previous blame, so it makes traversing history a tiny
> bit harder.

Ah, I see, thanks for the clarification.  I'm willing to take the
resulting blame, though. :)

> If you're actually doing the Tinkerboard and thus adding new things 
> this
> changes the whole judgement a bit too.

Yes, I need an OP1-based board for my upcoming Rockchip SoC binning
endeavor, which for me basically boils down to getting a TinkerBoard
2S.  Of course, I need to have my future TinkerBoard 2S working and
running mainline kernel, and what's a better way for doing that than
having its board dts upstreamed, for everyone's benefit. :)

> Like I was on the mindset-road of rk3399 is mostly done in terms of new
> boards, so what's in the kernel now will at max get some new 
> peripherals
> but is in general already mostly working.
> 
> If we're still adding new rk3399 boards, it does make more sense to go
> back and make the underlying parts nicer :-)

Yes, I see the RK3399 as an actively maintained part of the kernel. :)
With all that in mind, I hope the associated cleanups will be seen as
justified.