Skip to contentSkip to conversation

ARM: tegra20: cpuidle: add power-down state

PATCH Superseded Joseph Lo 4 participants 21 messages Dec 3, 2012 – Dec 5, 2012

Subsystems (3)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Joseph Lo

author
#0 lore raw

cover ARM: tegra20: cpuidle: add power-down state

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.

Please note.
We also fix the potensial deadlock issue for coupled cpuidle framework. ("cpuidle: coupled: fix the potensial race condition and deadlock") Before the patch be applied, please don't merge the last patch that apply coupled cpuidle for "powered-down" cpuidle driver.

Verified on Seaboard(Tegra20) and Cardhu(Tegra30).

Joseph Lo (5):
ARM: tegra20: cpuidle: add powered-down state for secondary CPU
ARM: tegra20: clocks: add CPU low-power function into
tegra_cpu_car_ops
ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit
ARM: tegra20: cpuidle: add powered-down state for CPU0
ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |  206 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/flowctrl.c        |   38 +++++-
 arch/arm/mach-tegra/flowctrl.h        |    4 +
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  198 +++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++
 arch/arm/mach-tegra/sleep.h           |   26 ++++
 arch/arm/mach-tegra/tegra20_clocks.c  |  102 ++++++++++++++++
 9 files changed, 591 insertions(+), 5 deletions(-)
author
#1 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

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

---
 arch/arm/mach-tegra/cpuidle-tegra20.c |   84 +++++++++++++++++++
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  145 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.h           |   23 +++++
 4 files changed, 254 insertions(+), 0 deletions(-)
arch/arm/mach-tegra/cpuidle-tegra20.c +84 -0
index d32e8b0..9d59a46 100644
@@ -22,21 +22,105 @@
 #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_driver tegra_idle_driver = {
 	.name = "tegra_idle",
 	.owner = THIS_MODULE,
 	.en_core_tk_irqen = 1,
+#ifdef CONFIG_PM_SLEEP
+	.state_count = 2,
+#else
 	.state_count = 1,
+#endif
 	.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 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);
+
+	smp_wmb();
+
+	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,
+				      struct cpuidle_driver *drv,
+				      int index)
+{
+	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+	bool entered_lp2 = false;
+
+	local_fiq_disable();
+
+	tegra_set_cpu_in_lp2(cpu);
+	cpu_pm_enter();
+
+	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;
arch/arm/mach-tegra/pm.c +2 -0
index 1b11707..db72ea9 100644
@@ -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();
 
 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;
arch/arm/mach-tegra/sleep-tegra20.S +145 -0

154 lines not on this page · raw message

arch/arm/mach-tegra/sleep.h +23 -0
index 9821ee7..a02f71a 100644
@@ -25,6 +25,19 @@
 					+ IO_PPSB_VIRT)
 #define TEGRA_CLK_RESET_VIRT (TEGRA_CLK_RESET_BASE - IO_PPSB_PHYS \
 					+ IO_PPSB_VIRT)
+#define TEGRA_PMC_VIRT	(TEGRA_PMC_BASE - IO_APB_PHYS + IO_APB_VIRT)
+
+/* PMC_SCRATCH37-39 and 41 are used for tegra_pen_lock and idle */
+#define PMC_SCRATCH37	0x130
+#define PMC_SCRATCH38	0x134
+#define PMC_SCRATCH39	0x138
+#define PMC_SCRATCH41	0x140
+
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+#define CPU_RESETTABLE		2
+#define CPU_RESETTABLE_SOON	1
+#define CPU_NOT_RESETTABLE	0
+#endif
 
 #ifdef __ASSEMBLY__
 /* returns the offset of the flow controller halt register for a cpu */
@@ -104,6 +117,8 @@ exit_l2_resume:
 .endm
 #endif /* CONFIG_CACHE_L2X0 */
 #else
+void tegra_pen_lock(void);
+void tegra_pen_unlock(void);
 void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
 
@@ -115,6 +130,14 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_clear_resettable(void);
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+void tegra20_cpu_set_resettable_soon(void);
+#else
+static inline void tegra20_cpu_set_resettable_soon(void) {}
+#endif
+
+int tegra20_sleep_cpu_secondary_finish(unsigned long);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
Signature · 1 line
1.7.0.4
#2 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

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

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/sleep-tegra20.S   |  145 +++++++++++++++++++++++++++++++++

Is it actually necessary to implement those parts of the code in assembly? It looks like it's mostly just memory read/write and a few barriers, all of which can be coded in regular C or invoked from C.

The reason I ask is that we eventually want to remove all hard-coded virtual memory addresses, such as TEGRA_PMC_VIRT, and it'll be a lot easier to access memory relative to a variable base address from C than assembly.

Now, perhaps we can solve this later; when we actually try to get this code into drivers/cpuidle/ and remove the hard-coded virtual memory addresses. Still, it'd be great if we didn't have to re-write the code as much (from .S->.c) when making those changes...

author
#3 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

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

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/sleep-tegra20.S   |  145 +++++++++++++++++++++++++++++++++

Is it actually necessary to implement those parts of the code in assembly? It looks like it's mostly just memory read/write and a few barriers, all of which can be coded in regular C or invoked from C.

The reason I ask is that we eventually want to remove all hard-coded virtual memory addresses, such as TEGRA_PMC_VIRT, and it'll be a lot easier to access memory relative to a variable base address from C than assembly.

