[U-Boot] [PATCH 0/3 v2] DMA ECC update

Changes since v1: - Enable instruction cache during 83xx sdram init
These changes bring the 83xx SDRAM ECC initialization in line with the 85xx/86xx boards and also fixes a minor bug in fsl_dma.c.
I don't have any 83xx boards to test on, so it would be appreciated if someone with 83xx hardware that uses ECC could give the patches a shot.
It'd be nice if SDRAM could be initialized via the DDR controller with CONFIG_ECC_INIT_VIA_DDRCONTROLLER on the 83xx platform too, but I'm not going to tackle it:)
The patches also resolve the compile error Stefan brought up with non-freescale boards with CONFIG_ECC.
Peter Tyser (3): 83xx: Default to using DMA to initialize SDRAM 83xx: Added CONFIG_MEM_INIT_VALUE for boards with ECC fsl_dma: Fix SDRAM initial value
cpu/mpc83xx/spd_sdram.c | 57 +++-------------------------------------- drivers/dma/fsl_dma.c | 18 ++++++++---- include/asm-ppc/config.h | 7 ++--- include/configs/MPC8349EMDS.h | 1 + include/configs/MPC8360EMDS.h | 1 + include/configs/MPC8360ERDK.h | 1 + include/configs/MPC837XEMDS.h | 1 + include/configs/MPC837XERDB.h | 1 + include/configs/TQM834x.h | 1 + include/configs/kmeter1.h | 1 + include/configs/sbc8349.h | 1 + 11 files changed, 27 insertions(+), 63 deletions(-)

When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
Signed-off-by: Peter Tyser ptyser@xes-inc.com --- cpu/mpc83xx/spd_sdram.c | 57 +++------------------------------------------ drivers/dma/fsl_dma.c | 16 +++++++++---- include/asm-ppc/config.h | 7 ++--- 3 files changed, 18 insertions(+), 62 deletions(-)
diff --git a/cpu/mpc83xx/spd_sdram.c b/cpu/mpc83xx/spd_sdram.c index 0f61180..8a09a7d 100644 --- a/cpu/mpc83xx/spd_sdram.c +++ b/cpu/mpc83xx/spd_sdram.c @@ -825,67 +825,18 @@ long int spd_sdram()
#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER) /* - * Use timebase counter, get_timer() is not availabe - * at this point of initialization yet. - */ -static __inline__ unsigned long get_tbms (void) -{ - unsigned long tbl; - unsigned long tbu1, tbu2; - unsigned long ms; - unsigned long long tmp; - - ulong tbclk = get_tbclk(); - - /* get the timebase ticks */ - do { - asm volatile ("mftbu %0":"=r" (tbu1):); - asm volatile ("mftb %0":"=r" (tbl):); - asm volatile ("mftbu %0":"=r" (tbu2):); - } while (tbu1 != tbu2); - - /* convert ticks to ms */ - tmp = (unsigned long long)(tbu1); - tmp = (tmp << 32); - tmp += (unsigned long long)(tbl); - ms = tmp/(tbclk/1000); - - return ms; -} - -/* * Initialize all of memory for ECC, then enable errors. */ void ddr_enable_ecc(unsigned int dram_size) { volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR; volatile ddr83xx_t *ddr= &immap->ddr; - unsigned long t_start, t_end; - register u64 *p; - register uint size; - unsigned int pattern[2]; - - icache_enable(); - t_start = get_tbms(); - pattern[0] = 0xdeadbeef; - pattern[1] = 0xdeadbeef; - -#if defined(CONFIG_DDR_ECC_INIT_VIA_DMA) - dma_meminit(pattern[0], dram_size); -#else - debug("ddr init: CPU FP write method\n"); - size = dram_size; - for (p = 0; p < (u64*)(size); p++) { - ppcDWstore((u32*)p, pattern); - } - __asm__ __volatile__ ("sync"); -#endif
- t_end = get_tbms(); - icache_disable(); + debug("\nInitializing ECC!\n"); + + dma_meminit(0xdeadbeef, dram_size);
- debug("\nREADY!!\n"); - debug("ddr init duration: %ld ms\n", t_end - t_start); + debug("\nREADY!\n");
/* Clear All ECC Errors */ if ((ddr->err_detect & ECC_ERROR_DETECT_MME) == ECC_ERROR_DETECT_MME) diff --git a/drivers/dma/fsl_dma.c b/drivers/dma/fsl_dma.c index df33e7a..90a96dd 100644 --- a/drivers/dma/fsl_dma.c +++ b/drivers/dma/fsl_dma.c @@ -140,17 +140,19 @@ int dmacpy(phys_addr_t dest, phys_addr_t src, phys_size_t count) { }
/* - * 85xx/86xx use dma to initialize SDRAM when !CONFIG_ECC_INIT_VIA_DDRCONTROLLER - * while 83xx uses dma to initialize SDRAM when CONFIG_DDR_ECC_INIT_VIA_DMA + * Use dma to initialize SDRAM when !CONFIG_ECC_INIT_VIA_DDRCONTROLLER */ -#if ((!defined CONFIG_MPC83xx && defined(CONFIG_DDR_ECC) && \ - !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) || \ - (defined(CONFIG_MPC83xx) && defined(CONFIG_DDR_ECC_INIT_VIA_DMA))) +#if (defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) void dma_meminit(uint val, uint size) { uint *p = 0; uint i = 0;
+#ifdef CONFIG_MPC83xx + /* Enabling the instruction cache greatly improves SDRAM init speed */ + icache_enable(); +#endif + for (*p = 0; p < (uint *)(8 * 1024); p++) { if (((uint)p & 0x1f) == 0) ppcDcbz((ulong)p); @@ -174,5 +176,9 @@ void dma_meminit(uint val, uint size)
for (i = 1; i < size / 0x800000; i++) dmacpy((0x800000 * i), 0, 0x800000); + +#ifdef CONFIG_MPC83xx + icache_disable(); +#endif } #endif diff --git a/include/asm-ppc/config.h b/include/asm-ppc/config.h index ca143c7..01668a9 100644 --- a/include/asm-ppc/config.h +++ b/include/asm-ppc/config.h @@ -29,10 +29,9 @@ #endif #endif
-#ifndef CONFIG_FSL_DMA -#if ((!defined CONFIG_MPC83xx && defined(CONFIG_DDR_ECC) && \ - !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) || \ - (defined(CONFIG_MPC83xx) && defined(CONFIG_DDR_ECC_INIT_VIA_DMA))) +#if (defined(CONFIG_MPC83xx) || defined(CONFIG_MPC85xx) || \ + defined(CONFIG_MPC86xx)) && !defined CONFIG_FSL_DMA +#if (defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) #define CONFIG_FSL_DMA #endif #endif

