Merge pull request #205 from antmicro/jboc/fifo

frontend/fifo: increase FIFO level only after data has actually been written

litedram/frontend/fifo.py

@@ -6,6 +6,8 @@ from litex.gen import *
 from litex.soc.interconnect import stream
 
 from litedram.frontend import dma
+from litedram.common import LiteDRAMNativePort
+from litedram.frontend.axi import LiteDRAMAXIPort
 
 
 def _inc(signal, modulo):
@@ -20,7 +22,7 @@ def _inc(signal, modulo):
 
 
 class _LiteDRAMFIFOCtrl(Module):
-    def __init__(self, base, depth, read_threshold, write_threshold):
+    def __init__(self, base, depth):
         self.base  = base
         self.depth = depth
         self.level = Signal(max=depth+1)
@@ -61,8 +63,8 @@ class _LiteDRAMFIFOCtrl(Module):
         ]
 
         self.comb += [
-            self.writable.eq(self.level < write_threshold),
+            self.writable.eq(self.level < depth),
-            self.readable.eq(self.level > read_threshold)
+            self.readable.eq(self.level > 0)
         ]
 
 
@@ -72,15 +74,20 @@ class _LiteDRAMFIFOWriter(Module):
 
         # # #
 
+        if isinstance(port, LiteDRAMNativePort):
+            write_ready = port.wdata.ready
+        elif isinstance(port, LiteDRAMAXIPort):
+            write_ready = port.w.ready
+        else:
+            raise NotImplementedError(port)
+
         self.submodules.writer = writer = dma.LiteDRAMDMAWriter(port, fifo_depth=32)
         self.comb += [
             writer.sink.valid.eq(sink.valid & ctrl.writable),
             writer.sink.address.eq(ctrl.base + ctrl.write_address),
             writer.sink.data.eq(sink.data),
-            If(writer.sink.valid & writer.sink.ready,
+            sink.ready.eq(writer.sink.valid & writer.sink.ready),
-                ctrl.write.eq(1),
+            ctrl.write.eq(write_ready),
-                sink.ready.eq(1)
-            )
         ]
 
 
@@ -102,19 +109,14 @@ class _LiteDRAMFIFOReader(Module):
 
 
 class LiteDRAMFIFO(Module):
-    def __init__(self, data_width, base, depth, write_port, read_port,
+    """LiteDRAM frontend that allows to use DRAM as a FIFO"""
-        read_threshold=None, write_threshold=None):
+    def __init__(self, data_width, base, depth, write_port, read_port):
         self.sink   = stream.Endpoint([("data", data_width)])
         self.source = stream.Endpoint([("data", data_width)])
 
         # # #
 