Quoted 4 lines

Now, perhaps we can solve this later; when we actually try to get this code into drivers/cpuidle/ and remove the hard-coded virtual memory addresses. Still, it'd be great if we didn't have to re-write the code as much (from .S->.c) when making those changes...

OK. Let me think how to do this for both Tegra20 and Tegra30.

Thanks,
Joseph

#4 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

In reply to Joseph Lo
Quoted 12 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/sleep-tegra20.S ENTRY(tegra20_cpu_shutdown) · line 92

Quoted 9 lines
arch/arm/mach-tegra/sleep-tegra20.S +8 -0
 
+/*
+ * tegra_pen_lock
+ *
+ * spinlock implementation with no atomic test-and-set and no coherence
+ * using Peterson's algorithm on strongly-ordered registers
+ * used to synchronize a cpu waking up from wfi with entering lp2 on idle
+ *
+ * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)

A link to a description of that algorithm would be useful.

Quoted 6 lines

+ * on cpu 0:
+ * SCRATCH38 = r2 = flag[0]
+ * SCRATCH39 = r3 = flag[1]
+ * on cpu1:
+ * SCRATCH39 = r2 = flag[1]
+ * SCRATCH38 = r3 = flag[0]

That implies that r2/r3 are used for different purposes on the 2 CPUs, and/or shadow the values of different registers. However, I see nothing in the code which is conditional on cpu ID.

author
#5 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

Quoted 23 lines · Stephen Warren
Quoted 12 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/sleep-tegra20.S ENTRY(tegra20_cpu_shutdown) · line 92

Quoted 9 lines
arch/arm/mach-tegra/sleep-tegra20.S +8 -0
 
+/*
+ * tegra_pen_lock
+ *
+ * spinlock implementation with no atomic test-and-set and no coherence
+ * using Peterson's algorithm on strongly-ordered registers
+ * used to synchronize a cpu waking up from wfi with entering lp2 on idle
+ *
+ * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)
Quoted 2 lines

A link to a description of that algorithm would be useful.

This algorithm was very famous. It can be very easy to find on the internet. But if you want the link of the paper, it may not be available. It usually need to access the paper as a member.

Can you accept a link from wikipedia?
http://en.wikipedia.org/wiki/Peterson's_algorithm#cite_ref-1

Quoted 10 lines
Quoted 6 lines

+ * on cpu 0:
+ * SCRATCH38 = r2 = flag[0]
+ * SCRATCH39 = r3 = flag[1]
+ * on cpu1:
+ * SCRATCH39 = r2 = flag[1]
+ * SCRATCH38 = r3 = flag[0]

That implies that r2/r3 are used for different purposes on the 2 CPUs, and/or shadow the values of different registers. However, I see nothing in the code which is conditional on cpu ID.

here:

+       cpu_id  r0
+       add     r1, r3, #PMC_SCRATCH37
+       cmp     r0, #0
+       addeq   r2, r3, #PMC_SCRATCH38
+       addeq   r3, r3, #PMC_SCRATCH39
+       addne   r2, r3, #PMC_SCRATCH39
+       addne   r3, r3, #PMC_SCRATCH38

It's proper to say that the SCRATCH38/SCRATCH39 are used for dedicate cpu lock flag on the 2 CPUs.

Thanks,
Joseph

#6 lore raw

PATCH 1/5 ARM: tegra20: cpuidle: add powered-down state for secondary CPU

Quoted 24 lines · Joseph Lo
Quoted 23 lines · Stephen Warren
Quoted 21 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/sleep-tegra20.S +8 -0
 
+/*
+ * tegra_pen_lock
+ *
+ * spinlock implementation with no atomic test-and-set and no coherence
+ * using Peterson's algorithm on strongly-ordered registers
+ * used to synchronize a cpu waking up from wfi with entering lp2 on idle
+ *
+ * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)
Quoted 8 lines
Quoted 2 lines

A link to a description of that algorithm would be useful.

This algorithm was very famous. It can be very easy to find on the internet. But if you want the link of the paper, it may not be available. It usually need to access the paper as a member.

Can you accept a link from wikipedia?
http://en.wikipedia.org/wiki/Peterson's_algorithm#cite_ref-1

Yes, that's fine (although I'd drop the anchor from the URL; i.e. remove "#cite_ref-1").

Quoted 19 lines
Quoted 10 lines
Quoted 6 lines

+ * on cpu 0:
+ * SCRATCH38 = r2 = flag[0]
+ * SCRATCH39 = r3 = flag[1]
+ * on cpu1:
+ * SCRATCH39 = r2 = flag[1]
+ * SCRATCH38 = r3 = flag[0]

That implies that r2/r3 are used for different purposes on the 2 CPUs, and/or shadow the values of different registers. However, I see nothing in the code which is conditional on cpu ID.

here:

arch/arm/mach-tegra/sleep-tegra20.S +7 -0
+       cpu_id  r0
+       add     r1, r3, #PMC_SCRATCH37
+       cmp     r0, #0
+       addeq   r2, r3, #PMC_SCRATCH38
+       addeq   r3, r3, #PMC_SCRATCH39
+       addne   r2, r3, #PMC_SCRATCH39
+       addne   r3, r3, #PMC_SCRATCH38
Quoted 2 lines

It's proper to say that the SCRATCH38/SCRATCH39 are used for dedicate cpu lock flag on the 2 CPUs.

Doh. I overlooked the "cpu_id" invocation.

author
#7 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

In reply to 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.

---
 arch/arm/mach-tegra/tegra20_clocks.c |  102 ++++++++++++++++++++++++++++++++++
 1 files changed, 102 insertions(+), 0 deletions(-)
arch/arm/mach-tegra/tegra20_clocks.c +102 -0
index 4eb6bc8..05968d7 100644
@@ -159,6 +159,31 @@
 #define CPU_CLOCK(cpu)	(0x1 << (8 + cpu))
 #define CPU_RESET(cpu)	(0x1111ul << (cpu))
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+#define CLK_RESET_PLLX_BASE	0xe0
+#define CLK_RESET_PLLX_MISC	0xe4
+
+#define CLK_RESET_SOURCE_CSITE	0x1d4
+
+#define CLK_RESET_CCLK_BURST_POLICY_SHIFT	28
+#define CLK_RESET_CCLK_RUN_POLICY_SHIFT		4
+#define CLK_RESET_CCLK_IDLE_POLICY_SHIFT	0
+#define CLK_RESET_CCLK_IDLE_POLICY		1
+#define CLK_RESET_CCLK_RUN_POLICY		2
+#define CLK_RESET_CCLK_BURST_POLICY_PLLX	8
+
+#ifdef CONFIG_PM_SLEEP
+static struct cpu_clk_suspend_context {
+	u32 pllx_misc;
+	u32 pllx_base;
+
+	u32 cpu_burst;
+	u32 clk_csite_src;
+	u32 cclk_divider;
+} tegra20_cpu_clk_sctx;
+#endif
+
 static void __iomem *reg_clk_base = IO_ADDRESS(TEGRA_CLK_RESET_BASE);
 static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
 
@@ -1609,12 +1634,89 @@ static void tegra20_disable_cpu_clock(u32 cpu)
 	       reg_clk_base + TEGRA_CLK_RST_CONTROLLER_CLK_CPU_CMPLX);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static bool tegra20_cpu_rail_off_ready(void)
+{
+	unsigned int cpu_rst_status;
+
+	cpu_rst_status = readl(reg_clk_base +
+			       TEGRA_CLK_RST_CONTROLLER_RST_CPU_CMPLX_SET);
+
+	if ((cpu_rst_status & 0x2) != 0x2)
+		return false;
+
+	return true;
+}
+
+static void tegra20_cpu_clock_suspend(void)
+{
+	/* switch coresite to clk_m, save off original source */
+	tegra20_cpu_clk_sctx.clk_csite_src =
+				readl(reg_clk_base + CLK_RESET_SOURCE_CSITE);
+	writel(3<<30, reg_clk_base + CLK_RESET_SOURCE_CSITE);
+
+	tegra20_cpu_clk_sctx.cpu_burst =
+				readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	tegra20_cpu_clk_sctx.pllx_base =
+				readl(reg_clk_base + CLK_RESET_PLLX_BASE);
+	tegra20_cpu_clk_sctx.pllx_misc =
+				readl(reg_clk_base + CLK_RESET_PLLX_MISC);
+	tegra20_cpu_clk_sctx.cclk_divider =
+				readl(reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+}
+
+static void tegra20_cpu_clock_resume(void)
+{
+	unsigned int reg, policy;
+
+	/* Is CPU complex already running on PLLX? */
+	reg = readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	policy = (reg >> CLK_RESET_CCLK_BURST_POLICY_SHIFT) & 0xF;
+
+	if (policy == CLK_RESET_CCLK_IDLE_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_IDLE_POLICY_SHIFT) & 0xF;
+	else if (policy == CLK_RESET_CCLK_RUN_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_RUN_POLICY_SHIFT) & 0xF;
+	else
+		BUG();
+
+	if (reg != CLK_RESET_CCLK_BURST_POLICY_PLLX) {
+		/* restore PLLX settings if CPU is on different PLL */
+		writel(tegra20_cpu_clk_sctx.pllx_misc,
+					reg_clk_base + CLK_RESET_PLLX_MISC);
+		writel(tegra20_cpu_clk_sctx.pllx_base,
+					reg_clk_base + CLK_RESET_PLLX_BASE);
+
+		/* wait for PLL stabilization if PLLX was enabled */
+		if (tegra20_cpu_clk_sctx.pllx_base & (1 << 30))
+			udelay(300);
+	}
+
+	/*
+	 * Restore original burst policy setting for calls resulting from CPU
+	 * LP2 in idle or system suspend.
+	 */
+	writel(tegra20_cpu_clk_sctx.cclk_divider,
+					reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+	writel(tegra20_cpu_clk_sctx.cpu_burst,
+					reg_clk_base + CLK_RESET_CCLK_BURST);
+
+	writel(tegra20_cpu_clk_sctx.clk_csite_src,
+					reg_clk_base + CLK_RESET_SOURCE_CSITE);
+}
+#endif
+
 static struct tegra_cpu_car_ops tegra20_cpu_car_ops = {
 	.wait_for_reset	= tegra20_wait_cpu_in_reset,
 	.put_in_reset	= tegra20_put_cpu_in_reset,
 	.out_of_reset	= tegra20_cpu_out_of_reset,
 	.enable_clock	= tegra20_enable_cpu_clock,
 	.disable_clock	= tegra20_disable_cpu_clock,
+#ifdef CONFIG_PM_SLEEP
+	.rail_off_ready = tegra20_cpu_rail_off_ready,
+	.suspend	= tegra20_cpu_clock_suspend,
+	.resume		= tegra20_cpu_clock_resume,
+#endif
 };
 
 void __init tegra20_cpu_car_ops_init(void)
Signature · 1 line
1.7.0.4
#8 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

arch/arm/mach-tegra/tegra20_clocks.c static void tegra20_disable_cpu_clock(u32 cpu) · line 1649

Quoted 12 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.

arch/arm/mach-tegra/tegra20_clocks.c +6 -0
 
+static bool tegra20_cpu_rail_off_ready(void)
 
+	if ((cpu_rst_status & 0x2) != 0x2)
+		return false;
+
+	return true;
+}

