enjoy-digital / litedram

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

ECP5: Use ODDRX1F for address and command bus #194

Open kingoflolz opened 4 years ago

kingoflolz commented 4 years ago

It should be possible to use ODDRX1F instead of ODDRX2F for the address and command bus, so that these signals can be placed on the top IO bank instead of just the left and right.

This seems like a relatively straightforward change, and I would be happy to provide a pull request, unless there are some nuances I'm not seeing?

enjoy-digital commented 4 years ago

Hi @kingoflolz, if you do the change and confirm the Versa ECP5 target (https://github.com/enjoy-digital/litex/blob/master/litex/boards/targets/versa_ecp5.py) is still building fine with it, that's probably something we could merge yes.

enjoy-digital commented 4 years ago

@kingoflolz: sorry i won't be able to spend time investigating this and i'm not sure it will be the case for someone else in the near future. I'll however be happy to test and integrate a validated solution if provided.

madscientist159 commented 3 years ago

@enjoy-digital Since it's been a year, and we've run straight into this problem on some custom hardware, any chance of re-visiting this request?

enjoy-digital commented 3 years ago

@madscientist159: If I remember correctly replacing the ODDRX2F with ODDRX1F was not as straightforward as expected and some time would need to be spent understanding why. If we want to go further on this, I would happy to test and do the integration of a validated solution (or spend time on this if someone has funds for it).

madscientist159 commented 3 years ago

We've investigated the issue and come up with a working solution here, tested to work on the Versa board: https://gitlab.raptorengineering.com/kestrel-collaboration/kestrel-litex/litedram/-/tree/ecp5ddr1x

The problem wasn't clock skew between ECLK/SCLK, it was the phase shift introduced by the ODDRX2 primitive configured the way it was in LiteX. The ODDRX1 primitive has less effective latency when clocked from SCLK, and its output needed to be delayed by one ECLK period as a result.

It isn't particularly easy to generate a low-skew inverted clock signal (needed to drive the ODDRX1 primitive) from an existing primary clock on these devices. We settled on a second CLKDIVF with some logic to use the ALIGNWD signal to ensure the clock is inverted, but the obvious disadvantage of this approach is the need to allow edge clock signals to cross to the other side of the device (wasting edge clock resources).

I do note that Lattice's documentation encourages the use of the TOP I/O bank for address and command lines. As a result I think it is important that the option be available to use the device as Lattice specifies, even if the logic for that case is a bit more complex.

If the overall approach looks good I can get a pull request together.

Thanks!

enjoy-digital commented 3 years ago

Thanks a lot @madscientist159 for the investigation and proposed solution, that's exactly the analysis that was required. The approach looks good yes and creating a PR would simplify integrating your solution yes. We can probably simplify the code a bit by creating wrappers for the primitives and avoid some python code duplication but I'm happy to handle this part.

aacuevas commented 2 years ago

Hi. I am in such a case, in which we require to move the address lines to a TOP bank, so here is some feedback. Our pinout requires data and command lines connected to side bank 7 and address to top bank 0, so they share a corner. We are also using Diamond due to some requisites, which was easily addressed.

When trying to build the design from @madscientist159, though, the router gave us an error related to DDR placement.

We have solved this by removing both ECLKBRIDGECS and connecting sys_2x_i_clk directly to the two ECLKSYNCB modules that feed the edge clocks to both CLKDIVF' forsys_clkand @madscientist159's phase-invertedsys_inv_clk`. This builds for our required pinout.

We have tested these changes with an OrangeCrab (Versas are not available) and they work. In this board, both data and address are connected to two side banks, so there might be unforeseen consequences of moving address to the top bank, but a design with that pinout pases PAR and the logic seems to work, so we are hopeful.

I will write an update once we build our custom hardware and test the final configuration, in case it might be useful for anyone else.

kkanics commented 2 weeks ago

Any updates on integrating this?