From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: Hsin-Te Yuan <yuanhsinte@chromium.org>
Cc: "Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Nícolas F. R. A. Prado" <nfraprado@collabora.com>,
"Bernhard Rosenkränzer" <bero@baylibre.com>,
"Balsam CHIHI" <bchihi@baylibre.com>,
"Alexandre Mergnat" <amergnat@baylibre.com>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH v2] arm64: dts: mediatek: mt8192: Add missing trip point in thermal zone
Date: Wed, 10 Apr 2024 14:17:46 +0200 [thread overview]
Message-ID: <0487ed41-0404-4215-a346-1a3daedc417f@collabora.com> (raw)
In-Reply-To: <CAHc4DNLrU=QVWODOzHXW16Ko1V=W31zkSHOi7rztiJMOMDcKqg@mail.gmail.com>
Il 10/04/24 14:06, Hsin-Te Yuan ha scritto:
> Hi Angelo,
>
> 1. 68°C is provided by MediaTek previously in downstream
> 2. I think sustainable power will be estimated by
> estimate_sustainable_power in drivers/thermal/gov_power_allocator.c,
> but I'll ask MediaTek if they can provide the precise value.
> 3. Sure. I'll do it in v3 after I check 2. with MediaTek
>
Perfect, thanks for the clarification.
Will wait for v3.
Cheers,
Angelo
> Regards,
> Hsin-Te
>
> On Wed, Apr 10, 2024 at 5:50 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 10/04/24 10:40, Hsin-Te Yuan ha scritto:
>>> According to Documentation/driver-api/thermal/power_allocator.rst, there
>>> should be two passive trip points. Adding the missing trip point to
>>> ensure that the governor works optimally.
>>>
>>> Fixes: c7a728051f4e ("arm64: dts: mediatek: mt8192: Add thermal nodes and thermal zones")
>>> Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
>>
>> That's better - but while I can agree about setting a general temperature
>> for that, I still have a few questions and requests:
>>
>> 1. Why is this 68°C? Was that value provided by MediaTek, or was it calculated?
>> 1b. If this was calculated, please, can you explain how?
>>
>> 2. The power allocator documentation also says that the governor works good
>> when a "sustainable dissipatable power" parameter is fed to it through the
>> `sustainable-power` devicetree property (with that being strictly board
>> specific and never SoC-global, as that depends on the form factor and on
>> the cooling method of the machine), can you please also add the right
>> sustainable power indication to the Chromebook devicetrees?
>> In the MT8192 specific case, that's mt8192-asurada.dtsi.
>>
>> 3. I just noticed that MT8192 is not the only one that would be affected by
>> the issue that you're describing in this commit; can you please perform a
>> similar change on the others, if parameters are known?
>>
>> Thanks,
>> Angelo
>>
>>> ---
>>> Changes in v2:
>>> - Clearify the reason of adding another passive trip point
>>> - Link to v1: https://lore.kernel.org/r/20240410-upstream-torvalds-master-v1-1-852e903f0cec@chromium.org
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 40 ++++++++++++++++++++++++++++++++
>>> 1 file changed, 40 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> index 05e401670bced..08d8bccc84669 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
>>> @@ -1959,6 +1959,11 @@ cpu0-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU0>;
>>>
>>> trips {
>>> + cpu0_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu0_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -1989,6 +1994,11 @@ cpu1-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU1>;
>>>
>>> trips {
>>> + cpu1_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu1_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2019,6 +2029,11 @@ cpu2-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU2>;
>>>
>>> trips {
>>> + cpu2_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu2_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2049,6 +2064,11 @@ cpu3-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_LITTLE_CPU3>;
>>>
>>> trips {
>>> + cpu3_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu3_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2079,6 +2099,11 @@ cpu4-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU0>;
>>>
>>> trips {
>>> + cpu4_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu4_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2109,6 +2134,11 @@ cpu5-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU1>;
>>>
>>> trips {
>>> + cpu5_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu5_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2139,6 +2169,11 @@ cpu6-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU2>;
>>>
>>> trips {
>>> + cpu6_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu6_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>> @@ -2169,6 +2204,11 @@ cpu7-thermal {
>>> thermal-sensors = <&lvts_mcu MT8192_MCU_BIG_CPU3>;
>>>
>>> trips {
>>> + cpu7_thres: trip-point {
>>> + temperature = <68000>;
>>> + hysteresis = <2000>;
>>> + type = "passive";
>>> + };
>>> cpu7_alert: trip-alert {
>>> temperature = <85000>;
>>> hysteresis = <2000>;
>>>
>>> ---
>>> base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23
>>> change-id: 20240410-upstream-torvalds-master-40aeff5416c7
>>>
>>> Best regards,
>>
>>
prev parent reply other threads:[~2024-04-10 12:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-10 8:40 [PATCH v2] arm64: dts: mediatek: mt8192: Add missing trip point in thermal zone Hsin-Te Yuan
2024-04-10 9:49 ` AngeloGioacchino Del Regno
2024-04-10 12:06 ` Hsin-Te Yuan
2024-04-10 12:17 ` AngeloGioacchino Del Regno [this message]
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=0487ed41-0404-4215-a346-1a3daedc417f@collabora.com \
--to=angelogioacchino.delregno@collabora.com \
--cc=amergnat@baylibre.com \
--cc=bchihi@baylibre.com \
--cc=bero@baylibre.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--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=nfraprado@collabora.com \
--cc=robh@kernel.org \
--cc=yuanhsinte@chromium.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