[PATCH 1/2] PM: EM: Add macro em_is_microwatts()

[PATCH 1/2] PM: EM: Add macro em_is_microwatts()

[PoShao Chen]

This patch adds a new macro, em_is_microwatts(), which checks if
the EM_PERF_DOMAIN_MICROWATTS flag is set for a given Energy Model.
This macro enables other parts of the kernel, such as cooling
devices, to easily determine the unit of power used by the Energy
Model and to perform the necessary conversions if the values
are provided in microwatts.

Signed-off-by: PoShao Chen <poshao.chen@mediatek.com>
---
 include/linux/energy_model.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index 770755df852f..68145b4368d1 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -92,6 +92,7 @@ struct em_perf_domain {
 
 #define em_span_cpus(em) (to_cpumask((em)->cpus))
 #define em_is_artificial(em) ((em)->flags & EM_PERF_DOMAIN_ARTIFICIAL)
+#define em_is_microwatts(em) ((em)->flags & EM_PERF_DOMAIN_MICROWATTS)
 
 #ifdef CONFIG_ENERGY_MODEL
 /*
-- 
2.18.0

[PATCH 2/2] thermal: cooling: Fix unneeded conversions in cpufreq_cooling and devfreq_cooling

[PoShao Chen]

Fix the incorrect division of power values by MICROWATT_PER_MILLIWATT for
non-microwatt units in the Energy Model (EM) by adding an
em_is_microwatts() check. This ensures that power values are only converted
when the EM specifies microwatts, allowing for accurate interpretation of
power according to the unit defined by the EM.

Signed-off-by: PoShao Chen <poshao.chen@mediatek.com>
---
 drivers/thermal/cpufreq_cooling.c |  6 ++++--
 drivers/thermal/devfreq_cooling.c | 12 ++++++++----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
index 9d1b1459700d..5324b9766843 100644
--- a/drivers/thermal/cpufreq_cooling.c
+++ b/drivers/thermal/cpufreq_cooling.c
@@ -120,7 +120,8 @@ static u32 cpu_freq_to_power(struct cpufreq_cooling_device *cpufreq_cdev,
 	}
 
 	power_mw = table[i + 1].power;
-	power_mw /= MICROWATT_PER_MILLIWATT;
+	if (em_is_microwatts(cpufreq_cdev->em))
+		power_mw /= MICROWATT_PER_MILLIWATT;
 	rcu_read_unlock();
 
 	return power_mw;
@@ -139,7 +140,8 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev,
 	for (i = cpufreq_cdev->max_level; i > 0; i--) {
 		/* Convert EM power to milli-Watts to make safe comparison */
 		em_power_mw = table[i].power;
-		em_power_mw /= MICROWATT_PER_MILLIWATT;
+		if (em_is_microwatts(cpufreq_cdev->em))
+			em_power_mw /= MICROWATT_PER_MILLIWATT;
 		if (power >= em_power_mw)
 			break;
 	}
diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 50dec24e967a..c28e0c4a63d6 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -222,7 +222,8 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
 			dfc->res_util = table[state].power;
 			rcu_read_unlock();
 
-			dfc->res_util /= MICROWATT_PER_MILLIWATT;
+			if (em_is_microwatts(dfc->em_pd))
+				dfc->res_util /= MICROWATT_PER_MILLIWATT;
 
 			dfc->res_util *= SCALE_ERROR_MITIGATION;
 
@@ -247,7 +248,8 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
 		*power = table[perf_idx].power;
 		rcu_read_unlock();
 
-		*power /= MICROWATT_PER_MILLIWATT;
+		if (em_is_microwatts(dfc->em_pd))
+			*power /= MICROWATT_PER_MILLIWATT;
 		/* Scale power for utilization */
 		*power *= status.busy_time;
 		*power >>= 10;
@@ -279,7 +281,8 @@ static int devfreq_cooling_state2power(struct thermal_cooling_device *cdev,
 	*power = table[perf_idx].power;
 	rcu_read_unlock();
 
-	*power /= MICROWATT_PER_MILLIWATT;
+	if (em_is_microwatts(dfc->em_pd))
+		*power /= MICROWATT_PER_MILLIWATT;
 
 	return 0;
 }
@@ -321,7 +324,8 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
 	for (i = dfc->max_state; i > 0; i--) {
 		/* Convert EM power to milli-Watts to make safe comparison */
 		em_power_mw = table[i].power;
-		em_power_mw /= MICROWATT_PER_MILLIWATT;
+		if (em_is_microwatts(dfc->em_pd))
+			em_power_mw /= MICROWATT_PER_MILLIWATT;
 		if (est_power >= em_power_mw)
 			break;
 	}
