2023-10-25 19:22:13

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 0/7] Minor cleanup for thermal gov power allocator

Hi all,

The patch set does some small clean up for Intelligent Power Allocator.
Those changes are not expected to alter the general functionality. They just
improve the code reading. Only patch 3/7 might improve the use case for
binding the governor to thermal zone (very unlikely in real products, but
it's needed for correctness).

The changes are based on top of current PM thermal branch, so with the
new trip points.

Regards,
Lukasz

Lukasz Luba (7):
thermal: gov_power_allocator: Rename trip_max_desired_temperature
thermal: gov_power_allocator: Setup trip points earlier
thermal: gov_power_allocator: Check the cooling devices only for
trip_max
thermal: gov_power_allocator: Rearrange the order of variables
thermal: gov_power_allocator: Use shorter variable when possible
thermal: gov_power_allocator: Remove unneeded local variables
thermal: gov_power_allocator: Clean needed variables at the beginning

drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
1 file changed, 64 insertions(+), 59 deletions(-)

--
2.25.1


2023-10-25 19:22:16

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 1/7] thermal: gov_power_allocator: Rename trip_max_desired_temperature

Refactor the code and drop the long name for the trip point. There is
a comment describing the field properly. Use shorten variable name so that
allow to make the code cleaner a bit.

