enjoy-digital / litedram

Small footprint and configurable DRAM core
Other
365 stars 115 forks source link

Refresh S6DDRPHY #182

Open enjoy-digital opened 4 years ago

enjoy-digital commented 4 years ago

The Spartan6 S6DDRPHY needs to be refreshed with the simplifications/improvements made to the others PHYs.

cklarhorst commented 3 years ago

@enjoy-digital Could you please elaborate a bit more on what simplifications/improvements are not implemented/missing for S6DDRPHY? Is one of the missing features that S6DDRPHY doesn't use IDELAY/ODELAY for data alignment?

enjoy-digital commented 3 years ago

@cklarhorst: sorry i forgot to answer, the S6DDRPHY is the first PHY that has been created and we improved things since then, so the architecture would need to be updated a bit and it's also possible some features are missing (DQS Pre/Post amble for example). That's not necessarilly something easy to work on when not familiar with LiteDRAM, so i was planning to work on that, but if you want to have a closer look, i could help you getting started.

norbertthiel commented 3 years ago

@enjoy-digital I am currently implementing Litex on a custom XC6SLX16-3 board which comes with LPDDR (Winbond W949D6KBH) It works fine using parameters copy from the pipistrello board - CPU clock at 83.3 MHz and memory at 166MHz

So, before start digging into this, I am wondering if you started refreshing the S6DDRPHY already?

enjoy-digital commented 3 years ago

@norbertthiel: Hi, the work has not been initiated in this no.

norbertthiel commented 3 years ago

@norbertthiel: Hi, the work has not been initiated in this no.

Hi and thanks for the reply. Any short-term plans for initiating this?

Maybe you can point me were to start looking at to fix my "problem":

memory supports 200MHz, FPGA has speedgrade 3

norbertthiel commented 3 years ago

@enjoy-digital like this:

image
enjoy-digital commented 3 years ago

Hi @norbertthiel,

the S6DDRPHY needs to be refreshed since as you saw is not very flexible and can require manual adjustments to get it working. Are the timings OK at 100MHz sys_clk? If yes, you could try to adjust the passed rd_bitslip/wr_bitslip values: https://github.com/enjoy-digital/litedram/blob/master/litedram/phy/s6ddrphy.py#L38.

I'd like the refresh work to be done in the next months but can't guarantee it.

Florent

norbertthiel commented 3 years ago

Hi @norbertthiel,

the S6DDRPHY needs to be refreshed since as you saw is not very flexible and can require manual adjustments to get it working. Are the timings OK at 100MHz sys_clk? If yes, you could try to adjust the passed rd_bitslip/wr_bitslip values: https://github.com/enjoy-digital/litedram/blob/master/litedram/phy/s6ddrphy.py#L38.

I'd like the refresh work to be done in the next months but can't guarantee it.

Florent

Thanks Florent, yepp I saw that a few timings are not flexible and I am going to play with that. Looking forward to your refresh work. Any other sys_clk timing I tried did not work.

I will play with bitslip and also thought that read_latency could have an impact.

Are timings OK at 100MHz sys_clk?

Didn’t connect litescope yet -probably gonna try soon - or do you suggest to use simulation?

enjoy-digital commented 3 years ago

For the timings, this is reported by ISE.

norbertthiel commented 3 years ago

For the timings, this is reported by ISE.

Ah those, sorry, yepp, constraints met, no problems reported

norbertthiel commented 3 years ago

@enjoy-digital quick update: sys_clk 100MHz (400MHz sdram_full) works with:

enjoy-digital commented 3 years ago

Great @norbertthiel, thanks for the feedback!

norbertthiel commented 3 years ago

Great @norbertthiel, thanks for the feedback!

in fact phase shifts 230 and 260 work stable - 240 and 270 did occasionally result in memtest errors - 260 and 230 seems to work reliably

and let me just say how much I enjoy using LiteX

enjoy-digital commented 3 years ago

Thanks, tweaking the S6DDRPHY's settings is not necessarily the easiest thing to start with :) (in 7-Series / Ultrascale these manual settings are almost all automated and the aim of this issue is also simplify this for Spartan6), but glad you manage to get it working, things should be easier now.