Skip to contentSkip to conversation

thermal/drivers/cpuidle_cooling: Change the set_cur_state function

PATCH zhuguangqing83 1 participant 1 message Sep 17, 2020

lore
Share & tools
lore
#0 lore raw
Quoted 41 lines · On 17/09/2020 13:15, zhuguangqing83 wrote:
Quoted 21 lines
Quoted 8 lines
Quoted 7 lines

From: zhuguangqing <[email protected]>

In the function cpuidle_cooling_set_cur_state(), if current_state is not equal to state and both current_state and state are greater than 0(scene 4 as follows), then maybe it should stop->start or restart idle_inject.

Quoted 6 lines

Sorry, I don't get it.

It is an update of the state, why do we need to restart the idle injection ? The state change will be automatically take into account by the idle injection code at the new injection cycle.

Thanks for your comments.

For example, we call cpuidle_cooling_set_cur_state() twice, first time the input parameter state is 20, second time the input parameter state is 30.

Quoted 6 lines

In current code, in the second call of cpuidle_cooling_set_cur_state(), current_state == 20, state == 30, then "if (current_state == 0 && state > 0)" is not satisfied, "else if (current_state > 0 && !state)" is not satisfied either, so we just update idle_cdev->state to 30 and idle_inject_set_duration to new injection cycle,but we do not call idle injection code.

Ok, I think understand your question.

When the idle injection is started, a timer is periodically calling the function play_idle_precise() with the idle duration. This one is updated by idle_inject_set_duration().

So when the state is changed, that changes the idle duration. At the next timer expiration, a few Milli seconds after, play_idle_precise() will be called with the new idle duration which was updated by idle_inject_set_duration().

Quoted 8 lines

There is no need to stop and start the idle injection at each update.

The new value is take into account automatically for the next cycle.

It does not really matter if the update is delayed. Restarting the idle injection at each update will be worth in the cooling context than waiting an idle cycle.

Ok, got it. Thanks.

Quoted 78 lines
Quoted 70 lines

In the example mentioned above, we should call idle injection code. If idle_inject_start() takes into account by the idle injection code at the new injection cycle, then just calling idle_inject_start() is ok. Otherwise, we need a restart or stop->start process to execute idle injection code at the new state 30.

Quoted 63 lines
Quoted 9 lines

The scenes changed is as follows,

scene current_state state action

 1              0          >0       start
 2              0          0        do nothing
 3              >0         0        stop
 4        >0 && !=state    >0       stop->start or restart
 5        >0 && ==state    >0       do nothing

Review

Quoted 6 lines
---
 drivers/thermal/cpuidle_cooling.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/drivers/thermal/cpuidle_cooling.c

Quoted 37 lines

index 78e3e8238116..868919ad3dda 100644

drivers/thermal/cpuidle_cooling.c +1 -1
@@ -113,7 +113,7 @@ static int cpuidle_cooling_get_cur_state(struct
 thermal_cooling_device *cdev,
 /**
  * cpuidle_cooling_set_cur_state - Set the current cooling state
  * @cdev: the thermal cooling device
- * @state: the target state
+ * @state: the target state, max value is 100
  *
  * The function checks first if we are initiating the mitigation which

* in turn wakes up all the idle injection tasks belonging to the idle

drivers/thermal/cpuidle_cooling.c +3 -0
@@ -130,6 +130,9 @@ static int cpuidle_cooling_set_cur_state(struct
 thermal_cooling_device *cdev,
 	unsigned long current_state = idle_cdev->state;
 	unsigned int runtime_us, idle_duration_us;
 
+	if (state > 100 || current_state == state)
+		return 0;
+
 	idle_cdev->state = state;
 
drivers/thermal/cpuidle_cooling.c +4 -1
 	idle_inject_get_duration(ii_dev, &runtime_us, &idle_duration_us);
@@ -140,8 +143,11 @@ static int cpuidle_cooling_set_cur_state(struct
 thermal_cooling_device *cdev,
 
 	if (current_state == 0 && state > 0) {
 		idle_inject_start(ii_dev);
-	} else if (current_state > 0 && !state)  {
+	} else if (current_state > 0 && !state) {
 		idle_inject_stop(ii_dev);
+	} else {
+		idle_inject_stop(ii_dev);
+		idle_inject_start(ii_dev);
 	}
 
Quoted 2 lines

return 0;

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

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