ARM: tegra20: cpuidle: add power-down state

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 <[email protected]>

---
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

arch/arm/mach-tegra/irq.c +15 -0

index b7886f1..c9976e3 100644
@@ -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;

arch/arm/mach-tegra/irq.h new file +22 -1

new file mode 100644
index 0000000..5142649
@@ -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

Quoted 8 lines · Joseph Lo

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.

Quoted 13 lines

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 <[email protected]>

---
V3:
* move the static mapping of GIC addr into tegra_pending_sgi V2:
* new in V2
---

Quoted 8 lines · Joseph Lo

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.

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 23 lines · Colin Cross

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 23 lines · Colin Cross

Quoted 20 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 7 lines

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.

Quoted 31 lines · Santosh Shilimkar

Quoted 30 lines · Colin Cross

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 1 line

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.

Quoted 32 lines · Peter De Schrijver

Quoted 31 lines · Santosh Shilimkar

Quoted 30 lines · Colin Cross

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

Quoted 1 line

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.

Quoted 45 lines · Santosh Shilimkar

Quoted 32 lines · Peter De Schrijver

Quoted 31 lines · Santosh Shilimkar

Quoted 30 lines · Colin Cross

Quoted 14 lines · Peter De Schrijver

Quoted 10 lines · Colin Cross

Quoted 8 lines · Joseph Lo

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.

Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

Quoted 1 line

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.

Quoted 3 lines

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?

Quoted 3 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI. The Tegra20 had a limition to power down both CPU cores. The secondary CPU must waits for CPU0 in powered-down state too. If the secondary CPU be woken up before CPU0 entering powered-down state, then it needs to restore its CPU states and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <[email protected]>
Gary King <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

---
V3:
* dynamic checking of the number of the state counts
* fix the code sequence for aborting cpu_suspend in tegra20_sleep_cpu_secondary_finish
V2:
* no change

---
 arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.h           |   23 +++++
 4 files changed, 261 insertions(+), 5 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +55 -4

index d32e8b0..716aef3 100644
@@ -22,28 +22,112 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/clockchips.h>
 
 #include <asm/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/smp_plat.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra20_idle_lp2(struct cpuidle_device *dev,
+                           struct cpuidle_driver *drv,
+                           int index);
+#endif
+
+static struct cpuidle_state tegra_idle_states[] = {
+       [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+       [1] = {
+               .enter                  = tegra20_idle_lp2,
+               .exit_latency           = 5000,
+               .target_residency       = 10000,
+               .power_usage            = 0,
+               .flags                  = CPUIDLE_FLAG_TIME_VALID,
+               .name                   = "powered-down",
+               .desc                   = "CPU power gated",
+       },
+#endif
+};
 
 static struct cpuidle_driver tegra_idle_driver = {
        .name = "tegra_idle",
        .owner = THIS_MODULE,
        .en_core_tk_irqen = 1,
-       .state_count = 1,
-       .states = {
-               [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
-       },
 };
 
 static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
+#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_SMP
+static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+                                        struct cpuidle_driver *drv,
+                                        int index)
+{
+       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+       cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
+
+       tegra20_cpu_clear_resettable();
+
+       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+       return true;
+}
+#else
+static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+                                               struct cpuidle_driver *drv,
+                                               int index)
+{
+       return true;
+}
+#endif
+
+static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,

arch/arm/mach-tegra/cpuidle-tegra20.c +18 -1

