| Message ID | 1641581806-32550-1-git-send-email-quic_manafm@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] thermal/core: Clear all mitigation when thermal zone is disabled | expand |
Hi Manaf,
semantically speaking disabling a thermal zone would be to detach the
thermal zone from its governor and stop the monitoring.
May be add the functions
- thermal_governor_attach(struct thermal_zone_device *tzd)
{
...
if (tz->governor && tz->governor->bind_to_tz) {
if (tz->governor->bind_to_tz(tz)) {
}
...
}
- thermal_governor_detach(struct thermal_zone_device *tzd)
{
...
if (tz->governor && tz->governor->unbind_from_tz)
tz->governor->unbind_from_tz(tz);
...
}
And add in the step_wise and power_allocator the reset of the governor's
data as well as the cooling device instances in the unbind_from_tz()
callback
Then, thermal_zone_device_enable() attaches and
thermal_zone_device_disable() detaches the governor.
Does it make sense ?
On 07/01/2022 19:56, 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
> until it is re-enabled.
>
> To avoid above mentioned issue, on thermal zone disable request
> reset thermal zone and clear mitigation for each trip explicitly.
>
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com>
> ---
> drivers/thermal/thermal_core.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 51374f4..e288c82 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -447,10 +447,18 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz,
>
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> - if (mode == THERMAL_DEVICE_ENABLED)
> + if (mode == THERMAL_DEVICE_ENABLED) {
> thermal_notify_tz_enable(tz->id);
> - else
> + } else {
> + int trip;
> +
> + /* make sure all previous throttlings are cleared */
> + thermal_zone_device_init(tz);
> + for (trip = 0; trip < tz->trips; trip++)
> + handle_thermal_trip(tz, trip);
> +
> thermal_notify_tz_disable(tz->id);
> + }
>
> return ret;
> }
>
HI Daniel, On 1/24/2022 6:35 AM, Pandruvada, Srinivas wrote: > On Sun, 2022-01-23 at 21:51 +0100, Daniel Lezcano wrote: >> Hi Manaf, >> >> semantically speaking disabling a thermal zone would be to detach the >> thermal zone from its governor and stop the monitoring. >> >> May be add the functions >> >> - thermal_governor_attach(struct thermal_zone_device *tzd) >> { >> ... >> if (tz->governor && tz->governor->bind_to_tz) { >> if (tz->governor->bind_to_tz(tz)) { >> } >> ... >> } >> >> - thermal_governor_detach(struct thermal_zone_device *tzd) >> { >> ... >> if (tz->governor && tz->governor->unbind_from_tz) >> tz->governor->unbind_from_tz(tz); >> ... >> } >> >> And add in the step_wise and power_allocator the reset of the >> governor's >> data as well as the cooling device instances in the unbind_from_tz() >> callback >> >> Then, thermal_zone_device_enable() attaches and >> thermal_zone_device_disable() detaches the governor. >> >> Does it make sense ? > This is better. > > Thanks, > Srinivas Yes, it makes sense. I will update it in v4 > >> >> On 07/01/2022 19:56, 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 >>> until it is re-enabled. >>> >>> To avoid above mentioned issue, on thermal zone disable request >>> reset thermal zone and clear mitigation for each trip explicitly. >>> >>> Signed-off-by: Manaf Meethalavalappu Pallikunhi >>> <quic_manafm@quicinc.com> >>> --- >>> drivers/thermal/thermal_core.c | 12 ++++++++++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/thermal/thermal_core.c >>> b/drivers/thermal/thermal_core.c >>> index 51374f4..e288c82 100644 >>> --- a/drivers/thermal/thermal_core.c >>> +++ b/drivers/thermal/thermal_core.c >>> @@ -447,10 +447,18 @@ static int >>> thermal_zone_device_set_mode(struct thermal_zone_device *tz, >>> >>> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); >>> >>> - if (mode == THERMAL_DEVICE_ENABLED) >>> + if (mode == THERMAL_DEVICE_ENABLED) { >>> thermal_notify_tz_enable(tz->id); >>> - else >>> + } else { >>> + int trip; >>> + >>> + /* make sure all previous throttlings are cleared >>> */ >>> + thermal_zone_device_init(tz); >>> + for (trip = 0; trip < tz->trips; trip++) >>> + handle_thermal_trip(tz, trip); >>> + >>> thermal_notify_tz_disable(tz->id); >>> + } >>> >>> return ret; >>> } >>> >>
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 51374f4..e288c82 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -447,10 +447,18 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz, thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); - if (mode == THERMAL_DEVICE_ENABLED) + if (mode == THERMAL_DEVICE_ENABLED) { thermal_notify_tz_enable(tz->id); - else + } else { + int trip; + + /* make sure all previous throttlings are cleared */ + thermal_zone_device_init(tz); + for (trip = 0; trip < tz->trips; trip++) + handle_thermal_trip(tz, trip); + thermal_notify_tz_disable(tz->id); + } return ret; }
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 until it is re-enabled. To avoid above mentioned issue, on thermal zone disable request reset thermal zone and clear mitigation for each trip explicitly. Signed-off-by: Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com> --- drivers/thermal/thermal_core.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)