On 05/15/2013 05:24 PM, Liviu Dudau wrote: > Hi Sebastian, > > On Mon, May 13, 2013 at 07:53:42PM +0100, Sebastian Capella wrote: >> Pass residency information to the mcpm_cpu_suspend. The information >> is taken from the target_residency of the intended C-state. >> >> When a platform uses multiple powerdown cstates, the residency information >> indicates which powerdown state is targeted. Multiple powerdown cstate >> information can be maintained in the device tree and the vendor specific >> handling will then have enough information to determine what power state to >> enter without needing additional changes to the big_little framework. >> >> Signed-off-by: Sebastian Capella <sebastian.cape...@linaro.org> >> --- >> drivers/cpuidle/arm_big_little.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cpuidle/arm_big_little.c >> b/drivers/cpuidle/arm_big_little.c >> index a430800..8332b05 100644 >> --- a/drivers/cpuidle/arm_big_little.c >> +++ b/drivers/cpuidle/arm_big_little.c > > I could not find a branch that contains this file. Which git tree and branch > are you using?
I believe it should apply to: https://git.linaro.org/gitweb?p=landing-teams/working/arm/kernel.git;a=blob;f=drivers/cpuidle/arm_big_little.c;h=a430800d4a74c8c5b759cd34fb5e272d8cf8f8a3;hb=67b5adf6a402921655d337d52845d6d48c6ef2d2#l66 > >> @@ -89,7 +89,7 @@ static int notrace bl_powerdown_finisher(unsigned long arg) >> unsigned int cpu = mpidr & 0xf; >> >> mcpm_set_entry_vector(cpu, cluster, cpu_resume); >> - mcpm_cpu_suspend(0); /* 0 should be replaced with better value here */ >> + mcpm_cpu_suspend(arg); >> return 1; >> } >> >> @@ -107,6 +107,7 @@ static int bl_enter_powerdown(struct cpuidle_device *dev, >> { >> struct timespec ts_preidle, ts_postidle, ts_idle; >> int ret; >> + struct cpuidle_state *state = &drv->states[idx]; >> >> /* Used to keep track of the total time in idle */ >> getnstimeofday(&ts_preidle); >> @@ -117,7 +118,8 @@ static int bl_enter_powerdown(struct cpuidle_device *dev, >> >> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu); >> >> - ret = cpu_suspend((unsigned long) dev, bl_powerdown_finisher); >> + ret = cpu_suspend((unsigned long) state->target_residency, >> + bl_powerdown_finisher); > > I don't think you should pass the target residency here but the intended > C-state. Think about what will happen when you run this in a guest kernel: is > the target_residency the same if the guest has been migrated from a big core > that might have a faster execution of the down/up path to a little core that > is slower? The intended C-state should stay the same, regardless of the actual > time it takes to get there and out, which affects the actual time spent inside > the state. > > Best regards, > Liviu > -- <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 _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
