Re: [PATCH v5 4/4] tools/thermal/thermal-engine: Take into account the thresholds API

Lukasz Luba
2024-10-21T20:09:27Z

On 10/14/24 10:43, Daniel Lezcano wrote:
> Enhance the thermal-engine skeleton with the thresholds added in the
> kernel and use the API exported by the thermal library.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> tools/thermal/thermal-engine/thermal-engine.c | 105 +++++++++++++++---
> 1 file changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/tools/thermal/thermal-engine/thermal-engine.c b/tools/thermal/thermal-engine/thermal-engine.c
> index 9b1476a2680f..0764dc754771 100644
> --- a/tools/thermal/thermal-engine/thermal-engine.c
> +++ b/tools/thermal/thermal-engine/thermal-engine.c
> @@ -38,6 +38,14 @@ struct thermal_data {
> struct thermal_handler *th;
> };
>
> +static int show_threshold(struct thermal_threshold *th, __maybe_unused void *arg)
> +{
> + INFO("threshold temp=%d, direction=%d\n",
> + th->temperature, th->direction);
> +
> + return 0;
> +}
> +
> static int show_trip(struct thermal_trip *tt, __maybe_unused void *arg)
> {
> INFO("trip id=%d, type=%d, temp=%d, hyst=%d\n",
> @@ -70,6 +78,8 @@ static int show_tz(struct thermal_zone *tz, __maybe_unused void *arg)
>
> for_each_thermal_trip(tz->trip, show_trip, NULL);
>
> + for_each_thermal_threshold(tz->thresholds, show_threshold, NULL);
> +
> show_temp(tz, arg);
>
> show_governor(tz, arg);
> @@ -77,6 +87,30 @@ static int show_tz(struct thermal_zone *tz, __maybe_unused void *arg)
> return 0;
> }
>
> +static int set_threshold(struct thermal_zone *tz, __maybe_unused void *arg)
> +{
> + struct thermal_handler *th = arg;
> + int thresholds[] = { 43000, 65000, 49000, 55000, 57000 };
> + size_t i;

minor: I would just make it normal 'int'.

> +
> + INFO("Setting threshold for thermal zone '%s', id=%d\n", tz->name, tz->id);
> +
> + if (thermal_cmd_threshold_flush(th, tz)) {
> + ERROR("Failed to flush all previous thresholds\n");
> + return -1;
> + }
> +
> + for (i = 0; i < sizeof(thresholds) / sizeof(thresholds[0]); i++)
> + if (thermal_cmd_threshold_add(th, tz, thresholds[i],
> + THERMAL_THRESHOLD_WAY_UP |
> + THERMAL_THRESHOLD_WAY_DOWN)) {
> + ERROR("Failed to set threshold\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static int tz_create(const char *name, int tz_id, __maybe_unused void *arg)
> {
> INFO("Thermal zone '%s'/%d created\n", name, tz_id);
> @@ -197,20 +231,62 @@ static int gov_change(int tz_id, const char *name, __maybe_unused void *arg)
> return 0;
> }
>
> +static int threshold_add(int tz_id, int temp, int direction, __maybe_unused void *arg)
> +{
> + INFO("Threshold added tz_id=%d: temp=%d, direction=%d\n", tz_id, temp, direction);
> +
> + return 0;
> +}
> +
> +static int threshold_delete(int tz_id, int temp, int direction, __maybe_unused void *arg)
> +{
> + INFO("Threshold deleted tz_id=%d: temp=%d, direction=%d\n", tz_id, temp, direction);
> +
> + return 0;
> +}
> +
> +static int threshold_flush(int tz_id, __maybe_unused void *arg)
> +{
> + INFO("Thresholds flushed tz_id=%d\n", tz_id);
> +
> + return 0;
> +}
> +
> +static int threshold_up(int tz_id, int temp, int prev_temp, __maybe_unused void *arg)
> +{
> + INFO("Threshold crossed way up tz_id=%d: temp=%d, prev_temp=%d\n",
> + tz_id, temp, prev_temp);
> +
> + return 0;
> +}
> +
> +static int threshold_down(int tz_id, int temp, int prev_temp, __maybe_unused void *arg)
> +{
> + INFO("Threshold crossed way down tz_id=%d: temp=%d, prev_temp=%d\n",
> + tz_id, temp, prev_temp);
> +
> + return 0;
> +}
> +
> static struct thermal_ops ops = {
> - .events.tz_create = tz_create,
> - .events.tz_delete = tz_delete,
> - .events.tz_disable = tz_disable,
> - .events.tz_enable = tz_enable,
> - .events.trip_high = trip_high,
> - .events.trip_low = trip_low,
> - .events.trip_add = trip_add,
> - .events.trip_delete = trip_delete,
> - .events.trip_change = trip_change,
> - .events.cdev_add = cdev_add,
> - .events.cdev_delete = cdev_delete,
> - .events.cdev_update = cdev_update,
> - .events.gov_change = gov_change
> + .events.tz_create = tz_create,
> + .events.tz_delete = tz_delete,
> + .events.tz_disable = tz_disable,
> + .events.tz_enable = tz_enable,
> + .events.trip_high = trip_high,
> + .events.trip_low = trip_low,
> + .events.trip_add = trip_add,
> + .events.trip_delete = trip_delete,
> + .events.trip_change = trip_change,
> + .events.cdev_add = cdev_add,
> + .events.cdev_delete = cdev_delete,
> + .events.cdev_update = cdev_update,
> + .events.gov_change = gov_change,
> + .events.threshold_add = threshold_add,
> + .events.threshold_delete = threshold_delete,
> + .events.threshold_flush = threshold_flush,
> + .events.threshold_up = threshold_up,
> + .events.threshold_down = threshold_down,
> };
>
> static int thermal_event(__maybe_unused int fd, __maybe_unused void *arg)
> @@ -280,6 +356,7 @@ enum {
> THERMAL_ENGINE_DAEMON_ERROR,
> THERMAL_ENGINE_LOG_ERROR,
> THERMAL_ENGINE_THERMAL_ERROR,
> + THERMAL_ENGINE_THRESHOLD_ERROR,
> THERMAL_ENGINE_MAINLOOP_ERROR,
> };
>
> @@ -318,6 +395,8 @@ int main(int argc, char *argv[])
> return THERMAL_ENGINE_THERMAL_ERROR;
> }
>
> + for_each_thermal_zone(td.tz, set_threshold, td.th);
> +
> for_each_thermal_zone(td.tz, show_tz, td.th);
>
> if (mainloop_init()) {

LGTM,

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>