[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0h9b2E0meeHGRtRv+=GbsABUs=L4RqAvAZuCksU+uN0cQ@mail.gmail.com>
Date: Wed, 14 Aug 2024 19:18:36 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Peter Kästle <peter@...e.net>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, Lukasz Luba <lukasz.luba@....com>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH v1 3/4] thermal: gov_bang_bang: Add .manage() callback
On Wed, Aug 14, 2024 at 12:50 AM Peter Kästle <peter@...e.net> wrote:
>
> On 13.08.24 16:27, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > After recent changes, the Bang-bang governor may not adjust the
> > initial configuration of cooling devices to the actual situation.
> >
> > Namely, if a cooling device bound to a certain trip point starts in
> > the "on" state and the thermal zone temperature is below the threshold
> > of that trip point, the trip point may never be crossed on the way up
> > in which case the state of the cooling device will never be adjusted
> > because the thermal core will never invoke the governor's
> > .trip_crossed() callback. [Note that there is no issue if the zone
> > temperature is at the trip threshold or above it to start with because
> > .trip_crossed() will be invoked then to indicate the start of thermal
> > mitigation for the given trip.]
> >
> > To address this, add a .manage() callback to the Bang-bang governor
> > and use it to ensure that all of the thermal instances managed by the
> > governor have been initialized properly and the states of all of the
> > cooling devices involved have been adjusted to the current zone
> > temperature as appropriate.
> >
> > Fixes: 530c932bdf75 ("thermal: gov_bang_bang: Use .trip_crossed() instead of .throttle()")
> > Link: https://lore.kernel.org/linux-pm/1bfbbae5-42b0-4c7d-9544-e98855715294@piie.net/
> > Cc: 6.10+ <stable@...r.kernel.org> # 6.10+
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> oops, previously sent from wrong email address, here again from correct one...
> Acked-by: Peter Kästle <peter@...e.net>
No worries and thanks for the ACKs!
I gather that they mean that the changes work for you.
> > ---
> > drivers/thermal/gov_bang_bang.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > Index: linux-pm/drivers/thermal/gov_bang_bang.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/gov_bang_bang.c
> > +++ linux-pm/drivers/thermal/gov_bang_bang.c
> > @@ -26,6 +26,7 @@ static void bang_bang_set_instance_targe
> > * when the trip is crossed on the way down.
> > */
> > instance->target = target;
> > + instance->initialized = true;
> >
> > dev_dbg(&instance->cdev->device, "target=%ld\n", instance->target);
> >
> > @@ -80,8 +81,37 @@ static void bang_bang_control(struct the
> > }
> > }
> >
> > +static void bang_bang_manage(struct thermal_zone_device *tz)
> > +{
> > + const struct thermal_trip_desc *td;
> > + struct thermal_instance *instance;
> > +
> > + for_each_trip_desc(tz, td) {
> > + const struct thermal_trip *trip = &td->trip;
> > +
> > + if (tz->temperature >= td->threshold ||
> > + trip->temperature == THERMAL_TEMP_INVALID ||
> > + trip->type == THERMAL_TRIP_CRITICAL ||
> > + trip->type == THERMAL_TRIP_HOT)
> > + continue;
> > +
> > + /*
> > + * If the initial cooling device state is "on", but the zone
> > + * temperature is not above the trip point, the core will not
> > + * call bang_bang_control() until the zone temperature reaches
> > + * the trip point temperature which may be never. In those
> > + * cases, set the initial state of the cooling device to 0.
> > + */
> > + list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> > + if (!instance->initialized && instance->trip == trip)
> > + bang_bang_set_instance_target(instance, 0);
> > + }
> > + }
> > +}
> > +
> > static struct thermal_governor thermal_gov_bang_bang = {
> > .name = "bang_bang",
> > .trip_crossed = bang_bang_control,
> > + .manage = bang_bang_manage,
> > };
> > THERMAL_GOVERNOR_DECLARE(thermal_gov_bang_bang);
> >
> >
> >
>
Powered by blists - more mailing lists