Perhaps simplify that to:

return cpu_rst_status & 2;

or perhaps if that generates an int->bool performance warning:

return !!(cpu_rst_status & 2);

or:

return (cpu_rst_status >> 1) & 1;

author
#9 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

Quoted 16 lines · Stephen Warren

arch/arm/mach-tegra/tegra20_clocks.c static void tegra20_disable_cpu_clock(u32 cpu) · line 1649

Quoted 12 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.

arch/arm/mach-tegra/tegra20_clocks.c +6 -0
 
+static bool tegra20_cpu_rail_off_ready(void)
 
+	if ((cpu_rst_status & 0x2) != 0x2)
+		return false;
+
+	return true;
+}

Perhaps simplify that to:

Quoted 5 lines

return cpu_rst_status & 2;

or perhaps if that generates an int->bool performance warning:

return !!(cpu_rst_status & 2);

OK. Will do this.

Thanks,
Joseph

#10 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

In reply to Joseph Lo
Quoted 7 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.

---
  arch/arm/mach-tegra/tegra20_clocks.c |  102 ++++++++++++++++++++++++++++++++++
  1 files changed, 102 insertions(+), 0 deletions(-)

I have a concern here, I am working on removing these ops and replacing it with clock.
Adding suspend/resume to these will make it more difficult to remove.

