[PATCH v2 0/7] Add support for Allwinner H6 DVFS

[PATCH v2 0/7] Add support for Allwinner H6 DVFS

[Clément Péron]

Hi Sunxi maintainers and members,

Now that required drivers are merged we can contibute on DVFS
support for Allwinner H6.

This serie is based on Yangtao Li serie[0] and Ondřej Jirman work[1].

Most of the OPP tables are taken from original vendor kernel[2].
Plus there is a new CPU frequencies at 1.6GHz, 1.7GHz and 1.8GHz.

I wrote a simple script to randomly set a frequency during
a random time[3].
With this script and using stress-ng during several hours, I didn't
see any issue. Moreover I have tested specifically the 1.8GHz on my
Beelink GS1, max thermal 85°C is reached very quickly and then the
SoC oscillates quickly between 1.5 and 1.8GHz. So i have added
1.6GHz and 1.7GHz my board now oscillate slower between 1.5GHz and
1.6GHz swapping every second and temperature is also morestable.

I also test that that offlining CPU0 and doing DVFS on other CPUs
works. As CPU regulator is only set for CPU0.

But maybe it doesn't cost much to set the regulator for all the CPUs?

Jernej test the GPU devfreq on several H6 board particulary the
Tanix TX6 which doesn't have a proper dedicated PMIC and doesn't
had any trouble with it.

Do you think I can enable GPU OPP for all H6 Boards?

Also Yangtao Li enable DVFS for Pine64, as I can't test it.
I'm waiting for his ack or someone who has a Pine64 and can test this
serie before reenabling this board.

Thanks,
Clément

0: https://patchwork.kernel.org/cover/10815117/
1: https://megous.com/git/linux/log/?h=ths-5.7
2: https://github.com/orangepi-xunlong/OrangePiH6_Linux4_9/blob/master/arch/arm64/boot/dts/sunxi/sun50iw6p1.dtsi#L345-L517
3: https://gist.github.com/clementperon/55a055dae3f13bbd14fb39c0069fe2e2

Changes since v1 (thanks to Ondřej Jirman):
  - Remove Polling thermal
  - Add Orange Pi boards
  - Remove minimal voltage change for Beelink GS1
  - Add ramp-deplay for GPU and CPU regulators
  - Push to thermal point to 85°C (Allwinner set them to 100°C and 115°C)
  - Added 1.6GHz and 1.7GHz to OPP table. 

Clément Péron (4):
  arm64: dts: allwinner: h6: Add GPU Operating Performance Points table
  arm64: configs: Enable sun50i cpufreq nvmem
  arm64: dts: allwinner: h6: Enable CPU and GPU opp tables for Beelink
    GS1
  arm64: dts: allwinner: h6: Enable CPU and GPU opp tables for Orange Pi
    boards

Ondrej Jirman (2):
  arm64: dts: allwinner: h6: Add thermal trip points/cooling map
  arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

Yangtao Li (1):
  arm64: dts: allwinner: h6: Add clock to CPU cores

 .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |  10 +-
 .../boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi | 121 ++++++++++++++++++
 .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi |  74 +++++++++++
 .../dts/allwinner/sun50i-h6-orangepi.dtsi     |  10 +-
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |  36 ++++++
 arch/arm64/configs/defconfig                  |   1 +
 6 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi

-- 
2.20.1

[PATCH v2 1/7] arm64: dts: allwinner: h6: Add clock to CPU cores

[Clément Péron]

From: Yangtao Li <tiny.windzz@gmail.com>

