2024-04-10 17:48:02

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1 06/16] thermal: gov_power_allocator: Use .manage() callback instead of .throttle()

From: Rafael J. Wysocki <[email protected]>

The Power Allocator governor really only wants to be called once per
thermal zone update and it does a special check to skip the extra,
from its perspective, invocations of the .throttle() callback.

Make it use .manage() instead of .throttle().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/thermal/gov_power_allocator.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/thermal/gov_power_allocator.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_power_allocator.c
+++ linux-pm/drivers/thermal/gov_power_allocator.c
@@ -395,7 +395,7 @@ static void divvy_up_power(struct power_
}
}

-static int allocate_power(struct thermal_zone_device *tz, int control_temp)
+static void allocate_power(struct thermal_zone_device *tz, int control_temp)
{
struct power_allocator_params *params = tz->governor_data;
unsigned int num_actors = params->num_actors;
@@ -410,7 +410,7 @@ static int allocate_power(struct thermal
int i = 0, ret;

if (!num_actors)
- return -ENODEV;
+ return;

/* Clean all buffers for new power estimations */
memset(power, 0, params->buffer_size);
@@ -471,8 +471,6 @@ static int allocate_power(struct thermal
num_actors, power_range,
max_allocatable_power, tz->temperature,
control_temp - tz->temperature);
-
- return 0;
}

/**
@@ -745,40 +743,32 @@ static void power_allocator_unbind(struc
tz->governor_data = NULL;
}

-static int power_allocator_throttle(struct thermal_zone_device *tz,
- const struct thermal_trip *trip)
+static void power_allocator_manage(struct thermal_zone_device *tz)
{
struct power_allocator_params *params = tz->governor_data;
+ const struct thermal_trip *trip = params->trip_switch_on;
bool update;

lockdep_assert_held(&tz->lock);

- /*
- * We get called for every trip point but we only need to do
- * our calculations once
- */
- if (trip != params->trip_max)
- return 0;
-
- trip = params->trip_switch_on;
if (trip && tz->temperature < trip->temperature) {
update = tz->passive;
tz->passive = 0;
reset_pid_controller(params);
allow_maximum_power(tz, update);
- return 0;
+ return;
}

tz->passive = 1;

- return allocate_power(tz, params->trip_max->temperature);
+ allocate_power(tz, params->trip_max->temperature);
}

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,
+ .manage = power_allocator_manage,
.update_tz = power_allocator_update_tz,
};
THERMAL_GOVERNOR_DECLARE(thermal_gov_power_allocator);





2024-04-19 09:57:50

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v1 06/16] thermal: gov_power_allocator: Use .manage() callback instead of .throttle()



On 4/10/24 17:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The Power Allocator governor really only wants to be called once per
> thermal zone update and it does a special check to skip the extra,
> from its perspective, invocations of the .throttle() callback.
>
> Make it use .manage() instead of .throttle().
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/thermal/gov_power_allocator.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
> Index: linux-pm/drivers/thermal/gov_power_allocator.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_power_allocator.c
> +++ linux-pm/drivers/thermal/gov_power_allocator.c
> @@ -395,7 +395,7 @@ static void divvy_up_power(struct power_
> }
> }
>
> -static int allocate_power(struct thermal_zone_device *tz, int control_temp)
> +static void allocate_power(struct thermal_zone_device *tz, int control_temp)
> {
> struct power_allocator_params *params = tz->governor_data;
> unsigned int num_actors = params->num_actors;
> @@ -410,7 +410,7 @@ static int allocate_power(struct thermal
> int i = 0, ret;
>
> if (!num_actors)
> - return -ENODEV;
> + return;
>
> /* Clean all buffers for new power estimations */
> memset(power, 0, params->buffer_size);
> @@ -471,8 +471,6 @@ static int allocate_power(struct thermal
> num_actors, power_range,
> max_allocatable_power, tz->temperature,
> control_temp - tz->temperature);
> -
> - return 0;
> }
>
> /**
> @@ -745,40 +743,32 @@ static void power_allocator_unbind(struc
> tz->governor_data = NULL;
> }
>
> -static int power_allocator_throttle(struct thermal_zone_device *tz,
> - const struct thermal_trip *trip)
> +static void power_allocator_manage(struct thermal_zone_device *tz)
> {
> struct power_allocator_params *params = tz->governor_data;
> + const struct thermal_trip *trip = params->trip_switch_on;
> bool update;
>
> lockdep_assert_held(&tz->lock);
>
> - /*
> - * We get called for every trip point but we only need to do
> - * our calculations once
> - */
> - if (trip != params->trip_max)
> - return 0;
> -
> - trip = params->trip_switch_on;
> if (trip && tz->temperature < trip->temperature) {
> update = tz->passive;
> tz->passive = 0;
> reset_pid_controller(params);
> allow_maximum_power(tz, update);
> - return 0;
> + return;
> }
>
> tz->passive = 1;
>
> - return allocate_power(tz, params->trip_max->temperature);
> + allocate_power(tz, params->trip_max->temperature);
> }
>
> 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,
> + .manage = power_allocator_manage,
> .update_tz = power_allocator_update_tz,
> };
> THERMAL_GOVERNOR_DECLARE(thermal_gov_power_allocator);
>
>
>

LGTM

Reviewed-by: Lukasz Luba <[email protected]>