Re: [PATCH 1/2] arm64: dts: rockchip: Fix multiple thermal zones conflict in rk3399.dtsi

[Robin Murphy]

On 14/06/2019 14:03, Daniel Lezcano wrote:
> On 14/06/2019 11:35, Heiko Stuebner wrote:
> > Hi Daniel,
> >
> > Am Dienstag, 4. Juni 2019, 18:57:57 CEST schrieb Daniel Lezcano:
> > > Currently the common thermal zones definitions for the rk3399 assumes
> > > multiple thermal zones are supported by the governors. This is not the
> > > case and each thermal zone has its own governor instance acting
> > > individually without collaboration with other governors.
> > >
> > > As the cooling device for the CPU and the GPU thermal zones is the
> > > same, each governors take different decisions for the same cooling
> > > device leading to conflicting instructions and an erratic behavior.
> > >
> > > As the cooling-maps is about to become an optional property, let's
> > > remove the cpu cooling device map from the GPU thermal zone.
> > >
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > > ---
> > >   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 ---------
> > >   1 file changed, 9 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > index 196ac9b78076..e1357e0f60f7 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > @@ -821,15 +821,6 @@
> > >   					type = "critical";
> > >   				};
> > >   			};
> > > -
> > > -			cooling-maps {
> > > -				map0 {
> > > -					trip = <&gpu_alert0>;
> > > -					cooling-device =
> > > -						<&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > -						<&cpu_b1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > -				};
> > > -			};
> > >   		};
> > >   	};
> >
> > my knowledge of the thermal framework is not that big, but what about the
> > rk3399-devices which further detail the cooling-maps like rk3399-gru-kevin
> > and the rk3399-nanopc-t4 with its fan-handling in the cooling-maps?
>
> The rk3399-gru-kevin is correct.
>
> The rk3399-nanopc-t4 is not correct because the cpu and the gpu are
> sharing the same cooling device (the fan). There are different
> configurations:
>
> 1. The cpu cooling device for the CPU and the fan for the GPU
>
> 2. Different trip points on the CPU thermal zone, eg. one to for the CPU
> cooling device and another one for the fan.
>
> There are some variant for the above. If this board is not on battery,
> you may want to give priority to the throughput, so activate the fan
> first and then cool down the CPU. Or if you are on battery, you may want
> to invert the trip points.
>
> In any case, it is not possible to share the same cooling device for
> different thermal zones.

OK, thanks for the clarification. I'll get my board set up again to 
figure out the best fix for rk3399-nanopc-t4 (FWIW most users are 
probably just using passive cooling or a plain DC fan anyway). You might 
want to raise this issue with the maintainers of 
arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi, since the 
everything-shared-by-everything approach in there was what I used as a 
reference.

Robin.