The ARM CPU cores are fed by the CPU clock from the CCU. Add a
reference to the clock for each CPU core, along with the clock
transition latency.

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

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 3329283e38ab..aef4ae760d5e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -25,6 +25,8 @@
 			device_type = "cpu";
 			reg = <0>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu1: cpu@1 {
@@ -32,6 +34,8 @@
 			device_type = "cpu";
 			reg = <1>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu2: cpu@2 {
@@ -39,6 +43,8 @@
 			device_type = "cpu";
 			reg = <2>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 
 		cpu3: cpu@3 {
@@ -46,6 +52,8 @@
 			device_type = "cpu";
 			reg = <3>;
 			enable-method = "psci";
+			clocks = <&ccu CLK_CPUX>;
+			clock-latency-ns = <244144>; /* 8 32k periods */
 		};
 	};
 
-- 
2.20.1

[PATCH v2 2/7] arm64: dts: allwinner: h6: Add thermal trip points/cooling map

[Clément Péron]

From: Ondrej Jirman <megous@megous.com>

This enables passive cooling by down-regulating CPU voltage
and frequency.

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

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index aef4ae760d5e..e0dd0757be0b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -908,6 +908,30 @@
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			thermal-sensors = <&ths 0>;
+
+			trips {
+				cpu_hot_trip: cpu-hot {
+					temperature = <85000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				cpu_very_hot_trip: cpu-very-hot {
+					temperature = <100000>;
+					hysteresis = <0>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				cpu-hot-limit {
+					trip = <&cpu_hot_trip>;
+					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>;
+				};
+			};
 		};
 
 		gpu-thermal {
-- 
2.20.1

[PATCH v2 3/7] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Clément Péron]

From: Ondrej Jirman <megous@megous.com>

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

Signed-off-by: Ondrej Jirman <megous@megous.com>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi | 121 ++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   4 +
 2 files changed, 125 insertions(+)
 create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
new file mode 100644
index 000000000000..8c1e413c6af9
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+// Copyright (C) 2020 Ondrej Jirman <megous@megous.com>
+// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
+
+/ {
+	cpu0_opp_table: opp_table0 {
+		compatible = "allwinner,sun50i-h6-operating-points";
+		nvmem-cells = <&speedbin_efuse>;
+		opp-shared;
+
+		opp@480000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <480000000>;
+
+			opp-microvolt-speed0 = <880000>;
+			opp-microvolt-speed1 = <820000>;
+			opp-microvolt-speed2 = <820000>;
+		};
+
+		opp@720000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <720000000>;
+
+			opp-microvolt-speed0 = <880000>;
+			opp-microvolt-speed1 = <820000>;
+			opp-microvolt-speed2 = <820000>;
+		};
+
+		opp@816000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <816000000>;
+
+			opp-microvolt-speed0 = <880000>;
+			opp-microvolt-speed1 = <820000>;
+			opp-microvolt-speed2 = <820000>;
+		};
+
+		opp@888000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <888000000>;
+
+			opp-microvolt-speed0 = <880000>;
+			opp-microvolt-speed1 = <820000>;
+			opp-microvolt-speed2 = <820000>;
+		};
+
+		opp@1080000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1080000000>;
+
+			opp-microvolt-speed0 = <940000>;
+			opp-microvolt-speed1 = <880000>;
+			opp-microvolt-speed2 = <880000>;
+		};
+
+		opp@1320000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1320000000>;
+
+			opp-microvolt-speed0 = <1000000>;
+			opp-microvolt-speed1 = <940000>;
+			opp-microvolt-speed2 = <940000>;
+		};
+
+		opp@1488000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1488000000>;
+
+			opp-microvolt-speed0 = <1060000>;
+			opp-microvolt-speed1 = <1000000>;
+			opp-microvolt-speed2 = <1000000>;
+		};
+
+		opp@1608000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1608000000>;
+
+			opp-microvolt-speed0 = <1090000>;
+			opp-microvolt-speed1 = <1030000>;
+			opp-microvolt-speed2 = <1030000>;
+		};
+
+		opp@1704000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1704000000>;
+
+			opp-microvolt-speed0 = <1120000>;
+			opp-microvolt-speed1 = <1060000>;
+			opp-microvolt-speed2 = <1060000>;
+		};
+
+		opp@1800000000 {
+			clock-latency-ns = <244144>; /* 8 32k periods */
+			opp-hz = /bits/ 64 <1800000000>;
+
+			opp-microvolt-speed0 = <1160000>;
+			opp-microvolt-speed1 = <1100000>;
+			opp-microvolt-speed2 = <1100000>;
+		};
+	};
+};
+
+&cpu0 {
+	operating-points-v2 = <&cpu0_opp_table>;
+	#cooling-cells = <2>;
+};
+
+&cpu1 {
+	operating-points-v2 = <&cpu0_opp_table>;
+	#cooling-cells = <2>;
+};
+
+&cpu2 {
+	operating-points-v2 = <&cpu0_opp_table>;
+	#cooling-cells = <2>;
+};
+
+&cpu3 {
+	operating-points-v2 = <&cpu0_opp_table>;
+	#cooling-cells = <2>;
+};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index e0dd0757be0b..6b7af858614a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -253,6 +253,10 @@
 			#address-cells = <1>;
 			#size-cells = <1>;
 
+			speedbin_efuse: speed@1c {
+				reg = <0x1c 0x4>;
+			};
+
 			ths_calibration: thermal-sensor-calibration@14 {
 				reg = <0x14 0x8>;
 			};
-- 
2.20.1

[PATCH v2 4/7] arm64: dts: allwinner: h6: Add GPU Operating Performance Points table

[Clément Péron]

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

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 74 +++++++++++++++++++
 1 file changed, 74 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..4a1814844fe0
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
+
+/ {
+	gpu_opp_table: opp_table1 {
+		compatible = "operating-points-v2";
+
+		opp@756000000 {
+			opp-hz = /bits/ 64 <756000000>;
+			opp-microvolt = <1040000>;
+		};
+		opp@624000000 {
+			opp-hz = /bits/ 64 <624000000>;
+			opp-microvolt = <950000>;
+		};
+		opp@576000000 {
+			opp-hz = /bits/ 64 <576000000>;
+			opp-microvolt = <930000>;
+		};
+		opp@540000000 {
+			opp-hz = /bits/ 64 <540000000>;
+			opp-microvolt = <910000>;
+		};
+		opp@504000000 {
+			opp-hz = /bits/ 64 <504000000>;
+			opp-microvolt = <890000>;
+		};
+		opp@456000000 {
+			opp-hz = /bits/ 64 <456000000>;
+			opp-microvolt = <870000>;
+		};
+		opp@432000000 {
+			opp-hz = /bits/ 64 <432000000>;
+			opp-microvolt = <860000>;
+		};
+		opp@420000000 {
+			opp-hz = /bits/ 64 <420000000>;
+			opp-microvolt = <850000>;
+		};
+		opp@408000000 {
+			opp-hz = /bits/ 64 <408000000>;
+			opp-microvolt = <840000>;
+		};
+		opp@384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+			opp-microvolt = <830000>;
+		};
+		opp@360000000 {
+			opp-hz = /bits/ 64 <360000000>;
+			opp-microvolt = <820000>;
+		};
+		opp@336000000 {
+			opp-hz = /bits/ 64 <336000000>;
+			opp-microvolt = <810000>;
+		};
+		opp@312000000 {
+			opp-hz = /bits/ 64 <312000000>;
+			opp-microvolt = <810000>;
+		};
+		opp@264000000 {
+			opp-hz = /bits/ 64 <264000000>;
+			opp-microvolt = <810000>;
+		};
+		opp@216000000 {
+			opp-hz = /bits/ 64 <216000000>;
+			opp-microvolt = <810000>;
+		};
+	};
+};
+
+&gpu {
+	operating-points-v2 = <&gpu_opp_table>;
+	#cooling-cells = <2>;
+};
-- 
2.20.1

