Re: [PATCH v1 07/16] thermal: gov_power_allocator: Eliminate a redundant variable

From: Daniel Lezcano
Date: Tue Apr 23 2024 - 14:03:11 EST


On 23/04/2024 19:54, Rafael J. Wysocki wrote:
On Tue, Apr 23, 2024 at 7:35 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:

On 10/04/2024 18:12, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Notice that the passive field in struct thermal_zone_device is not
used by the Power Allocator governor itself and so the ordering of
its updates with respect to allow_maximum_power() or allocate_power()
does not matter.

Accordingly, make power_allocator_manage() update that field right
before returning, which allows the current value of it to be passed
directly to allow_maximum_power() without using the additional update
variable that can be dropped.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---

The step_wise and the power allocator are changing the tz->passive
values, so telling the core to start and stop the passive mitigation timer.

It looks strange that a plugin controls the core internal and not the
opposite.

I'm wondering if it would not make sense to have the following ops:

.start
.stop

.start is called when the first trip point is crossed the way up
.stop is called when the first trip point is crossed the way down

- The core is responsible to start and stop the passive mitigation timer.

- the governors do no longer us tz->passive

The reset of the governor can happen at start or stop, as well as the
device cooling states.

I have a patch that simply increments tz->passive when a passive trip
point is passed on the way up and decrements it when a passive trip
point is crossed on the way down. It appears to work reasonably well.

Does it make the governors getting ride of it ? Or at least not changing its value ?

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