lkml.org 
[lkml]   [2023]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH V5] thermal/core/power_allocator: reset thermal governor when trip point is changed
    On Mon, Jul 10, 2023 at 11:38 AM Di Shen <di.shen@unisoc.com> wrote:
    >
    > When the thermal trip point is changed, the governor should
    > be reset so that the policy algorithm can be updated to adapt
    > to the new trip point.
    >
    > 1.Thermal governor is working for the passive trip point or active
    > trip point. Therefore, when the trip point is changed it should
    > reset the governor only for passic/active trip points.
    >
    > 2.For "power_allocator" governor reset, the parameters of pid
    > controller and the states of power cooling devices should be reset.
    >
    > 2.1
    > The IPA controls temperature using PID mechanism. It is a closed-
    > loop feedback monitoring system. It uses the gap between target
    > temperature and current temperature which says "error" as the
    > input of the PID controller:
    >
    > err = desired_temperature - current_temperature
    > P_max =
    > k_p * err + k_i * err_integral + k_d * diff_err + sustainable_power
    >
    > That algorithm can 'learn' from the 'observed' in the past reaction
    > for it's control decisions and accumulates that information in the
    > I(Integral) part so that it can conpensate for those 'learned'
    > mistakes.
    >
    > Based on the above, the most important is the desired temperature
    > comes from the trip point. When the trip point is changed, all the
    > previous learned errors won't help for the IPA. So the pid parameters
    > should be reset for IPA governor reset.
    >
    > 2.2
    > Other wise, the cooling devices should also be reset when the trip
    > point is changed.
    >
    > This patch adds an ops for the thermal_governor structure to reset
    > the governor and give the 'reset' function definition for power
    > allocator. The ops is called when the trip points are changed via
    > sysfs interface.
    >
    > Signed-off-by: Di Shen <di.shen@unisoc.com>
    > ---
    > drivers/thermal/gov_power_allocator.c | 9 +++++++++
    > drivers/thermal/thermal_trip.c | 5 +++++
    > include/linux/thermal.h | 3 +++
    > 3 files changed, 17 insertions(+)
    >
    > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
    > index 8642f1096b91..8d17a10671e4 100644
    > --- a/drivers/thermal/gov_power_allocator.c
    > +++ b/drivers/thermal/gov_power_allocator.c
    > @@ -729,10 +729,19 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id)
    > return allocate_power(tz, trip.temperature);
    > }
    >
    > +static void power_allocator_reset(struct thermal_zone_device *tz)
    > +{
    > + struct power_allocator_params *params = tz->governor_data;
    > +
    > + reset_pid_controller(params);
    > + allow_maximum_power(tz, true);
    > +}
    > +
    > static struct thermal_governor thermal_gov_power_allocator = {
    > .name = "power_allocator",
    > .bind_to_tz = power_allocator_bind,
    > .unbind_from_tz = power_allocator_unbind,
    > .throttle = power_allocator_throttle,
    > + .reset = power_allocator_reset,
    > };
    > THERMAL_GOVERNOR_DECLARE(thermal_gov_power_allocator);
    > diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c
    > index 907f3a4d7bc8..13bbe029c6ab 100644
    > --- a/drivers/thermal/thermal_trip.c
    > +++ b/drivers/thermal/thermal_trip.c
    > @@ -173,6 +173,11 @@ int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
    > if (tz->trips && (t.temperature != trip->temperature || t.hysteresis != trip->hysteresis))
    > tz->trips[trip_id] = *trip;
    >
    > + /* Reset the governor only when the trip type is active or passive. */
    > + ret = (trip->type == THERMAL_TRIP_PASSIVE || trip->type == THERMAL_TRIP_ACTIVE);
    > + if (ret && t.temperature != trip->temperature && tz->governor && tz->governor->reset)
    > + tz->governor->reset(tz);
    > +
    > thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,
    > trip->temperature, trip->hysteresis);
    >
    > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
    > index 87837094d549..d27d053319bf 100644
    > --- a/include/linux/thermal.h
    > +++ b/include/linux/thermal.h
    > @@ -197,6 +197,8 @@ struct thermal_zone_device {
    > * thermal zone.
    > * @throttle: callback called for every trip point even if temperature is
    > * below the trip point temperature
    > + * @reset: callback called for governor reset
    > + *
    > * @governor_list: node in thermal_governor_list (in thermal_core.c)
    > */
    > struct thermal_governor {
    > @@ -204,6 +206,7 @@ struct thermal_governor {
    > int (*bind_to_tz)(struct thermal_zone_device *tz);
    > void (*unbind_from_tz)(struct thermal_zone_device *tz);
    > int (*throttle)(struct thermal_zone_device *tz, int trip);
    > + void (*reset)(struct thermal_zone_device *tz);
    > struct list_head governor_list;
    > };
    >
    > --
    > 2.17.1
    >

    Sorry, the change log was not successfully saved to the commit message.
    Add it as following here:
    ---
    V5:
    - Simplify the reset ops, make it no return value and no specific
    trip ID as argument.
    - Extend the commit message.

    V4: [4]
    - Compared to V3, handle it in thermal core instead of in governor.
    - Add an ops to the governor structure, and call it when a trip
    point is changed
    - Define reset ops for power allocator.

    V3: [3]
    - Add fix tag.

    V2: [2]
    - Compared to v1, do not revert.
    - Add a variable(last_switch_on_temp) in power_allocator_params
    to record the last switch_on_temp value.
    - Adds a function to renew the update flag and update the
    last_switch_on_temp when thermal trips are writable.

    V1: [1]
    - Revert commit 0952177f2a1f.

    [1] https://lore.kernel.org/all/20230309135515.1232-1-di.shen@unisoc.com/
    [2] https://lore.kernel.org/all/20230315093008.17489-1-di.shen@unisoc.com/
    [3] https://lore.kernel.org/all/20230320095620.7480-1-di.shen@unisoc.com/
    [4] https://lore.kernel.org/all/20230619063534.12831-1-di.shen@unisoc.com/
    ---

    \
     
     \ /
      Last update: 2023-07-10 06:42    [W:5.226 / U:1.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site