linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state
@ 2012-12-03  3:00 Joseph Lo
  2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
@ 2012-12-03  3:00 ` Joseph Lo
  2012-12-03 18:18   ` Stephen Warren
  2012-12-03 18:31   ` Stephen Warren
  2012-12-03  3:00 ` [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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 <ccross@android.com>
Gary King <gking@nvidia.com>

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index d32e8b0..9d59a46 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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;
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 1b11707..db72ea9 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -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;
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index 72ce709..dfb2be5 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -21,6 +21,8 @@
 #include <linux/linkage.h>
 
 #include <asm/assembler.h>
+#include <asm/proc-fns.h>
+#include <asm/cp15.h>
 
 #include "sleep.h"
 #include "flowctrl.h"
@@ -78,3 +80,146 @@ ENTRY(tegra20_cpu_shutdown)
 	mov	pc, lr
 ENDPROC(tegra20_cpu_shutdown)
 #endif
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * 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)
+ * on cpu 0:
+ * SCRATCH38 = r2 = flag[0]
+ * SCRATCH39 = r3 = flag[1]
+ * on cpu1:
+ * SCRATCH39 = r2 = flag[1]
+ * SCRATCH38 = r3 = flag[0]
+ *
+ * must be called with MMU on
+ * corrupts r0-r3, r12
+ */
+ENTRY(tegra_pen_lock)
+	mov32	r3, TEGRA_PMC_VIRT
+	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
+
+	mov	r12, #1
+	str	r12, [r2]		@ flag[cpu] = 1
+	dsb
+	str	r12, [r1]		@ !turn = cpu
+1:	dsb
+	ldr	r12, [r3]
+	cmp	r12, #1			@ flag[!cpu] == 1?
+	ldreq	r12, [r1]
+	cmpeq	r12, r0			@ !turn == cpu?
+	beq	1b			@ while !turn == cpu && flag[!cpu] == 1
+
+	mov	pc, lr			@ locked
+ENDPROC(tegra_pen_lock)
+
+ENTRY(tegra_pen_unlock)
+	dsb
+	mov32	r3, TEGRA_PMC_VIRT
+	cpu_id	r0
+	cmp	r0, #0
+	addeq	r2, r3, #PMC_SCRATCH38
+	addne	r2, r3, #PMC_SCRATCH39
+	mov	r12, #0
+	str	r12, [r2]
+	mov     pc, lr
+ENDPROC(tegra_pen_unlock)
+
+/*
+ * tegra20_cpu_clear_resettable(void)
+ *
+ * Called to clear the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_clear_resettable)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_NOT_RESETTABLE
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_clear_resettable)
+
+/*
+ * tegra20_cpu_set_resettable_soon(void)
+ *
+ * Called to set the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_set_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE_SOON
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_set_resettable_soon)
+
+/*
+ * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
+ *
+ * Enters WFI on secondary CPU by exiting coherency.
+ */
+ENTRY(tegra20_sleep_cpu_secondary_finish)
+	mrc	p15, 0, r11, c1, c0, 1  @ save actlr before exiting coherency
+
+	/* Flush and disable the L1 data cache */
+	bl	tegra_disable_clean_inv_dcache
+
+	mov32	r0, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r3, #CPU_RESETTABLE
+	str	r3, [r0]
+
+	bl	cpu_do_idle
+
+	/*
+	 * cpu may be reset while in wfi, which will return through
+	 * tegra_resume to cpu_resume
+	 * or interrupt may wake wfi, which will return here
+	 * cpu state is unchanged - MMU is on, cache is on, coherency
+	 * is off, and the data cache is off
+	 *
+	 * r11 contains the original actlr
+	 */
+
+	bl	tegra_pen_lock
+
+	mov32	r3, TEGRA_PMC_VIRT
+	add	r0, r3, #PMC_SCRATCH41
+	mov	r3, #CPU_NOT_RESETTABLE
+	str	r3, [r0]
+
+	bl	tegra_pen_unlock
+
+	/* Re-enable the data cache */
+	mrc	p15, 0, r10, c1, c0, 0
+	orr	r10, r10, #CR_C
+	mcr	p15, 0, r10, c1, c0, 0
+	isb
+
+	mcr	p15, 0, r11, c1, c0, 1	@ reenable coherency
+
+	/* Invalidate the TLBs & BTAC */
+	mov	r1, #0
+	mcr	p15, 0, r1, c8, c3, 0	@ invalidate shared TLBs
+	mcr	p15, 0, r1, c7, c1, 6	@ invalidate shared BTAC
+	dsb
+	isb
+
+	/* the cpu was running with coherency disabled,
+	 * caches may be out of date */
+	bl	v7_flush_kern_cache_louis
+
+	ldmia	sp!, {r1 - r3}		@ pop phys pgd, virt SP, phys resume fn
+	mov     r0, #0			@ return success
+	mov     sp, r2			@ sp is stored in r2 by __cpu_suspend
+	ldmfd	sp!, {r4 - r11, pc}
+ENDPROC(tegra20_sleep_cpu_secondary_finish)
+#endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 9821ee7..a02f71a 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -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);
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
  2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