+
+       if (cpu == 0)
+               cpu_do_idle();
+       else
+               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
+
+       cpu_pm_exit();
+       tegra_clear_cpu_in_lp2(cpu);
+
+       local_fiq_enable();
+
+       smp_rmb();
+
+       return entered_lp2 ? index : 0;
+}
+#endif
+
 int __init tegra20_cpuidle_init(void)
 {
-       int ret;
+       int ret, i;
        unsigned int cpu;
        struct cpuidle_device *dev;
        struct cpuidle_driver *drv = &tegra_idle_driver;

Quoted 40 lines · Joseph Lo

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI. The Tegra20 had a limition to power down both CPU cores. The secondary CPU must waits for CPU0 in powered-down state too. If the secondary CPU be woken up before CPU0 entering powered-down state, then it needs to restore its CPU states and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <[email protected]>
Gary King <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

---
V3:
* dynamic checking of the number of the state counts
* fix the code sequence for aborting cpu_suspend in tegra20_sleep_cpu_secondary_finish
V2:
* no change

---
 arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.h           |   23 +++++
 4 files changed, 261 insertions(+), 5 deletions(-)

arch/arm/mach-tegra/cpuidle-tegra20.c +6 -0

index d32e8b0..716aef3 100644
+static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
+                                     struct cpuidle_driver *drv,
+                                     int index)
+{
+       u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+       bool entered_lp2 = false;

arch/arm/mach-tegra/cpuidle-tegra20.c line 100

Quoted 5 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +5 -0

+
+       local_fiq_disable();
+
+       tegra_set_cpu_in_lp2(cpu);
+       cpu_pm_enter();

You must check the return value from cpu_pm_enter and synchronize and abort both cpus.

Quoted 25 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +18 -1

+
+       if (cpu == 0)
+               cpu_do_idle();
+       else
+               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
+
+       cpu_pm_exit();
+       tegra_clear_cpu_in_lp2(cpu);
+
+       local_fiq_enable();
+
+       smp_rmb();
+
+       return entered_lp2 ? index : 0;
+}
+#endif
+
 int __init tegra20_cpuidle_init(void)
 {
-       int ret;
+       int ret, i;
        unsigned int cpu;
        struct cpuidle_device *dev;
        struct cpuidle_driver *drv = &tegra_idle_driver;

arch/arm/mach-tegra/cpuidle-tegra20.c line 133

Quoted 9 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +6 -0

+       drv->state_count = sizeof(tegra_idle_states) /
+                          sizeof(struct cpuidle_state);
+       for (i = 0; i < drv->state_count; i++)
+               memcpy(&drv->states[i], &tegra_idle_states[i],
+                      sizeof(struct cpuidle_state));
+
        ret = cpuidle_register_driver(&tegra_idle_driver);
        if (ret) {
                pr_err("CPUidle driver registration failed\n");

Is there a call to cpu_cluster_pm_enter/exit somewhere else?

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI. The Tegra20 had a limition to power down both CPU cores. The secondary CPU must waits for CPU0 in powered-down state too. If the secondary CPU be woken up before CPU0 entering powered-down state, then it needs to restore its CPU states and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

Quoted 10 lines · Joseph Lo

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI. The Tegra20 had a limition to power down both CPU cores. The secondary CPU must waits for CPU0 in powered-down state too. If the secondary CPU be woken up before CPU0 entering powered-down state, then it needs to restore its CPU states and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

arch/arm/mach-tegra/cpuidle-tegra20.c line 126

Quoted 6 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +2 -0

 
 int __init tegra20_cpuidle_init(void)
 
+	drv->state_count = sizeof(tegra_idle_states) /
+			   sizeof(struct cpuidle_state);

Use ARRAY_SIZE() there?

arch/arm/mach-tegra/cpuidle-tegra20.c line 129

Quoted 4 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0

+	for (i = 0; i < drv->state_count; i++)
+		memcpy(&drv->states[i], &tegra_idle_states[i],
+		       sizeof(struct cpuidle_state));

Can't you call memcpy() once:

memcpy(drv->states, tegra_idle_states,

		drv->state_count * sizeof(drv->states[0]));

... although I personally much preferred when all this was just static initialization directly in tegra_idle_driver, rather than all this messy copying. Really, struct cpuidle_driver should point at the array, rather than including it.

Quoted 9 lines

arch/arm/mach-tegra/pm.c +2 -0

@@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
 
 	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
 		last_cpu = true;
+	else if (phy_cpu_id == 1)
+		tegra20_cpu_set_resettable_soon();
 

arch/arm/mach-tegra/pm.c bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id) · line 180

Quoted 2 lines

arch/arm/mach-tegra/pm.c +0 -0

 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;

Shouldn't the code in that else branch have a run-time check for whether it's running on Tegra20? When compiled without Tegra20 support, tegra20_cpu_set_resettable_soon() is a dummy static inline, but when both Tegra20 and Tegra30 are compiled in, isn't that code going to run when it shouldn't; pm.c being a common file.

Quoted 9 lines · Stephen Warren

Quoted 8 lines · Joseph Lo

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI. The Tegra20 had a limition to power down both CPU cores. The secondary CPU must waits for CPU0 in powered-down state too. If the secondary CPU be woken up before CPU0 entering powered-down state, then it needs to restore its CPU states and waits for next chance.

Quoted 4 lines

arch/arm/mach-tegra/cpuidle-tegra20.c

Quoted 4 lines

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

arch/arm/mach-tegra/cpuidle-tegra20.c +0 -0

Quoted 5 lines

... although I personally much preferred when all this was just static initialization directly in tegra_idle_driver, rather than all this messy copying. Really, struct cpuidle_driver should point at the array, rather than including it.

I think so. If you strongly prefer the original style, I can rollback to the previous version here.

Quoted 9 lines

Quoted 9 lines

arch/arm/mach-tegra/pm.c +2 -0

@@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
 
 	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
 		last_cpu = true;
+	else if (phy_cpu_id == 1)
+		tegra20_cpu_set_resettable_soon();
 

Quoted 8 lines

arch/arm/mach-tegra/pm.c bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id) · line 180

