Hi all
On Fri, Feb 15, 2019 at 12:40 PM Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
Dipl.-Ing. Dr.techn. Philipp Tomsich Theobroma Systems Design und Consulting GmbH Seestadtstrasse 27 (Aspern IQ), A-1220 Wien, Austria Phone: +43 1 2369893-401, Fax: +43 1 2369893-9-401 Cell phone: +43 664 8346109 http://www.theobroma-systems.com
On 15.02.2019, at 12:31, Andre Przywara andre.przywara@arm.com wrote:
On Fri, 15 Feb 2019 11:18:38 +0100 Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
Hi Philipp,
On 14.02.2019, at 22:24, André Przywara andre.przywara@arm.com wrote:
On 14/02/2019 16:36, Philipp Tomsich wrote:
On 14.02.2019, at 16:58, Michael Trimarchi michael@amarulasolutions.com wrote:
Set two rank timing and exit self-refresh timing seems not done properly. We know use the same write that we are using on H5 silicon. Tested was done in A33 allwinner cpu, dual rank connection connected with two MT41K512M16HA-125:A memory model. Memory is configure as DDR3 1.5V
I will fix the commit
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com
V1->V2: adjust commit message
arch/arm/mach-sunxi/dram_sun8i_a33.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_a33.c b/arch/arm/mach-sunxi/dram_sun8i_a33.c index d73a93a132..355fe30aba 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_a33.c +++ b/arch/arm/mach-sunxi/dram_sun8i_a33.c @@ -146,7 +146,7 @@ static void auto_set_timing_para(struct dram_para *para) writel(reg_val, &mctl_ctl->dramtmg5); /* Set two rank timing and exit self-refresh timing */ clrsetbits_le32(&mctl_ctl->dramtmg8, (0xff << 8) | (0xff << 0),
- 0x33 << 8 | (0x8 << 0));
- 0x33 << 8 | (0x10 << 0));
How exactly does the change in constants match up with the commit message? What is the field at “<< 0”?
From looking at the ZynqMP register guide, which is reported to be close to the various Allwinner DRAM controllers, bits [6:0] are tXS: Self refresh exit delay. Increasing that should not hurt, and if I understand it correctly it only affects the time after the self-refresh *exit*, which happens only after (re-)initialisation(?).
Seems that affect the initialization sequence.
The “set two rank timing” comment doesn’t match our expectation that this will be self-refresh timings, as on the ZynqMP or the A80.
The comment is the original one
Self-refresh only happens, if someone puts the DRAM into self-refresh (i.e. “suspend to DRAM”) and then resumes it. I don’t see a reference to the error occurring from this.
So I was wondering about this as well. Indeed we don't seem to *explicitly* enter or exit Self-Refresh anywhere, but maybe this is done implicitly as part of some training phase? I might be wrong about this, but in some DDR3 documentation I see that changing the DLL state is connected to self refresh, and enabling the DLL is required as part of the initialisation? So is the DRAMC somehow triggering a self refresh exit "magically"? If I understand this correctly, our ranking test does reset the controller?
I’d need to reread JESD79 (a much more relaxing read than the daily news), to figure out whether there’s any valid reason to play with self-refresh during the training.
That said, if the field indeed was the exit self-refresh timing, this would usually be tXSDLL, encoded as tXSDLL/32. JESD79 specifies tESXR as tXSDLL (which in turn is tDLLK(min)), which is 512 nCK always: 0x10 would be 512 and 0x08 would only be 256… then again, this should only matter when exiting self-refresh.
What is the best option here? create a better commit or try to go in deep.
So I was looking at this: https://www.xilinx.com/html_docs/registers/ug1087/ddrc___dramtmg8.html and tXSDLL is bits [14:8], while the non-DLL tXS is bits [6:0].
Note that the ‘auto-enter self-refresh’ functionality is controlled through BIT(3) in PWRCTL on the A33, according to Allwinner’s basic_loader. I don’t know whether that may be turned on by the driver (i.e. didn’t check).
I don't see us touching pwrctl for the A33 (but for the A23).
OTOH, when working with an underdocumented DRAM register layout and once one has an educated guess at what register/setting may be affected, a DRAM protocol analyzer can provide conclusive answers by looking at the differences in behaviour with 0x8 and 0x10 in that bitfield…
Nice to have but It's not my case. The best for me is update the commit message and give more info there. Add the memory components and the layout of my setup, ask to test other boards before get included
Michael
All I have is a multimeter and no A33 ;-)
You are aware that was more of a philosophical rambling and not intended as a request to you… However, I am way more surprised that none of the parties having money riding on their A33 designs has gone through the required effort.
Cheers, Philipp.