-        if read_threshold is None:
+        self.submodules.ctrl   = _LiteDRAMFIFOCtrl(base, depth)
-            read_threshold = 0
-        if write_threshold is None:
-            write_threshold = depth
-
-        self.submodules.ctrl   = _LiteDRAMFIFOCtrl(base, depth, read_threshold, write_threshold)
         self.submodules.writer = _LiteDRAMFIFOWriter(data_width, write_port, self.ctrl)
         self.submodules.reader = _LiteDRAMFIFOReader(data_width, read_port, self.ctrl)
         self.comb += [

litedram/gen.py

@@ -609,9 +609,7 @@ class LiteDRAMCore(SoCCore):
                     base            = port["base"],
                     depth           = port["depth"],
                     write_port      = self.sdram.crossbar.get_port("write"),
-                    write_threshold = port["depth"] - 32, # FIXME
                     read_port       = self.sdram.crossbar.get_port("read"),
-                    read_threshold  = 32 # FIXME
                 )
                 self.submodules += fifo
                 self.comb += [

test/test_fifo.py

@@ -16,22 +16,47 @@ from litedram.frontend.fifo import _LiteDRAMFIFOWriter, _LiteDRAMFIFOReader
 
 from test.common import *
 
-class TestFIFO(unittest.TestCase):
-    @passive
-    def fifo_ctrl_flag_checker(self, fifo_ctrl, write_threshold, read_threshold):
-        # Checks the combinational logic
-        while True:
-            level = (yield fifo_ctrl.level)
-            self.assertEqual((yield fifo_ctrl.writable), level < write_threshold)
-            self.assertEqual((yield fifo_ctrl.readable), level > read_threshold)
-            yield
 
+class FIFODUT(Module):
+    def __init__(self, base, depth, data_width=8, address_width=32):
+        self.write_port = LiteDRAMNativeWritePort(address_width=32, data_width=data_width)
+        self.read_port  = LiteDRAMNativeReadPort(address_width=32,  data_width=data_width)
+        self.submodules.fifo = LiteDRAMFIFO(
+            data_width = data_width,
+            base       = base,
+            depth      = depth,
+            write_port = self.write_port,
+            read_port  = self.read_port,
+        )
+
+        margin = 8
+        self.memory = DRAMMemory(data_width, base + depth + margin)
+
+    def write(self, data):
+        yield self.fifo.sink.valid.eq(1)
+        yield self.fifo.sink.data.eq(data)
+        yield
+        while not (yield self.fifo.sink.ready):
+            yield
+        yield self.fifo.sink.valid.eq(0)
+
+    def read(self):
+        while not (yield self.fifo.source.valid):
+            yield
+        yield self.fifo.source.ready.eq(1)
+        data = (yield self.fifo.source.data)
+        yield
+        yield self.fifo.source.ready.eq(0)
+        yield
+        return data
+
+
+class TestFIFO(unittest.TestCase):
     # _LiteDRAMFIFOCtrl ----------------------------------------------------------------------------
 
     def test_fifo_ctrl_address_changes(self):
         # Verify FIFOCtrl address changes.
-        # We are ignoring thresholds (so readable/writable signals)
+        dut = _LiteDRAMFIFOCtrl(base=0, depth=16)
-        dut = _LiteDRAMFIFOCtrl(base=0, depth=16, read_threshold=0, write_threshold=16)
 
         def main_generator():
             self.assertEqual((yield dut.write_address), 0)
@@ -58,20 +83,15 @@ class TestFIFO(unittest.TestCase):
             yield
             self.assertEqual((yield dut.read_address), 24 % 16)
 
-        generators = [
+        run_simulation(dut, main_generator())
-            main_generator(),
-            self.fifo_ctrl_flag_checker(dut, write_threshold=16, read_threshold=0),
-        ]
-        run_simulation(dut, generators)
 
     def test_fifo_ctrl_level_changes(self):
         # Verify FIFOCtrl level changes.
-        dut = _LiteDRAMFIFOCtrl(base=0, depth=16, read_threshold=0, write_threshold=16)
+        dut = _LiteDRAMFIFOCtrl(base=0, depth=16)
 
         def main_generator():
             self.assertEqual((yield dut.level), 0)
 
-            # Level
             def check_level_diff(write, read, diff):
                 level = (yield dut.level)
                 yield dut.write.eq(write)
@@ -89,21 +109,15 @@ class TestFIFO(unittest.TestCase):
             check_level_diff(write=0, read=1, diff=-1)
             check_level_diff(write=0, read=1, diff=-1)
 
-        generators = [
+        run_simulation(dut, main_generator())
-            main_generator(),
-            self.fifo_ctrl_flag_checker(dut, write_threshold=16, read_threshold=0),
-        ]
-        run_simulation(dut, generators)
 
     # _LiteDRAMFIFOWriter --------------------------------------------------------------------------
 
-    def fifo_writer_test(self, depth, sequence_len, write_threshold):
+    def fifo_writer_test(self, depth, sequence_len, consume=False):
         class DUT(Module):
             def __init__(self):
                 self.port = LiteDRAMNativeWritePort(address_width=32, data_width=32)
-                ctrl = _LiteDRAMFIFOCtrl(base=8, depth=depth,
+                ctrl = _LiteDRAMFIFOCtrl(base=8, depth=depth)
-                    read_threshold  = 0,
-                    write_threshold = write_threshold)
                 self.submodules.ctrl = ctrl
                 writer = _LiteDRAMFIFOWriter(data_width=32, port=self.port, ctrl=ctrl)
                 self.submodules.writer = writer
@@ -122,6 +136,9 @@ class TestFIFO(unittest.TestCase):
                     yield
                 yield dut.writer.sink.valid.eq(0)
 
+                if consume:
+                    yield dut.ctrl.read.eq(1)
+
             for _ in range(16):
                 yield
 
@@ -129,9 +146,6 @@ class TestFIFO(unittest.TestCase):
         generators = [
             generator(dut),
             dut.memory.write_handler(dut.port),
-            self.fifo_ctrl_flag_checker(dut.ctrl,
-                write_threshold = write_threshold,
-                read_threshold  = 0),
             timeout_generator(1500),
         ]
         run_simulation(dut, generators)
@@ -143,29 +157,27 @@ class TestFIFO(unittest.TestCase):
 
     def test_fifo_writer_sequence(self):
         # Verify simple FIFOWriter sequence.
-        self.fifo_writer_test(sequence_len=48, depth=64, write_threshold=64)
+        self.fifo_writer_test(sequence_len=48, depth=64)
+
+    def test_fifo_writer_stops_when_full(self):
+        # Verify FIFOWriter won't continue writing if noone reads the data.
+        with self.assertRaises(TimeoutError):
+            self.fifo_writer_test(sequence_len=48, depth=32)
 
     def test_fifo_writer_address_wraps(self):
-        # Verify FIFOWriter sequence with address wraps.
+        # Verify FIFOWriter address wraps.
-        self.fifo_writer_test(sequence_len=48, depth=32, write_threshold=64)
+        self.fifo_writer_test(sequence_len=48, depth=32, consume=True)
-
-    def test_fifo_writer_stops_after_threshold(self):
-        # Verify FIFOWriter sequence with stop after threshold is reached.
-        with self.assertRaises(TimeoutError):
-            self.fifo_writer_test(sequence_len=48, depth=32, write_threshold=32)
 
     # _LiteDRAMFIFOReader --------------------------------------------------------------------------
 
-    def fifo_reader_test(self, depth, sequence_len, read_threshold, inital_writes=0):
+    def fifo_reader_test(self, depth, sequence_len, inital_writes=0):
         memory_data = [seed_to_data(i) for i in range(128)]
         read_data   = []
 
         class DUT(Module):
             def __init__(self):
                 self.port = LiteDRAMNativeReadPort(address_width=32, data_width=32)
-                ctrl = _LiteDRAMFIFOCtrl(base=8, depth=depth,
+                ctrl = _LiteDRAMFIFOCtrl(base=8, depth=depth)
-                    read_threshold  = read_threshold,
-                    write_threshold = depth)
                 reader = _LiteDRAMFIFOReader(data_width=32, port=self.port, ctrl=ctrl)
                 self.submodules.ctrl = ctrl
                 self.submodules.reader = reader
@@ -199,9 +211,6 @@ class TestFIFO(unittest.TestCase):
         generators = [
             reader(dut),
             dut.memory.read_handler(dut.port),
-            self.fifo_ctrl_flag_checker(dut.ctrl,
-                write_threshold = depth,
-                read_threshold  = read_threshold),
             timeout_generator(1500),
         ]
         run_simulation(dut, generators)
@@ -211,94 +220,76 @@ class TestFIFO(unittest.TestCase):
 
     def test_fifo_reader_sequence(self):
         # Verify simple FIFOReader sequence.
-        self.fifo_reader_test(sequence_len=48, depth=64, read_threshold=0)
+        self.fifo_reader_test(sequence_len=48, depth=64)
 
     def test_fifo_reader_address_wraps(self):
         # Verify FIFOReader sequence with address wraps.
-        self.fifo_reader_test(sequence_len=48, depth=32, read_threshold=0)
+        self.fifo_reader_test(sequence_len=48, depth=32)
-
-    def test_fifo_reader_requires_threshold(self):
-        # Verify FIFOReader sequence with start after threshold is reached.
-        with self.assertRaises(TimeoutError):
-            self.fifo_reader_test(sequence_len=48, depth=32, read_threshold=8)
-        # Will work after we perform the initial writes
-        self.fifo_reader_test(sequence_len=48, depth=32, read_threshold=8, inital_writes=8)
 
     # LiteDRAMFIFO ---------------------------------------------------------------------------------
 
-    def test_fifo_default_thresholds(self):
+    def test_fifo_continuous_stream_short(self):
-        # Verify FIFO with default threshold.
+        # Verify FIFO operation with continuous writes and reads without wrapping
-        # Defaults: read_threshold=0, write_threshold=depth
+        def generator(dut):
-        read_threshold, write_threshold = (0, 128)
-        write_port = LiteDRAMNativeWritePort(address_width=32, data_width=32)
-        read_port  = LiteDRAMNativeReadPort(address_width=32,  data_width=32)
-        fifo = LiteDRAMFIFO(data_width=32, base=0, depth=write_threshold,
-            write_port = write_port,
-            read_port  = read_port)
-
-        def generator():
-            yield write_port.cmd.ready.eq(1)
-            yield write_port.wdata.ready.eq(1)
-            for i in range(write_threshold):
-                yield fifo.sink.valid.eq(1)
-                yield fifo.sink.data.eq(0)
-                yield
-                while (yield fifo.sink.ready) == 0:
-                    yield
-            yield
-
-        checker = self.fifo_ctrl_flag_checker(fifo.ctrl, write_threshold, read_threshold)
-        run_simulation(fifo, [generator(), checker])
-
-    def test_fifo(self):
-        # Verify FIFO.
-        class DUT(Module):
-            def __init__(self):
-                self.write_port = LiteDRAMNativeWritePort(address_width=32, data_width=32)
-                self.read_port  = LiteDRAMNativeReadPort(address_width=32,  data_width=32)
-                self.submodules.fifo = LiteDRAMFIFO(
-                    data_width          = 32,
-                    depth               = 32,
-                    base                = 16,
-                    write_port          = self.write_port,
-                    read_port           = self.read_port,
-                    read_threshold      = 8,
-                    write_threshold     = 32 - 8
-                )
-
-                self.memory = DRAMMemory(32, 128)
-
-        def generator(dut, valid_random=90):
-            prng = random.Random(42)
-            # We need 8 more writes to account for read_threshold=8
-            for i in range(64 + 8):
-                while prng.randrange(100) < valid_random:
-                    yield
-                yield dut.fifo.sink.valid.eq(1)
-                yield dut.fifo.sink.data.eq(i)
-                yield
-                while (yield dut.fifo.sink.ready) != 1:
-                    yield
-                yield dut.fifo.sink.valid.eq(0)
-
-        def checker(dut, ready_random=90):
-            prng = random.Random(42)
             for i in range(64):
-                yield dut.fifo.source.ready.eq(0)
+                yield from dut.write(10 + i)
-                yield
-                while (yield dut.fifo.source.valid) != 1:
-                    yield
-                while prng.randrange(100) < ready_random:
-                    yield
-                yield dut.fifo.source.ready.eq(1)
-                self.assertEqual((yield dut.fifo.source.data), i)
-                yield
 
-        dut = DUT()
+        def checker(dut):
+            for i in range(64):
+                data = (yield from dut.read())
+                self.assertEqual(data, 10 + i)
+
+        dut = FIFODUT(base=16, depth=128)
         generators = [
             generator(dut),
             checker(dut),
             dut.memory.write_handler(dut.write_port),
-            dut.memory.read_handler(dut.read_port)
+            dut.memory.read_handler(dut.read_port),
+            timeout_generator(1500),
+        ]
+        run_simulation(dut, generators)
+
+    def test_fifo_continuous_stream_long(self):
+        # Verify FIFO operation with continuous writes and reads with wrapping
+        def generator(dut):
+            for i in range(64):
+                yield from dut.write(10 + i)
+
+        def checker(dut):
+            for i in range(64):
+                data = (yield from dut.read())
+                self.assertEqual(data, 10 + i)
+
+        dut = FIFODUT(base=16, depth=32)
+        generators = [
+            generator(dut),
+            checker(dut),
+            dut.memory.write_handler(dut.write_port),
+            dut.memory.read_handler(dut.read_port),
+            timeout_generator(1500),
+        ]
+        run_simulation(dut, generators)
+
+    def test_fifo_delayed_reader(self):
+        # Verify FIFO works correctly when reader starts reading only after writer is full
+        def generator(dut):
+            for i in range(64):
+                yield from dut.write(10 + i)
+
+        def checker(dut):
+            # Wait until both the internal writer FIFO and our in-memory FIFO are full
+            while (yield dut.fifo.ctrl.writable):
+                yield
+            for i in range(64):
+                data = (yield from dut.read())
+                self.assertEqual(data, 10 + i)
+
+        dut = FIFODUT(base=16, depth=32)
+        generators = [
+            generator(dut),
+            checker(dut),
+            dut.memory.write_handler(dut.write_port),
+            dut.memory.read_handler(dut.read_port),
+            timeout_generator(1500),
         ]
         run_simulation(dut, generators)