RE: [PATCH] thermal: imx8mm: Add get_trend ops

[Anson Huang <anson.huang@nxp.com>]

Hi, Daniel

> Subject: Re: [PATCH] thermal: imx8mm: Add get_trend ops
> 
> On 25/05/2020 04:46, Anson Huang wrote:
> > Hi, Daniel
> 
> [ ... ]
> 
> > 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.
> 
> You are right, what is expected is, when the temperature exceeds the passive
> trip threshold, a cooling action happens, the trend is raising in this case.
> 
> But in your test, it is not what is happening: the trip point is changing, not the
> temperature.
> 
> Probably, the cpufreq driver is at its lowest OPP, so there is no room for more
> cooling effect when changing the trip point.
> 
> IMO, the test is not right as the trip point is decreased to a temperature where
> actually the SoC is not hot.
> 
> If you want to test it easily, I recommend to use dhrystone, something like:
> 
>  dhrystone -t 6 -l 10000
> 
> That will make your board to heat immediately.

Thanks, I understand. To aligned with the formal test method, I will inform our test
team to update the test case to meet the requirement.

> 
> > That
> > means we have to implement our own .get_trend callback?
> 
> From my POV it must disappear, because it has little meaning. The governor is
> the one which should be dealing with that and call the corresponding cooling
> index.

OK, I will use common .get_trend() implementation.

> 
> > 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?
> 
> Yes, that is a good point. The hysteresis is supposed to do that. There is a work
> done by Andrzej Pietrasiewicz to disable / enable the thermal zones [1]. I think
> we should be able to fix that after the changes are done.

OK, then I will wait for this change. So to apply MAX cooling action immediately,
all expected changes for i.MX platforms are to assign min/max cooling index in
DT cooling map, I will summit a patch set then.

Thanks,
Anson.