Re: [U-Boot] [linux-sunxi] Re: [RFC PATCH 1/4] sunxi: add DDR2 support to H3-like DRAM controller
2017年1月6日 06:37于 Maxime Ripard maxime.ripard@free-electrons.com写道：
On Thu, Dec 29, 2016 at 03:00:58AM +0800, Icenowy Zheng wrote:
H3-like DRAM controller needs some special code to operate a DDR2 DRAM chip. Add the logic to probe such a chip.
As there's no commercial boards available now with H3 and DDR2 DRAM, the patch is developed and tested on a V3s chip, which has in-package DDR2 DRAM.
Signed-off-by: Icenowy Zheng icenowy@aosc.xyz
It would have been great if your previous patch renaming the H3 symbol was part of that serie.
arch/arm/mach-sunxi/dram_sun8i_h3.c | 114 ++++++++++++++++++++++++++++++++++-- board/sunxi/Kconfig | 11 ++++ 2 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c index 8e2527dee1..a48320e01c 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c @@ -22,6 +22,9 @@ struct dram_para { u8 bus_width; u8 dual_rank; u8 row_bits; +#ifdef CONFIG_SUNXI_H3_DRAM_DDR2
- u8 bank_bits;
+#endif }; static inline int ns_to_t(int nanoseconds) @@ -136,36 +139,77 @@ static void mctl_set_timing_params(struct dram_para *para) struct sunxi_mctl_ctl_reg * const mctl_ctl = (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tccd = 2; +#else
- u8 tccd = 1;
+#endif u8 tfaw = ns_to_t(50); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trrd = max(ns_to_t(10), 4); u8 trcd = ns_to_t(15); u8 trc = ns_to_t(53); u8 txp = max(ns_to_t(8), 3); u8 twtr = max(ns_to_t(8), 4); u8 trtp = max(ns_to_t(8), 4); +#else
- u8 trrd = max(ns_to_t(10), 2);
- u8 trcd = ns_to_t(20);
- u8 trc = ns_to_t(65);
- u8 txp = 2;
- u8 twtr = max(ns_to_t(8), 2);
- u8 trtp = max(ns_to_t(8), 2);
+#endif u8 twr = max(ns_to_t(15), 3); u8 trp = ns_to_t(15); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tras = ns_to_t(38); +#else
- u8 tras = ns_to_t(45);
+#endif u16 trefi = ns_to_t(7800) / 32; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u16 trfc = ns_to_t(350); +#else
- u16 trfc = ns_to_t(328);
+#endif u8 tmrw = 0; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tmrd = 4; +#else
- u8 tmrd = 2;
+#endif u8 tmod = 12; u8 tcke = 3; u8 tcksrx = 5; u8 tcksre = 5; u8 tckesr = 4; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trasmax = 24; +#else
- u8 trasmax = 27;
+#endif
Can't that be moved into a structure that would have different declaration, this is barely readable.
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tcl = 6; /* CL 12 */ u8 tcwl = 4; /* CWL 8 */ u8 t_rdata_en = 4; u8 wr_latency = 2;
+#else
- u8 tcl = 3; /* CL 12 */
- u8 tcwl = 3; /* CWL 8 */
Aren't the comments supposed to change?
- u8 t_rdata_en = 1;
- u8 wr_latency = 1;
+#endif
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u32 tdinit0 = (500 * CONFIG_DRAM_CLK) + 1; /* 500us */ u32 tdinit1 = (360 * CONFIG_DRAM_CLK) / 1000 + 1; /* 360ns */ +#else
- u32 tdinit0 = (400 * CONFIG_DRAM_CLK) + 1; /* 400us */
- u32 tdinit1 = (500 * CONFIG_DRAM_CLK) / 1000 + 1; /* 500ns */
+#endif u32 tdinit2 = (200 * CONFIG_DRAM_CLK) + 1; /* 200us */ u32 tdinit3 = (1 * CONFIG_DRAM_CLK) + 1; /* 1us */ @@ -174,9 +218,15 @@ static void mctl_set_timing_params(struct dram_para *para) u8 trd2wr = tcl + 2 + 1 - tcwl; /* RL + BL / 2 + 2 - WL */ /* set mode register */ +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 writel(0x1c70, &mctl_ctl->mr[0]); /* CL=11, WR=12 */ writel(0x40, &mctl_ctl->mr[1]); writel(0x18, &mctl_ctl->mr[2]); /* CWL=8 */ +#else
- writel(0x263, &mctl_ctl->mr[0]); /* CL=11, WR=12 */
- writel(0x4, &mctl_ctl->mr[1]);
- writel(0x0, &mctl_ctl->mr[2]); /* CWL=8 */
Ditto
+#endif writel(0x0, &mctl_ctl->mr[3]); /* set DRAM timing */ @@ -244,7 +294,12 @@ static void mctl_zq_calibration(struct dram_para *para) writel(0x0a0a0a0a, &mctl_ctl->zqdr[2]);
- for (i = 0; i < 6; i++) {
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2
- for (i = 0; i < 6; i++)
+#else
- for (i = 0; i < 4; i++)
+#endif
- {
This should also be put into that structure or a define.
u8 zq = (CONFIG_DRAM_ZQ >> (i * 4)) & 0xf; writel((zq << 20) | (zq << 16) | (zq << 12) | @@ -266,7 +321,9 @@ static void mctl_zq_calibration(struct dram_para *para) writel((zq_val[1] << 16) | zq_val[0], &mctl_ctl->zqdr[0]); writel((zq_val[3] << 16) | zq_val[2], &mctl_ctl->zqdr[1]); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 writel((zq_val[5] << 16) | zq_val[4], &mctl_ctl->zqdr[2]); +#endif } } @@ -275,8 +332,16 @@ static void mctl_set_cr(struct dram_para *para) struct sunxi_mctl_com_reg * const mctl_com = (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
- writel(MCTL_CR_BL8 | MCTL_CR_2T | MCTL_CR_DDR3 | MCTL_CR_INTERLEAVED |
- MCTL_CR_EIGHT_BANKS | MCTL_CR_BUS_WIDTH(para->bus_width) |
- writel(MCTL_CR_BL8 | MCTL_CR_2T |
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2
- MCTL_CR_DDR3 | MCTL_CR_EIGHT_BANKS |
- MCTL_CR_BUS_WIDTH(para->bus_width) |
+#else
- (para->bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : MCTL_CR_FOUR_BANKS) |
- MCTL_CR_DDR2 |
You can use a variable and then or is based on whether that option is enabled or not, this will be easier to read.
- MCTL_CR_32BIT /* fixme, thats wrong but what boot0 does */ |
What's wrong about it?
V3s DRAM seems to be 16-bit.
However, boot0 has this bit set, and without this bit, it cannot work.
According to Jens' guess (only guess), this may be something more like full-width and half-width.
+#endif
- MCTL_CR_INTERLEAVED |
(para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | MCTL_CR_PAGE_SIZE(para->page_size) | MCTL_CR_ROW_BITS(para->row_bits), &mctl_com->cr); @@ -380,7 +445,10 @@ static int mctl_channel_init(struct dram_para *para) mctl_zq_calibration(para);
- mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST | PIR_DRAMRST |
- mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST |
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2
- PIR_DRAMRST |
+#endif PIR_DRAMINIT | PIR_QSGATE); /* detect ranks and bus width */ @@ -435,12 +503,29 @@ static void mctl_auto_detect_dram_size(struct dram_para *para) /* detect row address bits */ para->page_size = 512; para->row_bits = 16; +#ifdef CONFIG_SUNXI_H3_DRAM_DDR2
- para->bank_bits = 2;
+#endif mctl_set_cr(para); for (para->row_bits = 11; para->row_bits < 16; para->row_bits++) +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 if (mctl_mem_matches((1 << (para->row_bits + 3)) * para->page_size)) +#else
- if (mctl_mem_matches((1 << (para->row_bits + para->bank_bits)) * para->page_size))
+#endif
Can't you use bank_bits all the time?
break; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2
- /* detect bank address bits */
- para->bank_bits = 3;
- mctl_set_cr(para);
- for (para->bank_bits = 2; para->bank_bits < 3; para->bank_bits++)
- if (mctl_mem_matches((1 << para->bank_bits) * para->page_size))
- break;
+#endif
/* detect page size */ para->page_size = 8192; mctl_set_cr(para); @@ -458,11 +543,19 @@ unsigned long sunxi_dram_init(void) (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE; struct dram_para para = { +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 .read_delays = 0x00007979, /* dram_tpr12 */ .write_delays = 0x6aaa0000, /* dram_tpr11 */ +#else
- .read_delays = 0x00007878, /* dram_tpr12 */
- .write_delays = 0x6a440000, /* dram_tpr11 */
+#endif .dual_rank = 0, .bus_width = 32, .row_bits = 15, +#ifdef CONFIG_SUNXI_H3_DRAM_DDR2
- .bank_bits = 3,
+#endif .page_size = 4096, }; @@ -477,7 +570,13 @@ unsigned long sunxi_dram_init(void) udelay(1); /* odt delay */ +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 writel(0x0c000400, &mctl_ctl->odtcfg); +#else
- writel(0x04000400, &mctl_ctl->odtcfg);
- clrbits_le32(&mctl_ctl->pgcr[2], (1 << 13));
+#endif
Some defines would be great.
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
On Fri, Jan 06, 2017 at 06:55:05AM +0800, Icenowy Zheng wrote:
- MCTL_CR_32BIT /* fixme, thats wrong but what boot0 does */ |
What's wrong about it?
V3s DRAM seems to be 16-bit.
However, boot0 has this bit set, and without this bit, it cannot work.
According to Jens' guess (only guess), this may be something more like full-width and half-width.
Ok. Please put that in the comments then.
Thanks! Maxime
09.01.2017, 18:01, "Maxime Ripard" maxime.ripard@free-electrons.com:
On Fri, Jan 06, 2017 at 06:55:05AM +0800, Icenowy Zheng wrote:
> > + MCTL_CR_32BIT /* fixme, thats wrong but what boot0 does */ | > > What's wrong about it?
V3s DRAM seems to be 16-bit.
However, boot0 has this bit set, and without this bit, it cannot work.
According to Jens' guess (only guess), this may be something more like full-width and half-width.
Ok. Please put that in the comments then.
Now I think it's not only guess.
See dram_sun8i_a33.h, which has MCTL_CR_BUSW8 = 0 << 12 and MCTL_CR_BUSW16 = 1 << 12.
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Hi,
On 05/01/17 22:55, Icenowy Zheng wrote:
2017年1月6日 06:37于 Maxime Ripard maxime.ripard@free-electrons.com写道：
On Thu, Dec 29, 2016 at 03:00:58AM +0800, Icenowy Zheng wrote:
H3-like DRAM controller needs some special code to operate a DDR2 DRAM chip. Add the logic to probe such a chip.
Out of curiosity, how did you came up with those values? It would be good if this was mentioned in the commit log.
As there's no commercial boards available now with H3 and DDR2 DRAM, the patch is developed and tested on a V3s chip, which has in-package DDR2 DRAM.
Signed-off-by: Icenowy Zheng icenowy@aosc.xyz
It would have been great if your previous patch renaming the H3 symbol was part of that serie.
arch/arm/mach-sunxi/dram_sun8i_h3.c | 114 ++++++++++++++++++++++++++++++++++-- board/sunxi/Kconfig | 11 ++++ 2 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c index 8e2527dee1..a48320e01c 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c @@ -22,6 +22,9 @@ struct dram_para { u8 bus_width; u8 dual_rank; u8 row_bits; +#ifdef CONFIG_SUNXI_H3_DRAM_DDR2
- u8 bank_bits;
+#endif };
static inline int ns_to_t(int nanoseconds) @@ -136,36 +139,77 @@ static void mctl_set_timing_params(struct dram_para *para) struct sunxi_mctl_ctl_reg * const mctl_ctl = (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tccd = 2; +#else
- u8 tccd = 1;
+#endif u8 tfaw = ns_to_t(50); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trrd = max(ns_to_t(10), 4); u8 trcd = ns_to_t(15); u8 trc = ns_to_t(53); u8 txp = max(ns_to_t(8), 3); u8 twtr = max(ns_to_t(8), 4); u8 trtp = max(ns_to_t(8), 4); +#else
- u8 trrd = max(ns_to_t(10), 2);
- u8 trcd = ns_to_t(20);
- u8 trc = ns_to_t(65);
- u8 txp = 2;
- u8 twtr = max(ns_to_t(8), 2);
- u8 trtp = max(ns_to_t(8), 2);
+#endif u8 twr = max(ns_to_t(15), 3); u8 trp = ns_to_t(15); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tras = ns_to_t(38); +#else
- u8 tras = ns_to_t(45);
+#endif u16 trefi = ns_to_t(7800) / 32; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u16 trfc = ns_to_t(350); +#else
- u16 trfc = ns_to_t(328);
+#endif
u8 tmrw = 0; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tmrd = 4; +#else
- u8 tmrd = 2;
+#endif u8 tmod = 12; u8 tcke = 3; u8 tcksrx = 5; u8 tcksre = 5; u8 tckesr = 4; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trasmax = 24; +#else
- u8 trasmax = 27;
+#endif
Can't that be moved into a structure that would have different declaration, this is barely readable.
I agree to that. If I got this correctly, the existing parameters here are actually DRAM chip specific. It just happens that we use DDR3-1333 parameters for all boards so far. Most A64 boards for instance sport DDR3-1600 capable chips, also we will need adjustments for the LPDDR3 chips used on the SoPine and Pinebook. So it would be good to address this properly.
Philipp has a quite elaborate rework to fix this and ease the way to allow board specific tunings of those parameters:
https://github.com/ptomsich/u-boot/commit/4ae474c756c3208a3bfaf36ed6f1850d46...
as part of that branch: https://github.com/ptomsich/u-boot/commits/a64uQ7-spl-wip
At some point in the future I wanted to clean this up and upstream it, I am wondering if you can take a look at this now?
Cheers, Andre.
participants (3)
-
Andre Przywara -
Icenowy Zheng -
Maxime Ripard