Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
Dear Kim, In message 1247098309-29122-2-git-send-email-pty...@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 pty...@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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Keep your head and your heart going in the right direction and you will not have to worry about your feet. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
On Jul 14, 2009, at 4:22 AM, Wolfgang Denk wrote: Dear Kim, In message 1247098309-29122-2-git-send-email-pty...@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 pty...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
On Tue, 2009-07-14 at 11:22 +0200, Wolfgang Denk wrote: Dear Kim, In message 1247098309-29122-2-git-send-email-pty...@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 pty...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
Dear Peter Tyser, In message 1247582520.30723.4.ca...@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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de If you believe that feeling bad or worrying long enough will change a past or future event, then you are residing on another planet with a different reality system. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
On Tue, 14 Jul 2009 09:42:00 -0500 Peter Tyser pty...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
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 i...@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 i...@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)
--
1.5.4.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/3 v2] 83xx: Default to using DMA to initialize SDRAM
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 pty...@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 / 0x80; i++)
dmacpy((0x80 * i), 0, 0x80);
+
+#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
