Merge pull request #56 from pulp-platform/small-improvements

Small improvements to atop_filter, demux, id_prepend, and xbar

Author: andreaskurth

CHANGELOG.md

@@ -8,10 +8,19 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Unreleased
 
 ### Added
+- `axi_xbar_intf`: Add interface variant of crossbar.
 
 ### Changed
 
 ### Fixed
+- `axi_atop_filter`: Fix ModelSim warnings by adding `default` statement.  The signal in the `case`
+  has a single bit, and both values were correctly handled in synthesis.  However, when starting
+  simulation, the signal has an undefined value, and ModelSim threw warnings that this violated the
+  `unique` condition.
+- `axi_demux`: Move `typedef` outside `generate` for compatibility with VCS.
+- `axi_id_prepend`:
+  - Fix text of some assertion messages.
+  - Fix case of prepending a single-bit ID.
 - `tb_axi_xbar`: Fix for localparam `AxiIdWidthSlaves` to be dependent on the number of masters.
 
 

src/axi_atop_filter.sv

@@ -246,6 +246,8 @@ module axi_atop_filter #(
           end
         end
       end
+
+      default: r_state_d = R_FEEDTHROUGH;
     endcase
   end
   // Feed all signals on AR through.

src/axi_demux.sv

@@ -51,24 +51,25 @@ module axi_demux #(
 
   localparam int unsigned IdCounterWidth = $clog2(MaxTrans);
 
+  //--------------------------------------
+  // Typedefs for the FIFOs / Queues
+  //--------------------------------------
+  typedef logic [AxiIdWidth-1:0] axi_id_t;
+  typedef struct packed {
+    aw_chan_t aw_chan;
+    select_t  aw_select;
+  } aw_chan_select_t;
+  typedef struct packed {
+    ar_chan_t ar_chan;
+    select_t  ar_select;
+  } ar_chan_select_t;
+
   // pass through if only one master port
   if (NoMstPorts == 32'h1) begin : gen_no_demux
     assign mst_reqs_o[0] = slv_req_i;
     assign slv_resp_o    = mst_resps_i;
   // other non degenerate cases
   end else begin : gen_demux
-    //--------------------------------------
-    // Typedefs for the Fifo's / Queues
-    //--------------------------------------
-    typedef logic [AxiIdWidth-1:0] axi_id_t;
-    typedef struct packed {
-      aw_chan_t aw_chan;
-      select_t  aw_select;
-    } aw_chan_select_t;
-    typedef struct packed {
-      ar_chan_t ar_chan;
-      select_t  ar_select;
-    } ar_chan_select_t;
 
     //--------------------------------------
     //--------------------------------------

src/axi_id_prepend.sv

@@ -76,17 +76,22 @@ module axi_id_prepend #(
 );
 
   // prepend the ID
-  always_comb begin
-    for (int unsigned i = 0; i < NoBus; i++) begin : gen_id_prepend
-      mst_aw_chans_o[i]                                   = slv_aw_chans_i[i];
-      mst_aw_chans_o[i].id[AxiIdWidthSlvPort+:PreIdWidth] = pre_id_i;
-      mst_ar_chans_o[i]                                   = slv_ar_chans_i[i];
-      mst_ar_chans_o[i].id[AxiIdWidthSlvPort+:PreIdWidth] = pre_id_i;
-      // The ID is in the highest bits of the struct, so an assignment from a channel with a wide ID
-      // to a channel with a shorter ID correctly cuts the prepended ID.
-      slv_b_chans_o[i] = mst_b_chans_i[i];
-      slv_r_chans_o[i] = mst_r_chans_i[i];
+  for (genvar i = 0; i < NoBus; i++) begin : gen_id_prepend
+    if (PreIdWidth == 0) begin : gen_no_prepend
+      assign mst_aw_chans_o[i] = slv_aw_chans_i[i];
+      assign mst_ar_chans_o[i] = slv_ar_chans_i[i];
+    end else begin : gen_prepend
+      always_comb begin
+        mst_aw_chans_o[i] = slv_aw_chans_i[i];
+        mst_ar_chans_o[i] = slv_ar_chans_i[i];
+        mst_aw_chans_o[i].id = {pre_id_i, slv_aw_chans_i[i].id[AxiIdWidthSlvPort-1:0]};
+        mst_ar_chans_o[i].id = {pre_id_i, slv_ar_chans_i[i].id[AxiIdWidthSlvPort-1:0]};
+      end
     end
