Merge pull request pulp-platform#268 from pulp-platform/xbar_pipeline…

…-tbenz

Add option to enable internal pipelining to `xbar`.

CHANGELOG.md

@@ -11,7 +11,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Add `axi_channel_compare.sv`: Non-synthesizable module comparing two AXI channels of the same type
 
 ### Changed
-- `axi_demux`: Replace write FIFO (`w_fifo`) with a write credit counter.
+- `axi_demux`: Replace FIFO between AW and W channel by a register plus a counter.  This prevents
+  AWs from being issued to one master port while Ws from another burst are ongoing to another
+  master port.  This is required to prevents deadlocks due to circular waits downstream.
+- `axi_xbar`: Add parameter `PipelineStages` to `axi_pkg::xbar_cfg_t`.  This adds `axi_multicuts`
+  in the crossed connections in the xbar between the demuxes and muxes.
+- `axi_pkg`: Add documentation to `xbar_cfg_t`.
 
 ### Fixed
 

scripts/run_vsim.sh

@@ -172,7 +172,7 @@ exec_test() {
                     for Atop in 0 1; do
                         for Exclusive in 0 1; do
                             for UniqueIds in 0 1; do
-                                call_vsim tb_axi_xbar -gTbNumMst=$NumMst -gTbNumSlv=$NumSlv \
+                                call_vsim tb_axi_xbar -gTbNumMasters=$NumMst -gTbNumSlaves=$NumSlv \
                                         -gTbEnAtop=$Atop -gTbEnExcl=$Exclusive \
                                         -gTbUniqueIds=$UniqueIds
                             done
@@ -202,6 +202,29 @@ exec_test() {
                 done
             done
             ;;
+        axi_xbar)
+            for GEN_ATOP in 0 1; do
+                for NUM_MST in 1 6; do
+                    for NUM_SLV in 2 9; do
+                        for MST_ID_USE in 3 5; do
+                            MST_ID=5
+                            for DATA_WIDTH in 64 256; do
+                                for PIPE in 0 1; do
+                                    call_vsim tb_axi_xbar -t 1ns -voptargs="+acc" \
+                                        -gTbNumMasters=$NUM_MST       \
+                                        -gTbNumSlaves=$NUM_SLV        \
+                                        -gTbAxiIdWidthMasters=$MST_ID \
+                                        -gTbAxiIdUsed=$MST_ID_USE     \
+                                        -gTbAxiDataWidth=$DATA_WIDTH  \
+                                        -gTbPipeline=$PIPE            \
+                                        -gTbEnAtop=$GEN_ATOP
+                                done
+                            done
+                        done
+                    done
+                done
+            done
+            ;;
         *)
             call_vsim tb_$1 -t 1ns -coverage -voptargs="+acc +cover=bcesfx"
             ;;

src/axi_demux.sv

