[PATCH v4 0/5] Allwinner H6 GPU devfreq

[PATCH v4 0/5] Allwinner H6 GPU devfreq

[Clément Péron]

Hi,

This is a refresh of previous patches sent to enable GPU Devfreq on H6
Beelink GS1 but that wasn't stable at that time[0].

With the recent fix on GPU PLL from Roman Stratiienko I have retested
and everything seems stable and works as expected[1].

Regards,
Clement

0: https://lore.kernel.org/lkml/CAJiuCce58Gaxf_Qg2cnMwvOgUqYU__eKb3MDX1Fe_+47htg2bA@mail.gmail.com/
1: https://lore.kernel.org/linux-arm-kernel/2562485.k3LOHGUjKi@kista/T/

Changes since v3:
 - Try to be more explicit for panfrost OPP patch
 - Fix typo

Changes since v2:
 - Fixes device-tree warnings
 - Add panfrost fix to enable regulator
 - Remove always-on regulator from device-tree
 - Update cooling map from vendor kernel


Clément Péron (5):
  arm64: defconfig: Enable devfreq cooling device
  arm64: dts: allwinner: h6: Add cooling map for GPU
  arm64: dts: allwinner: h6: Add GPU OPP table
  drm/panfrost: devfreq: set opp to the recommended one to configure
    regulator
  arm64: dts: allwinner: beelink-gs1: Enable GPU OPP

 .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |  1 +
 .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 51 ++++++++++-
 arch/arm64/configs/defconfig                  |  1 +
 drivers/gpu/drm/panfrost/panfrost_devfreq.c   | 11 +++
 5 files changed, 149 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi

-- 
2.34.1

[PATCH v4 1/5] arm64: defconfig: Enable devfreq cooling device

[Clément Péron]

Devfreq cooling device framework is used in Panfrost
to throttle GPU in order to regulate its temperature.

Enable this driver for ARM64 SoC.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 91e58cf59c99..e557ccac8d9c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -582,6 +582,7 @@ CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_INA3221=m
 CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
 CONFIG_CPU_THERMAL=y
+CONFIG_DEVFREQ_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_IMX_SC_THERMAL=m
 CONFIG_IMX8MM_THERMAL=m
-- 
2.34.1

[PATCH v4 2/5] arm64: dts: allwinner: h6: Add cooling map for GPU

[Clément Péron]

Add a simple cooling map for the GPU.

This cooling map come from the vendor kernel 4.9 with a
2°C hysteresis added.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 51 +++++++++++++++++++-
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 5a28303d3d4c..53f6660656ac 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -186,6 +186,7 @@ gpu: gpu@1800000 {
 			clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>;
 			clock-names = "core", "bus";
 			resets = <&ccu RST_BUS_GPU>;
+			#cooling-cells = <2>;
 			status = "disabled";
 		};
 
