[PATCH v2 0/4] Add upper and lower limits in IPA power budget calculation

[PATCH v2 0/4] Add upper and lower limits in IPA power budget calculation

[Lukasz Luba]

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.

This v2 contains for completeness a patch posted by Michael. The patches
are re-based on top of current thermal/linux-next branch.

Changes:
v2:
- added check for cdev_is_power_actor(), before using cdev->ops->state2power()
  pointed out by Daniel
- added Michael's patch for consistency
- re-based on top of current thermal/linux-next
v1:
 can be found here [2]

Regards,
Lukasz

[1] https://lore.kernel.org/linux-pm/20201007024332.30322-1-michael.kao@mediatek.com/
[2] https://lore.kernel.org/linux-pm/20201007122256.28080-1-lukasz.luba@arm.com/

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

Michael Kao (1):
  thermal: core: add upper and lower limits to power_actor_set_power

 drivers/thermal/gov_power_allocator.c | 40 +++++++++++-
 drivers/thermal/thermal_core.c        | 88 ---------------------------
 drivers/thermal/thermal_core.h        |  6 --
 3 files changed, 38 insertions(+), 96 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] thermal: core: add upper and lower limits to power_actor_set_power

[Lukasz Luba]

From: Michael Kao <michael.kao@mediatek.com>

The upper and lower limits of thermal throttle state in the
DT do not apply to the Intelligent Power Allocation (IPA) governor.
Add the clamping for cooling device upper and lower limits in the
power_actor_set_power() used by IPA.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Tested-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 drivers/thermal/thermal_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index c6d74bc1c90b..2ea3633b5d66 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -672,7 +672,7 @@ int power_actor_set_power(struct thermal_cooling_device *cdev,
 	if (ret)
 		return ret;
 
-	instance->target = state;
+	instance->target = clamp_val(state, instance->lower, instance->upper);
 	mutex_lock(&cdev->lock);
 	cdev->updated = false;
 	mutex_unlock(&cdev->lock);
-- 
2.17.1

[PATCH v2 2/4] thermal: power_allocator: respect upper and lower bounds for cooling device

[Lukasz Luba]

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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index ab0be26f0816..eb8c9afadf19 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -96,7 +96,10 @@ 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, &min_power))
+		if (!cdev_is_power_actor(cdev))
+			continue;
+
+		if (cdev->ops->state2power(cdev, instance->upper, &min_power))
 			continue;
 
 		sustainable_power += min_power;
@@ -398,7 +401,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, &max_power[i]))
+		if (cdev->ops->state2power(cdev, instance->lower,
+					   &max_power[i]))
 			continue;
 
 		total_req_power += req_power[i];
-- 
2.17.1

[PATCH v2 3/4] thermal: core: remove unused functions in power actor section

[Lukasz Luba]

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 | 47 ----------------------------------
 drivers/thermal/thermal_core.h |  4 ---
 2 files changed, 51 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 2ea3633b5d66..d5540bfeee5e 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -600,53 +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
- * @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,
-			      u32 *max_power)
-{
-	if (!cdev_is_power_actor(cdev))
-		return -EINVAL;
-
-	return cdev->ops->state2power(cdev, 0, max_power);
-}
-
-/**
- * power_actor_get_min_power() - get the mainimum power that a cdev can consume
- * @cdev:	pointer to &thermal_cooling_device
- * @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,
-			      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, 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 764c2de31771..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,
-			      u32 *max_power);
-int power_actor_get_min_power(struct thermal_cooling_device *cdev,
-			      u32 *min_power);
 int power_actor_set_power(struct thermal_cooling_device *cdev,
 			  struct thermal_instance *ti, u32 power);
 /**
-- 
2.17.1

[PATCH v2 4/4] thermal: move power_actor_set_power into IPA

[Lukasz Luba]

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 eb8c9afadf19..b29e21c56a4f 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -254,6 +254,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, 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 d5540bfeee5e..96349ba59725 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, 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

Re: [PATCH v2 0/4] Add upper and lower limits in IPA power budget calculation

[Daniel Lezcano]

Applied, thanks

On 15/10/2020 13:24, 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.
> 
> This v2 contains for completeness a patch posted by Michael. The patches
> are re-based on top of current thermal/linux-next branch.
> 
> Changes:
> v2:
> - added check for cdev_is_power_actor(), before using cdev->ops->state2power()
>   pointed out by Daniel
> - added Michael's patch for consistency
> - re-based on top of current thermal/linux-next
> v1:
>  can be found here [2]
> 
> Regards,
> Lukasz
> 
> [1] https://lore.kernel.org/linux-pm/20201007024332.30322-1-michael.kao@mediatek.com/
> [2] https://lore.kernel.org/linux-pm/20201007122256.28080-1-lukasz.luba@arm.com/
> 
> 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
> 
> Michael Kao (1):
>   thermal: core: add upper and lower limits to power_actor_set_power
> 
>  drivers/thermal/gov_power_allocator.c | 40 +++++++++++-
>  drivers/thermal/thermal_core.c        | 88 ---------------------------
>  drivers/thermal/thermal_core.h        |  6 --
>  3 files changed, 38 insertions(+), 96 deletions(-)
> 


-- 
<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

Re: [PATCH v2 0/4] Add upper and lower limits in IPA power budget calculation

[Lukasz Luba]

On 10/26/20 8:42 PM, Daniel Lezcano wrote:
> 
> Applied, thanks
> 

Thank you Daniel!