Received-SPF: pass (google.com: domain of linux-kernel+bounces-36909-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Di Shen <di.shen@unisoc.com>
To: <lukasz.luba@arm.com>, <rafael@kernel.org>, <daniel.lezcano@linaro.org>,
        <rui.zhang@intel.com>
CC: <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <xuewen.yan@unisoc.com>, <zhanglyra@gmail.com>, <orsonzhai@gmail.com>,
        <cindygm567@gmail.com>
Subject: [PATCH] thermal: power_allocator: initialize params->sustainable_power in the bind callback
Date: Wed, 24 Jan 2024 19:49:30 +0800
Message-ID: <20240124114930.12231-1-di.shen@unisoc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

If we set the PID constants k_* via sysfs before the IPA algorithm is
triggered, the constants would be changed after IPA is triggered which
means the k_* are set fail.

The process is as follow:
set k_* via sysfs
    |
set emul_temp via sysfs(temperature > switch_on_temp)
    |
throttle()
    |
temp > switch_on
    |
allocate_power
    |
pid_controller
    |
get_sustainable_power
    |
if (sustainable_power != params->sustainable_power)
because the params->sustainable_power is not initialized first,
so params->sustainable_power = 0, the condition is true, then
call the estimate_pid_constants().
		|
	estimate_pid_constants
		|
The k_* are overwritten, the k_* we set before are invalid.

For example:
unisoc:/sys/class/thermal/thermal_zone0 # cat policy
power_allocator
unisoc:/sys/class/thermal/thermal_zone0 # cat temp
32722
unisoc:/sys/class/thermal/thermal_zone0 # cat k_po
307
unisoc:/sys/class/thermal/thermal_zone0 # cat k_pu
614
unisoc:/sys/class/thermal/thermal_zone0 # cat k_i
61
unisoc:/sys/class/thermal/thermal_zone0 # cat k_*
0
61
307
614

unisoc:/sys/class/thermal/thermal_zone0 # echo 300 > k_po
unisoc:/sys/class/thermal/thermal_zone0 # echo 600 > k_pu
unisoc:/sys/class/thermal/thermal_zone0 # echo 60 >k_i
unisoc:/sys/class/thermal/thermal_zone0 # cat k_*
0
60
300
600

unisoc:/sys/class/thermal/thermal_zone0 # echo 70001 > emul_temp
unisoc:/sys/class/thermal/thermal_zone0 # cat k_*
0
61
307
614
unisoc:/sys/class/thermal/thermal_zone0 # echo 0 > emul_temp

This patch initializes params->sustainable_power when the governor
binds to thermal zone to avoid overwriting k_*.

The basic function won't be affected, as the k_* still can be estimated
if the sustainable_power is modified.

Signed-off-by: Di Shen <di.shen@unisoc.com>
---
 drivers/thermal/gov_power_allocator.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
index 81e061f183ad..1b17dc4c219c 100644
--- a/drivers/thermal/gov_power_allocator.c
+++ b/drivers/thermal/gov_power_allocator.c
@@ -711,6 +711,8 @@ 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");
+	else
+		params->sustainable_power = tz->tzp->sustainable_power;
 
 	estimate_pid_constants(tz, tz->tzp->sustainable_power,
 			       params->trip_switch_on,
-- 
2.17.1