* [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device
2020-10-07 12:22 [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Lukasz Luba
@ 2020-10-07 12:22 ` Lukasz Luba
2020-10-14 12:31 ` Daniel Lezcano
2020-10-07 12:22 ` [PATCH 2/3] thermal: core: remove unused functions in power actor section Lukasz Luba
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Lukasz Luba @ 2020-10-07 12:22 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: daniel.lezcano, amitk, Dietmar.Eggemann, lukasz.luba,
michael.kao, rui.zhang
The thermal cooling device specified in DT might be instantiated for
a thermal zone trip point with a limited set of OPPs to operate on. This
configuration should be supported by Intelligent Power Allocation (IPA),
since it is a standard for other governors. Change the code and allow IPA
to get power value of lower and upper bound set for a given cooling
device.
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
drivers/thermal/gov_power_allocator.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index dd59085f38f5..f9ee7787b325 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -96,7 +96,8 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
if (instance->trip != params->trip_max_desired_temperature)
continue;
- if (power_actor_get_min_power(cdev, tz, &min_power))
+ if (cdev->ops->state2power(cdev, tz, instance->upper,
+ &min_power))
continue;
sustainable_power += min_power;
@@ -404,7 +405,8 @@ static int allocate_power(struct thermal_zone_device *tz,
weighted_req_power[i] = frac_to_int(weight * req_power[i]);
- if (power_actor_get_max_power(cdev, tz, &max_power[i]))
+ if (cdev->ops->state2power(cdev, tz, instance->lower,
+ &max_power[i]))
continue;
total_req_power += req_power[i];
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device
2020-10-07 12:22 ` [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device Lukasz Luba
@ 2020-10-14 12:31 ` Daniel Lezcano
2020-10-14 16:05 ` Lukasz Luba
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Lezcano @ 2020-10-14 12:31 UTC (permalink / raw)
To: Lukasz Luba, linux-kernel, linux-pm
Cc: amitk, Dietmar.Eggemann, michael.kao, rui.zhang
On 07/10/2020 14:22, Lukasz Luba wrote:
> The thermal cooling device specified in DT might be instantiated for
> a thermal zone trip point with a limited set of OPPs to operate on. This
> configuration should be supported by Intelligent Power Allocation (IPA),
> since it is a standard for other governors. Change the code and allow IPA
> to get power value of lower and upper bound set for a given cooling
> device.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
> drivers/thermal/gov_power_allocator.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index dd59085f38f5..f9ee7787b325 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -96,7 +96,8 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
> if (instance->trip != params->trip_max_desired_temperature)
> continue;
>
> - if (power_actor_get_min_power(cdev, tz, &min_power))
> + if (cdev->ops->state2power(cdev, tz, instance->upper,
> + &min_power))
if (cdev->ops->state2power && cdev->ops->state2power(cdev, tz,
instance->upper,
&min_power))
?
> continue;
>
> sustainable_power += min_power;
> @@ -404,7 +405,8 @@ static int allocate_power(struct thermal_zone_device *tz,
>
> weighted_req_power[i] = frac_to_int(weight * req_power[i]);
>
> - if (power_actor_get_max_power(cdev, tz, &max_power[i]))
> + if (cdev->ops->state2power(cdev, tz, instance->lower,
> + &max_power[i]))
> continue;
Same here ?
> total_req_power += req_power[i];
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device
2020-10-14 12:31 ` Daniel Lezcano
@ 2020-10-14 16:05 ` Lukasz Luba
0 siblings, 0 replies; 7+ messages in thread
From: Lukasz Luba @ 2020-10-14 16:05 UTC (permalink / raw)
To: Daniel Lezcano
Cc: linux-kernel, linux-pm, amitk, Dietmar.Eggemann, michael.kao, rui.zhang
On 10/14/20 1:31 PM, Daniel Lezcano wrote:
> On 07/10/2020 14:22, Lukasz Luba wrote:
>> The thermal cooling device specified in DT might be instantiated for
>> a thermal zone trip point with a limited set of OPPs to operate on. This
>> configuration should be supported by Intelligent Power Allocation (IPA),
>> since it is a standard for other governors. Change the code and allow IPA
>> to get power value of lower and upper bound set for a given cooling
>> device.
>>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>> drivers/thermal/gov_power_allocator.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
>> index dd59085f38f5..f9ee7787b325 100644
>> --- a/drivers/thermal/gov_power_allocator.c
>> +++ b/drivers/thermal/gov_power_allocator.c
>> @@ -96,7 +96,8 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
>> if (instance->trip != params->trip_max_desired_temperature)
>> continue;
>>
>> - if (power_actor_get_min_power(cdev, tz, &min_power))
>> + if (cdev->ops->state2power(cdev, tz, instance->upper,
>> + &min_power))
>
> if (cdev->ops->state2power && cdev->ops->state2power(cdev, tz,
> instance->upper,
> &min_power))
>
> ?
Yes, worth to check. I had this in [1] and missed it here while playing
with re-base of these patch series and other test branches.
I will send v2 with the needed cdev_is_power_actor() check.
>
>> continue;
>>
>> sustainable_power += min_power;
>> @@ -404,7 +405,8 @@ static int allocate_power(struct thermal_zone_device *tz,
>>
>> weighted_req_power[i] = frac_to_int(weight * req_power[i]);
>>
>> - if (power_actor_get_max_power(cdev, tz, &max_power[i]))
>> + if (cdev->ops->state2power(cdev, tz, instance->lower,
>> + &max_power[i]))
>> continue;
>
> Same here ?
Inside that loop we check (just a few lines above):
if (!cdev_is_power_actor(cdev))
continue;
then we call this:
if (cdev->ops->state2power(cdev, tz, instance->lower,
&max_power[i]))
So it should be safe.
>
>> total_req_power += req_power[i];
>>
>
>
Thank you Daniel for reviewing this.
Regards,
Lukasz
[1]
https://lore.kernel.org/linux-pm/20201008170426.465-3-lukasz.luba@arm.com/
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] thermal: core: remove unused functions in power actor section
2020-10-07 12:22 [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Lukasz Luba
2020-10-07 12:22 ` [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device Lukasz Luba
@ 2020-10-07 12:22 ` Lukasz Luba
2020-10-07 12:22 ` [PATCH 3/3] thermal: move power_actor_set_power into IPA Lukasz Luba
2020-10-14 12:45 ` [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Daniel Lezcano
3 siblings, 0 replies; 7+ messages in thread
From: Lukasz Luba @ 2020-10-07 12:22 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: daniel.lezcano, amitk, Dietmar.Eggemann, lukasz.luba,
michael.kao, rui.zhang
Since the Intelligent Power Allocation (IPA) uses different way to get
minimum and maximum power for a given cooling device, the helper functions
are not needed. There is no other code which uses them, so remove the
helper functions.
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
drivers/thermal/thermal_core.c | 49 ----------------------------------
drivers/thermal/thermal_core.h | 4 ---
2 files changed, 53 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 1cd8721327fb..6cba54929ef8 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -600,55 +600,6 @@ static void thermal_zone_device_check(struct work_struct *work)
* how to estimate their devices power consumption.
*/
-/**
- * power_actor_get_max_power() - get the maximum power that a cdev can consume
- * @cdev: pointer to &thermal_cooling_device
- * @tz: a valid thermal zone device pointer
- * @max_power: pointer in which to store the maximum power
- *
- * Calculate the maximum power consumption in milliwats that the
- * cooling device can currently consume and store it in @max_power.
- *
- * Return: 0 on success, -EINVAL if @cdev doesn't support the
- * power_actor API or -E* on other error.
- */
-int power_actor_get_max_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz, u32 *max_power)
-{
- if (!cdev_is_power_actor(cdev))
- return -EINVAL;
-
- return cdev->ops->state2power(cdev, tz, 0, max_power);
-}
-
-/**
- * power_actor_get_min_power() - get the mainimum power that a cdev can consume
- * @cdev: pointer to &thermal_cooling_device
- * @tz: a valid thermal zone device pointer
- * @min_power: pointer in which to store the minimum power
- *
- * Calculate the minimum power consumption in milliwatts that the
- * cooling device can currently consume and store it in @min_power.
- *
- * Return: 0 on success, -EINVAL if @cdev doesn't support the
- * power_actor API or -E* on other error.
- */
-int power_actor_get_min_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz, u32 *min_power)
-{
- unsigned long max_state;
- int ret;
-
- if (!cdev_is_power_actor(cdev))
- return -EINVAL;
-
- ret = cdev->ops->get_max_state(cdev, &max_state);
- if (ret)
- return ret;
-
- return cdev->ops->state2power(cdev, tz, max_state, min_power);
-}
-
/**
* power_actor_set_power() - limit the maximum power a cooling device consumes
* @cdev: pointer to &thermal_cooling_device
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index e00fc5585ea8..14f8a829a84a 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -65,10 +65,6 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
cdev->ops->power2state;
}
-int power_actor_get_max_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz, u32 *max_power);
-int power_actor_get_min_power(struct thermal_cooling_device *cdev,
- struct thermal_zone_device *tz, u32 *min_power);
int power_actor_set_power(struct thermal_cooling_device *cdev,
struct thermal_instance *ti, u32 power);
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] thermal: move power_actor_set_power into IPA
2020-10-07 12:22 [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Lukasz Luba
2020-10-07 12:22 ` [PATCH 1/3] thermal: power_allocator: respect upper and lower bounds for cooling device Lukasz Luba
2020-10-07 12:22 ` [PATCH 2/3] thermal: core: remove unused functions in power actor section Lukasz Luba
@ 2020-10-07 12:22 ` Lukasz Luba
2020-10-14 12:45 ` [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Daniel Lezcano
3 siblings, 0 replies; 7+ messages in thread
From: Lukasz Luba @ 2020-10-07 12:22 UTC (permalink / raw)
To: linux-kernel, linux-pm
Cc: daniel.lezcano, amitk, Dietmar.Eggemann, lukasz.luba,
michael.kao, rui.zhang
Since the power actor section has one function power_actor_set_power()
move it into Intelligent Power Allocation (IPA). There is no other user
of that helper function. It would also allow to remove the check of
cdev_is_power_actor() because the code which calls it in IPA already does
the needed check. Make the function static since only IPA use it.
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
drivers/thermal/gov_power_allocator.c | 32 +++++++++++++++++++++
drivers/thermal/thermal_core.c | 41 ---------------------------
drivers/thermal/thermal_core.h | 2 --
3 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index f9ee7787b325..5009eb012dfe 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -258,6 +258,38 @@ static u32 pid_controller(struct thermal_zone_device *tz,
return power_range;
}
+/**
+ * power_actor_set_power() - limit the maximum power a cooling device consumes
+ * @cdev: pointer to &thermal_cooling_device
+ * @instance: thermal instance to update
+ * @power: the power in milliwatts
+ *
+ * Set the cooling device to consume at most @power milliwatts. The limit is
+ * expected to be a cap at the maximum power consumption.
+ *
+ * Return: 0 on success, -EINVAL if the cooling device does not
+ * implement the power actor API or -E* for other failures.
+ */
+static int
+power_actor_set_power(struct thermal_cooling_device *cdev,
+ struct thermal_instance *instance, u32 power)
+{
+ unsigned long state;
+ int ret;
+
+ ret = cdev->ops->power2state(cdev, instance->tz, power, &state);
+ if (ret)
+ return ret;
+
+ instance->target = clamp_val(state, instance->lower, instance->upper);
+ mutex_lock(&cdev->lock);
+ cdev->updated = false;
+ mutex_unlock(&cdev->lock);
+ thermal_cdev_update(cdev);
+
+ return 0;
+}
+
/**
* divvy_up_power() - divvy the allocated power between the actors
* @req_power: each actor's requested power
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 6cba54929ef8..fd14e4b949f3 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -593,47 +593,6 @@ static void thermal_zone_device_check(struct work_struct *work)
thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
}
-/*
- * Power actor section: interface to power actors to estimate power
- *
- * Set of functions used to interact to cooling devices that know
- * how to estimate their devices power consumption.
- */
-
-/**
- * power_actor_set_power() - limit the maximum power a cooling device consumes
- * @cdev: pointer to &thermal_cooling_device
- * @instance: thermal instance to update
- * @power: the power in milliwatts
- *
- * Set the cooling device to consume at most @power milliwatts. The limit is
- * expected to be a cap at the maximum power consumption.
- *
- * Return: 0 on success, -EINVAL if the cooling device does not
- * implement the power actor API or -E* for other failures.
- */
-int power_actor_set_power(struct thermal_cooling_device *cdev,
- struct thermal_instance *instance, u32 power)
-{
- unsigned long state;
- int ret;
-
- if (!cdev_is_power_actor(cdev))
- return -EINVAL;
-
- ret = cdev->ops->power2state(cdev, instance->tz, power, &state);
- if (ret)
- return ret;
-
- instance->target = clamp_val(state, instance->lower, instance->upper);
- mutex_lock(&cdev->lock);
- cdev->updated = false;
- mutex_unlock(&cdev->lock);
- thermal_cdev_update(cdev);
-
- return 0;
-}
-
void thermal_zone_device_rebind_exception(struct thermal_zone_device *tz,
const char *cdev_type, size_t size)
{
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 14f8a829a84a..fc887d6f23ff 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -65,8 +65,6 @@ static inline bool cdev_is_power_actor(struct thermal_cooling_device *cdev)
cdev->ops->power2state;
}
-int power_actor_set_power(struct thermal_cooling_device *cdev,
- struct thermal_instance *ti, u32 power);
/**
* struct thermal_trip - representation of a point in temperature domain
* @np: pointer to struct device_node that this trip point was created from
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] Add upper and lower limits in IPA power budget calculation
2020-10-07 12:22 [PATCH 0/3] Add upper and lower limits in IPA power budget calculation Lukasz Luba
` (2 preceding siblings ...)
2020-10-07 12:22 ` [PATCH 3/3] thermal: move power_actor_set_power into IPA Lukasz Luba
@ 2020-10-14 12:45 ` Daniel Lezcano
3 siblings, 0 replies; 7+ messages in thread
From: Daniel Lezcano @ 2020-10-14 12:45 UTC (permalink / raw)
To: Lukasz Luba, linux-kernel, linux-pm
Cc: amitk, Dietmar.Eggemann, michael.kao, rui.zhang
On 07/10/2020 14:22, Lukasz Luba wrote:
> Hi all,
>
> This patch set makes thermal governor Intelligent Power Allocation (IPA)
> aware of cooling device limits for upper and lower bounds and respects them
> in the internal power budget calculation.
> The patch set should be applied on top of some already posted IPA changes [1][2].
>
> Regards,
> Lukasz
>
> [1] https://lore.kernel.org/linux-pm/20201002122416.13659-1-lukasz.luba@arm.com/
> [2] https://lore.kernel.org/linux-pm/9ecedd8a-fbc3-895c-d79c-f05af5c90ae5@arm.com/T/#t
>
> Lukasz Luba (3):
> thermal: power_allocator: respect upper and lower bounds for cooling
> device
> thermal: core: remove unused functions in power actor section
> thermal: move power_actor_set_power into IPA
>
> drivers/thermal/gov_power_allocator.c | 38 ++++++++++-
> drivers/thermal/thermal_core.c | 90 ---------------------------
> drivers/thermal/thermal_core.h | 6 --
> 3 files changed, 36 insertions(+), 98 deletions(-)
Thanks for this series. Except a comment in patch 1/3, it looks good to me.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 7+ messages in thread