linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
@ 2021-10-16 15:45 Trevor Woerner
  2021-10-16 15:45 ` [PATCH 2/2] arm64: dts: rock64: add gpu regulator Trevor Woerner
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Trevor Woerner @ 2021-10-16 15:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Johan Jonker, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Robin Murphy,
	Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

Add an operating-points table and cooling entry to the GPU on the
RK3328 SoC to improve its performance. According to its datasheet[1]
the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
is 500MHz.

On my rock64 device, under x11, glmark2-es2 performance increased from
around 60 to just over 100. Same device running glmark2-es2 under
wayland/weston improved from just over 100 to just over 200.

[1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf

Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 8c821acb21ff..5e1dcf71e414 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -532,7 +532,8 @@ map0 {
 					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+							 <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
 					contribution = <4096>;
 				};
 			};
@@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
 		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
 		clock-names = "bus", "core";
 		resets = <&cru SRST_GPU_A>;
+		operating-points-v2 = <&gpu_opp_table>;
+		#cooling-cells = <2>;
+	};
+
+	gpu_opp_table: gpu-opp-table {
+		compatible = "operating-points-v2";
+
+		opp-200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <1100000>;
+		};
+		opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <1100000>;
+		};
+		opp-400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <1100000>;
+		};
+		opp-500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-microvolt = <1100000>;
+		};
 	};
 
 	h265e_mmu: iommu@ff330200 {
-- 
2.30.0.rc0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] arm64: dts: rock64: add gpu regulator
  2021-10-16 15:45 [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Trevor Woerner
@ 2021-10-16 15:45 ` Trevor Woerner
  2021-10-16 20:45 ` [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Johan Jonker
  2021-10-18 16:49 ` Nicolas Frattaroli
  2 siblings, 0 replies; 8+ messages in thread
From: Trevor Woerner @ 2021-10-16 15:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Katsuhiro Suzuki, Chen-Yu Tsai,
	Cameron Nemo,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

Add a regulator for the GPU in order to properly make use of the newly added
operations-table.

Signed-off-by: Trevor Woerner <twoerner@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 1b0f7e4551ea..e76673d5fdc5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -133,6 +133,10 @@ &cpu3 {
 	cpu-supply = <&vdd_arm>;
 };
 
