On Sun, 06 Apr 2014 11:49:22 +0200 Jens Kuske <jensku...@gmail.com> wrote:
> > On 06/04/14 04:19, Siarhei Siamashka wrote: > > On Wed, 26 Mar 2014 17:40:50 +0100 > > Jens Kuske <jensku...@gmail.com> wrote: > > > >> This patch unifies sun4i and sun[5,7]i autorefresh setup functions > >> and adds proper tRFC calculation. > >> > >> tRFC (REF command to ACT time) depends on DDR type and chip density. > >> > >> On sun4i there were two steps, 127.5ns for <=1Gb density and 327.5ns > >> for the rest. This fits DDR2 specification for 1Gb and 4Gb chips > >> and shouldn't be a problems with DDR3 chips <8Gb either, but it will > >> waste some performance. > >> > >> On sun[5,7]i however, tRFC was hardcoded to 131 clock cycles, which > >> seems to come from 4Gb DDR2 chips at 400MHz (327.5ns * 400MHz = 131). > >> For 4Gb DDR3 chips, like those on cubieboard2 and cubietruck, this > >> means the memory frequency was limited to ~435MHz if one doesn't want > >> to fall below the minimum specified tRFC for these chips. > >> > >> Signed-off-by: Jens Kuske <jensku...@gmail.com> > >> --- > >> arch/arm/cpu/armv7/sunxi/dram.c | 70 > >> ++++++++++++---------------------- > >> arch/arm/include/asm/arch-sunxi/dram.h | 4 ++ > >> 2 files changed, 28 insertions(+), 46 deletions(-) > >> > >> diff --git a/arch/arm/cpu/armv7/sunxi/dram.c > >> b/arch/arm/cpu/armv7/sunxi/dram.c > >> index 957db59..921f683 100644 > >> --- a/arch/arm/cpu/armv7/sunxi/dram.c > >> +++ b/arch/arm/cpu/armv7/sunxi/dram.c > >> @@ -412,53 +412,30 @@ static void dramc_clock_output_en(u32 on) > >> #endif > >> } > >> > >> -#ifdef CONFIG_SUN4I > >> -static void dramc_set_autorefresh_cycle(u32 clk) > >> -{ > >> - struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; > >> - u32 reg_val; > >> - u32 tmp_val; > >> - u32 reg_dcr; > >> - > >> - if (clk < 600) { > >> - reg_dcr = readl(&dram->dcr); > >> - if ((reg_dcr & DRAM_DCR_CHIP_DENSITY_MASK) <= > >> - DRAM_DCR_CHIP_DENSITY(DRAM_DCR_CHIP_DENSITY_1024M)) > >> - reg_val = (131 * clk) >> 10; > >> - else > >> - reg_val = (336 * clk) >> 10; > >> - > >> - tmp_val = (7987 * clk) >> 10; > >> - tmp_val = tmp_val * 9 - 200; > >> - reg_val |= tmp_val << 8; > >> - reg_val |= 0x8 << 24; > >> - writel(reg_val, &dram->drr); > >> - } else { > >> - writel(0x0, &dram->drr); > >> - } > >> -} > >> -#endif /* SUN4I */ > >> +static const u16 tRFC_table[2][6] = { > >> + /* 256Mb 512Mb 1Gb 2Gb 4Gb 8Gb */ > >> + /* DDR2 75ns 105ns 127.5ns 195ns 327.5ns invalid */ > >> + { 77, 108, 131, 200, 336, 336 }, > >> + /* DDR3 invalid 90ns 110ns 160ns 300ns 350ns */ > >> + { 93, 93, 113, 164, 308, 359 } > >> +}; > >> > >> -#if defined(CONFIG_SUN5I) || defined(CONFIG_SUN7I) > >> -static void dramc_set_autorefresh_cycle(u32 clk) > >> +static void dramc_set_autorefresh_cycle(u32 clk, u32 type, u32 density) > >> { > >> struct sunxi_dram_reg *dram = (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; > >> - u32 reg_val; > >> - u32 tmp_val; > >> - reg_val = 0x83; > >> - > >> - tmp_val = (7987 * clk) >> 10; > >> - tmp_val = tmp_val * 9 - 200; > >> - reg_val |= tmp_val << 8; > >> - reg_val |= 0x8 << 24; > >> - writel(reg_val, &dram->drr); > >> + u32 tRFC, tREFI; > >> + > >> + tRFC = (tRFC_table[type][density] * clk + 1023) >> 10; > >> + tREFI = (7987 * clk) >> 10; /* <= 7.8us */ > > > > Maybe I'm missing something, but what has happened to the "tmp_val = > > tmp_val * 9 - 200" part of the old code? > > > > Also looks like there was an intention to replace "reg_val |= 0x8 << > > 24" with the use of the new DRAM_DRR_BURST macro, but I can't see it > > anywhere either. > > > > I forgot to explain that, I removed the burst refresh. Don't know > whether this was a good idea, but there are some resources stating burst > refreshes reduce power consumption at the expense of increased latency. > Normally there is a tRFC-long refresh every tREFI, with burst there are > 9*tRFC breaks every 9*tREFI. This leads to dram "hangs" of up to 3us > every ~70us. > > "reg_val |= 0x8 << 24" made the controller issue 9 refresh commands in > burst, and thus the period between refresh had to be multiplied by 9. > I don't know why they subtract 200, but it looks like some safety > margin. Maybe I shouldn't have removed that... > > This patch was a RFC to discuss such things, so thanks for the comments. > Maybe it should be added again, but I don't think they originally wasted > any thoughts on this, they simply took the default values (reset-value > for burst is 0x8). I have found a very interesting document, which seems to describe some aspects of our dram controller: http://www.ti.com/lit/pdf/spruhn7 (TI KeyStone II Architecture DDR3 Memory Controller) Texas Instruments has apparently licensed the memory controller from the same IP vendor for their KeyStone II product. A lot of registers and bitfields are very similar to what we have in A10/A13/A20. The older TI KeyStone I used a completely different memory controller. Maybe they were not very happy with it and decided to just license something of a better quality ;-) Anyway, this KeyStone II DDR3 document contains some interesting sections about commands scheduling and autorefresh. This "burst" configuration knob might be actually a "backlog counter" limit and generally a good thing to have. Now we have a pile of documentation about similar dram controllers, that you have found earlier (RK30XX, bwdsp100). And also your nice find of RK29 sources from just a few days ago on irc: https://github.com/omegamoon/Rockchip-GPL-Kernel/blob/master/arch/arm/mach-rk29/ddr.c -- Best regards, Siarhei Siamashka -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.