When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
IIRC, the DMA init SDRAM is slower than the 64bit stores. It is why I added these code here.
I suggest to keep the way.
Signed-off-by: Peter Tyser ptyser@xes-inc.com
cpu/mpc83xx/spd_sdram.c | 57 +++------------------------------------------ drivers/dma/fsl_dma.c | 16 +++++++++---- include/asm-ppc/config.h | 7 ++--- 3 files changed, 18 insertions(+), 62 deletions(-)
diff --git a/cpu/mpc83xx/spd_sdram.c b/cpu/mpc83xx/spd_sdram.c index 0f61180..8a09a7d 100644 --- a/cpu/mpc83xx/spd_sdram.c +++ b/cpu/mpc83xx/spd_sdram.c @@ -825,67 +825,18 @@ long int spd_sdram()
#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER) /*
- Use timebase counter, get_timer() is not availabe
- at this point of initialization yet.
- */
-static __inline__ unsigned long get_tbms (void) -{
- unsigned long tbl;
- unsigned long tbu1, tbu2;
- unsigned long ms;
- unsigned long long tmp;
- ulong tbclk = get_tbclk();
- /* get the timebase ticks */
- do {
asm volatile ("mftbu %0":"=r" (tbu1):);
asm volatile ("mftb %0":"=r" (tbl):);
asm volatile ("mftbu %0":"=r" (tbu2):);
- } while (tbu1 != tbu2);
- /* convert ticks to ms */
- tmp = (unsigned long long)(tbu1);
- tmp = (tmp << 32);
- tmp += (unsigned long long)(tbl);
- ms = tmp/(tbclk/1000);
- return ms;
-}
-/*
- Initialize all of memory for ECC, then enable errors.
*/ void ddr_enable_ecc(unsigned int dram_size) { volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR; volatile ddr83xx_t *ddr= &immap->ddr;
- unsigned long t_start, t_end;
- register u64 *p;
- register uint size;
- unsigned int pattern[2];
- icache_enable();
- t_start = get_tbms();
- pattern[0] = 0xdeadbeef;
- pattern[1] = 0xdeadbeef;
-#if defined(CONFIG_DDR_ECC_INIT_VIA_DMA)
- dma_meminit(pattern[0], dram_size);
-#else
- debug("ddr init: CPU FP write method\n");
- size = dram_size;
- for (p = 0; p < (u64*)(size); p++) {
ppcDWstore((u32*)p, pattern);
- }
- __asm__ __volatile__ ("sync");
-#endif
- t_end = get_tbms();
- icache_disable();
- debug("\nInitializing ECC!\n");
- dma_meminit(0xdeadbeef, dram_size);
- debug("\nREADY!!\n");
- debug("ddr init duration: %ld ms\n", t_end - t_start);
debug("\nREADY!\n");
/* Clear All ECC Errors */ if ((ddr->err_detect & ECC_ERROR_DETECT_MME) ==
ECC_ERROR_DETECT_MME) diff --git a/drivers/dma/fsl_dma.c b/drivers/dma/fsl_dma.c index df33e7a..90a96dd 100644 --- a/drivers/dma/fsl_dma.c +++ b/drivers/dma/fsl_dma.c @@ -140,17 +140,19 @@ int dmacpy(phys_addr_t dest, phys_addr_t src, phys_size_t count) { }
/*
- 85xx/86xx use dma to initialize SDRAM when
!CONFIG_ECC_INIT_VIA_DDRCONTROLLER
- while 83xx uses dma to initialize SDRAM when
CONFIG_DDR_ECC_INIT_VIA_DMA
- Use dma to initialize SDRAM when
!CONFIG_ECC_INIT_VIA_DDRCONTROLLER */ -#if ((!defined CONFIG_MPC83xx && defined(CONFIG_DDR_ECC) && \
- !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) || \
- (defined(CONFIG_MPC83xx) &&
defined(CONFIG_DDR_ECC_INIT_VIA_DMA))) +#if (defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) void dma_meminit(uint val, uint size) { uint *p = 0; uint i = 0;
+#ifdef CONFIG_MPC83xx
- /* Enabling the instruction cache greatly improves
SDRAM init speed */
- icache_enable();
+#endif
- for (*p = 0; p < (uint *)(8 * 1024); p++) { if (((uint)p & 0x1f) == 0) ppcDcbz((ulong)p);
@@ -174,5 +176,9 @@ void dma_meminit(uint val, uint size)
for (i = 1; i < size / 0x800000; i++) dmacpy((0x800000 * i), 0, 0x800000);
+#ifdef CONFIG_MPC83xx
- icache_disable();
+#endif } #endif diff --git a/include/asm-ppc/config.h b/include/asm-ppc/config.h index ca143c7..01668a9 100644 --- a/include/asm-ppc/config.h +++ b/include/asm-ppc/config.h @@ -29,10 +29,9 @@ #endif #endif
-#ifndef CONFIG_FSL_DMA -#if ((!defined CONFIG_MPC83xx && defined(CONFIG_DDR_ECC) && \
- !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) || \
- (defined(CONFIG_MPC83xx) &&
defined(CONFIG_DDR_ECC_INIT_VIA_DMA))) +#if (defined(CONFIG_MPC83xx) || defined(CONFIG_MPC85xx) || \
- defined(CONFIG_MPC86xx)) && !defined CONFIG_FSL_DMA
+#if (defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)) #define CONFIG_FSL_DMA #endif
#endif
1.6.2.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Thu, 2009-07-09 at 08:24 +0800, Liu Dave-R63238 wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
IIRC, the DMA init SDRAM is slower than the 64bit stores. It is why I added these code here.
I suggest to keep the way.
According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
Best, Peter

