diff mbox series

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

Message ID 20241014094309.1430126-5-daniel.lezcano@linaro.org (mailing list archive)
State Superseded, archived
Headers show
Series Add thermal user thresholds support | expand

Commit Message

Daniel Lezcano Oct. 14, 2024, 9:43 a.m. UTC
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(-)

Comments

Lukasz Luba Oct. 21, 2024, 8:10 p.m. UTC | #1
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>
Daniel Lezcano Oct. 22, 2024, 7:52 a.m. UTC | #2
On 21/10/2024 22:10, Lukasz Luba wrote:
> 
> 
> 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'.

Why ? :)

The for loop compares i < sizeof(thresholds) / sizeof(thresholds[0])

sizeof() returns size_t

So we are comparing the same type.


>> +
>> +    INFO("Setting threshold for thermal zone '%s', id=%d\n", tz- 
>> >name, tz->id);
>> +

[ ... ]

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


Thanks for the review
diff mbox series

Patch

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;
+
+	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()) {