Messages in this thread | | | From | "zhuguangqing83" <> | Subject | Re: [PATCH] thermal/drivers/cpuidle_cooling: Change the set_cur_state function | Date | Thu, 17 Sep 2020 20:52:37 +0800 |
| |
> > On 17/09/2020 13:15, zhuguangqing83 wrote: > > > >>> From: zhuguangqing <zhuguangqing@xiaomi.com> > >>> > >>> 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. > >> > >> 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. > > > > 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(). > > 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.
> > 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. > > > >>> 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 > >>> > >>> Signed-off-by: zhuguangqing <zhuguangqing@xiaomi.com> > >>> --- > >>> drivers/thermal/cpuidle_cooling.c | 10 ++++++++-- > >>> 1 file changed, 8 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/thermal/cpuidle_cooling.c > >> b/drivers/thermal/cpuidle_cooling.c > >>> index 78e3e8238116..868919ad3dda 100644 > >>> --- a/drivers/thermal/cpuidle_cooling.c > >>> +++ b/drivers/thermal/cpuidle_cooling.c > >>> @@ -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 > >>> @@ -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; > >>> > >>> 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); > >>> } > >>> > >>> 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
| |