From ba95c1a2d122f96f68f23aec2b9942c1b0d02e82 Mon Sep 17 00:00:00 2001 From: Luke Kenneth Casson Leighton Date: Thu, 6 Aug 2020 16:38:47 +0100 Subject: [PATCH] fix LDST PortInterface FSM interaction when the WB Bus delayed "ack", the LDST PI FSM was not "listening" and assumed that the operation had completed. a bit of a rewrite was required to get it to wait until LD/ST operations had actually fully completed --- src/soc/experiment/pi2ls.py | 32 ++++++++++++++++++++++++++--- src/soc/experiment/pimem.py | 41 +++++++++++++++++++++++++------------ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/soc/experiment/pi2ls.py b/src/soc/experiment/pi2ls.py index b31386b0..3c421c2e 100644 --- a/src/soc/experiment/pi2ls.py +++ b/src/soc/experiment/pi2ls.py @@ -41,6 +41,7 @@ class Pi2LSUI(PortInterfaceBase): if lsui is None: lsui = LoadStoreUnitInterface(addr_wid, self.addrbits, data_wid) self.lsui = lsui + self.lsui_busy = Signal() self.valid_l = SRLatch(False, name="valid") def set_wr_addr(self, m, addr, mask): @@ -55,10 +56,10 @@ class Pi2LSUI(PortInterfaceBase): def set_wr_data(self, m, data, wen): # mask already done in addr setup m.d.comb += self.lsui.x_st_data_i.eq(data) - return ~self.lsui.x_busy_o + return (~self.lsui_busy) def get_rd_data(self, m): - return self.lsui.m_ld_data_o, ~self.lsui.x_busy_o + return self.lsui.m_ld_data_o, ~self.lsui_busy def elaborate(self, platform): m = super().elaborate(platform) @@ -71,12 +72,37 @@ class Pi2LSUI(PortInterfaceBase): m.d.comb += lsui.x_ld_i.eq(pi.is_ld_i) m.d.comb += lsui.x_st_i.eq(pi.is_st_i) + # ooo how annoying. x_busy_o is set synchronously, i.e. one + # clock too late for this converter to "notice". consequently, + # when trying to wait for ld/st, here: on the first cycle + # it goes "oh, x_busy_o isn't set, the ld/st must have been + # completed already, we must be done" when in fact it hasn't + # started. to "fix" that we actually have to have a full FSM + # tracking from when LD/ST starts, right the way through. sigh. + # first clock busy signal. needed because x_busy_o is sync + with m.FSM() as fsm: + with m.State("IDLE"): + # detect when ld/st starts. set busy *immediately* + with m.If((pi.is_ld_i | pi.is_st_i) & self.valid_l.q): + m.d.comb += self.lsui_busy.eq(1) + m.next = "BUSY" + with m.State("BUSY"): + # detect when busy drops: must then wait for ld/st to end.. + #m.d.comb += self.lsui_busy.eq(self.lsui.x_busy_o) + m.d.comb += self.lsui_busy.eq(1) + with m.If(~self.lsui.x_busy_o): + m.next = "WAITDEASSERT" + with m.State("WAITDEASSERT"): + # when no longer busy: back to start + with m.If(~self.valid_l.q): + m.next = "IDLE" + # indicate valid at both ends m.d.comb += self.lsui.m_valid_i.eq(self.valid_l.q) m.d.comb += self.lsui.x_valid_i.eq(self.valid_l.q) # reset the valid latch when not busy - m.d.comb += self.valid_l.r.eq(~pi.busy_o) # self.lsui.x_busy_o) + m.d.comb += self.valid_l.r.eq(~self.lsui_busy)#~pi.busy_o) # self.lsui.x_busy_o) return m diff --git a/src/soc/experiment/pimem.py b/src/soc/experiment/pimem.py index 3626b2e5..279e8971 100644 --- a/src/soc/experiment/pimem.py +++ b/src/soc/experiment/pimem.py @@ -160,13 +160,20 @@ class PortInterfaceBase(Elaboratable): # state-machine latches m.submodules.st_active = st_active = SRLatch(False, name="st_active") + m.submodules.st_done = st_done = SRLatch(False, name="st_done") m.submodules.ld_active = ld_active = SRLatch(False, name="ld_active") m.submodules.reset_l = reset_l = SRLatch(True, name="reset") m.submodules.adrok_l = adrok_l = SRLatch(False, name="addr_acked") m.submodules.busy_l = busy_l = SRLatch(False, name="busy") m.submodules.cyc_l = cyc_l = SRLatch(True, name="cyc") + comb += st_done.s.eq(0) + comb += st_done.r.eq(0) + comb += st_active.r.eq(0) + comb += ld_active.r.eq(0) comb += cyc_l.s.eq(0) comb += cyc_l.r.eq(0) + comb += busy_l.s.eq(0) + comb += busy_l.r.eq(0) sync += adrok_l.s.eq(0) comb += adrok_l.r.eq(0) @@ -176,14 +183,22 @@ class PortInterfaceBase(Elaboratable): lds = Signal(reset_less=True) sts = Signal(reset_less=True) pi = self.pi - comb += lds.eq(pi.is_ld_i & pi.busy_o) # ld-req signals - comb += sts.eq(pi.is_st_i & pi.busy_o) # st-req signals + comb += lds.eq(pi.is_ld_i) # ld-req signals + comb += sts.eq(pi.is_st_i) # st-req signals - # activate mode - with m.If(lds): - comb += ld_active.s.eq(1) # activate LD mode - with m.Elif(sts): - comb += st_active.s.eq(1) # activate ST mode + # detect busy "edge" + busy_delay = Signal() + busy_edge = Signal() + sync += busy_delay.eq(pi.busy_o) + comb += busy_edge.eq(pi.busy_o & ~busy_delay) + + # activate mode: only on "edge" + comb += ld_active.s.eq(lds & busy_edge) # activate LD mode + comb += st_active.s.eq(sts & busy_edge) # activate ST mode + + # LD/ST requested activates "busy" (only if not already busy) + with m.If(self.pi.is_ld_i | self.pi.is_st_i): + comb += busy_l.s.eq(~busy_delay) # if now in "LD" mode: wait for addr_ok, then send the address out # to memory, acknowledge address, and send out LD data @@ -234,6 +249,8 @@ class PortInterfaceBase(Elaboratable): # TODO: replace with link to LoadStoreUnitInterface.x_store_data # and also handle the ready/stall/busy protocol stok = self.set_wr_data(m, stdata, lenexp.lexp_o) + comb += st_done.s.eq(1) # store done trigger + with m.If(st_done.q): comb += reset_l.s.eq(stok) # reset mode after 1 cycle # ugly hack, due to simultaneous addr req-go acknowledge @@ -248,17 +265,15 @@ class PortInterfaceBase(Elaboratable): comb += st_active.r.eq(1) # leave the ST active for 1 cycle comb += reset_l.r.eq(1) # clear reset comb += adrok_l.r.eq(1) # address reset - - # LD/ST requested activates "busy" - with m.If(self.pi.is_ld_i | self.pi.is_st_i): - comb += busy_l.s.eq(1) + comb += st_done.r.eq(1) # store done reset # monitor for an exception or the completion of LD. with m.If(self.pi.addr_exc_o): comb += busy_l.r.eq(1) # however ST needs one cycle before busy is reset - with m.If(self.pi.st.ok | self.pi.ld.ok): + #with m.If(self.pi.st.ok | self.pi.ld.ok): + with m.If(reset_l.s): comb += cyc_l.s.eq(1) with m.If(cyc_l.q): @@ -266,7 +281,7 @@ class PortInterfaceBase(Elaboratable): comb += busy_l.r.eq(1) # busy latch outputs to interface - comb += self.pi.busy_o.eq(busy_l.q) + comb += pi.busy_o.eq(busy_l.q) return m -- 2.30.2