[PATCH v2 5/7] arm64: configs: Enable sun50i cpufreq nvmem

[Clément Péron]

Allwinner H6 needs this driver to be able to get
the correct speed_bin required for DVFS.

Enable this option in arm64 defconfig.

Signed-off-by: Clément Péron <peron.clem@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 4db223dbc549..28b25fc0347c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -83,6 +83,7 @@ CONFIG_CPU_FREQ_GOV_CONSERVATIVE=m
 CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y
 CONFIG_CPUFREQ_DT=y
 CONFIG_ACPI_CPPC_CPUFREQ=m
+CONFIG_ARM_ALLWINNER_SUN50I_CPUFREQ_NVMEM=y
 CONFIG_ARM_ARMADA_37XX_CPUFREQ=y
 CONFIG_ARM_SCPI_CPUFREQ=y
 CONFIG_ARM_IMX_CPUFREQ_DT=m
-- 
2.20.1

[PATCH v2 6/7] arm64: dts: allwinner: h6: Enable CPU and GPU opp tables for Beelink GS1

[Clément Péron]

Enable CPU and GPU opp tables for Beelink GS1.

This needs to change the CPU regulator max voltage to fit
the OPP table.

Also add the ramp-delay information to avoid any out of spec
running as the regulator is slower at reaching the voltage
requested compare to the PLL reaching the frequency.

There is no such information for AXP805 but similar PMIC (AXP813)
has a DVM (Dynamic Voltage scaling Management) ramp rate equal
to 2500uV/us.

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

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 df6d872c34e2..d76a0dd75222 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -4,6 +4,8 @@
 /dts-v1/;
 
 #include "sun50i-h6.dtsi"
+#include "sun50i-h6-cpu-opp.dtsi"
+#include "sun50i-h6-gpu-opp.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
 
@@ -70,6 +72,10 @@
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdca>;
+};
+
 &de {
 	status = "okay";
 };
@@ -227,7 +233,8 @@
 			reg_dcdca: dcdca {
 				regulator-always-on;
 				regulator-min-microvolt = <810000>;
-				regulator-max-microvolt = <1080000>;
+				regulator-max-microvolt = <1160000>;
+				regulator-ramp-delay = <2500>;
 				regulator-name = "vdd-cpu";
 			};
 
@@ -235,6 +242,7 @@
 				regulator-enable-ramp-delay = <32000>;
 				regulator-min-microvolt = <810000>;
 				regulator-max-microvolt = <1080000>;
+				regulator-ramp-delay = <2500>;
 				regulator-name = "vdd-gpu";
 			};
 
-- 
2.20.1

[PATCH v2 7/7] arm64: dts: allwinner: h6: Enable CPU and GPU opp tables for Orange Pi boards

[Clément Péron]

Enable CPU and GPU opp tables for Orange Pi boards.

This needs to change the CPU regulator max voltage to fit
the OPP table.

Also add the ramp-delay information to avoid any out of spec
running as the regulator is slower at reaching the voltage
requested compare to the PLL reaching the frequency.

There is no such information for AXP805 but similar PMIC (AXP813)
has a DVM (Dynamic Voltage scaling Management) ramp rate equal
to 2500uV/us.

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

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
index 37f4c57597d4..f3ba500ce4c6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
@@ -5,6 +5,8 @@
 /dts-v1/;
 
 #include "sun50i-h6.dtsi"
+#include "sun50i-h6-cpu-opp.dtsi"
+#include "sun50i-h6-gpu-opp.dtsi"
 
 #include <dt-bindings/gpio/gpio.h>
 
@@ -45,6 +47,10 @@
 	};
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdca>;
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -161,7 +167,8 @@
 			reg_dcdca: dcdca {
 				regulator-always-on;
 				regulator-min-microvolt = <810000>;
-				regulator-max-microvolt = <1080000>;
+				regulator-max-microvolt = <1160000>;
+				regulator-ramp-delay = <2500>;
 				regulator-name = "vdd-cpu";
 			};
 
@@ -169,6 +176,7 @@
 				regulator-enable-ramp-delay = <32000>;
 				regulator-min-microvolt = <810000>;
 				regulator-max-microvolt = <1080000>;
+				regulator-ramp-delay = <2500>;
 				regulator-name = "vdd-gpu";
 			};
 
-- 
2.20.1

Re: [PATCH v2 0/7] Add support for Allwinner H6 DVFS

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

Hi,

On Sun, Apr 05, 2020 at 07:35:54PM +0200, Clément Péron wrote:
> Now that required drivers are merged we can contibute on DVFS
> support for Allwinner H6.
>
> This serie is based on Yangtao Li serie[0] and Ondřej Jirman work[1].
>
> Most of the OPP tables are taken from original vendor kernel[2].
> Plus there is a new CPU frequencies at 1.6GHz, 1.7GHz and 1.8GHz.
>
> I wrote a simple script to randomly set a frequency during
> a random time[3].

