From 165264bfc854179d070ac53bb1a89727db50fc26 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 18 Jul 2024 15:08:58 +0200 Subject: [PATCH 1/6] tb_axi_dw_downsizer: Add initial stall to B and R Forces error fixed in #323 --- test/tb_axi_dw_downsizer.sv | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test/tb_axi_dw_downsizer.sv b/test/tb_axi_dw_downsizer.sv index 1edd3b16f..e687de066 100644 --- a/test/tb_axi_dw_downsizer.sv +++ b/test/tb_axi_dw_downsizer.sv @@ -21,6 +21,8 @@ module tb_axi_dw_downsizer #( parameter int unsigned TbAxiSlvPortDataWidth = 64 , parameter int unsigned TbAxiMstPortDataWidth = 32 , parameter int unsigned TbAxiUserWidth = 8 , + parameter int unsigned TbInitialBStallCycles = 1000, + parameter int unsigned TbInitialRStallCycles = 1000, // TB Parameters parameter time TbCyclTime = 10ns, parameter time TbApplTime = 2ns , @@ -35,6 +37,9 @@ module tb_axi_dw_downsizer #( logic rst_n; logic eos; + int unsigned b_stall; + int unsigned r_stall; + clk_rst_gen #( .ClkPeriod (TbCyclTime), .RstClkCycles (5 ) @@ -65,7 +70,31 @@ module tb_axi_dw_downsizer #( .AXI_USER_WIDTH(TbAxiUserWidth ) ) master (); - `AXI_ASSIGN(master, master_dv) + `AXI_ASSIGN_AW(master, master_dv) + `AXI_ASSIGN_W(master, master_dv) + `AXI_ASSIGN_AR(master, master_dv) + assign master_dv.b_id = master.b_id; + assign master_dv.b_resp = master.b_resp; + assign master_dv.b_user = master.b_user; + assign master_dv.b_valid = b_stall != 0 ? 1'b0 : master.b_valid; + assign master.b_ready = b_stall != 0 ? 1'b0 : master_dv.b_ready; + assign master_dv.r_id = master.r_id; + assign master_dv.r_data = master.r_data; + assign master_dv.r_resp = master.r_resp; + assign master_dv.r_last = master.r_last; + assign master_dv.r_user = master.r_user; + assign master_dv.r_valid = r_stall != 0 ? 1'b0 : master.r_valid; + assign master.r_ready = r_stall != 0 ? 1'b0 : master_dv.r_ready; + + always_ff @(posedge clk or negedge rst_n) begin : proc_ + if(~rst_n) begin + b_stall <= TbInitialBStallCycles; + r_stall <= TbInitialRStallCycles; + end else begin + b_stall <= b_stall == 0 ? 0 : b_stall-1; + r_stall <= r_stall == 0 ? 0 : r_stall-1; + end + end axi_test::axi_rand_master #( .AW (TbAxiAddrWidth ), From d3786169a8103e3f4123dc74f5fd0a03f2f036e7 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 18 Jul 2024 17:01:49 +0200 Subject: [PATCH 2/6] Modify test to force the error --- scripts/run_vsim.sh | 8 ++++++++ test/tb_axi_dw_downsizer.sv | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/run_vsim.sh b/scripts/run_vsim.sh index 9812c5ed5..aaf8c3f22 100755 --- a/scripts/run_vsim.sh +++ b/scripts/run_vsim.sh @@ -49,6 +49,14 @@ exec_test() { call_vsim tb_$1 ;; axi_dw_downsizer) + call_vsim tb_axi_dw_downsizer \ + -gTbAxiSlvPortDataWidth=32 \ + -gTbAxiMstPortDataWidth=16 \ + -gTbInitialBStallCycles=100000 -t 1ps + call_vsim tb_axi_dw_downsizer \ + -gTbAxiSlvPortDataWidth=32 \ + -gTbAxiMstPortDataWidth=16 \ + -gTbInitialRStallCycles=100000 -t 1ps for AxiSlvPortDataWidth in 8 16 32 64 128 256 512 1024; do for (( AxiMstPortDataWidth = 8; \ AxiMstPortDataWidth < $AxiSlvPortDataWidth; \ diff --git a/test/tb_axi_dw_downsizer.sv b/test/tb_axi_dw_downsizer.sv index e687de066..a11baf9d3 100644 --- a/test/tb_axi_dw_downsizer.sv +++ b/test/tb_axi_dw_downsizer.sv @@ -21,8 +21,8 @@ module tb_axi_dw_downsizer #( parameter int unsigned TbAxiSlvPortDataWidth = 64 , parameter int unsigned TbAxiMstPortDataWidth = 32 , parameter int unsigned TbAxiUserWidth = 8 , - parameter int unsigned TbInitialBStallCycles = 1000, - parameter int unsigned TbInitialRStallCycles = 1000, + parameter int unsigned TbInitialBStallCycles = 0, + parameter int unsigned TbInitialRStallCycles = 0, // TB Parameters parameter time TbCyclTime = 10ns, parameter time TbApplTime = 2ns , From 33cc1638ec4ac6cdd584d53dce603c52e87b97f8 Mon Sep 17 00:00:00 2001 From: Luca Colagrande Date: Tue, 3 Oct 2023 12:42:52 +0200 Subject: [PATCH 3/6] axi_dw_downsizer: Fix `i_forward_b_beats_queue` underflow --- src/axi_dw_downsizer.sv | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/axi_dw_downsizer.sv b/src/axi_dw_downsizer.sv index 88d774ad0..b1513775d 100644 --- a/src/axi_dw_downsizer.sv +++ b/src/axi_dw_downsizer.sv @@ -733,7 +733,7 @@ module axi_dw_downsizer #( automatic addr_t slv_port_offset = AxiSlvPortStrbWidth == 1 ? '0 : w_req_q.aw.addr[idx_width(AxiSlvPortStrbWidth)-1:0]; // Valid output - mst_req.w_valid = 1'b1 ; + mst_req.w_valid = !forward_b_beat_full; mst_req.w.last = w_req_q.aw.len == 0; mst_req.w.user = slv_req_i.w.user ; @@ -774,7 +774,7 @@ module axi_dw_downsizer #( // Trigger another burst request, if needed if (w_state_q == W_SPLIT_INCR_DOWNSIZE) // Finished current burst, but whole transaction hasn't finished - if (w_req_q.aw.len == '0 && w_req_q.burst_len != '0 && !forward_b_beat_full) begin + if (w_req_q.aw.len == '0 && w_req_q.burst_len != '0) begin w_req_d.aw_valid = 1'b1; w_req_d.aw.len = (w_req_d.burst_len <= 255) ? w_req_d.burst_len : 255; @@ -783,7 +783,7 @@ module axi_dw_downsizer #( forward_b_beat_push = 1'b1; end - if (w_req_q.burst_len == 0 && !forward_b_beat_full) begin + if (w_req_q.burst_len == 0) begin w_state_d = W_IDLE; forward_b_beat_push = 1'b1; From 7c37f95e0298553a559fe333bd3aa90e4b4fb0cb Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 17 Jul 2024 11:22:13 +0200 Subject: [PATCH 4/6] axi_dw_downsizer: Ensure begin/end for all blocks --- src/axi_dw_downsizer.sv | 55 ++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/src/axi_dw_downsizer.sv b/src/axi_dw_downsizer.sv index b1513775d..55484cfd5 100644 --- a/src/axi_dw_downsizer.sv +++ b/src/axi_dw_downsizer.sv @@ -545,9 +545,9 @@ module axi_dw_downsizer #( end // Request was accepted - if (!r_req_q.ar_valid) + if (!r_req_q.ar_valid) begin // Our turn - if ((idqueue_id == t) && idqueue_valid) + if ((idqueue_id == t) && idqueue_valid) begin // Ready to accept more data if (!slv_r_valid_tran[t] || (slv_r_valid_tran[t] && slv_r_ready_tran[t])) begin mst_r_ready_tran[t] = 1'b1; @@ -557,12 +557,13 @@ module axi_dw_downsizer #( automatic addr_t slv_port_offset = AxiSlvPortStrbWidth == 1 ? '0 : r_req_q.ar.addr[idx_width(AxiSlvPortStrbWidth)-1:0]; // Serialization - for (int b = 0; b < AxiSlvPortStrbWidth; b++) + for (int b = 0; b < AxiSlvPortStrbWidth; b++) begin if ((b >= slv_port_offset) && (b - slv_port_offset < (1 << r_req_q.orig_ar_size)) && (b + mst_port_offset - slv_port_offset < AxiMstPortStrbWidth)) begin r_data[b] = mst_resp.r.data[8*(b + mst_port_offset - slv_port_offset) +: 8]; end + end r_req_d.burst_len = r_req_q.burst_len - 1 ; r_req_d.ar.len = r_req_q.ar.len - 1 ; @@ -583,33 +584,44 @@ module axi_dw_downsizer #( end endcase - if (r_req_q.burst_len == 0) + if (r_req_q.burst_len == 0) begin idqueue_pop[t] = 1'b1; + end case (r_state_q) - R_PASSTHROUGH : + R_PASSTHROUGH : begin // Forward data as soon as we can r_req_d.r_valid = 1'b1; + end - R_INCR_DOWNSIZE, R_SPLIT_INCR_DOWNSIZE: + R_INCR_DOWNSIZE, R_SPLIT_INCR_DOWNSIZE: begin // Forward when the burst is finished, or after filling up a word - if (r_req_q.burst_len == 0 || (aligned_addr(r_req_d.ar.addr, r_req_q.orig_ar_size) != aligned_addr(r_req_q.ar.addr, r_req_q.orig_ar_size))) + if (r_req_q.burst_len == 0 || + (aligned_addr(r_req_d.ar.addr, r_req_q.orig_ar_size) != + aligned_addr(r_req_q.ar.addr, r_req_q.orig_ar_size) )) begin r_req_d.r_valid = 1'b1; + end + end endcase // Trigger another burst request, if needed - if (r_state_q == R_SPLIT_INCR_DOWNSIZE) + if (r_state_q == R_SPLIT_INCR_DOWNSIZE) begin // Finished current burst, but whole transaction hasn't finished if (r_req_q.ar.len == '0 && r_req_q.burst_len != '0) begin r_req_d.ar_valid = !r_req_q.injected_aw; r_req_d.ar.len = (r_req_d.burst_len <= 255) ? r_req_d.burst_len : 255; end + end end end + end + end - if (slv_r_valid_tran[t] && slv_r_ready_tran[t]) - if (r_req_q.burst_len == '1) + if (slv_r_valid_tran[t] && slv_r_ready_tran[t]) begin + if (r_req_q.burst_len == '1) begin r_state_d = R_IDLE; + end + end end endcase end @@ -709,8 +721,9 @@ module axi_dw_downsizer #( mst_req.b_ready = slv_req_i.b_ready; // Got an ack on the B channel. Pop transaction. - if (mst_req.b_ready && mst_resp.b_valid) + if (mst_req.b_ready && mst_resp.b_valid) begin forward_b_beat_pop = 1'b1; + end end else begin // Otherwise, just acknowlegde the B beats slv_resp_o.b_valid = 1'b0 ; @@ -727,7 +740,7 @@ module axi_dw_downsizer #( case (w_state_q) W_PASSTHROUGH, W_INCR_DOWNSIZE, W_SPLIT_INCR_DOWNSIZE: begin // Request was accepted - if (!w_req_q.aw_valid) + if (!w_req_q.aw_valid) begin if (slv_req_i.w_valid) begin automatic addr_t mst_port_offset = AxiMstPortStrbWidth == 1 ? '0 : w_req_q.aw.addr[idx_width(AxiMstPortStrbWidth)-1:0]; automatic addr_t slv_port_offset = AxiSlvPortStrbWidth == 1 ? '0 : w_req_q.aw.addr[idx_width(AxiSlvPortStrbWidth)-1:0]; @@ -738,15 +751,17 @@ module axi_dw_downsizer #( mst_req.w.user = slv_req_i.w.user ; // Lane steering - for (int b = 0; b < AxiSlvPortStrbWidth; b++) + for (int b = 0; b < AxiSlvPortStrbWidth; b++) begin if ((b >= slv_port_offset) && (b - slv_port_offset < (1 << w_req_q.orig_aw_size)) && (b + mst_port_offset - slv_port_offset < AxiMstPortStrbWidth)) begin w_data[b + mst_port_offset - slv_port_offset] = slv_req_i.w.data[8*b +: 8]; mst_req.w.strb[b + mst_port_offset - slv_port_offset] = slv_req_i.w.strb[b] ; end + end mst_req.w.data = w_data; end + end // Acknowledgment if (mst_resp.w_ready && mst_req.w_valid) begin @@ -763,16 +778,21 @@ module axi_dw_downsizer #( endcase case (w_state_q) - W_PASSTHROUGH: + W_PASSTHROUGH: begin slv_resp_o.w_ready = 1'b1; + end - W_INCR_DOWNSIZE, W_SPLIT_INCR_DOWNSIZE: - if (w_req_q.burst_len == 0 || (aligned_addr(w_req_d.aw.addr, w_req_q.orig_aw_size) != aligned_addr(w_req_q.aw.addr, w_req_q.orig_aw_size))) + W_INCR_DOWNSIZE, W_SPLIT_INCR_DOWNSIZE: begin + if (w_req_q.burst_len == 0 || + (aligned_addr(w_req_d.aw.addr, w_req_q.orig_aw_size) != + aligned_addr(w_req_q.aw.addr, w_req_q.orig_aw_size) )) begin slv_resp_o.w_ready = 1'b1; + end + end endcase // Trigger another burst request, if needed - if (w_state_q == W_SPLIT_INCR_DOWNSIZE) + if (w_state_q == W_SPLIT_INCR_DOWNSIZE) begin // Finished current burst, but whole transaction hasn't finished if (w_req_q.aw.len == '0 && w_req_q.burst_len != '0) begin w_req_d.aw_valid = 1'b1; @@ -782,6 +802,7 @@ module axi_dw_downsizer #( forward_b_beat_i = 1'b0; forward_b_beat_push = 1'b1; end + end if (w_req_q.burst_len == 0) begin w_state_d = W_IDLE; From 9a765c9085d2ce80a468de76cc7f0ea8e2f677fd Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 17 Jul 2024 11:38:54 +0200 Subject: [PATCH 5/6] axi_dw_downsizer: Only delay last W beat Ensures last W beat is not sent if B fifo is full, allowing all W beats except last to pass. --- src/axi_dw_downsizer.sv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/axi_dw_downsizer.sv b/src/axi_dw_downsizer.sv index 55484cfd5..949b3caf3 100644 --- a/src/axi_dw_downsizer.sv +++ b/src/axi_dw_downsizer.sv @@ -746,7 +746,7 @@ module axi_dw_downsizer #( automatic addr_t slv_port_offset = AxiSlvPortStrbWidth == 1 ? '0 : w_req_q.aw.addr[idx_width(AxiSlvPortStrbWidth)-1:0]; // Valid output - mst_req.w_valid = !forward_b_beat_full; + mst_req.w_valid = !(forward_b_beat_full && w_req_q.aw.len == 0); mst_req.w.last = w_req_q.aw.len == 0; mst_req.w.user = slv_req_i.w.user ; From 1d04d8873bbfbd77356d6fdc8f0facd41bdede1f Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Wed, 17 Jul 2024 11:39:01 +0200 Subject: [PATCH 6/6] Update Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca9cbfb12..4419e29d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `axi_bus_compare`: Fix mismatch detection. - `axi_to_detailed_mem`: Only respond with `exokay` if `lock` was set on the request. Bump `common_cells` for `mem_to_banks` fix. +- `axi_dw_downsizer`: Fix `i_forward_b_beats_queue` underflow. ## 0.39.3 - 2024-05-08 ### Added