Quoted 2 lines

arch/arm/mach-tegra/pm.c +0 -0

 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;

Shouldn't the code in that else branch have a run-time check for whether it's running on Tegra20? When compiled without Tegra20 support, tegra20_cpu_set_resettable_soon() is a dummy static inline, but when both Tegra20 and Tegra30 are compiled in, isn't that code going to run when it shouldn't; pm.c being a common file.

Because the code didn't hurt Tegra30, so I didn't add a runtime detection there. If you have concern, I can add runtime detection there.

Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These functions were used for CPU powered-down state maintenance.

Quoted 2 lines · Joseph Lo

Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These functions were used for CPU powered-down state maintenance.

I think this isn't adding those functions into tegra_cpu_car_ops, but rather implementing the already existing function pointers for Tegra20, right?

This patch touches the clock driver and Prashant will soon be posting patches that significantly rework the clock driver. Those will conflict. How have you worked with Prashant to resolve the conflicts? Will Prashant's patches be based on top of this series? Please explicitly describe the dependencies and resolution of conflicts when you post the next version. (below the ---, along with the changelog)

Quoted 8 lines · Stephen Warren

Quoted 2 lines · Joseph Lo

Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These functions were used for CPU powered-down state maintenance.

I think this isn't adding those functions into tegra_cpu_car_ops, but rather implementing the already existing function pointers for Tegra20, right?

Yes, exactly.

Quoted 7 lines

This patch touches the clock driver and Prashant will soon be posting patches that significantly rework the clock driver. Those will conflict. How have you worked with Prashant to resolve the conflicts? Will Prashant's patches be based on top of this series? Please explicitly describe the dependencies and resolution of conflicts when you post the next version. (below the ---, along with the changelog)

Yes, there are many patch series may have dependency each other recently. We also want to know what the sequence you expect to merge the patch series to your tree.

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one core to go into this mode before other core. The coupled cpuidle framework can help to sync the MPCore to coupled state then go into "powered-down" idle mode together. The driver can just assume the MPCore come into "powered-down" mode at the same time. No need to take care if the CPU_0 goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI for waiting CPU0 in the same state. When the CPU0 requests powered-down state, it attempts to put the secondary CPU into reset to prevent it from waking up. Then power down both CPUs together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <[email protected]>
Gary King <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

---
V3:
* sqash last two patch in previous version to support coupled cpuidle directly

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one core to go into this mode before other core. The coupled cpuidle framework can help to sync the MPCore to coupled state then go into "powered-down" idle mode together. The driver can just assume the MPCore come into "powered-down" mode at the same time. No need to take care if the CPU_0 goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI for waiting CPU0 in the same state. When the CPU0 requests powered-down state, it attempts to put the secondary CPU into reset to prevent it from waking up. Then power down both CPUs together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

Quoted 16 lines · Joseph Lo

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one core to go into this mode before other core. The coupled cpuidle framework can help to sync the MPCore to coupled state then go into "powered-down" idle mode together. The driver can just assume the MPCore come into "powered-down" mode at the same time. No need to take care if the CPU_0 goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI for waiting CPU0 in the same state. When the CPU0 requests powered-down state, it attempts to put the secondary CPU into reset to prevent it from waking up. Then power down both CPUs together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