If you ever need to do that ever again, cpufreq-ljt-stress-test (found
here https://github.com/ssvb/cpuburn-arm) has proven to be very
reliable to detect cpufreq related issues. stress-ng might not be
enough since the (at least older) Allwinner SoCs tend to create cache
corruption when undervolted, and that might not be unnoticed by
stress-ng but will be catched by cpufreq-ljt-stress-test.

Also, it will test each frequency, while random frequencies might skip
a few.

> With this script and using stress-ng during several hours, I didn't
> see any issue. Moreover I have tested specifically the 1.8GHz on my
> Beelink GS1, max thermal 85°C is reached very quickly and then the
> SoC oscillates quickly between 1.5 and 1.8GHz. So i have added
> 1.6GHz and 1.7GHz my board now oscillate slower between 1.5GHz and
> 1.6GHz swapping every second and temperature is also morestable.
>
> I also test that that offlining CPU0 and doing DVFS on other CPUs
> works. As CPU regulator is only set for CPU0.
>
> But maybe it doesn't cost much to set the regulator for all the CPUs?
>
> Jernej test the GPU devfreq on several H6 board particulary the
> Tanix TX6 which doesn't have a proper dedicated PMIC and doesn't
> had any trouble with it.
>
> Do you think I can enable GPU OPP for all H6 Boards?

It seems you're doing it?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/7] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5006 bytes --]

On Sun, Apr 05, 2020 at 07:35:57PM +0200, Clément Péron wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> Add an Operating Performance Points table for the CPU cores to
> enable Dynamic Voltage & Frequency Scaling on the H6.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi | 121 ++++++++++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   4 +
>  2 files changed, 125 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
> new file mode 100644
> index 000000000000..8c1e413c6af9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2020 Ondrej Jirman <megous@megous.com>
> +// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
> +
> +/ {
> +	cpu0_opp_table: opp_table0 {

Node names shouldn't have an underscore, this will trigger a DTC
warning.

> +		compatible = "allwinner,sun50i-h6-operating-points";
> +		nvmem-cells = <&speedbin_efuse>;
> +		opp-shared;
> +
> +		opp@480000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <480000000>;
> +
> +			opp-microvolt-speed0 = <880000>;
> +			opp-microvolt-speed1 = <820000>;
> +			opp-microvolt-speed2 = <820000>;
> +		};

And similarly, if you have a unit-address (the part after @), you
should have a matching reg property. You should use a dash instead.

> +		opp@720000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <720000000>;
> +
> +			opp-microvolt-speed0 = <880000>;
> +			opp-microvolt-speed1 = <820000>;
> +			opp-microvolt-speed2 = <820000>;
> +		};
> +
> +		opp@816000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <816000000>;
> +
> +			opp-microvolt-speed0 = <880000>;
> +			opp-microvolt-speed1 = <820000>;
> +			opp-microvolt-speed2 = <820000>;
> +		};
> +
> +		opp@888000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <888000000>;
> +
> +			opp-microvolt-speed0 = <880000>;
> +			opp-microvolt-speed1 = <820000>;
> +			opp-microvolt-speed2 = <820000>;
> +		};
> +
> +		opp@1080000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1080000000>;
> +
> +			opp-microvolt-speed0 = <940000>;
> +			opp-microvolt-speed1 = <880000>;
> +			opp-microvolt-speed2 = <880000>;
> +		};
> +
> +		opp@1320000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1320000000>;
> +
> +			opp-microvolt-speed0 = <1000000>;
> +			opp-microvolt-speed1 = <940000>;
> +			opp-microvolt-speed2 = <940000>;
> +		};
> +
> +		opp@1488000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1488000000>;
> +
> +			opp-microvolt-speed0 = <1060000>;
> +			opp-microvolt-speed1 = <1000000>;
> +			opp-microvolt-speed2 = <1000000>;
> +		};
> +
> +		opp@1608000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1608000000>;
> +
> +			opp-microvolt-speed0 = <1090000>;
> +			opp-microvolt-speed1 = <1030000>;
> +			opp-microvolt-speed2 = <1030000>;
> +		};
> +
> +		opp@1704000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1704000000>;
> +
> +			opp-microvolt-speed0 = <1120000>;
> +			opp-microvolt-speed1 = <1060000>;
> +			opp-microvolt-speed2 = <1060000>;
> +		};
> +
> +		opp@1800000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1800000000>;
> +
> +			opp-microvolt-speed0 = <1160000>;
> +			opp-microvolt-speed1 = <1100000>;
> +			opp-microvolt-speed2 = <1100000>;
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +	#cooling-cells = <2>;
> +};
> +
> +&cpu1 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +	#cooling-cells = <2>;
> +};
> +
> +&cpu2 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +	#cooling-cells = <2>;
> +};
> +
> +&cpu3 {
> +	operating-points-v2 = <&cpu0_opp_table>;
> +	#cooling-cells = <2>;
> +};
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index e0dd0757be0b..6b7af858614a 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -253,6 +253,10 @@
>  			#address-cells = <1>;
>  			#size-cells = <1>;
>
> +			speedbin_efuse: speed@1c {
> +				reg = <0x1c 0x4>;
> +			};
> +

You should order this by address, so after the THS calibration. Also,
using a less generic node name than "speed" would be great. What about
soc-bin ?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 4/7] arm64: dts: allwinner: h6: Add GPU Operating Performance Points table

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