According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
I did the DMA init test with the orignal DMA code. At that time The DMA method is slower than CPU method. Strange why the result is different.
Not sure what is the new DMA code?
DMA method needs read and write bus transaction, but CPU doesn't need the read transaction. Alougth the DMA may be burst read/write.
Anyway, the test result determine all.

Liu Dave-R63238 wrote:
According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
I did the DMA init test with the orignal DMA code. At that time The DMA method is slower than CPU method. Strange why the result is different.
Instruction cache enabled/disabled is the difference - it eliminates instruction fetches from competing with the DMA engine on the SDRAM bus. See my recent message: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/63152/focus=63308
FWIIW, in our experience with various PowerPC processors, using the CPU's "dcbz" (data cache block set to zero) instruction is also very fast, but it only works if you want to initialize memory to zero. You need to enable data caching (obviously) and not having instruction cache enabled would cripple that method too.
[snip]
Best regards, gvb

Jerry Van Baren wrote:
Liu Dave-R63238 wrote:
According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
I did the DMA init test with the orignal DMA code. At that time The DMA method is slower than CPU method. Strange why the result is different.
Instruction cache enabled/disabled is the difference - it eliminates instruction fetches from competing with the DMA engine on the SDRAM bus. See my recent message: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/63152/focus=63308
Oops, that isn't going to be exactly correct since we aren't running out of RAM yet so instruction fetching isn't going to compete on the SDRAM bus directly. I stand by the general theory, however, that it is bus contention of some sort - I would speculate the arbitration of the local bus to flash vs. the DMA engine to SDRAM. Logically they should be independent, but they may not be due to internal interference (both using the same internal bus?) or simplifying assumptions in the arbitrator implementation.
[snip]
Best regards, gvb

