From efe9a44c93ee946ed88d8096f24d3145a456cb73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Boczar?= Date: Mon, 11 May 2020 16:44:50 +0200 Subject: [PATCH] frontend/adaptation: clean up LiteDRAMNativePortUpConverter code --- litedram/frontend/adaptation.py | 92 +++++++++++++++------------------ test/test_adaptation.py | 10 ++-- 2 files changed, 47 insertions(+), 55 deletions(-) diff --git a/litedram/frontend/adaptation.py b/litedram/frontend/adaptation.py index 7d8abe2..1c068e4 100644 --- a/litedram/frontend/adaptation.py +++ b/litedram/frontend/adaptation.py @@ -139,6 +139,9 @@ class LiteDRAMNativePortUpConverter(Module): (when possible, ie when consecutive and bursting) - N writes from user are regrouped in a single one to the controller (when possible, ie when consecutive and bursting) + Incomplete writes/reads (i.e. with n < N) are handled automatically in the + middle of a burst, but last command has to use cmd.last=1 if the last burst + is not complete (not all N addresses have been used). """ def __init__(self, port_from, port_to, reverse=False): assert port_from.clock_domain == port_to.clock_domain @@ -155,24 +158,25 @@ class LiteDRAMNativePortUpConverter(Module): # Command ---------------------------------------------------------------------------------- # defines cmd type and the chunks that have been requested for the current port_to command - sel = Signal(ratio) - cmd_buffer = stream.SyncFIFO([("sel", ratio), ("we", 1)], 4) + sel = Signal(ratio) + cmd_buffer = stream.SyncFIFO([("sel", ratio), ("we", 1)], 4) self.submodules += cmd_buffer # store last received command - cmd_addr = Signal.like(port_from.cmd.addr) - cmd_we = Signal() - cmd_last = Signal() + cmd_addr = Signal.like(port_from.cmd.addr) + cmd_we = Signal() + cmd_last = Signal() # indicates that we need to proceed to the next port_to command - next_cmd = Signal() - addr_changed = Signal() + next_cmd = Signal() + addr_changed = Signal() # signals that indicate that write/read convertion has finished - wdata_finished = Signal() - rdata_finished = Signal() + wdata_finished = Signal() + rdata_finished = Signal() # used to prevent reading old memory value if previous command has written the same address - read_lock = Signal() - rw_collision = Signal() + read_lock = Signal() + rw_collision = Signal() self.comb += [ + cmd_buffer.source.ready.eq(wdata_finished | rdata_finished), addr_changed.eq(cmd_addr[log2_int(ratio):] != port_from.cmd.addr[log2_int(ratio):]), # collision happens on write to read transition when address does not change rw_collision.eq(cmd_we & (port_from.cmd.valid & ~port_from.cmd.we) & ~addr_changed), @@ -182,8 +186,8 @@ class LiteDRAMNativePortUpConverter(Module): # * we received all the `ratio` commands # * this is the last command in a sequence next_cmd.eq(addr_changed | (cmd_we != port_from.cmd.we) | (sel == 2**ratio - 1) | cmd_last), - # when the first command is received, send it immediatelly If(sel == 0, + # when the first command is received, send it immediatelly If(port_from.cmd.valid & ~read_lock, port_to.cmd.valid.eq(1), port_to.cmd.we.eq(port_from.cmd.we), @@ -192,7 +196,7 @@ class LiteDRAMNativePortUpConverter(Module): ) ).Else( # we have already sent the initial command, now either continue sending cmd.ready - # to the master or send the current command if we have to go to next command + # to the master or send the current command if we have to go to next one faster If(next_cmd, cmd_buffer.sink.valid.eq(1), cmd_buffer.sink.sel.eq(sel), @@ -201,11 +205,10 @@ class LiteDRAMNativePortUpConverter(Module): port_from.cmd.ready.eq(port_from.cmd.valid), ) ), - cmd_buffer.source.ready.eq(wdata_finished | rdata_finished) ] self.sync += [ - # whenever a command gets accepted, update `sel` bitmask and store the command info + # whenever a command gets accepted, store it and update `sel` based on address If(port_from.cmd.valid & port_from.cmd.ready, cmd_addr.eq(port_from.cmd.addr), cmd_we.eq(port_from.cmd.we), @@ -227,19 +230,17 @@ class LiteDRAMNativePortUpConverter(Module): # Read Datapath ---------------------------------------------------------------------------- if mode == "read" or mode == "both": - # buffers output from port_to - rdata_fifo = stream.SyncFIFO(port_to.rdata.description, ratio) - # connected to the buffered output + # queue received data not to loose it when it comes too fast + rdata_fifo = stream.SyncFIFO(port_to.rdata.description, ratio - 1) rdata_converter = stream.StrideConverter( port_to.rdata.description, port_from.rdata.description, reverse=reverse) self.submodules += rdata_fifo, rdata_converter - # bitmask shift register with single 1 bit and all other 0s + # shift register with a bitmask of current chunk rdata_chunk = Signal(ratio, reset=1) rdata_chunk_valid = Signal() - # whenever the converter spits data chunk we shift the chunk bitmask self.sync += \ If(rdata_converter.source.valid & rdata_converter.source.ready, @@ -247,84 +248,75 @@ class LiteDRAMNativePortUpConverter(Module): ) self.comb += [ - # port_to -> rdata_fifo -> rdata_converter + # port_to -> rdata_fifo -> rdata_converter -> port_from port_to.rdata.connect(rdata_fifo.sink), rdata_fifo.source.connect(rdata_converter.sink), - # chunk is valid if it's bit is in `sel` sent previously to the FIFO rdata_chunk_valid.eq((cmd_buffer.source.sel & rdata_chunk) != 0), - # whenever `sel` from FIFO is valid If(cmd_buffer.source.valid & ~cmd_buffer.source.we, - # if that chunk is valid we send it to the user port and wait for ready from user + # if that chunk is valid we send it to the user port and wait for ready If(rdata_chunk_valid, port_from.rdata.valid.eq(rdata_converter.source.valid), port_from.rdata.data.eq(rdata_converter.source.data), rdata_converter.source.ready.eq(port_from.rdata.ready) - # if this was not requested by `sel` then we just ack it - ).Else( + ).Else( # if this chunk was not requested in `sel`, ignore it rdata_converter.source.ready.eq(1) ), - rdata_finished.eq(rdata_converter.source.valid & rdata_converter.source.ready & rdata_chunk[ratio - 1]) + rdata_finished.eq(rdata_converter.source.valid & rdata_converter.source.ready + & rdata_chunk[ratio - 1]) ), ] # Write Datapath --------------------------------------------------------------------------- if mode == "write" or mode == "both": - wdata_fifo = stream.SyncFIFO(port_from.wdata.description, ratio) + # queue write data not to miss it when the lower chunks haven't been reqested + wdata_fifo = stream.SyncFIFO(port_from.wdata.description, ratio - 1) wdata_converter = stream.StrideConverter( port_from.wdata.description, port_to.wdata.description, reverse=reverse) self.submodules += wdata_converter, wdata_fifo - # bitmask shift register with single 1 bit and all other 0s + # shift register with a bitmask of current chunk wdata_chunk = Signal(ratio, reset=1) wdata_chunk_valid = Signal() - # whenever the converter spits data chunk we shift the chunk bitmask self.sync += \ If(wdata_converter.sink.valid & wdata_converter.sink.ready, wdata_chunk.eq(Cat(wdata_chunk[ratio-1], wdata_chunk[:ratio-1])) ) - # replicate sel so that each bit covers according part of we bitmask (1 sel bit may cover - # multiple bytes) + # replicate `sel` bits to match the width of port_to.wdata.we wdata_sel = Signal.like(port_to.wdata.we) - # self.comb += wdata_sel.eq( - # Cat([Replicate(cmd_buffer.source.sel[i], port_to.wdata.we.nbits // sel.nbits) for i in range(ratio)]) - # ) - - self.sync += [ - If(cmd_buffer.source.valid & cmd_buffer.source.we & wdata_chunk[ratio - 1], - wdata_sel.eq(Cat([Replicate(cmd_buffer.source.sel[i], port_to.wdata.we.nbits // sel.nbits) - for i in range(ratio)])) - ) + wdata_sel_parts = [ + Replicate(cmd_buffer.source.sel[i], port_to.wdata.we.nbits // sel.nbits) + for i in range(ratio) ] + self.sync += \ + If(cmd_buffer.source.valid & cmd_buffer.source.we & wdata_chunk[ratio - 1], + wdata_sel.eq(Cat(wdata_sel_parts)) + ) self.comb += [ + # port_from -> wdata_fifo -> wdata_converter port_from.wdata.connect(wdata_fifo.sink), - wdata_chunk_valid.eq((cmd_buffer.source.sel & wdata_chunk) != 0), - If(cmd_buffer.source.valid & cmd_buffer.source.we, + # when the current chunk is valid, read it from wdata_fifo If(wdata_chunk_valid, wdata_converter.sink.valid.eq(wdata_fifo.source.valid), wdata_converter.sink.data.eq(wdata_fifo.source.data), wdata_converter.sink.we.eq(wdata_fifo.source.we), wdata_fifo.source.ready.eq(1), - ).Else( + ).Else( # if chunk is not valid, send any data and do not advance fifo wdata_converter.sink.valid.eq(1), - wdata_converter.sink.data.eq(0), - wdata_converter.sink.we.eq(0), - wdata_fifo.source.ready.eq(0), ), ), - port_to.wdata.valid.eq(wdata_converter.source.valid), port_to.wdata.data.eq(wdata_converter.source.data), port_to.wdata.we.eq(wdata_converter.source.we & wdata_sel), wdata_converter.source.ready.eq(port_to.wdata.ready), - # wdata_finished.eq(wdata_converter.source.valid & wdata_converter.source.ready), - wdata_finished.eq(wdata_converter.sink.valid & wdata_converter.sink.ready & wdata_chunk[ratio-1]), + wdata_finished.eq(wdata_converter.sink.valid & wdata_converter.sink.ready + & wdata_chunk[ratio-1]), ] # LiteDRAMNativePortConverter ---------------------------------------------------------------------- diff --git a/test/test_adaptation.py b/test/test_adaptation.py index e7efc80..455b781 100644 --- a/test/test_adaptation.py +++ b/test/test_adaptation.py @@ -77,13 +77,13 @@ class CDCDUT(ConverterDUT): class TestAdaptation(MemoryTestDataMixin, unittest.TestCase): - def test_converter_down_ratio_must_be_integer(self): + def test_down_converter_ratio_must_be_integer(self): with self.assertRaises(ValueError) as cm: dut = ConverterDUT(user_data_width=64, native_data_width=24, mem_depth=128) dut.finalize() self.assertIn("ratio must be an int", str(cm.exception).lower()) - def test_converter_up_ratio_must_be_integer(self): + def test_up_converter_ratio_must_be_integer(self): with self.assertRaises(ValueError) as cm: dut = ConverterDUT(user_data_width=32, native_data_width=48, mem_depth=128) dut.finalize() @@ -155,7 +155,7 @@ class TestAdaptation(MemoryTestDataMixin, unittest.TestCase): # Verify 32-bit to 256-bit up-conversion. self.converter_test(test_data="32bit_to_256bit", user_data_width=32, native_data_width=256) - def test_converter_up_read_latencies(self): + def test_up_converter_read_latencies(self): # Verify that up-conversion works with different port reader latencies cases = { "1to2": dict(test_data="8bit_to_16bit", user_data_width=8, native_data_width=16), @@ -168,7 +168,7 @@ class TestAdaptation(MemoryTestDataMixin, unittest.TestCase): with self.subTest(conversion=conversion): self.converter_test(**kwargs, read_latency=latency) - def test_converter_down_read_latencies(self): + def test_down_converter_read_latencies(self): # Verify that down-conversion works with different port reader latencies cases = { "2to1": dict(test_data="64bit_to_32bit", user_data_width=64, native_data_width=32), @@ -330,7 +330,7 @@ class TestAdaptation(MemoryTestDataMixin, unittest.TestCase): self.converter_readback_test(dut, pattern=[], mem_expected=mem_expected, main_generator=main_generator) - def test_converter_up_not_aligned(self): + def test_up_converter_not_aligned(self): data = self.pattern_test_data["8bit_to_32bit_not_aligned"] dut = ConverterDUT(user_data_width=8, native_data_width=32, mem_depth=len(data["expected"]), separate_rw=False)