On Sun, Apr 05, 2020 at 07:35:58PM +0200, Clément Péron wrote:
> Add an Operating Performance Points table for the GPU to
> enable Dynamic Voltage & Frequency Scaling on the H6.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 74 +++++++++++++++++++
>  1 file changed, 74 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..4a1814844fe0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
> +
> +/ {
> +	gpu_opp_table: opp_table1 {

A node name is supposed to be a generic description, so what about
using gpu-opp-table (and cpu-opp-table) instead?

> +		compatible = "operating-points-v2";
> +
> +		opp@756000000 {
> +			opp-hz = /bits/ 64 <756000000>;
> +			opp-microvolt = <1040000>;
> +		};
> +		opp@624000000 {
> +			opp-hz = /bits/ 64 <624000000>;
> +			opp-microvolt = <950000>;
> +		};

Ordering by ascending frequency would be great here.

Also, why did you need to create a new DTSI for it? Is there some
downside to enabling it for all the users?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 5/7] arm64: configs: Enable sun50i cpufreq nvmem

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Sun, Apr 05, 2020 at 07:35:59PM +0200, Clément Péron wrote:
> Allwinner H6 needs this driver to be able to get
> the correct speed_bin required for DVFS.
>
> Enable this option in arm64 defconfig.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>

In general, it's better to put anything in arm64 defconfig as a
module, this defconfig is used by everyone.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 4/7] arm64: dts: allwinner: h6: Add GPU Operating Performance Points table

[Clément Péron]

Hi Maxime,

On Mon, 6 Apr 2020 at 10:16, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Sun, Apr 05, 2020 at 07:35:58PM +0200, Clément Péron wrote:
> > Add an Operating Performance Points table for the GPU to
> > enable Dynamic Voltage & Frequency Scaling on the H6.
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi | 74 +++++++++++++++++++
> >  1 file changed, 74 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..4a1814844fe0
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-gpu-opp.dtsi
> > @@ -0,0 +1,74 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
> > +
> > +/ {
> > +     gpu_opp_table: opp_table1 {
>
> A node name is supposed to be a generic description, so what about
> using gpu-opp-table (and cpu-opp-table) instead?
>
> > +             compatible = "operating-points-v2";
> > +
> > +             opp@756000000 {
> > +                     opp-hz = /bits/ 64 <756000000>;
> > +                     opp-microvolt = <1040000>;
> > +             };
> > +             opp@624000000 {
> > +                     opp-hz = /bits/ 64 <624000000>;
> > +                     opp-microvolt = <950000>;
> > +             };
>
> Ordering by ascending frequency would be great here.
Indeed will fix it.

>
> Also, why did you need to create a new DTSI for it? Is there some
> downside to enabling it for all the users?

I didn't want to enable it for all boards as I can't test them and not all board
have a dedicated regulator for VDD-GPU.
Hence my question in the cover,
But with Jernej Škrabec test on Tanix TX6 I think it can be safely
enable on all boards.

Regards,
Clement

>
> Maxime

Re: [PATCH v2 0/7] Add support for Allwinner H6 DVFS

[Clément Péron]

Hi Maxime,

On Mon, 6 Apr 2020 at 10:10, Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
> On Sun, Apr 05, 2020 at 07:35:54PM +0200, Clément Péron wrote:
> > Now that required drivers are merged we can contibute on DVFS
> > support for Allwinner H6.
> >
> > This serie is based on Yangtao Li serie[0] and Ondřej Jirman work[1].
> >
> > Most of the OPP tables are taken from original vendor kernel[2].
> > Plus there is a new CPU frequencies at 1.6GHz, 1.7GHz and 1.8GHz.
> >
> > I wrote a simple script to randomly set a frequency during
> > a random time[3].
>
> If you ever need to do that ever again, cpufreq-ljt-stress-test (found
> here https://github.com/ssvb/cpuburn-arm) has proven to be very
> reliable to detect cpufreq related issues. stress-ng might not be
> enough since the (at least older) Allwinner SoCs tend to create cache
> corruption when undervolted, and that might not be unnoticed by
> stress-ng but will be catched by cpufreq-ljt-stress-test.

Thanks for the tool and explanation. I will test the v3 with this one.

>
> Also, it will test each frequency, while random frequencies might skip
> a few.

I didn't count them but there was more than 2000lines, probability to miss
a frequency is really low. Also changing from 500MHz to 1.8GHz could have
trigger some out-of-spec issue.

But just to be sure I will add an init phase before doing random stuff.

>
> > With this script and using stress-ng during several hours, I didn't
> > see any issue. Moreover I have tested specifically the 1.8GHz on my
> > Beelink GS1, max thermal 85°C is reached very quickly and then the
> > SoC oscillates quickly between 1.5 and 1.8GHz. So i have added
> > 1.6GHz and 1.7GHz my board now oscillate slower between 1.5GHz and
> > 1.6GHz swapping every second and temperature is also morestable.
> >
> > I also test that that offlining CPU0 and doing DVFS on other CPUs
> > works. As CPU regulator is only set for CPU0.
> >
> > But maybe it doesn't cost much to set the regulator for all the CPUs?
> >
> > Jernej test the GPU devfreq on several H6 board particulary the
> > Tanix TX6 which doesn't have a proper dedicated PMIC and doesn't
> > had any trouble with it.
> >
> > Do you think I can enable GPU OPP for all H6 Boards?
>
> It seems you're doing it?
No I don't, see my answer on patch 4/7.

Thanks for your review,
Clément

>
> Maxime

Re: [PATCH v2 7/7] arm64: dts: allwinner: h6: Enable CPU and GPU opp tables for Orange Pi boards

[Clément Péron]

Hi,

On Sun, 5 Apr 2020 at 19:36, Clément Péron <peron.clem@gmail.com> wrote:
>
> Enable CPU and GPU opp tables for Orange Pi boards.
>
> This needs to change the CPU regulator max voltage to fit
> the OPP table.
>
> Also add the ramp-delay information to avoid any out of spec
> running as the regulator is slower at reaching the voltage
> requested compare to the PLL reaching the frequency.
>
> There is no such information for AXP805 but similar PMIC (AXP813)
> has a DVM (Dynamic Voltage scaling Management) ramp rate equal
> to 2500uV/us.
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> index 37f4c57597d4..f3ba500ce4c6 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi

Made a mistake here.
Ondrej Jirman test DVFS on Orange Pi 3 board which is not this device-tree file.

Will fix in V3.

Regards,
Clément

> @@ -5,6 +5,8 @@
>  /dts-v1/;
>
>  #include "sun50i-h6.dtsi"
> +#include "sun50i-h6-cpu-opp.dtsi"
> +#include "sun50i-h6-gpu-opp.dtsi"
>
>  #include <dt-bindings/gpio/gpio.h>
>
> @@ -45,6 +47,10 @@
>         };
>  };
>
> +&cpu0 {
> +       cpu-supply = <&reg_dcdca>;
> +};
> +
>  &ehci0 {
>         status = "okay";
>  };
> @@ -161,7 +167,8 @@
>                         reg_dcdca: dcdca {
>                                 regulator-always-on;
>                                 regulator-min-microvolt = <810000>;
> -                               regulator-max-microvolt = <1080000>;
> +                               regulator-max-microvolt = <1160000>;
> +                               regulator-ramp-delay = <2500>;
>                                 regulator-name = "vdd-cpu";
>                         };
>
> @@ -169,6 +176,7 @@
>                                 regulator-enable-ramp-delay = <32000>;
>                                 regulator-min-microvolt = <810000>;
>                                 regulator-max-microvolt = <1080000>;
> +                               regulator-ramp-delay = <2500>;
>                                 regulator-name = "vdd-gpu";
>                         };
>
> --
> 2.20.1
>

