On 10/8/2012 11:46 AM, Eric Nelson wrote:
Hi Troy,
This seems to be the patch where the rubber meets the road in much of this series.
On 10/03/2012 06:47 PM, Troy Kisky wrote:
Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com
board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg | 120 +++++++++++++++++++------- 1 file changed, 87 insertions(+), 33 deletions(-)
diff --git a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg index 9e20db0..f45f93e 100644 --- a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg +++ b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg @@ -49,6 +49,15 @@ BOOT_FROM sd
Address absolute address of the registervalue value to be stored in the register*/ +/*
- DDR3 settings
- MX6Q ddr is limited to 1066 Mhz, currently 1056 MHz(528 MHz
clock),
memory bus width: 64 bits, x16/x32/x64
- MX6DL ddr is limited to 800 MHz(400 MHz clock)
memory bus width: 64 bits, x16/x32/x64
- MX6SOLO ddr is limited to 800 MHz(400 MHz clock)
memory bus width: 32 bits, x16/x32- */
This comment seems to be critical to understanding some of what's below, since now three **different** memory configurations are represented in this file.
At a minimum, the file name should be changed to get rid of '6q' and '4x' since those don't necessarily apply.
Right, my preference is to put at back in board/freescale/mx6qsabrelite/imximage.cfg since MPDGCTRL0/1 DQS GATE registers and MMDC_MPRDDQBYnDL should be very board specific, but that may be meet by objections from Fabio.
WRITE_ENTRY1(IOM_DRAM_SDQS0, 0x00000030) WRITE_ENTRY1(IOM_DRAM_SDQS1, 0x00000030) WRITE_ENTRY1(IOM_DRAM_SDQS2, 0x00000030) @@ -90,6 +99,7 @@ WRITE_ENTRY1(IOM_GRP_B6DS, 0x00000030) WRITE_ENTRY1(IOM_GRP_B7DS, 0x00000030)
WRITE_ENTRY1(IOM_GRP_ADDDS, 0x00000030)
- /* (differential input) */ WRITE_ENTRY1(IOM_DDRMODE_CTL, 0x00020000) /* disable ddr pullups */
@@ -119,48 +129,92 @@ WRITE_ENTRY1(MMDC_P0_MDMISC, 0x00081740)
- MDSCR, con_req
*/ WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00008000)
- /*
- MDCFG0, tRFC=0x56 clocks, tXS=0x5b clocks
- tXP=4 clocks, tXPDLL=13 clocks
- tFAW=24 clocks, cas=8 cycles
- MDCFG0,
- MX6Q:
- tRFC=0x56 clocks, tXS=0x5b clocks, tXP=4 clocks, tXPDLL=13 clocks
- tFAW=24 clocks, cas=8 cycles
- MX6DL/SOLO:
- tRFC=0x6a clocks, tXS=0x6e clocks, tXP=3 clocks, tXPDLL=10 clocks
*/
tFAW=19 clocks, cas=6 cycles-WRITE_ENTRY1(MMDC_P0_MDCFG0, 0x555A7975) +WRITE_ENTRY2(MMDC_P0_MDCFG0, 0x555A7975, 0x696D5323)
Here's where I start to get lost in the macro-fu.
The WRITE_ENTRY1 macros make some sense to me in that they always write a single value to an offset whose address changes based on the processor type.
In order to understand WRITE_ENTRY2, you really have to know that the dual, solo, and sololite share many characteristics.
RIght, ENTRY2 is used when there are differences between mx6q, and mx6dl/solo. In the case above, the value 0x555A7975 is written if a mx6quad, and the value 0x696D5323 is written if a mx6duallite or mx6solo.
It's kinda hard to infer that knowledge from the code and I wonder if this structure will hold up under future revisions.
/*
- MDCFG1, tRDC=8, tRP=8, tRC=27,tRAS=20,tRPA=tRP+1,tWR=8
- tMRD=4, tCWL=6
- MDCFG1,
- MX6Q:
- tRDC=8, tRP=8, tRC=27, tRAS=20, tRPA=tRP+1, tWR=8, tMRD=4, tCWL=6
- MX6DL/SOLO:
*/
- tRDC=6, tRP=6, tRC=20, tRAS=15, tRPA=tRP+1, tWR=7, tMRD=4, tCWL=5
-WRITE_ENTRY1(MMDC_P0_MDCFG1, 0xFF538E64) +WRITE_ENTRY2(MMDC_P0_MDCFG1, 0xFF538E64, 0xB66E8C63)
- /*
*/ WRITE_ENTRY1(MMDC_P0_MDCFG2, 0x01FF00DB) WRITE_ENTRY1(MMDC_P0_MDRWD, 0x000026D2)
- MDCFG2,tDLLK=512,tRTP=4,tWTR=4,tRRD=4
- WRITE_ENTRY1(MMDC_P0_MDOR, 0x005B0E21)
-WRITE_ENTRY1(MMDC_P0_MDOTC, 0x09444040) -WRITE_ENTRY1(MMDC_P0_MDPDC, 0x00025576)
/*
- Mx6Q - 64 bit wide ddr
- MMDC_MDOTC,
- MX6Q:
- tAOFPD=2 cycles, tAONPD=2, tANPD=5, tAXPD=5, tODTLon=5,
tODT_idle_off=5
- MX6DL/SOLO:
- tAOFPD=1 cycles, tAONPD=1, tANPD=4, tAXPD=4, tODTLon=4,
tODT_idle_off=4
- */
+WRITE_ENTRY2(MMDC_P0_MDOTC, 0x09444040, 0x00333030)
+/*
- MDPDC - [17:16](2) => CKE pulse width = 3 cycles.
- [15:12](5) => PWDT_1 = 256 cycles
- [11:8](5) =>PWDR_0 = 256 cycles
- MX6Q: [2:0](6) => CKSRE = 6 cycles, [5:3](6) => CKSRX = 6
cycles
- MX6DL/SOLO: [2:0](5) => CKSRE = 5 cycles, [5:3](5) => CKSRX = 5
cycles
- */
+WRITE_ENTRY2(MMDC_P0_MDPDC, 0x00025576, 0x0002556D)
+/*
*/
- MX6Q/DL - 64 bit wide ddr
- last address is (1<<28 (base) + 1<<30 - 1) / (1<<25) =
1<<3 + 1<<5 - 1 = 8 + 0x20 -1 = 0x27+/*
- MX6SOLO - 32 bit wide ddr
- last address is (1<<28 (base) + 1<<29 - 1) / (1<<25) =
- 1<<3 + 1<<4 - 1 = 8 + 0x10 -1 = 0x17
- */ /* MDASP, CS0_END */
-WRITE_ENTRY1(MMDC_P0_MDASP, 0x00000027) +WRITE_ENTRY3(MMDC_P0_MDASP, 0x00000027, 0x00000027, 0x00000017)
Is it unreasonable to think there's a use case for 32-bit wide memory on a dual or quad?
Sure, they just would not use this file.
What happens when we populate 256Mx16 DDR chips?
Sabre Auto is already doing this.
Again, definitely a new file would be needed.
/*
- MDCTL, CS0 enable, CS1 disabled, row=14, col=10, burst=8,
width=64/32bit
- mx6q : row+col+bank+width=14+10+3+3=30 = 1G
- MDCTL, CS0 enable, CS1 disabled, row=14, col=10, burst=8
- MX6Q/DL: width=64bit row+col+bank+width=14+10+3+3=30 = 1G
*/
- MX6SOLO: width=32bit row+col+bank+width=14+10+3+2=29 = 512M
-WRITE_ENTRY1(MMDC_P0_MDCTL, 0x831A0000) +WRITE_ENTRY3(MMDC_P0_MDCTL, 0x831A0000, 0x831A0000, 0x83190000)
-/* MDSCR, con_req, LOAD MR2, CS0, A3,A10 set (CAS Write=6), RZQ/2 */ -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x04088032) +/*
- LOAD MR2: MDSCR, con_req, CS0, A10 set - RZQ/2
- MX6Q: A3 set(CAS Write=6)
- MX6DL/SOLO: (CAS Write=5)
- */
+WRITE_ENTRY2(MMDC_P0_MDSCR, 0x04088032, 0x04008032) /* LOAD MR3, CS0 */ WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00008033) -/* LOAD MR1, CS0, A1,A6 set Rtt=RZQ/2, ODI=RZQ/7 */ -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00428031) -/* LOAD MR0, CS0, A6,A8,A11 set CAS=8, WR=8, DLL reset */ -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x09408030)
+/*
- LOAD MR1, CS0
- MX6Q: A6 set: Rtt=RZQ/2, A1 set: ODI=RZQ/7
- MX6DL/SOLO: A2 set: Rtt=RZQ/4, ODI=RZQ/6
- */
+WRITE_ENTRY2(MMDC_P0_MDSCR, 0x00428031, 0x00048031)
+/* LOAD MR0, CS0 A8 set: DLL Reset
- MX6Q: A6 set: CAS=8 A11 set: WR=8
- MX6DL/SOLO: A4 set: CAS=5, A9,A10 set: WR=7
- */
+WRITE_ENTRY2(MMDC_P0_MDSCR, 0x09408030, 0x07208030)
/* ZQ calibrate, CS0 */ WRITE_ENTRY1(MMDC_P0_MDSCR, 0x04008040) @@ -173,18 +227,18 @@ WRITE_ENTRY1(MMDC_P0_MPODTCTRL, 0x00022227) WRITE_ENTRY1(MMDC_P1_MPODTCTRL, 0x00022227)
/* MPDGCTRL0/1 DQS GATE*/ -WRITE_ENTRY1(MMDC_P0_MPDGCTRL0, 0x434B0350) -WRITE_ENTRY1(MMDC_P0_MPDGCTRL1, 0x034C0359) -WRITE_ENTRY1(MMDC_P1_MPDGCTRL0, 0x434B0350) -WRITE_ENTRY1(MMDC_P1_MPDGCTRL1, 0x03650348) -WRITE_ENTRY1(MMDC_P0_MPRDDLCTL, 0x4436383B) -WRITE_ENTRY1(MMDC_P1_MPRDDLCTL, 0x39393341) -WRITE_ENTRY1(MMDC_P0_MPWRDLCTL, 0x35373933) -WRITE_ENTRY1(MMDC_P1_MPWRDLCTL, 0x48254A36) -WRITE_ENTRY1(MMDC_P0_MPWLDECTRL0, 0x001F001F) -WRITE_ENTRY1(MMDC_P0_MPWLDECTRL1, 0x001F001F) -WRITE_ENTRY1(MMDC_P1_MPWLDECTRL0, 0x00440044) -WRITE_ENTRY1(MMDC_P1_MPWLDECTRL1, 0x00440044) +WRITE_ENTRY2(MMDC_P0_MPDGCTRL0, 0x434B0350, 0x42350231) +WRITE_ENTRY2(MMDC_P0_MPDGCTRL1, 0x034C0359, 0x021A0218) +WRITE_ENTRY2(MMDC_P1_MPDGCTRL0, 0x434B0350, 0x42350231) +WRITE_ENTRY2(MMDC_P1_MPDGCTRL1, 0x03650348, 0x021A0218) +WRITE_ENTRY2(MMDC_P0_MPRDDLCTL, 0x4436383B, 0x4B4B4E49) +WRITE_ENTRY2(MMDC_P1_MPRDDLCTL, 0x39393341, 0x4B4B4E49) +WRITE_ENTRY2(MMDC_P0_MPWRDLCTL, 0x35373933, 0x3F3F3035) +WRITE_ENTRY2(MMDC_P1_MPWRDLCTL, 0x48254A36, 0x3F3F3035) +WRITE_ENTRY2(MMDC_P0_MPWLDECTRL0, 0x001F001F, 0x0040003C) +WRITE_ENTRY2(MMDC_P0_MPWLDECTRL1, 0x001F001F, 0x0032003E) +WRITE_ENTRY2(MMDC_P1_MPWLDECTRL0, 0x00440044, 0x0040003C) +WRITE_ENTRY2(MMDC_P1_MPWLDECTRL1, 0x00440044, 0x0032003E)
/* MPMUR0 - Complete calibration by forced measurement */ WRITE_ENTRY1(MMDC_P0_MPMUR0, 0x00000800)
It appears that only the memory configuration registers use WRITE_ENTRY2 or WRITE_ENTRY3 macros, and if so, I'd much rather see three separate files expressing the values, i.e.:
mx6q_4x_mt41j128.cfg mx6dl_4x_mt41j128.cfg mx6s_2x_mt41j128.cfg
That is definitely a valid option. But you lose the common settings of the iomux registers. Cut-n-pasting the files isn't so bad I guess, but updates such as mx6q_4x_mt41j128.cfg: use ddr3 mode for reset
Bits 19-18 of IOMUXC_IOMUXC_SW_PAD_CTL_PAD_DRAM_RESET should be 3 for DDR3 mode. The current value of 0 is reserved in TRM.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com
diff --git a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg index b859e2f..9c622c8 100644 --- a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg +++ b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg @@ -72,7 +72,7 @@ WRITE_ENTRY1(IOM_DRAM_RAS, 0x00020030) WRITE_ENTRY1(IOM_DRAM_SDCLK_0, 0x00020030) WRITE_ENTRY1(IOM_DRAM_SDCLK_1, 0x00020030)
-WRITE_ENTRY1(IOM_DRAM_RESET, 0x00020030) +WRITE_ENTRY1(IOM_DRAM_RESET, 0x000e0030) WRITE_ENTRY1(IOM_DRAM_SDCKE0, 0x00003000) WRITE_ENTRY1(IOM_DRAM_SDCKE1, 0x00003000) WRITE_ENTRY1(IOM_DRAM_SDBA2, 0x00000000)
Would then need to be applied to all three files.
I'm not certain the processor part of the name is the right designator though. If I understand correctly, the differences in values between 6q and 6dl are mostly based on the memory speed.
Is that right?
Correct...
It also seems valid that a board would want to use an i.mx6quad with a lower memory bus speed.
Especially if there is a layout problem.
If the address differences are taken care of by the preprocessor, it seems that a set like this would be more appropriate (and processor independent):
mx6_4x_mt41j128_1066.cfg mx6_4x_mt41j128_800.cfg mx6_2x_mt41j128_800.cfg
Right, this is definitely a valid option. And if different boards calibrated their DDR to the same settings (DQS and byte delays) it would make more sense.
But I still see this file as board specific and not so much memory type specific.
By splitting up the files, we can still check for differences between them using 'diff', and it will be easier to extend the set.
Note that meaningful diffs will require the use of symbolic names rather than varying addresses for registers.
It's also conceivable that calibration on a given layout will require per-board tweaks, but that's not clear at the moment.
You've clearly been through this in more detail than I have. Please let me know your thoughts.
Regards,
Eric