@@ -1072,9 +1073,55 @@ map0 {
 		};
 
 		gpu-thermal {
-			polling-delay-passive = <0>;
-			polling-delay = <0>;
+			polling-delay-passive = <1000>;
+			polling-delay = <2000>;
 			thermal-sensors = <&ths 1>;
+
+			trips {
+				gpu_alert0: gpu-alert-0 {
+					temperature = <95000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				gpu_alert1: gpu-alert-1 {
+					temperature = <100000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				gpu_alert2: gpu-alert-2 {
+					temperature = <105000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				gpu-crit {
+					temperature = <115000>;
+					hysteresis = <0>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				// Forbid the GPU to go over 756MHz
+				map0 {
+					trip = <&gpu_alert0>;
+					cooling-device = <&gpu 1 THERMAL_NO_LIMIT>;
+				};
+
+				// Forbid the GPU to go over 624MHz
+				map1 {
+					trip = <&gpu_alert1>;
+					cooling-device = <&gpu 2 THERMAL_NO_LIMIT>;
+				};
+
+				// Forbid the GPU to go over 576MHz
+				map2 {
+					trip = <&gpu_alert2>;
+					cooling-device = <&gpu 3 THERMAL_NO_LIMIT>;
+				};
+			};
 		};
 	};
 };
-- 
2.34.1

Re: [PATCH v4 2/5] arm64: dts: allwinner: h6: Add cooling map for GPU

[Jernej Škrabec]

Dne torek, 06. september 2022 ob 17:30:31 CEST je Clément Péron napisal(a):
> Add a simple cooling map for the GPU.
> 
> This cooling map come from the vendor kernel 4.9 with a
> 2°C hysteresis added.
> 
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table

[Clément Péron]

Add an Operating Performance Points table for the GPU to
enable Dynamic Voltage & Frequency Scaling on the H6.

The voltage range is set with minimal voltage set to the target
and the maximal voltage set to 1.2V. This allow DVFS framework to
work properly on board with fixed regulator.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
new file mode 100644
index 000000000000..b48049c4fc85
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+// Copyright (C) 2022 Clément Péron <peron.clem@gmail.com>
+
+/ {
+	gpu_opp_table: opp-table-gpu {
+		compatible = "operating-points-v2";
+
+		opp-216000000 {
+			opp-hz = /bits/ 64 <216000000>;
+			opp-microvolt = <810000 810000 1200000>;
+		};
+
+		opp-264000000 {
+			opp-hz = /bits/ 64 <264000000>;
+			opp-microvolt = <810000 810000 1200000>;
+		};
+
+		opp-312000000 {
+			opp-hz = /bits/ 64 <312000000>;
+			opp-microvolt = <810000 810000 1200000>;
+		};
+
+		opp-336000000 {
+			opp-hz = /bits/ 64 <336000000>;
+			opp-microvolt = <810000 810000 1200000>;
+		};
+
+		opp-360000000 {
+			opp-hz = /bits/ 64 <360000000>;
+			opp-microvolt = <820000 820000 1200000>;
+		};
+
+		opp-384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <830000 830000 1200000>;
+		};
+
+		opp-408000000 {
+			opp-hz = /bits/ 64 <408000000>;
+			opp-microvolt = <840000 840000 1200000>;
+		};
+
+		opp-420000000 {
+			opp-hz = /bits/ 64 <420000000>;
+			opp-microvolt = <850000 850000 1200000>;
+		};
+
+		opp-432000000 {
+			opp-hz = /bits/ 64 <432000000>;
+			opp-microvolt = <860000 860000 1200000>;
+		};
+
+		opp-456000000 {
+			opp-hz = /bits/ 64 <456000000>;
+			opp-microvolt = <870000 870000 1200000>;
+		};
+
+		opp-504000000 {
+			opp-hz = /bits/ 64 <504000000>;
+			opp-microvolt = <890000 890000 1200000>;
+		};
+
+		opp-540000000 {
+			opp-hz = /bits/ 64 <540000000>;
+			opp-microvolt = <910000 910000 1200000>;
+		};
+
+		opp-576000000 {
+			opp-hz = /bits/ 64 <576000000>;
+			opp-microvolt = <930000 930000 1200000>;
+		};
+
+		opp-624000000 {
+			opp-hz = /bits/ 64 <624000000>;
+			opp-microvolt = <950000 950000 1200000>;
+		};
+
+		opp-756000000 {
+			opp-hz = /bits/ 64 <756000000>;
+			opp-microvolt = <1040000 1040000 1200000>;
+		};
+	};
+};
+
+&gpu {
+	operating-points-v2 = <&gpu_opp_table>;
+};
-- 
2.34.1

Re: [PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table

[Jernej Škrabec]

Dne torek, 06. september 2022 ob 17:30:32 CEST je Clément Péron napisal(a):
> Add an Operating Performance Points table for the GPU to
> enable Dynamic Voltage & Frequency Scaling on the H6.
> 
> The voltage range is set with minimal voltage set to the target
> and the maximal voltage set to 1.2V. This allow DVFS framework to
> work properly on board with fixed regulator.
> 
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi new file mode 100644
> index 000000000000..b48049c4fc85
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2022 Clément Péron <peron.clem@gmail.com>
> +
> +/ {
> +	gpu_opp_table: opp-table-gpu {
> +		compatible = "operating-points-v2";
> +
> +		opp-216000000 {
> +			opp-hz = /bits/ 64 <216000000>;
> +			opp-microvolt = <810000 810000 1200000>;
> +		};
> +
> +		opp-264000000 {
> +			opp-hz = /bits/ 64 <264000000>;
> +			opp-microvolt = <810000 810000 1200000>;
> +		};

As mentioned in clock patch review, rates below 288 MHz are deemed unstable on 
GPU PLL by vendor GPU kernel driver. At least in the BSP version that I have. 
Did you test these points? If not, better to drop them.

Best regards,
Jernej

> +
> +		opp-312000000 {
> +			opp-hz = /bits/ 64 <312000000>;
> +			opp-microvolt = <810000 810000 1200000>;
> +		};
> +
> +		opp-336000000 {
> +			opp-hz = /bits/ 64 <336000000>;
> +			opp-microvolt = <810000 810000 1200000>;
> +		};
> +
> +		opp-360000000 {
> +			opp-hz = /bits/ 64 <360000000>;
> +			opp-microvolt = <820000 820000 1200000>;
> +		};
> +
> +		opp-384000000 {
> +			opp-hz = /bits/ 64 <384000000>;
> +			opp-microvolt = <830000 830000 1200000>;
> +		};
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <840000 840000 1200000>;
> +		};
> +
> +		opp-420000000 {
> +			opp-hz = /bits/ 64 <420000000>;
> +			opp-microvolt = <850000 850000 1200000>;
> +		};
> +
> +		opp-432000000 {
> +			opp-hz = /bits/ 64 <432000000>;
> +			opp-microvolt = <860000 860000 1200000>;
> +		};
> +
> +		opp-456000000 {
> +			opp-hz = /bits/ 64 <456000000>;
> +			opp-microvolt = <870000 870000 1200000>;
> +		};
> +
> +		opp-504000000 {
> +			opp-hz = /bits/ 64 <504000000>;
> +			opp-microvolt = <890000 890000 1200000>;
> +		};
> +
> +		opp-540000000 {
> +			opp-hz = /bits/ 64 <540000000>;
> +			opp-microvolt = <910000 910000 1200000>;
> +		};
> +
> +		opp-576000000 {
> +			opp-hz = /bits/ 64 <576000000>;
> +			opp-microvolt = <930000 930000 1200000>;
> +		};
> +
> +		opp-624000000 {
> +			opp-hz = /bits/ 64 <624000000>;
> +			opp-microvolt = <950000 950000 1200000>;
> +		};
> +
> +		opp-756000000 {
> +			opp-hz = /bits/ 64 <756000000>;
> +			opp-microvolt = <1040000 1040000 1200000>;
> +		};
> +	};
> +};
> +
> +&gpu {
> +	operating-points-v2 = <&gpu_opp_table>;
> +};
> --
> 2.34.1

Re: [PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table

[Clément Péron]

Hi Jernej,

On Tue, 6 Sept 2022 at 21:10, Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne torek, 06. september 2022 ob 17:30:32 CEST je Clément Péron napisal(a):
> > Add an Operating Performance Points table for the GPU to
> > enable Dynamic Voltage & Frequency Scaling on the H6.
> >
> > The voltage range is set with minimal voltage set to the target
> > and the maximal voltage set to 1.2V. This allow DVFS framework to
> > work properly on board with fixed regulator.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
> >  1 file changed, 87 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi new file mode 100644
> > index 000000000000..b48049c4fc85
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > @@ -0,0 +1,87 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +// Copyright (C) 2022 Clément Péron <peron.clem@gmail.com>
> > +
> > +/ {
> > +     gpu_opp_table: opp-table-gpu {
> > +             compatible = "operating-points-v2";
> > +
> > +             opp-216000000 {
> > +                     opp-hz = /bits/ 64 <216000000>;
> > +                     opp-microvolt = <810000 810000 1200000>;
> > +             };
> > +
> > +             opp-264000000 {
> > +                     opp-hz = /bits/ 64 <264000000>;
> > +                     opp-microvolt = <810000 810000 1200000>;
> > +             };
>
> As mentioned in clock patch review, rates below 288 MHz are deemed unstable on
> GPU PLL by vendor GPU kernel driver. At least in the BSP version that I have.
> Did you test these points? If not, better to drop them.

I changed the governor to userspace and set the freq to 216MHz / 264MHz
Run glmark2 and didn't observe any glitch nor issue.

I'm not sure if it's enough to say it's stable but I didn't observe
any strange behavior.

Regards,
Clement

>
> Best regards,
> Jernej
>
> > +
> > +             opp-312000000 {
> > +                     opp-hz = /bits/ 64 <312000000>;
> > +                     opp-microvolt = <810000 810000 1200000>;
> > +             };
> > +
> > +             opp-336000000 {
> > +                     opp-hz = /bits/ 64 <336000000>;
> > +                     opp-microvolt = <810000 810000 1200000>;
> > +             };
> > +
> > +             opp-360000000 {
> > +                     opp-hz = /bits/ 64 <360000000>;
> > +                     opp-microvolt = <820000 820000 1200000>;
> > +             };
> > +
> > +             opp-384000000 {
> > +                     opp-hz = /bits/ 64 <384000000>;
> > +                     opp-microvolt = <830000 830000 1200000>;
> > +             };
> > +
> > +             opp-408000000 {
> > +                     opp-hz = /bits/ 64 <408000000>;
> > +                     opp-microvolt = <840000 840000 1200000>;
> > +             };
> > +
> > +             opp-420000000 {
> > +                     opp-hz = /bits/ 64 <420000000>;
> > +                     opp-microvolt = <850000 850000 1200000>;
> > +             };
> > +
> > +             opp-432000000 {
> > +                     opp-hz = /bits/ 64 <432000000>;
> > +                     opp-microvolt = <860000 860000 1200000>;
> > +             };
> > +
> > +             opp-456000000 {
> > +                     opp-hz = /bits/ 64 <456000000>;
> > +                     opp-microvolt = <870000 870000 1200000>;
> > +             };
> > +
> > +             opp-504000000 {
> > +                     opp-hz = /bits/ 64 <504000000>;
> > +                     opp-microvolt = <890000 890000 1200000>;
> > +             };
> > +
> > +             opp-540000000 {
> > +                     opp-hz = /bits/ 64 <540000000>;
> > +                     opp-microvolt = <910000 910000 1200000>;
> > +             };
> > +
> > +             opp-576000000 {
> > +                     opp-hz = /bits/ 64 <576000000>;
> > +                     opp-microvolt = <930000 930000 1200000>;
> > +             };
> > +
> > +             opp-624000000 {
> > +                     opp-hz = /bits/ 64 <624000000>;
> > +                     opp-microvolt = <950000 950000 1200000>;
> > +             };
> > +
> > +             opp-756000000 {
> > +                     opp-hz = /bits/ 64 <756000000>;
> > +                     opp-microvolt = <1040000 1040000 1200000>;
> > +             };
> > +     };
> > +};
> > +
> > +&gpu {
> > +     operating-points-v2 = <&gpu_opp_table>;
> > +};
> > --
> > 2.34.1
>
>

Re: Re: [PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table

[Jernej Škrabec]

Dne torek, 06. september 2022 ob 21:26:34 CEST je Clément Péron napisal(a):
> Hi Jernej,
> 
> On Tue, 6 Sept 2022 at 21:10, Jernej Škrabec <jernej.skrabec@gmail.com> 
wrote:
> > Dne torek, 06. september 2022 ob 17:30:32 CEST je Clément Péron 
napisal(a):
> > > Add an Operating Performance Points table for the GPU to
> > > enable Dynamic Voltage & Frequency Scaling on the H6.
> > > 
> > > The voltage range is set with minimal voltage set to the target
> > > and the maximal voltage set to 1.2V. This allow DVFS framework to
> > > work properly on board with fixed regulator.
> > > 
> > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > ---
> > > 
> > >  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
> > >  1 file changed, 87 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > 
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi new file mode
> > > 100644
> > > index 000000000000..b48049c4fc85
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > @@ -0,0 +1,87 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +// Copyright (C) 2022 Clément Péron <peron.clem@gmail.com>
> > > +
> > > +/ {
> > > +     gpu_opp_table: opp-table-gpu {
> > > +             compatible = "operating-points-v2";
> > > +
> > > +             opp-216000000 {
> > > +                     opp-hz = /bits/ 64 <216000000>;
> > > +                     opp-microvolt = <810000 810000 1200000>;
> > > +             };
> > > +
> > > +             opp-264000000 {
> > > +                     opp-hz = /bits/ 64 <264000000>;
> > > +                     opp-microvolt = <810000 810000 1200000>;
> > > +             };
> > 
> > As mentioned in clock patch review, rates below 288 MHz are deemed
> > unstable on GPU PLL by vendor GPU kernel driver. At least in the BSP
> > version that I have. Did you test these points? If not, better to drop
> > them.
> 
> I changed the governor to userspace and set the freq to 216MHz / 264MHz
> Run glmark2 and didn't observe any glitch nor issue.
> 
> I'm not sure if it's enough to say it's stable but I didn't observe
> any strange behavior.

Ok then.

Forgot to ask, where did you get 1.2 V as an upper limit? H6 datasheet lists 
max. GPU voltage as 1.08 V.

Best regards,
Jernej

> 
> Regards,
> Clement
> 
> > Best regards,
> > Jernej
> > 
> > > +
> > > +             opp-312000000 {
> > > +                     opp-hz = /bits/ 64 <312000000>;
> > > +                     opp-microvolt = <810000 810000 1200000>;
> > > +             };
> > > +
> > > +             opp-336000000 {
> > > +                     opp-hz = /bits/ 64 <336000000>;
> > > +                     opp-microvolt = <810000 810000 1200000>;
> > > +             };
> > > +
> > > +             opp-360000000 {
> > > +                     opp-hz = /bits/ 64 <360000000>;
> > > +                     opp-microvolt = <820000 820000 1200000>;
> > > +             };
> > > +
> > > +             opp-384000000 {
> > > +                     opp-hz = /bits/ 64 <384000000>;
> > > +                     opp-microvolt = <830000 830000 1200000>;
> > > +             };
> > > +
> > > +             opp-408000000 {
> > > +                     opp-hz = /bits/ 64 <408000000>;
> > > +                     opp-microvolt = <840000 840000 1200000>;
> > > +             };
> > > +
> > > +             opp-420000000 {
> > > +                     opp-hz = /bits/ 64 <420000000>;
> > > +                     opp-microvolt = <850000 850000 1200000>;
> > > +             };
> > > +
> > > +             opp-432000000 {
> > > +                     opp-hz = /bits/ 64 <432000000>;
> > > +                     opp-microvolt = <860000 860000 1200000>;
> > > +             };
> > > +
> > > +             opp-456000000 {
> > > +                     opp-hz = /bits/ 64 <456000000>;
> > > +                     opp-microvolt = <870000 870000 1200000>;
> > > +             };
> > > +
> > > +             opp-504000000 {
> > > +                     opp-hz = /bits/ 64 <504000000>;
> > > +                     opp-microvolt = <890000 890000 1200000>;
> > > +             };
> > > +
> > > +             opp-540000000 {
> > > +                     opp-hz = /bits/ 64 <540000000>;
> > > +                     opp-microvolt = <910000 910000 1200000>;
> > > +             };
> > > +
> > > +             opp-576000000 {
> > > +                     opp-hz = /bits/ 64 <576000000>;
> > > +                     opp-microvolt = <930000 930000 1200000>;
> > > +             };
> > > +
> > > +             opp-624000000 {
> > > +                     opp-hz = /bits/ 64 <624000000>;
> > > +                     opp-microvolt = <950000 950000 1200000>;
> > > +             };
> > > +
> > > +             opp-756000000 {
> > > +                     opp-hz = /bits/ 64 <756000000>;
> > > +                     opp-microvolt = <1040000 1040000 1200000>;
> > > +             };
> > > +     };
> > > +};
> > > +
> > > +&gpu {
> > > +     operating-points-v2 = <&gpu_opp_table>;
> > > +};
> > > --
> > > 2.34.1

Re: Re: Re: [PATCH v4 3/5] arm64: dts: allwinner: h6: Add GPU OPP table

[Jernej Škrabec]

Dne četrtek, 08. september 2022 ob 18:26:31 CEST je Jernej Škrabec napisal(a):
> Dne torek, 06. september 2022 ob 21:26:34 CEST je Clément Péron napisal(a):
> > Hi Jernej,
> > 
> > On Tue, 6 Sept 2022 at 21:10, Jernej Škrabec <jernej.skrabec@gmail.com>
> 
> wrote:
> > > Dne torek, 06. september 2022 ob 17:30:32 CEST je Clément Péron
> 
> napisal(a):
> > > > Add an Operating Performance Points table for the GPU to
> > > > enable Dynamic Voltage & Frequency Scaling on the H6.
> > > > 
> > > > The voltage range is set with minimal voltage set to the target
> > > > and the maximal voltage set to 1.2V. This allow DVFS framework to
> > > > work properly on board with fixed regulator.
> > > > 
> > > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > > ---
> > > > 
> > > >  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87
> > > >  +++++++++++++++++++
> > > >  1 file changed, 87 insertions(+)
> > > >  create mode 100644
> > > >  arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > > b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi new file mode
> > > > 100644
> > > > index 000000000000..b48049c4fc85
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > > > @@ -0,0 +1,87 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > +// Copyright (C) 2022 Clément Péron <peron.clem@gmail.com>
> > > > +
> > > > +/ {
> > > > +     gpu_opp_table: opp-table-gpu {
> > > > +             compatible = "operating-points-v2";
> > > > +
> > > > +             opp-216000000 {
> > > > +                     opp-hz = /bits/ 64 <216000000>;
> > > > +                     opp-microvolt = <810000 810000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-264000000 {
> > > > +                     opp-hz = /bits/ 64 <264000000>;
> > > > +                     opp-microvolt = <810000 810000 1200000>;
> > > > +             };
> > > 
> > > As mentioned in clock patch review, rates below 288 MHz are deemed
> > > unstable on GPU PLL by vendor GPU kernel driver. At least in the BSP
> > > version that I have. Did you test these points? If not, better to drop
> > > them.
> > 
> > I changed the governor to userspace and set the freq to 216MHz / 264MHz
> > Run glmark2 and didn't observe any glitch nor issue.
> > 
> > I'm not sure if it's enough to say it's stable but I didn't observe
> > any strange behavior.
> 
> Ok then.
> 
> Forgot to ask, where did you get 1.2 V as an upper limit? H6 datasheet lists
> max. GPU voltage as 1.08 V.

To answer my own question, absolute max. voltage is 1.3 V, so 1.2 V is still 
somewhat acceptable and in practice, fixed regulator on Tanix TX6 board is 
around 1.12 V. Boards with PMIC can set lower voltage anyway.

All good.
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
 
Best regards,
Jernej
 
> > Regards,
> > Clement
> > 
> > > Best regards,
> > > Jernej
> > > 
> > > > +
> > > > +             opp-312000000 {
> > > > +                     opp-hz = /bits/ 64 <312000000>;
> > > > +                     opp-microvolt = <810000 810000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-336000000 {
> > > > +                     opp-hz = /bits/ 64 <336000000>;
> > > > +                     opp-microvolt = <810000 810000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-360000000 {
> > > > +                     opp-hz = /bits/ 64 <360000000>;
> > > > +                     opp-microvolt = <820000 820000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-384000000 {
> > > > +                     opp-hz = /bits/ 64 <384000000>;
> > > > +                     opp-microvolt = <830000 830000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-408000000 {
> > > > +                     opp-hz = /bits/ 64 <408000000>;
> > > > +                     opp-microvolt = <840000 840000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-420000000 {
> > > > +                     opp-hz = /bits/ 64 <420000000>;
> > > > +                     opp-microvolt = <850000 850000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-432000000 {
> > > > +                     opp-hz = /bits/ 64 <432000000>;
> > > > +                     opp-microvolt = <860000 860000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-456000000 {
> > > > +                     opp-hz = /bits/ 64 <456000000>;
> > > > +                     opp-microvolt = <870000 870000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-504000000 {
> > > > +                     opp-hz = /bits/ 64 <504000000>;
> > > > +                     opp-microvolt = <890000 890000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-540000000 {
> > > > +                     opp-hz = /bits/ 64 <540000000>;
> > > > +                     opp-microvolt = <910000 910000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-576000000 {
> > > > +                     opp-hz = /bits/ 64 <576000000>;
> > > > +                     opp-microvolt = <930000 930000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-624000000 {
> > > > +                     opp-hz = /bits/ 64 <624000000>;
> > > > +                     opp-microvolt = <950000 950000 1200000>;
> > > > +             };
> > > > +
> > > > +             opp-756000000 {
> > > > +                     opp-hz = /bits/ 64 <756000000>;
> > > > +                     opp-microvolt = <1040000 1040000 1200000>;
> > > > +             };
> > > > +     };
> > > > +};
> > > > +
> > > > +&gpu {
> > > > +     operating-points-v2 = <&gpu_opp_table>;
> > > > +};
> > > > --
> > > > 2.34.1

[PATCH v4 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure regulator

[Clément Péron]

Enabling panfrost GPU OPP with dynamic regulator will make OPP
responsible to enable and configure it.

Unfortunatly OPP configure and enable the regulator when an OPP
is asked to be set, which is not the case during
panfrost_devfreq_init().

This leave the regulator unconfigured and if no GPU load is
triggered, no OPP is asked to be set which make the regulator framework
switching it off during regulator_late_cleanup() without
noticing and therefore make the board hang as any access to GPU
memory space make bus locks up.

Call dev_pm_opp_set_opp() with the recommend OPP in
panfrost_devfreq_init() to enable the regulator, this will properly
configure and enable the regulator and will avoid any switch off
by regulator_late_cleanup().

Suggested-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 5110cd9b2425..fe5f12f16a63 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -131,6 +131,17 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
 		return PTR_ERR(opp);
 
 	panfrost_devfreq_profile.initial_freq = cur_freq;
+
+	/*
+	 * Set the recommend OPP this will enable and configure the regulator
+	 * if any and will avoid a switch off by regulator_late_cleanup()
+	 */
+	ret = dev_pm_opp_set_opp(dev, opp);
+	if (ret) {
+		DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
+		return ret;
+	}
+
 	dev_pm_opp_put(opp);
 
 	/*
-- 
2.34.1

Re: [PATCH v4 4/5] drm/panfrost: devfreq: set opp to the recommended one to configure regulator

[Steven Price]

On 06/09/2022 16:30, Clément Péron wrote:
> Enabling panfrost GPU OPP with dynamic regulator will make OPP
> responsible to enable and configure it.
> 
> Unfortunatly OPP configure and enable the regulator when an OPP

NIT: Unfortunately

> is asked to be set, which is not the case during
> panfrost_devfreq_init().
> 
> This leave the regulator unconfigured and if no GPU load is
> triggered, no OPP is asked to be set which make the regulator framework
> switching it off during regulator_late_cleanup() without
> noticing and therefore make the board hang as any access to GPU
> memory space make bus locks up.
> 
> Call dev_pm_opp_set_opp() with the recommend OPP in
> panfrost_devfreq_init() to enable the regulator, this will properly
> configure and enable the regulator and will avoid any switch off
> by regulator_late_cleanup().
> 
> Suggested-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Reviewed-by: Steven Price <steven.price@arm.com>

Note this same sequence is used in the Lima driver, so it would be good
to submit the fix there too as it presumably is affected by the same
issue. I've CC'd Qiang for visibility.

I'll push this patch to drm-misc-fixes (with the typo above fixed), the
device tree patches can go through a different tree.

Steve

> ---
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 5110cd9b2425..fe5f12f16a63 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -131,6 +131,17 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>  		return PTR_ERR(opp);
>  
>  	panfrost_devfreq_profile.initial_freq = cur_freq;
> +
> +	/*
> +	 * Set the recommend OPP this will enable and configure the regulator
> +	 * if any and will avoid a switch off by regulator_late_cleanup()
> +	 */
> +	ret = dev_pm_opp_set_opp(dev, opp);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
> +		return ret;
> +	}
> +
>  	dev_pm_opp_put(opp);
>  
>  	/*

[PATCH v4 5/5] arm64: dts: allwinner: beelink-gs1: Enable GPU OPP

[Clément Péron]

Enable GPU OPP table for Beelink GS1.

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
index 6249e9e02928..9ec49ac2f6fd 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -5,6 +5,7 @@
 
 #include "sun50i-h6.dtsi"
 #include "sun50i-h6-cpu-opp.dtsi"
+#include "sun50i-h6-gpu-opp.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
 
-- 
2.34.1

Re: [PATCH v4 5/5] arm64: dts: allwinner: beelink-gs1: Enable GPU OPP

[Jernej Škrabec]

Dne torek, 06. september 2022 ob 17:30:34 CEST je Clément Péron napisal(a):
> Enable GPU OPP table for Beelink GS1.
> 
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts index
> 6249e9e02928..9ec49ac2f6fd 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> @@ -5,6 +5,7 @@
> 
>  #include "sun50i-h6.dtsi"
>  #include "sun50i-h6-cpu-opp.dtsi"
> +#include "sun50i-h6-gpu-opp.dtsi"
> 
>  #include <dt-bindings/gpio/gpio.h>
> 
> --
> 2.34.1

Re: [PATCH v4 0/5] Allwinner H6 GPU devfreq

[Jernej Škrabec]

Dne torek, 06. september 2022 ob 17:30:29 CEST je Clément Péron napisal(a):
> Hi,
> 
> This is a refresh of previous patches sent to enable GPU Devfreq on H6
> Beelink GS1 but that wasn't stable at that time[0].
> 
> With the recent fix on GPU PLL from Roman Stratiienko I have retested
> and everything seems stable and works as expected[1].
> 
> Regards,
> Clement

All patches except patch 4 pushed to sunxi tree. Thanks!

Best regards,
Jernej

> 
> 0:
> https://lore.kernel.org/lkml/CAJiuCce58Gaxf_Qg2cnMwvOgUqYU__eKb3MDX1Fe_+47h
> tg2bA@mail.gmail.com/ 1:
> https://lore.kernel.org/linux-arm-kernel/2562485.k3LOHGUjKi@kista/T/
> 
> Changes since v3:
>  - Try to be more explicit for panfrost OPP patch
>  - Fix typo
> 
> Changes since v2:
>  - Fixes device-tree warnings
>  - Add panfrost fix to enable regulator
>  - Remove always-on regulator from device-tree
>  - Update cooling map from vendor kernel
> 
> 
> Clément Péron (5):
>   arm64: defconfig: Enable devfreq cooling device
>   arm64: dts: allwinner: h6: Add cooling map for GPU
>   arm64: dts: allwinner: h6: Add GPU OPP table
>   drm/panfrost: devfreq: set opp to the recommended one to configure
>     regulator
>   arm64: dts: allwinner: beelink-gs1: Enable GPU OPP
> 
>  .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |  1 +
>  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 87 +++++++++++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 51 ++++++++++-
>  arch/arm64/configs/defconfig                  |  1 +
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c   | 11 +++
>  5 files changed, 149 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> 
> --
> 2.34.1