Hello, On Wed, 9 Dec 2015 19:29:49 +0100 Jens Kuske <jensku...@gmail.com> wrote:
> On 09/12/15 09:40, Siarhei Siamashka wrote: > > Thanks for the explanations. I finally got lima-memtester up and > > running on H3 hardware (not that it was difficult, but just the amount > > of unnecessary compatibility breaks in the H3 SDK kernel was a bit > > unexpected and really discouraging, they are really doing almost > > *everything* in a different way compared to A10/A20 SDK kernel just > > for the sake of making things different): > > > > > > https://github.com/ssvb/lima-memtester/releases/tag/20151207-orange-pi-pc-fel-test > > > > Currently U-Boot v2016.01-rc2 fails the lima-memtester check unless > > the DRAM clock speed is reduced to 456 MHz on my Orange Pi PC. But > > if I use the boot0 bootloader to boot the same kernel image > > (uImage-3.4-sun8i-h3-lima-memtester), then the test works fine. > > This means that there must be still something wrong with the H3 > > DRAM init code in U-Boot, or maybe some clocks are wrong > > > > I tried to compare boot0 and the SDK source again and actually > found another difference. Did you dump the DRAM controller registers to find this difference between U-Boot and boot0? > The read delays (except for dqs) are doubled in boot0 before > writing them to the registers. Looks like they suddenly needed > higher values than possible with 4 bit. The register seems to > take 6 bit wide values. Well, we can always change the layout of data in this struct and allocate 8 bits per each delay value instead of 4 bits: struct dram_para para = { .read_delays = 0x00007979, .write_delays = 0x6aaa0000, .dual_rank = 0, .bus_width = 32, .row_bits = 15, .page_size = 4096, }; Are they originally the 'tpr11' and 'tpr12' parameters from FEX? Maybe they belong in Kconfig, with a comment about how to do all the needed conversion from FEX (multiplication by 2)? BTW, do these delays serve a somewhat similar purpose as the 'tpr3' parameter in A10/A13/A20? Later we could adapt the a10-tpr3-scan script and make a h3 variant of it for bruteforce searching optimal values of these delays. With the lima-memtester test failures, we have already discovered in an experimental way that correctly configuring these delays apparently affects reliability :-) > After fixing that, lima-memtester doesn't fail at 672 MHz anymore > on my Orange Pi Plus. Before it failed at 552 and higher. > Patch below. Thanks. With this U-Boot patch, lima-memtester does not fail anymore at 672 MHz on my Orange Pi PC board too. That's a major improvement over what is in U-boot 2016.01-rc2. If you are going to submit this patch to U-Boot, you can have my Tested-by: Siarhei Siamashka <siarhei.siamas...@gmail.com> Still increasing the DRAM clock speed even by one 24 MHz step to 696 MHz makes it fail again. So there does not seem to be much safety headroom available. I have prepared an updated v3 tarball with your fix and added a table to the wiki page: http://linux-sunxi.org/Orange_Pi_PC#DRAM_clock_speed_limit Maybe other people could test their Orange Pi PC boards too in order to see if the results are generally similar, better or worse. Your Orange Pi Plus is a different board and may in theory have a slightly different DRAM tracks routing. > Maybe we could also replaced the setbits with writel, because I > don't see a good reason not to overwrite the registers, they are > all zero after reset and have 0x00003f3f mask. Or maybe "clrsetbits_le32"? The use of "setbits_le32" indeed looks a bit strange here. I'm fine with "writel" too. In fact I would be fine with any fix that can quickly resolve the problem. > Jens > > > > diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun8i_h3.c > b/arch/arm/cpu/armv7/sunxi/dram_sun8i_h3.c > index b721d60..03bd013 100644 > --- a/arch/arm/cpu/armv7/sunxi/dram_sun8i_h3.c > +++ b/arch/arm/cpu/armv7/sunxi/dram_sun8i_h3.c > @@ -73,10 +73,10 @@ static void mctl_dq_delay(u32 read, u32 write) > > for (i = 0; i < 4; i++) { > val = DATX_IOCR_WRITE_DELAY((write >> (i * 4)) & 0xf) | > - DATX_IOCR_READ_DELAY((read >> (i * 4)) & 0xf); > + DATX_IOCR_READ_DELAY(((read >> (i * 4)) & 0xf) * 2); > > for (j = DATX_IOCR_DQ(0); j <= DATX_IOCR_DM; j++) > - setbits_le32(&mctl_ctl->datx[i].iocr[j], val); > + writel(val, &mctl_ctl->datx[i].iocr[j]); > } > > clrbits_le32(&mctl_ctl->pgcr[0], 1 << 26); > @@ -85,8 +85,8 @@ static void mctl_dq_delay(u32 read, u32 write) > val = DATX_IOCR_WRITE_DELAY((write >> (16 + i * 4)) & 0xf) | > DATX_IOCR_READ_DELAY((read >> (16 + i * 4)) & 0xf); > > - setbits_le32(&mctl_ctl->datx[i].iocr[DATX_IOCR_DQS], val); > - setbits_le32(&mctl_ctl->datx[i].iocr[DATX_IOCR_DQSN], val); > + writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQS]); > + writel(val, &mctl_ctl->datx[i].iocr[DATX_IOCR_DQSN]); > } > > setbits_le32(&mctl_ctl->pgcr[0], 1 << 26); -- 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.
