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 <[email protected]>
---
Changes in v2:
- Clearify the reason of adding another passive trip point
- Link to v1: https://lore.kernel.org/r/[email protected]
---
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,
--
Hsin-Te Yuan <[email protected]>
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 <[email protected]>
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/[email protected]
> ---
> 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,
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
Regards,
Hsin-Te
On Wed, Apr 10, 2024 at 5:50 PM AngeloGioacchino Del Regno
<[email protected]> 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 <[email protected]>
>
> 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/[email protected]
> > ---
> > 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,
>
>
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
> <[email protected]> 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 <[email protected]>
>>
>> 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/[email protected]
>>> ---
>>> 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,
>>
>>