-- 
2.18.0

Re: [PATCH 2/2] thermal: cooling: Fix unneeded conversions in cpufreq_cooling and devfreq_cooling

[Lukasz Luba]

Hi PoShao,

On 3/8/24 06:59, PoShao Chen wrote:
> Fix the incorrect division of power values by MICROWATT_PER_MILLIWATT for
> non-microwatt units in the Energy Model (EM) by adding an
> em_is_microwatts() check. This ensures that power values are only converted
> when the EM specifies microwatts, allowing for accurate interpretation of
> power according to the unit defined by the EM.
> 
> Signed-off-by: PoShao Chen <poshao.chen@mediatek.com>
> ---
>   drivers/thermal/cpufreq_cooling.c |  6 ++++--
>   drivers/thermal/devfreq_cooling.c | 12 ++++++++----
>   2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> index 9d1b1459700d..5324b9766843 100644
> --- a/drivers/thermal/cpufreq_cooling.c
> +++ b/drivers/thermal/cpufreq_cooling.c
> @@ -120,7 +120,8 @@ static u32 cpu_freq_to_power(struct cpufreq_cooling_device *cpufreq_cdev,
>   	}
>   
>   	power_mw = table[i + 1].power;
> -	power_mw /= MICROWATT_PER_MILLIWATT;
> +	if (em_is_microwatts(cpufreq_cdev->em))
> +		power_mw /= MICROWATT_PER_MILLIWATT;
>   	rcu_read_unlock();
>   
>   	return power_mw;
> @@ -139,7 +140,8 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev,
>   	for (i = cpufreq_cdev->max_level; i > 0; i--) {
>   		/* Convert EM power to milli-Watts to make safe comparison */
>   		em_power_mw = table[i].power;
> -		em_power_mw /= MICROWATT_PER_MILLIWATT;
> +		if (em_is_microwatts(cpufreq_cdev->em))
> +			em_power_mw /= MICROWATT_PER_MILLIWATT;
>   		if (power >= em_power_mw)
>   			break;
>   	}
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index 50dec24e967a..c28e0c4a63d6 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -222,7 +222,8 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
>   			dfc->res_util = table[state].power;
>   			rcu_read_unlock();
>   
> -			dfc->res_util /= MICROWATT_PER_MILLIWATT;
> +			if (em_is_microwatts(dfc->em_pd))
> +				dfc->res_util /= MICROWATT_PER_MILLIWATT;
>   
>   			dfc->res_util *= SCALE_ERROR_MITIGATION;
>   
> @@ -247,7 +248,8 @@ static int devfreq_cooling_get_requested_power(struct thermal_cooling_device *cd
>   		*power = table[perf_idx].power;
>   		rcu_read_unlock();
>   
> -		*power /= MICROWATT_PER_MILLIWATT;
> +		if (em_is_microwatts(dfc->em_pd))
> +			*power /= MICROWATT_PER_MILLIWATT;
>   		/* Scale power for utilization */
>   		*power *= status.busy_time;
>   		*power >>= 10;
> @@ -279,7 +281,8 @@ static int devfreq_cooling_state2power(struct thermal_cooling_device *cdev,
>   	*power = table[perf_idx].power;
>   	rcu_read_unlock();
>   
> -	*power /= MICROWATT_PER_MILLIWATT;
> +	if (em_is_microwatts(dfc->em_pd))
> +		*power /= MICROWATT_PER_MILLIWATT;
>   
>   	return 0;
>   }
> @@ -321,7 +324,8 @@ static int devfreq_cooling_power2state(struct thermal_cooling_device *cdev,
>   	for (i = dfc->max_state; i > 0; i--) {
>   		/* Convert EM power to milli-Watts to make safe comparison */
>   		em_power_mw = table[i].power;
> -		em_power_mw /= MICROWATT_PER_MILLIWATT;
> +		if (em_is_microwatts(dfc->em_pd))
> +			em_power_mw /= MICROWATT_PER_MILLIWATT;
>   		if (est_power >= em_power_mw)
>   			break;
>   	}

Thanks for the patches and reporting this!

I have checked the commit which introduced the micro-Watts.
All the drivers where aligned to register the new uW values, also
your mediatek-cpufreq-hw.c
ae6ccaa650380d243

I have also check current upstream and all drivers there provide
the uW to the em_dev_register_perf_domain().

Is it some out-of-tree kernel driver, which shows this issue?

I would need to check all places and force actually to provide the
uW to the EM registration function. It's pointless to try to support
mW just because the flag in the registration wasn't set by some
downstream driver.

Let me re-write that bit in the EM...

Regards,
Lukasz