Any other way to implement this?

Quoted 8 lines
arch/arm/mach-tegra/tegra20_clocks.c +0 -0
index 4eb6bc8..05968d7 100644
@@ -159,6 +159,31 @@
  #define CPU_CLOCK(cpu)	(0x1 << (8 + cpu))
  #define CPU_RESET(cpu)	(0x1111ul << (cpu))
 
author
#11 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

Quoted 9 lines · Prashant Gaikwad
Quoted 7 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.

---
  arch/arm/mach-tegra/tegra20_clocks.c |  102 ++++++++++++++++++++++++++++++++++
  1 files changed, 102 insertions(+), 0 deletions(-)
Quoted 6 lines

I have a concern here, I am working on removing these ops and replacing it with clock.
Adding suspend/resume to these will make it more difficult to remove.

Any other way to implement this?

Prashant,

Thanks for remind.
Actually the "tegra_cpu_car_ops" is more like reset & suspend/resume handling, it's not really related to clock framework. So it's more like some functions for CPU power management control. Maybe I can move them (Tegra20 & Tegra30) to another pm related file later.

How do you think, Stephen?

Thanks,
Joseph

#12 lore raw

PATCH 2/5 ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

Quoted 19 lines · Joseph Lo
Quoted 9 lines · Prashant Gaikwad
Quoted 7 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.

---
  arch/arm/mach-tegra/tegra20_clocks.c |  102 ++++++++++++++++++++++++++++++++++
  1 files changed, 102 insertions(+), 0 deletions(-)
Quoted 6 lines

I have a concern here, I am working on removing these ops and replacing it with clock.
Adding suspend/resume to these will make it more difficult to remove.

Any other way to implement this?

Prashant,

Quoted 7 lines

Thanks for remind.
Actually the "tegra_cpu_car_ops" is more like reset & suspend/resume handling, it's not really related to clock framework. So it's more like some functions for CPU power management control. Maybe I can move them (Tegra20 & Tegra30) to another pm related file later.

How do you think, Stephen?

Well, the CAR (Clock And Reset) HW module implements this functionality, right? As such, the code should be part of the clock driver. The whole reason we have the car_ops in the first place is so that only the clock driver touches the clock HW module. I hope we'll eventually do the same for all HW modules.

author
#13 lore raw

PATCH 3/5 ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit

In reply to Joseph Lo

The flow controller can help CPU to go into suspend mode (powered-down state). When CPU go into powered-down state, it needs some careful settings before getting into and after leaving. The enter and exit functions do that by configuring appropriate mode for flow controller.

---
 arch/arm/mach-tegra/flowctrl.c |   38 +++++++++++++++++++++++++++++++++-----
 arch/arm/mach-tegra/flowctrl.h |    4 ++++
 2 files changed, 37 insertions(+), 5 deletions(-)
arch/arm/mach-tegra/flowctrl.c +33 -5
index a2250dd..9c44788 100644
@@ -25,6 +25,7 @@
 
 #include "flowctrl.h"
 #include "iomap.h"