+    // The ID is in the highest bits of the struct, so an assignment from a channel with a wide ID
+    // to a channel with a shorter ID correctly cuts the prepended ID.
+    assign slv_b_chans_o[i] = mst_b_chans_i[i];
+    assign slv_r_chans_o[i] = mst_r_chans_i[i];
   end
 
   // assign the handshaking's and w channel
@@ -133,15 +138,15 @@ module axi_id_prepend #(
 
   ar_id   : assert final(
       mst_ar_chans_o[0].id[$bits(slv_ar_chans_i[0].id)-1:0] === slv_ar_chans_i[0].id)
-        else $fatal (1, "Something with the AW channel ID prepending went wrong.");
+        else $fatal (1, "Something with the AR channel ID prepending went wrong.");
   ar_addr : assert final(mst_ar_chans_o[0].addr === slv_ar_chans_i[0].addr)
-      else $fatal (1, "Something with the AW channel ID prepending went wrong.");
+      else $fatal (1, "Something with the AR channel ID prepending went wrong.");
   ar_len  : assert final(mst_ar_chans_o[0].len === slv_ar_chans_i[0].len)
-      else $fatal (1, "Something with the AW channel ID prepending went wrong.");
+      else $fatal (1, "Something with the AR channel ID prepending went wrong.");
   ar_size : assert final(mst_ar_chans_o[0].size === slv_ar_chans_i[0].size)
-      else $fatal (1, "Something with the AW channel ID prepending went wrong.");
+      else $fatal (1, "Something with the AR channel ID prepending went wrong.");
   ar_qos  : assert final(mst_ar_chans_o[0].qos === slv_ar_chans_i[0].qos)
-      else $fatal (1, "Something with the AW channel ID prepending went wrong.");
+      else $fatal (1, "Something with the AR channel ID prepending went wrong.");
 
   r_id    : assert final(mst_r_chans_i[0].id[$bits(slv_r_chans_o[0].id)-1:0] === slv_r_chans_o[0].id)
       else $fatal (1, "Something with the R channel ID stripping went wrong.");

src/axi_xbar.sv

