axi_dmac: Eliminate beat counter for the destination interfaces

Currently both the source side and the destination side interfaces employ a
beat counter to identify the last beat in a burst.

The burst memory already has an internal last signal on the destination
side. Exporting it allows the destination side interfaces to use it instead
of having to generate their own signal. This allows to eliminate the beat
counters on the destination side and simplify the data path logic.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

library/axi_dmac/axi_dmac_burst_memory.v

@@ -54,8 +54,11 @@ module axi_dmac_burst_memory #(
   output dest_data_valid,
   input dest_data_ready,
   output [DATA_WIDTH_DEST-1:0] dest_data,
+  output dest_data_last,
 
-  output [ID_WIDTH-1:0] dest_request_id
+  output [ID_WIDTH-1:0] dest_request_id,
+  input [ID_WIDTH-1:0] dest_data_request_id,
+  output [ID_WIDTH-1:0] dest_data_response_id
 );
 
 localparam DATA_WIDTH = DATA_WIDTH_SRC > DATA_WIDTH_DEST ?
@@ -202,7 +205,7 @@ assign dest_beat = dest_valid & dest_ready;
 assign dest_last_beat = dest_last & dest_beat;
 assign dest_raddr = {dest_id_reduced,dest_beat_counter};
 
-assign dest_burst_valid = dest_src_id != dest_id_next;
+assign dest_burst_valid = dest_data_request_id != dest_id_next;
 assign dest_burst_ready = ~dest_valid | dest_last_beat;
 
 /*
@@ -236,6 +239,25 @@ always @(posedge dest_clk) begin
   end
 end
 
+/*
+ * This clears dest_data_last after the last beat. Strictly speaking this is not
+ * necessary if this followed AXI handshaking rules since dest_data_last would
+ * be qualified by dest_data_valid and it is OK to retain the previous value of
+ * dest_data_last when dest_data_valid is not asserted. But clearing the signal
+ * here doesn't cost much and can simplify some of the more congested
+ * combinatorical logic further up the pipeline since we can assume that
+ * fifo_last == 1'b1 implies fifo_valid == 1'b1.
+ */
+always @(posedge dest_clk) begin
+  if (dest_reset == 1'b1) begin
+    dest_mem_data_last <= 1'b0;
+  end else if (dest_beat == 1'b1) begin
+    dest_mem_data_last <= dest_last;
+  end else if (dest_mem_data_ready == 1'b1) begin
+    dest_mem_data_last <= 1'b0;
+  end
+end
+
 assign dest_id_next_inc = inc_id(dest_id_next);
 
 always @(posedge dest_clk) begin
@@ -341,5 +363,6 @@ sync_bits #(
 );
 
 assign dest_request_id = dest_src_id;
+assign dest_data_response_id = dest_id;
 
 endmodule

library/axi_dmac/dest_axi_mm.v

@@ -61,15 +61,14 @@ module dmac_dest_mm_axi #(
   input  [ID_WIDTH-1:0]             request_id,
   output [ID_WIDTH-1:0]             response_id,
 
-  output [ID_WIDTH-1:0]             data_id,
   output [ID_WIDTH-1:0]             address_id,
-  input                               data_eot,
   input                               address_eot,
   input                               response_eot,
 
   input                               fifo_valid,
   output                              fifo_ready,
   input [DMA_DATA_WIDTH-1:0]        fifo_data,
+  input                               fifo_last,
 
   // Write address
   input                               m_axi_awready,
@@ -94,30 +93,8 @@ module dmac_dest_mm_axi #(
   output                              m_axi_bready
 );
 
-wire address_req_valid;
-wire address_req_ready;
-wire data_req_valid;
-wire data_req_ready;
-
 wire address_enabled;
 
-splitter #(
-  .NUM_M(2)
-) i_req_splitter (
-  .clk(m_axi_aclk),
-  .resetn(m_axi_aresetn),
-  .s_valid(req_valid),
-  .s_ready(req_ready),
-  .m_valid({
-    address_req_valid,
-    data_req_valid
-  }),
-  .m_ready({
-    address_req_ready,
-    data_req_ready
-  })
-);
-
 dmac_address_generator #(
   .ID_WIDTH(ID_WIDTH),
   .BEATS_PER_BURST_WIDTH(BEATS_PER_BURST_WIDTH),
@@ -135,8 +112,8 @@ dmac_address_generator #(
   .id(address_id),
   .request_id(request_id),
 
-  .req_valid(address_req_valid),
+  .req_valid(req_valid),
-  .req_ready(address_req_ready),
+  .req_ready(req_ready),
   .req_address(req_address),
   .req_last_burst_length(req_last_burst_length),
 
@@ -152,36 +129,10 @@ dmac_address_generator #(
   .cache(m_axi_awcache)
 );
 
-dmac_data_mover # (
+assign m_axi_wvalid = fifo_valid;
-  .ID_WIDTH(ID_WIDTH),
+assign fifo_ready = m_axi_wready;
-  .DATA_WIDTH(DMA_DATA_WIDTH),
+assign m_axi_wlast = fifo_last;
-  .BEATS_PER_BURST_WIDTH(BEATS_PER_BURST_WIDTH)
+assign m_axi_wdata = fifo_data;
-) i_data_mover (
-  .clk(m_axi_aclk),
-  .resetn(m_axi_aresetn),
-
-  /* Unused. AXI protocol guarantees ordering */
-  .enable(1'b1),
-  .enabled(),
-
-  .xfer_req(),
-
-  .request_id(address_id),
-  .response_id(data_id),
-  .eot(data_eot),
-
-  .req_valid(data_req_valid),
-  .req_ready(data_req_ready),
-  .req_last_burst_length(req_last_burst_length),
-
-  .s_axi_valid(fifo_valid),
-  .s_axi_ready(fifo_ready),
-  .s_axi_data(fifo_data),
-  .m_axi_valid(m_axi_wvalid),
-  .m_axi_ready(m_axi_wready),
-  .m_axi_data(m_axi_wdata),
-  .m_axi_last(m_axi_wlast)
-);
 
 assign m_axi_wstrb = {(DMA_DATA_WIDTH/8){1'b1}};
 

library/axi_dmac/dest_axi_stream.v

@@ -46,7 +46,6 @@ module dmac_dest_axi_stream #(
   output enabled,
   output xfer_req,
 
-  input [ID_WIDTH-1:0] request_id,
   output [ID_WIDTH-1:0] response_id,
   output [ID_WIDTH-1:0] data_id,
   input data_eot,
@@ -60,6 +59,7 @@ module dmac_dest_axi_stream #(
   output fifo_ready,
   input fifo_valid,
   input [S_AXIS_DATA_WIDTH-1:0] fifo_data,
+  input fifo_last,
 
   input req_valid,
   output req_ready,
@@ -72,50 +72,65 @@ module dmac_dest_axi_stream #(
   output [1:0] response_resp
 );
 
+`include "inc_id.h"
 
+reg data_enabled = 1'b0;
 reg req_xlast_d = 1'b0;
+reg active = 1'b0;
 
-wire data_enabled;
+reg [ID_WIDTH-1:0] id = 'h0;
-wire m_axis_last_s;
+
+/* Last beat of the burst */
+wire fifo_last_beat;
+/* Last beat of the segment */
+wire fifo_eot_beat;
+
+/* fifo_last == 1'b1 implies fifo_valid == 1'b1 */
+assign fifo_last_beat = fifo_ready & fifo_last;
+assign fifo_eot_beat = fifo_last_beat & data_eot;
+
+assign req_ready = fifo_eot_beat | ~active;
+assign data_id = id;
+assign xfer_req = active;
+
+assign m_axis_valid = fifo_valid & active;
+assign fifo_ready = m_axis_ready & active;
+assign m_axis_last = req_xlast_d & fifo_last & data_eot;
+assign m_axis_data = fifo_data;
 
 always @(posedge s_axis_aclk) begin
-  if(req_ready == 1'b1) begin
+  if (s_axis_aresetn == 1'b0) begin
+    data_enabled <= 1'b0;
+  end else if (enable == 1'b1) begin
+    data_enabled <= 1'b1;
+  end else if (m_axis_valid == 1'b0 || m_axis_ready == 1'b1) begin
+    data_enabled <= 1'b0;
+  end
+end
+
+always @(posedge s_axis_aclk) begin
+  if (req_ready == 1'b1) begin
     req_xlast_d <= req_xlast;
   end
 end
 
-assign m_axis_last = (req_xlast_d == 1'b1) ? m_axis_last_s : 1'b0;
+always @(posedge s_axis_aclk) begin
+  if (s_axis_aresetn == 1'b0) begin
+    active <= 1'b0;
+  end else if (req_valid == 1'b1) begin
+    active <= 1'b1;
+  end else if (fifo_eot_beat == 1'b1) begin
+    active <= 1'b0;
+  end
+end
 
-dmac_data_mover # (
+always @(posedge s_axis_aclk) begin
-  .ID_WIDTH(ID_WIDTH),
+  if (s_axis_aresetn == 1'b0) begin
-  .DATA_WIDTH(S_AXIS_DATA_WIDTH),
+    id <= 'h00;
-  .BEATS_PER_BURST_WIDTH(BEATS_PER_BURST_WIDTH),
+  end else if (fifo_last_beat == 1'b1) begin
-  .DISABLE_WAIT_FOR_ID(0),
+    id <= inc_id(id);
-  .LAST(1)
+  end
-) i_data_mover (
+end
-  .clk(s_axis_aclk),
-  .resetn(s_axis_aresetn),
-
-  .enable(enable),
-  .enabled(data_enabled),
-  .xfer_req(xfer_req),
-
-  .request_id(request_id),
-  .response_id(data_id),
-  .eot(data_eot),
-
-  .req_valid(req_valid),
-  .req_ready(req_ready),
-  .req_last_burst_length(req_last_burst_length),
-
-  .m_axi_ready(m_axis_ready),
-  .m_axi_valid(m_axis_valid),
-  .m_axi_data(m_axis_data),
-  .m_axi_last(m_axis_last_s),
-  .s_axi_ready(fifo_ready),
-  .s_axi_valid(fifo_valid),
-  .s_axi_data(fifo_data)
-);
 
 dmac_response_generator # (
   .ID_WIDTH(ID_WIDTH)
@@ -126,7 +141,7 @@ dmac_response_generator # (
   .enable(data_enabled),
   .enabled(enabled),
 
-  .request_id(data_id),
+  .request_id(id),
   .response_id(response_id),
 
   .eot(response_eot),

library/axi_dmac/dest_fifo_inf.v

@@ -45,9 +45,11 @@ module dmac_dest_fifo_inf #(
   input enable,
   output enabled,
 
-  input [ID_WIDTH-1:0] request_id,
+  input req_valid,
+  output req_ready,
+
   output [ID_WIDTH-1:0] response_id,
-  output [ID_WIDTH-1:0] data_id,
+  output reg [ID_WIDTH-1:0] data_id = 'h0,
   input data_eot,
   input response_eot,
 
@@ -61,10 +63,7 @@ module dmac_dest_fifo_inf #(
   output fifo_ready,
   input fifo_valid,
   input [DATA_WIDTH-1:0] fifo_data,
-
+  input fifo_last,
-  input req_valid,
-  output req_ready,
-  input [BEATS_PER_BURST_WIDTH-1:0] req_last_burst_length,
 
   output response_valid,
   input response_ready,
@@ -72,54 +71,54 @@ module dmac_dest_fifo_inf #(
   output [1:0] response_resp
 );
 
-wire [DATA_WIDTH-1:0]  dout_s;
+`include "inc_id.h"
-wire data_ready;
+
-wire data_valid;
+reg active = 1'b0;
+
+/* Last beat of the burst */
+wire fifo_last_beat;
+/* Last beat of the segment */
+wire fifo_eot_beat;
 
 assign enabled = enable;
-assign data_ready = en & (data_valid | ~enable);
+assign fifo_ready = en & (fifo_valid | ~enable);
 
-dmac_data_mover # (
+/* fifo_last == 1'b1 implies fifo_valid == 1'b1 */
-  .ID_WIDTH(ID_WIDTH),
+assign fifo_last_beat = fifo_ready & fifo_last;
-  .DATA_WIDTH(DATA_WIDTH),
+assign fifo_eot_beat = fifo_last_beat & data_eot;
-  .BEATS_PER_BURST_WIDTH(BEATS_PER_BURST_WIDTH),
-  .DISABLE_WAIT_FOR_ID(0)
-) i_data_mover (
-  .clk(clk),
-  .resetn(resetn),
 
-  .enable(enable),
+assign req_ready = fifo_eot_beat | ~active;
-  .enabled(),
+assign xfer_req = active;
-  .xfer_req(xfer_req),
-
-  .request_id(request_id),
-  .response_id(data_id),
-  .eot(data_eot),
-
-  .req_valid(req_valid),
-  .req_ready(req_ready),
-  .req_last_burst_length(req_last_burst_length),
-
-  .s_axi_ready(fifo_ready),
-  .s_axi_valid(fifo_valid),
-  .s_axi_data(fifo_data),
-  .m_axi_ready(data_ready),
-  .m_axi_valid(data_valid),
-  .m_axi_data(dout_s),
-  .m_axi_last()
-);
 
 always @(posedge clk) begin
   if (en) begin
-    dout <= (data_valid) ? dout_s : {DATA_WIDTH{1'b0}};
+    dout <= fifo_valid ? fifo_data : {DATA_WIDTH{1'b0}};
-    valid <= data_valid & enable;
+    valid <= fifo_valid & enable;
-    underflow <= ~(data_valid & enable);
+    underflow <= ~(fifo_valid & enable);
   end else begin
     valid <= 1'b0;
     underflow <= 1'b0;
   end
 end
 
+always @(posedge clk) begin
+  if (resetn == 1'b0) begin
+    data_id <= 'h00;
+  end else if (fifo_last_beat == 1'b1) begin
+    data_id <= inc_id(data_id);
+  end
+end
+
+always @(posedge clk) begin
+  if (resetn == 1'b0) begin
+    active <= 1'b0;
+  end else if (req_valid == 1'b1) begin
+    active <= 1'b1;
+  end else if (fifo_eot_beat == 1'b1) begin
+    active <= 1'b0;
+  end
+end
+
 dmac_response_generator # (
   .ID_WIDTH(ID_WIDTH)
 ) i_response_generator (

library/axi_dmac/request_arb.v

@@ -223,14 +223,18 @@ wire [1:0] dest_response_resp;
 wire dest_response_resp_eot;
 
 wire [ID_WIDTH-1:0] dest_request_id;
+wire [ID_WIDTH-1:0] dest_data_request_id;
+wire [ID_WIDTH-1:0] dest_data_response_id;
 wire [ID_WIDTH-1:0] dest_response_id;
 
 wire dest_valid;
 wire dest_ready;
 wire [DMA_DATA_WIDTH_DEST-1:0] dest_data;
+wire dest_last;
 wire dest_fifo_valid;
 wire dest_fifo_ready;
 wire [DMA_DATA_WIDTH_DEST-1:0] dest_fifo_data;
+wire dest_fifo_last;
 
 wire src_req_valid;
 wire src_req_ready;
@@ -293,14 +297,14 @@ generate if (DMA_TYPE_DEST == DMA_TYPE_MM_AXI) begin
 assign dest_clk = m_dest_axi_aclk;
 assign dest_ext_resetn = m_dest_axi_aresetn;
 
-wire [ID_WIDTH-1:0] dest_data_id;
 wire [ID_WIDTH-1:0] dest_address_id;
 wire dest_address_eot = eot_mem[dest_address_id];
-wire dest_data_eot = eot_mem[dest_data_id];
 wire dest_response_eot = eot_mem[dest_response_id];
 
 assign dbg_dest_address_id = dest_address_id;
-assign dbg_dest_data_id = dest_data_id;
+assign dbg_dest_data_id = dest_data_response_id;
+
+assign dest_data_request_id = dest_address_id;
 
 dmac_dest_mm_axi #(
   .ID_WIDTH(ID_WIDTH),
@@ -329,16 +333,15 @@ dmac_dest_mm_axi #(
   .request_id(dest_request_id),
   .response_id(dest_response_id),
 
-  .data_id(dest_data_id),
   .address_id(dest_address_id),
 
   .address_eot(dest_address_eot),
-  .data_eot(dest_data_eot),
   .response_eot(dest_response_eot),
 
   .fifo_valid(dest_valid),
   .fifo_ready(dest_ready),
   .fifo_data(dest_data),
+  .fifo_last(dest_last),
 
   .m_axi_awready(m_axi_awready),
   .m_axi_awvalid(m_axi_awvalid),
@@ -388,6 +391,8 @@ wire [ID_WIDTH-1:0] data_id;
 wire data_eot = eot_mem[data_id];
 wire response_eot = eot_mem[dest_response_id];
 
+assign dest_data_request_id = dest_request_id;
+
 assign dbg_dest_address_id = 'h00;
 assign dbg_dest_data_id = data_id;
 
@@ -412,7 +417,6 @@ dmac_dest_axi_stream #(
   .response_resp(dest_response_resp),
   .response_resp_eot(dest_response_resp_eot),
 
-  .request_id(dest_request_id),
   .response_id(dest_response_id),
   .data_id(data_id),
   .xfer_req(m_axis_xfer_req),
@@ -423,6 +427,7 @@ dmac_dest_axi_stream #(
   .fifo_valid(dest_valid),
   .fifo_ready(dest_ready),
   .fifo_data(dest_data),
+  .fifo_last(dest_last),
 
   .m_axis_valid(m_axis_valid),
   .m_axis_ready(m_axis_ready),
@@ -449,6 +454,8 @@ wire [ID_WIDTH-1:0] data_id;
 wire data_eot = eot_mem[data_id];
 wire response_eot = eot_mem[dest_response_id];
 
+assign dest_data_request_id = dest_request_id;
+
 assign dbg_dest_address_id = 'h00;
 assign dbg_dest_data_id = data_id;
 
@@ -465,14 +472,12 @@ dmac_dest_fifo_inf #(
 
   .req_valid(dest_req_valid),
   .req_ready(dest_req_ready),
-  .req_last_burst_length(dest_req_last_burst_length),
 
   .response_valid(dest_response_valid),
   .response_ready(dest_response_ready),
   .response_resp(dest_response_resp),
   .response_resp_eot(dest_response_resp_eot),
 
-  .request_id(dest_request_id),
   .response_id(dest_response_id),
   .data_id(data_id),
 
@@ -482,6 +487,7 @@ dmac_dest_fifo_inf #(
   .fifo_valid(dest_valid),
   .fifo_ready(dest_ready),
   .fifo_data(dest_data),
+  .fifo_last(dest_last),
 
   .en(fifo_rd_en),
   .valid(fifo_rd_valid),
@@ -743,30 +749,40 @@ axi_dmac_burst_memory #(
   .dest_data_valid(dest_fifo_valid),
   .dest_data_ready(dest_fifo_ready),
   .dest_data(dest_fifo_data),
+  .dest_data_last(dest_fifo_last),
 
-  .dest_request_id(dest_request_id)
+  .dest_request_id(dest_request_id),
+  .dest_data_request_id(dest_data_request_id),
+  .dest_data_response_id(dest_data_response_id)
 );
 
 wire _dest_valid;
 wire _dest_ready;
 wire [DMA_DATA_WIDTH_DEST-1:0] _dest_data;
+wire _dest_last;
 
 axi_register_slice #(
-  .DATA_WIDTH(DMA_DATA_WIDTH_DEST),
+  .DATA_WIDTH(DMA_DATA_WIDTH_DEST + 1),
   .FORWARD_REGISTERED(AXI_SLICE_DEST)
 ) i_dest_slice2 (
   .clk(dest_clk),
   .resetn(dest_resetn),
   .s_axi_valid(dest_fifo_valid),
   .s_axi_ready(dest_fifo_ready),
-  .s_axi_data(dest_fifo_data),
+  .s_axi_data({
+    dest_fifo_last,
+    dest_fifo_data
+  }),
   .m_axi_valid(_dest_valid),
   .m_axi_ready(_dest_ready),
-  .m_axi_data(_dest_data)
+  .m_axi_data({
+    _dest_last,
+    _dest_data
+  })
 );
 
 axi_register_slice #(
-  .DATA_WIDTH(DMA_DATA_WIDTH_DEST),
+  .DATA_WIDTH(DMA_DATA_WIDTH_DEST + 1),
   .FORWARD_REGISTERED(AXI_SLICE_DEST),
   .BACKWARD_REGISTERED(AXI_SLICE_DEST)
 ) i_dest_slice (
@@ -774,10 +790,16 @@ axi_register_slice #(
   .resetn(dest_resetn),
   .s_axi_valid(_dest_valid),
   .s_axi_ready(_dest_ready),
-  .s_axi_data(_dest_data),
+  .s_axi_data({
+    _dest_last,
+    _dest_data
+  }),
   .m_axi_valid(dest_valid),
   .m_axi_ready(dest_ready),
-  .m_axi_data(dest_data)
+  .m_axi_data({
+    dest_last,
+    dest_data
+  })
 );
 
 splitter #(

library/axi_dmac/tb/dma_read_shutdown_tb

@@ -6,7 +6,7 @@ SOURCE+=" ../axi_dmac_transfer.v ../request_arb.v ../request_generator.v ../spli
 SOURCE+=" ../2d_transfer.v"
 SOURCE+=" ../axi_dmac_resize_src.v ../axi_dmac_resize_dest.v"
 SOURCE+=" ../axi_dmac_burst_memory.v"
-SOURCE+=" ../axi_dmac_reset_manager.v ../data_mover.v ../axi_register_slice.v"
+SOURCE+=" ../axi_dmac_reset_manager.v ../axi_register_slice.v"
 SOURCE+=" ../dest_fifo_inf.v"
 SOURCE+=" ../src_axi_mm.v ../address_generator.v ../response_generator.v"
 SOURCE+=" ../../util_axis_fifo/util_axis_fifo.v"

library/axi_dmac/tb/dma_read_tb

@@ -5,7 +5,7 @@ SOURCE+=" axi_read_slave.v axi_slave.v"
 SOURCE+=" ../axi_dmac_transfer.v ../2d_transfer.v ../request_arb.v ../request_generator.v ../splitter.v"
 SOURCE+=" ../axi_dmac_resize_src.v ../axi_dmac_resize_dest.v"
 SOURCE+=" ../axi_dmac_burst_memory.v"
-SOURCE+=" ../axi_dmac_reset_manager.v ../data_mover.v ../axi_register_slice.v"
+SOURCE+=" ../axi_dmac_reset_manager.v ../axi_register_slice.v"
 SOURCE+=" ../dest_fifo_inf.v"
 SOURCE+=" ../src_axi_mm.v ../address_generator.v ../response_generator.v"
 SOURCE+=" ../../util_axis_fifo/util_axis_fifo.v"