Re: [PATCH v2 3/7] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Clément Péron]

Hi,

On Mon, 6 Apr 2020 at 10:14, Maxime Ripard <maxime@cerno.tech> wrote:
>
> On Sun, Apr 05, 2020 at 07:35:57PM +0200, Clément Péron wrote:
> > From: Ondrej Jirman <megous@megous.com>
> >
> > Add an Operating Performance Points table for the CPU cores to
> > enable Dynamic Voltage & Frequency Scaling on the H6.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi | 121 ++++++++++++++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  |   4 +
> >  2 files changed, 125 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
> > new file mode 100644
> > index 000000000000..8c1e413c6af9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-cpu-opp.dtsi
> > @@ -0,0 +1,121 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +// Copyright (C) 2020 Ondrej Jirman <megous@megous.com>
> > +// Copyright (C) 2020 Clément Péron <peron.clem@gmail.com>
> > +
> > +/ {
> > +     cpu0_opp_table: opp_table0 {
>
> Node names shouldn't have an underscore, this will trigger a DTC
> warning.
>
> > +             compatible = "allwinner,sun50i-h6-operating-points";
> > +             nvmem-cells = <&speedbin_efuse>;
> > +             opp-shared;
> > +
> > +             opp@480000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <480000000>;
> > +
> > +                     opp-microvolt-speed0 = <880000>;
> > +                     opp-microvolt-speed1 = <820000>;
> > +                     opp-microvolt-speed2 = <820000>;
> > +             };
>
> And similarly, if you have a unit-address (the part after @), you
> should have a matching reg property. You should use a dash instead.
>
> > +             opp@720000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <720000000>;
> > +
> > +                     opp-microvolt-speed0 = <880000>;
> > +                     opp-microvolt-speed1 = <820000>;
> > +                     opp-microvolt-speed2 = <820000>;
> > +             };
> > +
> > +             opp@816000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <816000000>;
> > +
> > +                     opp-microvolt-speed0 = <880000>;
> > +                     opp-microvolt-speed1 = <820000>;
> > +                     opp-microvolt-speed2 = <820000>;
> > +             };
> > +
> > +             opp@888000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <888000000>;
> > +
> > +                     opp-microvolt-speed0 = <880000>;
> > +                     opp-microvolt-speed1 = <820000>;
> > +                     opp-microvolt-speed2 = <820000>;
> > +             };
> > +
> > +             opp@1080000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1080000000>;
> > +
> > +                     opp-microvolt-speed0 = <940000>;
> > +                     opp-microvolt-speed1 = <880000>;
> > +                     opp-microvolt-speed2 = <880000>;
> > +             };
> > +
> > +             opp@1320000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1320000000>;
> > +
> > +                     opp-microvolt-speed0 = <1000000>;
> > +                     opp-microvolt-speed1 = <940000>;
> > +                     opp-microvolt-speed2 = <940000>;
> > +             };
> > +
> > +             opp@1488000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1488000000>;
> > +
> > +                     opp-microvolt-speed0 = <1060000>;
> > +                     opp-microvolt-speed1 = <1000000>;
> > +                     opp-microvolt-speed2 = <1000000>;
> > +             };
> > +
> > +             opp@1608000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1608000000>;
> > +
> > +                     opp-microvolt-speed0 = <1090000>;
> > +                     opp-microvolt-speed1 = <1030000>;
> > +                     opp-microvolt-speed2 = <1030000>;
> > +             };
> > +
> > +             opp@1704000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1704000000>;
> > +
> > +                     opp-microvolt-speed0 = <1120000>;
> > +                     opp-microvolt-speed1 = <1060000>;
> > +                     opp-microvolt-speed2 = <1060000>;
> > +             };
> > +
> > +             opp@1800000000 {
> > +                     clock-latency-ns = <244144>; /* 8 32k periods */
> > +                     opp-hz = /bits/ 64 <1800000000>;
> > +
> > +                     opp-microvolt-speed0 = <1160000>;
> > +                     opp-microvolt-speed1 = <1100000>;
> > +                     opp-microvolt-speed2 = <1100000>;
> > +             };
> > +     };
> > +};
> > +
> > +&cpu0 {
> > +     operating-points-v2 = <&cpu0_opp_table>;
> > +     #cooling-cells = <2>;
> > +};
> > +
> > +&cpu1 {
> > +     operating-points-v2 = <&cpu0_opp_table>;
> > +     #cooling-cells = <2>;
> > +};
> > +
> > +&cpu2 {
> > +     operating-points-v2 = <&cpu0_opp_table>;
> > +     #cooling-cells = <2>;
> > +};
> > +
> > +&cpu3 {
> > +     operating-points-v2 = <&cpu0_opp_table>;
> > +     #cooling-cells = <2>;
> > +};
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > index e0dd0757be0b..6b7af858614a 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -253,6 +253,10 @@
> >                       #address-cells = <1>;
> >                       #size-cells = <1>;
> >
> > +                     speedbin_efuse: speed@1c {
> > +                             reg = <0x1c 0x4>;
> > +                     };
> > +
>
> You should order this by address, so after the THS calibration. Also,
> using a less generic node name than "speed" would be great. What about
> soc-bin ?