This change is not expected to alter the general functionality.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/gov_power_allocator.c | 40 ++++++++++++---------------
1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 83d4f451b1a97..97a8a6e4e1b0b 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -59,9 +59,8 @@ static inline s64 div_frac(s64 x, s64 y)
* governor switches on when this trip point is crossed.
* If the thermal zone only has one passive trip point,
* @trip_switch_on should be NULL.
- * @trip_max_desired_temperature: last passive trip point of the thermal
- * zone. The temperature we are
- * controlling for.
+ * @trip_max: last passive trip point of the thermal zone. The
+ * temperature we are controlling for.
*/
struct power_allocator_params {
bool allocated_tzp;
@@ -69,7 +68,7 @@ struct power_allocator_params {
s32 prev_err;
u32 sustainable_power;
const struct thermal_trip *trip_switch_on;
- const struct thermal_trip *trip_max_desired_temperature;
+ const struct thermal_trip *trip_max;
};

/**
@@ -93,7 +92,7 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
struct thermal_cooling_device *cdev = instance->cdev;
u32 min_power;

- if (instance->trip != params->trip_max_desired_temperature)
+ if (instance->trip != params->trip_max)
continue;

if (!cdev_is_power_actor(cdev))
@@ -379,8 +378,7 @@ static int allocate_power(struct thermal_zone_device *tz,
{
struct thermal_instance *instance;
struct power_allocator_params *params = tz->governor_data;
- const struct thermal_trip *trip_max_desired_temperature =
- params->trip_max_desired_temperature;
+ const struct thermal_trip *trip_max = params->trip_max;
u32 *req_power, *max_power, *granted_power, *extra_actor_power;
u32 *weighted_req_power;
u32 total_req_power, max_allocatable_power, total_weighted_req_power;
@@ -390,7 +388,7 @@ static int allocate_power(struct thermal_zone_device *tz,
num_actors = 0;
total_weight = 0;
list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
- if ((instance->trip == trip_max_desired_temperature) &&
+ if ((instance->trip == trip_max) &&
cdev_is_power_actor(instance->cdev)) {
num_actors++;
total_weight += instance->weight;
@@ -429,7 +427,7 @@ static int allocate_power(struct thermal_zone_device *tz,
int weight;
struct thermal_cooling_device *cdev = instance->cdev;

- if (instance->trip != trip_max_desired_temperature)
+ if (instance->trip != trip_max)
continue;

if (!cdev_is_power_actor(cdev))
@@ -465,7 +463,7 @@ static int allocate_power(struct thermal_zone_device *tz,
total_granted_power = 0;
i = 0;
list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
- if (instance->trip != trip_max_desired_temperature)
+ if (instance->trip != trip_max)
continue;

if (!cdev_is_power_actor(instance->cdev))
@@ -531,13 +529,13 @@ static void get_governor_trips(struct thermal_zone_device *tz,

if (last_passive) {
params->trip_switch_on = first_passive;
- params->trip_max_desired_temperature = last_passive;
+ params->trip_max = last_passive;
} else if (first_passive) {
params->trip_switch_on = NULL;
- params->trip_max_desired_temperature = first_passive;
+ params->trip_max = first_passive;
} else {
params->trip_switch_on = NULL;
- params->trip_max_desired_temperature = last_active;
+ params->trip_max = last_active;
}
}

@@ -556,8 +554,8 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update)
list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
struct thermal_cooling_device *cdev = instance->cdev;

- if (instance->trip != params->trip_max_desired_temperature ||
- (!cdev_is_power_actor(instance->cdev)))
+ if (instance->trip != params->trip_max ||
+ !cdev_is_power_actor(instance->cdev))
continue;

instance->target = 0;
@@ -642,12 +640,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)

get_governor_trips(tz, params);

- if (params->trip_max_desired_temperature) {
- int temp = params->trip_max_desired_temperature->temperature;
-
+ if (params->trip_max)
estimate_pid_constants(tz, tz->tzp->sustainable_power,
- params->trip_switch_on, temp);
- }
+ params->trip_switch_on,
+ params->trip_max->temperature);

reset_pid_controller(params);

@@ -688,7 +684,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz,
* We get called for every trip point but we only need to do
* our calculations once
*/
- if (trip != params->trip_max_desired_temperature)
+ if (trip != params->trip_max)
return 0;

trip = params->trip_switch_on;
@@ -702,7 +698,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz,

tz->passive = 1;

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

static struct thermal_governor thermal_gov_power_allocator = {
--
2.25.1

2023-10-25 19:22:17

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 2/7] thermal: gov_power_allocator: Setup trip points earlier

Setup the trip points at the beginning of the binding function. This
simplifies the code a bit and allows further cleaning. Add also the
check if the last passive trip point is found and fail binding if not.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/gov_power_allocator.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 97a8a6e4e1b0b..0dfc5b5ab5235 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -617,14 +617,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
int ret;
struct power_allocator_params *params;

- ret = check_power_actors(tz);
- if (ret)
- return ret;
-
params = kzalloc(sizeof(*params), GFP_KERNEL);
if (!params)
return -ENOMEM;

+ get_governor_trips(tz, params);
+ if (!params->trip_max) {
+ dev_warn(&tz->device, "power_allocator: missing trip_max\n");
+ kfree(params);
+ return -EINVAL;
+ }
+
+ ret = check_power_actors(tz);
+ if (ret) {
+ dev_warn(&tz->device, "power_allocator: binding failed\n");
+ kfree(params);
+ return ret;
+ }
+
if (!tz->tzp) {
tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL);
if (!tz->tzp) {
@@ -638,12 +648,9 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
if (!tz->tzp->sustainable_power)
dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n");

- get_governor_trips(tz, params);
-
- if (params->trip_max)
- estimate_pid_constants(tz, tz->tzp->sustainable_power,
- params->trip_switch_on,
- params->trip_max->temperature);
+ estimate_pid_constants(tz, tz->tzp->sustainable_power,
+ params->trip_switch_on,
+ params->trip_max->temperature);

reset_pid_controller(params);

--
2.25.1

2023-10-25 19:22:24

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 5/7] thermal: gov_power_allocator: Use shorter variable when possible

The 'cdev' pointer is initialed, so there is no need to use
'instance->cdev'.

This change is not expected to alter the general functionality.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/gov_power_allocator.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 79621b42ead38..0f7f8278eacc5 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -560,7 +560,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update)
continue;

