2022-09-06 16:23:58

by Clément Péron

[permalink] [raw]
Subject: [PATCH v4 0/5] Allwinner H6 GPU devfreq

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


2022-09-06 16:24:01

by Clément Péron

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

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 <[email protected]>
---
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

2022-09-06 16:24:01

by Clément Péron

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

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 <[email protected]>
Acked-by: Jernej Skrabec <[email protected]>
---
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

2022-09-06 16:24:46

by Clément Péron

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

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 <[email protected]>
Signed-off-by: Clément Péron <[email protected]>
---
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

2022-09-06 16:25:26

by Clément Péron

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

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 <[email protected]>
---
.../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 <[email protected]>
+
+/ {
+ 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

2022-09-06 16:27:36

by Clément Péron

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

Enable GPU OPP table for Beelink GS1.

Signed-off-by: Clément Péron <[email protected]>
---
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

2022-09-06 19:37:36

by Clément Péron

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

Hi Jernej,

On Tue, 6 Sept 2022 at 21:10, Jernej Škrabec <[email protected]> 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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +/ {
> > + 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
>
>

2022-09-06 19:42:42

by Jernej Škrabec

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +/ {
> + 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


2022-09-08 10:30:47

by Steven Price

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

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 <[email protected]>
> Signed-off-by: Clément Péron <[email protected]>

Reviewed-by: Steven Price <[email protected]>

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);
>
> /*

2022-09-08 16:46:43

by Jernej Škrabec

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

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 <[email protected]>
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 <[email protected]>
> > > ---
> > >
> > > .../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 <[email protected]>
> > > +
> > > +/ {
> > > + 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


2022-09-08 17:35:24

by Jernej Škrabec

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

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 <[email protected]>

Acked-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2022-09-08 17:36:39

by Jernej Škrabec

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

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 <[email protected]>
>
> 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 <[email protected]>
> > > > ---
> > > >
> > > > .../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 <[email protected]>
> > > > +
> > > > +/ {
> > > > + 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 <[email protected]>

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


2022-09-08 17:36:39

by Jernej Škrabec

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

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 <[email protected]>

Acked-by: Jernej Skrabec <[email protected]>

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


2022-09-08 20:34:50

by Jernej Škrabec

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

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
> [email protected]/ 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