2024-01-01 00:00:19

by Alexey Klimov

[permalink] [raw]
Subject: [PATCH RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU

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 <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
Signed-off-by: Alexey Klimov <[email protected]>
---
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";
+ };
+ };
};
};

--
2.40.1



2024-01-03 01:41:34

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCH RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU

在 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 <[email protected]>
> Cc: Jernej Skrabec <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
>  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";
> +                               };
> +                       };
>                 };
>         };
>  

2024-01-09 20:13:46

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU

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 <[email protected]>
> Cc: Jernej Skrabec <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> 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";
> + };
> + };
> };
> };
>
>





2024-01-12 16:33:03

by Andre Przywara

[permalink] [raw]
Subject: Re: [PATCH RESEND] arm64: dts: allwinner: a64: Add thermal trip points for GPU

On Mon, 1 Jan 2024 00:00:08 +0000
Alexey Klimov <[email protected]> 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 <[email protected]>
> Cc: Jernej Skrabec <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
> Signed-off-by: Alexey Klimov <[email protected]>
> ---
> 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";
> + };
> + };
> };
> };
>