| Message ID | 20240410-upstream-torvalds-master-v2-1-679777847b63@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] arm64: dts: mediatek: mt8192: Add missing trip point in thermal zone | expand |
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,
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 <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, > >
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, >> >>
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>;
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> --- 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(+) --- base-commit: 20cb38a7af88dc40095da7c2c9094da3873fea23 change-id: 20240410-upstream-torvalds-master-40aeff5416c7 Best regards,