From fa0f3b27778639caa4c473309500956bdc83e3d6 Mon Sep 17 00:00:00 2001 From: John Sully Date: Sat, 22 Sep 2018 17:15:19 +0200 Subject: [PATCH 1/2] Use the ready signal for cas_allowed so that arbitrators know not to iterate --- litedram/core/bankmachine.py | 27 ++++++++++++--------------- litedram/core/multiplexer.py | 3 +-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/litedram/core/bankmachine.py b/litedram/core/bankmachine.py index 1df3aa6..d03c8df 100644 --- a/litedram/core/bankmachine.py +++ b/litedram/core/bankmachine.py @@ -32,7 +32,6 @@ class BankMachine(Module): self.refresh_req = Signal() self.refresh_gnt = Signal() self.ras_allowed = ras_allowed = Signal() - self.cas_allowed = cas_allowed = Signal() a = settings.geom.addressbits ba = settings.geom.bankbits + log2_int(nranks) self.cmd = cmd = stream.Endpoint(cmd_request_rw_layout(a, ba)) @@ -108,20 +107,18 @@ class BankMachine(Module): ).Elif(cmd_buffer.source.valid, If(has_openrow, If(hit, - If(cas_allowed, - cmd.valid.eq(1), - If(cmd_buffer.source.we, - req.wdata_ready.eq(cmd.ready), - cmd.is_write.eq(1), - cmd.we.eq(1), - ).Else( - req.rdata_valid.eq(cmd.ready), - cmd.is_read.eq(1) - ), - cmd.cas.eq(1), - If(cmd.ready & auto_precharge, - NextState("AUTOPRECHARGE") - ) + cmd.valid.eq(1), + If(cmd_buffer.source.we, + req.wdata_ready.eq(cmd.ready), + cmd.is_write.eq(1), + cmd.we.eq(1), + ).Else( + req.rdata_valid.eq(cmd.ready), + cmd.is_read.eq(1) + ), + cmd.cas.eq(1), + If(cmd.ready & auto_precharge, + NextState("AUTOPRECHARGE") ) ).Else( NextState("PRECHARGE") diff --git a/litedram/core/multiplexer.py b/litedram/core/multiplexer.py index 2e8eb68..541e464 100644 --- a/litedram/core/multiplexer.py +++ b/litedram/core/multiplexer.py @@ -226,7 +226,6 @@ class Multiplexer(Module, AutoCSR): # CAS control self.comb += cas_allowed.eq(tccdcon.ready) - self.comb += [bm.cas_allowed.eq(cas_allowed) for bm in bank_machines] # tWTR timing (Write to Read delay) self.submodules.twtrcon = twtrcon = tXXDController( @@ -306,7 +305,7 @@ class Multiplexer(Module, AutoCSR): choose_req.want_reads.eq(1), choose_cmd.want_activates.eq(ras_allowed), choose_cmd.cmd.ready.eq(~choose_cmd.activate() | ras_allowed), - choose_req.cmd.ready.eq(1), + choose_req.cmd.ready.eq(cas_allowed), steerer_sel(steerer, "read"), If(write_available, # TODO: switch only after several cycles of ~read_available? From c4bd842cdfd559cd7409efde4c2709e17796ba34 Mon Sep 17 00:00:00 2001 From: John Sully Date: Sun, 23 Sep 2018 01:21:18 +0200 Subject: [PATCH 2/2] Fix many bugs --- litedram/core/bankmachine.py | 5 ++--- litedram/core/multiplexer.py | 15 ++++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/litedram/core/bankmachine.py b/litedram/core/bankmachine.py index d03c8df..839c96f 100644 --- a/litedram/core/bankmachine.py +++ b/litedram/core/bankmachine.py @@ -31,7 +31,6 @@ class BankMachine(Module): self.req = req = Record(cmd_layout(aw)) self.refresh_req = Signal() self.refresh_gnt = Signal() - self.ras_allowed = ras_allowed = Signal() a = settings.geom.addressbits ba = settings.geom.bankbits + log2_int(nranks) self.cmd = cmd = stream.Endpoint(cmd_request_rw_layout(a, ba)) @@ -150,9 +149,9 @@ class BankMachine(Module): fsm.act("ACTIVATE", sel_row_addr.eq(1), track_open.eq(1), - cmd.valid.eq(ras_allowed), + cmd.valid.eq(1), cmd.is_cmd.eq(1), - If(cmd.ready & ras_allowed, + If(cmd.ready, NextState("TRCD") ), cmd.ras.eq(1) diff --git a/litedram/core/multiplexer.py b/litedram/core/multiplexer.py index 541e464..526bb1c 100644 --- a/litedram/core/multiplexer.py +++ b/litedram/core/multiplexer.py @@ -63,7 +63,9 @@ class _CommandChooser(Module): If(cmd.valid & cmd.ready & (arbiter.grant == i), request.ready.eq(1) ) - self.comb += arbiter.ce.eq(cmd.ready) + # Arbitrate if we're accepting commands, *or* if we are not but the current selection is not valid + # This is to ensure that a valid command is selected when cmd.ready goes high + self.comb += arbiter.ce.eq(cmd.ready | ~cmd.valid) # helpers def accept(self): @@ -91,7 +93,7 @@ class _Steerer(Module): if not hasattr(cmd, "valid"): return 0 else: - return cmd.valid & getattr(cmd, attr) + return cmd.valid & cmd.ready & getattr(cmd, attr) for phase, sel in zip(dfi.phases, self.sel): nranks = len(phase.cs_n) @@ -114,9 +116,9 @@ class _Steerer(Module): self.sync += [ phase.address.eq(Array(cmd.a for cmd in commands)[sel]), - phase.cas_n.eq(~Array(cmd.cas for cmd in commands)[sel]), - phase.ras_n.eq(~Array(cmd.ras for cmd in commands)[sel]), - phase.we_n.eq(~Array(cmd.we for cmd in commands)[sel]) + phase.cas_n.eq(~Array(valid_and(cmd, "cas") for cmd in commands)[sel]), + phase.ras_n.eq(~Array(valid_and(cmd, "ras") for cmd in commands)[sel]), + phase.we_n.eq(~Array(valid_and(cmd, "we") for cmd in commands)[sel]) ] rddata_ens = Array(valid_and(cmd, "is_read") for cmd in commands) @@ -218,7 +220,6 @@ class Multiplexer(Module, AutoCSR): # RAS control self.comb += ras_allowed.eq(trrdcon.ready & tfawcon.ready) - self.comb += [bm.ras_allowed.eq(ras_allowed) for bm in bank_machines] # tCCD timing (Column to Column delay) self.submodules.tccdcon = tccdcon = tXXDController(settings.timing.tCCD) @@ -322,7 +323,7 @@ class Multiplexer(Module, AutoCSR): choose_req.want_writes.eq(1), choose_cmd.want_activates.eq(ras_allowed), choose_cmd.cmd.ready.eq(~choose_cmd.activate() | ras_allowed), - choose_req.cmd.ready.eq(1), + choose_req.cmd.ready.eq(cas_allowed), steerer_sel(steerer, "write"), If(read_available, If(~write_available | max_write_time,