+#include "fuse.h"
 
 u8 flowctrl_offset_halt_cpu[] = {
 	FLOW_CTRL_HALT_CPU0_EVENTS,
@@ -75,11 +76,26 @@ void flowctrl_cpu_suspend_enter(unsigned int cpuid)
 	int i;
 
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfe */
+		reg |= TEGRA20_FLOW_CTRL_CSR_WFE_CPU0 << cpuid;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfi */
+		reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;
+		break;
+	}
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr flag */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event flag */
-	reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;	/* pwr gating on wfi */
 	reg |= FLOW_CTRL_CSR_ENABLE;			/* pwr gating */
 	flowctrl_write_cpu_csr(cpuid, reg);
 
@@ -99,8 +115,20 @@ void flowctrl_cpu_suspend_exit(unsigned int cpuid)
 
 	/* Disable powergating via flow controller for CPU0 */
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	}
 	reg &= ~FLOW_CTRL_CSR_ENABLE;			/* clear enable */
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event */
arch/arm/mach-tegra/flowctrl.h +4 -0
index 0798dec..67eab56 100644
@@ -34,6 +34,10 @@
 #define FLOW_CTRL_HALT_CPU1_EVENTS	0x14
 #define FLOW_CTRL_CPU1_CSR		0x18
 
+#define TEGRA20_FLOW_CTRL_CSR_WFE_CPU0		(1 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP	(3 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP	0
+
 #define TEGRA30_FLOW_CTRL_CSR_WFI_CPU0		(1 << 8)
 #define TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP	(0xF << 4)
 #define TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP	(0xF << 8)
Signature · 1 line
1.7.0.4
author
#14 lore raw

PATCH 4/5 ARM: tegra20: cpuidle: add powered-down state for CPU0

In reply to 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 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]>

---
 arch/arm/mach-tegra/cpuidle-tegra20.c |  129 ++++++++++++++++++++++++++++++++-
 arch/arm/mach-tegra/sleep-tegra20.S   |   53 ++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++++
 arch/arm/mach-tegra/sleep.h           |    3 +
 4 files changed, 200 insertions(+), 4 deletions(-)
arch/arm/mach-tegra/cpuidle-tegra20.c +125 -4
index 9d59a46..9371a00 100644
@@ -32,6 +32,9 @@
 
 #include "pm.h"
 #include "sleep.h"
+#include "iomap.h"
+#include "tegra_cpu_car.h"
+#include "flowctrl.h"
 
 #ifdef CONFIG_PM_SLEEP
 static int tegra20_idle_lp2(struct cpuidle_device *dev,
@@ -68,6 +71,114 @@ static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
 #ifdef CONFIG_PM_SLEEP
 #ifdef CONFIG_SMP
+static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
+
+static int tegra20_reset_sleeping_cpu(int cpu)
+{
+	int ret = 0;
+
+	BUG_ON(cpu == 0);
+	BUG_ON(cpu == smp_processor_id());
+	tegra_pen_lock();
+
+	if (readl(pmc + PMC_SCRATCH41) == CPU_RESETTABLE)
+		tegra20_cpu_shutdown(cpu);
+	else
+		ret = -EINVAL;
+
+	tegra_pen_unlock();
+
+	return ret;
+}
+
+static void tegra20_wake_reset_cpu(int cpu)
+{
+	BUG_ON(cpu == 0);
+	BUG_ON(cpu == smp_processor_id());
+
+	tegra_pen_lock();
+
+	tegra20_cpu_clear_resettable();
+
+	/* enable cpu clock on cpu */
+	tegra_enable_cpu_clock(cpu);
+
+	/* take the CPU out of reset */
+	tegra_cpu_out_of_reset(cpu);
+
+	/* unhalt the cpu */
+	flowctrl_write_cpu_halt(cpu, 0);
+
+	tegra_pen_unlock();
+}
+
+static int tegra20_reset_other_cpus(int cpu)
+{
+	int i;
+	int ret = 0;
+
+	BUG_ON(cpu != 0);
+
+	for_each_online_cpu(i) {
+		if (i != cpu) {
+			if (tegra20_reset_sleeping_cpu(i)) {
+				ret = -EBUSY;
+				break;
+			}
+		}
+	}
+
+	if (ret) {
+		for_each_online_cpu(i) {
+			if (i != cpu)
+				tegra20_wake_reset_cpu(i);
+		}
+		return ret;
+	}
+
+	return 0;
+}
+#else
+static inline void tegra20_wake_reset_cpu(int cpu)
+{
+}
+
+static inline int tegra20_reset_other_cpus(int cpu)
+{
+	return 0;
+}
+#endif
+
+static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
+					   struct cpuidle_driver *drv,
+					   int index)
+{
+	int i;
+	struct cpuidle_state *state = &drv->states[index];
+	u32 cpu_on_time = state->exit_latency;
+	u32 cpu_off_time = state->target_residency - state->exit_latency;
+
+	while (tegra20_cpu_is_resettable_soon())
+		cpu_relax();
+
+	if (tegra20_reset_other_cpus(dev->cpu) || !tegra_cpu_rail_off_ready())
+		return false;
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	for_each_online_cpu(i) {
+		if (i != dev->cpu)
+			tegra20_wake_reset_cpu(i);
+	}
+
+	return true;
+}
+
+#ifdef CONFIG_SMP
 static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 					 struct cpuidle_driver *drv,
 					 int index)
