Aw: Re: [PATCH] arm: dts: mt7623: increase passive cooling trip

[Frank Wunderlich]

Hi Kurt,

> Gesendet: Samstag, 05. März 2022 um 16:54 Uhr
> Von: "Kurt Fitzner" <kurt at va1der.ca>

> This issue and patch just came to my attention, my apologies for late 
> comments.
> 
> The solution provided in this patch is, I believe, not optimal.

does your Patch fixes the issue? if yes you can send a Patch based on current mainline dts.

it looks a bit weired to me having active before passive...but i'm no expert in the cooling trips. 
i just increased the temps :)

please see notes below

> On 2021-07-25 1:34 p.m., Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w at public-files.de>
> > 
> > MT7623/BPI-R2 has idle temperature after bootup from 48 degrees celsius
> > increase the passive trip temp threshold to not trottle CPU frequency at
> > this temperature
> 
> The dts file for the mt7623 has four trip points:
> - Trip point 0, Type=Passive @ 47°C (originally, now 57°C)
> - Trip point 1, Type=Active @ 67°C
> - Trip point 2, Type=Hot @ 87°C
> - Trip point 3, Type=Critical @ 107°C
> 
> It makes little sense to have passive CPU throttling employed *before* 
> engaging a fan.  Generally users want a fan (if present) to be engaged 
> before intentional performance degradation is employed.
> 
> >   					cpu_passive: cpu-passive {
> > -						temperature = <47000>;
> > +						temperature = <57000>;
> 
> The old temperature of 47°C appears, in practice, to be a good 
> temperature for an active cooling trip point.  It is just above the 
> temperature that a generally idle mt7623 sits at in enclosures where 
> this SoC is employed, but also a temperature that a CPU under load will 
> quickly reach.
> 
> Therefore I propose the types of trip points 0 and 1 be reversed and the 
> temperatures left as they were.
> 
> 
> --- a/arch/arm/boot/dts/mt7623.dtsi	2022-02-27 18:36:33 -0400
> +++ b/arch/arm/boot/dts/mt7623.dtsi	2022-03-05 11:37:52 -0400
> @@ -158,19 +158,19 @@
> 
>   				thermal-sensors = <&thermal 0>;
> 
>   				trips {
>   					cpu_passive: cpu-passive {
> -						temperature = <57000>;
> +						temperature = <47000>;
>   						hysteresis = <2000>;
> -						type = "passive";
> +						type = "active";
>   					};

if type is different, imho the name+label should be changed too

>   					cpu_active: cpu-active {
>   						temperature = <67000>;
>   						hysteresis = <2000>;
> -						type = "active";
> +						type = "passive";
>   					};

dito

>   					cpu_hot: cpu-hot {
>   						temperature = <87000>;
>   						hysteresis = <2000>;

regards Frank