On Thu, Jul 21, 2011 at 10:10 PM, Santosh Shilimkar <santosh.shilim...@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.shilim...@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<ccr...@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<ccr...@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. _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev