OMAP3: run the ASM sleep code from DDR

From: Jean Pihet <[email protected]>

Most of the ASM sleep code (in arch/arm/mach-omap2/sleep34xx.S) is copied to internal SRAM at boot and after wake-up from CORE OFF mode. However only a small part of the code really needs to run from internal SRAM.

This fix lets most of the ASM idle code run from the DDR
in order to minimize the SRAM usage and the overhead in the code copy.

The only pieces of code that are mandatory in SRAM are:
- the i443 erratum WA,
- the i581 erratum WA,
- the security extension code.

SRAM usage:
- original code:
. 560 bytes for omap3_sram_configure_core_dpll (used by DVFS), . 852 bytes for omap_sram_idle (used by suspend/resume in RETention), . 124 bytes for es3_sdrc_fix (used by suspend/resume in OFF mode on ES3.x), . 108 bytes for save_secure_ram_context (used on HS parts only).

With this fix the usage for suspend/resume in RETention goes down 288 bytes, so the gain in SRAM usage for suspend/resume is 564 bytes.

Also fixed the SRAM initialization sequence to avoid an unnecessary copy to SRAM at boot time and for readability.

Tested on Beagleboard (ES2.x) in idle with full RET and OFF modes.

Signed-off-by Jean Pihet <[email protected]>

Quoted 29 lines · jean.pihet at newoldbits.com

From: Jean Pihet <[email protected]>

Most of the ASM sleep code (in arch/arm/mach-omap2/sleep34xx.S) is copied to internal SRAM at boot and after wake-up from CORE OFF mode. However only a small part of the code really needs to run from internal SRAM.

This fix lets most of the ASM idle code run from the DDR
in order to minimize the SRAM usage and the overhead in the code copy.

The only pieces of code that are mandatory in SRAM are:
- the i443 erratum WA,
- the i581 erratum WA,
- the security extension code.

SRAM usage:
- original code:
? . 560 bytes for omap3_sram_configure_core_dpll (used by DVFS), ? . 852 bytes for omap_sram_idle (used by suspend/resume in RETention), ? . 124 bytes for es3_sdrc_fix (used by suspend/resume in OFF mode on ES3.x), ? . 108 bytes for save_secure_ram_context (used on HS parts only).

With this fix the usage for suspend/resume in RETention goes down 288 bytes, so the gain in SRAM usage for suspend/resume is 564 bytes.

Also fixed the SRAM initialization sequence to avoid an unnecessary copy to SRAM at boot time and for readability.

Tested on Beagleboard (ES2.x) in idle with full RET and OFF modes.

Quoted 1 line

Signed-off-by Jean Pihet <[email protected]>

Reviewed-by Kevin Hilman <[email protected]>

Also tested retention and off on 3430/n900, 3530/Overo and 3630/Zoom3

Tested-by Kevin Hilman <[email protected]>

Russell, if you're OK with it, can you add it to your suspend branch for the upcoming merge window?

Kevin

Russell, if you're OK with it, can you add it to your suspend branch for the upcoming merge window?

Quoted 2 lines · Kevin Hilman

Russell, if you're OK with it, can you add it to your suspend branch for the upcoming merge window?

Yes - though I think we can go a little bit further - this patch is on top of my code so far, and is untested. There isn't a need for the saving of these registers to be in assembly because we can read them just as easily from C code.

Comments?

 arch/arm/mach-omap2/pm.h        |    2 +-
 arch/arm/mach-omap2/pm34xx.c    |   19 +++++++++++++++++--
 arch/arm/mach-omap2/sleep34xx.S |   12 ++----------
 3 files changed, 20 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/pm.h +1 -1

index 45bcfce..4984cca 100644
@@ -92,7 +92,7 @@ extern void omap24xx_idle_loop_suspend(void);
 
 extern void omap24xx_cpu_suspend(u32 dll_ctrl, void __iomem *sdrc_dlla_ctrl,
 					void __iomem *sdrc_power);
-extern void omap34xx_cpu_suspend(u32 *addr, int save_state);
+extern void omap34xx_cpu_suspend(int save_state);
 extern int save_secure_ram_context(u32 *addr);
 extern void omap3_save_scratchpad_contents(void);
 

arch/arm/mach-omap2/pm34xx.c +1 -1

index 3e9a13e..6366352 100644
@@ -78,7 +78,7 @@ struct power_state {
 
 static LIST_HEAD(pwrst_list);
 
-static void (*_omap_sram_idle)(u32 *addr, int save_state);
+static void (*_omap_sram_idle)(int save_state);
 
 static int (*_omap_save_secure_sram)(u32 *addr);
 

arch/arm/mach-omap2/pm34xx.c +11 -1

+	*save++ = 1;
+	*save++ = val;
+
+	asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
+	*save++ = 1;
+	*save++ = val;
+}
+
 static void omap34xx_do_sram_idle(unsigned long save_state)
 {
-	_omap_sram_idle(omap3_arm_context, save_state);
+	_omap_sram_idle(save_state);
 }
 
 void omap_sram_idle(void)

@@ -412,6 +425,8 @@ void omap_sram_idle(void)
 	 * get saved. The rest is placed on the stack, and restored
 	 * from there before resuming.
 	 */