instance->target = 0;
- mutex_lock(&instance->cdev->lock);
+ mutex_lock(&cdev->lock);
/*
* Call for updating the cooling devices local stats and avoid
* periods of dozen of seconds when those have not been
@@ -569,9 +569,9 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update)
cdev->ops->get_requested_power(cdev, &req_power);

if (update)
- __thermal_cdev_update(instance->cdev);
+ __thermal_cdev_update(cdev);

- mutex_unlock(&instance->cdev->lock);
+ mutex_unlock(&cdev->lock);
}
}

--
2.25.1

2023-10-25 19:22:43

by Lukasz Luba

[permalink] [raw]
Subject: [PATCH 7/7] thermal: gov_power_allocator: Clean needed variables at the beginning

Rearrange the local variables setup. This improves the reading of the code
and allows to better see the initial values;

This change is not expected to alter the general functionality.

Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/thermal/gov_power_allocator.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index e6d2f0fe8d2fd..785fff14223d8 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -376,16 +376,19 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors,

static int allocate_power(struct thermal_zone_device *tz, int control_temp)
{
- u32 total_req_power, max_allocatable_power, total_weighted_req_power;
u32 *req_power, *max_power, *granted_power, *extra_actor_power;
struct power_allocator_params *params = tz->governor_data;
- u32 total_granted_power, power_range;
struct thermal_cooling_device *cdev;
struct thermal_instance *instance;
+ u32 total_weighted_req_power = 0;
+ u32 max_allocatable_power = 0;
+ u32 total_granted_power = 0;
+ u32 total_req_power = 0;
u32 *weighted_req_power;
+ u32 power_range, weight;
int total_weight = 0;
int num_actors = 0;
- int i, weight;
+ int i = 0;

list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
if ((instance->trip == params->trip_max) &&
@@ -418,11 +421,6 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp)
extra_actor_power = &req_power[3 * num_actors];
weighted_req_power = &req_power[4 * num_actors];

- i = 0;
- total_weighted_req_power = 0;
- total_req_power = 0;
- max_allocatable_power = 0;
-
list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
cdev = instance->cdev;

@@ -459,7 +457,6 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp)
total_weighted_req_power, power_range, granted_power,
extra_actor_power);

- total_granted_power = 0;
i = 0;
list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
if (instance->trip != params->trip_max)
--
2.25.1

2023-10-26 08:55:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator

On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
>
> Hi all,
>
> The patch set does some small clean up for Intelligent Power Allocator.
> Those changes are not expected to alter the general functionality. They just
> improve the code reading. Only patch 3/7 might improve the use case for
> binding the governor to thermal zone (very unlikely in real products, but
> it's needed for correctness).
>
> The changes are based on top of current PM thermal branch, so with the
> new trip points.
>
> Regards,
> Lukasz
>
> Lukasz Luba (7):
> thermal: gov_power_allocator: Rename trip_max_desired_temperature
> thermal: gov_power_allocator: Setup trip points earlier
> thermal: gov_power_allocator: Check the cooling devices only for
> trip_max
> thermal: gov_power_allocator: Rearrange the order of variables
> thermal: gov_power_allocator: Use shorter variable when possible
> thermal: gov_power_allocator: Remove unneeded local variables
> thermal: gov_power_allocator: Clean needed variables at the beginning
>
> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
> 1 file changed, 64 insertions(+), 59 deletions(-)
>
> --

The series looks good to me overall, but I'd prefer to make these
changes in the 6.8 cycle, because the 6.7 merge window is around the
corner and there is quite a bit of thermal material in this cycle
already.

2023-10-26 12:22:22

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator



On 10/26/23 09:54, Rafael J. Wysocki wrote:
> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
>>
>> Hi all,
>>
>> The patch set does some small clean up for Intelligent Power Allocator.
>> Those changes are not expected to alter the general functionality. They just
>> improve the code reading. Only patch 3/7 might improve the use case for
>> binding the governor to thermal zone (very unlikely in real products, but
>> it's needed for correctness).
>>
>> The changes are based on top of current PM thermal branch, so with the
>> new trip points.
>>
>> Regards,
>> Lukasz
>>
>> Lukasz Luba (7):
>> thermal: gov_power_allocator: Rename trip_max_desired_temperature
>> thermal: gov_power_allocator: Setup trip points earlier
>> thermal: gov_power_allocator: Check the cooling devices only for
>> trip_max
>> thermal: gov_power_allocator: Rearrange the order of variables
>> thermal: gov_power_allocator: Use shorter variable when possible
>> thermal: gov_power_allocator: Remove unneeded local variables
>> thermal: gov_power_allocator: Clean needed variables at the beginning
>>
>> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
>> 1 file changed, 64 insertions(+), 59 deletions(-)
>>
>> --
>
> The series looks good to me overall, but I'd prefer to make these
> changes in the 6.8 cycle, because the 6.7 merge window is around the
> corner and there is quite a bit of thermal material in this cycle
> already.

Thanks for having a look! Yes, I agree, we can wait after the
merge window. It just have to be cleaned one day a bit and I postponed
this a few times, so no rush ;)

2023-11-23 15:19:12

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator

Hi Rafael,

Gentle ping

On 10/26/23 13:22, Lukasz Luba wrote:
>
>
> On 10/26/23 09:54, Rafael J. Wysocki wrote:
>> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
>>>
>>> Hi all,
>>>
>>> The patch set does some small clean up for Intelligent Power Allocator.
>>> Those changes are not expected to alter the general functionality.
>>> They just
>>> improve the code reading. Only patch 3/7 might improve the use case for
>>> binding the governor to thermal zone (very unlikely in real products,
>>> but
>>> it's needed for correctness).
>>>
>>> The changes are based on top of current PM thermal branch, so with the
>>> new trip points.
>>>
>>> Regards,
>>> Lukasz
>>>
>>> Lukasz Luba (7):
>>>    thermal: gov_power_allocator: Rename trip_max_desired_temperature
>>>    thermal: gov_power_allocator: Setup trip points earlier
>>>    thermal: gov_power_allocator: Check the cooling devices only for
>>>      trip_max
>>>    thermal: gov_power_allocator: Rearrange the order of variables
>>>    thermal: gov_power_allocator: Use shorter variable when possible
>>>    thermal: gov_power_allocator: Remove unneeded local variables
>>>    thermal: gov_power_allocator: Clean needed variables at the beginning
>>>
>>>   drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
>>>   1 file changed, 64 insertions(+), 59 deletions(-)
>>>
>>> --
>>
>> The series looks good to me overall, but I'd prefer to make these
>> changes in the 6.8 cycle, because the 6.7 merge window is around the
>> corner and there is quite a bit of thermal material in this cycle
>> already.
>
> Thanks for having a look! Yes, I agree, we can wait after the
> merge window. It just have to be cleaned one day a bit and I postponed
> this a few times, so no rush ;)

I've seen you've created the new pm/thermal. Could you consider to take
those in, please?

I would send some RFC on top showing the issue with reading back the CPU
max frequency from the PM_QoS chain.

Regards,
Lukasz

2023-11-23 19:50:27

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator

Hi Lukasz,

On Thu, Nov 23, 2023 at 4:19 PM Lukasz Luba <[email protected]> wrote:
>
> Hi Rafael,
>
> Gentle ping
>
> On 10/26/23 13:22, Lukasz Luba wrote:
> >
> >
> > On 10/26/23 09:54, Rafael J. Wysocki wrote:
> >> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
> >>>
> >>> Hi all,
> >>>
> >>> The patch set does some small clean up for Intelligent Power Allocator.
> >>> Those changes are not expected to alter the general functionality.
> >>> They just
> >>> improve the code reading. Only patch 3/7 might improve the use case for
> >>> binding the governor to thermal zone (very unlikely in real products,
> >>> but
> >>> it's needed for correctness).
> >>>
> >>> The changes are based on top of current PM thermal branch, so with the
> >>> new trip points.
> >>>
> >>> Regards,
> >>> Lukasz
> >>>
> >>> Lukasz Luba (7):
> >>> thermal: gov_power_allocator: Rename trip_max_desired_temperature
> >>> thermal: gov_power_allocator: Setup trip points earlier
> >>> thermal: gov_power_allocator: Check the cooling devices only for
> >>> trip_max
> >>> thermal: gov_power_allocator: Rearrange the order of variables
> >>> thermal: gov_power_allocator: Use shorter variable when possible
> >>> thermal: gov_power_allocator: Remove unneeded local variables
> >>> thermal: gov_power_allocator: Clean needed variables at the beginning
> >>>
> >>> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
> >>> 1 file changed, 64 insertions(+), 59 deletions(-)
> >>>
> >>> --
> >>
> >> The series looks good to me overall, but I'd prefer to make these
> >> changes in the 6.8 cycle, because the 6.7 merge window is around the
> >> corner and there is quite a bit of thermal material in this cycle
> >> already.
> >
> > Thanks for having a look! Yes, I agree, we can wait after the
> > merge window. It just have to be cleaned one day a bit and I postponed
> > this a few times, so no rush ;)
>
> I've seen you've created the new pm/thermal. Could you consider to take
> those in, please?

Sure, I'll get to them presumably tomorrow and if not then early next week.

> I would send some RFC on top showing the issue with reading back the CPU
> max frequency from the PM_QoS chain.

Sounds good.

2023-11-24 07:45:11

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator



On 11/23/23 19:50, Rafael J. Wysocki wrote:
> Hi Lukasz,
>
> On Thu, Nov 23, 2023 at 4:19 PM Lukasz Luba <[email protected]> wrote:
>>
>> Hi Rafael,
>>
>> Gentle ping
>>
>> On 10/26/23 13:22, Lukasz Luba wrote:
>>>
>>>
>>> On 10/26/23 09:54, Rafael J. Wysocki wrote:
>>>> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> The patch set does some small clean up for Intelligent Power Allocator.
>>>>> Those changes are not expected to alter the general functionality.
>>>>> They just
>>>>> improve the code reading. Only patch 3/7 might improve the use case for
>>>>> binding the governor to thermal zone (very unlikely in real products,
>>>>> but
>>>>> it's needed for correctness).
>>>>>
>>>>> The changes are based on top of current PM thermal branch, so with the
>>>>> new trip points.
>>>>>
>>>>> Regards,
>>>>> Lukasz
>>>>>
>>>>> Lukasz Luba (7):
>>>>> thermal: gov_power_allocator: Rename trip_max_desired_temperature
>>>>> thermal: gov_power_allocator: Setup trip points earlier
>>>>> thermal: gov_power_allocator: Check the cooling devices only for
>>>>> trip_max
>>>>> thermal: gov_power_allocator: Rearrange the order of variables
>>>>> thermal: gov_power_allocator: Use shorter variable when possible
>>>>> thermal: gov_power_allocator: Remove unneeded local variables
>>>>> thermal: gov_power_allocator: Clean needed variables at the beginning
>>>>>
>>>>> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
>>>>> 1 file changed, 64 insertions(+), 59 deletions(-)
>>>>>
>>>>> --
>>>>
>>>> The series looks good to me overall, but I'd prefer to make these
>>>> changes in the 6.8 cycle, because the 6.7 merge window is around the
>>>> corner and there is quite a bit of thermal material in this cycle
>>>> already.
>>>
>>> Thanks for having a look! Yes, I agree, we can wait after the
>>> merge window. It just have to be cleaned one day a bit and I postponed
>>> this a few times, so no rush ;)
>>
>> I've seen you've created the new pm/thermal. Could you consider to take
>> those in, please?
>
> Sure, I'll get to them presumably tomorrow and if not then early next week.

OK, thank you Rafael!

>
>> I would send some RFC on top showing the issue with reading back the CPU
>> max frequency from the PM_QoS chain.
>
> Sounds good.

2023-11-28 15:17:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator

On Fri, Nov 24, 2023 at 8:44 AM Lukasz Luba <[email protected]> wrote:
>
>
>
> On 11/23/23 19:50, Rafael J. Wysocki wrote:
> > Hi Lukasz,
> >
> > On Thu, Nov 23, 2023 at 4:19 PM Lukasz Luba <[email protected]> wrote:
> >>
> >> Hi Rafael,
> >>
> >> Gentle ping
> >>
> >> On 10/26/23 13:22, Lukasz Luba wrote:
> >>>
> >>>
> >>> On 10/26/23 09:54, Rafael J. Wysocki wrote:
> >>>> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
> >>>>>
> >>>>> Hi all,
> >>>>>
> >>>>> The patch set does some small clean up for Intelligent Power Allocator.
> >>>>> Those changes are not expected to alter the general functionality.
> >>>>> They just
> >>>>> improve the code reading. Only patch 3/7 might improve the use case for
> >>>>> binding the governor to thermal zone (very unlikely in real products,
> >>>>> but
> >>>>> it's needed for correctness).
> >>>>>
> >>>>> The changes are based on top of current PM thermal branch, so with the
> >>>>> new trip points.
> >>>>>
> >>>>> Regards,
> >>>>> Lukasz
> >>>>>
> >>>>> Lukasz Luba (7):
> >>>>> thermal: gov_power_allocator: Rename trip_max_desired_temperature
> >>>>> thermal: gov_power_allocator: Setup trip points earlier
> >>>>> thermal: gov_power_allocator: Check the cooling devices only for
> >>>>> trip_max
> >>>>> thermal: gov_power_allocator: Rearrange the order of variables
> >>>>> thermal: gov_power_allocator: Use shorter variable when possible
> >>>>> thermal: gov_power_allocator: Remove unneeded local variables
> >>>>> thermal: gov_power_allocator: Clean needed variables at the beginning
> >>>>>
> >>>>> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
> >>>>> 1 file changed, 64 insertions(+), 59 deletions(-)
> >>>>>
> >>>>> --
> >>>>
> >>>> The series looks good to me overall, but I'd prefer to make these
> >>>> changes in the 6.8 cycle, because the 6.7 merge window is around the
> >>>> corner and there is quite a bit of thermal material in this cycle
> >>>> already.
> >>>
> >>> Thanks for having a look! Yes, I agree, we can wait after the
> >>> merge window. It just have to be cleaned one day a bit and I postponed
> >>> this a few times, so no rush ;)
> >>
> >> I've seen you've created the new pm/thermal. Could you consider to take
> >> those in, please?
> >
> > Sure, I'll get to them presumably tomorrow and if not then early next week.
>
> OK, thank you Rafael!

I've queued up the whole lot for 6.8 and in the process I've edited
all of the changelogs and some subjects for clarity and English
grammar improvements.

Thanks!

2023-11-28 15:31:17

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH 0/7] Minor cleanup for thermal gov power allocator



On 11/28/23 15:17, Rafael J. Wysocki wrote:
> On Fri, Nov 24, 2023 at 8:44 AM Lukasz Luba <[email protected]> wrote:
>>
>>
>>
>> On 11/23/23 19:50, Rafael J. Wysocki wrote:
>>> Hi Lukasz,
>>>
>>> On Thu, Nov 23, 2023 at 4:19 PM Lukasz Luba <[email protected]> wrote:
>>>>
>>>> Hi Rafael,
>>>>
>>>> Gentle ping
>>>>
>>>> On 10/26/23 13:22, Lukasz Luba wrote:
>>>>>
>>>>>
>>>>> On 10/26/23 09:54, Rafael J. Wysocki wrote:
>>>>>> On Wed, Oct 25, 2023 at 9:21 PM Lukasz Luba <[email protected]> wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> The patch set does some small clean up for Intelligent Power Allocator.
>>>>>>> Those changes are not expected to alter the general functionality.
>>>>>>> They just
>>>>>>> improve the code reading. Only patch 3/7 might improve the use case for
>>>>>>> binding the governor to thermal zone (very unlikely in real products,
>>>>>>> but
>>>>>>> it's needed for correctness).
>>>>>>>
>>>>>>> The changes are based on top of current PM thermal branch, so with the
>>>>>>> new trip points.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Lukasz
>>>>>>>
>>>>>>> Lukasz Luba (7):
>>>>>>> thermal: gov_power_allocator: Rename trip_max_desired_temperature
>>>>>>> thermal: gov_power_allocator: Setup trip points earlier
>>>>>>> thermal: gov_power_allocator: Check the cooling devices only for
>>>>>>> trip_max
>>>>>>> thermal: gov_power_allocator: Rearrange the order of variables
>>>>>>> thermal: gov_power_allocator: Use shorter variable when possible
>>>>>>> thermal: gov_power_allocator: Remove unneeded local variables
>>>>>>> thermal: gov_power_allocator: Clean needed variables at the beginning
>>>>>>>
>>>>>>> drivers/thermal/gov_power_allocator.c | 123 ++++++++++++++------------
>>>>>>> 1 file changed, 64 insertions(+), 59 deletions(-)
>>>>>>>
>>>>>>> --
>>>>>>
>>>>>> The series looks good to me overall, but I'd prefer to make these
>>>>>> changes in the 6.8 cycle, because the 6.7 merge window is around the
>>>>>> corner and there is quite a bit of thermal material in this cycle
>>>>>> already.
>>>>>
>>>>> Thanks for having a look! Yes, I agree, we can wait after the
>>>>> merge window. It just have to be cleaned one day a bit and I postponed
>>>>> this a few times, so no rush ;)
>>>>
>>>> I've seen you've created the new pm/thermal. Could you consider to take
>>>> those in, please?
>>>
>>> Sure, I'll get to them presumably tomorrow and if not then early next week.
>>
>> OK, thank you Rafael!
>
> I've queued up the whole lot for 6.8 and in the process I've edited
> all of the changelogs and some subjects for clarity and English
> grammar improvements.

Thank you and I really appreciate the improvements!