Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

[Colin Cross]

On Tue, Jan 25, 2011 at 12:39 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> Santosh,
>
>> > > Maybe we need a notifier list which can be told when cpuidle
>> > events
>> > > happen, so that parts of the system such as VFP and L2 cache
>> > support
>> > > can do the right thing without having platforms add lots of stuff
>> > like
>> > >
>> > >         gic_secondary_init();
>> > >         gic_restore_interrupt_types();
>> > >         vfp_enable();
>> > >         l2x0_enable();
>> > >         twd_enable();
>> > >         ... etc ...
>> > >
>> > > in their SoC specific code.
>> >
>> > But do we need a strict order between such operations? The notifier
>> > call
>> > chain isn't too flexible.
>> >
>> I guess it does depends on how the archs have integrated a9. Example
>> on OMAP there are different power modes possible.
>>       1. CPU context ,TWD lost
>>       2. CPU context ,TWD + L1 is lost
>>       3. CPU context + L1 is lost + GIC lost
>>       4. CPU context + L1 is lost + GIC lost + L2 lost
>> So there is need to have flexibility of calling these function
>> based on power modes. I don't know how notifiers can give
>> this flexibility
>
> Well if you set the priority fields in the notifier blocks correctly
> then you can just return NOTIFY_STOP when you've saved/restored as much
> as you want. This assumes of course that you can identify which power
> mode you're entering/leaving and that each one is `deeper' than the previous.

I doubt its possible to create an order that will work for all
architectures, and returning NOTIFY_STOP would require the decision on
when to finish to be made by the notifier block instead of the
platform code.

Tegra has three possible idle modes:

1.  WFI - nothing reset
2.  CPU, TWD, L1, GIC lost, L2 needs to be disabled but not reset
3.  CPU, TWD, L1, GIC, and L2 lost

CPU and L1 are already handled by the platform-specific suspend code.
TWD is handled by the clockevents broadcast notifiers.  That leaves L2
and GIC.

The L2 can be idled in two ways: just disable it, but keep the
contents, which prevents having to refill the cache, or a full reset.
Disabling can already be handled by outer_cache_disable().

The GIC needs to be idled differently depending on which cpus are
idle.  On Tegra, if both cpus are idle, the secondary cpu needs to
disable its GIC and go to WFI, while the first cpu saves the GIC
distributor state and powers off both cpus.

While it seems possible to handle all of these cases with a notifier
chain, the amount of platform-specific knowledge and ordering seems
too much to make it worthwhile.