On Wed, Jul 08, 2009 at 07:32:26PM -0500, Peter Tyser wrote:
On Thu, 2009-07-09 at 08:24 +0800, Liu Dave-R63238 wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
IIRC, the DMA init SDRAM is slower than the 64bit stores. It is why I added these code here.
I suggest to keep the way.
According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
I didn't check the SDRAM init time with code, only by a rough estimate. (Counting "one one-thousand, two one-thousand..."). I can add back the time measuring code, and be really sure which one is faster.
Both the DMA and CPU methods are definitely on the same order of magnitude. The time taken by the CFI flash driver is MUCH longer than the SDRAM initialization. I wonder, should the icache be enabled for that as well?
I'll do some more testing when I get back to the office tomorrow.
Ira

On Wed, Jul 08, 2009 at 05:58:50PM -0700, Ira W. Snyder wrote:
On Wed, Jul 08, 2009 at 07:32:26PM -0500, Peter Tyser wrote:
On Thu, 2009-07-09 at 08:24 +0800, Liu Dave-R63238 wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
IIRC, the DMA init SDRAM is slower than the 64bit stores. It is why I added these code here.
I suggest to keep the way.
According to Ira, the DMA method was faster than the cpu method: "It makes the DMA initialization normal speed again. The DMA in the for loop takes the longest (as expected).
So yes, strangely it (enabling the icache) makes a HUGE difference. The total time is <3 seconds now. It is now faster than the previous CPU method."
Logically the DMA method should be faster, and Ira's results seem to reinforce this. I don't have an 83xx board to test on, so let me know if others have different results than Ira.
I didn't check the SDRAM init time with code, only by a rough estimate. (Counting "one one-thousand, two one-thousand..."). I can add back the time measuring code, and be really sure which one is faster.
Both the DMA and CPU methods are definitely on the same order of magnitude. The time taken by the CFI flash driver is MUCH longer than the SDRAM initialization. I wonder, should the icache be enabled for that as well?
I'll do some more testing when I get back to the office tomorrow.
Ok, I've added back the get_tbms() code, and created a routine to initialize ECC with the CPU. I've inlined my patch below, just for reference.
DMA 945ms CPU 581ms
As an interesting comparison, I also benchmarked the method of using the DDR controller to initialize ECC.
DDRC 129ms
So there you have it. Dave Liu is correct, the CPU method is faster, though definitely on the same order of magnitude with the icache_enable()/icache_disable() patch.
I may stick with the DDRC method on my board, the difference is just amazing. The flash init still takes up most of the boot process, however.
Peter, feel free to fold the patch below into your own work. Ira
From 5cc24043352a90bf8ac89b5db0bc07a97e24d5d1 Mon Sep 17 00:00:00 2001
From: Ira W. Snyder iws@ovro.caltech.edu Date: Thu, 9 Jul 2009 08:45:09 -0700 Subject: [PATCH] mpc83xx: initialize ECC SDRAM with CPU
Signed-off-by: Ira W. Snyder iws@ovro.caltech.edu --- cpu/mpc83xx/spd_sdram.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/cpu/mpc83xx/spd_sdram.c b/cpu/mpc83xx/spd_sdram.c index ab6a2bb..7e093ae 100644 --- a/cpu/mpc83xx/spd_sdram.c +++ b/cpu/mpc83xx/spd_sdram.c @@ -1,3 +1,4 @@ +#define DEBUG 1 /* * (C) Copyright 2006-2007 Freescale Semiconductor, Inc. * @@ -825,18 +826,75 @@ long int spd_sdram()
#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER) /* + * Use timebase counter, get_timer() is not availabe + * at this point of initialization yet. + */ +static unsigned long get_tbms (void) +{ + unsigned long tbl; + unsigned long tbu1, tbu2; + unsigned long ms; + unsigned long long tmp; + + ulong tbclk = get_tbclk(); + + /* get the timebase ticks */ + do { + asm volatile ("mftbu %0":"=r" (tbu1):); + asm volatile ("mftb %0":"=r" (tbl):); + asm volatile ("mftbu %0":"=r" (tbu2):); + } while (tbu1 != tbu2); + + /* convert ticks to ms */ + tmp = (unsigned long long)(tbu1); + tmp = (tmp << 32); + tmp += (unsigned long long)(tbl); + ms = tmp/(tbclk/1000); + + return ms; +} + +void cpu_meminit(uint val, uint size) +{ + register u64 *p; + unsigned int pattern[2]; + + icache_enable(); + + pattern[0] = val; + pattern[1] = val; + + for (p = 0; p < (u64*)(size); p++) + ppcDWstore((u32*)p, pattern); + + asm volatile ("sync"); + + icache_disable(); +} + +/* * Initialize all of memory for ECC, then enable errors. */ void ddr_enable_ecc(unsigned int dram_size) { volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR; volatile ddr83xx_t *ddr= &immap->ddr; + unsigned long s, e;
debug("\nInitializing ECC!\n");
+ s = get_tbms(); +#if 0 + debug("\nInitializing ECC (using DMA)!\n"); dma_meminit(CONFIG_MEM_INIT_VALUE, dram_size); +#else + debug("\nInitializing ECC (using CPU)!\n"); + cpu_meminit(CONFIG_MEM_INIT_VALUE, dram_size); +#endif + e = get_tbms();
debug("\nREADY!\n"); + debug("ddr init duration: %ld ms\n", e - s);
/* Clear All ECC Errors */ if ((ddr->err_detect & ECC_ERROR_DETECT_MME) == ECC_ERROR_DETECT_MME)

