From 214cfdcaebaa4ab7a94b6a4308a4f07e8da7454e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Boczar?= Date: Mon, 20 Jul 2020 18:08:56 +0200 Subject: [PATCH] soc/interconnect/axi: lock AXILiteArbiter until all requests have been responded to --- litex/soc/interconnect/axi.py | 49 +++++++++++++++++++--- test/test_axi.py | 79 +++++++++++++++++++++++++++++------ 2 files changed, 111 insertions(+), 17 deletions(-) diff --git a/litex/soc/interconnect/axi.py b/litex/soc/interconnect/axi.py index 14ffe0303..40fae8be5 100644 --- a/litex/soc/interconnect/axi.py +++ b/litex/soc/interconnect/axi.py @@ -936,15 +936,40 @@ class AXILiteInterconnectPointToPoint(Module): def __init__(self, master, slave): self.comb += master.connect(slave) + +class AXILiteRequestCounter(Module): + def __init__(self, request, response, max_requests=256): + self.counter = counter = Signal(max=max_requests) + self.full = full = Signal() + self.empty = empty = Signal() + self.stall = stall = Signal() + + self.comb += [ + full.eq(counter == max_requests - 1), + empty.eq(counter == 0), + stall.eq(request & full), + ] + + self.sync += [ + If(request & response, + counter.eq(counter) + ).Elif(request & ~full, + counter.eq(counter + 1) + ).Elif(response & ~empty, + counter.eq(counter - 1) + ), + ] + class AXILiteArbiter(Module): """AXI Lite arbiter Arbitrate between master interfaces and connect one to the target. - Arbitration is done separately for write and read channels. + New master will not be selected until all requests have been responded to. + Arbitration for write and read channels is done separately. """ def __init__(self, masters, target): - self.submodules.rr_write = roundrobin.RoundRobin(len(masters)) - self.submodules.rr_read = roundrobin.RoundRobin(len(masters)) + self.submodules.rr_write = roundrobin.RoundRobin(len(masters), roundrobin.SP_CE) + self.submodules.rr_read = roundrobin.RoundRobin(len(masters), roundrobin.SP_CE) def get_sig(interface, channel, name): return getattr(getattr(interface, channel), name) @@ -968,9 +993,23 @@ class AXILiteArbiter(Module): else: self.comb += dest.eq(source) + # allow to change rr.grant only after all requests from a master have been responded to + self.submodules.wr_counter = wr_counter = AXILiteRequestCounter( + request=target.aw.valid & target.aw.ready, response=target.b.valid & target.b.ready) + self.submodules.rd_counter = rd_counter = AXILiteRequestCounter( + request=target.ar.valid & target.ar.ready, response=target.r.valid & target.r.ready) + + # switch to next request only if there are no responses pending + self.comb += [ + self.rr_write.ce.eq(~(target.aw.valid | target.w.valid | target.b.valid) & wr_counter.empty), + self.rr_read.ce.eq(~(target.ar.valid | target.r.valid) & rd_counter.empty), + ] + # connect bus requests to round-robin selectors - self.comb += self.rr_write.request.eq(Cat(*[m.aw.valid | m.w.valid | m.b.valid for m in masters])) - self.comb += self.rr_read.request.eq(Cat(*[m.ar.valid | m.r.valid for m in masters])) + self.comb += [ + self.rr_write.request.eq(Cat(*[m.aw.valid | m.w.valid | m.b.valid for m in masters])), + self.rr_read.request.eq(Cat(*[m.ar.valid | m.r.valid for m in masters])), + ] class AXILiteDecoder(Module): # slaves is a list of pairs: diff --git a/test/test_axi.py b/test/test_axi.py index e0b81af22..ee5d4107d 100644 --- a/test/test_axi.py +++ b/test/test_axi.py @@ -330,32 +330,34 @@ class TestAXI(unittest.TestCase): # TestAXILite -------------------------------------------------------------------------------------- class AXILiteChecker: - def __init__(self, latency=None, rdata_generator=None): - self.latency = latency or (lambda: 0) + def __init__(self, ready_latency=None, response_latency=None, rdata_generator=None): + self.ready_latency = ready_latency or (lambda: 0) + self.response_latency = response_latency or (lambda: 0) self.rdata_generator = rdata_generator or (lambda adr: 0xbaadc0de) self.writes = [] # (addr, data, strb) self.reads = [] # (addr, data) - def delay(self): - for _ in range(self.latency()): + def delay(self, latency): + for _ in range(latency()): yield def handle_write(self, axi_lite): while not (yield axi_lite.aw.valid): yield - yield from self.delay() + yield from self.delay(self.ready_latency) addr = (yield axi_lite.aw.addr) yield axi_lite.aw.ready.eq(1) yield yield axi_lite.aw.ready.eq(0) while not (yield axi_lite.w.valid): yield - yield from self.delay() + yield from self.delay(self.ready_latency) data = (yield axi_lite.w.data) strb = (yield axi_lite.w.strb) yield axi_lite.w.ready.eq(1) yield yield axi_lite.w.ready.eq(0) + yield from self.delay(self.response_latency) yield axi_lite.b.valid.eq(1) yield axi_lite.b.resp.eq(RESP_OKAY) yield @@ -367,11 +369,12 @@ class AXILiteChecker: def handle_read(self, axi_lite): while not (yield axi_lite.ar.valid): yield - yield from self.delay() + yield from self.delay(self.ready_latency) addr = (yield axi_lite.ar.addr) yield axi_lite.ar.ready.eq(1) yield yield axi_lite.ar.ready.eq(0) + yield from self.delay(self.response_latency) data = self.rdata_generator(addr) yield axi_lite.r.valid.eq(1) yield axi_lite.r.resp.eq(RESP_OKAY) @@ -560,7 +563,7 @@ class TestAXILite(unittest.TestCase): return _latency dut = DUT(width_from=width_from, width_to=width_to) - checker = AXILiteChecker(latency, rdata_generator) + checker = AXILiteChecker(ready_latency=latency, rdata_generator=rdata_generator) run_simulation(dut, [generator(dut.master), checker.handler(dut.slave)]) self.assertEqual(checker.writes, write_expected) self.assertEqual(checker.reads, read_expected) @@ -644,6 +647,9 @@ class TestAXILite(unittest.TestCase): self.converter_test(width_from=32, width_to=16, write_pattern=write_pattern, write_expected=write_expected) +# TestAXILiteInterconnet --------------------------------------------------------------------------- + +class TestAXILiteInterconnect(unittest.TestCase): def axilite_pattern_generator(self, axi_lite, pattern): for rw, addr, data in pattern: assert rw in ["w", "r"] @@ -657,7 +663,7 @@ class TestAXILite(unittest.TestCase): for _ in range(16): yield - def test_axilite_interconnect_p2p(self): + def test_interconnect_p2p(self): class DUT(Module): def __init__(self): self.master = master = AXILiteInterface() @@ -687,7 +693,7 @@ class TestAXILite(unittest.TestCase): self.assertEqual(checker.writes, [(addr, data, 0b1111) for rw, addr, data in pattern if rw == "w"]) self.assertEqual(checker.reads, [(addr, data) for rw, addr, data in pattern if rw == "r"]) - def test_axilite_timeout(self): + def test_timeout(self): class DUT(Module): def __init__(self): self.master = master = AXILiteInterface() @@ -727,7 +733,7 @@ class TestAXILite(unittest.TestCase): ] run_simulation(dut, generators) - def test_axilite_arbiter(self): + def test_arbiter_order(self): class DUT(Module): def __init__(self, n_masters): self.masters = [AXILiteInterface() for _ in range(n_masters)] @@ -759,7 +765,7 @@ class TestAXILite(unittest.TestCase): checker = AXILiteChecker() generators = [generator(i, master, delay=0) for i, master in enumerate(dut.masters)] generators += [timeout(300), checker.handler(dut.slave)] - run_simulation(dut, generators, vcd_name='sim.vcd') + run_simulation(dut, generators) order = [0, 1, 2, 3, 100, 101, 102, 103, 200, 201, 202, 203] self.assertEqual([addr for addr, data, strb in checker.writes], order) self.assertEqual([addr for addr, data in checker.reads], order) @@ -774,3 +780,52 @@ class TestAXILite(unittest.TestCase): order = [0, 100, 200, 1, 101, 201, 2, 102, 202, 3, 103, 203] self.assertEqual([addr for addr, data, strb in checker.writes], order) self.assertEqual([addr for addr, data in checker.reads], order) + + def test_arbiter_holds_grant_until_response(self): + class DUT(Module): + def __init__(self, n_masters): + self.masters = [AXILiteInterface() for _ in range(n_masters)] + self.slave = AXILiteInterface() + self.submodules.arbiter = AXILiteArbiter(self.masters, self.slave) + + def generator(n, axi_lite, delay=0): + def gen(i): + return 100*n + i + + for i in range(4): + resp = (yield from axi_lite.write(gen(i), gen(i))) + self.assertEqual(resp, RESP_OKAY) + for _ in range(delay): + yield + for i in range(4): + data, resp = (yield from axi_lite.read(gen(i))) + self.assertEqual(resp, RESP_OKAY) + for _ in range(delay): + yield + for _ in range(8): + yield + + + n_masters = 3 + + # with no delay each master will do all transfers at once + with self.subTest(delay=0): + dut = DUT(n_masters) + checker = AXILiteChecker(response_latency=lambda: 3) + generators = [generator(i, master, delay=0) for i, master in enumerate(dut.masters)] + generators += [timeout(300), checker.handler(dut.slave)] + run_simulation(dut, generators) + order = [0, 1, 2, 3, 100, 101, 102, 103, 200, 201, 202, 203] + self.assertEqual([addr for addr, data, strb in checker.writes], order) + self.assertEqual([addr for addr, data in checker.reads], order) + + # with some delay, the round-robin arbiter will iterate over masters + with self.subTest(delay=1): + dut = DUT(n_masters) + checker = AXILiteChecker(response_latency=lambda: 3) + generators = [generator(i, master, delay=1) for i, master in enumerate(dut.masters)] + generators += [timeout(300), checker.handler(dut.slave)] + run_simulation(dut, generators, vcd_name='sim.vcd') + order = [0, 100, 200, 1, 101, 201, 2, 102, 202, 3, 103, 203] + self.assertEqual([addr for addr, data, strb in checker.writes], order) + self.assertEqual([addr for addr, data in checker.reads], order)