From 79b54ca86fae01795452ff436db43a2d027f5611 Mon Sep 17 00:00:00 2001
From: David Anderson <dave@natulte.net>
Date: Sun, 8 Sep 2024 09:26:59 -0700
Subject: [PATCH] vram/MemArbiter: add a granted_port method to make downstream
 wiring easier

To implement the mux tree that feeds into RAM ports, we need to know the
port index of the grantee to be able to wire it up. In theory we could
dispense with the per-port grant signal, but keeping it around allows
each client to deal with local concerns separate from the port routing.
---
 vram/MemArbiter.bsv      | 97 +++++++++++++++++++++++-----------------
 vram/MemArbiter_Test.bsv | 22 ++++-----
 2 files changed, 67 insertions(+), 52 deletions(-)

diff --git a/vram/MemArbiter.bsv b/vram/MemArbiter.bsv
index 1363464..bf61806 100644
--- a/vram/MemArbiter.bsv
+++ b/vram/MemArbiter.bsv
@@ -14,6 +14,8 @@ typedef struct {
    addr addr;
 } MemArbiterOp#(type addr) deriving (Bits, Eq, FShow);
 
+// mem_ops_conflict reports whether memory accesses a and b would
+// cause undefined behavior if they proceed simultaneously.
 function Bool mem_ops_conflict(Maybe#(MemArbiterOp#(addr)) a, Maybe#(MemArbiterOp#(addr)) b)
    provisos(Eq#(addr));
 
@@ -35,34 +37,34 @@ interface MemArbiter#(numeric type num_clients, type addr);
    // ports allow clients to request memory access.
    interface Vector#(num_clients, MemArbiterServer#(addr)) ports;
 
+   // granted_port returns the index in ports of the client that is
+   // being granted its request.
+   method UInt#(TLog#(num_clients)) granted_port();
+
    // The following methods are to support arbiter chaining.
    //
-   // Suppose you're arbitrating access to a dual-port
-   // memory. Typically, such a memory specifies that if one port is
-   // writing to an address, the other must not concurrently read or
-   // write that same address. This means the arbiters attached to
-   // each memory port must cooperate to avoid simultaneously granting
+   // Suppose you're arbitrating access to a dual-port memory.
+   // Typically, such a memory specifies that if one port is writing
+   // to an address, the other must not concurrently read or write
+   // that same address. This means the arbiters attached to each
+   // memory port must cooperate to avoid simultaneously granting
    // conflicting requests from their clients.
    //
-   // Calling conflict prevents the arbiter from granting a concurrent
-   // request that would result in a write-write, read-write or
-   // write-read conflict. granted_op emits the operation that the
-   // arbiter is granting, if any.
+   // conflict_in supplies an already granted operation that this
+   // arbiter must avoid conflicting with. conflict_out emits the
+   // operation that the arbiter is granting, if any.
    //
-   // MemArbiter intances are Connectable: mkConnection(a, b) gives
-   // conflict priority to a. That is, b only grants requests that
-   // don't conflict with a's grant.
+   // mkConnection(firstArbiter, secondArbiter) gives conflict
+   // priority to firstArbiter. That is, secondArbiter only grants
+   // requests that don't conflict with grants made by firstArbiter.
    (* always_ready *)
-   method Action conflict(MemArbiterOp#(addr) conflict);
-   method MemArbiterOp#(addr) granted_op();
+   method Action conflict_in(MemArbiterOp#(addr) conflict);
+   method MemArbiterOp#(addr) conflict_out();
 endinterface
 
 instance Connectable#(MemArbiter#(m, addr), MemArbiter#(n, addr));
    module mkConnection(MemArbiter#(m, addr) a, MemArbiter#(n, addr) b, Empty ifc);
-      (* fire_when_enabled *)
-      rule forward_conflict;
-         b.conflict(a.granted_op);
-      endrule
+      mkConnection(a.conflict_out, b.conflict_in);
    endmodule
 endinstance
 
@@ -71,13 +73,14 @@ endinstance
 module mkPriorityMemArbiter(MemArbiter#(num_clients, addr))
    provisos (Bits#(addr, _),
              Eq#(addr),
-             Min#(num_clients, 1, 1));
+             Min#(num_clients, 1, 1),
+             Alias#(client_idx, UInt#(TLog#(num_clients))));
 
    Vector#(num_clients, RWire#(MemArbiterOp#(addr))) reqs <- replicateM(mkRWire());
    Wire#(Vector#(num_clients, Bool)) grants <- mkBypassWire();
 
-   RWire#(MemArbiterOp#(addr)) conflict_in <- mkRWire();
-   RWire#(MemArbiterOp#(addr)) granted_op_out <- mkRWire();
+   RWire#(MemArbiterOp#(addr)) conflict_op <- mkRWire();
+   RWire#(client_idx) granted_idx <- mkRWire();
 
    (* no_implicit_conditions, fire_when_enabled *)
    rule grant_requests;
@@ -86,11 +89,11 @@ module mkPriorityMemArbiter(MemArbiter#(num_clients, addr))
 
       for (Integer i=0; i<valueOf(num_clients); i=i+1) begin
          if (reqs[i].wget() matches tagged Valid .req &&&
-             !mem_ops_conflict(conflict_in.wget(), reqs[i].wget()) &&&
+             !mem_ops_conflict(conflict_op.wget(), reqs[i].wget()) &&&
              !done) begin
                        done = True;
                        grant[i] = True;
-                       granted_op_out.wset(req);
+                       granted_idx.wset(fromInteger(i));
                     end
       end
 
@@ -105,24 +108,29 @@ module mkPriorityMemArbiter(MemArbiter#(num_clients, addr))
                   endinterface);
 
    interface ports = _ifcs;
-   method conflict = conflict_in.wset;
-   method MemArbiterOp#(addr) granted_op() if (granted_op_out.wget() matches tagged Valid .op);
+   method client_idx granted_port() if (granted_idx.wget() matches tagged Valid .idx);
+      return idx;
+   endmethod
+   method MemArbiterOp#(addr) conflict_out() if (granted_idx.wget() matches tagged Valid .idx &&&
+                                                 reqs[idx].wget() matches tagged Valid .op);
       return op;
    endmethod
+   method conflict_in = conflict_op.wset;
 endmodule
 
 typedef struct {
    Vector#(n, Bool) grant_vec;
-   Maybe#(MemArbiterOp#(addr)) granted_op;
+   Maybe#(UInt#(TLog#(n))) granted_idx;
 } GrantResult#(numeric type n, type addr) deriving (Bits, Eq, FShow);
 
 // select_grant computes which one entry of requests should be
 // granted. Priority order is descending starting from
 // requests[hipri].
 function GrantResult#(n, addr) select_grant(Vector#(n, Maybe#(MemArbiterOp#(addr))) requests,
-                                            UInt#(TLog#(n)) hipri,
+                                            client_idx hipri,
                                             Maybe#(MemArbiterOp#(addr)) conflict)
-   provisos (Eq#(addr));
+   provisos (Eq#(addr),
+             Alias#(client_idx, UInt#(TLog#(n))));
 
    function onehot(idx);
       let ret = replicate(False);
@@ -131,13 +139,15 @@ function GrantResult#(n, addr) select_grant(Vector#(n, Maybe#(MemArbiterOp#(addr
    endfunction
 
    function GrantResult#(n, addr) do_fold(GrantResult#(n, addr) acc,
-                                          Tuple2#(UInt#(TLog#(n)),
+                                          Tuple2#(client_idx,
                                                   Maybe#(MemArbiterOp#(addr))) next);
       match {.idx, .mreq} = next;
-      if (mreq matches tagged Valid .req &&& acc.granted_op matches tagged Invalid &&& !mem_ops_conflict(conflict, mreq))
+      if (mreq matches tagged Valid .req &&&
+          acc.granted_idx matches tagged Invalid &&&
+          !mem_ops_conflict(conflict, mreq))
          return GrantResult{
             grant_vec: onehot(idx),
-            granted_op: tagged Valid req
+            granted_idx: tagged Valid idx
          };
       else
          // Previous grant won, not requesting, or request not satisfiable.
@@ -148,7 +158,7 @@ function GrantResult#(n, addr) select_grant(Vector#(n, Maybe#(MemArbiterOp#(addr
    let rot = rotateBy(in, fromInteger(valueOf(n)-1)-hipri+1);
    let seed = GrantResult{
       grant_vec: replicate(False),
-      granted_op: tagged Invalid
+      granted_idx: tagged Invalid
    };
    return foldl(do_fold, seed, rot);
 endfunction
@@ -156,19 +166,20 @@ endfunction
 module mkRoundRobinMemArbiter(MemArbiter#(num_clients, addr))
    provisos (Bits#(addr, _),
              Eq#(addr),
-             Min#(num_clients, 1, 1));
+             Min#(num_clients, 1, 1),
+             Alias#(client_idx, UInt#(TLog#(num_clients))));
 
    Vector#(num_clients, RWire#(MemArbiterOp#(addr))) reqs <- replicateM(mkRWire);
    Wire#(Vector#(num_clients, Bool)) grants <- mkBypassWire();
 
-   RWire#(MemArbiterOp#(addr)) conflict_in <- mkRWire();
-   RWire#(MemArbiterOp#(addr)) granted_op_out <- mkRWire();
+   RWire#(MemArbiterOp#(addr)) conflict_op <- mkRWire();
+   RWire#(client_idx) granted_idx_out <- mkRWire();
 
    // high_prio is the index of the client that should be first in
    // line to receive access. Every time we grant access to a client,
    // the one after that in sequence becomes high_prio in the next
    // round.
-   Reg#(UInt#(TLog#(num_clients))) high_prio <- mkReg(0);
+   Reg#(client_idx) high_prio <- mkReg(0);
 
    function Maybe#(_t) get_mreq(RWire#(_t) w);
       return w.wget();
@@ -176,11 +187,11 @@ module mkRoundRobinMemArbiter(MemArbiter#(num_clients, addr))
 
    rule grant;
       let in = map(get_mreq, reqs);
-      let res = select_grant(in, high_prio, conflict_in.wget());
+      let res = select_grant(in, high_prio, conflict_op.wget());
 
       grants <= res.grant_vec;
-      if (res.granted_op matches tagged Valid .op) begin
-         granted_op_out.wset(op);
+      if (res.granted_idx matches tagged Valid .idx) begin
+         granted_idx_out.wset(idx);
          high_prio <= validValue(findElem(True, rotateR(res.grant_vec)));
       end
    endrule
@@ -193,10 +204,14 @@ module mkRoundRobinMemArbiter(MemArbiter#(num_clients, addr))
                   endinterface);
 
    interface ports = _ifcs;
-   method conflict = conflict_in.wset;
-   method MemArbiterOp#(addr) granted_op() if (granted_op_out.wget() matches tagged Valid .op);
+   method client_idx granted_port() if (granted_idx_out.wget() matches tagged Valid .idx);
+      return idx;
+   endmethod
+   method MemArbiterOp#(addr) conflict_out() if (granted_idx_out.wget() matches tagged Valid .idx &&&
+                                                reqs[idx].wget() matches tagged Valid .op);
       return op;
    endmethod
+   method conflict_in = conflict_op.wset;
 endmodule
 
 endpackage
diff --git a/vram/MemArbiter_Test.bsv b/vram/MemArbiter_Test.bsv
index 1b2debd..fca0888 100644
--- a/vram/MemArbiter_Test.bsv
+++ b/vram/MemArbiter_Test.bsv
@@ -19,7 +19,7 @@ typedef struct {
    Maybe#(MemArbiterOp#(Addr)) conflict;
 
    Vector#(n, Bool) want_grants;
-   Maybe#(MemArbiterOp#(Addr)) want_granted_op;
+   Maybe#(MemArbiterOp#(Addr)) want_conflict_out;
 } TestCase#(numeric type n) deriving (Bits, Eq);
 
 function Maybe#(MemArbiterOp#(Addr)) read(Addr addr);
@@ -54,13 +54,13 @@ function TestCase#(n) testCase(String name,
                                Vector#(n, Maybe#(MemArbiterOp#(Addr))) reqs,
                                Maybe#(MemArbiterOp#(Addr)) conflict,
                                Vector#(n, Bool) want_grants,
-                               Maybe#(MemArbiterOp#(Addr)) want_granted_op);
+                               Maybe#(MemArbiterOp#(Addr)) want_conflict_out);
    return TestCase{
       name: name,
       reqs: reqs,
       conflict: conflict,
       want_grants: want_grants,
-      want_granted_op: want_granted_op
+      want_conflict_out: want_conflict_out
    };
 endfunction
 
@@ -84,14 +84,14 @@ module mkArbiterTB(MemArbiter#(n, Addr) dut, Vector#(m, TestCase#(n)) tests, TB
 
    (* no_implicit_conditions, fire_when_enabled *)
    rule forbid (running && isValid(tests[idx].conflict));
-      dut.conflict(validValue(tests[idx].conflict));
+      dut.conflict_in(validValue(tests[idx].conflict));
    endrule
 
-   Wire#(Maybe#(MemArbiterOp#(Addr))) got_granted_op <- mkDWire(tagged Invalid);
+   Wire#(Maybe#(MemArbiterOp#(Addr))) got_conflict_out <- mkDWire(tagged Invalid);
 
    (* fire_when_enabled *)
-   rule collect_granted_op (running);
-      got_granted_op <= tagged Valid dut.granted_op();
+   rule collect_conflict_out (running);
+      got_conflict_out <= tagged Valid dut.conflict_out();
    endrule
 
    function Fmt req_s(Maybe#(MemArbiterOp#(Addr)) v);
@@ -107,13 +107,13 @@ module mkArbiterTB(MemArbiter#(n, Addr) dut, Vector#(m, TestCase#(n)) tests, TB
       let test = tests[idx];
       let reqs = test.reqs;
       let want_grants = test.want_grants;
-      let want_granted_op = test.want_granted_op;
+      let want_conflict_out = test.want_conflict_out;
       Vector#(n, Bool) got_grants = newVector;
       for (Integer i=0; i<valueOf(n); i=i+1)
          got_grants[i] = dut.ports[i].grant();
 
       $display("RUN %s (%0d)", tests[idx].name, idx);
-      if (got_grants != want_grants || got_granted_op != want_granted_op) begin
+      if (got_grants != want_grants || got_conflict_out != want_conflict_out) begin
          $display("input:");
          for (Integer i=0; i<valueOf(n); i=i+1)
             $display("    ", $format("%0d", i), ": ", req_s(reqs[i]));
@@ -121,10 +121,10 @@ module mkArbiterTB(MemArbiter#(n, Addr) dut, Vector#(m, TestCase#(n)) tests, TB
 
          $display("  output:");
          $display("    grants: ", fshow(got_grants));
-         $display("    granted: ", fshow(got_granted_op));
+         $display("    granted: ", fshow(got_conflict_out));
 
          $display("  want grants: ", fshow(tests[idx].want_grants));
-         $display("  want granted: ", fshow(want_granted_op));
+         $display("  want granted: ", fshow(want_conflict_out));
          dynamicAssert(False, "wrong arbiter output");
       end