Re: [PATCH v5] drivers: thermal: clear all mitigation when thermal zone is disabled

[Daniel Lezcano]

On 15/02/2022 10:57, Lukasz Luba wrote:
> On 2/14/22 8:00 PM, Manaf Meethalavalappu Pallikunhi wrote:
> > On 1/31/2022 12:55 PM, Lukasz Luba wrote:
> > > Hi Manaf,
> > >
> > > On 1/27/22 6:11 PM, Manaf Meethalavalappu Pallikunhi wrote:
> > > > Whenever a thermal zone is in trip violated state, there is a chance
> > > > that the same thermal zone mode can be disabled either via
> > > > thermal core API or via thermal zone sysfs. Once it is disabled,
> > > > the framework bails out any re-evaluation of thermal zone. It leads
> > > > to a case where if it is already in mitigation state, it will stay
> > > > the same state forever.
> > > >
> > > > To avoid above mentioned issue, add support to bind/unbind
> > > > governor from thermal zone during thermal zone mode change request
> > > > and clear all existing throttling in governor unbind_from_tz()
> > > > callback.
> > >
> > > I have one use case:
> > > This would be a bit dangerous, e.g. to switch governors while there is a
> > > high temperature. Although, sounds reasonable to left a 'default' state
> > > for a next governor.
> > I believe only way to change the governror via userspace at runtime.
> >
> > Just re-evaluate thermal zone  (thermal_zone_device_update) 
> > immediately after
> >
> > thermal_zone_device_set_policy()  in same policy_store() context, 
> > isn't it good enough ?
>
> It depends. The code would switch the governors very fast, in the
> meantime notifying about possible full speed of CPU (cooling state = 0).
> If the task scheduler goes via schedutil (cpufreq governor) at that
> moment and decides to set this max frequency, it will be set.
> This is situation with your patch, since you added in IPA unbind
> 'allow_maximum_power()'.
> Then the new governor is bind, evaluates the max cooling state, the
> notification about reduced max freq is sent to schedutil (a workqueue
> will call .sugov_limits() callback) and lower freq would be set.
>
> Now there are things which are not greatly covered by these 4
> involved sub-systems (thermal fwk, schedutil, scheduler, HW).
> It takes time. It also depends when the actual HW freq is possible to be
> set. It might take a few milli-seconds or even a dozes of milli-seconds
> (depends on HW).
>
> Without your change, we avoid such situation while switching the
> thermal governors.
>
> For your requirement, which is 'mode' enable/disable it OK to
> un-throttle.
>
> It's probably something to Rafael and Daniel to judge if we want to
> pay that cost and introduce this racy time slot.
>
> Maybe there is a way to implement your needed feature differently.
> Unfortunately, I'm super busy with other stuff this month so I cannot
> spent much time investigating this.

IMO, we should be able to disable a thermal zone, no mitigation will 
happen but somehow we can keep the hot / critical trip points enabled, 
so if the temperature crosses these that would trigger an action for safety.

However, for me it is still unclear what means "disabling a thermal 
zone" exactly ?

Maybe we should clarify that before going further


--
<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