Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 23 Apr 2024 19:54:49 +0200 | Subject | Re: [PATCH v1 07/16] thermal: gov_power_allocator: Eliminate a redundant variable |
| |
On Tue, Apr 23, 2024 at 7:35 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 10/04/2024 18:12, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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@intel.com> > > --- > > 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.
| |