Ira W. Snyder wrote:
On Wed, Jul 08, 2009 at 05:58:50PM -0700, Ira W. Snyder wrote:
On Wed, Jul 08, 2009 at 07:32:26PM -0500, Peter Tyser wrote:
On Thu, 2009-07-09 at 08:24 +0800, Liu Dave-R63238 wrote:
[snip]
Both the DMA and CPU methods are definitely on the same order of magnitude. The time taken by the CFI flash driver is MUCH longer than the SDRAM initialization. I wonder, should the icache be enabled for that as well?
I'll do some more testing when I get back to the office tomorrow.
Ok, I've added back the get_tbms() code, and created a routine to initialize ECC with the CPU. I've inlined my patch below, just for reference.
DMA 945ms CPU 581ms
As an interesting comparison, I also benchmarked the method of using the DDR controller to initialize ECC.
DDRC 129ms
So there you have it. Dave Liu is correct, the CPU method is faster, though definitely on the same order of magnitude with the icache_enable()/icache_disable() patch.
Thanks for the measurements and the update.
I may stick with the DDRC method on my board, the difference is just amazing. The flash init still takes up most of the boot process, however.
Peter, feel free to fold the patch below into your own work. Ira
[snip]
FWIIW, and it doesn't materially change your results, but it would have been better to duplicate the get_tbms() call and put it directly around the *_meminit() calls rather than having the print statements inside the timed part. Since U-Boot printing is a blocking call (generally), you are adding the UART Tx time to your measurement.
#if 0 debug("\nInitializing ECC (using DMA)!\n"); s = get_tbms(); dma_meminit(CONFIG_MEM_INIT_VALUE, dram_size); #else debug("\nInitializing ECC (using CPU)!\n"); s = get_tbms(); cpu_meminit(CONFIG_MEM_INIT_VALUE, dram_size); #endif e = get_tbms();
- s = get_tbms();
+#if 0
- debug("\nInitializing ECC (using DMA)!\n"); dma_meminit(CONFIG_MEM_INIT_VALUE, dram_size);
+#else
- debug("\nInitializing ECC (using CPU)!\n");
- cpu_meminit(CONFIG_MEM_INIT_VALUE, dram_size);
+#endif
e = get_tbms();
debug("\nREADY!\n");
debug("ddr init duration: %ld ms\n", e - s);
/* Clear All ECC Errors */ if ((ddr->err_detect & ECC_ERROR_DETECT_MME) == ECC_ERROR_DETECT_MME)
Thanks, gvb

Dear Kim,
In message 1247098309-29122-2-git-send-email-ptyser@xes-inc.com Peter Tyse wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
Signed-off-by: Peter Tyser ptyser@xes-inc.com
cpu/mpc83xx/spd_sdram.c | 57 +++------------------------------------------ drivers/dma/fsl_dma.c | 16 +++++++++---- include/asm-ppc/config.h | 7 ++--- 3 files changed, 18 insertions(+), 62 deletions(-)
It seems this patch (especially the include/asm-ppc/config.h changes) fix build problems for some other (non-FSL) boards, so I would like to get this in quickly.
Do you plan to send a pull request including these patches any time soon?
Alternatively, should I go ahead and apply just this patch (1/3) directly?
Best regards,
Wolfgang Denk

On Jul 14, 2009, at 4:22 AM, Wolfgang Denk wrote:
Dear Kim,
In message 1247098309-29122-2-git-send-email-ptyser@xes-inc.com Peter Tyse wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
Signed-off-by: Peter Tyser ptyser@xes-inc.com
cpu/mpc83xx/spd_sdram.c | 57 ++ +------------------------------------------ drivers/dma/fsl_dma.c | 16 +++++++++---- include/asm-ppc/config.h | 7 ++--- 3 files changed, 18 insertions(+), 62 deletions(-)
It seems this patch (especially the include/asm-ppc/config.h changes) fix build problems for some other (non-FSL) boards, so I would like to get this in quickly.
Do you plan to send a pull request including these patches any time soon?
Alternatively, should I go ahead and apply just this patch (1/3) directly?
Best regards,
Wolfgang Denk
I'll poke Kim on these and resolve this one way or another today.
- k

On Tue, 2009-07-14 at 11:22 +0200, Wolfgang Denk wrote:
Dear Kim,
In message 1247098309-29122-2-git-send-email-ptyser@xes-inc.com Peter Tyse wrote:
When SDRAM ECC is enabled and CONFIG_ECC_INIT_VIA_DDRCONTROLLER is not defined use DMA to set SDRAM to a known state. Previously a sequence of 64-bit stores was used.
Signed-off-by: Peter Tyser ptyser@xes-inc.com
cpu/mpc83xx/spd_sdram.c | 57 +++------------------------------------------ drivers/dma/fsl_dma.c | 16 +++++++++---- include/asm-ppc/config.h | 7 ++--- 3 files changed, 18 insertions(+), 62 deletions(-)
It seems this patch (especially the include/asm-ppc/config.h changes) fix build problems for some other (non-FSL) boards, so I would like to get this in quickly.
Do you plan to send a pull request including these patches any time soon?
Alternatively, should I go ahead and apply just this patch (1/3) directly?
There was some discussion about whether to use DMA to initialize SDRAM on the 83xx architecture (which this patch does). After some benchmarking by Ira Synder it turns out that using DMA is actually slower than using the initial method of a series of 64-bit writes. Thus I'm assuming Kim would prefer not to use DMA and thus this patch should be discarded.
Kim, let me know if this is the case and I'll resubmit a patch just to fix the build error. Patches 2 and 3 are still applicable even if we discard patch 1.
Best, Peter

Dear Peter Tyser,
In message 1247582520.30723.4.camel@localhost.localdomain you wrote:
There was some discussion about whether to use DMA to initialize SDRAM on the 83xx architecture (which this patch does). After some benchmarking by Ira Synder it turns out that using DMA is actually slower than using the initial method of a series of 64-bit writes. Thus I'm assuming Kim would prefer not to use DMA and thus this patch should be discarded.
Kim, let me know if this is the case and I'll resubmit a patch just to fix the build error. Patches 2 and 3 are still applicable even if we discard patch 1.
We need at least the include/asm-ppc/config.h changes as these fix a real build problem.
Best regards,
Wolfgang Denk

On Tue, 14 Jul 2009 09:42:00 -0500 Peter Tyser ptyser@xes-inc.com wrote:
I'm assuming Kim would prefer not to use DMA and thus this patch should be discarded.
yeah, I prefer speed :)
Kim, let me know if this is the case and I'll resubmit a patch just to fix the build error. Patches 2 and 3 are still applicable even if we discard patch 1.
ok
WD, please feel free to apply the fix, otherwise I'll have a pull request for you coming up.
Kim

On Tue, 2009-07-14 at 12:32 -0500, Kim Phillips wrote:
On Tue, 14 Jul 2009 09:42:00 -0500 Peter Tyser ptyser@xes-inc.com wrote:
I'm assuming Kim would prefer not to use DMA and thus this patch should be discarded.
yeah, I prefer speed :)
It looked like Ira tested using the DDR controller for initialization and it was ~4x as fast as the CPU method. If someone out there in 83xx land is looking to shave a few ms off their boot time (and they use ECC) it shouldn't be that hard to add support for the CONFIG_ECC_INIT_VIA_DDRCONTROLLER similar to the 85xx and 86xx architectures.
Kim, let me know if this is the case and I'll resubmit a patch just to fix the build error. Patches 2 and 3 are still applicable even if we discard patch 1.
ok
WD, please feel free to apply the fix, otherwise I'll have a pull request for you coming up.
I'll submit a patch just fixing the build error shortly.
Peter

