diff mbox

[V3,1/5] ARM: tegra: add pending SGI checking API

Message ID 1355797861-12759-2-git-send-email-josephl@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joseph Lo Dec. 18, 2012, 2:30 a.m. UTC
The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
include the power of GIC. That caused the SGI (Software Generated
Interrupt) been lost. Because the SGI can't wake up the CPU that in
the "powered-down" CPU idle mode. We need to check if there is any
pending SGI when go into "powered-down" CPU idle mode. This is important
especially when applying the coupled cpuidle framework into "power-down"
cpuidle dirver. Because the coupled cpuidle framework may have the
chance that misses IPI_SINGLE_FUNC handling sometimes.

For the PPI or SPI, something like the legacy peripheral interrupt. It
still can be maintained by Tegra legacy interrupt controller. If there
is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
CPU can be woken up immediately. So we don't need to take care the same
situation for PPI or SPI.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* move the static mapping of GIC addr into tegra_pending_sgi
V2:
* new in V2
---
 arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
 arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h

Comments

Colin Cross Dec. 18, 2012, 2:42 a.m. UTC | #1
On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> include the power of GIC. That caused the SGI (Software Generated
> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> the "powered-down" CPU idle mode. We need to check if there is any
> pending SGI when go into "powered-down" CPU idle mode. This is important
> especially when applying the coupled cpuidle framework into "power-down"
> cpuidle dirver. Because the coupled cpuidle framework may have the
> chance that misses IPI_SINGLE_FUNC handling sometimes.

This problem exists for any GIC-based SoC, and needs to be fixed in
gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

> For the PPI or SPI, something like the legacy peripheral interrupt. It
> still can be maintained by Tegra legacy interrupt controller. If there
> is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> CPU can be woken up immediately. So we don't need to take care the same
> situation for PPI or SPI.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * move the static mapping of GIC addr into tegra_pending_sgi
> V2:
> * new in V2
> ---
>  arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
>  arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-tegra/irq.h
>
> diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
> index b7886f1..c9976e3 100644
> --- a/arch/arm/mach-tegra/irq.c
> +++ b/arch/arm/mach-tegra/irq.c
> @@ -45,6 +45,8 @@
>
>  #define FIRST_LEGACY_IRQ 32
>
> +#define SGI_MASK 0xFFFF
> +
>  static int num_ictlrs;
>
>  static void __iomem *ictlr_reg_base[] = {
> @@ -55,6 +57,19 @@ static void __iomem *ictlr_reg_base[] = {
>         IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
>  };
>
> +bool tegra_pending_sgi(void)
> +{
> +       u32 pending_set;
> +       void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
> +
> +       pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
> +
> +       if (pending_set & SGI_MASK)
> +               return true;
> +
> +       return false;
> +}
> +
>  static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
>  {
>         void __iomem *base;
> diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
> new file mode 100644
> index 0000000..5142649
> --- /dev/null
> +++ b/arch/arm/mach-tegra/irq.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __TEGRA_IRQ_H
> +#define __TEGRA_IRQ_H
> +
> +bool tegra_pending_sgi(void);
> +
> +#endif
> --
> 1.7.0.4
>
Joseph Lo Dec. 18, 2012, 2:57 a.m. UTC | #2
On Tue, 2012-12-18 at 10:42 +0800, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> 
Indeed, but may not need to be taken care for every SoC. It's depend on
HW design. Different SoC had different design about it. For ex, some SoC
only put CPU core to power saving mode not include GIC, or there is
another irq controller can handle the case when CPU go into power saving
mode. Differenc SoC had different usage here (some need to check all
pending irq, some need to check SGI only and some even no need to
consider this). So putting this into to common code may not a good
choice.

Thanks,
Joseph

> > For the PPI or SPI, something like the legacy peripheral interrupt. It
> > still can be maintained by Tegra legacy interrupt controller. If there
> > is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> > CPU can be woken up immediately. So we don't need to take care the same
> > situation for PPI or SPI.
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
> > V3:
> > * move the static mapping of GIC addr into tegra_pending_sgi
> > V2:
> > * new in V2
> > ---
Peter De Schrijver Dec. 18, 2012, 10:15 a.m. UTC | #3
On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
cluster is railgated, including the GIC. This causes a pending IPI to be lost.
But for example on OMAP4, only the actual CPU cores are powergated. The GIC
stays alive until also the core domain hits idle. By that time a potential
pending IPI has long woken up the target CPU again, so no additional
checks are needed for functional correct behavior.

Cheers,

Peter.
Colin Cross Dec. 18, 2012, 7:36 p.m. UTC | #4
On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>> > include the power of GIC. That caused the SGI (Software Generated
>> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
>> > the "powered-down" CPU idle mode. We need to check if there is any
>> > pending SGI when go into "powered-down" CPU idle mode. This is important
>> > especially when applying the coupled cpuidle framework into "power-down"
>> > cpuidle dirver. Because the coupled cpuidle framework may have the
>> > chance that misses IPI_SINGLE_FUNC handling sometimes.
>>
>> This problem exists for any GIC-based SoC, and needs to be fixed in
>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>
> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> stays alive until also the core domain hits idle. By that time a potential
> pending IPI has long woken up the target CPU again, so no additional
> checks are needed for functional correct behavior.

I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
rail for the GIC in retention, and I don't think an IPI will wake it
up.  I believe the same problem also exists for Exynos5.  In any case,
checking for an IPI early during idle and aborting won't hurt those
platforms, so I still think it should be in the GIC driver and not by
mapping the GIC registers into a separate driver.
Joseph Lo Dec. 19, 2012, 1:06 a.m. UTC | #5
On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
> > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >> > include the power of GIC. That caused the SGI (Software Generated
> >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >> > the "powered-down" CPU idle mode. We need to check if there is any
> >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> >> > especially when applying the coupled cpuidle framework into "power-down"
> >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>
> >> This problem exists for any GIC-based SoC, and needs to be fixed in
> >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >
> > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > stays alive until also the core domain hits idle. By that time a potential
> > pending IPI has long woken up the target CPU again, so no additional
> > checks are needed for functional correct behavior.
> 
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.

Hi Colin,

If I move this code into common GIC driver, should I just take care
pending SGI also?

I will try to create a RFC patch for this.

Thanks,
Joseph
Joseph Lo Dec. 19, 2012, 3:47 a.m. UTC | #6
On Wed, 2012-12-19 at 09:06 +0800, Joseph Lo wrote:
> On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> > > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> > >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > >> > include the power of GIC. That caused the SGI (Software Generated
> > >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > >> > the "powered-down" CPU idle mode. We need to check if there is any
> > >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > >> > especially when applying the coupled cpuidle framework into "power-down"
> > >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> > >>
> > >> This problem exists for any GIC-based SoC, and needs to be fixed in
> > >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> > >
> > > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > > stays alive until also the core domain hits idle. By that time a potential
> > > pending IPI has long woken up the target CPU again, so no additional
> > > checks are needed for functional correct behavior.
> > 
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> 
> If I move this code into common GIC driver, should I just take care
> pending SGI also?
> 
> I will try to create a RFC patch for this.
> 
Hi Colin,

I just checked the scenario to abort cpu_pm_enter when there is any
pending SGI. This may break the current CPU idle driver that already
using cpu_pm_enter. If these drivers didn't handle the error return of
"cpu_pm_enter", the error will cause the CPU_PM_ENTER_FAILED be
triggered. Then the idle code will fail.

The other solution may just like this patch did. Adding an API
"gic_pending_sgi" to check if there is a pending SGI before putting the
CPU into low power mode. But I still don't think this is a common code
to be put there. It's still SoC specific code. So I still prefer the
current solution in this patch for Tegra.

Thanks,
Joseph
Santosh Shilimkar Dec. 20, 2012, 7:16 a.m. UTC | #7
On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>> include the power of GIC. That caused the SGI (Software Generated
>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>
>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>
>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>> stays alive until also the core domain hits idle. By that time a potential
>> pending IPI has long woken up the target CPU again, so no additional
>> checks are needed for functional correct behavior.
>
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.
>
I second Colin on the GIC power states on OMAP.

Regards
Santosh
Peter De Schrijver Dec. 20, 2012, 9:34 a.m. UTC | #8
On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> >> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>> include the power of GIC. That caused the SGI (Software Generated
> >>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>
> >>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>
> >> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >> stays alive until also the core domain hits idle. By that time a potential
> >> pending IPI has long woken up the target CPU again, so no additional
> >> checks are needed for functional correct behavior.
> >
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> >
> I second Colin on the GIC power states on OMAP.

But even then PRCM will only trigger the transition to retention voltage after
both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
PRCM starts the transition.

Cheers,

Peter.
Santosh Shilimkar Dec. 20, 2012, 9:49 a.m. UTC | #9
On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>> <pdeschrijver@nvidia.com> wrote:
>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>
>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>
>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>> stays alive until also the core domain hits idle. By that time a potential
>>>> pending IPI has long woken up the target CPU again, so no additional
>>>> checks are needed for functional correct behavior.
>>>
>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>> checking for an IPI early during idle and aborting won't hurt those
>>> platforms, so I still think it should be in the GIC driver and not by
>>> mapping the GIC registers into a separate driver.
>>>
>> I second Colin on the GIC power states on OMAP.
>
> But even then PRCM will only trigger the transition to retention voltage after
> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> PRCM starts the transition.
>
No it won't. You are talking about the voltage domain retention while
the point is SGI becomes useless once CPU power domain transition to
low power state. And btw, voltage transition also can not happen just
with two CPU's in retention. It does have dependency like cluster power 
state to hit retention.

CPUs have their own power domain state and once they enter into any low
power state apart from ON power state, SGI doesn't work. hope this is
clear.

Regards
Santosh
Peter De Schrijver Dec. 20, 2012, 9:59 a.m. UTC | #10
On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> > On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> >> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> >>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> >>> <pdeschrijver@nvidia.com> wrote:
> >>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>>>> include the power of GIC. That caused the SGI (Software Generated
> >>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>>>
> >>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>>>
> >>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >>>> stays alive until also the core domain hits idle. By that time a potential
> >>>> pending IPI has long woken up the target CPU again, so no additional
> >>>> checks are needed for functional correct behavior.
> >>>
> >>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> >>> rail for the GIC in retention, and I don't think an IPI will wake it
> >>> up.  I believe the same problem also exists for Exynos5.  In any case,
> >>> checking for an IPI early during idle and aborting won't hurt those
> >>> platforms, so I still think it should be in the GIC driver and not by
> >>> mapping the GIC registers into a separate driver.
> >>>
> >> I second Colin on the GIC power states on OMAP.
> >
> > But even then PRCM will only trigger the transition to retention voltage after
> > both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> > PRCM starts the transition.
> >
> No it won't. You are talking about the voltage domain retention while
> the point is SGI becomes useless once CPU power domain transition to
> low power state. And btw, voltage transition also can not happen just
> with two CPU's in retention. It does have dependency like cluster power 
> state to hit retention.
> 
> CPUs have their own power domain state and once they enter into any low
> power state apart from ON power state, SGI doesn't work. hope this is
> clear.

So why does OMAP4 have working coupled idle without the SGI check then?

Cheers,

Peter.
Santosh Shilimkar Dec. 20, 2012, 10:24 a.m. UTC | #11
On Thursday 20 December 2012 03:29 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
>> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
>>> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>>>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>>>> <pdeschrijver@nvidia.com> wrote:
>>>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>>>
>>>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>>>
>>>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>>>> stays alive until also the core domain hits idle. By that time a potential
>>>>>> pending IPI has long woken up the target CPU again, so no additional
>>>>>> checks are needed for functional correct behavior.
>>>>>
>>>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>>>> checking for an IPI early during idle and aborting won't hurt those
>>>>> platforms, so I still think it should be in the GIC driver and not by
>>>>> mapping the GIC registers into a separate driver.
>>>>>
>>>> I second Colin on the GIC power states on OMAP.
>>>
>>> But even then PRCM will only trigger the transition to retention voltage after
>>> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
>>> PRCM starts the transition.
>>>
>> No it won't. You are talking about the voltage domain retention while
>> the point is SGI becomes useless once CPU power domain transition to
>> low power state. And btw, voltage transition also can not happen just
>> with two CPU's in retention. It does have dependency like cluster power
>> state to hit retention.
>>
>> CPUs have their own power domain state and once they enter into any low
>> power state apart from ON power state, SGI doesn't work. hope this is
>> clear.
>
> So why does OMAP4 have working coupled idle without the SGI check then?
>
Because whenever the OMAP4 CPUs enter into any power states apart from
ON, clock-domain force wakeup method us being used to wakeup
secondary CPUs. And secondly CPU RET is not supported on purpose because
of some known IP block issues.

Regards
Santosh
Peter De Schrijver Dec. 20, 2012, 11:14 a.m. UTC | #12
> >
> > So why does OMAP4 have working coupled idle without the SGI check then?
> >
> Because whenever the OMAP4 CPUs enter into any power states apart from
> ON, clock-domain force wakeup method us being used to wakeup
> secondary CPUs. And secondly CPU RET is not supported on purpose because
> of some known IP block issues.
> 

Aha. That explains indeed why you don't need the check.

Cheers,

Peter.
Santosh Shilimkar Dec. 20, 2012, 12:06 p.m. UTC | #13
On Thursday 20 December 2012 04:44 PM, Peter De Schrijver wrote:
>>>
>>> So why does OMAP4 have working coupled idle without the SGI check then?
>>>
>> Because whenever the OMAP4 CPUs enter into any power states apart from
>> ON, clock-domain force wakeup method us being used to wakeup
>> secondary CPUs. And secondly CPU RET is not supported on purpose because
>> of some known IP block issues.
>>
>
> Aha. That explains indeed why you don't need the check.
>
Just for the information on OMAP5 SOC, CPU RET is supported and the
couple idle works on it without IPI. As I mentioned, we do use
clock-domain force wakeup method for wakeup. Code is out of the
tree hence i didn't mention that in first place.

Regards
Santosh
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index b7886f1..c9976e3 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -45,6 +45,8 @@ 
 
 #define FIRST_LEGACY_IRQ 32
 
+#define SGI_MASK 0xFFFF
+
 static int num_ictlrs;
 
 static void __iomem *ictlr_reg_base[] = {
@@ -55,6 +57,19 @@  static void __iomem *ictlr_reg_base[] = {
 	IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
 };
 
+bool tegra_pending_sgi(void)
+{
+	u32 pending_set;
+	void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
+
+	pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
+
+	if (pending_set & SGI_MASK)
+		return true;
+
+	return false;
+}
+
 static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
 {
 	void __iomem *base;
diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
new file mode 100644
index 0000000..5142649
--- /dev/null
+++ b/arch/arm/mach-tegra/irq.h
@@ -0,0 +1,22 @@ 
+/*
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __TEGRA_IRQ_H
+#define __TEGRA_IRQ_H
+
+bool tegra_pending_sgi(void);
+
+#endif