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.capella@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