Indeed it's too generic.
I will keep coherency with i.MX8 and use "cpu_speed_grade"

Thanks for the review,
Clement

>
> Maxime

Re: [PATCH v2 3/7] arm64: dts: allwinner: h6: Add CPU Operating Performance Points table

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

On Mon, Apr 06, 2020 at 08:03:34PM +0200, Clément Péron wrote:
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > > @@ -253,6 +253,10 @@
> > >                       #address-cells = <1>;
> > >                       #size-cells = <1>;
> > >
> > > +                     speedbin_efuse: speed@1c {
> > > +                             reg = <0x1c 0x4>;
> > > +                     };
> > > +
> >
> > You should order this by address, so after the THS calibration. Also,
> > using a less generic node name than "speed" would be great. What about
> > soc-bin ?
>
> Indeed it's too generic.
> I will keep coherency with i.MX8 and use "cpu_speed_grade"

It works for me, but remember that you can't use underscores :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/7] Add support for Allwinner H6 DVFS

[Clément Péron]

Hi Maxime,

On Mon, 6 Apr 2020 at 11:07, Clément Péron <peron.clem@gmail.com> wrote:
>
> Hi Maxime,
>
> On Mon, 6 Apr 2020 at 10:10, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Sun, Apr 05, 2020 at 07:35:54PM +0200, Clément Péron wrote:
> > > Now that required drivers are merged we can contibute on DVFS
> > > support for Allwinner H6.
> > >
> > > This serie is based on Yangtao Li serie[0] and Ondřej Jirman work[1].
> > >
> > > Most of the OPP tables are taken from original vendor kernel[2].
> > > Plus there is a new CPU frequencies at 1.6GHz, 1.7GHz and 1.8GHz.
> > >
> > > I wrote a simple script to randomly set a frequency during
> > > a random time[3].
> >
> > If you ever need to do that ever again, cpufreq-ljt-stress-test (found
> > here https://github.com/ssvb/cpuburn-arm) has proven to be very
> > reliable to detect cpufreq related issues. stress-ng might not be
> > enough since the (at least older) Allwinner SoCs tend to create cache
> > corruption when undervolted, and that might not be unnoticed by
> > stress-ng but will be catched by cpufreq-ljt-stress-test.

The test seems good (see result below) :

# cpufreq-ljt-stress-test
Creating './whitenoise-1920x1080.jpg' ... done
CPU stress test, which is doing JPEG decoding by libjpeg-turbo
at different cpufreq operating points.

Testing CPU 0
 1800 MHz ............................................................ OK
 1704 MHz ............................................................ OK
 1608 MHz ............................................................ OK
 1488 MHz ............................................................ OK
 1320 MHz ............................................................ OK
 1080 MHz ............................................................ OK
  888 MHz ............................................................ OK
  816 MHz ............................................................ OK
  720 MHz ............................................................ OK
  480 MHz ............................................................ OK

Testing CPU 1
 1800 MHz ............................................................ OK
 1704 MHz ............................................................ OK
 1608 MHz ............................................................ OK
 1488 MHz ............................................................ OK
 1320 MHz ............................................................ OK
 1080 MHz ............................................................ OK
  888 MHz ............................................................ OK
  816 MHz ............................................................ OK
  720 MHz ............................................................ OK
  480 MHz ............................................................ OK

Testing CPU 2
 1800 MHz ............................................................ OK
 1704 MHz ............................................................ OK
 1608 MHz ............................................................ OK
 1488 MHz ............................................................ OK
 1320 MHz ............................................................ OK
 1080 MHz ............................................................ OK
  888 MHz ............................................................ OK
  816 MHz ............................................................ OK
  720 MHz ............................................................ OK
  480 MHz ............................................................ OK

Testing CPU 3
 1800 MHz ............................................................ OK
 1704 MHz ............................................................ OK
 1608 MHz ............................................................ OK
 1488 MHz ............................................................ OK
 1320 MHz ............................................................ OK
 1080 MHz ............................................................ OK
  888 MHz ............................................................ OK
  816 MHz ............................................................ OK
  720 MHz ............................................................ OK
  480 MHz ............................................................ OK

Overall result : PASSED




>
> Thanks for the tool and explanation. I will test the v3 with this one.
>
> >
> > Also, it will test each frequency, while random frequencies might skip
> > a few.
>
> I didn't count them but there was more than 2000lines, probability to miss
> a frequency is really low. Also changing from 500MHz to 1.8GHz could have
> trigger some out-of-spec issue.
>
> But just to be sure I will add an init phase before doing random stuff.
>
> >
> > > With this script and using stress-ng during several hours, I didn't
> > > see any issue. Moreover I have tested specifically the 1.8GHz on my
> > > Beelink GS1, max thermal 85°C is reached very quickly and then the
> > > SoC oscillates quickly between 1.5 and 1.8GHz. So i have added
> > > 1.6GHz and 1.7GHz my board now oscillate slower between 1.5GHz and
> > > 1.6GHz swapping every second and temperature is also morestable.
> > >
> > > I also test that that offlining CPU0 and doing DVFS on other CPUs
> > > works. As CPU regulator is only set for CPU0.
> > >
> > > But maybe it doesn't cost much to set the regulator for all the CPUs?
> > >
> > > Jernej test the GPU devfreq on several H6 board particulary the
> > > Tanix TX6 which doesn't have a proper dedicated PMIC and doesn't
> > > had any trouble with it.
> > >
> > > Do you think I can enable GPU OPP for all H6 Boards?
> >
> > It seems you're doing it?
> No I don't, see my answer on patch 4/7.
>
> Thanks for your review,
> Clément
>
> >
> > Maxime

Re: [PATCH v2 0/7] Add support for Allwinner H6 DVFS

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4718 bytes --]