@@ -220,3 +220,90 @@ module axi_xbar #(
   `endif
   // pragma translate_on
 endmodule
+
+`include "axi/assign.svh"
+`include "axi/typedef.svh"
+
+module axi_xbar_intf #(
+  parameter int unsigned AXI_USER_WIDTH =  0,
+  parameter axi_pkg::xbar_cfg_t Cfg     = '0,
+  parameter type rule_t                 = axi_pkg::xbar_rule_64_t
+) (
+  input  logic                                                    clk_i,
+  input  logic                                                    rst_ni,
+  input  logic                                                    test_i,
+  AXI_BUS.Slave                                                   slv_ports [Cfg.NoSlvPorts-1:0],
+  AXI_BUS.Master                                                  mst_ports [Cfg.NoMstPorts-1:0],
+  input  rule_t [Cfg.NoAddrRules-1:0]                             addr_map_i,
+  input  logic  [Cfg.NoSlvPorts-1:0]                              en_default_mst_port_i,
+  input  logic  [Cfg.NoSlvPorts-1:0][$clog2(Cfg.NoMstPorts)-1:0]  default_mst_port_i
+);
+
+  localparam int unsigned AxiIdWidthMstPorts = Cfg.AxiIdWidthSlvPorts + $clog2(Cfg.NoSlvPorts);
+
+  typedef logic [AxiIdWidthMstPorts     -1:0] id_mst_t;
+  typedef logic [Cfg.AxiIdWidthSlvPorts -1:0] id_slv_t;
+  typedef logic [Cfg.AxiAddrWidth       -1:0] addr_t;
+  typedef logic [Cfg.AxiDataWidth       -1:0] data_t;
+  typedef logic [Cfg.AxiDataWidth/8     -1:0] strb_t;
+  typedef logic [AXI_USER_WIDTH         -1:0] user_t;
+
+  `AXI_TYPEDEF_AW_CHAN_T ( mst_aw_chan_t, addr_t, id_mst_t,         user_t)
+  `AXI_TYPEDEF_AW_CHAN_T ( slv_aw_chan_t, addr_t, id_slv_t,         user_t)
+  `AXI_TYPEDEF_W_CHAN_T  (      w_chan_t, data_t,           strb_t, user_t)
+  `AXI_TYPEDEF_B_CHAN_T  (  mst_b_chan_t,         id_mst_t,         user_t)
+  `AXI_TYPEDEF_B_CHAN_T  (  slv_b_chan_t,         id_slv_t,         user_t)
+  `AXI_TYPEDEF_AR_CHAN_T ( mst_ar_chan_t, addr_t, id_mst_t,         user_t)
+  `AXI_TYPEDEF_AR_CHAN_T ( slv_ar_chan_t, addr_t, id_slv_t,         user_t)
+  `AXI_TYPEDEF_R_CHAN_T  (  mst_r_chan_t, data_t, id_mst_t,         user_t)
+  `AXI_TYPEDEF_R_CHAN_T  (  slv_r_chan_t, data_t, id_slv_t,         user_t)
+  `AXI_TYPEDEF_REQ_T     (     mst_req_t, mst_aw_chan_t, w_chan_t, mst_ar_chan_t)
+  `AXI_TYPEDEF_REQ_T     (     slv_req_t, slv_aw_chan_t, w_chan_t, slv_ar_chan_t)
+  `AXI_TYPEDEF_RESP_T    (    mst_resp_t,  mst_b_chan_t, mst_r_chan_t)
+  `AXI_TYPEDEF_RESP_T    (    slv_resp_t,  slv_b_chan_t, slv_r_chan_t)
+
+  mst_req_t   [Cfg.NoMstPorts-1:0]  mst_reqs;
+  mst_resp_t  [Cfg.NoMstPorts-1:0]  mst_resps;
+  slv_req_t   [Cfg.NoSlvPorts-1:0]  slv_reqs;
+  slv_resp_t  [Cfg.NoSlvPorts-1:0]  slv_resps;
+
+  for (genvar i = 0; i < Cfg.NoMstPorts; i++) begin : gen_assign_mst
+    `AXI_ASSIGN_FROM_REQ  (mst_ports[i], mst_reqs[i] );
+    `AXI_ASSIGN_TO_RESP   (mst_resps[i], mst_ports[i]);
+  end
+
+  for (genvar i = 0; i < Cfg.NoSlvPorts; i++) begin : gen_assign_slv
+    `AXI_ASSIGN_TO_REQ    (slv_reqs[i],  slv_ports[i]);
+    `AXI_ASSIGN_FROM_RESP (slv_ports[i], slv_resps[i]);
+  end
+
+  axi_xbar #(
+    .Cfg  (Cfg),
+    .slv_aw_chan_t  ( slv_aw_chan_t ),
+    .mst_aw_chan_t  ( mst_aw_chan_t ),
+    .w_chan_t       ( w_chan_t      ),
+    .slv_b_chan_t   ( slv_b_chan_t  ),
+    .mst_b_chan_t   ( mst_b_chan_t  ),
+    .slv_ar_chan_t  ( slv_ar_chan_t ),
+    .mst_ar_chan_t  ( mst_ar_chan_t ),
+    .slv_r_chan_t   ( slv_r_chan_t  ),
+    .mst_r_chan_t   ( mst_r_chan_t  ),
+    .slv_req_t      ( slv_req_t     ),
+    .slv_resp_t     ( slv_resp_t    ),
+    .mst_req_t      ( mst_req_t     ),
+    .mst_resp_t     ( mst_resp_t    ),
+    .rule_t         ( rule_t        )
+  ) i_xbar (
+    .clk_i,
+    .rst_ni,
+    .test_i,
+    .slv_ports_req_i  (slv_reqs ),
+    .slv_ports_resp_o (slv_resps),
+    .mst_ports_req_o  (mst_reqs ),
+    .mst_ports_resp_i (mst_resps),
+    .addr_map_i,
+    .en_default_mst_port_i,
+    .default_mst_port_i
+  );
+
+endmodule