Message-ID: <DB3PR0402MB39162E6A876BA54A80B0DCE2F5B30@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Date:   Mon, 25 May 2020 02:46:19 +0000
From:   Anson Huang <anson.huang@....com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        "rui.zhang@...el.com" <rui.zhang@...el.com>,
        "amit.kucheria@...durent.com" <amit.kucheria@...durent.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH] thermal: imx8mm: Add get_trend ops

Hi, Daniel


> Subject: RE: [PATCH] thermal: imx8mm: Add get_trend ops
> 
> Hi, Daniel
> 
> > Subject: Re: [PATCH] thermal: imx8mm: Add get_trend ops
> >
> > On 23/05/2020 02:35, Anson Huang wrote:
> > > Hi, Daniel
> > >
> > >
> > >> Subject: Re: [PATCH] thermal: imx8mm: Add get_trend ops
> > >>
> > >> On 13/05/2020 04:58, Anson Huang wrote:
> > >>> Add get_trend ops for i.MX8MM thermal to apply fast cooling
> > >>> mechanism, when temperature exceeds passive trip point, the
> > >>> highest cooling action will be applied, and when temperature drops
> > >>> to lower than the margin below passive trip point, the lowest
> > >>> cooling action will be applied.
> > >>
> > >> You are not describing what is the goal of this change.
> > >
> > > The goal of this change is to make sure whenever temperature exceeds
> > > passive trip point, the highest cooling action will be applied
> > > immediately, e.g., if there are many cpufreq OPP, the default
> > > cooling will be step by step, it will take some more rounds to make
> > > cpufreq drop to
> > lowest OPP, while on i.MX, we expect the cpufreq drop to lowest OPP
> > immediately.
> >
> > Whatever the slope of the temperature increase?
> 
> Yes.
> 
> >
> > >> IIUC, the resulting change will be an on/off action. The thermal
> > >> zone is mitigated with the highest cooling effect, so the lowest
> > >> OPP, then the temperature trend is stable until it goes below the
> > >> trip - margin where the mitigation is stopped.
> > >
> > > Yes, your understanding is correctly, once the temperature exceeds
> > > passive trip point, the highest cooling action will be applied
> > > immediately and then it will be stable there until temperature drop
> > > to trip - margin, then the cooling action will be cancelled, the
> > > margin is to avoid
> > the back and forth near the passive trip point.
> > >
> > >>
> > >> Except, I'm missing something, setting a trip point with a 10000
> > >> hysteresis and a cooling map min/max set to the highest opp will
> > >> result on
> > the same.
> > >
> > > Yes setting cooling map min/max cooling state to highest OPP will
> > > make the highest cooling action applied immediately, and to have the
> > > function of cooling action being cancelled when temperature drops to
> > > trip - margin, I have to define another trip point, say passive trip
> > > point is 85000, and cooling map min/max set to highest OPP in
> > > passive trip point then add another trip point named "active" with
> > > 75000, and
> > without any cooling map in it, right?
> >
> > May be I misunderstood but only the change as below is needed. No need
> > to add a trip point, especially an 'active' trip which is a for an
> > active cooling device like a fan.
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > index cc7152ecedd9..bea263bd06b4 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > @@ -231,10 +231,10 @@ cooling-maps {
> >  				map0 {
> >  					trip = <&cpu_alert0>;
> >  					cooling-device =
> > -						<&A53_0 THERMAL_NO_LIMIT
> > THERMAL_NO_LIMIT>,
> > -						<&A53_1 THERMAL_NO_LIMIT
> > THERMAL_NO_LIMIT>,
> > -						<&A53_2 THERMAL_NO_LIMIT
> > THERMAL_NO_LIMIT>,
> > -						<&A53_3 THERMAL_NO_LIMIT
> > THERMAL_NO_LIMIT>;
> > +						<&A53_0 2 2>,
> > +						<&A53_1 2 2>,
> > +						<&A53_2 2 2>,
> > +						<&A53_3 2 2>
> >  				};
> >  			};
> >  		};
> >
> >
> 
> Thanks, I will have a try to see if it meets our expectation.

I tried modifying the min/max to '2' in cooling map, it works that whenever cooling
action is needed, the max cooling action will be applied. But I also noticed some behaviors
which NOT as expected:

1. to easy the test, I enable the " CONFIG_THERMAL_WRITABLE_TRIPS", and just modify the
passive trip threshold to trigger the cooling action, this is much more easy then putting the board
into an oven to increase the SoC temperature or running many high loading test to increase the temperature,
but when I modify the passive trip threshold to be lower than current temperature, the cooling action is NOT
triggered immediately, it is because the default step_wise governor will NOT trigger the cooling action when
the trend is THERMAL_TREND_STABLE.
But what expected is, when the temperature is exceed the passive trip threshold, the cooling action can be
triggered immediately no matter the trend is stable or raising. That means we have to implement our own
.get_trend callback?

2. No margin for releasing the cooling action, for example, if cooling action is triggered, when the temperature
drops below the passive trip threshold, the cooling action will be cancelled immediately, if SoC keeps running
at full performance, the temperature will increase very soon, which may cause the SoC keep triggering/cancelling
the cooling action around the passive trip threshold. If there is a margin, the situation will be much better.

Do you have any idea/comment about them?

Thanks,
Anson

Powered by blists - more mailing lists