| Message ID | 1355797861-12759-2-git-send-email-josephl@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 > > ---
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.
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.
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
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
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
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.
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
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.
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
> > > > 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.
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 --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
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