@ 2012-12-03  3:00 ` Joseph Lo
  2012-12-03 18:20   ` Stephen Warren
  2012-12-04  5:12   ` Prashant Gaikwad
  2012-12-03  3:00 ` [PATCH 3/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit Joseph Lo
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/tegra20_clocks.c |  102 ++++++++++++++++++++++++++++++++++
 1 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c
index 4eb6bc8..05968d7 100644
--- a/arch/arm/mach-tegra/tegra20_clocks.c
+++ b/arch/arm/mach-tegra/tegra20_clocks.c
@@ -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)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit
  2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
  2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
  2012-12-03  3:00 ` [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
@ 2012-12-03  3:00 ` Joseph Lo
  2012-12-03  3:00 ` [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0 Joseph Lo
  2012-12-03  3:00 ` [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode Joseph Lo
  4 siblings, 0 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/flowctrl.c |   38 +++++++++++++++++++++++++++++++++-----
 arch/arm/mach-tegra/flowctrl.h |    4 ++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-tegra/flowctrl.c b/arch/arm/mach-tegra/flowctrl.c
index a2250dd..9c44788 100644
--- a/arch/arm/mach-tegra/flowctrl.c
+++ b/arch/arm/mach-tegra/flowctrl.c
@@ -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 */
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 0798dec..67eab56 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -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)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0
  2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
                   ` (2 preceding siblings ...)
  2012-12-03  3:00 ` [PATCH 3/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit Joseph Lo
@ 2012-12-03  3:00 ` Joseph Lo
  2012-12-03 18:40   ` Stephen Warren
  2012-12-03  3:00 ` [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode Joseph Lo
  4 siblings, 1 reply; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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 <ccross@android.com>
Gary King <gking@nvidia.com>

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 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(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 9d59a46..9371a00 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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");
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index dfb2be5..8b8ab5f 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -60,6 +60,9 @@ ENDPROC(tegra20_hotplug_shutdown)
 ENTRY(tegra20_cpu_shutdown)
 	cmp	r0, #0
 	moveq	pc, lr			@ must not be called for CPU 0
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE
+	str	r12, [r1]
 
 	cpu_to_halt_reg r1, r0
 	ldr	r3, =TEGRA_FLOW_CTRL_VIRT
@@ -163,6 +166,21 @@ ENTRY(tegra20_cpu_set_resettable_soon)
 ENDPROC(tegra20_cpu_set_resettable_soon)
 
 /*
+ * tegra20_cpu_is_resettable_soon(void)
+ *
+ * Returns true if the "resettable soon" flag in PMC_SCRATCH41 has been
+ * set because it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_is_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	ldr	r12, [r1]
+	cmp	r12, #CPU_RESETTABLE_SOON
+	moveq	r0, #1
+	movne	r0, #0
+	mov	pc, lr
+ENDPROC(tegra20_cpu_is_resettable_soon)
+
+/*
  * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
  *
  * Enters WFI on secondary CPU by exiting coherency.
@@ -222,4 +240,39 @@ ENTRY(tegra20_sleep_cpu_secondary_finish)
 	mov     sp, r2			@ sp is stored in r2 by __cpu_suspend
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(tegra20_sleep_cpu_secondary_finish)
+
+/*
+ * tegra20_tear_down_cpu
+ *
+ * Switches the CPU cluster to PLL-P and enters sleep.
+ */
+ENTRY(tegra20_tear_down_cpu)
+	bl	tegra_cpu_pllp
+	b	tegra20_enter_sleep
+ENDPROC(tegra20_tear_down_cpu)
+
+/*
+ * tegra20_enter_sleep
+ *
+ * uses flow controller to enter sleep state
+ * executes from IRAM with SDRAM in selfrefresh when target state is LP0 or LP1
+ * executes from SDRAM with target state is LP2
+ */
+tegra20_enter_sleep:
+	mov32   r6, TEGRA_FLOW_CTRL_BASE
+
+	mov     r0, #FLOW_CTRL_WAIT_FOR_INTERRUPT
+	orr	r0, r0, #FLOW_CTRL_HALT_CPU_IRQ | FLOW_CTRL_HALT_CPU_FIQ
+	cpu_id	r1
+	cpu_to_halt_reg r1, r1
+	str	r0, [r6, r1]
+	dsb
+	ldr	r0, [r6, r1] /* memory barrier */
+
+halted:
+	dsb
+	wfe	/* CPU should be power gated here */
+	isb
+	b	halted
+
 #endif
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 26afa7c..32beb88 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -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
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index a02f71a..4d2b173 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -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);
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode
  2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
                   ` (3 preceding siblings ...)
  2012-12-03  3:00 ` [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0 Joseph Lo
@ 2012-12-03  3:00 ` Joseph Lo
  2012-12-03 18:52   ` Stephen Warren
  4 siblings, 1 reply; 21+ messages in thread
From: Joseph Lo @ 2012-12-03  3:00 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |   37 +++++++++++++++++----------------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index e426d1b..e07241a 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -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
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 9371a00..02b2cf7 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -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);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
@ 2012-12-03 18:18   ` Stephen Warren
  2012-12-04  6:47     ` Joseph Lo
  2012-12-03 18:31   ` Stephen Warren
  1 sibling, 1 reply; 21+ messages in thread
From: Stephen Warren @ 2012-12-03 18:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/02/2012 08:00 PM, Joseph Lo wrote:
> 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   |  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...

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-03  3:00 ` [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
@ 2012-12-03 18:20   ` Stephen Warren
  2012-12-04  4:28     ` Joseph Lo
  2012-12-04  5:12   ` Prashant Gaikwad
  1 sibling, 1 reply; 21+ messages in thread
From: Stephen Warren @ 2012-12-03 18:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/02/2012 08:00 PM, Joseph Lo wrote:
> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> functions were used for CPU powered-down state maintenance.

> diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
  2012-12-03 18:18   ` Stephen Warren
@ 2012-12-03 18:31   ` Stephen Warren
  2012-12-04  7:05     ` Joseph Lo
  1 sibling, 1 reply; 21+ messages in thread
From: Stephen Warren @ 2012-12-03 18:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/02/2012 08:00 PM, Joseph Lo wrote:
> 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".

> diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S

> +/*
> + * 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.

> + * 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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0
  2012-12-03  3:00 ` [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0 Joseph Lo
@ 2012-12-03 18:40   ` Stephen Warren
  2012-12-04  7:25     ` Joseph Lo
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Warren @ 2012-12-03 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/02/2012 08:00 PM, Joseph Lo wrote:
> 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".

> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c

> +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().

> +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;
> +}

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

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

> diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
> diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S

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

> +/*
> + * 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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode
  2012-12-03  3:00 ` [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode Joseph Lo
@ 2012-12-03 18:52   ` Stephen Warren
  2012-12-04 10:20     ` Joseph Lo
  2012-12-04 12:41     ` Peter De Schrijver
  0 siblings, 2 replies; 21+ messages in thread
From: Stephen Warren @ 2012-12-03 18:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/02/2012 08:00 PM, Joseph Lo wrote:
> 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?

> +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);
> -	}

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

> @@ -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).

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-03 18:20   ` Stephen Warren
@ 2012-12-04  4:28     ` Joseph Lo
  0 siblings, 0 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-04  4:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 02:20 +0800, Stephen Warren wrote:
> On 12/02/2012 08:00 PM, Joseph Lo wrote:
> > Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> > functions were used for CPU powered-down state maintenance.
> 
> > diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c
> 
> > +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);

OK. Will do this.

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-03  3:00 ` [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
  2012-12-03 18:20   ` Stephen Warren
@ 2012-12-04  5:12   ` Prashant Gaikwad
  2012-12-04  5:25     ` Joseph Lo
  1 sibling, 1 reply; 21+ messages in thread
From: Prashant Gaikwad @ 2012-12-04  5:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 03 December 2012 08:30 AM, Joseph Lo wrote:
> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> functions were used for CPU powered-down state maintenance.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
>   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?

> diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c
> index 4eb6bc8..05968d7 100644
> --- a/arch/arm/mach-tegra/tegra20_clocks.c
> +++ b/arch/arm/mach-tegra/tegra20_clocks.c
> @@ -159,6 +159,31 @@
>   #define CPU_CLOCK(cpu)	(0x1 << (8 + cpu))
>   #define CPU_RESET(cpu)	(0x1111ul << (cpu))
>   

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-04  5:12   ` Prashant Gaikwad
@ 2012-12-04  5:25     ` Joseph Lo
  2012-12-05 18:34       ` Stephen Warren
  0 siblings, 1 reply; 21+ messages in thread
From: Joseph Lo @ 2012-12-04  5:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 13:12 +0800, Prashant Gaikwad wrote:
> On Monday 03 December 2012 08:30 AM, Joseph Lo wrote:
> > Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> > functions were used for CPU powered-down state maintenance.
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
> >   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?
> 

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-03 18:18   ` Stephen Warren
@ 2012-12-04  6:47     ` Joseph Lo
  0 siblings, 0 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-04  6:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 02:18 +0800, Stephen Warren wrote:
> On 12/02/2012 08:00 PM, Joseph Lo wrote:
> > 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   |  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...

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

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-03 18:31   ` Stephen Warren
@ 2012-12-04  7:05     ` Joseph Lo
  2012-12-05 18:36       ` Stephen Warren
  0 siblings, 1 reply; 21+ messages in thread
From: Joseph Lo @ 2012-12-04  7:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 02:31 +0800, Stephen Warren wrote:
> On 12/02/2012 08:00 PM, Joseph Lo wrote:
> > 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".
> 
> > diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
> 
> > +/*
> > + * 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.
> 
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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0
  2012-12-03 18:40   ` Stephen Warren
@ 2012-12-04  7:25     ` Joseph Lo
  0 siblings, 0 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-04  7:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 02:40 +0800, Stephen Warren wrote:
> On 12/02/2012 08:00 PM, Joseph Lo wrote:
> > 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".
> 
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> 
> > +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.

> 
> > diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
> > diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
> 
> 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.

> > +/*
> > + * 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode
  2012-12-03 18:52   ` Stephen Warren
@ 2012-12-04 10:20     ` Joseph Lo
  2012-12-04 12:41     ` Peter De Schrijver
  1 sibling, 0 replies; 21+ messages in thread
From: Joseph Lo @ 2012-12-04 10:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2012-12-04 at 02:52 +0800, Stephen Warren wrote:
> On 12/02/2012 08:00 PM, Joseph Lo wrote:
> > 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.


> > +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);
> > -	}
> 
> 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.

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

> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode
  2012-12-03 18:52   ` Stephen Warren
  2012-12-04 10:20     ` Joseph Lo
@ 2012-12-04 12:41     ` Peter De Schrijver
  1 sibling, 0 replies; 21+ messages in thread
From: Peter De Schrijver @ 2012-12-04 12:41 UTC (permalink / raw)
  To: linux-arm-kernel

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

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops
  2012-12-04  5:25     ` Joseph Lo
@ 2012-12-05 18:34       ` Stephen Warren
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Warren @ 2012-12-05 18:34 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/03/2012 10:25 PM, Joseph Lo wrote:
> On Tue, 2012-12-04 at 13:12 +0800, Prashant Gaikwad wrote:
>> On Monday 03 December 2012 08:30 AM, Joseph Lo wrote:
>>> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
>>> functions were used for CPU powered-down state maintenance.
>>>
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>>   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?
>>
> 
> 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?

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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  2012-12-04  7:05     ` Joseph Lo
@ 2012-12-05 18:36       ` Stephen Warren
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Warren @ 2012-12-05 18:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/04/2012 12:05 AM, Joseph Lo wrote:
> On Tue, 2012-12-04 at 02:31 +0800, Stephen Warren wrote:
>> On 12/02/2012 08:00 PM, Joseph Lo wrote:
>>> 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".
>>
>>> diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
>>
>>> +/*
>>> + * 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.
>>
> 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").

>>> + * 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.

Doh. I overlooked the "cpu_id" invocation.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2012-12-05 18:36 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-03  3:00 [PATCH 0/5] ARM: tegra20: cpuidle: add power-down state Joseph Lo
2012-12-03  3:00 ` [PATCH 1/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU Joseph Lo
2012-12-03 18:18   ` Stephen Warren
2012-12-04  6:47     ` Joseph Lo
2012-12-03 18:31   ` Stephen Warren
2012-12-04  7:05     ` Joseph Lo
2012-12-05 18:36       ` Stephen Warren
2012-12-03  3:00 ` [PATCH 2/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
2012-12-03 18:20   ` Stephen Warren
2012-12-04  4:28     ` Joseph Lo
2012-12-04  5:12   ` Prashant Gaikwad
2012-12-04  5:25     ` Joseph Lo
2012-12-05 18:34       ` Stephen Warren
2012-12-03  3:00 ` [PATCH 3/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit Joseph Lo
2012-12-03  3:00 ` [PATCH 4/5] ARM: tegra20: cpuidle: add powered-down state for CPU0 Joseph Lo
2012-12-03 18:40   ` Stephen Warren
2012-12-04  7:25     ` Joseph Lo
2012-12-03  3:00 ` [PATCH 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode Joseph Lo
2012-12-03 18:52   ` Stephen Warren
2012-12-04 10:20     ` Joseph Lo
2012-12-04 12:41     ` Peter De Schrijver

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).