If the ordering requirements are close enough between platforms, or
non-existent (I don't think there are any requirements on Tegra), a
notifier chain with a mask of the hardware that is being reset could
work.  On Tegra, in case 2 above, CPU1 would call:

idle_notify(IDLE_ENTER | GIC_DISABLE)

and CPU0 would call:

idle_notify(IDLE_ENTER | GIC_DISABLE | GIC_SAVE | L2_DISABLE)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

[Russell King - ARM Linux]

On Fri, Feb 04, 2011 at 05:32:26PM -0600, Colin Cross wrote:
> On Tue, Jan 25, 2011 at 12:39 PM, Will Deacon <will.deacon@arm.com> wrote:
> > Well if you set the priority fields in the notifier blocks correctly
> > then you can just return NOTIFY_STOP when you've saved/restored as much
> > as you want. This assumes of course that you can identify which power
> > mode you're entering/leaving and that each one is `deeper' than the previous.
> 
> I doubt its possible to create an order that will work for all
> architectures, and returning NOTIFY_STOP would require the decision on
> when to finish to be made by the notifier block instead of the
> platform code.
> 
> Tegra has three possible idle modes:
> 
> 1.  WFI - nothing reset
> 2.  CPU, TWD, L1, GIC lost, L2 needs to be disabled but not reset
> 3.  CPU, TWD, L1, GIC, and L2 lost

(2) and (3) don't sound like per-cpu modes but system modes.  If you're
having to disable L2, then your other CPU can't be active.

> CPU and L1 are already handled by the platform-specific suspend code.
> TWD is handled by the clockevents broadcast notifiers.  That leaves L2
> and GIC.

GIC can be handled in just the same way - upon a CPU idling and it
being decided that the CPU should enter low power mode, the idle states
are entered which does what's required with TWD, L1, VFP, Neon, etc.
We just need the GIC CPU interface included in there.

When both CPUs are idled, then the L2 comes into play, and then modes
(2) and (3) become possible and this is where you start doing the extra
stuff.

Note that you have to do it that way anyway, because you can't save
the state of the other CPU's GIC without doing an IPI call, which
could kick it out of its idle mode.

Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

[Colin Cross]

On Fri, Feb 4, 2011 at 5:43 PM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Fri, Feb 04, 2011 at 05:32:26PM -0600, Colin Cross wrote:
>> On Tue, Jan 25, 2011 at 12:39 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
>> > Well if you set the priority fields in the notifier blocks correctly
>> > then you can just return NOTIFY_STOP when you've saved/restored as much
>> > as you want. This assumes of course that you can identify which power
>> > mode you're entering/leaving and that each one is `deeper' than the previous.
>>
>> I doubt its possible to create an order that will work for all
>> architectures, and returning NOTIFY_STOP would require the decision on
>> when to finish to be made by the notifier block instead of the
>> platform code.
>>
>> Tegra has three possible idle modes:
>>
>> 1.  WFI - nothing reset
>> 2.  CPU, TWD, L1, GIC lost, L2 needs to be disabled but not reset
>> 3.  CPU, TWD, L1, GIC, and L2 lost
>
> (2) and (3) don't sound like per-cpu modes but system modes.  If you're
> having to disable L2, then your other CPU can't be active.

Yes, 2 and 3 require both CPUs to be idle.  Unfortunately, on Tegra,
it is important to use at least 2 as much as possible, because the two
CPUs are not individually power gated.

>> CPU and L1 are already handled by the platform-specific suspend code.
>> TWD is handled by the clockevents broadcast notifiers.  That leaves L2
>> and GIC.
>
> GIC can be handled in just the same way - upon a CPU idling and it
> being decided that the CPU should enter low power mode, the idle states
> are entered which does what's required with TWD, L1, VFP, Neon, etc.
> We just need the GIC CPU interface included in there.
>
> When both CPUs are idled, then the L2 comes into play, and then modes
> (2) and (3) become possible and this is where you start doing the extra
> stuff.

Are you suggesting that the idle notifiers only handle TWD, L1, VFP,
Neon, and GIC?  That would simplify things, as there are probably no
ordering requirements, and they should be the same for any platform
that uses them.

> Note that you have to do it that way anyway, because you can't save
> the state of the other CPU's GIC without doing an IPI call, which
> could kick it out of its idle mode.

There is currently no state that needs to be saved in the GIC CPU
registers, they can all be reinitialized.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

[Santosh Shilimkar]

> -----Original Message-----
> From: ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org [mailto:ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org] On Behalf Of
> Colin Cross
> Sent: Saturday, February 05, 2011 7:15 AM
> To: Russell King - ARM Linux
> Cc: Will Deacon; Santosh Shilimkar; Catalin Marinas; Linus Walleij;
> konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Tony Lindgren; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org; linux-arm-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Subject: Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for
> re-enabling l2x0
>
> On Fri, Feb 4, 2011 at 5:43 PM, Russell King - ARM Linux
> <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> > On Fri, Feb 04, 2011 at 05:32:26PM -0600, Colin Cross wrote:
> >> On Tue, Jan 25, 2011 at 12:39 PM, Will Deacon
> <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> >> > Well if you set the priority fields in the notifier blocks
> correctly
> >> > then you can just return NOTIFY_STOP when you've saved/restored
> as much
> >> > as you want. This assumes of course that you can identify which
> power
> >> > mode you're entering/leaving and that each one is `deeper' than
> the previous.
> >>
> >> I doubt its possible to create an order that will work for all
> >> architectures, and returning NOTIFY_STOP would require the
> decision on
> >> when to finish to be made by the notifier block instead of the
> >> platform code.
> >>
> >> Tegra has three possible idle modes:
> >>
> >> 1.  WFI - nothing reset
> >> 2.  CPU, TWD, L1, GIC lost, L2 needs to be disabled but not reset
> >> 3.  CPU, TWD, L1, GIC, and L2 lost
> >
> > (2) and (3) don't sound like per-cpu modes but system modes.  If
> you're
> > having to disable L2, then your other CPU can't be active.
>
> Yes, 2 and 3 require both CPUs to be idle.  Unfortunately, on Tegra,
> it is important to use at least 2 as much as possible, because the
> two
> CPUs are not individually power gated.
>
> >> CPU and L1 are already handled by the platform-specific suspend
> code.
> >> TWD is handled by the clockevents broadcast notifiers.  That
> leaves L2
> >> and GIC.
> >
> > GIC can be handled in just the same way - upon a CPU idling and it
> > being decided that the CPU should enter low power mode, the idle
> states
> > are entered which does what's required with TWD, L1, VFP, Neon,
> etc.
> > We just need the GIC CPU interface included in there.
> >
> > When both CPUs are idled, then the L2 comes into play, and then
> modes
> > (2) and (3) become possible and this is where you start doing the
> extra
> > stuff.
>
> Are you suggesting that the idle notifiers only handle TWD, L1, VFP,
> Neon, and GIC?  That would simplify things, as there are probably no
> ordering requirements, and they should be the same for any platform
> that uses them.
>
> > Note that you have to do it that way anyway, because you can't
> save
> > the state of the other CPU's GIC without doing an IPI call, which
> > could kick it out of its idle mode.
>
> There is currently no state that needs to be saved in the GIC CPU
> registers, they can all be reinitialized.
GIC save/restore on OMAP follows different strategy. There is a
Predefined layout to save content and restore is done atomically
by boot ROM code.
L2 cache also same case. Only AUXCTRL needs to be programmed on
wakeup from low power mode and that too with secure call. Rest
of the registers are managed by boot ROM code.

TWD is already managed through framework. Othe CPU low power
sequence is very small and OMAP has restrictions on the last
core to go down and first to wakeup.

So at least I don't see any use of common notifiers for GIC
and L2 will help OMAP lower power code.

Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support for re-enabling l2x0

[Russell King - ARM Linux]

On Sat, Feb 05, 2011 at 01:21:24PM +0530, Santosh Shilimkar wrote:
> GIC save/restore on OMAP follows different strategy. There is a
> Predefined layout to save content and restore is done atomically
> by boot ROM code.
> L2 cache also same case. Only AUXCTRL needs to be programmed on
> wakeup from low power mode and that too with secure call. Rest
> of the registers are managed by boot ROM code.
> 
> TWD is already managed through framework. Othe CPU low power
> sequence is very small and OMAP has restrictions on the last
> core to go down and first to wakeup.
> 
> So at least I don't see any use of common notifiers for GIC
> and L2 will help OMAP lower power code.

What this means is that we're going to end up littering things like GIC
and other stuff with lots of individual SoC specific code to save state
into individual SoC specific structures.  This is not sane, and we're
not going to corrupt generic code with SoC specific code.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support forre-enabling l2x0

[Santosh Shilimkar]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> Sent: Saturday, February 05, 2011 3:18 PM
> To: Santosh Shilimkar
> Cc: Colin Cross; Will Deacon; Catalin Marinas; Linus Walleij;
> konkers@android.com; Tony Lindgren; linux-kernel@vger.kernel.org;
> linux-tegra@vger.kernel.org; olof@lixom.net; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support
> forre-enabling l2x0
>
> On Sat, Feb 05, 2011 at 01:21:24PM +0530, Santosh Shilimkar wrote:
> > GIC save/restore on OMAP follows different strategy. There is a
> > Predefined layout to save content and restore is done atomically
> > by boot ROM code.
> > L2 cache also same case. Only AUXCTRL needs to be programmed on
> > wakeup from low power mode and that too with secure call. Rest
> > of the registers are managed by boot ROM code.
> >
> > TWD is already managed through framework. Othe CPU low power
> > sequence is very small and OMAP has restrictions on the last
> > core to go down and first to wakeup.
> >
> > So at least I don't see any use of common notifiers for GIC
> > and L2 will help OMAP lower power code.
>
> What this means is that we're going to end up littering things like
> GIC
> and other stuff with lots of individual SoC specific code to save
> state
> into individual SoC specific structures.  This is not sane, and
> we're
> not going to corrupt generic code with SoC specific code.

Fully agree and hence flagged it early.

Regards,
Santosh

Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support forre-enabling l2x0

[Colin Cross]

On Sat, Feb 5, 2011 at 4:41 AM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
>> -----Original Message-----
>> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
>> Sent: Saturday, February 05, 2011 3:18 PM
>> To: Santosh Shilimkar
>> Cc: Colin Cross; Will Deacon; Catalin Marinas; Linus Walleij;
>> konkers@android.com; Tony Lindgren; linux-kernel@vger.kernel.org;
>> linux-tegra@vger.kernel.org; olof@lixom.net; linux-arm-
>> kernel@lists.infradead.org
>> Subject: Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support
>> forre-enabling l2x0
>>
>> On Sat, Feb 05, 2011 at 01:21:24PM +0530, Santosh Shilimkar wrote:
>> > GIC save/restore on OMAP follows different strategy. There is a
>> > Predefined layout to save content and restore is done atomically
>> > by boot ROM code.
>> > L2 cache also same case. Only AUXCTRL needs to be programmed on
>> > wakeup from low power mode and that too with secure call. Rest
>> > of the registers are managed by boot ROM code.
>> >
>> > TWD is already managed through framework. Othe CPU low power
>> > sequence is very small and OMAP has restrictions on the last
>> > core to go down and first to wakeup.
>> >
>> > So at least I don't see any use of common notifiers for GIC
>> > and L2 will help OMAP lower power code.
>>
>> What this means is that we're going to end up littering things like
>> GIC
>> and other stuff with lots of individual SoC specific code to save
>> state
>> into individual SoC specific structures.  This is not sane, and
>> we're
>> not going to corrupt generic code with SoC specific code.
>
> Fully agree and hence flagged it early.
>
> Regards,
> Santosh
>

Would putting dummy values in the areas the boot ROM uses and then
letting the common GIC code restore over them cause any problems?

RE: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support forre-enabling l2x0

[Santosh Shilimkar]

> -----Original Message-----
> From: ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org [mailto:ccross-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org] On Behalf Of
> Colin Cross
> Sent: Saturday, February 05, 2011 10:06 PM
> To: Santosh Shilimkar
> Cc: Russell King - ARM Linux; Will Deacon; Catalin Marinas; Linus
> Walleij; konkers-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Tony Lindgren; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org;
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Subject: Re: [PATCH v2 04/28] ARM: mm: cache-l2x0: Add support
> forre-enabling l2x0
>
> On Sat, Feb 5, 2011 at 4:41 AM, Santosh Shilimkar
> <santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:
[....]

> >> On Sat, Feb 05, 2011 at 01:21:24PM +0530, Santosh Shilimkar
> wrote:
> >> > GIC save/restore on OMAP follows different strategy. There is a
> >> > Predefined layout to save content and restore is done
> atomically
> >> > by boot ROM code.
> >> > L2 cache also same case. Only AUXCTRL needs to be programmed on
> >> > wakeup from low power mode and that too with secure call. Rest
> >> > of the registers are managed by boot ROM code.
> >> >
> >> > TWD is already managed through framework. Othe CPU low power
> >> > sequence is very small and OMAP has restrictions on the last
> >> > core to go down and first to wakeup.
> >> >
> >> > So at least I don't see any use of common notifiers for GIC
> >> > and L2 will help OMAP lower power code.
> >>
> >> What this means is that we're going to end up littering things
> like
> >> GIC
> >> and other stuff with lots of individual SoC specific code to save
> >> state
> >> into individual SoC specific structures.  This is not sane, and
> >> we're
> >> not going to corrupt generic code with SoC specific code.
> >
> > Fully agree and hence flagged it early.
> >
[....]

>
> Would putting dummy values in the areas the boot ROM uses and then
> letting the common GIC code restore over them cause any problems?

Ya there are few issue. GIC and GIC OMAP extension are managed
together by BOOT ROM code. It's far optimal save and restore. Only
needed registers from OMAP point of view are saved/restored.
And for such reasons I would not like to use dummy stuff.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html