clean up SDRAM module system

[enjoy-digital]

Can you describe what you have in mind?

[sbourdeauducq]

One problem is the margin parameter to the ns function depends on the ratio of the PHY. With the current architecture it is not possible to use ns correctly. The ns function also only supports 1:1 and 1:2 system:command ratios at the moment.

Another (minor) problem is the data structures (dictionaries vs. NamedTuples) are different between the module system and the SDRAM controllers. A single type of data structure should be used consistently.

[enjoy-digital]

I'm going to look at the margin parameter in a few hours.

For the second point, is it better like this:

class SDRAMModule:
    def __init__(self, clk_freq, memtype):
        self.clk_freq = clk_freq
        self.memtype = memtype
        self.geom_settings = GeomSettings(
            bankbits=log2_int(self.nbanks),
            rowbits=log2_int(self.nrows),
            colbits=log2_int(self.ncols),
        )
        self.timing_settings = TimingSettings(
            tRP=self.ns(self.tRP),
            tRCD=self.ns(self.tRCD),
            tWR=self.ns(self.tWR),
            tWTR=self.tWTR,
            tREFI=self.ns(self.tREFI, False),
            tRFC=self.ns(self.tRFC)
        )

# SDR
class IS42S16160(SDRAMModule):
    # geom
    nbanks = 4
    nrows  = 8192
    ncols  = 512

    # timing
    tRP   = 20
    tWR   = 20
    tRCD  = 20
    tWR   = 20
    tWTR  = 2
    tREFI = 64*1000*1000/8192
    tRFC  = 70
    def __init__(self, clk_freq):
        SDRAMModule.__init__(self, clk_freq, "SDR")

[sbourdeauducq]

Why does memtype require special treatment and overloading of __init__?

[enjoy-digital]

OK, fine with that?:

class SDRAMModule:
    def __init__(self, clk_freq):
        self.clk_freq = clk_freq
        self.geom_settings = GeomSettings(
            bankbits=log2_int(self.nbanks),
            rowbits=log2_int(self.nrows),
            colbits=log2_int(self.ncols),
        )
        self.timing_settings = TimingSettings(
            tRP=self.ns(self.tRP),
            tRCD=self.ns(self.tRCD),
            tWR=self.ns(self.tWR),
            tWTR=self.tWTR,
            tREFI=self.ns(self.tREFI, False),
            tRFC=self.ns(self.tRFC)
        )

# SDR
class IS42S16160(SDRAMModule):
    memtype = "SDR"
    # geom
    nbanks = 4
    nrows  = 8192
    ncols  = 512
    # timing
    tRP   = 20
    tWR   = 20
    tRCD  = 20
    tWR   = 20
    tWTR  = 2
    tREFI = 64*1000*1000/8192
    tRFC  = 70
    def __init__(self, clk_freq):
        SDRAMModule.__init__(self, clk_freq)

[sbourdeauducq]

You can remove the IS42S16160.__init__ then. And if you want to keep this architecture, SDRAMModule.__init__ will probably need another parameter that gives information about the PHY (1:1, half rate, quarter rate) so that the ns function can apply the appropriate compensation. Otherwise ok.

[enjoy-digital]

fixed with https://github.com/m-labs/misoc/commit/ed0e4518781b70177e4140934db32e1382df514f and https://github.com/m-labs/misoc/commit/452cbc34cb539348bdea6361b2c91d74a116de78.