Signed-off-by: Peter Tyser ptyser@xes-inc.com --- cpu/mpc83xx/spd_sdram.c | 2 +- include/configs/MPC8349EMDS.h | 1 + include/configs/MPC8360EMDS.h | 1 + include/configs/MPC8360ERDK.h | 1 + include/configs/MPC837XEMDS.h | 1 + include/configs/MPC837XERDB.h | 1 + include/configs/TQM834x.h | 1 + include/configs/kmeter1.h | 1 + include/configs/sbc8349.h | 1 + 9 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/cpu/mpc83xx/spd_sdram.c b/cpu/mpc83xx/spd_sdram.c index 8a09a7d..ab6a2bb 100644 --- a/cpu/mpc83xx/spd_sdram.c +++ b/cpu/mpc83xx/spd_sdram.c @@ -834,7 +834,7 @@ void ddr_enable_ecc(unsigned int dram_size)
debug("\nInitializing ECC!\n");
- dma_meminit(0xdeadbeef, dram_size); + dma_meminit(CONFIG_MEM_INIT_VALUE, dram_size);
debug("\nREADY!\n");
diff --git a/include/configs/MPC8349EMDS.h b/include/configs/MPC8349EMDS.h index ea5fbff..35e6e84 100644 --- a/include/configs/MPC8349EMDS.h +++ b/include/configs/MPC8349EMDS.h @@ -76,6 +76,7 @@ */ #define CONFIG_DDR_ECC /* support DDR ECC function */ #define CONFIG_DDR_ECC_CMD /* use DDR ECC user commands */ +#define CONFIG_MEM_INIT_VALUE 0xDeadBeef #define CONFIG_SPD_EEPROM /* use SPD EEPROM for DDR setup*/
/* diff --git a/include/configs/MPC8360EMDS.h b/include/configs/MPC8360EMDS.h index 3497ba0..27b5a58 100644 --- a/include/configs/MPC8360EMDS.h +++ b/include/configs/MPC8360EMDS.h @@ -109,6 +109,7 @@
#define CONFIG_DDR_ECC /* support DDR ECC function */ #define CONFIG_DDR_ECC_CMD /* Use DDR ECC user commands */ +#define CONFIG_MEM_INIT_VALUE 0xDeadBeef
/* * DDRCDR - DDR Control Driver Register diff --git a/include/configs/MPC8360ERDK.h b/include/configs/MPC8360ERDK.h index f584435..c2ff098 100644 --- a/include/configs/MPC8360ERDK.h +++ b/include/configs/MPC8360ERDK.h @@ -91,6 +91,7 @@
#define CONFIG_DDR_ECC /* support DDR ECC function */ #define CONFIG_DDR_ECC_CMD /* Use DDR ECC user commands */ +#define CONFIG_MEM_INIT_VALUE 0xDeadBeef
/* * DDRCDR - DDR Control Driver Register diff --git a/include/configs/MPC837XEMDS.h b/include/configs/MPC837XEMDS.h index 4befcab..aa17220 100644 --- a/include/configs/MPC837XEMDS.h +++ b/include/configs/MPC837XEMDS.h @@ -129,6 +129,7 @@
#undef CONFIG_DDR_ECC /* support DDR ECC function */ #undef CONFIG_DDR_ECC_CMD /* Use DDR ECC user commands */ +#undef CONFIG_MEM_INIT_VALUE
#define CONFIG_SPD_EEPROM /* Use SPD EEPROM for DDR setup */ #define CONFIG_NEVER_ASSERT_ODT_TO_CPU /* Never assert ODT to internal IOs */ diff --git a/include/configs/MPC837XERDB.h b/include/configs/MPC837XERDB.h index 2b7d629..6a91395 100644 --- a/include/configs/MPC837XERDB.h +++ b/include/configs/MPC837XERDB.h @@ -156,6 +156,7 @@
#undef CONFIG_DDR_ECC /* support DDR ECC function */ #undef CONFIG_DDR_ECC_CMD /* Use DDR ECC user commands */ +#undef CONFIG_MEM_INIT_VALUE
#undef CONFIG_NEVER_ASSERT_ODT_TO_CPU /* Never assert ODT to internal IOs */
diff --git a/include/configs/TQM834x.h b/include/configs/TQM834x.h index efade69..fbb2e48 100644 --- a/include/configs/TQM834x.h +++ b/include/configs/TQM834x.h @@ -68,6 +68,7 @@ #define CONFIG_SYS_DDR_SDRAM_BASE CONFIG_SYS_DDR_BASE #define DDR_CASLAT_25 /* CASLAT set to 2.5 */ #undef CONFIG_DDR_ECC /* only for ECC DDR module */ +#undef CONFIG_MEM_INIT_VALUE #undef CONFIG_SPD_EEPROM /* do not use SPD EEPROM for DDR setup */
#undef CONFIG_SYS_DRAM_TEST /* memory test, takes time */ diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h index 19da133..d9c88f2 100644 --- a/include/configs/kmeter1.h +++ b/include/configs/kmeter1.h @@ -84,6 +84,7 @@ #define CFG_83XX_DDR_USES_CS0
#undef CONFIG_DDR_ECC +#undef CONFIG_MEM_INIT_VALUE
/* * DDRCDR - DDR Control Driver Register diff --git a/include/configs/sbc8349.h b/include/configs/sbc8349.h index 84a251a..5a62c69 100644 --- a/include/configs/sbc8349.h +++ b/include/configs/sbc8349.h @@ -74,6 +74,7 @@ */ #undef CONFIG_DDR_ECC /* only for ECC DDR module */ #undef CONFIG_DDR_ECC_CMD /* use DDR ECC user commands */ +#undef CONFIG_MEM_INIT_VALUE #define CONFIG_SPD_EEPROM /* use SPD EEPROM for DDR setup*/ #define CONFIG_SYS_83XX_DDR_USES_CS0 /* WRS; Fsl board uses CS2/CS3 */

The initial SDRAM value was being hardcoded to CONFIG_MEM_INIT_VALUE instead of the value passed in 'val'.
Signed-off-by: Peter Tyser ptyser@xes-inc.com --- drivers/dma/fsl_dma.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/fsl_dma.c b/drivers/dma/fsl_dma.c index 90a96dd..9ad07fe 100644 --- a/drivers/dma/fsl_dma.c +++ b/drivers/dma/fsl_dma.c @@ -157,7 +157,7 @@ void dma_meminit(uint val, uint size) if (((uint)p & 0x1f) == 0) ppcDcbz((ulong)p);
- *p = (uint)CONFIG_MEM_INIT_VALUE; + *p = val;
if (((uint)p & 0x1c) == 0x1c) ppcDcbf((ulong)p);
participants (7)
-
Ira W. Snyder
-
Jerry Van Baren
-
Kim Phillips
-
Kumar Gala
-
Liu Dave-R63238
-
Peter Tyser
-
Wolfgang Denk