| Message ID | b3c25cedfa3a9b8a4b481e714992afe62b84ca3b.1531899325.git.amit.kucheria@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] arm64: dts: sdm845: enable tsens thermal zones | expand |
Hi Amit, On Wed, Jul 18, 2018 at 01:19:17PM +0530, Amit Kucheria wrote: > One thermal zone per cpu is defined > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++++++++++ > 1 file changed, 170 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 01ff146..a75be7c 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -340,4 +340,174 @@ > }; > }; > }; > + > + thermal-zones { > + cpu0-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; In the context of the TSENS patches you mentioned that you are working on interrupt support. Can the polling delays be removed once that is merged? > + thermal-sensors = <&tsens0 1>; > + > + trips { > + cpu_alert0: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit0: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu1-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 2>; > + > + trips { > + cpu_alert1: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit1: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu2-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 3>; > + > + trips { > + cpu_alert2: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit2: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu3-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 4>; > + > + trips { > + cpu_alert3: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit3: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu4-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 7>; > + > + trips { > + cpu_alert4: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit4: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu5-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 8>; > + > + trips { > + cpu_alert5: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit5: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu6-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 9>; > + > + trips { > + cpu_alert6: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit6: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu7-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 10>; > + > + trips { > + cpu_alert7: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit7: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; Dumb DT question: the trip information is the same for all CPUs. Would it be possible to have a single node and refer to it with a phandle? I suppose the anwer is no and even if it was possible we probably wouldn't want it, since it would complicate overriding settings for a specific CPU (should that ever be needed ...) or cluster. Just wondering. > + }; > + }; > }; I don't have documentation to verify that the sensors and CPUs match, but it is in line with what I've seen in some Android tree, so it seems alright ;-) Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Tested-by: Matthias Kaehlcke <mka@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Andy, Do you have any feedback for this patch? Regards, Amit On Wed, Jul 18, 2018 at 1:19 PM, Amit Kucheria <amit.kucheria@linaro.org> wrote: > One thermal zone per cpu is defined > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++++++++++ > 1 file changed, 170 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 01ff146..a75be7c 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -340,4 +340,174 @@ > }; > }; > }; > + > + thermal-zones { > + cpu0-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 1>; > + > + trips { > + cpu_alert0: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit0: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu1-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 2>; > + > + trips { > + cpu_alert1: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit1: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu2-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 3>; > + > + trips { > + cpu_alert2: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit2: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu3-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 4>; > + > + trips { > + cpu_alert3: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit3: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu4-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 7>; > + > + trips { > + cpu_alert4: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit4: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu5-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 8>; > + > + trips { > + cpu_alert5: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit5: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu6-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 9>; > + > + trips { > + cpu_alert6: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit6: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu7-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 10>; > + > + trips { > + cpu_alert7: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit7: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + }; > }; > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/07/2018 09:49, Amit Kucheria wrote: > One thermal zone per cpu is defined The thermal zones are very close, especially when the CPUs belong to the same 'cluster'. Very likely the temperature will propagate from one core to another core, so when one core reaches the trip0, there is good chance the other cores will be close and cross the trip0 threshold too. Having multiple thermal zones, one per CPU, may trigger an interrupts storm with the passive polling timer delay. Does this board have a cooling device per CPU also ? > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++++++++++ > 1 file changed, 170 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 01ff146..a75be7c 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -340,4 +340,174 @@ > }; > }; > }; > + > + thermal-zones { > + cpu0-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 1>; > + > + trips { > + cpu_alert0: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit0: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu1-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 2>; > + > + trips { > + cpu_alert1: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit1: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu2-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 3>; > + > + trips { > + cpu_alert2: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit2: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu3-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 4>; > + > + trips { > + cpu_alert3: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit3: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu4-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 7>; > + > + trips { > + cpu_alert4: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit4: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu5-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 8>; > + > + trips { > + cpu_alert5: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit5: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu6-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 9>; > + > + trips { > + cpu_alert6: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit6: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu7-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 10>; > + > + trips { > + cpu_alert7: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit7: trip1 { > + temperature = <110000>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + }; > }; > -- <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
On Wed, Aug 29, 2018 at 11:39:12AM +0200, Daniel Lezcano wrote: > On 18/07/2018 09:49, Amit Kucheria wrote: > > One thermal zone per cpu is defined > > The thermal zones are very close, especially when the CPUs belong to the > same 'cluster'. Very likely the temperature will propagate from one core > to another core, so when one core reaches the trip0, there is good > chance the other cores will be close and cross the trip0 threshold too. > > Having multiple thermal zones, one per CPU, may trigger an interrupts > storm with the passive polling timer delay. > > Does this board have a cooling device per CPU also ? The cpufreq driver is still under development: https://lore.kernel.org/patchwork/patch/966833/ . It currently doesn't register cooling devices, but since CPUs from a 'cluster' use the same cpufreq policy I think there will be one cooling device per 'cluster'.
On Wed, Aug 29, 2018 at 3:09 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 18/07/2018 09:49, Amit Kucheria wrote: > > One thermal zone per cpu is defined > > The thermal zones are very close, especially when the CPUs belong to the > same 'cluster'. Very likely the temperature will propagate from one core > to another core, so when one core reaches the trip0, there is good > chance the other cores will be close and cross the trip0 threshold too. Agreed. Once we have all the thermal and cpufreq bits in place, I'd like to examine optimisations such as a virtual sensor, using the coefficients to add weights, etc. > Having multiple thermal zones, one per CPU, may trigger an interrupts > storm with the passive polling timer delay. I'm working on irq support for tsens that should alleviate that. > Does this board have a cooling device per CPU also ? As Matthias mentioned, DVFS is per cluster (4 cpus to a cluster).
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 01ff146..a75be7c 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -340,4 +340,174 @@ }; }; }; + + thermal-zones { + cpu0-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 1>; + + trips { + cpu_alert0: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit0: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu1-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 2>; + + trips { + cpu_alert1: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit1: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu2-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 3>; + + trips { + cpu_alert2: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit2: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu3-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 4>; + + trips { + cpu_alert3: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit3: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu4-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 7>; + + trips { + cpu_alert4: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit4: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu5-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 8>; + + trips { + cpu_alert5: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit5: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu6-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 9>; + + trips { + cpu_alert6: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit6: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + + cpu7-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + + thermal-sensors = <&tsens0 10>; + + trips { + cpu_alert7: trip0 { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + cpu_crit7: trip1 { + temperature = <110000>; + hysteresis = <1000>; + type = "critical"; + }; + }; + }; + }; };
One thermal zone per cpu is defined Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html