search

enjoy-digital / litedram

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

Add support for DDR4 RDIMMs #189

Closed daveshah1 closed 4 years ago

daveshah1 commented 4 years ago

I would appreciate some feedback on the structure, it seems like perhaps is_rdimm and drive strengths should be a module property, but I can't find a nice way of glueing that in.

This also still needs regression testing on a non-RDIMM platform too, I can do that if you want.

daveshah1 commented 4 years ago

See also enjoy-digital/litex#490

enjoy-digital commented 4 years ago

Thanks! i'll review it in the next days.

enjoy-digital commented 4 years ago

In modules.py, we should probably create specialized SDRAMModules:

class SDRAMRegisteredModule(SDRAMModule): registered = True

class SDRModule(SDRAMModule): memtype = "SDR"
class SDRRegisteredModule(SDRAMRegisteredModule): memtype = "SDR"

...

class DDR4Module(SDRAMModule): memtype = "DDR4"
class DDR4RegisteredModule(SDRAMRegisteredModule): memtype = "DDR4"

And use them as the base class for the modules. This would simplify the definition and would provide the registered attribute that would be used by the PHY/Core.

In PhySettings, we could rename is_rdimm to registered attribute. Does it make sense for you? If so, i'm happy to do the changes and do the regression testing on targets with Unbuffered memory. (unless you want to do it, but the improvements here are more related to LiteDRAM itself than your PR).

daveshah1 commented 4 years ago

Yes, that sounds good. The RCD drive strength settings come from the SPD data and so should also be a property of DDR4RegisteredModule. Please feel free to make these changes.

enjoy-digital commented 4 years ago

Thanks, merged. I'll do the adaptations.