Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221029051449.30678-1-manivannan.sadhasivam@linaro.org>
 <20221029051449.30678-13-manivannan.sadhasivam@linaro.org> <90b7e0e0-a354-f64d-8c53-aa80df684a3a@somainline.org>
In-Reply-To: <90b7e0e0-a354-f64d-8c53-aa80df684a3a@somainline.org>
From:   Steev Klimaszewski <steev@kali.org>
Date:   Sat, 29 Oct 2022 14:07:25 -0500
Message-ID: <CAKXuJqhzNvuCh8q0d6F5+X9XxNzx5vOYy490X4-LAXy6WhN49g@mail.gmail.com>
Subject: Re: [PATCH v2 12/12] arm64: dts: qcom: sc8280xp-x13s: Add thermal
 zone support
To:     Konrad Dybcio <konrad.dybcio@somainline.org>
Cc:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        andersson@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, johan+linaro@kernel.org,
        quic_jprakash@quicinc.com, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Oct 29, 2022 at 9:29 AM Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:
>
>
>
> On 29.10.2022 07:14, Manivannan Sadhasivam wrote:
> > Add thermal zone support by making use of the thermistor SYS_THERM6.
> > Based on experiments, this thermistor seems to reflect the actual
> > surface temperature of the laptop.
> >
> > For the cooling device, all BIG CPU cores are throttle down to keep the
> s/throttle/throttled
>
> Is it okay to let the 4xA78C run at full throttle in thermal emergencies though?
> > temperature at a sane level.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > index ca77c19c6d0d..96e2fa72f782 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> > @@ -29,6 +29,52 @@ backlight {
> >               pinctrl-0 = <&edp_bl_en>, <&edp_bl_pwm>;
> >       };
> >
> > +     thermal-zones {
> > +             skin-temp-thermal {
> > +                     polling-delay-passive = <250>;
> > +                     polling-delay = <0>;
> > +                     thermal-sensors = <&pmk8280_adc_tm 5>;
> > +
> > +                     trips {
> > +                             skin_temp_alert0: trip-point0 {
> > +                                     temperature = <55000>;
> > +                                     hysteresis = <1000>;
> > +                                     type = "passive";
> > +                             };
> > +
> > +                             skin_temp_alert1: trip-point1 {
> > +                                     temperature = <58000>;
> > +                                     hysteresis = <1000>;
> > +                                     type = "passive";
> > +                             };
> > +
> > +                             skin-temp-crit {
> > +                                     temperature = <73000>;
> Ouch, I didn't know we were serving burnt fingers at the cafeteria today :D
>
> Or maybe this just looks scary.. The laptop looks plastic, so maybe it won't cause instant
> burns?
>

Disclaimer: I'm an end user that likes to cosplay as a kernel
developer sometimes...

IMO, it just looks scary.  the skin-temp, afaik, is the temperature of
the shell of the laptop, so once it reaches 73C, it would be crit.  In
practice (I've been testing this patch set for a while), I've never
been able to hit 73C, even with the ambient temperature in my
apartment being 77F and multiple compilations running.  I've also
tested a hack to increase the trips to a little bit higher (65/68/73)
and still not been able to do so.

On the flip side, without this patchset, we do not have any thermal
trips at all, and the machine will quite happily run up to 90C (and
I've not seen it shut down, but I wouldn't use it on my lap when this
is occurring...)

> Konrad
> > +                                     hysteresis = <1000>;
> > +                                     type = "critical";
> > +                             };
> > +                     };
> > +
> > +                     cooling-maps {
> > +                             map0 {
> > +                                     trip = <&skin_temp_alert0>;
> > +                                     cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +
> > +                             map1 {
> > +                                     trip = <&skin_temp_alert1>;
> > +                                     cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                                      <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +
> >       vreg_edp_bl: regulator-edp-bl {
> >               compatible = "regulator-fixed";
> >