arch/arm/mach-tegra/cpuidle-tegra20.c static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device); · line 92

Quoted 3 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +1 -0

 
+static void tegra20_wake_reset_cpu_1(void)

Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly more descriptive name? I assume the function works on CPU1, which is assumed to be in reset, and removes reset from the CPU so it boots.

arch/arm/mach-tegra/cpuidle-tegra20.c int __init tegra20_cpuidle_init(void) · line 246

Quoted 7 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0

@@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
 	for_each_possible_cpu(cpu) {
 		dev = &per_cpu(tegra_idle_device, cpu);
 		dev->cpu = cpu;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif

That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be enabled for this file to be compiled. So, you can drop the ifdef, and make the code uncondtional.

config ARCH_TEGRA_2x_SOC
bool "Enable support for Tegra20 family"

(quoted diff) +1 -0

+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 

	select ARCH_REQUIRE_GPIOLIB 
	select ARM_ERRATA_720789 
	select ARM_ERRATA_742230 

Quoted 22 lines · Stephen Warren

Quoted 20 lines · Joseph Lo

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one core to go into this mode before other core. The coupled cpuidle framework can help to sync the MPCore to coupled state then go into "powered-down" idle mode together. The driver can just assume the MPCore come into "powered-down" mode at the same time. No need to take care if the CPU_0 goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores. When the secondary CPU requests to enter powered-down state, it saves its own contexts and then enters WFI for waiting CPU0 in the same state. When the CPU0 requests powered-down state, it attempts to put the secondary CPU into reset to prevent it from waking up. Then power down both CPUs together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code which is exactly the same meaning of "CPU power down".

arch/arm/mach-tegra/cpuidle-tegra20.c +1 -0

 
+static void tegra20_wake_reset_cpu_1(void)

Quoted 4 lines

Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly more descriptive name? I assume the function works on CPU1, which is assumed to be in reset, and removes reset from the CPU so it boots.

Yes.
Do you mean this function running on CPU1? It runs on CPU0 to wake up CPU1. About the function name, it looks more appropriate.

Quoted 11 lines

arch/arm/mach-tegra/cpuidle-tegra20.c int __init tegra20_cpuidle_init(void) · line 246

Quoted 7 lines

arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0

@@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
 	for_each_possible_cpu(cpu) {
 		dev = &per_cpu(tegra_idle_device, cpu);
 		dev->cpu = cpu;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif

That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be enabled for this file to be compiled. So, you can drop the ifdef, and make the code uncondtional.

No, I found I need this if I want to make the device works on UP case. Then I need to do something like below, when we disable SMP.

Quoted 6 lines

config ARCH_TEGRA_2x_SOC
bool "Enable support for Tegra20 family"

(quoted diff) +1 -0

+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 

	select ARCH_REQUIRE_GPIOLIB 
	select ARM_ERRATA_720789 
	select ARM_ERRATA_742230 

This will be updated in next version as well.

This adds a "powered-down" state in cpuidle for Tegra20. It requires power gating both CPU cores. When the CPU1 requests to enter "powered-down" state, it saves its own state and then enters WFI. When the CPU0 requests the same state, it attempts to put the CPU1 into reset to prevent it from waking up. Then power down both CPUs together and turn off the CPU rail.

If the CPU1 be woken up before CPU0 entering powered-down state, then it needs to restore it's CPU state and waits for next chance.

Quoted 8 lines · Joseph Lo

This adds a "powered-down" state in cpuidle for Tegra20. It requires power gating both CPU cores. When the CPU1 requests to enter "powered-down" state, it saves its own state and then enters WFI. When the CPU0 requests the same state, it attempts to put the CPU1 into reset to prevent it from waking up. Then power down both CPUs together and turn off the CPU rail.

If the CPU1 be woken up before CPU0 entering powered-down state, then it needs to restore it's CPU state and waits for next chance.

Joseph,

What's the status of this series? It looks like there are quite a few comments on V3; I assume you'll post a new version to address those. Will you base the new version on top of Prashant's common clock cleanup to remove the conflicts with that?