From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Eric Woudstra <ericwouds@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix mt7622.dtsi thermal cpu
Date: Wed, 23 Jun 2021 22:08:22 +0200 [thread overview]
Message-ID: <fb633034-96e5-6165-b43f-290ae1a65cfd@linaro.org> (raw)
In-Reply-To: <47261865-00e3-41eb-bb36-2b939f81f1e8@gmail.com>
On 23/06/2021 20:43, Eric Woudstra wrote:
>
> I choose "hot" before, because 87 degrees seems ok to start frequency
> throttling. But, yes, it should be passive.
>
> 87 is still quite low if I compare this temperature with the
> wrt3200acm Marvell dual core arm soc. They even went above 100
> degrees so I feel for an arm processor inside a router box it is fine
> to use 87 degrees But maybe someone at Mediatek can give some more
> details about operating temperatures.
Sometimes, the SoC vendor puts a high temperature in the DT just to
export the thermal zone and deal with it from userspace. So putting the
high temp allow the userspace (usually a thermal engine - Android stuff)
to deal with the mitigation without a kernel interaction.
Having more than 100°C could be this kind of setup. Only the operating
temperature from the hardware documentation will tell the safe
temperature for the silicon.
IMO, 77°C is a good compromise until getting the documented temp. 87°C
sounds to me a bit too hot.
> It may be possible to leave the active map in the device tree as some
> users of the bananapi might choose to install a fan as it is one of
> the options.
The active trip only makes sense if the cooling device is a fan (or any
active device), so the mapping points to a fan node, like:
https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-khadas-edge.dtsi#n192
If there is no such [pwm] fan output on the board, no active trip point
should be added.
> Get BlueMail for Android
>
> On Jun 23, 2021, 5:58 PM, at 5:58 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> On 23/06/2021 17:35, Eric Woudstra wrote:
>>> It is only useful to set 1 map with the regulated temperature for
>>> cpu frequency throttling. Same as in the kernel document
>>> example.
>>>
>>>
>>> It has no use to set frequency scaling on 2 different
>>> temperature trip points, as the lowest one makes sure the higher
>>> one(s) are never reached.
>>
>> I looked more closely the DT and there is a misunderstanding of
>> the thermal framework in the definition.
>>
>> There is one trip point with the passive type and the cpu cooling
>> device, followed by a second trip point with the active type *but*
>> the same cpu cooling device. That is wrong.
>>
>> And finally, there is the hot trip point as a third mapping and
>> the same cooling device.
>>
>> The hot trip point is only there to notify userspace and let it
>> take an immediate action to prevent an emergency shutdown when
>> reaching the critical temperature.
>>
>>> It can be applied only at 1 trip point. Multiple trip points is
>>> only usefully for fan control to make sure the fan is not too
>>> noisy when it is not necessary to be noisy.
>>>
>>>
>>> The CPU will almost come to a dead stop when it starts to pass
>>> the lowest thermal map with frequency throttling.
>>>
>>> This is why it is a bug and needs a fix, not only adjustment.
>>
>> Yes, you are right. It should be something like (verbatim copy):
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
>> b/arch/arm64/boot/dts/mediatek/mt7622.dtsi index
>> 890a942ec608..88c81d24f4ff 100644 ---
>> a/arch/arm64/boot/dts/mediatek/mt7622.dtsi +++
>> b/arch/arm64/boot/dts/mediatek/mt7622.dtsi @@ -136,24 +136,18 @@
>> secmon_reserved: secmon@43000000 {
>>
>> thermal-zones { cpu_thermal: cpu-thermal { -
>> polling-delay-passive = <1000>; + polling-delay-passive = <250>;
>> polling-delay = <1000>;
>>
>> thermal-sensors = <&thermal 0>;
>>
>> trips { cpu_passive: cpu-passive { - temperature = <47000>; +
>> temperature = <77000>; hysteresis = <2000>; type = "passive"; };
>>
>> - cpu_active: cpu-active { - temperature = <67000>; -
>> hysteresis = <2000>; - type = "active"; - }; - cpu_hot:
>> cpu-hot { temperature = <87000>; hysteresis = <2000>; @@ -173,18
>> +167,6 @@ map0 { cooling-device = <&cpu0 THERMAL_NO_LIMIT
>> THERMAL_NO_LIMIT>, <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; }; -
>> - map1 { - trip = <&cpu_active>; - cooling-device =
>> <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, - <&cpu1
>> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; - }; - - map2 { -
>> trip = <&cpu_hot>; - cooling-device = <&cpu0 THERMAL_NO_LIMIT
>> THERMAL_NO_LIMIT>, - <&cpu1 THERMAL_NO_LIMIT
>> THERMAL_NO_LIMIT>; - }; }; }; };
>>
>>
>> -- <http://www.linaro.org/> Linaro.org │ Open source software for
>> ARM SoCs
>>
>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
next prev parent reply other threads:[~2021-06-23 20:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-19 12:19 [PATCH] Fix mt7622.dtsi thermal cpu ericwouds
2021-06-21 18:29 ` Daniel Lezcano
2021-06-23 15:35 ` Eric Woudstra
2021-06-23 15:58 ` Daniel Lezcano
2021-06-23 18:43 ` Eric Woudstra
2021-06-23 20:08 ` Daniel Lezcano [this message]
2021-06-24 9:59 ` Eric Woudstra
2021-06-24 10:21 ` Daniel Lezcano
2021-06-24 13:29 ` Eric Woudstra
2021-06-25 8:16 ` Aw: " Frank Wunderlich
2021-06-25 9:22 ` Daniel Golle
2021-06-25 9:31 ` Aw: " Frank Wunderlich
2021-06-25 10:11 ` Daniel Golle
2021-06-25 9:57 ` Aw: " Daniel Lezcano
2021-06-25 11:03 ` Aw: " Frank Wunderlich
2021-06-25 11:07 ` Eric Woudstra
2021-06-25 11:47 ` Aw: " Daniel Lezcano
2021-06-25 12:28 ` Aw: " Frank Wunderlich
2021-06-25 12:50 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fb633034-96e5-6165-b43f-290ae1a65cfd@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=ericwouds@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).