@@ -99,16 +210,22 @@ static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
 {
 	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
 	bool entered_lp2 = false;
+	bool last_cpu;
 
 	local_fiq_disable();
 
-	tegra_set_cpu_in_lp2(cpu);
+	last_cpu =  tegra_set_cpu_in_lp2(cpu);
 	cpu_pm_enter();
 
-	if (cpu == 0)
-		cpu_do_idle();
-	else
+	if (cpu == 0) {
+		if (last_cpu)
+			entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv,
+								     index);
+		else
+			cpu_do_idle();
+	} else {
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
+	}
 
 	cpu_pm_exit();
 	tegra_clear_cpu_in_lp2(cpu);
@@ -128,6 +245,10 @@ int __init tegra20_cpuidle_init(void)
 	struct cpuidle_device *dev;
 	struct cpuidle_driver *drv = &tegra_idle_driver;
 
+#ifdef CONFIG_PM_SLEEP
+	tegra_tear_down_cpu = tegra20_tear_down_cpu;
+#endif
+
 	ret = cpuidle_register_driver(&tegra_idle_driver);
 	if (ret) {
 		pr_err("CPUidle driver registration failed\n");
arch/arm/mach-tegra/sleep-tegra20.S +53 -0

69 lines not on this page · raw message

arch/arm/mach-tegra/sleep.S +19 -0
index 26afa7c..32beb88 100644
@@ -34,6 +34,9 @@
 #include "flowctrl.h"
 #include "sleep.h"
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+
 #ifdef CONFIG_PM_SLEEP
 /*
  * tegra_disable_clean_inv_dcache
@@ -108,4 +111,20 @@ ENTRY(tegra_shut_off_mmu)
 	mov	pc, r0
 ENDPROC(tegra_shut_off_mmu)
 	.popsection
+
+/*
+ * tegra_cpu_pllp
+ *
+ * In LP2 the normal cpu clock pllx will be turned off. Switch the CPU to pllp
+ */
+ENTRY(tegra_cpu_pllp)
+	/* in LP2 idle (SDRAM active), set the CPU burst policy to PLLP */
+	mov32	r5, TEGRA_CLK_RESET_BASE
+	mov	r0, #(2 << 28)			@ burst policy = run mode
+	orr	r0, r0, #(4 << 4)		@ use PLLP in run mode burst
+	str	r0, [r5, #CLK_RESET_CCLK_BURST]
+	mov	r0, #0
+	str	r0, [r5, #CLK_RESET_CCLK_DIVIDER]
+	mov	pc, lr
+ENDPROC(tegra_cpu_pllp)
 #endif
arch/arm/mach-tegra/sleep.h +3 -0
index a02f71a..4d2b173 100644
@@ -130,6 +130,8 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_shutdown(int cpu);
+int tegra20_cpu_is_resettable_soon(void);
 void tegra20_cpu_clear_resettable(void);
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 void tegra20_cpu_set_resettable_soon(void);
@@ -138,6 +140,7 @@ static inline void tegra20_cpu_set_resettable_soon(void) {}
 #endif
 
 int tegra20_sleep_cpu_secondary_finish(unsigned long);
+void tegra20_tear_down_cpu(void);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
Signature · 1 line
1.7.0.4
#15 lore raw

PATCH 4/5 ARM: tegra20: cpuidle: add powered-down state for CPU0

Quoted 11 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 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 81

Quoted 7 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +6 -0
 
+static int tegra20_reset_sleeping_cpu(int cpu)
+{
+	int ret = 0;
+
+	BUG_ON(cpu == 0);
+	BUG_ON(cpu == smp_processor_id());

Will this code ever be used on anything other than Tegra20? I assume not since it's in a Tegra20-specific file. Given that, it seems much of the code could be made a lot simpler, e.g. by removing the "cpu" parameter here, which would avoid requiring those BUG()s. You'd probably want to rename the function e.g. tegra20_reset_cpu_1_sleeping().

Quoted 16 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +16 -0
+static int tegra20_reset_other_cpus(int cpu)
+{
+	int i;
+	int ret = 0;
+
+	BUG_ON(cpu != 0);
+
+	for_each_online_cpu(i) {
+		if (i != cpu) {
+			if (tegra20_reset_sleeping_cpu(i)) {
+				ret = -EBUSY;
+				break;
+			}
+		}
+	}
+

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

Quoted 10 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +10 -0
+	if (ret) {
+		for_each_online_cpu(i) {
+			if (i != cpu)
+				tegra20_wake_reset_cpu(i);
+		}
+		return ret;
+	}
+
+	return 0;
+}

Equally, couldn't that simply be:

static int tegra20_reset_cpu_1(void)

{
	if (!cpu_is_online() || !tegra20_reset_cpu1_sleeping())
		return 0;

	tegra20_wake_reset_cpu_1();
	return -EBUSY;
}

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

Quoted 8 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +7 -0
+static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
+					   struct cpuidle_driver *drv,
+					   int index)
 
+	for_each_online_cpu(i) {
+		if (i != dev->cpu)
+			tegra20_wake_reset_cpu(i);
+	}

Similarly there, we know CPU 0 is executing the code and the only other CPU is CPU 1, so:

if (cpu_is_online(1))

	tegra20_wake_reset_cpu(1);

? Admittedly the savings aren't so clear there, since there's less code to begin with, but it's still more obvious that way that there are fewer cases the code will ever need to cover.

arch/arm/mach-tegra/sleep.S

Quoted 2 lines
arch/arm/mach-tegra/sleep-tegra20.S +0 -0
arch/arm/mach-tegra/sleep.S +0 -0

Similar comments about C-vs-assembly here as before, for at least some of the functions.

arch/arm/mach-tegra/sleep.S ENTRY(tegra_shut_off_mmu) · line 120

Quoted 6 lines
arch/arm/mach-tegra/sleep.S +6 -0
+/*
+ * tegra_cpu_pllp
+ *
+ * In LP2 the normal cpu clock pllx will be turned off. Switch the CPU to pllp
+ */
+ENTRY(tegra_cpu_pllp)

There's no verb in that function name. tegra_switch_cpu_to_pllp() might be a better name.

author
#16 lore raw

PATCH 4/5 ARM: tegra20: cpuidle: add powered-down state for CPU0

Quoted 13 lines · Stephen Warren
Quoted 11 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 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 +0 -0
Quoted 13 lines

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

Quoted 6 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +6 -0
+static int tegra20_reset_sleeping_cpu(int cpu)
+{
+	int ret = 0;
+
+	BUG_ON(cpu == 0);
+	BUG_ON(cpu == smp_processor_id());

Will this code ever be used on anything other than Tegra20? I assume not since it's in a Tegra20-specific file. Given that, it seems much of the code could be made a lot simpler, e.g. by removing the "cpu" parameter here, which would avoid requiring those BUG()s. You'd probably want to rename the function e.g. tegra20_reset_cpu_1_sleeping().

Good idea. Will follow your suggestion to refine this.

Quoted 7 lines

arch/arm/mach-tegra/sleep.S

Quoted 3 lines
arch/arm/mach-tegra/sleep-tegra20.S +0 -0
arch/arm/mach-tegra/sleep.S +0 -0

Similar comments about C-vs-assembly here as before, for at least some of the functions.

OK. I will think about this. Will do it later (maybe after LP1/LP0 suspend code be ready). I need to confirm whole the scenario here first.

Quoted 9 lines

arch/arm/mach-tegra/sleep.S ENTRY(tegra_shut_off_mmu) · line 120

Quoted 6 lines
arch/arm/mach-tegra/sleep.S +6 -0
+/*
+ * tegra_cpu_pllp
+ *
+ * In LP2 the normal cpu clock pllx will be turned off. Switch the CPU to pllp
+ */
+ENTRY(tegra_cpu_pllp)

There's no verb in that function name. tegra_switch_cpu_to_pllp() might be a better name.

OK. Will fix.

Thanks,
Joseph

author
#17 lore raw

PATCH 5/5 ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

In reply to 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).

---
 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |   37 +++++++++++++++++----------------
 2 files changed, 20 insertions(+), 18 deletions(-)
arch/arm/mach-tegra/Kconfig +1 -0
index e426d1b..e07241a 100644
@@ -4,6 +4,7 @@ comment "NVIDIA Tegra options"
 
 config ARCH_TEGRA_2x_SOC
 	bool "Enable support for Tegra20 family"
+	select ARCH_NEEDS_CPU_IDLE_COUPLED
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_742230
arch/arm/mach-tegra/cpuidle-tegra20.c +19 -18
index 9371a00..02b2cf7 100644
@@ -37,9 +37,10 @@
 #include "flowctrl.h"
 
 #ifdef CONFIG_PM_SLEEP
-static int tegra20_idle_lp2(struct cpuidle_device *dev,
-			    struct cpuidle_driver *drv,
-			    int index);
+static atomic_t abort_barrier;
+static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+				    struct cpuidle_driver *drv,
+				    int index);
 #endif
 
 static struct cpuidle_driver tegra_idle_driver = {
@@ -55,11 +56,12 @@ static struct cpuidle_driver tegra_idle_driver = {
 		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
 #ifdef CONFIG_PM_SLEEP
 		[1] = {
-			.enter			= tegra20_idle_lp2,
+			.enter			= tegra20_idle_lp2_coupled,
 			.exit_latency		= 5000,
 			.target_residency	= 10000,
 			.power_usage		= 0,
-			.flags			= CPUIDLE_FLAG_TIME_VALID,
+			.flags			= CPUIDLE_FLAG_TIME_VALID |
+						  CPUIDLE_FLAG_COUPLED,
 			.name			= "powered-down",
 			.desc			= "CPU power gated",
 		},
@@ -204,28 +206,24 @@ static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 }
 #endif
 
-static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
-				      struct cpuidle_driver *drv,
-				      int index)
+static int __cpuinit tegra20_idle_lp2_coupled(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;
-	bool last_cpu;
+
+	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
 
 	local_fiq_disable();
 
-	last_cpu =  tegra_set_cpu_in_lp2(cpu);
+	tegra_set_cpu_in_lp2(cpu);
 	cpu_pm_enter();
 
-	if (cpu == 0) {
-		if (last_cpu)
-			entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv,
-								     index);
-		else
-			cpu_do_idle();
-	} else {
+	if (cpu == 0)
+		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
+	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
-	}
 
 	cpu_pm_exit();
 	tegra_clear_cpu_in_lp2(cpu);
@@ -258,6 +256,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
 
 		dev->state_count = drv->state_count;
 		ret = cpuidle_register_device(dev);
Signature · 1 line
1.7.0.4
#18 lore raw

PATCH 5/5 ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

Quoted 6 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).

I wonder if it wouldn't be simpler to squash this patch into one of the earlier patches, and just use coupled cpuidle from the very start?

Quoted 6 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +3 -2
+static int __cpuinit tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+					      struct cpuidle_driver *drv,
+					      int index)
 
