| Message ID | 1432046953-12945-1-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 19, 2015 at 03:49:12PM +0100, Tomeu Vizoso wrote: > This callback is expected to do the same as enter() but it has to > guarantee that interrupts aren't enabled at any point in its execution, > as the tick is frozen. > > It will be called when the system goes to suspend-to-idle and will > reduce power usage because CPUs won't be awaken for unnecessary IRQs. > > By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code > for both the enter() and enter_freeze() callbacks. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > v3: * Set CPUIDLE_FLAG_TIMER_STOP to simplify things as suggested by > Lorenzo Pieralisi > v2: * Disable FIQs also when suspending-to-idle > --- > arch/arm/mach-tegra/cpuidle-tegra114.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c > index 155807f..7e59ac0 100644 > --- a/arch/arm/mach-tegra/cpuidle-tegra114.c > +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c > @@ -44,16 +44,12 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, > tegra_set_cpu_in_lp2(); > cpu_pm_enter(); > > - tick_broadcast_enter(); > - > call_firmware_op(prepare_idle); > > /* Do suspend by ourselves if the firmware does not implement it */ > if (call_firmware_op(do_idle, 0) == -ENOSYS) > cpu_suspend(0, tegra30_sleep_cpu_secondary_finish); > > - tick_broadcast_exit(); > - > cpu_pm_exit(); > tegra_clear_cpu_in_lp2(); > > @@ -61,6 +57,13 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, > > return index; > } > + > +static void tegra114_idle_enter_freeze(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, > + int index) > +{ > + tegra114_idle_power_down(dev, drv, index); > +} You do not need this function, remove it and update the enter_freeze pointer below with tegra114_idle_power_down please. Thanks, Lorenzo > #endif > > static struct cpuidle_driver tegra_idle_driver = { > @@ -72,8 +75,10 @@ static struct cpuidle_driver tegra_idle_driver = { > #ifdef CONFIG_PM_SLEEP > [1] = { > .enter = tegra114_idle_power_down, > + .enter_freeze = tegra114_idle_enter_freeze, > .exit_latency = 500, > .target_residency = 1000, > + .flags = CPUIDLE_FLAG_TIMER_STOP, > .power_usage = 0, > .name = "powered-down", > .desc = "CPU power gated", > -- > 2.4.1 >
On 05/20/2015 10:44 AM, Lorenzo Pieralisi wrote: > On Tue, May 19, 2015 at 03:49:12PM +0100, Tomeu Vizoso wrote: >> This callback is expected to do the same as enter() but it has to >> guarantee that interrupts aren't enabled at any point in its execution, >> as the tick is frozen. >> >> It will be called when the system goes to suspend-to-idle and will >> reduce power usage because CPUs won't be awaken for unnecessary IRQs. >> >> By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code >> for both the enter() and enter_freeze() callbacks. >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> >> --- >> >> v3: * Set CPUIDLE_FLAG_TIMER_STOP to simplify things as suggested by >> Lorenzo Pieralisi >> v2: * Disable FIQs also when suspending-to-idle >> --- >> arch/arm/mach-tegra/cpuidle-tegra114.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c >> index 155807f..7e59ac0 100644 >> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c >> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c >> @@ -44,16 +44,12 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, >> tegra_set_cpu_in_lp2(); >> cpu_pm_enter(); >> >> - tick_broadcast_enter(); >> - >> call_firmware_op(prepare_idle); >> >> /* Do suspend by ourselves if the firmware does not implement it */ >> if (call_firmware_op(do_idle, 0) == -ENOSYS) >> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish); >> >> - tick_broadcast_exit(); >> - >> cpu_pm_exit(); >> tegra_clear_cpu_in_lp2(); >> >> @@ -61,6 +57,13 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, >> >> return index; >> } >> + >> +static void tegra114_idle_enter_freeze(struct cpuidle_device *dev, >> + struct cpuidle_driver *drv, >> + int index) >> +{ >> + tegra114_idle_power_down(dev, drv, index); >> +} > > You do not need this function, remove it and update the enter_freeze > pointer below with tegra114_idle_power_down please. How so? The callbacks have different signatures. Regards, Tomeu > Thanks, > Lorenzo > >> #endif >> >> static struct cpuidle_driver tegra_idle_driver = { >> @@ -72,8 +75,10 @@ static struct cpuidle_driver tegra_idle_driver = { >> #ifdef CONFIG_PM_SLEEP >> [1] = { >> .enter = tegra114_idle_power_down, >> + .enter_freeze = tegra114_idle_enter_freeze, >> .exit_latency = 500, >> .target_residency = 1000, >> + .flags = CPUIDLE_FLAG_TIMER_STOP, >> .power_usage = 0, >> .name = "powered-down", >> .desc = "CPU power gated", >> -- >> 2.4.1 >>
On Wed, May 20, 2015 at 09:55:41AM +0100, Tomeu Vizoso wrote: > On 05/20/2015 10:44 AM, Lorenzo Pieralisi wrote: > > On Tue, May 19, 2015 at 03:49:12PM +0100, Tomeu Vizoso wrote: > >> This callback is expected to do the same as enter() but it has to > >> guarantee that interrupts aren't enabled at any point in its execution, > >> as the tick is frozen. > >> > >> It will be called when the system goes to suspend-to-idle and will > >> reduce power usage because CPUs won't be awaken for unnecessary IRQs. > >> > >> By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code > >> for both the enter() and enter_freeze() callbacks. > >> > >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> > >> --- > >> > >> v3: * Set CPUIDLE_FLAG_TIMER_STOP to simplify things as suggested by > >> Lorenzo Pieralisi > >> v2: * Disable FIQs also when suspending-to-idle > >> --- > >> arch/arm/mach-tegra/cpuidle-tegra114.c | 13 +++++++++---- > >> 1 file changed, 9 insertions(+), 4 deletions(-) > >> > >> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c > >> index 155807f..7e59ac0 100644 > >> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c > >> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c > >> @@ -44,16 +44,12 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, > >> tegra_set_cpu_in_lp2(); > >> cpu_pm_enter(); > >> > >> - tick_broadcast_enter(); > >> - > >> call_firmware_op(prepare_idle); > >> > >> /* Do suspend by ourselves if the firmware does not implement it */ > >> if (call_firmware_op(do_idle, 0) == -ENOSYS) > >> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish); > >> > >> - tick_broadcast_exit(); > >> - > >> cpu_pm_exit(); > >> tegra_clear_cpu_in_lp2(); > >> > >> @@ -61,6 +57,13 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, > >> > >> return index; > >> } > >> + > >> +static void tegra114_idle_enter_freeze(struct cpuidle_device *dev, > >> + struct cpuidle_driver *drv, > >> + int index) > >> +{ > >> + tegra114_idle_power_down(dev, drv, index); > >> +} > > > > You do not need this function, remove it and update the enter_freeze > > pointer below with tegra114_idle_power_down please. > > How so? The callbacks have different signatures. Sorry, glossed over it, never mind. Lorenzo
On Tuesday, May 19, 2015 04:49:12 PM Tomeu Vizoso wrote: > This callback is expected to do the same as enter() but it has to > guarantee that interrupts aren't enabled at any point in its execution, > as the tick is frozen. > > It will be called when the system goes to suspend-to-idle and will > reduce power usage because CPUs won't be awaken for unnecessary IRQs. > > By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code > for both the enter() and enter_freeze() callbacks. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Makes sense to me, but I suppose it should go in via arm-soc.
On 21 May 2015 at 03:02, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, May 19, 2015 04:49:12 PM Tomeu Vizoso wrote: >> This callback is expected to do the same as enter() but it has to >> guarantee that interrupts aren't enabled at any point in its execution, >> as the tick is frozen. >> >> It will be called when the system goes to suspend-to-idle and will >> reduce power usage because CPUs won't be awaken for unnecessary IRQs. >> >> By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code >> for both the enter() and enter_freeze() callbacks. >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Makes sense to me, but I suppose it should go in via arm-soc. Hi Thierry, what's the plan regarding merging of this patch? It needs df8d9ee ("cpuidle: Run tick_broadcast_exit() with disabled interrupts") so it doesn't break boot. Thanks, Tomeu
On Thu, Jun 18, 2015 at 09:09:29AM +0200, Tomeu Vizoso wrote: > On 21 May 2015 at 03:02, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, May 19, 2015 04:49:12 PM Tomeu Vizoso wrote: > >> This callback is expected to do the same as enter() but it has to > >> guarantee that interrupts aren't enabled at any point in its execution, > >> as the tick is frozen. > >> > >> It will be called when the system goes to suspend-to-idle and will > >> reduce power usage because CPUs won't be awaken for unnecessary IRQs. > >> > >> By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code > >> for both the enter() and enter_freeze() callbacks. > >> > >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Makes sense to me, but I suppose it should go in via arm-soc. > > Hi Thierry, > > what's the plan regarding merging of this patch? > > It needs df8d9ee ("cpuidle: Run tick_broadcast_exit() with disabled > interrupts") so it doesn't break boot. By the looks of it the above patch was merged in 4.1, so I can easily take this through the Tegra tree. The commit message doesn't say whether or not this fixes a bug. Is this urgent? Should it go into 4.2? It sounds more like it's a power-saving feature, so 4.3 is probably more appropriate. Thierry
On 29 June 2015 at 11:00, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, Jun 18, 2015 at 09:09:29AM +0200, Tomeu Vizoso wrote: >> On 21 May 2015 at 03:02, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> > On Tuesday, May 19, 2015 04:49:12 PM Tomeu Vizoso wrote: >> >> This callback is expected to do the same as enter() but it has to >> >> guarantee that interrupts aren't enabled at any point in its execution, >> >> as the tick is frozen. >> >> >> >> It will be called when the system goes to suspend-to-idle and will >> >> reduce power usage because CPUs won't be awaken for unnecessary IRQs. >> >> >> >> By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code >> >> for both the enter() and enter_freeze() callbacks. >> >> >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> > >> > Makes sense to me, but I suppose it should go in via arm-soc. >> >> Hi Thierry, >> >> what's the plan regarding merging of this patch? >> >> It needs df8d9ee ("cpuidle: Run tick_broadcast_exit() with disabled >> interrupts") so it doesn't break boot. > > By the looks of it the above patch was merged in 4.1, so I can easily > take this through the Tegra tree. The commit message doesn't say whether > or not this fixes a bug. Is this urgent? Should it go into 4.2? It > sounds more like it's a power-saving feature, so 4.3 is probably more > appropriate. Yeah, it doesn't fix any bug and its effect should only be reduced power consumption when in suspend-to-idle. Thanks, Tomeu > Thierry
On Tue, May 19, 2015 at 04:49:12PM +0200, Tomeu Vizoso wrote: > This callback is expected to do the same as enter() but it has to > guarantee that interrupts aren't enabled at any point in its execution, > as the tick is frozen. > > It will be called when the system goes to suspend-to-idle and will > reduce power usage because CPUs won't be awaken for unnecessary IRQs. > > By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code > for both the enter() and enter_freeze() callbacks. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > v3: * Set CPUIDLE_FLAG_TIMER_STOP to simplify things as suggested by > Lorenzo Pieralisi > v2: * Disable FIQs also when suspending-to-idle > --- > arch/arm/mach-tegra/cpuidle-tegra114.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) Applied, thanks. Thierry
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c index 155807f..7e59ac0 100644 --- a/arch/arm/mach-tegra/cpuidle-tegra114.c +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c @@ -44,16 +44,12 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, tegra_set_cpu_in_lp2(); cpu_pm_enter(); - tick_broadcast_enter(); - call_firmware_op(prepare_idle); /* Do suspend by ourselves if the firmware does not implement it */ if (call_firmware_op(do_idle, 0) == -ENOSYS) cpu_suspend(0, tegra30_sleep_cpu_secondary_finish); - tick_broadcast_exit(); - cpu_pm_exit(); tegra_clear_cpu_in_lp2(); @@ -61,6 +57,13 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev, return index; } + +static void tegra114_idle_enter_freeze(struct cpuidle_device *dev, + struct cpuidle_driver *drv, + int index) +{ + tegra114_idle_power_down(dev, drv, index); +} #endif static struct cpuidle_driver tegra_idle_driver = { @@ -72,8 +75,10 @@ static struct cpuidle_driver tegra_idle_driver = { #ifdef CONFIG_PM_SLEEP [1] = { .enter = tegra114_idle_power_down, + .enter_freeze = tegra114_idle_enter_freeze, .exit_latency = 500, .target_residency = 1000, + .flags = CPUIDLE_FLAG_TIMER_STOP, .power_usage = 0, .name = "powered-down", .desc = "CPU power gated",
This callback is expected to do the same as enter() but it has to guarantee that interrupts aren't enabled at any point in its execution, as the tick is frozen. It will be called when the system goes to suspend-to-idle and will reduce power usage because CPUs won't be awaken for unnecessary IRQs. By setting the CPUIDLE_FLAG_TIMER_STOP flag, we can reuse the same code for both the enter() and enter_freeze() callbacks. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- v3: * Set CPUIDLE_FLAG_TIMER_STOP to simplify things as suggested by Lorenzo Pieralisi v2: * Disable FIQs also when suspending-to-idle --- arch/arm/mach-tegra/cpuidle-tegra114.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)