@@ -289,10 +289,10 @@ module axi_demux #(
         if (!aw_id_cnt_full && (w_open != {IdCounterWidth{1'b1}}) &&
             (!(ar_id_cnt_full && slv_aw_chan.atop[axi_pkg::ATOP_R_RESP]) ||
              !AtopSupport)) begin
-          /// There is a valid AW vector make the id lookup and go further, if it passes.
-          /// Also stall if previous transmitted AWs still have active W's in flight.
-          /// This prevents deadlocking of the W channel. The counters are there for the
-          /// Handling of the B responses.
+          // There is a valid AW vector make the id lookup and go further, if it passes.
+          // Also stall if previous transmitted AWs still have active W's in flight.
+          // This prevents deadlocking of the W channel. The counters are there for the
+          // Handling of the B responses.
           if (slv_aw_valid &&
                 ((w_open == '0) || (w_select == slv_aw_select)) &&
                 (!aw_select_occupied || (slv_aw_select == lookup_aw_select))) begin
@@ -817,7 +817,6 @@ module axi_demux_intf #(
   parameter int unsigned MAX_TRANS        = 32'd8,
   parameter int unsigned AXI_LOOK_BITS    = 32'd3,
   parameter bit          UNIQUE_IDS       = 1'b0,
-  parameter bit          FALL_THROUGH     = 1'b0,
   parameter bit          SPILL_AW         = 1'b1,
   parameter bit          SPILL_W          = 1'b0,
   parameter bit          SPILL_B          = 1'b0,

src/axi_pkg.sv

@@ -393,17 +393,44 @@ package axi_pkg;
 
   /// Configuration for `axi_xbar`.
   typedef struct packed {
+    /// Number of slave ports of the crossbar.
+    /// This many master modules are connected to it.
     int unsigned   NoSlvPorts;
+    /// Number of master ports of the crossbar.
+    /// This many slave modules are connected to it.
     int unsigned   NoMstPorts;
+    /// Maximum number of open transactions each master connected to the crossbar can have in
+    /// flight at the same time.
     int unsigned   MaxMstTrans;
+    /// Maximum number of open transactions each slave connected to the crossbar can have in
+    /// flight at the same time.
     int unsigned   MaxSlvTrans;
+    /// Determine if the internal FIFOs of the crossbar are instantiated in fallthrough mode.
+    /// 0: No fallthrough
+    /// 1: Fallthrough
     bit            FallThrough;
+    /// The Latency mode of the xbar. This determines if the channels on the ports have
+    /// a spill register instantiated.
+    /// Example configurations are provided with the enum `xbar_latency_e`.
     xbar_latency_e LatencyMode;
+    /// This is the number of `axi_multicut` stages instantiated in the line cross of the channels.
+    /// Having multiple stages can potentially add a large number of FFs!
+    int unsigned   PipelineStages;
+    /// AXI ID width of the salve ports. The ID width of the master ports is determined
+    /// Automatically. See `axi_mux` for details.
     int unsigned   AxiIdWidthSlvPorts;
+    /// The used ID portion to determine if a different salve is used for the same ID.
+    /// See `axi_demux` for details.
     int unsigned   AxiIdUsedSlvPorts;
+    /// Are IDs unique?
     bit            UniqueIds;
+    /// AXI4+ATOP address field width.
     int unsigned   AxiAddrWidth;
+    /// AXI4+ATOP data field width.
     int unsigned   AxiDataWidth;
+    /// The number of address rules defined for routing of the transactions.
+    /// Each master port can have multiple rules, should have however at least one.
+    /// If a transaction can not be routed the xbar will answer with an `axi_pkg::RESP_DECERR`.
     int unsigned   NoAddrRules;
   } xbar_cfg_t;
 

src/axi_xbar.sv

@@ -13,50 +13,94 @@
 // - Andreas Kurth <[email protected]>
 // - Florian Zaruba <[email protected]>
 
-// axi_xbar: Fully-connected AXI4+ATOP crossbar with an arbitrary number of slave and master ports.
-// See `doc/axi_xbar.md` for the documentation, including the definition of parameters and ports.
+/// axi_xbar: Fully-connected AXI4+ATOP crossbar with an arbitrary number of slave and master ports.
+/// See `doc/axi_xbar.md` for the documentation, including the definition of parameters and ports.
 module axi_xbar
 import cf_math_pkg::idx_width;
 #(
+  /// Configuration struct for the crossbar see `axi_pkg` for fields and definitions.
   parameter axi_pkg::xbar_cfg_t Cfg                                   = '0,
+  /// Enable atomic operations support.
   parameter bit  ATOPs                                                = 1'b1,
+  /// Connectivity matrix
   parameter bit [Cfg.NoSlvPorts-1:0][Cfg.NoMstPorts-1:0] Connectivity = '1,
+  /// AXI4+ATOP AW channel struct type for the slave ports.
   parameter type slv_aw_chan_t                                        = logic,
+  /// AXI4+ATOP AW channel struct type for the master ports.
   parameter type mst_aw_chan_t                                        = logic,
+  /// AXI4+ATOP W channel struct type for all ports.
   parameter type w_chan_t                                             = logic,
+  /// AXI4+ATOP B channel struct type for the slave ports.
   parameter type slv_b_chan_t                                         = logic,
+  /// AXI4+ATOP B channel struct type for the master ports.
   parameter type mst_b_chan_t                                         = logic,
+  /// AXI4+ATOP AR channel struct type for the slave ports.  
   parameter type slv_ar_chan_t                                        = logic,
+  /// AXI4+ATOP AR channel struct type for the master ports.
   parameter type mst_ar_chan_t                                        = logic,
+  /// AXI4+ATOP R channel struct type for the slave ports.  
   parameter type slv_r_chan_t                                         = logic,
+  /// AXI4+ATOP R channel struct type for the master ports.
   parameter type mst_r_chan_t                                         = logic,
+  /// AXI4+ATOP request struct type for the slave ports.
   parameter type slv_req_t                                            = logic,
+  /// AXI4+ATOP response struct type for the slave ports.
   parameter type slv_resp_t                                           = logic,
+  /// AXI4+ATOP request struct type for the master ports.
   parameter type mst_req_t                                            = logic,
+  /// AXI4+ATOP response struct type for the master ports
   parameter type mst_resp_t                                           = logic,
+  /// Address rule type for the address decoders from `common_cells:addr_decode`.
+  /// Example types are provided in `axi_pkg`.
+  /// Required struct fields:
+  /// ```
+  /// typedef struct packed {
+  ///   int unsigned idx;
+  ///   axi_addr_t   start_addr;
+  ///   axi_addr_t   end_addr;
+  /// } rule_t;
+  /// ```
   parameter type rule_t                                               = axi_pkg::xbar_rule_64_t
 `ifdef VCS
   , localparam int unsigned MstPortsIdxWidth =
       (Cfg.NoMstPorts == 32'd1) ? 32'd1 : unsigned'($clog2(Cfg.NoMstPorts))
 `endif
 ) (
+  /// Clock, positive edge triggered.
   input  logic                                                          clk_i,
+  /// Asynchronous reset, active low.  
   input  logic                                                          rst_ni,
+  /// Testmode enable, active high.
   input  logic                                                          test_i,
+  /// AXI4+ATOP requests to the slave ports.  
   input  slv_req_t  [Cfg.NoSlvPorts-1:0]                                slv_ports_req_i,
+  /// AXI4+ATOP responses of the slave ports.  
   output slv_resp_t [Cfg.NoSlvPorts-1:0]                                slv_ports_resp_o,
+  /// AXI4+ATOP requests of the master ports.  
   output mst_req_t  [Cfg.NoMstPorts-1:0]                                mst_ports_req_o,
+  /// AXI4+ATOP responses to the master ports.  
   input  mst_resp_t [Cfg.NoMstPorts-1:0]                                mst_ports_resp_i,
+  /// Address map array input for the crossbar. This map is global for the whole module.
+  /// It is used for routing the transactions to the respective master ports.
+  /// Each master port can have multiple different rules.
   input  rule_t     [Cfg.NoAddrRules-1:0]                               addr_map_i,
+  /// Enable default master port.
   input  logic      [Cfg.NoSlvPorts-1:0]                                en_default_mst_port_i,
 `ifdef VCS
+  /// Enables a default master port for each slave port. When this is enabled unmapped
+  /// transactions get issued at the master port given by `default_mst_port_i`.
+  /// When not used, tie to `'0`.  
   input  logic      [Cfg.NoSlvPorts-1:0][MstPortsIdxWidth-1:0]          default_mst_port_i
 `else
+  /// Enables a default master port for each slave port. When this is enabled unmapped
+  /// transactions get issued at the master port given by `default_mst_port_i`.
+  /// When not used, tie to `'0`.  
   input  logic      [Cfg.NoSlvPorts-1:0][idx_width(Cfg.NoMstPorts)-1:0] default_mst_port_i
 `endif
 );
 
-  typedef logic [Cfg.AxiAddrWidth-1:0]           addr_t;
+  // Address tpye for inidvidual address signals
+  typedef logic [Cfg.AxiAddrWidth-1:0] addr_t;
   // to account for the decoding error slave
 `ifdef VCS
   localparam int unsigned MstPortsIdxWidthOne =
@@ -204,8 +248,23 @@ import cf_math_pkg::idx_width;
   for (genvar i = 0; i < Cfg.NoSlvPorts; i++) begin : gen_xbar_slv_cross
     for (genvar j = 0; j < Cfg.NoMstPorts; j++) begin : gen_xbar_mst_cross
       if (Connectivity[i][j]) begin : gen_connection
-        assign mst_reqs[j][i]  = slv_reqs[i][j];
-        assign slv_resps[i][j] = mst_resps[j][i];
+        axi_multicut #(
+          .NoCuts     ( Cfg.PipelineStages ),
+          .aw_chan_t  ( slv_aw_chan_t      ),
+          .w_chan_t   ( w_chan_t           ),
+          .b_chan_t   ( slv_b_chan_t       ),
+          .ar_chan_t  ( slv_ar_chan_t      ),
+          .r_chan_t   ( slv_r_chan_t       ),
+          .axi_req_t  ( slv_req_t          ),
+          .axi_resp_t ( slv_resp_t         )
+        ) i_axi_multicut_xbar_pipeline (
+          .clk_i,
+          .rst_ni,
+          .slv_req_i  ( slv_reqs[i][j]  ),
+          .slv_resp_o ( slv_resps[i][j] ),
+          .mst_req_o  ( mst_reqs[j][i]  ),
+          .mst_resp_i ( mst_resps[j][i] )
+        );
 
       end else begin : gen_no_connection
         assign mst_reqs[j][i] = '0;