Received-SPF: pass (google.com: domain of linux-kernel+bounces-33147-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240106222357.23835-1-alchark@gmail.com> <e0302da12345e5539583b2c96d747592@manjaro.org>
 <CABjd4Yw5wTLyK5OPw2S-ipPVCw7RTUeF2J0RgH-Vyis-ng8rTw@mail.gmail.com>
 <d7f2f25071a4d7c72bd286b11836dce7@manjaro.org> <CABjd4Yz11D8ThcT-oCWsQf9jL2idChFYSRYVVu3KNnzwoOwkKQ@mail.gmail.com>
 <f5c05015e042b11a51a9af26c35f18ed@manjaro.org> <CABjd4Yy91MAd2wALp4KQiEub9OyxU+MR+ti5KA_c+yvZT5xaqQ@mail.gmail.com>
 <81a5410c3dbedbd4fe9ce60ab236700c@manjaro.org> <CABjd4YwxoPrKRPmjn-eWsU1N5fXCjAbnRwfKD9DLKgeL2=vHrA@mail.gmail.com>
 <a15edb98c32ca79b75bd4eaf64734561@manjaro.org>
In-Reply-To: <a15edb98c32ca79b75bd4eaf64734561@manjaro.org>
From: Alexey Charkov <alchark@gmail.com>
Date: Mon, 22 Jan 2024 18:20:54 +0400
Message-ID: <CABjd4Yxw_wgjRRO12Zjy75=CA_uuucexVnhVRt3fURJFYQqYSw@mail.gmail.com>
Subject: Re: [PATCH] arm64: dts: rockchip: enable built-in thermal monitoring
 on rk3588
To: Dragan Simic <dsimic@manjaro.org>
Cc: Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Heiko Stuebner <heiko@sntech.de>, Sebastian Reichel <sebastian.reichel@collabora.com>, 
	Cristian Ciocaltea <cristian.ciocaltea@collabora.com>, 
	Christopher Obbard <chris.obbard@collabora.com>, =?UTF-8?B?VGFtw6FzIFN6xbFjcw==?= <szucst@iit.uni-miskolc.hu>, 
	Shreeya Patel <shreeya.patel@collabora.com>, Kever Yang <kever.yang@rock-chips.com>, 
	Chris Morgan <macromorgan@hotmail.com>, devicetree@vger.kernel.org, 
	linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 22, 2024 at 11:57=E2=80=AFAM Dragan Simic <dsimic@manjaro.org> =
wrote:
>
> On 2024-01-22 08:36, Alexey Charkov wrote:
> > On Mon, Jan 22, 2024 at 10:22=E2=80=AFAM Dragan Simic <dsimic@manjaro.o=
rg>
> > wrote:
> >> On 2024-01-22 07:03, Alexey Charkov wrote:
> >> > On Mon, Jan 22, 2024 at 8:55=E2=80=AFAM Dragan Simic <dsimic@manjaro=
org> wrote:
> >> >> On 2024-01-21 19:56, Alexey Charkov wrote:
> >> >> > I think I'll also add a board-specific active cooling mechanism o=
n the
> >> >> > package level in the next iteration, given that Rock 5B has a PWM=
 fan
> >> >> > defined as a cooling device. That will go in the separate patch t=
hat
> >> >> > updates rk3588-rock-5b.dts (your feedback to v2 of this patch is =
also
> >> >> > duly noted, thank you!)
> >> >>
> >> >> Great, thanks.  Sure, making use of the Rock 5B's support for attac=
hing
> >> >> a PWM-controlled cooling fan is the way to go.
> >> >>
> >> >> Just to reiterate a bit, any "active" trip points belong to the boa=
rd
> >> >> dts file(s), because having a cooling fan is a board-specific featu=
re.
> >> >> As a note, you may also want to have a look at the RockPro64 dts(i)
> >> >> files, for example;  the RockPro64 also comes with a cooling fan
> >> >> connector and the associated PWM fan control logic.
> >> >
> >> > Thanks for the pointer! There is also a helpful doc within devicetre=
e
> >> > bindings descriptions, although it sits under hwmon which was a bit
> >> > confusing to me. I've already tested it locally (by adding to the
> >> > board dts), and it spins up and down quite nicely, and even modulate=
s
> >> > the fan speed swiftly when the load changes - yay!
> >>
> >> Nice!  Also, isn't it like magic? :)  To me, turning LEDs on/off and
> >> controlling fans acts as some kind of a "bridge" between the virtual
> >> and the real world. :)
> >
> > Oh yes! I also keep admiring how one can add just a couple of lines of
> > text here and there that's not even real code, and the whole kernel
> > machinery starts crunching numbers, analyzing temperatures, running
> > PID loops, etc etc so that I could enjoy the satisfying whistle of a
> > small fan when I type `make -j8` :-D
>
> Yes, it's very satisfying, :) and it also demonstrates the true power
> of the device trees as hardware definitions.  Just a few more lines and
> the cooling works! :)
>
> >> As a suggestion, it would be good to test with a couple of different
> >> fans, to make sure that the PWM values work well for more that one fan
> >> model.  The Rock 5B requires a 5 V fan, if I'm not mistaken?
> >
> > It is 5V, yes. I only have one fan to try though, and I simply relied
> > on the PWM values that are already defined in the upstream
> > rk3588-rock-5b.dts. They don't look ideal for my particular fan,
> > because the lowest non-zero cooling state currently uses a PWM value
> > of 95, which doesn't always make it spin up. But in the end it doesn't
> > seem to matter that much, because that tiny fan needs to spin at full
> > 255 whenever all eight cores are loaded (and even then it can only
> > balance the temperature at around 60.5=D0=A1), and when the load is lig=
hter
> > (such as during various ./configure runs) it just switches off
> > completely as the temperature goes down to 46C even with the fan not
> > spinning.
>
> I see, 5 V fans unfortunately aren't very common.  I'm not sure why
> Radxa opted for 5 V there;  maybe the goal was to use Raspberry Pi 5 V
> fans, but using those tiny fans doesn't make much sense, IMHO.
>
> I think you can freely adjust the PWM values a bit to make your fan
> start reliably at the lowest state, regardless of how rarely that state
> will be used.  See, if your fan doesn't spin up reliably with the
> current
> lowest state, chances for other fan models not to spin up are quite
> high.
> IOW, it's better to play safe there, if you agree.
>
> What kind of heatsink are you using with your Rock 5B?  Ah yes, and
> what's the actual model of the fan you're using?

I use Radxa's 4012 heatsink-fan assembly that comes as an add-on
option when buying the board itself from Allnet. I guess I'll include
slightly adjusted PWM values in the rk3588-rock-5b.dts patch to better
represent my fan's "preferred" range (in my experience a PWM value of
around 120 is the reliable lower end - it would continue spinning
below that point but won't always start without being pushed manually)

> > I don't currently use the GPU/NPU/VPU though - maybe those would
> > produce more moderate load which could benefit from spinning the fan
> > at medium speeds.
>
> Perhaps, but it will need to be tested at some point.  Have you tried
> loading only one or two CPU cores?

I do see the full range of PWM values being used, including
intermediate ones. It doesn't go zero to hero :) My point was more
about the default fan not being super mighty vs. the full package
thermal output, which will likely mean that intermediate values are
rarely used. But I'll double check with more varied loads to make sure
it behaves in a sensible way (especially given that I'll be testing
purely interrupt-driven operation per Daniel's guidance in the other
sub-thread).

Best regards,
Alexey