* [PATCH v5 1/8] arm64: dts: rockchip: add thermal zones information on RK3588
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 2/8] arm64: dts: rockchip: enable thermal management on all RK3588 boards Alexey Charkov
` (10 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
This includes the necessary device tree data to allow thermal
monitoring on RK3588(s) using the on-chip TSADC device, along with
trip points for automatic thermal management.
Each of the CPU clusters (one for the little cores and two for
the big cores) get a passive cooling trip point at 85C, which
will trigger DVFS throttling of the respective cluster upon
reaching a high temperature condition.
All zones also have a critical trip point at 115C, which will
trigger a reset.
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 147 ++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 629049f3dc16..574359279867 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -10,6 +10,7 @@
#include <dt-bindings/reset/rockchip,rk3588-cru.h>
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/ata/ahci.h>
+#include <dt-bindings/thermal/thermal.h>
/ {
compatible = "rockchip,rk3588";
@@ -2368,6 +2369,152 @@ pwm15: pwm@febf0030 {
status = "disabled";
};
+ thermal_zones: thermal-zones {
+ /* sensor near the center of the SoC */
+ package_thermal: package-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 0>;
+
+ trips {
+ package_crit: package-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ /* sensor between A76 cores 0 and 1 */
+ bigcore0_thermal: bigcore0-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 1>;
+
+ trips {
+ bigcore0_alert: bigcore0-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ bigcore0_crit: bigcore0-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ cooling-maps {
+ map0 {
+ trip = <&bigcore0_alert>;
+ cooling-device =
+ <&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu_b1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ /* sensor between A76 cores 2 and 3 */
+ bigcore2_thermal: bigcore2-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 2>;
+
+ trips {
+ bigcore2_alert: bigcore2-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ bigcore2_crit: bigcore2-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ cooling-maps {
+ map0 {
+ trip = <&bigcore2_alert>;
+ cooling-device =
+ <&cpu_b2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu_b3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ /* sensor between the four A55 cores */
+ little_core_thermal: littlecore-thermal {
+ polling-delay-passive = <100>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 3>;
+
+ trips {
+ littlecore_alert: littlecore-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ littlecore_crit: littlecore-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ cooling-maps {
+ map0 {
+ trip = <&littlecore_alert>;
+ cooling-device =
+ <&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu_l1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu_l2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu_l3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
+ };
+
+ /* sensor near the PD_CENTER power domain */
+ center_thermal: center-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 4>;
+
+ trips {
+ center_crit: center-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu_thermal: gpu-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 5>;
+
+ trips {
+ gpu_crit: gpu-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+
+ npu_thermal: npu-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <&tsadc 6>;
+
+ trips {
+ npu_crit: npu-crit {
+ temperature = <115000>;
+ hysteresis = <0>;
+ type = "critical";
+ };
+ };
+ };
+ };
+
tsadc: tsadc@fec00000 {
compatible = "rockchip,rk3588-tsadc";
reg = <0x0 0xfec00000 0x0 0x400>;
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 2/8] arm64: dts: rockchip: enable thermal management on all RK3588 boards
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 1/8] arm64: dts: rockchip: add thermal zones information on RK3588 Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 3/8] arm64: dts: rockchip: add passive GPU cooling on RK3588 Alexey Charkov
` (9 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
This enables the on-chip thermal monitoring sensor (TSADC) on all
RK3588(s) boards that don't have it enabled yet. It provides temperature
monitoring for the SoC and emergency thermal shutdowns, and is thus
important to have in place before CPU DVFS is enabled, as high CPU
operating performance points can overheat the chip quickly in the
absence of thermal management.
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 ++++
arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 ++++
8 files changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts b/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
index 98c622b27647..c667704ba985 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-armsom-sige7.dts
@@ -673,6 +673,10 @@ regulator-state-mem {
};
};
+&tsadc {
+ status = "okay";
+};
+
&u2phy0 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-common.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-common.dtsi
index 709d348cf06b..03fd193be253 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-common.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-edgeble-neu6a-common.dtsi
@@ -466,3 +466,7 @@ regulator-state-mem {
};
};
};
+
+&tsadc {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
index 7be2190244ba..7c3696a3ad3a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
@@ -1131,6 +1131,10 @@ &sata0 {
status = "okay";
};
+&tsadc {
+ status = "okay";
+};
+
&u2phy0 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts b/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
index 009566d881f3..230e630820b4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts
@@ -376,6 +376,10 @@ &sdmmc {
status = "okay";
};
+&tsadc {
+ status = "okay";
+};
+
&u2phy2 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index 4e2bf4eaef2b..afcc38a5bed8 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -749,6 +749,10 @@ regulator-state-mem {
};
};
+&tsadc {
+ status = "okay";
+};
+
&uart2 {
pinctrl-0 = <&uart2m0_xfer>;
status = "okay";
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts b/arch/arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts
index 9090c5c99f2a..d0021524e7f9 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts
@@ -648,6 +648,10 @@ regulator-state-mem {
};
};
+&tsadc {
+ status = "okay";
+};
+
&u2phy2 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
index 6b9206ce4a03..77bcf0f6b028 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
@@ -601,6 +601,10 @@ regulator-state-mem {
};
};
+&tsadc {
+ status = "okay";
+};
+
&uart2 {
pinctrl-0 = <&uart2m0_xfer>;
status = "okay";
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index 8e2a07612d17..c671a61d3aef 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -697,6 +697,10 @@ regulator-state-mem {
};
};
+&tsadc {
+ status = "okay";
+};
+
&u2phy0 {
status = "okay";
};
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 3/8] arm64: dts: rockchip: add passive GPU cooling on RK3588
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 1/8] arm64: dts: rockchip: add thermal zones information on RK3588 Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 2/8] arm64: dts: rockchip: enable thermal management on all RK3588 boards Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 4/8] arm64: dts: rockchip: enable automatic fan control on Rock 5B Alexey Charkov
` (8 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
As the GPU support on RK3588 has been merged upstream, along with OPP
values, add a corresponding cooling map for passive cooling using the GPU.
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 574359279867..758aff5e040b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -2487,17 +2487,29 @@ center_crit: center-crit {
};
gpu_thermal: gpu-thermal {
- polling-delay-passive = <0>;
+ polling-delay-passive = <100>;
polling-delay = <0>;
thermal-sensors = <&tsadc 5>;
trips {
+ gpu_alert: gpu-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
gpu_crit: gpu-crit {
temperature = <115000>;
hysteresis = <0>;
type = "critical";
};
};
+ cooling-maps {
+ map0 {
+ trip = <&gpu_alert>;
+ cooling-device =
+ <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+ };
+ };
};
npu_thermal: npu-thermal {
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 4/8] arm64: dts: rockchip: enable automatic fan control on Rock 5B
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (2 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 3/8] arm64: dts: rockchip: add passive GPU cooling on RK3588 Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 5/8] arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588 Alexey Charkov
` (7 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
This links the PWM fan on Radxa Rock 5B as an active cooling device
managed automatically by the thermal subsystem, with a target SoC
temperature of 65C and a minimum-spin interval from 55C to 65C to
ensure airflow when the system gets warm
Helped-by: Dragan Simic <dsimic@manjaro.org>
Reviewed-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 30 ++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
index afcc38a5bed8..27dd95f92f33 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
@@ -52,7 +52,7 @@ led_rgb_b {
fan: pwm-fan {
compatible = "pwm-fan";
- cooling-levels = <0 95 145 195 255>;
+ cooling-levels = <0 120 150 180 210 240 255>;
fan-supply = <&vcc5v0_sys>;
pwms = <&pwm1 0 50000 0>;
#cooling-cells = <2>;
@@ -286,6 +286,34 @@ i2s0_8ch_p0_0: endpoint {
};
};
+&package_thermal {
+ polling-delay = <1000>;
+
+ trips {
+ package_fan0: package-fan0 {
+ temperature = <55000>;
+ hysteresis = <2000>;
+ type = "active";
+ };
+ package_fan1: package-fan1 {
+ temperature = <65000>;
+ hysteresis = <2000>;
+ type = "active";
+ };
+ };
+
+ cooling-maps {
+ map1 {
+ trip = <&package_fan0>;
+ cooling-device = <&fan THERMAL_NO_LIMIT 1>;
+ };
+ map2 {
+ trip = <&package_fan1>;
+ cooling-device = <&fan 2 THERMAL_NO_LIMIT>;
+ };
+ };
+};
+
&pcie2x1l0 {
pinctrl-names = "default";
pinctrl-0 = <&pcie2_0_rst>;
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 5/8] arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (3 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 4/8] arm64: dts: rockchip: enable automatic fan control on Rock 5B Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 6/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588 Alexey Charkov
` (6 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
RK3588 chips allow for their CPU cores to be powered by a different
supply vs. their corresponding memory interfaces, and two of the
boards currently upstream do that (EVB1 and QuartzPro64).
The voltage of the memory interface though has to match that of the
CPU cores that use it, which downstream kernels achieve by the means
of a custom cpufreq driver which adjusts both at the same time.
It seems that regulator coupling is a more appropriate generic
interface for it, so this patch introduces coupling to affected
device trees to ensure that memory interface voltage is also updated
whenever cpufreq switches between CPU OPPs.
Note that other boards, such as Radxa Rock 5B, define both the CPU
and memory interface regulators as aliases to the same DT node, so
this doesn't apply there.
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 12 ++++++++++++
arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++++++++++++
2 files changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
index 7c3696a3ad3a..00f660d50127 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts
@@ -878,6 +878,8 @@ regulators {
vdd_cpu_big1_s0: dcdc-reg1 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big1_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -890,6 +892,8 @@ regulator-state-mem {
vdd_cpu_big0_s0: dcdc-reg2 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big0_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -902,6 +906,8 @@ regulator-state-mem {
vdd_cpu_lit_s0: dcdc-reg3 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_lit_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <950000>;
regulator-ramp-delay = <12500>;
@@ -926,6 +932,8 @@ regulator-state-mem {
vdd_cpu_big1_mem_s0: dcdc-reg5 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big1_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -939,6 +947,8 @@ regulator-state-mem {
vdd_cpu_big0_mem_s0: dcdc-reg6 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big0_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -963,6 +973,8 @@ regulator-state-mem {
vdd_cpu_lit_mem_s0: dcdc-reg8 {
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_lit_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <950000>;
regulator-ramp-delay = <12500>;
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
index b4f22d95ac0e..baeb08d665c7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-quartzpro64.dts
@@ -832,6 +832,8 @@ vdd_cpu_big1_s0: dcdc-reg1 {
regulator-name = "vdd_cpu_big1_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big1_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -845,6 +847,8 @@ vdd_cpu_big0_s0: dcdc-reg2 {
regulator-name = "vdd_cpu_big0_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big0_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -858,6 +862,8 @@ vdd_cpu_lit_s0: dcdc-reg3 {
regulator-name = "vdd_cpu_lit_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_lit_mem_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <550000>;
regulator-max-microvolt = <950000>;
regulator-ramp-delay = <12500>;
@@ -884,6 +890,8 @@ vdd_cpu_big1_mem_s0: dcdc-reg5 {
regulator-name = "vdd_cpu_big1_mem_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big1_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -898,6 +906,8 @@ vdd_cpu_big0_mem_s0: dcdc-reg6 {
regulator-name = "vdd_cpu_big0_mem_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_big0_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <1050000>;
regulator-ramp-delay = <12500>;
@@ -924,6 +934,8 @@ vdd_cpu_lit_mem_s0: dcdc-reg8 {
regulator-name = "vdd_cpu_lit_mem_s0";
regulator-always-on;
regulator-boot-on;
+ regulator-coupled-with = <&vdd_cpu_lit_s0>;
+ regulator-coupled-max-spread = <10000>;
regulator-min-microvolt = <675000>;
regulator-max-microvolt = <950000>;
regulator-ramp-delay = <12500>;
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 6/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (4 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 5/8] arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588 Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2024-06-17 18:28 ` [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j Alexey Charkov
` (5 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
By default the CPUs on RK3588 start up in a conservative performance
mode. Add frequency and voltage mappings to the device tree to enable
dynamic scaling via cpufreq.
OPP values are adapted from Radxa's downstream kernel for Rock 5B [1],
stripping them down to the minimum frequency and voltage combinations
as expected by the generic upstream cpufreq-dt driver, and also dropping
those OPPs that don't differ in voltage but only in frequency (keeping
the top frequency OPP in each case).
Note that this patch ignores voltage scaling for the CPU memory
interface which the downstream kernel does through a custom cpufreq
driver, and which is why the downstream version has two sets of voltage
values for each OPP (the second one being meant for the memory
interface supply regulator). This is done instead via regulator
coupling between CPU and memory interface supplies on affected boards.
This has been tested on Rock 5B with u-boot 2023.11 compiled from
Collabora's integration tree [2] with binary bl31 and appears to be
stable both under active cooling and passive cooling (with throttling)
[1] https://github.com/radxa/kernel/blob/stable-5.10-rock5/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
[2] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/u-boot
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 149 +++++++++++++++++++++++++++
arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
3 files changed, 151 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
new file mode 100644
index 000000000000..35bbc3c2134f
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/ {
+ cluster0_opp_table: opp-table-cluster0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1008000000 {
+ opp-hz = /bits/ 64 <1008000000>;
+ opp-microvolt = <675000 675000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <712500 712500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <762500 762500 950000>;
+ clock-latency-ns = <40000>;
+ opp-suspend;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <850000 850000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <950000 950000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+
+ cluster1_opp_table: opp-table-cluster1 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <675000 675000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <725000 725000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <762500 762500 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <850000 850000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <925000 925000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2208000000 {
+ opp-hz = /bits/ 64 <2208000000>;
+ opp-microvolt = <987500 987500 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2400000000 {
+ opp-hz = /bits/ 64 <2400000000>;
+ opp-microvolt = <1000000 1000000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+
+ cluster2_opp_table: opp-table-cluster2 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <675000 675000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <725000 725000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <762500 762500 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <850000 850000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <925000 925000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2208000000 {
+ opp-hz = /bits/ 64 <2208000000>;
+ opp-microvolt = <987500 987500 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2400000000 {
+ opp-hz = /bits/ 64 <2400000000>;
+ opp-microvolt = <1000000 1000000 1000000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+};
+
+&cpu_b0 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_b1 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_b2 {
+ operating-points-v2 = <&cluster2_opp_table>;
+};
+
+&cpu_b3 {
+ operating-points-v2 = <&cluster2_opp_table>;
+};
+
+&cpu_l0 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l1 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l2 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l3 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 0bbeee399a63..7462cc1e1007 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -5,3 +5,4 @@
*/
#include "rk3588-extra.dtsi"
+#include "rk3588-opp.dtsi"
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index a379269147c4..c7fecf8fe7ec 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -5,3 +5,4 @@
*/
#include "rk3588-base.dtsi"
+#include "rk3588-opp.dtsi"
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (5 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 6/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588 Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2025-02-11 16:32 ` Quentin Schulz
2024-06-17 18:28 ` [PATCH v5 8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j Alexey Charkov
` (4 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
RK3588j is the 'industrial' variant of RK3588, and it uses a different
set of OPPs both in terms of allowed frequencies and in terms of
applicable voltages at each frequency setpoint.
Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
enable dynamic CPU frequency scaling.
OPP values are derived from Rockchip downstream sources [1] by taking
only those OPPs which have the highest frequency for a given voltage
level and dropping the rest (if they are included, the kernel complains
at boot time about them being inefficient)
[1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
1 file changed, 108 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
index 0bbeee399a63..b7e69553857b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
@@ -5,3 +5,111 @@
*/
#include "rk3588-extra.dtsi"
+
+/ {
+ cluster0_opp_table: opp-table-cluster0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ opp-suspend;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <887500 887500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1704000000 {
+ opp-hz = /bits/ 64 <1704000000>;
+ opp-microvolt = <937500 937500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+
+ cluster1_opp_table: opp-table-cluster1 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <787500 787500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <875000 875000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <950000 950000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+
+ cluster2_opp_table: opp-table-cluster2 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <787500 787500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <875000 875000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <950000 950000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ };
+};
+
+&cpu_b0 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_b1 {
+ operating-points-v2 = <&cluster1_opp_table>;
+};
+
+&cpu_b2 {
+ operating-points-v2 = <&cluster2_opp_table>;
+};
+
+&cpu_b3 {
+ operating-points-v2 = <&cluster2_opp_table>;
+};
+
+&cpu_l0 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l1 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l2 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
+
+&cpu_l3 {
+ operating-points-v2 = <&cluster0_opp_table>;
+};
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2024-06-17 18:28 ` [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j Alexey Charkov
@ 2025-02-11 16:32 ` Quentin Schulz
2025-02-15 18:59 ` Alexey Charkov
0 siblings, 1 reply; 32+ messages in thread
From: Quentin Schulz @ 2025-02-11 16:32 UTC (permalink / raw)
To: Alexey Charkov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hi all,
On 6/17/24 8:28 PM, Alexey Charkov wrote:
> RK3588j is the 'industrial' variant of RK3588, and it uses a different
> set of OPPs both in terms of allowed frequencies and in terms of
> applicable voltages at each frequency setpoint.
>
> Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
> enable dynamic CPU frequency scaling.
>
> OPP values are derived from Rockchip downstream sources [1] by taking
> only those OPPs which have the highest frequency for a given voltage
> level and dropping the rest (if they are included, the kernel complains
> at boot time about them being inefficient)
>
> [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>
Funny stuff actually. Rockchip have their own downstream cpufreq driver
which autodetects at runtime the SoC variant and filter out OPPs based
on that info. And this patch is based on those values and filters.
However, they also decided that maybe this isn't the best way to do it
and they decided to have an rk3588j.dtsi where they remove some of those
OPPs instead of just updating the mask/filter in their base dtsi
(rk3588s.dtsi downstream). See
https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
So...
> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
> 1 file changed, 108 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> index 0bbeee399a63..b7e69553857b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> @@ -5,3 +5,111 @@
> */
>
> #include "rk3588-extra.dtsi"
> +
> +/ {
> + cluster0_opp_table: opp-table-cluster0 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-1416000000 {
> + opp-hz = /bits/ 64 <1416000000>;
> + opp-microvolt = <750000 750000 950000>;
> + clock-latency-ns = <40000>;
> + opp-suspend;
> + };
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <887500 887500 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-1704000000 {
> + opp-hz = /bits/ 64 <1704000000>;
> + opp-microvolt = <937500 937500 950000>;
> + clock-latency-ns = <40000>;
> + };
None of those are valid according to Rockchip, we should have
opp-1296000000 {
opp-hz = /bits/ 64 <1296000000>;
opp-microvolt = <750000 750000 950000>;
clock-latency-ns = <40000>;
opp-suspend;
};
instead?
and...
> + };
> +
> + cluster1_opp_table: opp-table-cluster1 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-1416000000 {
> + opp-hz = /bits/ 64 <1416000000>;
> + opp-microvolt = <750000 750000 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <787500 787500 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-1800000000 {
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <875000 875000 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <950000 950000 950000>;
> + clock-latency-ns = <40000>;
> + };
opp-1800000000 and opp-2016000000 should be removed.
and...
> + };
> +
> + cluster2_opp_table: opp-table-cluster2 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-1416000000 {
> + opp-hz = /bits/ 64 <1416000000>;
> + opp-microvolt = <750000 750000 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <787500 787500 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-1800000000 {
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <875000 875000 950000>;
> + clock-latency-ns = <40000>;
> + };
> + opp-2016000000 {
> + opp-hz = /bits/ 64 <2016000000>;
> + opp-microvolt = <950000 950000 950000>;
> + clock-latency-ns = <40000>;
> + };
opp-1800000000 and opp-2016000000 should be removed as well.
Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
for awareness on Rockchip side :)
I guess another option could be to mark those above as
turbo-mode;
though no clue what this would entail. From a glance at cpufreq, it
seems that for Rockchip since we use the default cpufreq-dt, it would
mark those as unusable, so essentially a no-op, so better remove them
entirely?
Cheers,
Quentin
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-11 16:32 ` Quentin Schulz
@ 2025-02-15 18:59 ` Alexey Charkov
2025-02-15 20:30 ` Heiko Stübner
0 siblings, 1 reply; 32+ messages in thread
From: Alexey Charkov @ 2025-02-15 18:59 UTC (permalink / raw)
To: Quentin Schulz
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hi Quentin,
On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi all,
>
> On 6/17/24 8:28 PM, Alexey Charkov wrote:
> > RK3588j is the 'industrial' variant of RK3588, and it uses a different
> > set of OPPs both in terms of allowed frequencies and in terms of
> > applicable voltages at each frequency setpoint.
> >
> > Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
> > enable dynamic CPU frequency scaling.
> >
> > OPP values are derived from Rockchip downstream sources [1] by taking
> > only those OPPs which have the highest frequency for a given voltage
> > level and dropping the rest (if they are included, the kernel complains
> > at boot time about them being inefficient)
> >
> > [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> >
>
> Funny stuff actually. Rockchip have their own downstream cpufreq driver
> which autodetects at runtime the SoC variant and filter out OPPs based
> on that info. And this patch is based on those values and filters.
>
> However, they also decided that maybe this isn't the best way to do it
> and they decided to have an rk3588j.dtsi where they remove some of those
> OPPs instead of just updating the mask/filter in their base dtsi
> (rk3588s.dtsi downstream). See
> https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
Funny stuff indeed! Judging by the comments in the file you
referenced, those higher OPP values constitute an 'overdrive' mode,
and apparently the chip shouldn't stay in those for prolonged periods
of time. However, I couldn't locate any dts file inside Rockchip
sources that would include this rk3588j.dtsi - so not sure if we
should follow what it says too zealously.
> So...
>
> > Signed-off-by: Alexey Charkov <alchark@gmail.com>
> > ---
> > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
> > 1 file changed, 108 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > index 0bbeee399a63..b7e69553857b 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > @@ -5,3 +5,111 @@
> > */
> >
> > #include "rk3588-extra.dtsi"
> > +
> > +/ {
> > + cluster0_opp_table: opp-table-cluster0 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-1416000000 {
> > + opp-hz = /bits/ 64 <1416000000>;
> > + opp-microvolt = <750000 750000 950000>;
> > + clock-latency-ns = <40000>;
> > + opp-suspend;
> > + };
> > + opp-1608000000 {
> > + opp-hz = /bits/ 64 <1608000000>;
> > + opp-microvolt = <887500 887500 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-1704000000 {
> > + opp-hz = /bits/ 64 <1704000000>;
> > + opp-microvolt = <937500 937500 950000>;
> > + clock-latency-ns = <40000>;
> > + };
>
> None of those are valid according to Rockchip, we should have
Well, valid but more taxing on the hardware apparently.
> opp-1296000000 {
> opp-hz = /bits/ 64 <1296000000>;
> opp-microvolt = <750000 750000 950000>;
> clock-latency-ns = <40000>;
> opp-suspend;
> };
>
> instead?
I dropped this one because it uses a lower frequency but same voltage
as the 1.416 GHz one, thus being 'inefficient' as the thermal
subsystem says in the logs. It can be added back if we decide to
remove opp-1416000000.
> and...
>
> > + };
> > +
> > + cluster1_opp_table: opp-table-cluster1 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-1416000000 {
> > + opp-hz = /bits/ 64 <1416000000>;
> > + opp-microvolt = <750000 750000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-1608000000 {
> > + opp-hz = /bits/ 64 <1608000000>;
> > + opp-microvolt = <787500 787500 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-1800000000 {
> > + opp-hz = /bits/ 64 <1800000000>;
> > + opp-microvolt = <875000 875000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-2016000000 {
> > + opp-hz = /bits/ 64 <2016000000>;
> > + opp-microvolt = <950000 950000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
>
> opp-1800000000 and opp-2016000000 should be removed.
>
> and...
>
> > + };
> > +
> > + cluster2_opp_table: opp-table-cluster2 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-1416000000 {
> > + opp-hz = /bits/ 64 <1416000000>;
> > + opp-microvolt = <750000 750000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-1608000000 {
> > + opp-hz = /bits/ 64 <1608000000>;
> > + opp-microvolt = <787500 787500 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-1800000000 {
> > + opp-hz = /bits/ 64 <1800000000>;
> > + opp-microvolt = <875000 875000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
> > + opp-2016000000 {
> > + opp-hz = /bits/ 64 <2016000000>;
> > + opp-microvolt = <950000 950000 950000>;
> > + clock-latency-ns = <40000>;
> > + };
>
> opp-1800000000 and opp-2016000000 should be removed as well.
>
> Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
> for awareness on Rockchip side :)
>
> I guess another option could be to mark those above as
>
> turbo-mode;
>
> though no clue what this would entail. From a glance at cpufreq, it
> seems that for Rockchip since we use the default cpufreq-dt, it would
> mark those as unusable, so essentially a no-op, so better remove them
> entirely?
I believe the opp core just marks 'turbo-mode' opps as
CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
cpufreq core. But then to actually use those boost frequencies I would
guess the governor needs to somehow know the power/thermal constraints
of the chip?.. Don't know where that is defined.
Best,
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-15 18:59 ` Alexey Charkov
@ 2025-02-15 20:30 ` Heiko Stübner
2025-02-15 21:26 ` Dragan Simic
2025-02-16 12:32 ` Alexey Charkov
0 siblings, 2 replies; 32+ messages in thread
From: Heiko Stübner @ 2025-02-15 20:30 UTC (permalink / raw)
To: Quentin Schulz, Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano,
Dragan Simic, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Kever Yang
Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
> > On 6/17/24 8:28 PM, Alexey Charkov wrote:
> > > RK3588j is the 'industrial' variant of RK3588, and it uses a different
> > > set of OPPs both in terms of allowed frequencies and in terms of
> > > applicable voltages at each frequency setpoint.
> > >
> > > Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
> > > enable dynamic CPU frequency scaling.
> > >
> > > OPP values are derived from Rockchip downstream sources [1] by taking
> > > only those OPPs which have the highest frequency for a given voltage
> > > level and dropping the rest (if they are included, the kernel complains
> > > at boot time about them being inefficient)
> > >
> > > [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > >
> >
> > Funny stuff actually. Rockchip have their own downstream cpufreq driver
> > which autodetects at runtime the SoC variant and filter out OPPs based
> > on that info. And this patch is based on those values and filters.
> >
> > However, they also decided that maybe this isn't the best way to do it
> > and they decided to have an rk3588j.dtsi where they remove some of those
> > OPPs instead of just updating the mask/filter in their base dtsi
> > (rk3588s.dtsi downstream). See
> > https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>
> Funny stuff indeed! Judging by the comments in the file you
> referenced, those higher OPP values constitute an 'overdrive' mode,
> and apparently the chip shouldn't stay in those for prolonged periods
> of time. However, I couldn't locate any dts file inside Rockchip
> sources that would include this rk3588j.dtsi - so not sure if we
> should follow what it says too zealously.
>
> > So...
> >
> > > Signed-off-by: Alexey Charkov <alchark@gmail.com>
> > > ---
> > > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
> > > 1 file changed, 108 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > index 0bbeee399a63..b7e69553857b 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > @@ -5,3 +5,111 @@
> > > */
> > >
> > > #include "rk3588-extra.dtsi"
> > > +
> > > +/ {
> > > + cluster0_opp_table: opp-table-cluster0 {
> > > + compatible = "operating-points-v2";
> > > + opp-shared;
> > > +
> > > + opp-1416000000 {
> > > + opp-hz = /bits/ 64 <1416000000>;
> > > + opp-microvolt = <750000 750000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + opp-suspend;
> > > + };
> > > + opp-1608000000 {
> > > + opp-hz = /bits/ 64 <1608000000>;
> > > + opp-microvolt = <887500 887500 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-1704000000 {
> > > + opp-hz = /bits/ 64 <1704000000>;
> > > + opp-microvolt = <937500 937500 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> >
> > None of those are valid according to Rockchip, we should have
>
> Well, valid but more taxing on the hardware apparently.
>
> > opp-1296000000 {
> > opp-hz = /bits/ 64 <1296000000>;
> > opp-microvolt = <750000 750000 950000>;
> > clock-latency-ns = <40000>;
> > opp-suspend;
> > };
> >
> > instead?
>
> I dropped this one because it uses a lower frequency but same voltage
> as the 1.416 GHz one, thus being 'inefficient' as the thermal
> subsystem says in the logs. It can be added back if we decide to
> remove opp-1416000000.
>
> > and...
> >
> > > + };
> > > +
> > > + cluster1_opp_table: opp-table-cluster1 {
> > > + compatible = "operating-points-v2";
> > > + opp-shared;
> > > +
> > > + opp-1416000000 {
> > > + opp-hz = /bits/ 64 <1416000000>;
> > > + opp-microvolt = <750000 750000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-1608000000 {
> > > + opp-hz = /bits/ 64 <1608000000>;
> > > + opp-microvolt = <787500 787500 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-1800000000 {
> > > + opp-hz = /bits/ 64 <1800000000>;
> > > + opp-microvolt = <875000 875000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-2016000000 {
> > > + opp-hz = /bits/ 64 <2016000000>;
> > > + opp-microvolt = <950000 950000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> >
> > opp-1800000000 and opp-2016000000 should be removed.
> >
> > and...
> >
> > > + };
> > > +
> > > + cluster2_opp_table: opp-table-cluster2 {
> > > + compatible = "operating-points-v2";
> > > + opp-shared;
> > > +
> > > + opp-1416000000 {
> > > + opp-hz = /bits/ 64 <1416000000>;
> > > + opp-microvolt = <750000 750000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-1608000000 {
> > > + opp-hz = /bits/ 64 <1608000000>;
> > > + opp-microvolt = <787500 787500 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-1800000000 {
> > > + opp-hz = /bits/ 64 <1800000000>;
> > > + opp-microvolt = <875000 875000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> > > + opp-2016000000 {
> > > + opp-hz = /bits/ 64 <2016000000>;
> > > + opp-microvolt = <950000 950000 950000>;
> > > + clock-latency-ns = <40000>;
> > > + };
> >
> > opp-1800000000 and opp-2016000000 should be removed as well.
> >
> > Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
> > for awareness on Rockchip side :)
> >
> > I guess another option could be to mark those above as
> >
> > turbo-mode;
> >
> > though no clue what this would entail. From a glance at cpufreq, it
> > seems that for Rockchip since we use the default cpufreq-dt, it would
> > mark those as unusable, so essentially a no-op, so better remove them
> > entirely?
>
> I believe the opp core just marks 'turbo-mode' opps as
> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
> cpufreq core. But then to actually use those boost frequencies I would
> guess the governor needs to somehow know the power/thermal constraints
> of the chip?.. Don't know where that is defined.
personally I don't believe in "boost" frequencies - except when they are
declared officially.
Rockchip for the longest time maintains that running the chip outside
the declared power/rate envelope can reduce its overall lifetime.
So for Rockchip in mainline a "it runs stable for me" has never been a
suitable reasoning ;-) .
So while the situation might be strange for the rk3588j, I really only want
correct frequencies here please - not any pseudo "turbo freqs".
I don't care that much what people do on their own device{s/trees}, but
the devicetrees we supply centrally (and to u-boot, etc) should only
contain frequencies vetted by the manufacturer.
Heiko
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-15 20:30 ` Heiko Stübner
@ 2025-02-15 21:26 ` Dragan Simic
2025-02-16 12:32 ` Alexey Charkov
1 sibling, 0 replies; 32+ messages in thread
From: Dragan Simic @ 2025-02-15 21:26 UTC (permalink / raw)
To: Heiko Stübner
Cc: Quentin Schulz, Alexey Charkov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Daniel Lezcano, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hello all,
On 2025-02-15 21:30, Heiko Stübner wrote:
> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz
>> <quentin.schulz@cherry.de> wrote:
>> > On 6/17/24 8:28 PM, Alexey Charkov wrote:
>> > > RK3588j is the 'industrial' variant of RK3588, and it uses a different
>> > > set of OPPs both in terms of allowed frequencies and in terms of
>> > > applicable voltages at each frequency setpoint.
>> > >
>> > > Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
>> > > enable dynamic CPU frequency scaling.
>> > >
>> > > OPP values are derived from Rockchip downstream sources [1] by taking
>> > > only those OPPs which have the highest frequency for a given voltage
>> > > level and dropping the rest (if they are included, the kernel complains
>> > > at boot time about them being inefficient)
>> > >
>> > > [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>> > >
>> >
>> > Funny stuff actually. Rockchip have their own downstream cpufreq driver
>> > which autodetects at runtime the SoC variant and filter out OPPs based
>> > on that info. And this patch is based on those values and filters.
>> >
>> > However, they also decided that maybe this isn't the best way to do it
>> > and they decided to have an rk3588j.dtsi where they remove some of those
>> > OPPs instead of just updating the mask/filter in their base dtsi
>> > (rk3588s.dtsi downstream). See
>> > https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>
>> Funny stuff indeed! Judging by the comments in the file you
>> referenced, those higher OPP values constitute an 'overdrive' mode,
>> and apparently the chip shouldn't stay in those for prolonged periods
>> of time. However, I couldn't locate any dts file inside Rockchip
>> sources that would include this rk3588j.dtsi - so not sure if we
>> should follow what it says too zealously.
>>
>> > So...
>> >
>> > > Signed-off-by: Alexey Charkov <alchark@gmail.com>
>> > > ---
>> > > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
>> > > 1 file changed, 108 insertions(+)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>> > > index 0bbeee399a63..b7e69553857b 100644
>> > > --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>> > > @@ -5,3 +5,111 @@
>> > > */
>> > >
>> > > #include "rk3588-extra.dtsi"
>> > > +
>> > > +/ {
>> > > + cluster0_opp_table: opp-table-cluster0 {
>> > > + compatible = "operating-points-v2";
>> > > + opp-shared;
>> > > +
>> > > + opp-1416000000 {
>> > > + opp-hz = /bits/ 64 <1416000000>;
>> > > + opp-microvolt = <750000 750000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + opp-suspend;
>> > > + };
>> > > + opp-1608000000 {
>> > > + opp-hz = /bits/ 64 <1608000000>;
>> > > + opp-microvolt = <887500 887500 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-1704000000 {
>> > > + opp-hz = /bits/ 64 <1704000000>;
>> > > + opp-microvolt = <937500 937500 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> >
>> > None of those are valid according to Rockchip, we should have
>>
>> Well, valid but more taxing on the hardware apparently.
>>
>> > opp-1296000000 {
>> > opp-hz = /bits/ 64 <1296000000>;
>> > opp-microvolt = <750000 750000 950000>;
>> > clock-latency-ns = <40000>;
>> > opp-suspend;
>> > };
>> >
>> > instead?
>>
>> I dropped this one because it uses a lower frequency but same voltage
>> as the 1.416 GHz one, thus being 'inefficient' as the thermal
>> subsystem says in the logs. It can be added back if we decide to
>> remove opp-1416000000.
>>
>> > and...
>> >
>> > > + };
>> > > +
>> > > + cluster1_opp_table: opp-table-cluster1 {
>> > > + compatible = "operating-points-v2";
>> > > + opp-shared;
>> > > +
>> > > + opp-1416000000 {
>> > > + opp-hz = /bits/ 64 <1416000000>;
>> > > + opp-microvolt = <750000 750000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-1608000000 {
>> > > + opp-hz = /bits/ 64 <1608000000>;
>> > > + opp-microvolt = <787500 787500 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-1800000000 {
>> > > + opp-hz = /bits/ 64 <1800000000>;
>> > > + opp-microvolt = <875000 875000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-2016000000 {
>> > > + opp-hz = /bits/ 64 <2016000000>;
>> > > + opp-microvolt = <950000 950000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> >
>> > opp-1800000000 and opp-2016000000 should be removed.
>> >
>> > and...
>> >
>> > > + };
>> > > +
>> > > + cluster2_opp_table: opp-table-cluster2 {
>> > > + compatible = "operating-points-v2";
>> > > + opp-shared;
>> > > +
>> > > + opp-1416000000 {
>> > > + opp-hz = /bits/ 64 <1416000000>;
>> > > + opp-microvolt = <750000 750000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-1608000000 {
>> > > + opp-hz = /bits/ 64 <1608000000>;
>> > > + opp-microvolt = <787500 787500 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-1800000000 {
>> > > + opp-hz = /bits/ 64 <1800000000>;
>> > > + opp-microvolt = <875000 875000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> > > + opp-2016000000 {
>> > > + opp-hz = /bits/ 64 <2016000000>;
>> > > + opp-microvolt = <950000 950000 950000>;
>> > > + clock-latency-ns = <40000>;
>> > > + };
>> >
>> > opp-1800000000 and opp-2016000000 should be removed as well.
>> >
>> > Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
>> > for awareness on Rockchip side :)
>> >
>> > I guess another option could be to mark those above as
>> >
>> > turbo-mode;
>> >
>> > though no clue what this would entail. From a glance at cpufreq, it
>> > seems that for Rockchip since we use the default cpufreq-dt, it would
>> > mark those as unusable, so essentially a no-op, so better remove them
>> > entirely?
>>
>> I believe the opp core just marks 'turbo-mode' opps as
>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
>> cpufreq core. But then to actually use those boost frequencies I would
>> guess the governor needs to somehow know the power/thermal constraints
>> of the chip?.. Don't know where that is defined.
>
> personally I don't believe in "boost" frequencies - except when they
> are
> declared officially.
>
> Rockchip for the longest time maintains that running the chip outside
> the declared power/rate envelope can reduce its overall lifetime.
FWIW, as an additional data point, some other SoC manufacturers
officially state that the expected lifetime of their chips gets
reduced when they are run at higher temperatures, which are still
within the permitted temperature ranges.
> So for Rockchip in mainline a "it runs stable for me" has never been a
> suitable reasoning ;-) .
>
> So while the situation might be strange for the rk3588j, I really only
> want
> correct frequencies here please - not any pseudo "turbo freqs".
>
> I don't care that much what people do on their own device{s/trees}, but
> the devicetrees we supply centrally (and to u-boot, etc) should only
> contain frequencies vetted by the manufacturer.
Totally agreed, "works for me" is simply not applicable here. As we
know, some samples of the same CPU or SoC may be luckier when it comes
to the silicon lottery that got the binned, meaning that only the most
conservative frequencies and voltages, as assigned by the manufacturer,
are what we can provide as the OPPs.
I'm having a more detailed look into this, and I'll come back with,
hopefully, some additional comments. :)
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-15 20:30 ` Heiko Stübner
2025-02-15 21:26 ` Dragan Simic
@ 2025-02-16 12:32 ` Alexey Charkov
2025-02-17 16:24 ` Quentin Schulz
2025-03-12 10:15 ` Quentin Schulz
1 sibling, 2 replies; 32+ messages in thread
From: Alexey Charkov @ 2025-02-16 12:32 UTC (permalink / raw)
To: Heiko Stübner
Cc: Quentin Schulz, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hi Heiko,
On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
> > On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
> > > On 6/17/24 8:28 PM, Alexey Charkov wrote:
> > > > RK3588j is the 'industrial' variant of RK3588, and it uses a different
> > > > set of OPPs both in terms of allowed frequencies and in terms of
> > > > applicable voltages at each frequency setpoint.
> > > >
> > > > Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
> > > > enable dynamic CPU frequency scaling.
> > > >
> > > > OPP values are derived from Rockchip downstream sources [1] by taking
> > > > only those OPPs which have the highest frequency for a given voltage
> > > > level and dropping the rest (if they are included, the kernel complains
> > > > at boot time about them being inefficient)
> > > >
> > > > [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> > > >
> > >
> > > Funny stuff actually. Rockchip have their own downstream cpufreq driver
> > > which autodetects at runtime the SoC variant and filter out OPPs based
> > > on that info. And this patch is based on those values and filters.
> > >
> > > However, they also decided that maybe this isn't the best way to do it
> > > and they decided to have an rk3588j.dtsi where they remove some of those
> > > OPPs instead of just updating the mask/filter in their base dtsi
> > > (rk3588s.dtsi downstream). See
> > > https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> >
> > Funny stuff indeed! Judging by the comments in the file you
> > referenced, those higher OPP values constitute an 'overdrive' mode,
> > and apparently the chip shouldn't stay in those for prolonged periods
> > of time. However, I couldn't locate any dts file inside Rockchip
> > sources that would include this rk3588j.dtsi - so not sure if we
> > should follow what it says too zealously.
> >
> > > So...
> > >
> > > > Signed-off-by: Alexey Charkov <alchark@gmail.com>
> > > > ---
> > > > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
> > > > 1 file changed, 108 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > > index 0bbeee399a63..b7e69553857b 100644
> > > > --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> > > > @@ -5,3 +5,111 @@
> > > > */
> > > >
> > > > #include "rk3588-extra.dtsi"
> > > > +
> > > > +/ {
> > > > + cluster0_opp_table: opp-table-cluster0 {
> > > > + compatible = "operating-points-v2";
> > > > + opp-shared;
> > > > +
> > > > + opp-1416000000 {
> > > > + opp-hz = /bits/ 64 <1416000000>;
> > > > + opp-microvolt = <750000 750000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + opp-suspend;
> > > > + };
> > > > + opp-1608000000 {
> > > > + opp-hz = /bits/ 64 <1608000000>;
> > > > + opp-microvolt = <887500 887500 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-1704000000 {
> > > > + opp-hz = /bits/ 64 <1704000000>;
> > > > + opp-microvolt = <937500 937500 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > >
> > > None of those are valid according to Rockchip, we should have
> >
> > Well, valid but more taxing on the hardware apparently.
> >
> > > opp-1296000000 {
> > > opp-hz = /bits/ 64 <1296000000>;
> > > opp-microvolt = <750000 750000 950000>;
> > > clock-latency-ns = <40000>;
> > > opp-suspend;
> > > };
> > >
> > > instead?
> >
> > I dropped this one because it uses a lower frequency but same voltage
> > as the 1.416 GHz one, thus being 'inefficient' as the thermal
> > subsystem says in the logs. It can be added back if we decide to
> > remove opp-1416000000.
> >
> > > and...
> > >
> > > > + };
> > > > +
> > > > + cluster1_opp_table: opp-table-cluster1 {
> > > > + compatible = "operating-points-v2";
> > > > + opp-shared;
> > > > +
> > > > + opp-1416000000 {
> > > > + opp-hz = /bits/ 64 <1416000000>;
> > > > + opp-microvolt = <750000 750000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-1608000000 {
> > > > + opp-hz = /bits/ 64 <1608000000>;
> > > > + opp-microvolt = <787500 787500 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-1800000000 {
> > > > + opp-hz = /bits/ 64 <1800000000>;
> > > > + opp-microvolt = <875000 875000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-2016000000 {
> > > > + opp-hz = /bits/ 64 <2016000000>;
> > > > + opp-microvolt = <950000 950000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > >
> > > opp-1800000000 and opp-2016000000 should be removed.
> > >
> > > and...
> > >
> > > > + };
> > > > +
> > > > + cluster2_opp_table: opp-table-cluster2 {
> > > > + compatible = "operating-points-v2";
> > > > + opp-shared;
> > > > +
> > > > + opp-1416000000 {
> > > > + opp-hz = /bits/ 64 <1416000000>;
> > > > + opp-microvolt = <750000 750000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-1608000000 {
> > > > + opp-hz = /bits/ 64 <1608000000>;
> > > > + opp-microvolt = <787500 787500 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-1800000000 {
> > > > + opp-hz = /bits/ 64 <1800000000>;
> > > > + opp-microvolt = <875000 875000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > > > + opp-2016000000 {
> > > > + opp-hz = /bits/ 64 <2016000000>;
> > > > + opp-microvolt = <950000 950000 950000>;
> > > > + clock-latency-ns = <40000>;
> > > > + };
> > >
> > > opp-1800000000 and opp-2016000000 should be removed as well.
> > >
> > > Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
> > > for awareness on Rockchip side :)
> > >
> > > I guess another option could be to mark those above as
> > >
> > > turbo-mode;
> > >
> > > though no clue what this would entail. From a glance at cpufreq, it
> > > seems that for Rockchip since we use the default cpufreq-dt, it would
> > > mark those as unusable, so essentially a no-op, so better remove them
> > > entirely?
> >
> > I believe the opp core just marks 'turbo-mode' opps as
> > CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
> > cpufreq core. But then to actually use those boost frequencies I would
> > guess the governor needs to somehow know the power/thermal constraints
> > of the chip?.. Don't know where that is defined.
>
> personally I don't believe in "boost" frequencies - except when they are
> declared officially.
>
> Rockchip for the longest time maintains that running the chip outside
> the declared power/rate envelope can reduce its overall lifetime.
>
> So for Rockchip in mainline a "it runs stable for me" has never been a
> suitable reasoning ;-) .
My key concern here was perhaps that we are looking at a file inside
the Rockchip source tree which looks relevant by the name of it, but
doesn't actually get included anywhere for any of the boards. This may
or may not constitute an endorsement by Rockchip of any OPPs listed
there :-D
I haven't seen a TRM for the J variant, nor do I have a board with
RK3588J to run tests, so it would be great if Kever could chime in
with how these OPPs are different from the others (or not).
> So while the situation might be strange for the rk3588j, I really only want
> correct frequencies here please - not any pseudo "turbo freqs".
>
> I don't care that much what people do on their own device{s/trees}, but
> the devicetrees we supply centrally (and to u-boot, etc) should only
> contain frequencies vetted by the manufacturer.
Fair enough. Let's just try and get another data point on whether
these frequencies are vetted or not.
Best regards,
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-16 12:32 ` Alexey Charkov
@ 2025-02-17 16:24 ` Quentin Schulz
2025-03-12 10:15 ` Quentin Schulz
1 sibling, 0 replies; 32+ messages in thread
From: Quentin Schulz @ 2025-02-17 16:24 UTC (permalink / raw)
To: Alexey Charkov, Heiko Stübner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano,
Dragan Simic, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Kever Yang
Hi all,
On 2/16/25 1:32 PM, Alexey Charkov wrote:
> Hi Heiko,
>
> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de> wrote:
>>
>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
>>>>> RK3588j is the 'industrial' variant of RK3588, and it uses a different
>>>>> set of OPPs both in terms of allowed frequencies and in terms of
>>>>> applicable voltages at each frequency setpoint.
>>>>>
>>>>> Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
>>>>> enable dynamic CPU frequency scaling.
>>>>>
>>>>> OPP values are derived from Rockchip downstream sources [1] by taking
>>>>> only those OPPs which have the highest frequency for a given voltage
>>>>> level and dropping the rest (if they are included, the kernel complains
>>>>> at boot time about them being inefficient)
>>>>>
>>>>> [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>>>>>
>>>>
>>>> Funny stuff actually. Rockchip have their own downstream cpufreq driver
>>>> which autodetects at runtime the SoC variant and filter out OPPs based
>>>> on that info. And this patch is based on those values and filters.
>>>>
>>>> However, they also decided that maybe this isn't the best way to do it
>>>> and they decided to have an rk3588j.dtsi where they remove some of those
>>>> OPPs instead of just updating the mask/filter in their base dtsi
>>>> (rk3588s.dtsi downstream). See
>>>> https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>
>>> Funny stuff indeed! Judging by the comments in the file you
>>> referenced, those higher OPP values constitute an 'overdrive' mode,
>>> and apparently the chip shouldn't stay in those for prolonged periods
>>> of time. However, I couldn't locate any dts file inside Rockchip
>>> sources that would include this rk3588j.dtsi - so not sure if we
>>> should follow what it says too zealously.
>>>
That was a clear oversight on my side.
The commit adding support for the J/M OPPs in rk3588s.dtsi downstream
just precedes the one adding the removal of the J OPPs in rk3588j.dtsi,
so not sure what was the intended effect there. I'll open a ticket with
them to see if I can get some answer until/unless Kever chimes in.
>>>> So...
>>>>
>>>>> Signed-off-by: Alexey Charkov <alchark@gmail.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
>>>>> 1 file changed, 108 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> index 0bbeee399a63..b7e69553857b 100644
>>>>> --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
>>>>> @@ -5,3 +5,111 @@
>>>>> */
>>>>>
>>>>> #include "rk3588-extra.dtsi"
>>>>> +
>>>>> +/ {
>>>>> + cluster0_opp_table: opp-table-cluster0 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + opp-suspend;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <887500 887500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1704000000 {
>>>>> + opp-hz = /bits/ 64 <1704000000>;
>>>>> + opp-microvolt = <937500 937500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> None of those are valid according to Rockchip, we should have
>>>
>>> Well, valid but more taxing on the hardware apparently.
>>>
>>>> opp-1296000000 {
>>>> opp-hz = /bits/ 64 <1296000000>;
>>>> opp-microvolt = <750000 750000 950000>;
>>>> clock-latency-ns = <40000>;
>>>> opp-suspend;
>>>> };
>>>>
>>>> instead?
>>>
>>> I dropped this one because it uses a lower frequency but same voltage
>>> as the 1.416 GHz one, thus being 'inefficient' as the thermal
>>> subsystem says in the logs. It can be added back if we decide to
>>> remove opp-1416000000.
That was exactly my point, the 1.296GHz OPP would then be the highest
frequency at that voltage.
>>>
>>>> and...
>>>>
>>>>> + };
>>>>> +
>>>>> + cluster1_opp_table: opp-table-cluster1 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1800000000 {
>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-2016000000 {
>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> opp-1800000000 and opp-2016000000 should be removed.
>>>>
>>>> and...
>>>>
>>>>> + };
>>>>> +
>>>>> + cluster2_opp_table: opp-table-cluster2 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1800000000 {
>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-2016000000 {
>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>>>>
>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
>>>> for awareness on Rockchip side :)
>>>>
>>>> I guess another option could be to mark those above as
>>>>
>>>> turbo-mode;
>>>>
>>>> though no clue what this would entail. From a glance at cpufreq, it
>>>> seems that for Rockchip since we use the default cpufreq-dt, it would
>>>> mark those as unusable, so essentially a no-op, so better remove them
>>>> entirely?
>>>
>>> I believe the opp core just marks 'turbo-mode' opps as
>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
>>> cpufreq core. But then to actually use those boost frequencies I would
>>> guess the governor needs to somehow know the power/thermal constraints
>>> of the chip?.. Don't know where that is defined.
>>
>> personally I don't believe in "boost" frequencies - except when they are
>> declared officially.
>>
>> Rockchip for the longest time maintains that running the chip outside
>> the declared power/rate envelope can reduce its overall lifetime.
>>
>> So for Rockchip in mainline a "it runs stable for me" has never been a
>> suitable reasoning ;-) .
>
> My key concern here was perhaps that we are looking at a file inside
> the Rockchip source tree which looks relevant by the name of it, but
> doesn't actually get included anywhere for any of the boards. This may
> or may not constitute an endorsement by Rockchip of any OPPs listed
> there :-D
>
Will ask for confirmation through their ticket system.
> I haven't seen a TRM for the J variant, nor do I have a board with
> RK3588J to run tests, so it would be great if Kever could chime in
> with how these OPPs are different from the others (or not).
>
I do have access to some J variant product but that won't help if you
need to run the boards for months/years at the highest freq in a
temperature-controlled chamber at the highest supported temperature for
the SoC. I don't think there's a TRM for the J variant, that wouldn't
make sense as it's the exact same SoC, just a different binning.
Similarly to what was done for RK3588S, RK3588S2, RK3582, RK3583, etc..
I don't think there's a TRM for those, but there's a datasheet, c.f.
https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
but that says 1.6GHz for the A76 cores and 1.3GHz for the A55 while the
the j-m OPP in rk3558s.dtsi downstream list the highest as 1.7GHz (but
rk3588j.dtsi removes it).
Will report if Rockchip has answered on their ticket system.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-02-16 12:32 ` Alexey Charkov
2025-02-17 16:24 ` Quentin Schulz
@ 2025-03-12 10:15 ` Quentin Schulz
2025-03-12 10:34 ` Dragan Simic
1 sibling, 1 reply; 32+ messages in thread
From: Quentin Schulz @ 2025-03-12 10:15 UTC (permalink / raw)
To: Alexey Charkov, Heiko Stübner
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano,
Dragan Simic, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Kever Yang
Hi all,
On 2/16/25 1:32 PM, Alexey Charkov wrote:
> Hi Heiko,
>
> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de> wrote:
>>
>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
[...]
>>>>> + };
>>>>> +
>>>>> + cluster2_opp_table: opp-table-cluster2 {
>>>>> + compatible = "operating-points-v2";
>>>>> + opp-shared;
>>>>> +
>>>>> + opp-1416000000 {
>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1608000000 {
>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-1800000000 {
>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>> + opp-2016000000 {
>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>> + clock-latency-ns = <40000>;
>>>>> + };
>>>>
>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>>>>
>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
>>>> for awareness on Rockchip side :)
>>>>
>>>> I guess another option could be to mark those above as
>>>>
>>>> turbo-mode;
>>>>
>>>> though no clue what this would entail. From a glance at cpufreq, it
>>>> seems that for Rockchip since we use the default cpufreq-dt, it would
>>>> mark those as unusable, so essentially a no-op, so better remove them
>>>> entirely?
>>>
>>> I believe the opp core just marks 'turbo-mode' opps as
>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to the
>>> cpufreq core. But then to actually use those boost frequencies I would
>>> guess the governor needs to somehow know the power/thermal constraints
>>> of the chip?.. Don't know where that is defined.
>>
>> personally I don't believe in "boost" frequencies - except when they are
>> declared officially.
>>
>> Rockchip for the longest time maintains that running the chip outside
>> the declared power/rate envelope can reduce its overall lifetime.
>>
>> So for Rockchip in mainline a "it runs stable for me" has never been a
>> suitable reasoning ;-) .
>
> My key concern here was perhaps that we are looking at a file inside
> the Rockchip source tree which looks relevant by the name of it, but
> doesn't actually get included anywhere for any of the boards. This may
> or may not constitute an endorsement by Rockchip of any OPPs listed
> there :-D
>
Rockchip support stated:
"""
If you run overdrive mode, you do not need to include rk3588j.dtsi, and
you can change it to #incldue rk3588.dtsi to ensure that the maximum
frequency can reach 2GHz
below picture from datasheet.
"""
The picture is the table 3-2 Recommended operating conditions, page 30
from the RK3588J datasheet, e.g.
https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
What is overdrive?
"""
Overdrive mode brings higher frequency, and the voltage will increase
accordingly. Under
the overdrive mode for a long time, the chipset may shorten the
lifetime, especially in
high temperature condition
"""
according to the same datasheet, end of the same table, page 31.
so this seems like a turbo-mode frequency to me.
Additionally, the note for the "normal mode" (the one with the lower
freqs) states:
"""
Normal mode means the chipset works under safety voltage and frequency.
For the
industrial environment, highly recommend to keep in normal mode, the
lifetime is
reasonably guaranteed.
"""
I believe "industrial environment" means RK3588J used in the
temperatures that aren't OK for the standard RK3588 variant?
> I haven't seen a TRM for the J variant, nor do I have a board with
> RK3588J to run tests, so it would be great if Kever could chime in
> with how these OPPs are different from the others (or not).
>
>> So while the situation might be strange for the rk3588j, I really only want
>> correct frequencies here please - not any pseudo "turbo freqs".
>>
>> I don't care that much what people do on their own device{s/trees}, but
>> the devicetrees we supply centrally (and to u-boot, etc) should only
>> contain frequencies vetted by the manufacturer.
>
> Fair enough. Let's just try and get another data point on whether
> these frequencies are vetted or not.
>
So the higher freqs seems to be vetted (and used by default on
Rockchip's BSP kernel), it just feels like you aren't really supposed to
run those higher frequencies all the time? And especially not in
"industrial environment"?
I would assume we want to stay on the safer side and remove those higher
frequencies? Heiko?
Cheers,
Quentin
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-03-12 10:15 ` Quentin Schulz
@ 2025-03-12 10:34 ` Dragan Simic
2025-03-13 10:42 ` Dragan Simic
0 siblings, 1 reply; 32+ messages in thread
From: Dragan Simic @ 2025-03-12 10:34 UTC (permalink / raw)
To: Quentin Schulz
Cc: Alexey Charkov, Heiko Stübner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano, Viresh Kumar,
Chen-Yu Tsai, Diederik de Haas, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, Kever Yang
Hello Quentin,
On 2025-03-12 11:15, Quentin Schulz wrote:
> On 2/16/25 1:32 PM, Alexey Charkov wrote:
>> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de>
>> wrote:
>>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz
>>>> <quentin.schulz@cherry.de> wrote:
>>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
> [...]
>>>>>> + };
>>>>>> +
>>>>>> + cluster2_opp_table: opp-table-cluster2 {
>>>>>> + compatible = "operating-points-v2";
>>>>>> + opp-shared;
>>>>>> +
>>>>>> + opp-1416000000 {
>>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>>> + clock-latency-ns = <40000>;
>>>>>> + };
>>>>>> + opp-1608000000 {
>>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>>> + clock-latency-ns = <40000>;
>>>>>> + };
>>>>>> + opp-1800000000 {
>>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>>> + clock-latency-ns = <40000>;
>>>>>> + };
>>>>>> + opp-2016000000 {
>>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>>> + clock-latency-ns = <40000>;
>>>>>> + };
>>>>>
>>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>>>>>
>>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at
>>>>> least
>>>>> for awareness on Rockchip side :)
>>>>>
>>>>> I guess another option could be to mark those above as
>>>>>
>>>>> turbo-mode;
>>>>>
>>>>> though no clue what this would entail. From a glance at cpufreq, it
>>>>> seems that for Rockchip since we use the default cpufreq-dt, it
>>>>> would
>>>>> mark those as unusable, so essentially a no-op, so better remove
>>>>> them
>>>>> entirely?
>>>>
>>>> I believe the opp core just marks 'turbo-mode' opps as
>>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to
>>>> the
>>>> cpufreq core. But then to actually use those boost frequencies I
>>>> would
>>>> guess the governor needs to somehow know the power/thermal
>>>> constraints
>>>> of the chip?.. Don't know where that is defined.
>>>
>>> personally I don't believe in "boost" frequencies - except when they
>>> are
>>> declared officially.
>>>
>>> Rockchip for the longest time maintains that running the chip outside
>>> the declared power/rate envelope can reduce its overall lifetime.
>>>
>>> So for Rockchip in mainline a "it runs stable for me" has never been
>>> a
>>> suitable reasoning ;-) .
>>
>> My key concern here was perhaps that we are looking at a file inside
>> the Rockchip source tree which looks relevant by the name of it, but
>> doesn't actually get included anywhere for any of the boards. This may
>> or may not constitute an endorsement by Rockchip of any OPPs listed
>> there :-D
>
> Rockchip support stated:
>
> """
> If you run overdrive mode, you do not need to include rk3588j.dtsi,
> and you can change it to #incldue rk3588.dtsi to ensure that the
> maximum frequency can reach 2GHz
>
> below picture from datasheet.
> """
>
> The picture is the table 3-2 Recommended operating conditions, page 30
> from the RK3588J datasheet, e.g.
> https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
>
> What is overdrive?
>
> """
> Overdrive mode brings higher frequency, and the voltage will increase
> accordingly. Under
> the overdrive mode for a long time, the chipset may shorten the
> lifetime, especially in high temperature condition
> """
>
> according to the same datasheet, end of the same table, page 31.
>
> so this seems like a turbo-mode frequency to me.
>
> Additionally, the note for the "normal mode" (the one with the lower
> freqs) states:
>
> """
> Normal mode means the chipset works under safety voltage and frequency.
> For the
> industrial environment, highly recommend to keep in normal mode, the
> lifetime is
> reasonably guaranteed.
> """
>
> I believe "industrial environment" means RK3588J used in the
> temperatures that aren't OK for the standard RK3588 variant?
Thanks a lot for obtaining these clarifications!
Yes, I'd say that in this case "industrial environment" boils down
to the extended temperature range for the RK3588J variant.
>> I haven't seen a TRM for the J variant, nor do I have a board with
>> RK3588J to run tests, so it would be great if Kever could chime in
>> with how these OPPs are different from the others (or not).
>>
>>> So while the situation might be strange for the rk3588j, I really
>>> only want
>>> correct frequencies here please - not any pseudo "turbo freqs".
>>>
>>> I don't care that much what people do on their own device{s/trees},
>>> but
>>> the devicetrees we supply centrally (and to u-boot, etc) should only
>>> contain frequencies vetted by the manufacturer.
>>
>> Fair enough. Let's just try and get another data point on whether
>> these frequencies are vetted or not.
>
> So the higher freqs seems to be vetted (and used by default on
> Rockchip's BSP kernel), it just feels like you aren't really supposed
> to run those higher frequencies all the time? And especially not in
> "industrial environment"?
>
> I would assume we want to stay on the safer side and remove those
> higher frequencies? Heiko?
I agree, we should remove the higher-frequency OPPs. I'd like
to go through everything once again in detail and prepare a patch
that removes the unsafe OPPs, and you could review it once it's
on the ML, to make sure it's fine.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-03-12 10:34 ` Dragan Simic
@ 2025-03-13 10:42 ` Dragan Simic
2025-03-13 19:00 ` Heiko Stuebner
0 siblings, 1 reply; 32+ messages in thread
From: Dragan Simic @ 2025-03-13 10:42 UTC (permalink / raw)
To: Quentin Schulz
Cc: Alexey Charkov, Heiko Stübner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano, Viresh Kumar,
Chen-Yu Tsai, Diederik de Haas, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, Kever Yang
Hello all,
On 2025-03-12 11:34, Dragan Simic wrote:
> On 2025-03-12 11:15, Quentin Schulz wrote:
>> On 2/16/25 1:32 PM, Alexey Charkov wrote:
>>> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de>
>>> wrote:
>>>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>>>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz
>>>>> <quentin.schulz@cherry.de> wrote:
>>>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
>> [...]
>>>>>>> + };
>>>>>>> +
>>>>>>> + cluster2_opp_table: opp-table-cluster2 {
>>>>>>> + compatible = "operating-points-v2";
>>>>>>> + opp-shared;
>>>>>>> +
>>>>>>> + opp-1416000000 {
>>>>>>> + opp-hz = /bits/ 64 <1416000000>;
>>>>>>> + opp-microvolt = <750000 750000 950000>;
>>>>>>> + clock-latency-ns = <40000>;
>>>>>>> + };
>>>>>>> + opp-1608000000 {
>>>>>>> + opp-hz = /bits/ 64 <1608000000>;
>>>>>>> + opp-microvolt = <787500 787500 950000>;
>>>>>>> + clock-latency-ns = <40000>;
>>>>>>> + };
>>>>>>> + opp-1800000000 {
>>>>>>> + opp-hz = /bits/ 64 <1800000000>;
>>>>>>> + opp-microvolt = <875000 875000 950000>;
>>>>>>> + clock-latency-ns = <40000>;
>>>>>>> + };
>>>>>>> + opp-2016000000 {
>>>>>>> + opp-hz = /bits/ 64 <2016000000>;
>>>>>>> + opp-microvolt = <950000 950000 950000>;
>>>>>>> + clock-latency-ns = <40000>;
>>>>>>> + };
>>>>>>
>>>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>>>>>>
>>>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at
>>>>>> least
>>>>>> for awareness on Rockchip side :)
>>>>>>
>>>>>> I guess another option could be to mark those above as
>>>>>>
>>>>>> turbo-mode;
>>>>>>
>>>>>> though no clue what this would entail. From a glance at cpufreq,
>>>>>> it
>>>>>> seems that for Rockchip since we use the default cpufreq-dt, it
>>>>>> would
>>>>>> mark those as unusable, so essentially a no-op, so better remove
>>>>>> them
>>>>>> entirely?
>>>>>
>>>>> I believe the opp core just marks 'turbo-mode' opps as
>>>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to
>>>>> the
>>>>> cpufreq core. But then to actually use those boost frequencies I
>>>>> would
>>>>> guess the governor needs to somehow know the power/thermal
>>>>> constraints
>>>>> of the chip?.. Don't know where that is defined.
>>>>
>>>> personally I don't believe in "boost" frequencies - except when they
>>>> are
>>>> declared officially.
>>>>
>>>> Rockchip for the longest time maintains that running the chip
>>>> outside
>>>> the declared power/rate envelope can reduce its overall lifetime.
>>>>
>>>> So for Rockchip in mainline a "it runs stable for me" has never been
>>>> a
>>>> suitable reasoning ;-) .
>>>
>>> My key concern here was perhaps that we are looking at a file inside
>>> the Rockchip source tree which looks relevant by the name of it, but
>>> doesn't actually get included anywhere for any of the boards. This
>>> may
>>> or may not constitute an endorsement by Rockchip of any OPPs listed
>>> there :-D
>>
>> Rockchip support stated:
>>
>> """
>> If you run overdrive mode, you do not need to include rk3588j.dtsi,
>> and you can change it to #incldue rk3588.dtsi to ensure that the
>> maximum frequency can reach 2GHz
>>
>> below picture from datasheet.
>> """
>>
>> The picture is the table 3-2 Recommended operating conditions, page 30
>> from the RK3588J datasheet, e.g.
>> https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
>>
>> What is overdrive?
>>
>> """
>> Overdrive mode brings higher frequency, and the voltage will increase
>> accordingly. Under
>> the overdrive mode for a long time, the chipset may shorten the
>> lifetime, especially in high temperature condition
>> """
>>
>> according to the same datasheet, end of the same table, page 31.
>>
>> so this seems like a turbo-mode frequency to me.
>>
>> Additionally, the note for the "normal mode" (the one with the lower
>> freqs) states:
>>
>> """
>> Normal mode means the chipset works under safety voltage and
>> frequency. For the
>> industrial environment, highly recommend to keep in normal mode, the
>> lifetime is
>> reasonably guaranteed.
>> """
>>
>> I believe "industrial environment" means RK3588J used in the
>> temperatures that aren't OK for the standard RK3588 variant?
>
> Thanks a lot for obtaining these clarifications!
>
> Yes, I'd say that in this case "industrial environment" boils down
> to the extended temperature range for the RK3588J variant.
>
>>> I haven't seen a TRM for the J variant, nor do I have a board with
>>> RK3588J to run tests, so it would be great if Kever could chime in
>>> with how these OPPs are different from the others (or not).
>>>
>>>> So while the situation might be strange for the rk3588j, I really
>>>> only want
>>>> correct frequencies here please - not any pseudo "turbo freqs".
>>>>
>>>> I don't care that much what people do on their own device{s/trees},
>>>> but
>>>> the devicetrees we supply centrally (and to u-boot, etc) should only
>>>> contain frequencies vetted by the manufacturer.
>>>
>>> Fair enough. Let's just try and get another data point on whether
>>> these frequencies are vetted or not.
>>
>> So the higher freqs seems to be vetted (and used by default on
>> Rockchip's BSP kernel), it just feels like you aren't really supposed
>> to run those higher frequencies all the time? And especially not in
>> "industrial environment"?
>>
>> I would assume we want to stay on the safer side and remove those
>> higher frequencies? Heiko?
>
> I agree, we should remove the higher-frequency OPPs. I'd like
> to go through everything once again in detail and prepare a patch
> that removes the unsafe OPPs, and you could review it once it's
> on the ML, to make sure it's fine.
Just as a note, everything above (and even a bit more) is confirmed
and clearly described in the publicly available RK3588J datasheet,
which I'll provide as a reference in my upcoming patch.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-03-13 10:42 ` Dragan Simic
@ 2025-03-13 19:00 ` Heiko Stuebner
2025-03-13 19:43 ` Dragan Simic
0 siblings, 1 reply; 32+ messages in thread
From: Heiko Stuebner @ 2025-03-13 19:00 UTC (permalink / raw)
To: Quentin Schulz, Dragan Simic
Cc: Alexey Charkov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Lezcano, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Kever Yang
Am Donnerstag, 13. März 2025, 11:42:17 MEZ schrieb Dragan Simic:
> Hello all,
>
> On 2025-03-12 11:34, Dragan Simic wrote:
> > On 2025-03-12 11:15, Quentin Schulz wrote:
> >> On 2/16/25 1:32 PM, Alexey Charkov wrote:
> >>> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de>
> >>> wrote:
> >>>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
> >>>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz
> >>>>> <quentin.schulz@cherry.de> wrote:
> >>>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
> >> [...]
> >>>>>>> + };
> >>>>>>> +
> >>>>>>> + cluster2_opp_table: opp-table-cluster2 {
> >>>>>>> + compatible = "operating-points-v2";
> >>>>>>> + opp-shared;
> >>>>>>> +
> >>>>>>> + opp-1416000000 {
> >>>>>>> + opp-hz = /bits/ 64 <1416000000>;
> >>>>>>> + opp-microvolt = <750000 750000 950000>;
> >>>>>>> + clock-latency-ns = <40000>;
> >>>>>>> + };
> >>>>>>> + opp-1608000000 {
> >>>>>>> + opp-hz = /bits/ 64 <1608000000>;
> >>>>>>> + opp-microvolt = <787500 787500 950000>;
> >>>>>>> + clock-latency-ns = <40000>;
> >>>>>>> + };
> >>>>>>> + opp-1800000000 {
> >>>>>>> + opp-hz = /bits/ 64 <1800000000>;
> >>>>>>> + opp-microvolt = <875000 875000 950000>;
> >>>>>>> + clock-latency-ns = <40000>;
> >>>>>>> + };
> >>>>>>> + opp-2016000000 {
> >>>>>>> + opp-hz = /bits/ 64 <2016000000>;
> >>>>>>> + opp-microvolt = <950000 950000 950000>;
> >>>>>>> + clock-latency-ns = <40000>;
> >>>>>>> + };
> >>>>>>
> >>>>>> opp-1800000000 and opp-2016000000 should be removed as well.
> >>>>>>
> >>>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at
> >>>>>> least
> >>>>>> for awareness on Rockchip side :)
> >>>>>>
> >>>>>> I guess another option could be to mark those above as
> >>>>>>
> >>>>>> turbo-mode;
> >>>>>>
> >>>>>> though no clue what this would entail. From a glance at cpufreq,
> >>>>>> it
> >>>>>> seems that for Rockchip since we use the default cpufreq-dt, it
> >>>>>> would
> >>>>>> mark those as unusable, so essentially a no-op, so better remove
> >>>>>> them
> >>>>>> entirely?
> >>>>>
> >>>>> I believe the opp core just marks 'turbo-mode' opps as
> >>>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to
> >>>>> the
> >>>>> cpufreq core. But then to actually use those boost frequencies I
> >>>>> would
> >>>>> guess the governor needs to somehow know the power/thermal
> >>>>> constraints
> >>>>> of the chip?.. Don't know where that is defined.
> >>>>
> >>>> personally I don't believe in "boost" frequencies - except when they
> >>>> are
> >>>> declared officially.
> >>>>
> >>>> Rockchip for the longest time maintains that running the chip
> >>>> outside
> >>>> the declared power/rate envelope can reduce its overall lifetime.
> >>>>
> >>>> So for Rockchip in mainline a "it runs stable for me" has never been
> >>>> a
> >>>> suitable reasoning ;-) .
> >>>
> >>> My key concern here was perhaps that we are looking at a file inside
> >>> the Rockchip source tree which looks relevant by the name of it, but
> >>> doesn't actually get included anywhere for any of the boards. This
> >>> may
> >>> or may not constitute an endorsement by Rockchip of any OPPs listed
> >>> there :-D
> >>
> >> Rockchip support stated:
> >>
> >> """
> >> If you run overdrive mode, you do not need to include rk3588j.dtsi,
> >> and you can change it to #incldue rk3588.dtsi to ensure that the
> >> maximum frequency can reach 2GHz
> >>
> >> below picture from datasheet.
> >> """
> >>
> >> The picture is the table 3-2 Recommended operating conditions, page 30
> >> from the RK3588J datasheet, e.g.
> >> https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
> >>
> >> What is overdrive?
> >>
> >> """
> >> Overdrive mode brings higher frequency, and the voltage will increase
> >> accordingly. Under
> >> the overdrive mode for a long time, the chipset may shorten the
> >> lifetime, especially in high temperature condition
> >> """
> >>
> >> according to the same datasheet, end of the same table, page 31.
> >>
> >> so this seems like a turbo-mode frequency to me.
> >>
> >> Additionally, the note for the "normal mode" (the one with the lower
> >> freqs) states:
> >>
> >> """
> >> Normal mode means the chipset works under safety voltage and
> >> frequency. For the
> >> industrial environment, highly recommend to keep in normal mode, the
> >> lifetime is
> >> reasonably guaranteed.
> >> """
> >>
> >> I believe "industrial environment" means RK3588J used in the
> >> temperatures that aren't OK for the standard RK3588 variant?
> >
> > Thanks a lot for obtaining these clarifications!
> >
> > Yes, I'd say that in this case "industrial environment" boils down
> > to the extended temperature range for the RK3588J variant.
> >
> >>> I haven't seen a TRM for the J variant, nor do I have a board with
> >>> RK3588J to run tests, so it would be great if Kever could chime in
> >>> with how these OPPs are different from the others (or not).
> >>>
> >>>> So while the situation might be strange for the rk3588j, I really
> >>>> only want
> >>>> correct frequencies here please - not any pseudo "turbo freqs".
> >>>>
> >>>> I don't care that much what people do on their own device{s/trees},
> >>>> but
> >>>> the devicetrees we supply centrally (and to u-boot, etc) should only
> >>>> contain frequencies vetted by the manufacturer.
> >>>
> >>> Fair enough. Let's just try and get another data point on whether
> >>> these frequencies are vetted or not.
> >>
> >> So the higher freqs seems to be vetted (and used by default on
> >> Rockchip's BSP kernel), it just feels like you aren't really supposed
> >> to run those higher frequencies all the time? And especially not in
> >> "industrial environment"?
> >>
> >> I would assume we want to stay on the safer side and remove those
> >> higher frequencies? Heiko?
> >
> > I agree, we should remove the higher-frequency OPPs. I'd like
> > to go through everything once again in detail and prepare a patch
> > that removes the unsafe OPPs, and you could review it once it's
> > on the ML, to make sure it's fine.
>
> Just as a note, everything above (and even a bit more) is confirmed
> and clearly described in the publicly available RK3588J datasheet,
> which I'll provide as a reference in my upcoming patch.
so just to reiterate my stance, in mainline I really only want frequencies
that are not possibly influencing the lifetime of the chip.
It doesn't even matter about the variant we're talking about being
industrial :-) . When someone is using mainline I want them to be
reasonable assured that we don't have stuff in here that may affect
the lifetime of their board.
All gambling on performance for possible lifetime reduction people
can do on their own ... for example with a dt-overlay ;-) .
So TL;DR, I agree to both Quentin and Dragan
Heiko
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-03-13 19:00 ` Heiko Stuebner
@ 2025-03-13 19:43 ` Dragan Simic
2025-03-21 3:37 ` Dragan Simic
0 siblings, 1 reply; 32+ messages in thread
From: Dragan Simic @ 2025-03-13 19:43 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Quentin Schulz, Alexey Charkov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Daniel Lezcano, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hello Heiko,
On 2025-03-13 20:00, Heiko Stuebner wrote:
> Am Donnerstag, 13. März 2025, 11:42:17 MEZ schrieb Dragan Simic:
>> On 2025-03-12 11:34, Dragan Simic wrote:
>> > On 2025-03-12 11:15, Quentin Schulz wrote:
>> >> On 2/16/25 1:32 PM, Alexey Charkov wrote:
>> >>> On Sat, Feb 15, 2025 at 11:30 PM Heiko Stübner <heiko@sntech.de>
>> >>> wrote:
>> >>>> Am Samstag, 15. Februar 2025, 19:59:44 MEZ schrieb Alexey Charkov:
>> >>>>> On Tue, Feb 11, 2025 at 7:32 PM Quentin Schulz
>> >>>>> <quentin.schulz@cherry.de> wrote:
>> >>>>>> On 6/17/24 8:28 PM, Alexey Charkov wrote:
>> >> [...]
>> >>>>>>> + };
>> >>>>>>> +
>> >>>>>>> + cluster2_opp_table: opp-table-cluster2 {
>> >>>>>>> + compatible = "operating-points-v2";
>> >>>>>>> + opp-shared;
>> >>>>>>> +
>> >>>>>>> + opp-1416000000 {
>> >>>>>>> + opp-hz = /bits/ 64 <1416000000>;
>> >>>>>>> + opp-microvolt = <750000 750000 950000>;
>> >>>>>>> + clock-latency-ns = <40000>;
>> >>>>>>> + };
>> >>>>>>> + opp-1608000000 {
>> >>>>>>> + opp-hz = /bits/ 64 <1608000000>;
>> >>>>>>> + opp-microvolt = <787500 787500 950000>;
>> >>>>>>> + clock-latency-ns = <40000>;
>> >>>>>>> + };
>> >>>>>>> + opp-1800000000 {
>> >>>>>>> + opp-hz = /bits/ 64 <1800000000>;
>> >>>>>>> + opp-microvolt = <875000 875000 950000>;
>> >>>>>>> + clock-latency-ns = <40000>;
>> >>>>>>> + };
>> >>>>>>> + opp-2016000000 {
>> >>>>>>> + opp-hz = /bits/ 64 <2016000000>;
>> >>>>>>> + opp-microvolt = <950000 950000 950000>;
>> >>>>>>> + clock-latency-ns = <40000>;
>> >>>>>>> + };
>> >>>>>>
>> >>>>>> opp-1800000000 and opp-2016000000 should be removed as well.
>> >>>>>>
>> >>>>>> Did I misunderstand what Rockchip did here? Adding Kever in Cc at
>> >>>>>> least
>> >>>>>> for awareness on Rockchip side :)
>> >>>>>>
>> >>>>>> I guess another option could be to mark those above as
>> >>>>>>
>> >>>>>> turbo-mode;
>> >>>>>>
>> >>>>>> though no clue what this would entail. From a glance at cpufreq,
>> >>>>>> it
>> >>>>>> seems that for Rockchip since we use the default cpufreq-dt, it
>> >>>>>> would
>> >>>>>> mark those as unusable, so essentially a no-op, so better remove
>> >>>>>> them
>> >>>>>> entirely?
>> >>>>>
>> >>>>> I believe the opp core just marks 'turbo-mode' opps as
>> >>>>> CPUFREQ_BOOST_FREQ, and cpufreq-dt only passes that flag along to
>> >>>>> the
>> >>>>> cpufreq core. But then to actually use those boost frequencies I
>> >>>>> would
>> >>>>> guess the governor needs to somehow know the power/thermal
>> >>>>> constraints
>> >>>>> of the chip?.. Don't know where that is defined.
>> >>>>
>> >>>> personally I don't believe in "boost" frequencies - except when they
>> >>>> are
>> >>>> declared officially.
>> >>>>
>> >>>> Rockchip for the longest time maintains that running the chip
>> >>>> outside
>> >>>> the declared power/rate envelope can reduce its overall lifetime.
>> >>>>
>> >>>> So for Rockchip in mainline a "it runs stable for me" has never been
>> >>>> a
>> >>>> suitable reasoning ;-) .
>> >>>
>> >>> My key concern here was perhaps that we are looking at a file inside
>> >>> the Rockchip source tree which looks relevant by the name of it, but
>> >>> doesn't actually get included anywhere for any of the boards. This
>> >>> may
>> >>> or may not constitute an endorsement by Rockchip of any OPPs listed
>> >>> there :-D
>> >>
>> >> Rockchip support stated:
>> >>
>> >> """
>> >> If you run overdrive mode, you do not need to include rk3588j.dtsi,
>> >> and you can change it to #incldue rk3588.dtsi to ensure that the
>> >> maximum frequency can reach 2GHz
>> >>
>> >> below picture from datasheet.
>> >> """
>> >>
>> >> The picture is the table 3-2 Recommended operating conditions, page 30
>> >> from the RK3588J datasheet, e.g.
>> >> https://www.lcsc.com/datasheet/lcsc_datasheet_2403201054_Rockchip-RK3588J_C22364189.pdf
>> >>
>> >> What is overdrive?
>> >>
>> >> """
>> >> Overdrive mode brings higher frequency, and the voltage will increase
>> >> accordingly. Under
>> >> the overdrive mode for a long time, the chipset may shorten the
>> >> lifetime, especially in high temperature condition
>> >> """
>> >>
>> >> according to the same datasheet, end of the same table, page 31.
>> >>
>> >> so this seems like a turbo-mode frequency to me.
>> >>
>> >> Additionally, the note for the "normal mode" (the one with the lower
>> >> freqs) states:
>> >>
>> >> """
>> >> Normal mode means the chipset works under safety voltage and
>> >> frequency. For the
>> >> industrial environment, highly recommend to keep in normal mode, the
>> >> lifetime is
>> >> reasonably guaranteed.
>> >> """
>> >>
>> >> I believe "industrial environment" means RK3588J used in the
>> >> temperatures that aren't OK for the standard RK3588 variant?
>> >
>> > Thanks a lot for obtaining these clarifications!
>> >
>> > Yes, I'd say that in this case "industrial environment" boils down
>> > to the extended temperature range for the RK3588J variant.
>> >
>> >>> I haven't seen a TRM for the J variant, nor do I have a board with
>> >>> RK3588J to run tests, so it would be great if Kever could chime in
>> >>> with how these OPPs are different from the others (or not).
>> >>>
>> >>>> So while the situation might be strange for the rk3588j, I really
>> >>>> only want
>> >>>> correct frequencies here please - not any pseudo "turbo freqs".
>> >>>>
>> >>>> I don't care that much what people do on their own device{s/trees},
>> >>>> but
>> >>>> the devicetrees we supply centrally (and to u-boot, etc) should only
>> >>>> contain frequencies vetted by the manufacturer.
>> >>>
>> >>> Fair enough. Let's just try and get another data point on whether
>> >>> these frequencies are vetted or not.
>> >>
>> >> So the higher freqs seems to be vetted (and used by default on
>> >> Rockchip's BSP kernel), it just feels like you aren't really supposed
>> >> to run those higher frequencies all the time? And especially not in
>> >> "industrial environment"?
>> >>
>> >> I would assume we want to stay on the safer side and remove those
>> >> higher frequencies? Heiko?
>> >
>> > I agree, we should remove the higher-frequency OPPs. I'd like
>> > to go through everything once again in detail and prepare a patch
>> > that removes the unsafe OPPs, and you could review it once it's
>> > on the ML, to make sure it's fine.
>>
>> Just as a note, everything above (and even a bit more) is confirmed
>> and clearly described in the publicly available RK3588J datasheet,
>> which I'll provide as a reference in my upcoming patch.
>
> so just to reiterate my stance, in mainline I really only want
> frequencies
> that are not possibly influencing the lifetime of the chip.
>
> It doesn't even matter about the variant we're talking about being
> industrial :-) . When someone is using mainline I want them to be
> reasonable assured that we don't have stuff in here that may affect
> the lifetime of their board.
>
> All gambling on performance for possible lifetime reduction people
> can do on their own ... for example with a dt-overlay ;-) .
>
> So TL;DR, I agree to both Quentin and Dragan
Thanks! Indeed, we must provide only the OPPs that are declared
by the manufacturer to be always safe for the particular SoC
variant. The RK3588J is actually a good example, because it, in
theory, can run safely at higher OPPs as well, but only when not
enjoying the extended temperature range that the RK3588J, as an
SoC variant targeted at industrial applications, is specifically
made (or binned) for.
Thus, we must support only the RK3588J OPPs that are declared to
be safe throughout the entire extended temperature range, while
anyone who actually can assure that their RK3588J-based board is
never going to run within the extended temperature range, probably
may safely apply an overlay that adds the higher OPPs. As we
obviously can't know what will be the temperature conditions, we
may provide only the lower OPPs that are always safe.
I'll finish the patch and send it over tomorrow or so... I still
need to go through the changes once again, to make 100% sure I've
missed nothing, and that I haven't included anything extraneous. :)
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
2025-03-13 19:43 ` Dragan Simic
@ 2025-03-21 3:37 ` Dragan Simic
0 siblings, 0 replies; 32+ messages in thread
From: Dragan Simic @ 2025-03-21 3:37 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Quentin Schulz, Alexey Charkov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Daniel Lezcano, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Kever Yang
Hello all,
On 2025-03-13 20:43, Dragan Simic wrote:
> On 2025-03-13 20:00, Heiko Stuebner wrote:
>> Am Donnerstag, 13. März 2025, 11:42:17 MEZ schrieb Dragan Simic:
>>> On 2025-03-12 11:34, Dragan Simic wrote:
>>> Just as a note, everything above (and even a bit more) is confirmed
>>> and clearly described in the publicly available RK3588J datasheet,
>>> which I'll provide as a reference in my upcoming patch.
>>
>> so just to reiterate my stance, in mainline I really only want
>> frequencies
>> that are not possibly influencing the lifetime of the chip.
>>
>> It doesn't even matter about the variant we're talking about being
>> industrial :-) . When someone is using mainline I want them to be
>> reasonable assured that we don't have stuff in here that may affect
>> the lifetime of their board.
>>
>> All gambling on performance for possible lifetime reduction people
>> can do on their own ... for example with a dt-overlay ;-) .
>>
>> So TL;DR, I agree to both Quentin and Dragan
>
> Thanks! Indeed, we must provide only the OPPs that are declared
> by the manufacturer to be always safe for the particular SoC
> variant. The RK3588J is actually a good example, because it, in
> theory, can run safely at higher OPPs as well, but only when not
> enjoying the extended temperature range that the RK3588J, as an
> SoC variant targeted at industrial applications, is specifically
> made (or binned) for.
>
> Thus, we must support only the RK3588J OPPs that are declared to
> be safe throughout the entire extended temperature range, while
> anyone who actually can assure that their RK3588J-based board is
> never going to run within the extended temperature range, probably
> may safely apply an overlay that adds the higher OPPs. As we
> obviously can't know what will be the temperature conditions, we
> may provide only the lower OPPs that are always safe.
>
> I'll finish the patch and send it over tomorrow or so... I still
> need to go through the changes once again, to make 100% sure I've
> missed nothing, and that I haven't included anything extraneous. :)
For future reference and for anyone interested, below is the link
to the above-mentioned patch.
https://lore.kernel.org/linux-rockchip/f929da061de35925ea591c969f985430e23c4a7e.1742526811.git.dsimic@manjaro.org/T/#u
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v5 8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (6 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j Alexey Charkov
@ 2024-06-17 18:28 ` Alexey Charkov
2025-02-11 16:34 ` Quentin Schulz
2024-06-17 18:56 ` [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Dragan Simic
` (3 subsequent siblings)
11 siblings, 1 reply; 32+ messages in thread
From: Alexey Charkov @ 2024-06-17 18:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alexey Charkov
RK3588j uses a different set of OPPs for its GPU, both in terms of
allowed frequencies and in terms of voltages.
Move the GPU OPPs table into per-variant .dtsi files to accommodate
for this difference.
The table for RK3588j is adapted from Rockchip downstream sources [1],
while RK3588 one is moved verbatim into the per-variant .dtsi file.
The values provided for RK3588 in the downstream sources match those
in the original commit.
[1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
Fixes: 6fca4edb93d3 ("arm64: dts: rockchip: Add rk3588 GPU node")
Signed-off-by: Alexey Charkov <alchark@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 38 -------------------------
arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 41 +++++++++++++++++++++++++++
arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 33 +++++++++++++++++++++
3 files changed, 74 insertions(+), 38 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 758aff5e040b..3d918874aa02 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -451,46 +451,8 @@ gpu: gpu@fb000000 {
<GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH 0>,
<GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH 0>;
interrupt-names = "job", "mmu", "gpu";
- operating-points-v2 = <&gpu_opp_table>;
power-domains = <&power RK3588_PD_GPU>;
status = "disabled";
-
- gpu_opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp-300000000 {
- opp-hz = /bits/ 64 <300000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-400000000 {
- opp-hz = /bits/ 64 <400000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-500000000 {
- opp-hz = /bits/ 64 <500000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-600000000 {
- opp-hz = /bits/ 64 <600000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-700000000 {
- opp-hz = /bits/ 64 <700000000>;
- opp-microvolt = <700000 700000 850000>;
- };
- opp-800000000 {
- opp-hz = /bits/ 64 <800000000>;
- opp-microvolt = <750000 750000 850000>;
- };
- opp-900000000 {
- opp-hz = /bits/ 64 <900000000>;
- opp-microvolt = <800000 800000 850000>;
- };
- opp-1000000000 {
- opp-hz = /bits/ 64 <1000000000>;
- opp-microvolt = <850000 850000 850000>;
- };
- };
};
usb_host0_xhci: usb@fc000000 {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
index 35bbc3c2134f..0f1a77697351 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
@@ -114,6 +114,43 @@ opp-2400000000 {
clock-latency-ns = <40000>;
};
};
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-700000000 {
+ opp-hz = /bits/ 64 <700000000>;
+ opp-microvolt = <700000 700000 850000>;
+ };
+ opp-800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-900000000 {
+ opp-hz = /bits/ 64 <900000000>;
+ opp-microvolt = <800000 800000 850000>;
+ };
+ opp-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <850000 850000 850000>;
+ };
+ };
};
&cpu_b0 {
@@ -147,3 +184,7 @@ &cpu_l2 {
&cpu_l3 {
operating-points-v2 = <&cluster0_opp_table>;
};
+
+&gpu {
+ operating-points-v2 = <&gpu_opp_table>;
+};
diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
index b7e69553857b..bce72bac4503 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
@@ -80,6 +80,35 @@ opp-2016000000 {
clock-latency-ns = <40000>;
};
};
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-700000000 {
+ opp-hz = /bits/ 64 <700000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-850000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <787500 787500 850000>;
+ };
+ };
};
&cpu_b0 {
@@ -113,3 +142,7 @@ &cpu_l2 {
&cpu_l3 {
operating-points-v2 = <&cluster0_opp_table>;
};
+
+&gpu {
+ operating-points-v2 = <&gpu_opp_table>;
+};
--
2.45.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v5 8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
2024-06-17 18:28 ` [PATCH v5 8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j Alexey Charkov
@ 2025-02-11 16:34 ` Quentin Schulz
0 siblings, 0 replies; 32+ messages in thread
From: Quentin Schulz @ 2025-02-11 16:34 UTC (permalink / raw)
To: Alexey Charkov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Heiko Stuebner
Cc: Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
Hi all,
On 6/17/24 8:28 PM, Alexey Charkov wrote:
> RK3588j uses a different set of OPPs for its GPU, both in terms of
> allowed frequencies and in terms of voltages.
>
> Move the GPU OPPs table into per-variant .dtsi files to accommodate
> for this difference.
>
> The table for RK3588j is adapted from Rockchip downstream sources [1],
> while RK3588 one is moved verbatim into the per-variant .dtsi file.
> The values provided for RK3588 in the downstream sources match those
> in the original commit.
>
> [1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>
> Fixes: 6fca4edb93d3 ("arm64: dts: rockchip: Add rk3588 GPU node")
> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 38 -------------------------
> arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 41 +++++++++++++++++++++++++++
> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 33 +++++++++++++++++++++
> 3 files changed, 74 insertions(+), 38 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> index 758aff5e040b..3d918874aa02 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> @@ -451,46 +451,8 @@ gpu: gpu@fb000000 {
> <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH 0>,
> <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH 0>;
> interrupt-names = "job", "mmu", "gpu";
> - operating-points-v2 = <&gpu_opp_table>;
> power-domains = <&power RK3588_PD_GPU>;
> status = "disabled";
> -
> - gpu_opp_table: opp-table {
> - compatible = "operating-points-v2";
> -
> - opp-300000000 {
> - opp-hz = /bits/ 64 <300000000>;
> - opp-microvolt = <675000 675000 850000>;
> - };
> - opp-400000000 {
> - opp-hz = /bits/ 64 <400000000>;
> - opp-microvolt = <675000 675000 850000>;
> - };
> - opp-500000000 {
> - opp-hz = /bits/ 64 <500000000>;
> - opp-microvolt = <675000 675000 850000>;
> - };
> - opp-600000000 {
> - opp-hz = /bits/ 64 <600000000>;
> - opp-microvolt = <675000 675000 850000>;
> - };
> - opp-700000000 {
> - opp-hz = /bits/ 64 <700000000>;
> - opp-microvolt = <700000 700000 850000>;
> - };
> - opp-800000000 {
> - opp-hz = /bits/ 64 <800000000>;
> - opp-microvolt = <750000 750000 850000>;
> - };
> - opp-900000000 {
> - opp-hz = /bits/ 64 <900000000>;
> - opp-microvolt = <800000 800000 850000>;
> - };
> - opp-1000000000 {
> - opp-hz = /bits/ 64 <1000000000>;
> - opp-microvolt = <850000 850000 850000>;
> - };
> - };
> };
>
> usb_host0_xhci: usb@fc000000 {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
> index 35bbc3c2134f..0f1a77697351 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi
> @@ -114,6 +114,43 @@ opp-2400000000 {
> clock-latency-ns = <40000>;
> };
> };
> +
> + gpu_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <675000 675000 850000>;
> + };
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <675000 675000 850000>;
> + };
> + opp-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + opp-microvolt = <675000 675000 850000>;
> + };
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + opp-microvolt = <675000 675000 850000>;
> + };
> + opp-700000000 {
> + opp-hz = /bits/ 64 <700000000>;
> + opp-microvolt = <700000 700000 850000>;
> + };
> + opp-800000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-900000000 {
> + opp-hz = /bits/ 64 <900000000>;
> + opp-microvolt = <800000 800000 850000>;
> + };
> + opp-1000000000 {
> + opp-hz = /bits/ 64 <1000000000>;
> + opp-microvolt = <850000 850000 850000>;
> + };
> + };
> };
>
> &cpu_b0 {
> @@ -147,3 +184,7 @@ &cpu_l2 {
> &cpu_l3 {
> operating-points-v2 = <&cluster0_opp_table>;
> };
> +
> +&gpu {
> + operating-points-v2 = <&gpu_opp_table>;
> +};
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> index b7e69553857b..bce72bac4503 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
> @@ -80,6 +80,35 @@ opp-2016000000 {
> clock-latency-ns = <40000>;
> };
> };
> +
> + gpu_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-700000000 {
> + opp-hz = /bits/ 64 <700000000>;
> + opp-microvolt = <750000 750000 850000>;
> + };
> + opp-850000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <787500 787500 850000>;
> + };
Same remark as for the CPU OPPs, here Rockchip removes opp-850000000 for
the GPU OPPs on RK3588J.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (7 preceding siblings ...)
2024-06-17 18:28 ` [PATCH v5 8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j Alexey Charkov
@ 2024-06-17 18:56 ` Dragan Simic
2024-06-20 20:39 ` (subset) " Heiko Stuebner
` (2 subsequent siblings)
11 siblings, 0 replies; 32+ messages in thread
From: Dragan Simic @ 2024-06-17 18:56 UTC (permalink / raw)
To: Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Daniel Lezcano, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, linux-rockchip, linux-kernel
Hello Alexey,
On 2024-06-17 20:28, Alexey Charkov wrote:
> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> active cooling on Radxa Rock 5B via the provided PWM fan.
>
> Some RK3588 boards use separate regulators to supply CPUs and their
> respective memory interfaces, so this is handled by coupling those
> regulators in affected boards' device trees to ensure that their
> voltage is adjusted in step.
>
> This also enables the built-in thermal sensor (TSADC) for all boards
> that don't currently have it enabled, using the default CRU based
> emergency thermal reset. This default configuration only uses on-SoC
> devices and doesn't rely on any external wiring, thus it should work
> for all devices (tested only on Rock 5B though).
>
> The boards that have TSADC_SHUT signal wired to the PMIC reset line
> can choose to override the default reset logic in favour of GPIO
> driven (PMIC assisted) reset, but in my testing it didn't work on
> Radxa Rock 5B - maybe I'm reading the schematic wrong and it doesn't
> support PMIC assisted reset after all.
>
> Fan control on Rock 5B has been split into two intervals: let it spin
> at the minimum cooling state between 55C and 65C, and then accelerate
> if the system crosses the 65C mark - thanks to Dragan for suggesting.
> This lets some cooling setups with beefier heatsinks and/or larger
> fan fins to stay in the quietest non-zero fan state while still
> gaining potential benefits from the airflow it generates, and
> possibly avoiding noisy speeds altogether for some workloads.
I should be able to test an RK3588 cooling setup with a really beefy
heatsink in the near future, so we'll see how well the "silent zone"
performs in practice and not just in theory. :)
> OPPs help actually scale CPU frequencies up and down for both cooling
> and performance - tested on Rock 5B under varied loads. I've dropped
> those OPPs that cause frequency reductions without accompanying
> decrease
> in CPU voltage, as they don't seem to be adding much benefit in day to
> day use, while the kernel log gets a number of "OPP is inefficient"
> lines.
>
> Note that this submission doesn't touch the SRAM read margin updates or
> the OPP calibration based on silicon quality which the downstream
> driver
> does and which were mentioned in [1]. It works as it is (also confirmed
> by
> Sebastian in his follow-up message [2]), and it is stable in my testing
> on
> Rock 5B, so it sounds better to merge a simple version first and then
> extend when/if required.
>
> This patch series has been rebased on top of Heiko's recent for-next
> branch
> with Dragan's patch [3] which rearranges the .dtsi files for
> per-variant OPPs.
> As a result, it now includes separate CPU OPP tables for RK3588(s) and
> RK3588j.
Thanks for the current iteration of this series! I'll review it
in detail and perform thorough stress testing on my ROCK 5B as soon
as possible.
> GPU OPPs have also been split out to accommodate for the difference in
> RK3588j.
>
> [1]
> https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/
> [2]
> https://lore.kernel.org/linux-rockchip/pkyne4g2cln27dcdu3jm7bqdqpmd2kwkbguiolmozntjuiajrb@gvq4nupzna4o/
> [3]
> https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>
> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> ---
> Changes in v5:
> - Rebased against linux-rockchip/for-next with Dragan's .dtsi
> reshuffling on top
> - Added separate OPP values for RK3588j (these also apply to RK3588m)
> - Separated GPU OPP values for RK3588j (RK3588m ones differ slightly,
> not included here)
> - Dragan's patch:
> https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
> - Link to v4:
> https://lore.kernel.org/r/20240506-rk-dts-additions-v4-0-271023ddfd40@gmail.com
>
> Changes in v4:
> - Rebased against linux-rockchip/for-next
> - Reordered DT nodes alphabetically as pointed out by Diederik
> - Moved the TSADC enablement to per-board .dts/.dtsi files
> - Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
> - Dropped second passive cooling trips altogether to keep things simple
> - Added a cooling map for passive GPU cooling (in a separate patch)
> - Link to v3:
> https://lore.kernel.org/r/20240229-rk-dts-additions-v3-0-6afe8473a631@gmail.com
>
> Changes in v3:
> - Added regulator coupling for EVB1 and QuartzPro64
> - Enabled the TSADC for all boards in .dtsi, not just Rock 5B (thanks
> ChenYu)
> - Added comments regarding two passive cooling trips in each zone
> (thanks Dragan)
> - Fixed active cooling map numbering for Radxa Rock 5B (thanks Dragan)
> - Dropped Daniel's Acked-by tag from the Rock 5B fan patch, as there's
> been quite some
> churn there since the version he acknowledged
> - Link to v2:
> https://lore.kernel.org/r/20240130-rk-dts-additions-v2-0-c6222c4c78df@gmail.com
>
> Changes in v2:
> - Dropped the rfkill patch which Heiko has already applied
> - Set higher 'polling-delay-passive' (100 instead of 20)
> - Name all cooling maps starting from map0 in each respective zone
> - Drop 'contribution' properties from passive cooling maps
> - Link to v1:
> https://lore.kernel.org/r/20240125-rk-dts-additions-v1-0-5879275db36f@gmail.com
>
> ---
> Alexey Charkov (8):
> arm64: dts: rockchip: add thermal zones information on RK3588
> arm64: dts: rockchip: enable thermal management on all RK3588
> boards
> arm64: dts: rockchip: add passive GPU cooling on RK3588
> arm64: dts: rockchip: enable automatic fan control on Rock 5B
> arm64: dts: rockchip: Add CPU/memory regulator coupling for
> RK3588
> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
> arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
>
> .../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 197
> +++++++++++++++++----
> .../dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 16 ++
> arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 190
> ++++++++++++++++++++
> .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 34 +++-
> .../arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 +
> .../arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 +
> arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 141
> +++++++++++++++
> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
> 14 files changed, 577 insertions(+), 39 deletions(-)
> ---
> base-commit: 5cc74606bf40a2bbaccd3e3bb2781f637baebde5
> change-id: 20240124-rk-dts-additions-a6d7b52787b9
>
> Best regards,
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: (subset) [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (8 preceding siblings ...)
2024-06-17 18:56 ` [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Dragan Simic
@ 2024-06-20 20:39 ` Heiko Stuebner
2024-06-21 6:15 ` Alexey Charkov
2024-06-24 16:20 ` Heiko Stuebner
2024-07-07 9:39 ` Piotr Oniszczuk
11 siblings, 1 reply; 32+ messages in thread
From: Heiko Stuebner @ 2024-06-20 20:39 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Alexey Charkov, Conor Dooley
Cc: Heiko Stuebner, Diederik de Haas, Daniel Lezcano, devicetree,
Viresh Kumar, Chen-Yu Tsai, linux-rockchip, Dragan Simic,
linux-kernel, linux-arm-kernel
On Mon, 17 Jun 2024 22:28:50 +0400, Alexey Charkov wrote:
> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> active cooling on Radxa Rock 5B via the provided PWM fan.
>
> Some RK3588 boards use separate regulators to supply CPUs and their
> respective memory interfaces, so this is handled by coupling those
> regulators in affected boards' device trees to ensure that their
> voltage is adjusted in step.
>
> [...]
Applied, thanks!
[1/8] arm64: dts: rockchip: add thermal zones information on RK3588
commit: 97d61227d6bb0023a325ab2f87e4438a858207a2
[2/8] arm64: dts: rockchip: enable thermal management on all RK3588 boards
commit: 4afa9056ed9e3d9ff036f3576cb137a011448295
[3/8] arm64: dts: rockchip: add passive GPU cooling on RK3588
commit: d64d337f1856bd0e5cbfc60b6f0458ad4951d05e
[4/8] arm64: dts: rockchip: enable automatic fan control on Rock 5B
commit: 2aab8905a843aef624565c014a34d155f8702135
Don't worry, I'll look at the other patches too.
Was just easier on my mind to this in blocks ;-) .
Best regards,
--
Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: (subset) [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-06-20 20:39 ` (subset) " Heiko Stuebner
@ 2024-06-21 6:15 ` Alexey Charkov
0 siblings, 0 replies; 32+ messages in thread
From: Alexey Charkov @ 2024-06-21 6:15 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Krzysztof Kozlowski, Rob Herring, Conor Dooley, Diederik de Haas,
Daniel Lezcano, devicetree, Viresh Kumar, Chen-Yu Tsai,
linux-rockchip, Dragan Simic, linux-kernel, linux-arm-kernel
On Fri, Jun 21, 2024 at 12:40 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> On Mon, 17 Jun 2024 22:28:50 +0400, Alexey Charkov wrote:
> > This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> > active cooling on Radxa Rock 5B via the provided PWM fan.
> >
> > Some RK3588 boards use separate regulators to supply CPUs and their
> > respective memory interfaces, so this is handled by coupling those
> > regulators in affected boards' device trees to ensure that their
> > voltage is adjusted in step.
> >
> > [...]
>
> Applied, thanks!
>
> [1/8] arm64: dts: rockchip: add thermal zones information on RK3588
> commit: 97d61227d6bb0023a325ab2f87e4438a858207a2
> [2/8] arm64: dts: rockchip: enable thermal management on all RK3588 boards
> commit: 4afa9056ed9e3d9ff036f3576cb137a011448295
> [3/8] arm64: dts: rockchip: add passive GPU cooling on RK3588
> commit: d64d337f1856bd0e5cbfc60b6f0458ad4951d05e
> [4/8] arm64: dts: rockchip: enable automatic fan control on Rock 5B
> commit: 2aab8905a843aef624565c014a34d155f8702135
>
>
> Don't worry, I'll look at the other patches too.
> Was just easier on my mind to this in blocks ;-) .
Great, thanks a lot Heiko!
Best regards,
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: (subset) [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (9 preceding siblings ...)
2024-06-20 20:39 ` (subset) " Heiko Stuebner
@ 2024-06-24 16:20 ` Heiko Stuebner
2024-07-07 9:39 ` Piotr Oniszczuk
11 siblings, 0 replies; 32+ messages in thread
From: Heiko Stuebner @ 2024-06-24 16:20 UTC (permalink / raw)
To: Conor Dooley, Alexey Charkov, Rob Herring, Krzysztof Kozlowski
Cc: Heiko Stuebner, Dragan Simic, linux-kernel, devicetree,
Diederik de Haas, Daniel Lezcano, linux-rockchip,
linux-arm-kernel, Viresh Kumar, Chen-Yu Tsai
On Mon, 17 Jun 2024 22:28:50 +0400, Alexey Charkov wrote:
> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> active cooling on Radxa Rock 5B via the provided PWM fan.
>
> Some RK3588 boards use separate regulators to supply CPUs and their
> respective memory interfaces, so this is handled by coupling those
> regulators in affected boards' device trees to ensure that their
> voltage is adjusted in step.
>
> [...]
Applied, thanks!
[5/8] arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
commit: 0ba0560982bc8d0c3fb3ca209fd0ed29f81402ac
[6/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
commit: 276856db91b46eaa7a4c19226c096a9dc899a3e9
[7/8] arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
commit: 667885a6865832eb0678c7e02e47a3392f177ecb
[8/8] arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
commit: a7b2070505a2a09ea65fa0c8c480c97f62d1978d
Best regards,
--
Heiko Stuebner <heiko@sntech.de>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-06-17 18:28 [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan Alexey Charkov
` (10 preceding siblings ...)
2024-06-24 16:20 ` Heiko Stuebner
@ 2024-07-07 9:39 ` Piotr Oniszczuk
2024-07-07 11:11 ` Heiko Stübner
11 siblings, 1 reply; 32+ messages in thread
From: Piotr Oniszczuk @ 2024-07-07 9:39 UTC (permalink / raw)
To: Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel,
open list:ARM/Rockchip SoC..., linux-kernel
Alexey,
I’m playing with this series on rock5c on 6.10-rc6.
Is code in this series enough to get working pwm-fan on rock5c?
(of course after adding required changes from rokc5b dts to rock5c dts)
In my case i’m getting constantly full speed of fan on my rock5c.
hw seems ok as echo 96 > /sys/devices/platform/pwm-fan/hwmon/hwmon0/pwm1 changes fans speed as expected.
May you pls hint me what i’m missing here?
> Wiadomość napisana przez Alexey Charkov <alchark@gmail.com> w dniu 17.06.2024, o godz. 20:28:
>
> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> active cooling on Radxa Rock 5B via the provided PWM fan.
>
> Some RK3588 boards use separate regulators to supply CPUs and their
> respective memory interfaces, so this is handled by coupling those
> regulators in affected boards' device trees to ensure that their
> voltage is adjusted in step.
>
> This also enables the built-in thermal sensor (TSADC) for all boards
> that don't currently have it enabled, using the default CRU based
> emergency thermal reset. This default configuration only uses on-SoC
> devices and doesn't rely on any external wiring, thus it should work
> for all devices (tested only on Rock 5B though).
>
> The boards that have TSADC_SHUT signal wired to the PMIC reset line
> can choose to override the default reset logic in favour of GPIO
> driven (PMIC assisted) reset, but in my testing it didn't work on
> Radxa Rock 5B - maybe I'm reading the schematic wrong and it doesn't
> support PMIC assisted reset after all.
>
> Fan control on Rock 5B has been split into two intervals: let it spin
> at the minimum cooling state between 55C and 65C, and then accelerate
> if the system crosses the 65C mark - thanks to Dragan for suggesting.
> This lets some cooling setups with beefier heatsinks and/or larger
> fan fins to stay in the quietest non-zero fan state while still
> gaining potential benefits from the airflow it generates, and
> possibly avoiding noisy speeds altogether for some workloads.
>
> OPPs help actually scale CPU frequencies up and down for both cooling
> and performance - tested on Rock 5B under varied loads. I've dropped
> those OPPs that cause frequency reductions without accompanying decrease
> in CPU voltage, as they don't seem to be adding much benefit in day to
> day use, while the kernel log gets a number of "OPP is inefficient" lines.
>
> Note that this submission doesn't touch the SRAM read margin updates or
> the OPP calibration based on silicon quality which the downstream driver
> does and which were mentioned in [1]. It works as it is (also confirmed by
> Sebastian in his follow-up message [2]), and it is stable in my testing on
> Rock 5B, so it sounds better to merge a simple version first and then
> extend when/if required.
>
> This patch series has been rebased on top of Heiko's recent for-next branch
> with Dragan's patch [3] which rearranges the .dtsi files for per-variant OPPs.
> As a result, it now includes separate CPU OPP tables for RK3588(s) and RK3588j.
>
> GPU OPPs have also been split out to accommodate for the difference in RK3588j.
>
> [1] https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/
> [2] https://lore.kernel.org/linux-rockchip/pkyne4g2cln27dcdu3jm7bqdqpmd2kwkbguiolmozntjuiajrb@gvq4nupzna4o/
> [3] https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>
> Signed-off-by: Alexey Charkov <alchark@gmail.com>
> ---
> Changes in v5:
> - Rebased against linux-rockchip/for-next with Dragan's .dtsi reshuffling on top
> - Added separate OPP values for RK3588j (these also apply to RK3588m)
> - Separated GPU OPP values for RK3588j (RK3588m ones differ slightly, not included here)
> - Dragan's patch: https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
> - Link to v4: https://lore.kernel.org/r/20240506-rk-dts-additions-v4-0-271023ddfd40@gmail.com
>
> Changes in v4:
> - Rebased against linux-rockchip/for-next
> - Reordered DT nodes alphabetically as pointed out by Diederik
> - Moved the TSADC enablement to per-board .dts/.dtsi files
> - Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
> - Dropped second passive cooling trips altogether to keep things simple
> - Added a cooling map for passive GPU cooling (in a separate patch)
> - Link to v3: https://lore.kernel.org/r/20240229-rk-dts-additions-v3-0-6afe8473a631@gmail.com
>
> Changes in v3:
> - Added regulator coupling for EVB1 and QuartzPro64
> - Enabled the TSADC for all boards in .dtsi, not just Rock 5B (thanks ChenYu)
> - Added comments regarding two passive cooling trips in each zone (thanks Dragan)
> - Fixed active cooling map numbering for Radxa Rock 5B (thanks Dragan)
> - Dropped Daniel's Acked-by tag from the Rock 5B fan patch, as there's been quite some
> churn there since the version he acknowledged
> - Link to v2: https://lore.kernel.org/r/20240130-rk-dts-additions-v2-0-c6222c4c78df@gmail.com
>
> Changes in v2:
> - Dropped the rfkill patch which Heiko has already applied
> - Set higher 'polling-delay-passive' (100 instead of 20)
> - Name all cooling maps starting from map0 in each respective zone
> - Drop 'contribution' properties from passive cooling maps
> - Link to v1: https://lore.kernel.org/r/20240125-rk-dts-additions-v1-0-5879275db36f@gmail.com
>
> ---
> Alexey Charkov (8):
> arm64: dts: rockchip: add thermal zones information on RK3588
> arm64: dts: rockchip: enable thermal management on all RK3588 boards
> arm64: dts: rockchip: add passive GPU cooling on RK3588
> arm64: dts: rockchip: enable automatic fan control on Rock 5B
> arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
> arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
>
> .../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 197 +++++++++++++++++----
> .../dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 16 ++
> arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 190 ++++++++++++++++++++
> .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++
> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 34 +++-
> .../arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 +
> .../arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 +
> arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 141 +++++++++++++++
> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 +
> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
> 14 files changed, 577 insertions(+), 39 deletions(-)
> ---
> base-commit: 5cc74606bf40a2bbaccd3e3bb2781f637baebde5
> change-id: 20240124-rk-dts-additions-a6d7b52787b9
>
> Best regards,
> --
> Alexey Charkov <alchark@gmail.com>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-07-07 9:39 ` Piotr Oniszczuk
@ 2024-07-07 11:11 ` Heiko Stübner
2024-07-07 12:37 ` Piotr Oniszczuk
0 siblings, 1 reply; 32+ messages in thread
From: Heiko Stübner @ 2024-07-07 11:11 UTC (permalink / raw)
To: Alexey Charkov, Piotr Oniszczuk
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano,
Dragan Simic, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, open list:ARM/Rockchip SoC...,
linux-kernel
Hey,
Am Sonntag, 7. Juli 2024, 11:39:57 CEST schrieb Piotr Oniszczuk:
> Alexey,
> I’m playing with this series on rock5c on 6.10-rc6.
>
> Is code in this series enough to get working pwm-fan on rock5c?
> (of course after adding required changes from rokc5b dts to rock5c dts)
>
> In my case i’m getting constantly full speed of fan on my rock5c.
>
> hw seems ok as echo 96 > /sys/devices/platform/pwm-fan/hwmon/hwmon0/pwm1 changes fans speed as expected.
>
> May you pls hint me what i’m missing here?
at least on my rock 5 itx patches, I get varying fan-speeds.
The fan starts high and then lowers its speed once the cpu-regulators
and every is set up.
While I was working on the dts and the cpu-supplies were not yet working,
the fan speed stayed high, so maybe check that frequency scaling actually
works? And of course you need the thermal map to handle the fan.
Also of course I don't see a rock5c patch anywhere, so where did that
board dts come from?
Heiko
> > Wiadomość napisana przez Alexey Charkov <alchark@gmail.com> w dniu 17.06.2024, o godz. 20:28:
> >
> > This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
> > active cooling on Radxa Rock 5B via the provided PWM fan.
> >
> > Some RK3588 boards use separate regulators to supply CPUs and their
> > respective memory interfaces, so this is handled by coupling those
> > regulators in affected boards' device trees to ensure that their
> > voltage is adjusted in step.
> >
> > This also enables the built-in thermal sensor (TSADC) for all boards
> > that don't currently have it enabled, using the default CRU based
> > emergency thermal reset. This default configuration only uses on-SoC
> > devices and doesn't rely on any external wiring, thus it should work
> > for all devices (tested only on Rock 5B though).
> >
> > The boards that have TSADC_SHUT signal wired to the PMIC reset line
> > can choose to override the default reset logic in favour of GPIO
> > driven (PMIC assisted) reset, but in my testing it didn't work on
> > Radxa Rock 5B - maybe I'm reading the schematic wrong and it doesn't
> > support PMIC assisted reset after all.
> >
> > Fan control on Rock 5B has been split into two intervals: let it spin
> > at the minimum cooling state between 55C and 65C, and then accelerate
> > if the system crosses the 65C mark - thanks to Dragan for suggesting.
> > This lets some cooling setups with beefier heatsinks and/or larger
> > fan fins to stay in the quietest non-zero fan state while still
> > gaining potential benefits from the airflow it generates, and
> > possibly avoiding noisy speeds altogether for some workloads.
> >
> > OPPs help actually scale CPU frequencies up and down for both cooling
> > and performance - tested on Rock 5B under varied loads. I've dropped
> > those OPPs that cause frequency reductions without accompanying decrease
> > in CPU voltage, as they don't seem to be adding much benefit in day to
> > day use, while the kernel log gets a number of "OPP is inefficient" lines.
> >
> > Note that this submission doesn't touch the SRAM read margin updates or
> > the OPP calibration based on silicon quality which the downstream driver
> > does and which were mentioned in [1]. It works as it is (also confirmed by
> > Sebastian in his follow-up message [2]), and it is stable in my testing on
> > Rock 5B, so it sounds better to merge a simple version first and then
> > extend when/if required.
> >
> > This patch series has been rebased on top of Heiko's recent for-next branch
> > with Dragan's patch [3] which rearranges the .dtsi files for per-variant OPPs.
> > As a result, it now includes separate CPU OPP tables for RK3588(s) and RK3588j.
> >
> > GPU OPPs have also been split out to accommodate for the difference in RK3588j.
> >
> > [1] https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/
> > [2] https://lore.kernel.org/linux-rockchip/pkyne4g2cln27dcdu3jm7bqdqpmd2kwkbguiolmozntjuiajrb@gvq4nupzna4o/
> > [3] https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
> >
> > Signed-off-by: Alexey Charkov <alchark@gmail.com>
> > ---
> > Changes in v5:
> > - Rebased against linux-rockchip/for-next with Dragan's .dtsi reshuffling on top
> > - Added separate OPP values for RK3588j (these also apply to RK3588m)
> > - Separated GPU OPP values for RK3588j (RK3588m ones differ slightly, not included here)
> > - Dragan's patch: https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
> > - Link to v4: https://lore.kernel.org/r/20240506-rk-dts-additions-v4-0-271023ddfd40@gmail.com
> >
> > Changes in v4:
> > - Rebased against linux-rockchip/for-next
> > - Reordered DT nodes alphabetically as pointed out by Diederik
> > - Moved the TSADC enablement to per-board .dts/.dtsi files
> > - Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
> > - Dropped second passive cooling trips altogether to keep things simple
> > - Added a cooling map for passive GPU cooling (in a separate patch)
> > - Link to v3: https://lore.kernel.org/r/20240229-rk-dts-additions-v3-0-6afe8473a631@gmail.com
> >
> > Changes in v3:
> > - Added regulator coupling for EVB1 and QuartzPro64
> > - Enabled the TSADC for all boards in .dtsi, not just Rock 5B (thanks ChenYu)
> > - Added comments regarding two passive cooling trips in each zone (thanks Dragan)
> > - Fixed active cooling map numbering for Radxa Rock 5B (thanks Dragan)
> > - Dropped Daniel's Acked-by tag from the Rock 5B fan patch, as there's been quite some
> > churn there since the version he acknowledged
> > - Link to v2: https://lore.kernel.org/r/20240130-rk-dts-additions-v2-0-c6222c4c78df@gmail.com
> >
> > Changes in v2:
> > - Dropped the rfkill patch which Heiko has already applied
> > - Set higher 'polling-delay-passive' (100 instead of 20)
> > - Name all cooling maps starting from map0 in each respective zone
> > - Drop 'contribution' properties from passive cooling maps
> > - Link to v1: https://lore.kernel.org/r/20240125-rk-dts-additions-v1-0-5879275db36f@gmail.com
> >
> > ---
> > Alexey Charkov (8):
> > arm64: dts: rockchip: add thermal zones information on RK3588
> > arm64: dts: rockchip: enable thermal management on all RK3588 boards
> > arm64: dts: rockchip: add passive GPU cooling on RK3588
> > arm64: dts: rockchip: enable automatic fan control on Rock 5B
> > arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
> > arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
> > arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
> > arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
> >
> > .../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
> > arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 197 +++++++++++++++++----
> > .../dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 +
> > arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 16 ++
> > arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
> > arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 190 ++++++++++++++++++++
> > .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++
> > arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 34 +++-
> > .../arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 +
> > .../arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 +
> > arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
> > arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 141 +++++++++++++++
> > arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 +
> > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
> > 14 files changed, 577 insertions(+), 39 deletions(-)
> > ---
> > base-commit: 5cc74606bf40a2bbaccd3e3bb2781f637baebde5
> > change-id: 20240124-rk-dts-additions-a6d7b52787b9
> >
> > Best regards,
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-07-07 11:11 ` Heiko Stübner
@ 2024-07-07 12:37 ` Piotr Oniszczuk
2024-07-07 18:32 ` Piotr Oniszczuk
0 siblings, 1 reply; 32+ messages in thread
From: Piotr Oniszczuk @ 2024-07-07 12:37 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Alexey Charkov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel,
open list:ARM/Rockchip SoC..., linux-kernel
Heiko,
pls see inline
> Wiadomość napisana przez Heiko Stübner <heiko@sntech.de> w dniu 07.07.2024, o godz. 13:11:
>
> Hey,
>
> Am Sonntag, 7. Juli 2024, 11:39:57 CEST schrieb Piotr Oniszczuk:
>> Alexey,
>> I’m playing with this series on rock5c on 6.10-rc6.
>>
>> Is code in this series enough to get working pwm-fan on rock5c?
>> (of course after adding required changes from rokc5b dts to rock5c dts)
>>
>> In my case i’m getting constantly full speed of fan on my rock5c.
>>
>> hw seems ok as echo 96 > /sys/devices/platform/pwm-fan/hwmon/hwmon0/pwm1 changes fans speed as expected.
>>
>> May you pls hint me what i’m missing here?
>
> at least on my rock 5 itx patches, I get varying fan-speeds.
> The fan starts high and then lowers its speed once the cpu-regulators
> and every is set up.
Ah - ok.
I verified and it looks there was typo from my side in dts fan stanza :-/
Now it works as expected :-)
Many thx for your time!
>
> While I was working on the dts and the cpu-supplies were not yet working,
> the fan speed stayed high, so maybe check that frequency scaling actually
> works?
> And of course you need the thermal map to handle the fan.
>
> Also of course I don't see a rock5c patch anywhere, so where did that
> board dts come from?
rock5c is my development: https://gist.github.com/warpme/6b2fa9004d8b28c0e43fa16b0b6595f3
>
>
> Heiko
>
>>> Wiadomość napisana przez Alexey Charkov <alchark@gmail.com> w dniu 17.06.2024, o godz. 20:28:
>>>
>>> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
>>> active cooling on Radxa Rock 5B via the provided PWM fan.
>>>
>>> Some RK3588 boards use separate regulators to supply CPUs and their
>>> respective memory interfaces, so this is handled by coupling those
>>> regulators in affected boards' device trees to ensure that their
>>> voltage is adjusted in step.
>>>
>>> This also enables the built-in thermal sensor (TSADC) for all boards
>>> that don't currently have it enabled, using the default CRU based
>>> emergency thermal reset. This default configuration only uses on-SoC
>>> devices and doesn't rely on any external wiring, thus it should work
>>> for all devices (tested only on Rock 5B though).
>>>
>>> The boards that have TSADC_SHUT signal wired to the PMIC reset line
>>> can choose to override the default reset logic in favour of GPIO
>>> driven (PMIC assisted) reset, but in my testing it didn't work on
>>> Radxa Rock 5B - maybe I'm reading the schematic wrong and it doesn't
>>> support PMIC assisted reset after all.
>>>
>>> Fan control on Rock 5B has been split into two intervals: let it spin
>>> at the minimum cooling state between 55C and 65C, and then accelerate
>>> if the system crosses the 65C mark - thanks to Dragan for suggesting.
>>> This lets some cooling setups with beefier heatsinks and/or larger
>>> fan fins to stay in the quietest non-zero fan state while still
>>> gaining potential benefits from the airflow it generates, and
>>> possibly avoiding noisy speeds altogether for some workloads.
>>>
>>> OPPs help actually scale CPU frequencies up and down for both cooling
>>> and performance - tested on Rock 5B under varied loads. I've dropped
>>> those OPPs that cause frequency reductions without accompanying decrease
>>> in CPU voltage, as they don't seem to be adding much benefit in day to
>>> day use, while the kernel log gets a number of "OPP is inefficient" lines.
>>>
>>> Note that this submission doesn't touch the SRAM read margin updates or
>>> the OPP calibration based on silicon quality which the downstream driver
>>> does and which were mentioned in [1]. It works as it is (also confirmed by
>>> Sebastian in his follow-up message [2]), and it is stable in my testing on
>>> Rock 5B, so it sounds better to merge a simple version first and then
>>> extend when/if required.
>>>
>>> This patch series has been rebased on top of Heiko's recent for-next branch
>>> with Dragan's patch [3] which rearranges the .dtsi files for per-variant OPPs.
>>> As a result, it now includes separate CPU OPP tables for RK3588(s) and RK3588j.
>>>
>>> GPU OPPs have also been split out to accommodate for the difference in RK3588j.
>>>
>>> [1] https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/
>>> [2] https://lore.kernel.org/linux-rockchip/pkyne4g2cln27dcdu3jm7bqdqpmd2kwkbguiolmozntjuiajrb@gvq4nupzna4o/
>>> [3] https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>>>
>>> Signed-off-by: Alexey Charkov <alchark@gmail.com>
>>> ---
>>> Changes in v5:
>>> - Rebased against linux-rockchip/for-next with Dragan's .dtsi reshuffling on top
>>> - Added separate OPP values for RK3588j (these also apply to RK3588m)
>>> - Separated GPU OPP values for RK3588j (RK3588m ones differ slightly, not included here)
>>> - Dragan's patch: https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>>> - Link to v4: https://lore.kernel.org/r/20240506-rk-dts-additions-v4-0-271023ddfd40@gmail.com
>>>
>>> Changes in v4:
>>> - Rebased against linux-rockchip/for-next
>>> - Reordered DT nodes alphabetically as pointed out by Diederik
>>> - Moved the TSADC enablement to per-board .dts/.dtsi files
>>> - Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
>>> - Dropped second passive cooling trips altogether to keep things simple
>>> - Added a cooling map for passive GPU cooling (in a separate patch)
>>> - Link to v3: https://lore.kernel.org/r/20240229-rk-dts-additions-v3-0-6afe8473a631@gmail.com
>>>
>>> Changes in v3:
>>> - Added regulator coupling for EVB1 and QuartzPro64
>>> - Enabled the TSADC for all boards in .dtsi, not just Rock 5B (thanks ChenYu)
>>> - Added comments regarding two passive cooling trips in each zone (thanks Dragan)
>>> - Fixed active cooling map numbering for Radxa Rock 5B (thanks Dragan)
>>> - Dropped Daniel's Acked-by tag from the Rock 5B fan patch, as there's been quite some
>>> churn there since the version he acknowledged
>>> - Link to v2: https://lore.kernel.org/r/20240130-rk-dts-additions-v2-0-c6222c4c78df@gmail.com
>>>
>>> Changes in v2:
>>> - Dropped the rfkill patch which Heiko has already applied
>>> - Set higher 'polling-delay-passive' (100 instead of 20)
>>> - Name all cooling maps starting from map0 in each respective zone
>>> - Drop 'contribution' properties from passive cooling maps
>>> - Link to v1: https://lore.kernel.org/r/20240125-rk-dts-additions-v1-0-5879275db36f@gmail.com
>>>
>>> ---
>>> Alexey Charkov (8):
>>> arm64: dts: rockchip: add thermal zones information on RK3588
>>> arm64: dts: rockchip: enable thermal management on all RK3588 boards
>>> arm64: dts: rockchip: add passive GPU cooling on RK3588
>>> arm64: dts: rockchip: enable automatic fan control on Rock 5B
>>> arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
>>> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
>>> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
>>> arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
>>>
>>> .../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
>>> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 197 +++++++++++++++++----
>>> .../dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 +
>>> arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 16 ++
>>> arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
>>> arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 190 ++++++++++++++++++++
>>> .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++
>>> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 34 +++-
>>> .../arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 +
>>> .../arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 +
>>> arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
>>> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 141 +++++++++++++++
>>> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 +
>>> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
>>> 14 files changed, 577 insertions(+), 39 deletions(-)
>>> ---
>>> base-commit: 5cc74606bf40a2bbaccd3e3bb2781f637baebde5
>>> change-id: 20240124-rk-dts-additions-a6d7b52787b9
>>>
>>> Best regards,
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-07-07 12:37 ` Piotr Oniszczuk
@ 2024-07-07 18:32 ` Piotr Oniszczuk
2024-07-08 7:59 ` Alexey Charkov
0 siblings, 1 reply; 32+ messages in thread
From: Piotr Oniszczuk @ 2024-07-07 18:32 UTC (permalink / raw)
To: Heiko Stuebner, Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Lezcano,
Dragan Simic, Viresh Kumar, Chen-Yu Tsai, Diederik de Haas,
devicetree, linux-arm-kernel, open list:ARM/Rockchip SoC...,
linux-kernel
Heiko, Alexey,
After some more tests: is varying fan-speeds working stable for you?
In my case - 1 per few reboots results with board enters state with: constant full speed and no any reaction for cpu temp.
In such state - I need multiple hw poweroffs (remove usb-c plug) to get fan-speeds working again.
When board is such state - all seems to work ok (frequency scaling, etc) except fan is constantly full speed…
Is it varying fan-speed working stable for you?
Maybe my issue is some kernel modules loading race?
> Wiadomość napisana przez Piotr Oniszczuk <piotr.oniszczuk@gmail.com> w dniu 07.07.2024, o godz. 14:37:
>
> Heiko,
> pls see inline
>
>> Wiadomość napisana przez Heiko Stübner <heiko@sntech.de> w dniu 07.07.2024, o godz. 13:11:
>>
>> Hey,
>>
>> Am Sonntag, 7. Juli 2024, 11:39:57 CEST schrieb Piotr Oniszczuk:
>>> Alexey,
>>> I’m playing with this series on rock5c on 6.10-rc6.
>>>
>>> Is code in this series enough to get working pwm-fan on rock5c?
>>> (of course after adding required changes from rokc5b dts to rock5c dts)
>>>
>>> In my case i’m getting constantly full speed of fan on my rock5c.
>>>
>>> hw seems ok as echo 96 > /sys/devices/platform/pwm-fan/hwmon/hwmon0/pwm1 changes fans speed as expected.
>>>
>>> May you pls hint me what i’m missing here?
>>
>> at least on my rock 5 itx patches, I get varying fan-speeds.
>> The fan starts high and then lowers its speed once the cpu-regulators
>> and every is set up.
>
> Ah - ok.
> I verified and it looks there was typo from my side in dts fan stanza :-/
> Now it works as expected :-)
>
> Many thx for your time!
>
>>
>> While I was working on the dts and the cpu-supplies were not yet working,
>> the fan speed stayed high, so maybe check that frequency scaling actually
>> works?
>> And of course you need the thermal map to handle the fan.
>
>>
>> Also of course I don't see a rock5c patch anywhere, so where did that
>> board dts come from?
>
> rock5c is my development: https://gist.github.com/warpme/6b2fa9004d8b28c0e43fa16b0b6595f3
>
>>
>>
>> Heiko
>>
>>>> Wiadomość napisana przez Alexey Charkov <alchark@gmail.com> w dniu 17.06.2024, o godz. 20:28:
>>>>
>>>> This enables thermal monitoring and CPU DVFS on RK3588(s), as well as
>>>> active cooling on Radxa Rock 5B via the provided PWM fan.
>>>>
>>>> Some RK3588 boards use separate regulators to supply CPUs and their
>>>> respective memory interfaces, so this is handled by coupling those
>>>> regulators in affected boards' device trees to ensure that their
>>>> voltage is adjusted in step.
>>>>
>>>> This also enables the built-in thermal sensor (TSADC) for all boards
>>>> that don't currently have it enabled, using the default CRU based
>>>> emergency thermal reset. This default configuration only uses on-SoC
>>>> devices and doesn't rely on any external wiring, thus it should work
>>>> for all devices (tested only on Rock 5B though).
>>>>
>>>> The boards that have TSADC_SHUT signal wired to the PMIC reset line
>>>> can choose to override the default reset logic in favour of GPIO
>>>> driven (PMIC assisted) reset, but in my testing it didn't work on
>>>> Radxa Rock 5B - maybe I'm reading the schematic wrong and it doesn't
>>>> support PMIC assisted reset after all.
>>>>
>>>> Fan control on Rock 5B has been split into two intervals: let it spin
>>>> at the minimum cooling state between 55C and 65C, and then accelerate
>>>> if the system crosses the 65C mark - thanks to Dragan for suggesting.
>>>> This lets some cooling setups with beefier heatsinks and/or larger
>>>> fan fins to stay in the quietest non-zero fan state while still
>>>> gaining potential benefits from the airflow it generates, and
>>>> possibly avoiding noisy speeds altogether for some workloads.
>>>>
>>>> OPPs help actually scale CPU frequencies up and down for both cooling
>>>> and performance - tested on Rock 5B under varied loads. I've dropped
>>>> those OPPs that cause frequency reductions without accompanying decrease
>>>> in CPU voltage, as they don't seem to be adding much benefit in day to
>>>> day use, while the kernel log gets a number of "OPP is inefficient" lines.
>>>>
>>>> Note that this submission doesn't touch the SRAM read margin updates or
>>>> the OPP calibration based on silicon quality which the downstream driver
>>>> does and which were mentioned in [1]. It works as it is (also confirmed by
>>>> Sebastian in his follow-up message [2]), and it is stable in my testing on
>>>> Rock 5B, so it sounds better to merge a simple version first and then
>>>> extend when/if required.
>>>>
>>>> This patch series has been rebased on top of Heiko's recent for-next branch
>>>> with Dragan's patch [3] which rearranges the .dtsi files for per-variant OPPs.
>>>> As a result, it now includes separate CPU OPP tables for RK3588(s) and RK3588j.
>>>>
>>>> GPU OPPs have also been split out to accommodate for the difference in RK3588j.
>>>>
>>>> [1] https://lore.kernel.org/linux-rockchip/CABjd4YzTL=5S7cS8ACNAYVa730WA3iGd5L_wP1Vn9=f83RCORA@mail.gmail.com/
>>>> [2] https://lore.kernel.org/linux-rockchip/pkyne4g2cln27dcdu3jm7bqdqpmd2kwkbguiolmozntjuiajrb@gvq4nupzna4o/
>>>> [3] https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>>>>
>>>> Signed-off-by: Alexey Charkov <alchark@gmail.com>
>>>> ---
>>>> Changes in v5:
>>>> - Rebased against linux-rockchip/for-next with Dragan's .dtsi reshuffling on top
>>>> - Added separate OPP values for RK3588j (these also apply to RK3588m)
>>>> - Separated GPU OPP values for RK3588j (RK3588m ones differ slightly, not included here)
>>>> - Dragan's patch: https://lore.kernel.org/linux-rockchip/9ffedc0e2ca7f167d9d795b2a8f43cb9f56a653b.1717923308.git.dsimic@manjaro.org/
>>>> - Link to v4: https://lore.kernel.org/r/20240506-rk-dts-additions-v4-0-271023ddfd40@gmail.com
>>>>
>>>> Changes in v4:
>>>> - Rebased against linux-rockchip/for-next
>>>> - Reordered DT nodes alphabetically as pointed out by Diederik
>>>> - Moved the TSADC enablement to per-board .dts/.dtsi files
>>>> - Dropped extra "inefficient" OPPs (same voltage - lower frequencies)
>>>> - Dropped second passive cooling trips altogether to keep things simple
>>>> - Added a cooling map for passive GPU cooling (in a separate patch)
>>>> - Link to v3: https://lore.kernel.org/r/20240229-rk-dts-additions-v3-0-6afe8473a631@gmail.com
>>>>
>>>> Changes in v3:
>>>> - Added regulator coupling for EVB1 and QuartzPro64
>>>> - Enabled the TSADC for all boards in .dtsi, not just Rock 5B (thanks ChenYu)
>>>> - Added comments regarding two passive cooling trips in each zone (thanks Dragan)
>>>> - Fixed active cooling map numbering for Radxa Rock 5B (thanks Dragan)
>>>> - Dropped Daniel's Acked-by tag from the Rock 5B fan patch, as there's been quite some
>>>> churn there since the version he acknowledged
>>>> - Link to v2: https://lore.kernel.org/r/20240130-rk-dts-additions-v2-0-c6222c4c78df@gmail.com
>>>>
>>>> Changes in v2:
>>>> - Dropped the rfkill patch which Heiko has already applied
>>>> - Set higher 'polling-delay-passive' (100 instead of 20)
>>>> - Name all cooling maps starting from map0 in each respective zone
>>>> - Drop 'contribution' properties from passive cooling maps
>>>> - Link to v1: https://lore.kernel.org/r/20240125-rk-dts-additions-v1-0-5879275db36f@gmail.com
>>>>
>>>> ---
>>>> Alexey Charkov (8):
>>>> arm64: dts: rockchip: add thermal zones information on RK3588
>>>> arm64: dts: rockchip: enable thermal management on all RK3588 boards
>>>> arm64: dts: rockchip: add passive GPU cooling on RK3588
>>>> arm64: dts: rockchip: enable automatic fan control on Rock 5B
>>>> arm64: dts: rockchip: Add CPU/memory regulator coupling for RK3588
>>>> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588
>>>> arm64: dts: rockchip: Add OPP data for CPU cores on RK3588j
>>>> arm64: dts: rockchip: Split GPU OPPs of RK3588 and RK3588j
>>>>
>>>> .../boot/dts/rockchip/rk3588-armsom-sige7.dts | 4 +
>>>> arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 197 +++++++++++++++++----
>>>> .../dts/rockchip/rk3588-edgeble-neu6a-common.dtsi | 4 +
>>>> arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 16 ++
>>>> arch/arm64/boot/dts/rockchip/rk3588-ok3588-c.dts | 4 +
>>>> arch/arm64/boot/dts/rockchip/rk3588-opp.dtsi | 190 ++++++++++++++++++++
>>>> .../arm64/boot/dts/rockchip/rk3588-quartzpro64.dts | 12 ++
>>>> arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 34 +++-
>>>> .../arm64/boot/dts/rockchip/rk3588-toybrick-x0.dts | 4 +
>>>> .../arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 4 +
>>>> arch/arm64/boot/dts/rockchip/rk3588.dtsi | 1 +
>>>> arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 141 +++++++++++++++
>>>> arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 4 +
>>>> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 1 +
>>>> 14 files changed, 577 insertions(+), 39 deletions(-)
>>>> ---
>>>> base-commit: 5cc74606bf40a2bbaccd3e3bb2781f637baebde5
>>>> change-id: 20240124-rk-dts-additions-a6d7b52787b9
>>>>
>>>> Best regards,
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-07-07 18:32 ` Piotr Oniszczuk
@ 2024-07-08 7:59 ` Alexey Charkov
2024-07-08 10:29 ` Piotr Oniszczuk
0 siblings, 1 reply; 32+ messages in thread
From: Alexey Charkov @ 2024-07-08 7:59 UTC (permalink / raw)
To: Piotr Oniszczuk
Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel,
open list:ARM/Rockchip SoC..., linux-kernel
Hi Piotr,
On Sun, Jul 7, 2024 at 9:32 PM Piotr Oniszczuk
<piotr.oniszczuk@gmail.com> wrote:
>
> Heiko, Alexey,
>
> After some more tests: is varying fan-speeds working stable for you?
Yes, in my testing on Rock 5B it's been stable.
> In my case - 1 per few reboots results with board enters state with: constant full speed and no any reaction for cpu temp.
> In such state - I need multiple hw poweroffs (remove usb-c plug) to get fan-speeds working again.
> When board is such state - all seems to work ok (frequency scaling, etc) except fan is constantly full speed…
One thought: could you please check which thermal governor gets
loaded? I used stepwise in my testing.
Best regards,
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 0/8] RK3588 and Rock 5B dts additions: thermal, OPP and fan
2024-07-08 7:59 ` Alexey Charkov
@ 2024-07-08 10:29 ` Piotr Oniszczuk
0 siblings, 0 replies; 32+ messages in thread
From: Piotr Oniszczuk @ 2024-07-08 10:29 UTC (permalink / raw)
To: Alexey Charkov
Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Daniel Lezcano, Dragan Simic, Viresh Kumar, Chen-Yu Tsai,
Diederik de Haas, devicetree, linux-arm-kernel,
open list:ARM/Rockchip SoC..., linux-kernel
Alexey,
pls see inline
> Wiadomość napisana przez Alexey Charkov <alchark@gmail.com> w dniu 08.07.2024, o godz. 09:59:
>
> Hi Piotr,
>
> On Sun, Jul 7, 2024 at 9:32 PM Piotr Oniszczuk
> <piotr.oniszczuk@gmail.com> wrote:
>>
>> Heiko, Alexey,
>>
>> After some more tests: is varying fan-speeds working stable for you?
>
> Yes, in my testing on Rock 5B it's been stable.
>
>> In my case - 1 per few reboots results with board enters state with: constant full speed and no any reaction for cpu temp.
>> In such state - I need multiple hw poweroffs (remove usb-c plug) to get fan-speeds working again.
>> When board is such state - all seems to work ok (frequency scaling, etc) except fan is constantly full speed…
>
> One thought: could you please check which thermal governor gets
> loaded? I used stepwise in my testing.
>
this is from system when - after boot - i have constant full speed of fan
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone0/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone1/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone2/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone3/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone4/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone5/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone6/policy
step_wise
root@myth-frontend-3614ae04f23f:~ # cat /sys/class/thermal/thermal_zone7/policy
dmesg: https://gist.github.com/warpme/5d12df382ce353205c6ff0c37f5b4791
lsmod: https://gist.github.com/warpme/1c74b3be2cabe85366f227594d8a3e90
pls let me know is there anything else i can provide to investigate this issue...
^ permalink raw reply [flat|nested] 32+ messages in thread