Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1431035 > unrolled thread
| Started by | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| First post | 2016-06-25 03:10 +0200 |
| Last post | 2016-06-27 03:20 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-25 03:10 +0200
Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Zhaoyang Huang <zhaoyang.huang@linaro.org> - 2016-06-27 03:20 +0200
| From | "Rafael J. Wysocki" <rafael@kernel.org> |
|---|---|
| Date | 2016-06-25 03:10 +0200 |
| Subject | Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select |
| Message-ID | <rNKgF-7lq-5@gated-at.bofh.it> |
On Fri, Jun 17, 2016 at 11:13 AM, Zhaoyang Huang
<zhaoyang.huang@linaro.org> wrote:
> In previous version, cpu_pm_enter is invoked
By whom? Not by the core surely?
> after the governor select the state, which cause the executing time of cpu_pm_enter
> is included in the idle time. Moving it before the state selection.
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
> ---
> kernel/sched/idle.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index bd12c6c..929da2e 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -5,6 +5,7 @@
> #include <linux/cpu.h>
> #include <linux/cpuidle.h>
> #include <linux/cpuhotplug.h>
> +#include <linux/cpu_pm.h>
> #include <linux/tick.h>
> #include <linux/mm.h>
> #include <linux/stackprotector.h>
> @@ -130,6 +131,7 @@ static void cpuidle_idle_call(void)
> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> int next_state, entered_state;
> + int ret;
>
> /*
> * Check if the idle task must be rescheduled. If it is the
> @@ -174,12 +176,16 @@ static void cpuidle_idle_call(void)
> /*
> * Ask the cpuidle framework to choose a convenient idle state.
> */
> - next_state = cpuidle_select(drv, dev);
> - entered_state = call_cpuidle(drv, dev, next_state);
> - /*
> - * Give the governor an opportunity to reflect on the outcome
> - */
> - cpuidle_reflect(dev, entered_state);
> + ret = cpu_pm_enter();
"To move" usually means "take it away from there and put it here" as
far as kernel patches are concerned, but I only see it added here.
> + if (!ret) {
> + next_state = cpuidle_select(drv, dev);
> + entered_state = call_cpuidle(drv, dev, next_state);
> + cpu_pm_exit();
> + /*
> + * Give the governor an opportunity to reflect on the outcome
> + */
> + cpuidle_reflect(dev, entered_state);
> + }
> }
>
> exit_idle:
> --
No way I will agree to add that notification stuff to the core.
Thanks,
Rafael
[toc] | [next] | [standalone]
| From | Zhaoyang Huang <zhaoyang.huang@linaro.org> |
|---|---|
| Date | 2016-06-27 03:20 +0200 |
| Message-ID | <rOtnr-1Tq-1@gated-at.bofh.it> |
| In reply to | #1431035 |
On 25 June 2016 at 09:09, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Jun 17, 2016 at 11:13 AM, Zhaoyang Huang
> <zhaoyang.huang@linaro.org> wrote:
>> In previous version, cpu_pm_enter is invoked
>
> By whom? Not by the core surely?
>
>> after the governor select the state, which cause the executing time of cpu_pm_enter
>> is included in the idle time. Moving it before the state selection.
>>
>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
>> ---
>> kernel/sched/idle.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index bd12c6c..929da2e 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -5,6 +5,7 @@
>> #include <linux/cpu.h>
>> #include <linux/cpuidle.h>
>> #include <linux/cpuhotplug.h>
>> +#include <linux/cpu_pm.h>
>> #include <linux/tick.h>
>> #include <linux/mm.h>
>> #include <linux/stackprotector.h>
>> @@ -130,6 +131,7 @@ static void cpuidle_idle_call(void)
>> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>> int next_state, entered_state;
>> + int ret;
>>
>> /*
>> * Check if the idle task must be rescheduled. If it is the
>> @@ -174,12 +176,16 @@ static void cpuidle_idle_call(void)
>> /*
>> * Ask the cpuidle framework to choose a convenient idle state.
>> */
>> - next_state = cpuidle_select(drv, dev);
>> - entered_state = call_cpuidle(drv, dev, next_state);
>> - /*
>> - * Give the governor an opportunity to reflect on the outcome
>> - */
>> - cpuidle_reflect(dev, entered_state);
>> + ret = cpu_pm_enter();
>
> "To move" usually means "take it away from there and put it here" as
> far as kernel patches are concerned, but I only see it added here.
>
The API is provided by kernel, while invoked by cpuidle driver now. We
have ever move 'CPUFREQ_PRECHANGE' from cpufreq driver to framework. I
do the same thing here.
>> + if (!ret) {
>> + next_state = cpuidle_select(drv, dev);
>> + entered_state = call_cpuidle(drv, dev, next_state);
>> + cpu_pm_exit();
>> + /*
>> + * Give the governor an opportunity to reflect on the outcome
>> + */
>> + cpuidle_reflect(dev, entered_state);
>> + }
>> }
>>
>> exit_idle:
>> --
>
> No way I will agree to add that notification stuff to the core.
>
It is not add, just move.
> Thanks,
> Rafael
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web