+&gpu {
+	mali-supply = <&vdd_logic>;
+};
+
 &emmc {
 	bus-width = <8>;
 	cap-mmc-highspeed;
-- 
2.30.0.rc0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-16 15:45 [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Trevor Woerner
  2021-10-16 15:45 ` [PATCH 2/2] arm64: dts: rock64: add gpu regulator Trevor Woerner
@ 2021-10-16 20:45 ` Johan Jonker
  2021-10-17 15:29   ` Trevor Woerner
  2021-10-18 16:49 ` Nicolas Frattaroli
  2 siblings, 1 reply; 8+ messages in thread
From: Johan Jonker @ 2021-10-16 20:45 UTC (permalink / raw)
  To: Trevor Woerner, linux-kernel
  Cc: Rob Herring, Heiko Stuebner, Chen-Yu Tsai, David Wu,
	Ezequiel Garcia, Cameron Nemo, Robin Murphy, Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support



On 10/16/21 5:45 PM, Trevor Woerner wrote:
> Add an operating-points table and cooling entry to the GPU on the
> RK3328 SoC to improve its performance. According to its datasheet[1]
> the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
> is 500MHz.
> 
> On my rock64 device, under x11, glmark2-es2 performance increased from
> around 60 to just over 100. Same device running glmark2-es2 under
> wayland/weston improved from just over 100 to just over 200.
> 
> [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
> 
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> index 8c821acb21ff..5e1dcf71e414 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> @@ -532,7 +532,8 @@ map0 {
>  					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> -							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>  					contribution = <4096>;
>  				};
>  			};
> @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
>  		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
>  		clock-names = "bus", "core";
>  		resets = <&cru SRST_GPU_A>;
> +		operating-points-v2 = <&gpu_opp_table>;
> +		#cooling-cells = <2>;
> +	};
> +

> +	gpu_opp_table: gpu-opp-table {

After the conversion to YAML of the Operating Performance Points(OPP)
binding the operating-points-v2 property expects the nodename to have
the '^opp-table(-[a-z0-9]+)?$' format.

make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/opp/opp-v2.yaml

> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-500000000 {
> +			opp-hz = /bits/ 64 <500000000>;
> +			opp-microvolt = <1100000>;
> +		};
>  	};

opp-microvolt has the same value for every node vs. table below?

See also previous discussion:

https://lore.kernel.org/linux-rockchip/3c95c29b-6c07-5945-ac22-d683997e1ca0@arm.com/

Is that now fixed/checked?

Copy from manufacturer tree:

	gpu_opp_table: gpu-opp-table {
		compatible = "operating-points-v2";

		rockchip,leakage-voltage-sel = <
			1   10    0
			11  254   1
		>;
		nvmem-cells = <&logic_leakage>;
		nvmem-cell-names = "gpu_leakage";

		opp-200000000 {
			opp-hz = /bits/ 64 <200000000>;
			opp-microvolt = <950000>;
			opp-microvolt-L0 = <950000>;
			opp-microvolt-L1 = <950000>;
		};
		opp-300000000 {
			opp-hz = /bits/ 64 <300000000>;
			opp-microvolt = <975000>;
			opp-microvolt-L0 = <975000>;
			opp-microvolt-L1 = <950000>;
		};
		opp-400000000 {
			opp-hz = /bits/ 64 <400000000>;
			opp-microvolt = <1050000>;
			opp-microvolt-L0 = <1050000>;
			opp-microvolt-L1 = <1025000>;
		};
		opp-500000000 {
			opp-hz = /bits/ 64 <500000000>;
			opp-microvolt = <1150000>;
			opp-microvolt-L0 = <1150000>;
			opp-microvolt-L1 = <1100000>;
		};
	};
>  
>  	h265e_mmu: iommu@ff330200 {
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-16 20:45 ` [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Johan Jonker
@ 2021-10-17 15:29   ` Trevor Woerner
  2021-10-18 18:09     ` Robin Murphy
  0 siblings, 1 reply; 8+ messages in thread
From: Trevor Woerner @ 2021-10-17 15:29 UTC (permalink / raw)
  To: Johan Jonker
  Cc: linux-kernel, Rob Herring, Heiko Stuebner, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Robin Murphy,
	Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

On Sat 2021-10-16 @ 10:45:04 PM, Johan Jonker wrote:
> On 10/16/21 5:45 PM, Trevor Woerner wrote:
> > Add an operating-points table and cooling entry to the GPU on the
> > RK3328 SoC to improve its performance. According to its datasheet[1]
> > the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
> > is 500MHz.
> > 
> > On my rock64 device, under x11, glmark2-es2 performance increased from
> > around 60 to just over 100. Same device running glmark2-es2 under
> > wayland/weston improved from just over 100 to just over 200.
> > 
> > [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
> > 
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > index 8c821acb21ff..5e1dcf71e414 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> > @@ -532,7 +532,8 @@ map0 {
> >  					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >  							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >  							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > -							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +							 <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> >  					contribution = <4096>;
> >  				};
> >  			};
> > @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
> >  		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
> >  		clock-names = "bus", "core";
> >  		resets = <&cru SRST_GPU_A>;
> > +		operating-points-v2 = <&gpu_opp_table>;
> > +		#cooling-cells = <2>;
> > +	};
> > +
> 
> > +	gpu_opp_table: gpu-opp-table {
> 
> After the conversion to YAML of the Operating Performance Points(OPP)
> binding the operating-points-v2 property expects the nodename to have
> the '^opp-table(-[a-z0-9]+)?$' format.
> 
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/opp/opp-v2.yaml

Thanks, I wasn't aware.

> > +		compatible = "operating-points-v2";
> > +
> > +		opp-200000000 {
> > +			opp-hz = /bits/ 64 <200000000>;
> > +			opp-microvolt = <1100000>;
> > +		};
> > +		opp-300000000 {
> > +			opp-hz = /bits/ 64 <300000000>;
> > +			opp-microvolt = <1100000>;
> > +		};
> > +		opp-400000000 {
> > +			opp-hz = /bits/ 64 <400000000>;
> > +			opp-microvolt = <1100000>;
> > +		};
> > +		opp-500000000 {
> > +			opp-hz = /bits/ 64 <500000000>;
> > +			opp-microvolt = <1100000>;
> > +		};
> >  	};
> 
> opp-microvolt has the same value for every node vs. table below?

On page 1 of the schematic for the rock64
https://files.pine64.org/doc/rock64/ROCK64_Schematic_v3.0_20181105.pdf is a
table ("Power Timing") showing BUCK1 at 1.1V. I interpreted this to mean that
VDD_LOG should always be at 1.1V, regardless of frequency.

> See also previous discussion:
> 
> https://lore.kernel.org/linux-rockchip/3c95c29b-6c07-5945-ac22-d683997e1ca0@arm.com/
> 
> Is that now fixed/checked?

I wasn't aware of the previous/on-going discussion regarding a gpu opp table
for this SoC. Perhaps that explains my suspicions? I couldn't help wonder why
the frequency is always reported as 163840000 even when I have an opp table
that only has the 500MHz entry?

I'll investigate whether I can prove or disprove the scaling is actually
occurring?

Best regards,
	Trevor

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-16 15:45 [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Trevor Woerner
  2021-10-16 15:45 ` [PATCH 2/2] arm64: dts: rock64: add gpu regulator Trevor Woerner
  2021-10-16 20:45 ` [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Johan Jonker
@ 2021-10-18 16:49 ` Nicolas Frattaroli
  2021-10-19 15:52   ` Alex Bee
  2 siblings, 1 reply; 8+ messages in thread
From: Nicolas Frattaroli @ 2021-10-18 16:49 UTC (permalink / raw)
  To: linux-kernel, linux-rockchip
  Cc: Rob Herring, Heiko Stuebner, Johan Jonker, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Robin Murphy,
	Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support, Trevor Woerner

On Samstag, 16. Oktober 2021 17:45:44 CEST Trevor Woerner wrote:
> Add an operating-points table and cooling entry to the GPU on the
> RK3328 SoC to improve its performance. According to its datasheet[1]
> the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
> is 500MHz.
> 
> On my rock64 device, under x11, glmark2-es2 performance increased from
> around 60 to just over 100. Same device running glmark2-es2 under
> wayland/weston improved from just over 100 to just over 200.
> 
> [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
> 
> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 8c821acb21ff..5e1dcf71e414
> 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> @@ -532,7 +532,8 @@ map0 {
>  					cooling-device = <&cpu0 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  							 <&cpu1 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>  							 <&cpu2 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> -							 <&cpu3 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +							 <&cpu3 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&gpu 
THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>  					contribution = <4096>;
>  				};
>  			};
> @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
>  		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
>  		clock-names = "bus", "core";
>  		resets = <&cru SRST_GPU_A>;
> +		operating-points-v2 = <&gpu_opp_table>;
> +		#cooling-cells = <2>;
> +	};
> +
> +	gpu_opp_table: gpu-opp-table {
> +		compatible = "operating-points-v2";
> +
> +		opp-200000000 {
> +			opp-hz = /bits/ 64 <200000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-400000000 {
> +			opp-hz = /bits/ 64 <400000000>;
> +			opp-microvolt = <1100000>;
> +		};
> +		opp-500000000 {
> +			opp-hz = /bits/ 64 <500000000>;
> +			opp-microvolt = <1100000>;
> +		};
>  	};
> 
>  	h265e_mmu: iommu@ff330200 {

As for whether this works as described on a ROCK64 for glmark2-es2-wayland:

Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>

There is some stuff worth noting that LibreELEC does on this SoC[1]:

1. they use 1.05V for all OPPs up to and including 400 MHz
2. they run 500 MHz at 1.15V instead (though 1.10V seemed to work for both of 
us)
3. they disable 500 MHz because 1.15V was apparently too high for rkvdec.

3 is currently not very relevant because mainline Linux has no rkvdec node in 
the rk3328 dtsi, and we're not running at 1.15V.

I've decided to add their rkvdec dtsi patch[2] on top anyway, and saw no 
complaints from the rkvdec module while glmark2-es2-drm was running. However, 
it's not like I tried to actually hardware decode video while it was running 
because the userspace situation still won't let me without compiling entirely 
too much stuff from git. Though the rkvdec module was loaded and present.

[1]: https://github.com/LibreELEC/LibreELEC.tv/commit/
9a6be0d36ba7ff3c3d5df798682d47a1de594ac0
[2]: https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/
patches/linux/default/linux-1001-v4l2-rockchip.patch#L860-L935




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-17 15:29   ` Trevor Woerner
@ 2021-10-18 18:09     ` Robin Murphy
  0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2021-10-18 18:09 UTC (permalink / raw)
  To: Trevor Woerner, Johan Jonker
  Cc: linux-kernel, Rob Herring, Heiko Stuebner, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support,
	open list:ARM/Rockchip SoC support

On 2021-10-17 16:29, Trevor Woerner wrote:
> On Sat 2021-10-16 @ 10:45:04 PM, Johan Jonker wrote:
>> On 10/16/21 5:45 PM, Trevor Woerner wrote:
>>> Add an operating-points table and cooling entry to the GPU on the
>>> RK3328 SoC to improve its performance. According to its datasheet[1]
>>> the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
>>> is 500MHz.
>>>
>>> On my rock64 device, under x11, glmark2-es2 performance increased from
>>> around 60 to just over 100. Same device running glmark2-es2 under
>>> wayland/weston improved from just over 100 to just over 200.
>>>
>>> [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
>>>
>>> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
>>> ---
>>>   arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
>>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>> index 8c821acb21ff..5e1dcf71e414 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>>> @@ -532,7 +532,8 @@ map0 {
>>>   					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>>   							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>>   							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>> -							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>> +							 <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>>   					contribution = <4096>;
>>>   				};
>>>   			};
>>> @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
>>>   		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
>>>   		clock-names = "bus", "core";
>>>   		resets = <&cru SRST_GPU_A>;
>>> +		operating-points-v2 = <&gpu_opp_table>;
>>> +		#cooling-cells = <2>;
>>> +	};
>>> +
>>
>>> +	gpu_opp_table: gpu-opp-table {
>>
>> After the conversion to YAML of the Operating Performance Points(OPP)
>> binding the operating-points-v2 property expects the nodename to have
>> the '^opp-table(-[a-z0-9]+)?$' format.
>>
>> make ARCH=arm64 dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/opp/opp-v2.yaml
> 
> Thanks, I wasn't aware.
> 
>>> +		compatible = "operating-points-v2";
>>> +
>>> +		opp-200000000 {
>>> +			opp-hz = /bits/ 64 <200000000>;
>>> +			opp-microvolt = <1100000>;
>>> +		};
>>> +		opp-300000000 {
>>> +			opp-hz = /bits/ 64 <300000000>;
>>> +			opp-microvolt = <1100000>;
>>> +		};
>>> +		opp-400000000 {
>>> +			opp-hz = /bits/ 64 <400000000>;
>>> +			opp-microvolt = <1100000>;
>>> +		};
>>> +		opp-500000000 {
>>> +			opp-hz = /bits/ 64 <500000000>;
>>> +			opp-microvolt = <1100000>;
>>> +		};
>>>   	};
>>
>> opp-microvolt has the same value for every node vs. table below?
> 
> On page 1 of the schematic for the rock64
> https://files.pine64.org/doc/rock64/ROCK64_Schematic_v3.0_20181105.pdf is a
> table ("Power Timing") showing BUCK1 at 1.1V. I interpreted this to mean that
> VDD_LOG should always be at 1.1V, regardless of frequency.

No, that's just the default voltage that BUCK1 itself starts up at - 
looks like that table is an unfinished attempt to summarise the Power 
Sequence section from the RK805 datasheet.

>> See also previous discussion:
>>
>> https://lore.kernel.org/linux-rockchip/3c95c29b-6c07-5945-ac22-d683997e1ca0@arm.com/
>>
>> Is that now fixed/checked?
> 
> I wasn't aware of the previous/on-going discussion regarding a gpu opp table
> for this SoC. Perhaps that explains my suspicions? I couldn't help wonder why
> the frequency is always reported as 163840000 even when I have an opp table
> that only has the 500MHz entry?

FWIW the usual culprit for clocks not changing is inadvertently not 
having devfreq and/or the simple_ondemand governor enabled. However, I 
do seem to recall that devfreq doesn't explicitly fix up an out-of-spec 
clock to a known OPP on startup like cpufreq does - I think it only 
actually touches the clocks and regulators when transitioning between 
OPPs, so if it only has one it might possibly end up in a pathological 
state where that effectively never happens; I don't remember exactly. 
Unfortunately all my boards are out of action for various reasons at the 
moment so I can't readily check how I was running mine, but from memory 
I think I ended up with slightly tweaked voltages based on a survey of 
several other BSP kernels, and the 200-300MHz points just disabled to 
avoid undervolting the memory controller once lima voltage scaling was 
working properly.

Cheers,
Robin.

> 
> I'll investigate whether I can prove or disprove the scaling is actually
> occurring?
> 
> Best regards,
> 	Trevor
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-18 16:49 ` Nicolas Frattaroli
@ 2021-10-19 15:52   ` Alex Bee
  2021-10-19 16:19     ` Nicolas Frattaroli
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bee @ 2021-10-19 15:52 UTC (permalink / raw)
  To: Nicolas Frattaroli, linux-kernel, linux-rockchip
  Cc: Rob Herring, Heiko Stuebner, Johan Jonker, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Robin Murphy,
	Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support, Trevor Woerner

Am 18.10.21 um 18:49 schrieb Nicolas Frattaroli:
> On Samstag, 16. Oktober 2021 17:45:44 CEST Trevor Woerner wrote:
>> Add an operating-points table and cooling entry to the GPU on the
>> RK3328 SoC to improve its performance. According to its datasheet[1]
>> the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
>> is 500MHz.
>>
>> On my rock64 device, under x11, glmark2-es2 performance increased from
>> around 60 to just over 100. Same device running glmark2-es2 under
>> wayland/weston improved from just over 100 to just over 200.
>>
>> [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
>>
>> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
>> ---
>>   arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
>>   1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 8c821acb21ff..5e1dcf71e414
>> 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
>> @@ -532,7 +532,8 @@ map0 {
>>   					cooling-device = <&cpu0
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>   							 <&cpu1
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>>   							 <&cpu2
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> -							 <&cpu3
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +							 <&cpu3
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&gpu
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>>   					contribution = <4096>;
>>   				};
>>   			};
>> @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
>>   		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
>>   		clock-names = "bus", "core";
>>   		resets = <&cru SRST_GPU_A>;
>> +		operating-points-v2 = <&gpu_opp_table>;
>> +		#cooling-cells = <2>;
>> +	};
>> +
>> +	gpu_opp_table: gpu-opp-table {
>> +		compatible = "operating-points-v2";
>> +
>> +		opp-200000000 {
>> +			opp-hz = /bits/ 64 <200000000>;
>> +			opp-microvolt = <1100000>;
>> +		};
>> +		opp-300000000 {
>> +			opp-hz = /bits/ 64 <300000000>;
>> +			opp-microvolt = <1100000>;
>> +		};
>> +		opp-400000000 {
>> +			opp-hz = /bits/ 64 <400000000>;
>> +			opp-microvolt = <1100000>;
>> +		};
>> +		opp-500000000 {
>> +			opp-hz = /bits/ 64 <500000000>;
>> +			opp-microvolt = <1100000>;
>> +		};
>>   	};
>>
>>   	h265e_mmu: iommu@ff330200 {
> 
> As for whether this works as described on a ROCK64 for glmark2-es2-wayland:

The probably most "convenient" and also future-proof solution upstream 
for that is to define voltage ranges á la



opp-200000000 {

		opp-hz = /bits/ 64 <200000000>;

		opp-microvolt = <950000 950000 1150000>;



};

and so on.

And then adapt the regulator-min-microvolt of the logic regulator like

vdd_logic: DCDC_REG1 {

	regulator-name = "vdd_logic";

	regulator-min-microvolt = <1050000>;

...
};

That way all opp-points will be taken, but its ensured, that vdd_log 
never goes below 1.05 V

> 
> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> 
> There is some stuff worth noting that LibreELEC does on this SoC[1]:
> 
> 1. they use 1.05V for all OPPs up to and including 400 MHz
> 2. they run 500 MHz at 1.15V instead (though 1.10V seemed to work for both of
> us)

That might be true for your boards, but note that the required voltage 
is per "leakage level" defined in efuse bits - something we do not 
support for Rockchip upstream currently - see [1]

> 3. they disable 500 MHz because 1.15V was apparently too high for rkvdec.
> 
> 3 is currently not very relevant because mainline Linux has no rkvdec node in
> the rk3328 dtsi, and we're not running at 1.15V.
> 
> I've decided to add their rkvdec dtsi patch[2] on top anyway, and saw no
> complaints from the rkvdec module while glmark2-es2-drm was running. However,
> it's not like I tried to actually hardware decode video while it was running
> because the userspace situation still won't let me without compiling entirely
> too much stuff from git. Though the rkvdec module was loaded and present.
You will see no complaints from the module , but you will see the SoC 
crashing if both is running at the same time - see rkvdec-opp-table 
downstream [2]

[1] 
https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm64/boot/dts/rockchip/rk3328.dtsi#L750-L751

[2] 
https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm64/boot/dts/rockchip/rk3328.dtsi#L840-L867

Alex

> [1]: https://github.com/LibreELEC/LibreELEC.tv/commit/
> 9a6be0d36ba7ff3c3d5df798682d47a1de594ac0
> [2]: https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/
> patches/linux/default/linux-1001-v4l2-rockchip.patch#L860-L935
> 
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] arm64: dts: rk3328: add gpu opp table
  2021-10-19 15:52   ` Alex Bee
@ 2021-10-19 16:19     ` Nicolas Frattaroli
  0 siblings, 0 replies; 8+ messages in thread
From: Nicolas Frattaroli @ 2021-10-19 16:19 UTC (permalink / raw)
  To: linux-kernel, linux-rockchip, Alex Bee
  Cc: Rob Herring, Heiko Stuebner, Johan Jonker, Chen-Yu Tsai,
	David Wu, Ezequiel Garcia, Cameron Nemo, Robin Murphy,
	Elaine Zhang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/Rockchip SoC support, Trevor Woerner

On Dienstag, 19. Oktober 2021 17:52:05 CEST Alex Bee wrote:
> Am 18.10.21 um 18:49 schrieb Nicolas Frattaroli:
> > On Samstag, 16. Oktober 2021 17:45:44 CEST Trevor Woerner wrote:
> >> Add an operating-points table and cooling entry to the GPU on the
> >> RK3328 SoC to improve its performance. According to its datasheet[1]
> >> the maximum frequency of the Mali-450 MP2 GPU found on the RK3328 SoC
> >> is 500MHz.
> >>
> >> On my rock64 device, under x11, glmark2-es2 performance increased from
> >> around 60 to just over 100. Same device running glmark2-es2 under
> >> wayland/weston improved from just over 100 to just over 200.
> >>
> >> [1] https://rockchip.fr/RK3328%20datasheet%20V1.2.pdf
> >>
> >> Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> >> ---
> >>   arch/arm64/boot/dts/rockchip/rk3328.dtsi | 26 +++++++++++++++++++++++-
> >>   1 file changed, 25 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> >> b/arch/arm64/boot/dts/rockchip/rk3328.dtsi index 8c821acb21ff..5e1dcf71e414
> >> 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
> >> @@ -532,7 +532,8 @@ map0 {
> >>   					cooling-device = <&cpu0
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >>   							 <&cpu1
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >>   							 <&cpu2
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >> -							 <&cpu3
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> >> +							 <&cpu3
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> >> +							 <&gpu
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> >>   					contribution = <4096>;
> >>   				};
> >>   			};
> >> @@ -617,6 +618,29 @@ gpu: gpu@ff300000 {
> >>   		clocks = <&cru ACLK_GPU>, <&cru ACLK_GPU>;
> >>   		clock-names = "bus", "core";
> >>   		resets = <&cru SRST_GPU_A>;
> >> +		operating-points-v2 = <&gpu_opp_table>;
> >> +		#cooling-cells = <2>;
> >> +	};
> >> +
> >> +	gpu_opp_table: gpu-opp-table {
> >> +		compatible = "operating-points-v2";
> >> +
> >> +		opp-200000000 {
> >> +			opp-hz = /bits/ 64 <200000000>;
> >> +			opp-microvolt = <1100000>;
> >> +		};
> >> +		opp-300000000 {
> >> +			opp-hz = /bits/ 64 <300000000>;
> >> +			opp-microvolt = <1100000>;
> >> +		};
> >> +		opp-400000000 {
> >> +			opp-hz = /bits/ 64 <400000000>;
> >> +			opp-microvolt = <1100000>;
> >> +		};
> >> +		opp-500000000 {
> >> +			opp-hz = /bits/ 64 <500000000>;
> >> +			opp-microvolt = <1100000>;
> >> +		};
> >>   	};
> >>
> >>   	h265e_mmu: iommu@ff330200 {
> > 
> > As for whether this works as described on a ROCK64 for glmark2-es2-wayland:
> 
> The probably most "convenient" and also future-proof solution upstream 
> for that is to define voltage ranges á la
> 
> 
> 
> opp-200000000 {
> 
> 		opp-hz = /bits/ 64 <200000000>;
> 
> 		opp-microvolt = <950000 950000 1150000>;
> 
> 
> 
> };
> 
> and so on.
> 
> And then adapt the regulator-min-microvolt of the logic regulator like
> 
> vdd_logic: DCDC_REG1 {
> 
> 	regulator-name = "vdd_logic";
> 
> 	regulator-min-microvolt = <1050000>;
> 
> ...
> };
> 
> That way all opp-points will be taken, but its ensured, that vdd_log 
> never goes below 1.05 V
> 
> > 
> > Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > 
> > There is some stuff worth noting that LibreELEC does on this SoC[1]:
> > 
> > 1. they use 1.05V for all OPPs up to and including 400 MHz
> > 2. they run 500 MHz at 1.15V instead (though 1.10V seemed to work for both of
> > us)
> 
> That might be true for your boards, but note that the required voltage 
> is per "leakage level" defined in efuse bits - something we do not 
> support for Rockchip upstream currently - see [1]

Thank you for the insight.

If we had the efuse bits, is there an already existing mechanism to only
enable the 500 MHz OPP if the chip happens to be one of those able to do
it at 1.10V?

Something like only having opp-microvolt-L1 defined and having the opp
be ignored if we're not an L1 chip.

> 
> > 3. they disable 500 MHz because 1.15V was apparently too high for rkvdec.
> > 
> > 3 is currently not very relevant because mainline Linux has no rkvdec node in
> > the rk3328 dtsi, and we're not running at 1.15V.
> > 
> > I've decided to add their rkvdec dtsi patch[2] on top anyway, and saw no
> > complaints from the rkvdec module while glmark2-es2-drm was running. However,
> > it's not like I tried to actually hardware decode video while it was running
> > because the userspace situation still won't let me without compiling entirely
> > too much stuff from git. Though the rkvdec module was loaded and present.
> You will see no complaints from the module , but you will see the SoC 
> crashing if both is running at the same time - see rkvdec-opp-table 
> downstream [2]

I did end up finding an old arm64 build I had of a patched ffmpeg that works
for the current kernel, and

  ffmpeg -hwaccel drm -i file -f null -

was running stably with glmark2-es2-drm running at the same time, though I
also did not have an rkvdec OPP table.

Either way, thank you for clearing things up. I guess the easiest way to
get more useful clocks on mainline right now is to disable the 500 MHz
OPP and define them all in the way you've suggested.

Regards,
Nicolas Frattaroli

> 
> [1] 
> https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm64/boot/dts/rockchip/rk3328.dtsi#L750-L751
> 
> [2] 
> https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm64/boot/dts/rockchip/rk3328.dtsi#L840-L867
> 
> Alex
> 
> > [1]: https://github.com/LibreELEC/LibreELEC.tv/commit/
> > 9a6be0d36ba7ff3c3d5df798682d47a1de594ac0
> > [2]: https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/
> > patches/linux/default/linux-1001-v4l2-rockchip.patch#L860-L935
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip
> > 
> 
> 





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-10-19 16:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-16 15:45 [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Trevor Woerner
2021-10-16 15:45 ` [PATCH 2/2] arm64: dts: rock64: add gpu regulator Trevor Woerner
2021-10-16 20:45 ` [PATCH 1/2] arm64: dts: rk3328: add gpu opp table Johan Jonker
2021-10-17 15:29   ` Trevor Woerner
2021-10-18 18:09     ` Robin Murphy
2021-10-18 16:49 ` Nicolas Frattaroli
2021-10-19 15:52   ` Alex Bee
2021-10-19 16:19     ` Nicolas Frattaroli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).