Hi Clement,

On Mon, Apr 13, 2020 at 03:37:19PM +0200, Clément Péron wrote:
> On Mon, 6 Apr 2020 at 11:07, Clément Péron <peron.clem@gmail.com> wrote:
> > On Mon, 6 Apr 2020 at 10:10, Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > Hi,
> > >
> > > On Sun, Apr 05, 2020 at 07:35:54PM +0200, Clément Péron wrote:
> > > > Now that required drivers are merged we can contibute on DVFS
> > > > support for Allwinner H6.
> > > >
> > > > This serie is based on Yangtao Li serie[0] and Ondřej Jirman work[1].
> > > >
> > > > Most of the OPP tables are taken from original vendor kernel[2].
> > > > Plus there is a new CPU frequencies at 1.6GHz, 1.7GHz and 1.8GHz.
> > > >
> > > > I wrote a simple script to randomly set a frequency during
> > > > a random time[3].
> > >
> > > If you ever need to do that ever again, cpufreq-ljt-stress-test (found
> > > here https://github.com/ssvb/cpuburn-arm) has proven to be very
> > > reliable to detect cpufreq related issues. stress-ng might not be
> > > enough since the (at least older) Allwinner SoCs tend to create cache
> > > corruption when undervolted, and that might not be unnoticed by
> > > stress-ng but will be catched by cpufreq-ljt-stress-test.
>
> The test seems good (see result below) :
>
> # cpufreq-ljt-stress-test
> Creating './whitenoise-1920x1080.jpg' ... done
> CPU stress test, which is doing JPEG decoding by libjpeg-turbo
> at different cpufreq operating points.
>
> Testing CPU 0
>  1800 MHz ............................................................ OK
>  1704 MHz ............................................................ OK
>  1608 MHz ............................................................ OK
>  1488 MHz ............................................................ OK
>  1320 MHz ............................................................ OK
>  1080 MHz ............................................................ OK
>   888 MHz ............................................................ OK
>   816 MHz ............................................................ OK
>   720 MHz ............................................................ OK
>   480 MHz ............................................................ OK
>
> Testing CPU 1
>  1800 MHz ............................................................ OK
>  1704 MHz ............................................................ OK
>  1608 MHz ............................................................ OK
>  1488 MHz ............................................................ OK
>  1320 MHz ............................................................ OK
>  1080 MHz ............................................................ OK
>   888 MHz ............................................................ OK
>   816 MHz ............................................................ OK
>   720 MHz ............................................................ OK
>   480 MHz ............................................................ OK
>
> Testing CPU 2
>  1800 MHz ............................................................ OK
>  1704 MHz ............................................................ OK
>  1608 MHz ............................................................ OK
>  1488 MHz ............................................................ OK
>  1320 MHz ............................................................ OK
>  1080 MHz ............................................................ OK
>   888 MHz ............................................................ OK
>   816 MHz ............................................................ OK
>   720 MHz ............................................................ OK
>   480 MHz ............................................................ OK
>
> Testing CPU 3
>  1800 MHz ............................................................ OK
>  1704 MHz ............................................................ OK
>  1608 MHz ............................................................ OK
>  1488 MHz ............................................................ OK
>  1320 MHz ............................................................ OK
>  1080 MHz ............................................................ OK
>   888 MHz ............................................................ OK
>   816 MHz ............................................................ OK
>   720 MHz ............................................................ OK
>   480 MHz ............................................................ OK
>
> Overall result : PASSED

Awesome, thanks for taking the time to do it :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]