From 686b359792f982e867acc6c8cc3574ad6fa930b0 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Tue, 30 Aug 2022 17:28:34 +0200 Subject: [PATCH 1/3] axi_to_mem: handle asymmetric backpressure Improve performance for asymmetric backpressure on the memory channel. --- src/axi_to_mem.sv | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/axi_to_mem.sv b/src/axi_to_mem.sv index f6954de24..de70065d6 100644 --- a/src/axi_to_mem.sv +++ b/src/axi_to_mem.sv @@ -352,7 +352,8 @@ module axi_to_mem #( .DataWidth ( DataWidth ), .NumBanks ( NumBanks ), .HideStrb ( HideStrb ), - .MaxTrans ( BufDepth ) + .MaxTrans ( BufDepth ), + .FifoDepth ( 2 ) ) i_mem_to_banks ( .clk_i, .rst_ni, @@ -565,6 +566,8 @@ module mem_to_banks #( parameter bit HideStrb = 1'b0, /// Number of outstanding transactions parameter int unsigned MaxTrans = 32'b1, + /// FIFO depth, must be >=1 + parameter int unsigned FifoDepth = 1, /// Dependent parameter, do not override! Address type. localparam type addr_t = logic [AddrWidth-1:0], /// Dependent parameter, do not override! Input data type. @@ -650,19 +653,23 @@ module mem_to_banks #( assign bank_req[i].strb = strb_i[i*BytesPerBank+:BytesPerBank]; assign bank_req[i].atop = atop_i; assign bank_req[i].we = we_i; - fall_through_register #( - .T ( req_t ) + stream_fifo #( + .FALL_THROUGH ( 1'b1 ), + .DATA_WIDTH ( $bits(req_t) ), + .DEPTH ( FifoDepth ), + .T ( req_t ) ) i_ft_reg ( .clk_i, .rst_ni, - .clr_i ( 1'b0 ), + .flush_i ( 1'b0 ), .testmode_i ( 1'b0 ), + .usage_o (), + .data_i ( bank_req[i] ), .valid_i ( req_valid ), .ready_o ( req_ready[i] ), - .data_i ( bank_req[i] ), + .data_o ( bank_oup[i] ), .valid_o ( bank_req_internal[i] ), - .ready_i ( bank_gnt_internal[i] ), - .data_o ( bank_oup[i] ) + .ready_i ( bank_gnt_internal[i] ) ); assign bank_addr_o[i] = bank_oup[i].addr; assign bank_wdata_o[i] = bank_oup[i].wdata; @@ -709,19 +716,23 @@ module mem_to_banks #( // Handle responses. for (genvar i = 0; unsigned'(i) < NumBanks; i++) begin : gen_resp_regs - fall_through_register #( - .T ( oup_data_t ) + stream_fifo #( + .FALL_THROUGH ( 1'b1 ), + .DATA_WIDTH ( $bits(oup_data_t) ), + .DEPTH ( FifoDepth ), + .T ( oup_data_t ) ) i_ft_reg ( .clk_i, .rst_ni, - .clr_i ( 1'b0 ), + .flush_i ( 1'b0 ), .testmode_i ( 1'b0 ), + .usage_o (), + .data_i ( bank_rdata_i[i] ), .valid_i ( bank_rvalid_i[i] ), .ready_o ( resp_ready[i] ), - .data_i ( bank_rdata_i[i] ), .data_o ( rdata_o[i*BitsPerBank+:BitsPerBank] ), - .ready_i ( rvalid_o & !dead_response[i] ), - .valid_o ( resp_valid[i] ) + .valid_o ( resp_valid[i] ), + .ready_i ( rvalid_o & !dead_response[i] ) ); end assign rvalid_o = &(resp_valid | dead_response); From ccc7e34ed99ffa3bcad577d9ebd68fe2a39a4830 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Tue, 30 Aug 2022 17:56:18 +0200 Subject: [PATCH 2/3] axi_to_mem: propagate parameter for output fifo depth --- src/axi_to_mem.sv | 47 +++++++++++++++++++---------------- src/axi_to_mem_banked.sv | 24 +++++++++++------- src/axi_to_mem_interleaved.sv | 36 +++++++++++++++------------ src/axi_to_mem_split.sv | 41 +++++++++++++++++------------- 4 files changed, 85 insertions(+), 63 deletions(-) diff --git a/src/axi_to_mem.sv b/src/axi_to_mem.sv index de70065d6..1b8fd590e 100644 --- a/src/axi_to_mem.sv +++ b/src/axi_to_mem.sv @@ -34,6 +34,8 @@ module axi_to_mem #( parameter int unsigned BufDepth = 1, /// Hide write requests if the strb == '0 parameter bit HideStrb = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OutFifoDepth = 1, /// Dependent parameter, do not override. Memory address type. localparam type addr_t = logic [AddrWidth-1:0], /// Dependent parameter, do not override. Memory data type. @@ -348,12 +350,12 @@ module axi_to_mem #( // Split single memory request to desired number of banks. mem_to_banks #( - .AddrWidth ( AddrWidth ), - .DataWidth ( DataWidth ), - .NumBanks ( NumBanks ), - .HideStrb ( HideStrb ), - .MaxTrans ( BufDepth ), - .FifoDepth ( 2 ) + .AddrWidth ( AddrWidth ), + .DataWidth ( DataWidth ), + .NumBanks ( NumBanks ), + .HideStrb ( HideStrb ), + .MaxTrans ( BufDepth ), + .FifoDepth ( OutFifoDepth ) ) i_mem_to_banks ( .clk_i, .rst_ni, @@ -464,19 +466,21 @@ endmodule /// Interface wrapper for module `axi_to_mem`. module axi_to_mem_intf #( /// See `axi_to_mem`, parameter `AddrWidth`. - parameter int unsigned ADDR_WIDTH = 32'd0, + parameter int unsigned ADDR_WIDTH = 32'd0, /// See `axi_to_mem`, parameter `DataWidth`. - parameter int unsigned DATA_WIDTH = 32'd0, + parameter int unsigned DATA_WIDTH = 32'd0, /// AXI4+ATOP ID width. - parameter int unsigned ID_WIDTH = 32'd0, + parameter int unsigned ID_WIDTH = 32'd0, /// AXI4+ATOP user width. - parameter int unsigned USER_WIDTH = 32'd0, + parameter int unsigned USER_WIDTH = 32'd0, /// See `axi_to_mem`, parameter `NumBanks`. - parameter int unsigned NUM_BANKS = 32'd0, + parameter int unsigned NUM_BANKS = 32'd0, /// See `axi_to_mem`, parameter `BufDepth`. - parameter int unsigned BUF_DEPTH = 32'd1, + parameter int unsigned BUF_DEPTH = 32'd1, /// Hide write requests if the strb == '0 - parameter bit HIDE_STRB = 1'b0, + parameter bit HIDE_STRB = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OUT_FIFO_DEPTH = 32'd1, /// Dependent parameter, do not override. See `axi_to_mem`, parameter `addr_t`. localparam type addr_t = logic [ADDR_WIDTH-1:0], /// Dependent parameter, do not override. See `axi_to_mem`, parameter `mem_data_t`. @@ -527,14 +531,15 @@ module axi_to_mem_intf #( `AXI_ASSIGN_TO_REQ(req, slv) `AXI_ASSIGN_FROM_RESP(slv, resp) axi_to_mem #( - .axi_req_t ( req_t ), - .axi_resp_t ( resp_t ), - .AddrWidth ( ADDR_WIDTH ), - .DataWidth ( DATA_WIDTH ), - .IdWidth ( ID_WIDTH ), - .NumBanks ( NUM_BANKS ), - .BufDepth ( BUF_DEPTH ), - .HideStrb ( HIDE_STRB ) + .axi_req_t ( req_t ), + .axi_resp_t ( resp_t ), + .AddrWidth ( ADDR_WIDTH ), + .DataWidth ( DATA_WIDTH ), + .IdWidth ( ID_WIDTH ), + .NumBanks ( NUM_BANKS ), + .BufDepth ( BUF_DEPTH ), + .HideStrb ( HIDE_STRB ), + .OutFifoDepth ( OUT_FIFO_DEPTH ) ) i_axi_to_mem ( .clk_i, .rst_ni, diff --git a/src/axi_to_mem_banked.sv b/src/axi_to_mem_banked.sv index 81998277f..e50383d02 100644 --- a/src/axi_to_mem_banked.sv +++ b/src/axi_to_mem_banked.sv @@ -53,6 +53,8 @@ module axi_to_mem_banked #( parameter int unsigned MemLatency = 32'd1, /// Hide write requests if the strb == '0 parameter bit HideStrb = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OutFifoDepth = 1, /// DEPENDENT PARAMETER, DO NOT OVERWRITE! Address type of the memory request. parameter type mem_addr_t = logic [MemAddrWidth-1:0], /// DEPENDENT PARAMETER, DO NOT OVERWRITE! Atomic operation type for the memory request. @@ -183,14 +185,15 @@ module axi_to_mem_banked #( // Careful, request / grant // Only assert grant, if there is a ready axi_to_mem #( - .axi_req_t ( axi_req_t ), - .axi_resp_t( axi_resp_t ), - .AddrWidth ( AxiAddrWidth ), - .DataWidth ( AxiDataWidth ), - .IdWidth ( AxiIdWidth ), - .NumBanks ( BanksPerAxiChannel ), - .BufDepth ( MemLatency ), - .HideStrb ( HideStrb ) + .axi_req_t ( axi_req_t ), + .axi_resp_t ( axi_resp_t ), + .AddrWidth ( AxiAddrWidth ), + .DataWidth ( AxiDataWidth ), + .IdWidth ( AxiIdWidth ), + .NumBanks ( BanksPerAxiChannel ), + .BufDepth ( MemLatency ), + .HideStrb ( HideStrb ), + .OutFifoDepth ( OutFifoDepth ) ) i_axi_to_mem ( .clk_i, .rst_ni, @@ -331,6 +334,8 @@ module axi_to_mem_banked_intf #( parameter int unsigned MEM_LATENCY = 32'd1, /// Hide write requests if the strb == '0 parameter bit HIDE_STRB = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OUT_FIFO_DEPTH = 32'd1, // DEPENDENT PARAMETERS, DO NOT OVERWRITE! parameter type mem_addr_t = logic [MEM_ADDR_WIDTH-1:0], parameter type mem_atop_t = logic [5:0], @@ -398,7 +403,8 @@ module axi_to_mem_banked_intf #( .MemAddrWidth ( MEM_ADDR_WIDTH ), .MemDataWidth ( MEM_DATA_WIDTH ), .MemLatency ( MEM_LATENCY ), - .HideStrb ( HIDE_STRB ) + .HideStrb ( HIDE_STRB ), + .OutFifoDepth ( OUT_FIFO_DEPTH ) ) i_axi_to_mem_banked ( .clk_i, .rst_ni, diff --git a/src/axi_to_mem_interleaved.sv b/src/axi_to_mem_interleaved.sv index f306612e0..a744c1ec0 100644 --- a/src/axi_to_mem_interleaved.sv +++ b/src/axi_to_mem_interleaved.sv @@ -34,6 +34,8 @@ module axi_to_mem_interleaved #( parameter int unsigned BufDepth = 1, /// Hide write requests if the strb == '0 parameter bit HideStrb = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OutFifoDepth = 1, /// Dependent parameter, do not override. Memory address type. parameter type addr_t = logic [AddrWidth-1:0], /// Dependent parameter, do not override. Memory data type. @@ -115,14 +117,15 @@ module axi_to_mem_interleaved #( end axi_to_mem #( - .axi_req_t ( axi_req_t ), - .axi_resp_t ( axi_resp_t ), - .AddrWidth ( AddrWidth ), - .DataWidth ( DataWidth ), - .IdWidth ( IdWidth ), - .NumBanks ( NumBanks ), - .BufDepth ( BufDepth ), - .HideStrb ( HideStrb ) + .axi_req_t ( axi_req_t ), + .axi_resp_t ( axi_resp_t ), + .AddrWidth ( AddrWidth ), + .DataWidth ( DataWidth ), + .IdWidth ( IdWidth ), + .NumBanks ( NumBanks ), + .BufDepth ( BufDepth ), + .HideStrb ( HideStrb ), + .OutFifoDepth( OutFifoDepth ) ) i_axi_to_mem_write ( .clk_i ( clk_i ), .rst_ni ( rst_ni ), @@ -141,14 +144,15 @@ module axi_to_mem_interleaved #( ); axi_to_mem #( - .axi_req_t ( axi_req_t ), - .axi_resp_t ( axi_resp_t ), - .AddrWidth ( AddrWidth ), - .DataWidth ( DataWidth ), - .IdWidth ( IdWidth ), - .NumBanks ( NumBanks ), - .BufDepth ( BufDepth ), - .HideStrb ( HideStrb ) + .axi_req_t ( axi_req_t ), + .axi_resp_t ( axi_resp_t ), + .AddrWidth ( AddrWidth ), + .DataWidth ( DataWidth ), + .IdWidth ( IdWidth ), + .NumBanks ( NumBanks ), + .BufDepth ( BufDepth ), + .HideStrb ( HideStrb ), + .OutFifoDepth ( OutFifoDepth ) ) i_axi_to_mem_read ( .clk_i ( clk_i ), .rst_ni ( rst_ni ), diff --git a/src/axi_to_mem_split.sv b/src/axi_to_mem_split.sv index a4e19ec6e..a51350393 100644 --- a/src/axi_to_mem_split.sv +++ b/src/axi_to_mem_split.sv @@ -34,6 +34,8 @@ module axi_to_mem_split #( parameter int unsigned BufDepth = 0, /// Hide write requests if the strb == '0 parameter bit HideStrb = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OutFifoDepth = 1, /// Dependent parameters, do not override. Number of memory ports. parameter int unsigned NumMemPorts = 2*AxiDataWidth/MemDataWidth, /// Dependent parameter, do not override. Memory address type. @@ -104,14 +106,15 @@ module axi_to_mem_split #( assign busy_o = read_busy || write_busy; axi_to_mem #( - .axi_req_t ( axi_req_t ), - .axi_resp_t ( axi_resp_t ), - .AddrWidth ( AddrWidth ), - .DataWidth ( AxiDataWidth ), - .IdWidth ( IdWidth ), - .NumBanks ( NumMemPorts/2 ), - .BufDepth ( BufDepth ), - .HideStrb ( 1'b0 ) + .axi_req_t ( axi_req_t ), + .axi_resp_t ( axi_resp_t ), + .AddrWidth ( AddrWidth ), + .DataWidth ( AxiDataWidth ), + .IdWidth ( IdWidth ), + .NumBanks ( NumMemPorts/2 ), + .BufDepth ( BufDepth ), + .HideStrb ( 1'b0 ), + .OutFifoDepth ( OutFifoDepth ) ) i_axi_to_mem_read ( .clk_i, .rst_ni, @@ -130,14 +133,15 @@ module axi_to_mem_split #( ); axi_to_mem #( - .axi_req_t ( axi_req_t ), - .axi_resp_t ( axi_resp_t ), - .AddrWidth ( AddrWidth ), - .DataWidth ( AxiDataWidth ), - .IdWidth ( IdWidth ), - .NumBanks ( NumMemPorts/2 ), - .BufDepth ( BufDepth ), - .HideStrb ( HideStrb ) + .axi_req_t ( axi_req_t ), + .axi_resp_t ( axi_resp_t ), + .AddrWidth ( AddrWidth ), + .DataWidth ( AxiDataWidth ), + .IdWidth ( IdWidth ), + .NumBanks ( NumMemPorts/2 ), + .BufDepth ( BufDepth ), + .HideStrb ( HideStrb ), + .OutFifoDepth ( OutFifoDepth ) ) i_axi_to_mem_write ( .clk_i, .rst_ni, @@ -174,6 +178,8 @@ module axi_to_mem_split_intf #( parameter int unsigned BUF_DEPTH = 0, /// Hide write requests if the strb == '0 parameter bit HIDE_STRB = 1'b0, + /// Depth of output fifo/fall_through_register. Increase for asymmetric backpressure (contention) on banks. + parameter int unsigned OUT_FIFO_DEPTH = 32'd1, /// Dependent parameters, do not override. Number of memory ports. parameter int unsigned NUM_MEM_PORTS = 2*AXI_DATA_WIDTH/MEM_DATA_WIDTH, /// Dependent parameter, do not override. See `axi_to_mem`, parameter `addr_t`. @@ -230,7 +236,8 @@ module axi_to_mem_split_intf #( .IdWidth ( AXI_ID_WIDTH ), .MemDataWidth ( MEM_DATA_WIDTH ), // must divide `AxiDataWidth` without remainder .BufDepth ( BUF_DEPTH ), - .HideStrb ( HIDE_STRB ) + .HideStrb ( HIDE_STRB ), + .OutFifoDepth ( OUT_FIFO_DEPTH ) ) i_axi_to_mem_split ( .clk_i, .rst_ni, From 8ff9ce31d2aa9d9f5ab486413ee3203902b66862 Mon Sep 17 00:00:00 2001 From: Thomas Benz Date: Wed, 28 Sep 2022 11:22:58 +0200 Subject: [PATCH 3/3] changelog: Update `axi_to_mem` performance updates --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd9d4a63a..17d1a1791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - Improve compatibility with FuseSoC - Improve compatibility with Vivado XSIM +- Performance improvements to `axi_to_mem` - Use `scripts/update_authors` to update authors, slight manual fixes performed. ### Fixed