From da769094fdf8d336e9b328e2834e6d37e7bc4706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Boczar?= Date: Tue, 1 Jun 2021 12:50:01 +0200 Subject: [PATCH] phy/lpddr4: fix edge case error with CommandsPipeline ignoring a command Command was being ignored when it occurred on the last phase and the next command would invalidate the first phase. Now it is fixed and a regression test is included. A fix in ConstBitSlip has been added due to wrong Verilog being generated with cycles=1, register=False. --- litedram/phy/utils.py | 27 +++++++++++++++------------ test/test_lpddr4.py | 18 ++++++++++-------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/litedram/phy/utils.py b/litedram/phy/utils.py index 5691f0a..22215cc 100644 --- a/litedram/phy/utils.py +++ b/litedram/phy/utils.py @@ -59,7 +59,11 @@ class ConstBitSlip(Module): self.sync += r.eq(Cat(r[dw:], self.i)) else: reg = Signal(cycles*dw, reset_less=True) - self.sync += reg.eq(Cat(reg[dw:], self.i)) + # Cat with slice of len=0 generates incorrect Verilog + if len(reg[dw:]) > 0: + self.sync += reg.eq(Cat(reg[dw:], self.i)) + else: + self.sync += reg.eq(self.i) self.comb += r.eq(Cat(reg, self.i)) self.comb += self.o.eq(r[slp+1:dw+slp+1]) @@ -191,6 +195,7 @@ class CommandsPipeline(Module): nphases = len(adapters) self.cs = Signal(cs_ser_width) self.ca = [Signal(ca_ser_width) for _ in range(ca_nbits)] + assert cmd_nphases_span <= nphases # # # @@ -198,7 +203,7 @@ class CommandsPipeline(Module): n_previous = cmd_nphases_span - 1 # Create a history of valid adapters used for masking overlapping ones - valids = ConstBitSlip(dw=nphases, slp=0) + valids = ConstBitSlip(dw=nphases, slp=0, cycles=1, register=False) self.submodules += valids self.comb += valids.i.eq(Cat(a.valid for a in adapters)) valids_hist = valids.r @@ -216,23 +221,21 @@ class CommandsPipeline(Module): allowed = ~reduce(or_, valids_hist[nphases+phase - n_previous:nphases+phase]) # Use CS and CA of given adapter slipped by `phase` bits - cs_bs = ConstBitSlip(dw=cs_ser_width, slp=phase) + cs_bs = ConstBitSlip(dw=cs_ser_width, slp=phase, cycles=1) self.submodules += cs_bs - self.comb += cs_bs.i.eq(Cat(adapter.cs)), - cs_mask = Replicate(allowed, len(cs_bs.o)) - cs = cs_bs.o & cs_mask - cs_per_adapter.append(cs) + cs_mask = Replicate(allowed, len(cs_bs.i)) + self.comb += cs_bs.i.eq(Cat(adapter.cs) & cs_mask), + cs_per_adapter.append(cs_bs.o) # For CA we need to do the same for each bit ca_bits = [] for bit in range(ca_nbits): - ca_bs = ConstBitSlip(dw=ca_ser_width, slp=phase) + ca_bs = ConstBitSlip(dw=ca_ser_width, slp=phase, cycles=1) self.submodules += ca_bs - ca_bit_hist = [adapter.ca[i][bit] for i in range(cmd_nphases_span)] - self.comb += ca_bs.i.eq(Cat(*ca_bit_hist)), + ca_bit_hist = [ca[bit] for ca in adapter.ca] ca_mask = Replicate(allowed, len(ca_bs.o)) - ca = ca_bs.o & ca_mask - ca_per_adapter[bit].append(ca) + self.comb += ca_bs.i.eq(Cat(*ca_bit_hist) & ca_mask), + ca_per_adapter[bit].append(ca_bs.o) # OR all the masked signals self.comb += self.cs.eq(reduce(or_, cs_per_adapter)) diff --git a/test/test_lpddr4.py b/test/test_lpddr4.py index 85d3cf1..1d2c3a2 100644 --- a/test/test_lpddr4.py +++ b/test/test_lpddr4.py @@ -149,19 +149,21 @@ class LPDDR4Tests(unittest.TestCase): read = dict(cs_n=0, cas_n=0, ras_n=1, we_n=1) self.run_test(LPDDR4SimPHY(sys_clk_freq=self.SYS_CLK_FREQ), dfi_sequence = [ - {0: read, 3: read}, # p4 should be ignored + {0: read, 3: read}, # p3 should be ignored {0: read, 4: read}, {6: read}, {0: read}, # ignored + {7: read}, + {3: read}, # not ignored ], pad_checkers = {"sys8x_90": { - 'cs': latency + '10100000' + '10101010' + '00000010' + '10000000', - 'ca0': latency + '00000000' + '00000000' + '00000000' + '00000000', - 'ca1': latency + '10100000' + '10101010' + '00000010' + '10000000', - 'ca2': latency + '00000000' + '00000000' + '00000000' + '00000000', - 'ca3': latency + '0x000000' + '0x000x00' + '0000000x' + '00000000', - 'ca4': latency + '00100000' + '00100010' + '00000000' + '10000000', - 'ca5': latency + '00000000' + '00000000' + '00000000' + '00000000', + 'cs': latency + '10100000' + '10101010' + '00000010' + '10000000' + '00000001' + '01010100', + 'ca0': latency + '00000000' + '00000000' + '00000000' + '00000000' + '00000000' + '00000000', + 'ca1': latency + '10100000' + '10101010' + '00000010' + '10000000' + '00000001' + '01010100', + 'ca2': latency + '00000000' + '00000000' + '00000000' + '00000000' + '00000000' + '00000000', + 'ca3': latency + '0x000000' + '0x000x00' + '0000000x' + '00000000' + '00000000' + 'x000x000', + 'ca4': latency + '00100000' + '00100010' + '00000000' + '10000000' + '00000000' + '01000100', + 'ca5': latency + '00000000' + '00000000' + '00000000' + '00000000' + '00000000' + '00000000', }}, )