lkml.org 
[lkml]   [2023]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v1 4/6] thermal: gov_power_allocator: Use trip pointers instead of trip indices
From


On 10/6/23 18:47, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Modify the power allocator thermal governor to use trip pointers instead
> of trip indices everywhere except for the power_allocator_throttle()
> second argument that will be changed subsequently along with the
> definition of the .throttle() governor callback.
>
> The general functionality is not expected to be changed.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/thermal/gov_power_allocator.c | 123 +++++++++++++---------------------
> 1 file changed, 49 insertions(+), 74 deletions(-)
>

[snip]

> @@ -636,7 +619,6 @@ static int power_allocator_bind(struct t
> {
> int ret;
> struct power_allocator_params *params;
> - struct thermal_trip trip;
>
> ret = check_power_actors(tz);
> if (ret)
> @@ -662,12 +644,13 @@ static int power_allocator_bind(struct t
> get_governor_trips(tz, params);
>
> if (tz->num_trips > 0) {
> - ret = __thermal_zone_get_trip(tz, params->trip_max_desired_temperature,
> - &trip);
> - if (!ret)
> + const struct thermal_trip *trip;
> +
> + trip = params->trip_max_desired_temperature;
> + if (trip)
> estimate_pid_constants(tz, tz->tzp->sustainable_power,
> params->trip_switch_on,
> - trip.temperature);
> + trip->temperature);
> }

The code check for the populated pointer (by earlier new
get_governor_trips(tz, params)) :

if (params->trip_max_desired_temperature) {
int temp = params->trip_max_desired_temperature->temperature;

estimate_pid_constants(...)
}

looks better (what you have figured out already).

Other than that the patch LGTM:

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

I have also tested this is a few ways and it still works as expected:
1. Power allocation in different conditions
2. Trip points properly recognized with this nice get_governor_trips(),
here is the test output, with many tricky trip point setups:

use case A:
5 trip points:
- 2 passive for 50, 60 degC
- 1 active 70degC
- 2 passive 70, 85 degC
- 1 critical 120 degC

expected IPA trip points: 50, 85 deg => 35 degC IPA operating range

[ 24.578806] Power allocator: IPA: trip->temperature=50000
[ 24.578824] Power allocator: IPA: passive trip->temperature=50000
[ 24.578838] Power allocator: IPA: fist passive trip->temperature=50000
[ 24.578851] Power allocator: IPA: trip->temperature=60000
[ 24.578863] Power allocator: IPA: passive trip->temperature=60000
[ 24.578875] Power allocator: IPA: trip->temperature=70000
[ 24.578888] Power allocator: IPA: active trip->temperature=70000
[ 24.578900] Power allocator: IPA: trip->temperature=120000
[ 24.578912] Power allocator: IPA: trip->temperature=70000
[ 24.578925] Power allocator: IPA: passive trip->temperature=70000
[ 24.578937] Power allocator: IPA: trip->temperature=85000
[ 24.578950] Power allocator: IPA: passive trip->temperature=85000
[ 24.578964] Power allocator: IPA: trip_switch_on->temperature=50000
control_temp=85000
[ 24.578978] Power allocator: IPA: temperature_threshold=35000

-------------------------------------------------------------
use case B:
5 trip points:
- 2 active for 50, 60 degC
- 1 active 70degC
- 2 passive 70, 85 degC
- 1 critical 120 degC

expected IPA trip points: 70, 85 deg => 15 degC IPA operating range

[ 27.402474] Power allocator: IPA: trip->temperature=50000
[ 27.402492] Power allocator: IPA: active trip->temperature=50000
[ 27.402505] Power allocator: IPA: trip->temperature=60000
[ 27.402518] Power allocator: IPA: active trip->temperature=60000
[ 27.402531] Power allocator: IPA: trip->temperature=70000
[ 27.402544] Power allocator: IPA: active trip->temperature=70000
[ 27.402557] Power allocator: IPA: trip->temperature=120000
[ 27.402570] Power allocator: IPA: trip->temperature=70000
[ 27.402582] Power allocator: IPA: passive trip->temperature=70000
[ 27.402596] Power allocator: IPA: fist passive trip->temperature=70000
[ 27.402608] Power allocator: IPA: trip->temperature=85000
[ 27.402622] Power allocator: IPA: passive trip->temperature=85000
[ 27.402635] Power allocator: IPA: trip_switch_on->temperature=70000
control_temp=85000
[ 27.402649] Power allocator: IPA: temperature_threshold=15000

--------------------------------------------------------
use case C:
6 trip points:
- 2 active for 50, 60 degC
- 1 active 70degC
- 3 passive 70, 85, 90 degC
- 1 critical 120 degC

expected IPA trip points: 50, 85 deg => 20 degC IPA operating range

[ 36.998907] Power allocator: IPA: trip->temperature=50000
[ 36.998921] Power allocator: IPA: active trip->temperature=50000
[ 36.998935] Power allocator: IPA: trip->temperature=60000
[ 36.998948] Power allocator: IPA: active trip->temperature=60000
[ 36.998960] Power allocator: IPA: trip->temperature=70000
[ 36.998973] Power allocator: IPA: active trip->temperature=70000
[ 36.998985] Power allocator: IPA: trip->temperature=120000
[ 36.998999] Power allocator: IPA: trip->temperature=70000
[ 36.999011] Power allocator: IPA: passive trip->temperature=70000
[ 36.999024] Power allocator: IPA: fist passive trip->temperature=70000
[ 36.999037] Power allocator: IPA: trip->temperature=85000
[ 36.999049] Power allocator: IPA: passive trip->temperature=85000
[ 36.999062] Power allocator: IPA: trip->temperature=90000
[ 36.999074] Power allocator: IPA: passive trip->temperature=90000
[ 36.999087] Power allocator: IPA: trip_switch_on->temperature=70000
control_temp=90000
[ 36.999101] Power allocator: IPA: temperature_threshold=20000

\
 
 \ /
  Last update: 2023-10-20 18:36    [W:0.223 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site