diff mbox series

[RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU

Message ID 20240101000008.65747-1-alexey.klimov@linaro.org
State New
Headers show
Series [RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU | expand

Commit Message

Alexey Klimov Jan. 1, 2024, midnight UTC
Without trip points for GPU, the following errors are printed in the
dmesg log and the sun8i-thermal driver fails to load:

thermal_sys: Failed to find 'trips' node
thermal_sys: Failed to find trip points for thermal-sensor id=1
sun8i-thermal: probe of 1c25000.thermal-sensor failed with error -22

When thermal zones are defined, trip points definitions are mandatory.
Trip values for the GPU are assumed to be the same values as the CPU
ones. The available specs do not provide any hints about thermal regimes
for the GPU and it seems GPU is implemented on the same die as the CPU.

Tested on Pine a64+.

Cc: Samuel Holland <samuel@sholland.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

Comments

Icenowy Zheng Jan. 3, 2024, 1:40 a.m. UTC | #1
在 2024-01-01星期一的 00:00 +0000,Alexey Klimov写道:
> Without trip points for GPU, the following errors are printed in the
> dmesg log and the sun8i-thermal driver fails to load:
> 
> thermal_sys: Failed to find 'trips' node
> thermal_sys: Failed to find trip points for thermal-sensor id=1
> sun8i-thermal: probe of 1c25000.thermal-sensor failed with error -22
> 
> When thermal zones are defined, trip points definitions are
> mandatory.

I suggest everyone seeing this patch have a look on my patch at [1],
which makes trip points not so mandatory -- it's at least once an
allowed behavior, but lost when converting DT binding to YAML.

[1]
https://lists.infradead.org/pipermail/linux-arm-kernel/2023-July/852088.html

> Trip values for the GPU are assumed to be the same values as the CPU
> ones. The available specs do not provide any hints about thermal
> regimes
> for the GPU and it seems GPU is implemented on the same die as the
> CPU.
> 
> Tested on Pine a64+.
> 
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 46
> +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 62f45f71ec65..07963eea1bf0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -243,6 +243,29 @@ gpu0_thermal: gpu0-thermal {
>                         polling-delay-passive = <0>;
>                         polling-delay = <0>;
>                         thermal-sensors = <&ths 1>;
> +
> +                       trips {
> +                               gpu0_alert0: gpu0_alert0 {
> +                                       /* milliCelsius */
> +                                       temperature = <75000>;
> +                                       hysteresis = <2000>;
> +                                       type = "passive";
> +                               };
> +
> +                               gpu0_alert1: gpu0_alert1 {
> +                                       /* milliCelsius */
> +                                       temperature = <90000>;
> +                                       hysteresis = <2000>;
> +                                       type = "hot";
> +                               };
> +
> +                               gpu0_crit: gpu0_crit {
> +                                       /* milliCelsius */
> +                                       temperature = <110000>;
> +                                       hysteresis = <2000>;
> +                                       type = "critical";
> +                               };
> +                       };
>                 };
>  
>                 gpu1_thermal: gpu1-thermal {
> @@ -250,6 +273,29 @@ gpu1_thermal: gpu1-thermal {
>                         polling-delay-passive = <0>;
>                         polling-delay = <0>;
>                         thermal-sensors = <&ths 2>;
> +
> +                       trips {
> +                               gpu1_alert0: gpu1_alert0 {
> +                                       /* milliCelsius */
> +                                       temperature = <75000>;
> +                                       hysteresis = <2000>;
> +                                       type = "passive";
> +                               };
> +
> +                               gpu1_alert1: gpu1_alert1 {
> +                                       /* milliCelsius */
> +                                       temperature = <90000>;
> +                                       hysteresis = <2000>;
> +                                       type = "hot";
> +                               };
> +
> +                               gpu1_crit: gpu1_crit {
> +                                       /* milliCelsius */
> +                                       temperature = <110000>;
> +                                       hysteresis = <2000>;
> +                                       type = "critical";
> +                               };
> +                       };
>                 };
>         };
>
Jernej Škrabec Jan. 9, 2024, 8:13 p.m. UTC | #2
Hi Alexey!

Dne ponedeljek, 01. januar 2024 ob 01:00:08 CET je Alexey Klimov napisal(a):
> Without trip points for GPU, the following errors are printed in the
> dmesg log and the sun8i-thermal driver fails to load:
> 
> thermal_sys: Failed to find 'trips' node
> thermal_sys: Failed to find trip points for thermal-sensor id=1
> sun8i-thermal: probe of 1c25000.thermal-sensor failed with error -22

Please no Linux specific talk. Since DT is OS neutral let just talk about HW.

> 
> When thermal zones are defined, trip points definitions are mandatory.
> Trip values for the GPU are assumed to be the same values as the CPU
> ones. The available specs do not provide any hints about thermal regimes
> for the GPU and it seems GPU is implemented on the same die as the CPU.
> 
> Tested on Pine a64+.
> 
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 62f45f71ec65..07963eea1bf0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -243,6 +243,29 @@ gpu0_thermal: gpu0-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 1>;
> +
> +			trips {
> +				gpu0_alert0: gpu0_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};

Since GPU has OPP, can you add cooling maps with at least first trip point?

Best regards,
Jernej

> +
> +				gpu0_alert1: gpu0_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				gpu0_crit: gpu0_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};
>  
>  		gpu1_thermal: gpu1-thermal {
> @@ -250,6 +273,29 @@ gpu1_thermal: gpu1-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 2>;
> +
> +			trips {
> +				gpu1_alert0: gpu1_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				gpu1_alert1: gpu1_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				gpu1_crit: gpu1_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};
>  	};
>  
>
Andre Przywara Jan. 12, 2024, 4:32 p.m. UTC | #3
On Mon,  1 Jan 2024 00:00:08 +0000
Alexey Klimov <alexey.klimov@linaro.org> wrote:

Hi Alexey,

> Without trip points for GPU, the following errors are printed in the
> dmesg log and the sun8i-thermal driver fails to load:
> 
> thermal_sys: Failed to find 'trips' node
> thermal_sys: Failed to find trip points for thermal-sensor id=1
> sun8i-thermal: probe of 1c25000.thermal-sensor failed with error -22

Regardless of whether we should really *require* trip points (what Icenowy
wanted to fix), I think it's good to have those values in the DT.

The only question I have: where do those values come from? Is this coming
from some BSP, or some downstream repository? If there are multiple
sources: are the values across them consistent?
I have seen a lot careless and unreflecting copy&paste in the past, so
just want to make sure we get the right values.

Cheers,
Andre

> When thermal zones are defined, trip points definitions are mandatory.
> Trip values for the GPU are assumed to be the same values as the CPU
> ones. The available specs do not provide any hints about thermal regimes
> for the GPU and it seems GPU is implemented on the same die as the CPU.
> 
> Tested on Pine a64+.
> 
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 62f45f71ec65..07963eea1bf0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -243,6 +243,29 @@ gpu0_thermal: gpu0-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 1>;
> +
> +			trips {
> +				gpu0_alert0: gpu0_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				gpu0_alert1: gpu0_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				gpu0_crit: gpu0_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};
>  
>  		gpu1_thermal: gpu1-thermal {
> @@ -250,6 +273,29 @@ gpu1_thermal: gpu1-thermal {
>  			polling-delay-passive = <0>;
>  			polling-delay = <0>;
>  			thermal-sensors = <&ths 2>;
> +
> +			trips {
> +				gpu1_alert0: gpu1_alert0 {
> +					/* milliCelsius */
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				gpu1_alert1: gpu1_alert1 {
> +					/* milliCelsius */
> +					temperature = <90000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				gpu1_crit: gpu1_crit {
> +					/* milliCelsius */
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
>  		};
>  	};
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 62f45f71ec65..07963eea1bf0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -243,6 +243,29 @@  gpu0_thermal: gpu0-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			thermal-sensors = <&ths 1>;
+
+			trips {
+				gpu0_alert0: gpu0_alert0 {
+					/* milliCelsius */
+					temperature = <75000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				gpu0_alert1: gpu0_alert1 {
+					/* milliCelsius */
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "hot";
+				};
+
+				gpu0_crit: gpu0_crit {
+					/* milliCelsius */
+					temperature = <110000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
 		};
 
 		gpu1_thermal: gpu1-thermal {
@@ -250,6 +273,29 @@  gpu1_thermal: gpu1-thermal {
 			polling-delay-passive = <0>;
 			polling-delay = <0>;
 			thermal-sensors = <&ths 2>;
+
+			trips {
+				gpu1_alert0: gpu1_alert0 {
+					/* milliCelsius */
+					temperature = <75000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				gpu1_alert1: gpu1_alert1 {
+					/* milliCelsius */
+					temperature = <90000>;
+					hysteresis = <2000>;
+					type = "hot";
+				};
+
+				gpu1_crit: gpu1_crit {
+					/* milliCelsius */
+					temperature = <110000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
 		};
 	};