-	if (cpu == 0) {
-		if (last_cpu)

arch/arm/mach-tegra/cpuidle-tegra20.c static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev, · line 228

Quoted 10 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +3 -6
-			entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv,
-								     index);
-		else
-			cpu_do_idle();
-	} else {
+	if (cpu == 0)
+		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
+	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
-	}

So I assume that coupled cpuidle only calls this function on CPU 0 once it has guaranteed that CPU n are all in this same idle state. What does CPU 0 do now, when it wants to enter LP2 but can't because CPU n aren't in LP2? Do we need to explicitly provide some kind of function to implement this waiting state, or does coupled cpuidle ensure the LP3 is entered, or implement WFI itself, or ...?

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

Quoted 5 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0
@@ -258,6 +256,9 @@ int __init tegra20_cpuidle_init(void)
 
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif

What if that changes; I assume couple cpuidle handles that by registering a notifier?

Is there any way that the kernel can boot with only CPU 0 "plugged in", but later have the other CPU hotplugged in? In other words, should that hard-code a mask (3?) that describes the HW, rather than relying on the runtime hotplug status? (think about what happens when this idle code is moved into drivers/cpuidle/ and built as a module, and hence could be initialized with only 1 CPU hotplugged in).

author
#19 lore raw

PATCH 5/5 ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

