Messages in this thread | | | From | Di Shen <> | Date | Mon, 10 Jul 2023 12:41:23 +0800 | Subject | Re: [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/ ---
| |