[PATCH 03/17] ARM: gic: Use cpu pm notifiers to save gic state

[Colin Cross]

On Thu, Jul 21, 2011 at 10:10 PM, Santosh Shilimkar
<santosh.shilimkar at ti.com> wrote:
> On 7/22/2011 12:36 AM, Colin Cross wrote:
>>
>> On Thu, Jul 21, 2011 at 3:46 AM, Santosh Shilimkar
>> <santosh.shilimkar at ti.com>  wrote:
>>>
>>> On 7/21/2011 3:57 PM, Lorenzo Pieralisi wrote:
>>>>
>>>> On Thu, Jul 21, 2011 at 09:32:12AM +0100, Santosh Shilimkar wrote:
>>>>>
>>>>> Lorenzo, Colin,
>>>>>
>>>>> On 7/7/2011 9:20 PM, Lorenzo Pieralisi wrote:
>>>>>>
>>>>>> From: Colin Cross<ccross at android.com>
>>>>>>
>>>>>> When the cpu is powered down in a low power mode, the gic cpu
>>>>>> interface may be reset, and when the cpu complex is powered
>>>>>> down, the gic distributor may also be reset.
>>>>>>
>>>>>> This patch uses CPU_PM_ENTER and CPU_PM_EXIT notifiers to save
>>>>>> and restore the gic cpu interface registers, and the
>>>>>> CPU_COMPLEX_PM_ENTER and CPU_COMPLEX_PM_EXIT notifiers to save
>>>>>> and restore the gic distributor registers.
>>>>>>
>>>>>> Signed-off-by: Colin Cross<ccross at android.com>
>>>>>> ---
>>>>>>   arch/arm/common/gic.c |  212
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>   1 files changed, 212 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
>>>>>> index 4ddd0a6..8d62e07 100644
>>>>>> --- a/arch/arm/common/gic.c
>>>>>> +++ b/arch/arm/common/gic.c
>>>>>
>>>>> [...]
>>>>>
>>>>> I missed one more comment in the last review.
>>>>>
>>>>>> +static int gic_notifier(struct notifier_block *self, unsigned long
>>>>>> cmd,
>>>>>>        void *v)
>>>>>> +{
>>>>>> +       int i;
>>>>>> +
>>>>>> +       for (i = 0; i<     MAX_GIC_NR; i++) {
>>>>>> +               switch (cmd) {
>>>>>> +               case CPU_PM_ENTER:
>>>>>> +                       gic_cpu_save(i);
>>>>>
>>>>> On OMAP, GIC cpu interface context is lost only when CPU cluster
>>>>> is powered down.
>>>>
>>>> Yes, it's true, but that's the only chance we have to save the GIC CPU
>>>> IF
>>>> state if the GIC context is lost, right ?
>>>> It is a private memory map per processor; I agree, it might be useless
>>>> if just one CPU is shutdown, but at that point in time you do not know
>>>> the state of other CPUs. If the cluster moves to a state where GIC
>>>> context
>>>> is lost at least you had the GIC CPU IF state saved. If we do not
>>>> save it, well, there is no way to do that anymore since the last CPU
>>>> cannot
>>>> access other CPUs GIC CPU IF registers (or better, banked GIC
>>>> distributor
>>>> registers).
>>>> If you force hotplug on CPUs other than 0 (that's the way it is done on
>>>> OMAP4
>>>> in cpuidle, right ?) to hit deep low-power states you reinit the GIC CPU
>>>> IF
>>>> state as per cold boot, so yes, it is useless there.
>>>>
>>> Actually, on OMAP there is no need to save any CPU interface registers.
>>>
>>> For my OMAP4 PM rebasing, for time-being I will go with exported
>>> GIC functions so that I don't have too many redundancies with GIC
>>> save/restore code.
>>
>> I think you should try to balance cpu idle latency with reuse of
>> common code.  In this case, you are avoiding restoring 7 registers by
>> reimplementing the bare minimum that is necessary for OMAP4, which is
>> unlikely to make a measurable impact on wakeup latency.  Can you try
>> starting with reusing all the common code, and add some timestamps
>> during wakeup to measure where the longest delays are, to determine
>> where you should diverge from the common code and use omap-optimized
>> code?
>
> I am going to use all the common code but having them exported
> functions gives more flexibility to call them in right and needed
> places. As discussed earlier, I plan to use the common GIC code
> wherever it's needed on OMAP.
>
> My main point was we are saving and restoring GIC CPU interface
> registers for a case where they are actually not lost.

Yes, but you're still avoiding 7 registers, which is unlikely to be
worth the complexity of calling these functions differently from every
other platform.