Quoted 11 lines · Stephen Warren
Quoted 6 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).

I wonder if it wouldn't be simpler to squash this patch into one of the earlier patches, and just use coupled cpuidle from the very start?

OK. I can do this. And I want to add one more patch that can cover the IPI miss handling issue in coupled cpuidle framework. I will prepare that for V2. And it means that after V2, you don't need to consider the fix that I purposed for coupled cpuidle framework. You can merge it if it pass your review.

I won't squash this in V2 for you to check what difference. If you think that is ok, I will squash this in V3.

Quoted 15 lines
Quoted 15 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +6 -7
+static int __cpuinit tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+					      struct cpuidle_driver *drv,
+					      int index)
 
-	if (cpu == 0) {
-		if (last_cpu)
-			entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv,
-								     index);
-		else
-			cpu_do_idle();
-	} else {
+	if (cpu == 0)
+		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
+	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
Quoted 9 lines

arch/arm/mach-tegra/cpuidle-tegra20.c static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev, · line 228

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

So I assume that coupled cpuidle only calls this function on CPU 0 once it has guaranteed that CPU n are all in this same idle state. What does CPU 0 do now, when it wants to enter LP2 but can't because CPU n aren't in LP2? Do we need to explicitly provide some kind of function to implement this waiting state, or does coupled cpuidle ensure the LP3 is entered, or implement WFI itself, or ...?

Indeed, this is important for CPU0. For Tegra30, we definitely need a loop to sync CPUn to power down status first. For Tegra20, the CPU0 only need to put CPU1 in reset. And we handle these procedure when CPU0 going to LP2.

Quoted 9 lines

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

Quoted 5 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0
@@ -258,6 +256,9 @@ int __init tegra20_cpuidle_init(void)
 
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif

What if that changes; I assume couple cpuidle handles that by registering a notifier?

Yes. This was used for the coupled driver initialization. It need to know how many CPUs need to be coupled when registration. It also use the notifier for updating the mask.

Quoted 6 lines

Is there any way that the kernel can boot with only CPU 0 "plugged in", but later have the other CPU hotplugged in? In other words, should that hard-code a mask (3?) that describes the HW, rather than relying on the runtime hotplug status? (think about what happens when this idle code is moved into drivers/cpuidle/ and built as a module, and hence could be initialized with only 1 CPU hotplugged in).

Hmmm. Currently it can't get these info from HW. The procedure is just like above. Syncing the cpu online mask to know how many CPUs need to be coupled and updating the mask by notifier.

Thanks,
Joseph

#20 lore raw

PATCH 5/5 ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

In reply to Stephen Warren
Quoted 10 lines

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

Quoted 6 lines
arch/arm/mach-tegra/cpuidle-tegra20.c +3 -0
@@ -258,6 +256,9 @@ int __init tegra20_cpuidle_init(void)
 
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif

What if that changes; I assume couple cpuidle handles that by registering a notifier?

Yes. see drivers/cpuidle/coupled.c:651

Quoted 6 lines

Is there any way that the kernel can boot with only CPU 0 "plugged in", but later have the other CPU hotplugged in? In other words, should that hard-code a mask (3?) that describes the HW, rather than relying on the runtime hotplug status? (think about what happens when this idle code is moved into drivers/cpuidle/ and built as a module, and hence could be initialized with only 1 CPU hotplugged in).

Using the cpu_possible_mask might be a better idea. The only other user of coupled cpuidle (OMAP4) also uses cpu_online_mask however.

Cheers,

Peter.