+	if (save_state)
+		omap34xx_save_context(omap3_arm_context);
 	if (save_state == 1 || save_state == 3)
 		cpu_suspend(save_state, omap34xx_do_sram_idle);
 	else

arch/arm/mach-omap2/sleep34xx.S +2 -10

index d18f52e..3335753 100644
@@ -150,8 +150,7 @@ ENTRY(omap34xx_cpu_suspend)
 	stmfd	sp!, {r4 - r11, lr}	@ save registers on stack
 
 	/*
-	 * r0 contains CPU context save/restore pointer in sdram
-	 * r1 contains information about saving context:
+	 * r0 contains information about saving context:
 	 *   0 - No context lost
 	 *   1 - Only L1 and logic lost
 	 *   2 - Only L2 lost (Even L1 is retained we clean it along with L2)

@@ -159,18 +158,11 @@ ENTRY(omap34xx_cpu_suspend)
 	 */
 
 	/* Directly jump to WFI is the context save is not required */
-	cmp	r1, #0x0
+	cmp	r0, #0x0
 	beq	omap3_do_wfi
 
 	/* Otherwise fall through to the save context code */
 save_context_wfi:
-	mov	r8, r0			@ Store SDRAM address in r8
-	mrc	p15, 0, r5, c1, c0, 1	@ Read Auxiliary Control Register
-	mov	r4, #0x1		@ Number of parameters for restore call
-	stmia	r8!, {r4-r5}		@ Push parameters for restore call
-	mrc	p15, 1, r5, c9, c0, 2	@ Read L2 AUX ctrl register
-	stmia	r8!, {r4-r5}		@ Push parameters for restore call
-
 	/*
 	 * jump out to kernel flush routine
 	 *  - reuse that code is better

Quoted 8 lines · Russell King - ARM Linux

Quoted 2 lines · Kevin Hilman

Russell, if you're OK with it, can you add it to your suspend branch for the upcoming merge window?

Yes - though I think we can go a little bit further - this patch is on top of my code so far, and is untested. There isn't a need for the saving of these registers to be in assembly because we can read them just as easily from C code.

Indeed

Quoted 1 line

Comments?

Looks good to me (although untested) care to respin on top of $SUBJECT patch?

Minor comments below...

Quoted 13 lines · Kevin Hilman

Quoted 8 lines · Russell King - ARM Linux

Quoted 2 lines · Kevin Hilman

Russell, if you're OK with it, can you add it to your suspend branch for the upcoming merge window?

Yes - though I think we can go a little bit further - this patch is on top of my code so far, and is untested. There isn't a need for the saving of these registers to be in assembly because we can read them just as easily from C code.

Indeed

Quoted 6 lines

Quoted 1 line

Comments?

Looks good to me (although untested) care to respin on top of $SUBJECT patch?

Minor comments below...

Done.

arch/arm/mach-omap2/sleep34xx.S +42 -0

+/*
+ * This function implements the erratum ID i581 WA:
+ *  SDRC state restore before accessing the SDRAM
+ *
+ * Only used at return from non-OFF mode. For OFF
+ * mode the ROM code configures the SDRC and
+ * the DPLL before calling the restore code directly
+ * from DDR.
+ */
+
+/* Make sure SDRC accesses are ok */
+wait_sdrc_ok:
+
+/* DPLL3 must be locked before accessing the SDRC. Maybe the HW ensures this */
+       ldr     r4, cm_idlest_ckgen
+wait_dpll3_lock:
+       ldr     r5, [r4]
+       tst     r5, #1
+       beq     wait_dpll3_lock
+
+       ldr     r4, cm_idlest1_core
+wait_sdrc_ready:
+       ldr     r5, [r4]
+       tst     r5, #0x2
+       bne     wait_sdrc_ready
+       /* allow DLL powerdown upon hw idle req */
+       ldr     r4, sdrc_power
+       ldr     r5, [r4]
+       bic     r5, r5, #0x40
+       str     r5, [r4]
+
+/*
+ * PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
+ * base instead.
+ * Be careful not to clobber r7 when maintaing this code.
+ */
+
+is_dll_in_lock_mode:
+       /* Is dll in lock mode? */
+       ldr     r4, sdrc_dlla_ctrl
+       ldr     r5, [r4]
+       tst     r5, #0x4

From: Jean Pihet <[email protected]>

Most of the ASM sleep code (in arch/arm/mach-omap2/sleep34xx.S) is copied to internal SRAM at boot and after wake-up from CORE OFF mode. However only a small part of the code really needs to run from internal SRAM.

This fix lets most of the ASM idle code run from the DDR
in order to minimize the SRAM usage and the overhead in the code copy.

Quoted 6 lines · jean.pihet at newoldbits.com

From: Jean Pihet <[email protected]>

Most of the ASM sleep code (in arch/arm/mach-omap2/sleep34xx.S) is copied to internal SRAM at boot and after wake-up from CORE OFF mode. However only a small part of the code really needs to run from internal SRAM.

Quoted 2 lines

This fix lets most of the ASM idle code run from the DDR
in order to minimize the SRAM usage and the overhead in the code copy.

So... what do you plan to use sram for? Because I don't think the speedup is worth the complexity...