axi_dmac: Enforce transfer length and stride alignments

In its current implementation the DMAC requires that the length of a transfer is aligned to the widest interface. E.g. if the widest interface is 128 bits wide the length of the transfer needs to be a multiple of 16 bytes. If the requested length is not aligned to the interface width it will be rounded up. This works fine as long as both interfaces have the same width. If they have different widths it is possible that the length is rounded up to different values on the source and destination side. In that case the DMA will deadlock because the transfer lengths don't match and either not enough of too much data is delivered from the source to the destination side. Currently it is up to software to make sure that such an invalid configuration is not possible. Also enforce this requirement in the DMAC itself by setting the LSBs of the transfer length to a fixed 1 so that the length is always aligned to the widest interface. Software can also use this to discover the length alignment requirement, by first writing a zero to the length register and then reading the register back. The LSBs of the read back value will be non-zero indicating the alignment requirement. In a similar way the stride needs to be aligned to the width of its respective interface, so the generated addresses stay aligned. Enforce this in the same way by keeping the LSBs cleared. Increment the minor version number to reflect these changes. Signed-off-by: Lars-Peter Clausen <[email protected]>
analogdevicesinc · Jul 3, 2018 · 8ddcffc · 8ddcffc
1 parent c4cb3df
commit 8ddcffc
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 15 deletions.
diff --git a/library/axi_dmac/axi_dmac.v b/library/axi_dmac/axi_dmac.v
@@ -276,6 +276,11 @@ localparam REAL_MAX_BYTES_PER_BURST =
   BYTES_PER_BURST_LIMIT < MAX_BYTES_PER_BURST ?
     BYTES_PER_BURST_LIMIT : MAX_BYTES_PER_BURST;
 
+/* Align to the length to the wider interface */
+localparam DMA_LENGTH_ALIGN =
+  BYTES_PER_BEAT_WIDTH_DEST < BYTES_PER_BEAT_WIDTH_SRC ?
+    BYTES_PER_BEAT_WIDTH_SRC : BYTES_PER_BEAT_WIDTH_DEST;
+
 // ID signals from the DMAC, just for debugging
 wire [ID_WIDTH-1:0] dest_request_id;
 wire [ID_WIDTH-1:0] dest_data_id;
@@ -351,6 +356,7 @@ axi_dmac_regmap #(
   .BYTES_PER_BEAT_WIDTH_SRC(BYTES_PER_BEAT_WIDTH_SRC),
   .DMA_AXI_ADDR_WIDTH(DMA_AXI_ADDR_WIDTH),
   .DMA_LENGTH_WIDTH(DMA_LENGTH_WIDTH),
+  .DMA_LENGTH_ALIGN(DMA_LENGTH_ALIGN),
   .DMA_CYCLIC(CYCLIC),
   .HAS_DEST_ADDR(HAS_DEST_ADDR),
   .HAS_SRC_ADDR(HAS_SRC_ADDR),

diff --git a/library/axi_dmac/axi_dmac_regmap.v b/library/axi_dmac/axi_dmac_regmap.v
@@ -40,6 +40,7 @@ module axi_dmac_regmap #(
   parameter BYTES_PER_BEAT_WIDTH_SRC = 1,
   parameter DMA_AXI_ADDR_WIDTH = 32,
   parameter DMA_LENGTH_WIDTH = 24,
+  parameter DMA_LENGTH_ALIGN = 3,
   parameter DMA_CYCLIC = 0,
   parameter HAS_DEST_ADDR = 1,
   parameter HAS_SRC_ADDR = 1,
@@ -103,7 +104,7 @@ module axi_dmac_regmap #(
   input [31:0] dbg_ids1
 );
 
-localparam PCORE_VERSION = 'h00040062;
+localparam PCORE_VERSION = 'h00040161;
 
 // Register interface signals
 reg [31:0] up_rdata = 32'h00;
@@ -210,6 +211,7 @@ axi_dmac_regmap_request #(
   .BYTES_PER_BEAT_WIDTH_SRC(BYTES_PER_BEAT_WIDTH_SRC),
   .DMA_AXI_ADDR_WIDTH(DMA_AXI_ADDR_WIDTH),
   .DMA_LENGTH_WIDTH(DMA_LENGTH_WIDTH),
+  .DMA_LENGTH_ALIGN(DMA_LENGTH_ALIGN),
   .DMA_CYCLIC(DMA_CYCLIC),
   .HAS_DEST_ADDR(HAS_DEST_ADDR),
   .HAS_SRC_ADDR(HAS_SRC_ADDR),

diff --git a/library/axi_dmac/axi_dmac_regmap_request.v b/library/axi_dmac/axi_dmac_regmap_request.v
@@ -39,6 +39,7 @@ module axi_dmac_regmap_request #(
   parameter BYTES_PER_BEAT_WIDTH_SRC = 1,
   parameter DMA_AXI_ADDR_WIDTH = 32,
   parameter DMA_LENGTH_WIDTH = 24,
+  parameter DMA_LENGTH_ALIGN = 3,
   parameter DMA_CYCLIC = 0,
   parameter HAS_DEST_ADDR = 1,
   parameter HAS_SRC_ADDR = 1,
@@ -88,7 +89,7 @@ reg [3:0] up_transfer_done_bitmap = 4'b0;
 
 reg [DMA_AXI_ADDR_WIDTH-1:BYTES_PER_BEAT_WIDTH_DEST] up_dma_dest_address = 'h00;
 reg [DMA_AXI_ADDR_WIDTH-1:BYTES_PER_BEAT_WIDTH_SRC]  up_dma_src_address = 'h00;
-reg [DMA_LENGTH_WIDTH-1:0] up_dma_x_length = 'h00;
+reg [DMA_LENGTH_WIDTH-1:0] up_dma_x_length = {DMA_LENGTH_ALIGN{1'b1}};
 reg up_dma_cyclic = DMA_CYCLIC ? 1'b1 : 1'b0;
 reg up_dma_last = 1'b1;
 
@@ -102,7 +103,7 @@ always @(posedge clk) begin
   if (reset == 1'b1) begin
     up_dma_src_address <= 'h00;
     up_dma_dest_address <= 'h00;
-    up_dma_x_length <= 'h00;
+    up_dma_x_length[DMA_LENGTH_WIDTH-1:DMA_LENGTH_ALIGN] <= 'h00;
     up_dma_req_valid <= 1'b0;
     up_dma_cyclic <= DMA_CYCLIC ? 1'b1 : 1'b0;
     up_dma_last <= 1'b1;
@@ -125,7 +126,7 @@ always @(posedge clk) begin
       end
       9'h104: up_dma_dest_address <= up_wdata[DMA_AXI_ADDR_WIDTH-1:BYTES_PER_BEAT_WIDTH_DEST];
       9'h105: up_dma_src_address <= up_wdata[DMA_AXI_ADDR_WIDTH-1:BYTES_PER_BEAT_WIDTH_SRC];
-      9'h106: up_dma_x_length <= up_wdata[DMA_LENGTH_WIDTH-1:0];
+      9'h106: up_dma_x_length[DMA_LENGTH_WIDTH-1:DMA_LENGTH_ALIGN] <= up_wdata[DMA_LENGTH_WIDTH-1:DMA_LENGTH_ALIGN];
       endcase
     end
   end
@@ -157,13 +158,13 @@ if (DMA_2D_TRANSFER == 1) begin
   always @(posedge clk) begin
     if (reset == 1'b1) begin
       up_dma_y_length <= 'h00;
-      up_dma_dest_stride <= 'h00;
-      up_dma_src_stride <= 'h00;
+      up_dma_dest_stride[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_DEST] <= 'h00;
+      up_dma_src_stride[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_SRC] <= 'h00;
     end else if (up_wreq == 1'b1) begin
       case (up_waddr)
       9'h107: up_dma_y_length <= up_wdata[DMA_LENGTH_WIDTH-1:0];
-      9'h108: up_dma_dest_stride <= up_wdata[DMA_LENGTH_WIDTH-1:0];
-      9'h109: up_dma_src_stride <= up_wdata[DMA_LENGTH_WIDTH-1:0];
+      9'h108: up_dma_dest_stride[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_DEST] <= up_wdata[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_DEST];
+      9'h109: up_dma_src_stride[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_SRC] <= up_wdata[DMA_LENGTH_WIDTH-1:BYTES_PER_BEAT_WIDTH_SRC];
       endcase
     end
   end

diff --git a/library/axi_dmac/tb/regmap_tb.v b/library/axi_dmac/tb/regmap_tb.v
@@ -45,7 +45,10 @@ module dmac_regmap_tb;
   localparam BYTES_PER_BEAT = 1;
   localparam DMA_AXI_ADDR_WIDTH = 32;
 
+  localparam LENGTH_ALIGN = 2;
   localparam LENGTH_MASK = {DMA_LENGTH_WIDTH{1'b1}};
+  localparam LENGTH_ALIGN_MASK = {LENGTH_ALIGN{1'b1}};
+  localparam STRIDE_MASK = {{DMA_LENGTH_WIDTH-BYTES_PER_BEAT{1'b1}},{BYTES_PER_BEAT{1'b0}}}
   localparam ADDR_MASK = {{DMA_AXI_ADDR_WIDTH-BYTES_PER_BEAT{1'b1}},{BYTES_PER_BEAT{1'b0}}};
 
   localparam VAL_DBG_SRC_ADDR = 32'h76543210;
@@ -167,11 +170,12 @@ module dmac_regmap_tb;
     for (i = 0; i < NUM_REGS; i = i + 1)
       expected_reg_mem[i] <= 'h00;
     /* Non zero power-on-reset values */
-    set_reset_reg_value('h00, 32'h00040062); /* PCORE version register */
+    set_reset_reg_value('h00, 32'h00040161); /* PCORE version register */
     set_reset_reg_value('h0c, 32'h444d4143); /* PCORE magic register */
     set_reset_reg_value('h80, 'h3); /* IRQ mask */
 
-    set_reset_reg_value('h40c, 'h1); /* Flags */
+    set_reset_reg_value('h40c, 'h3); /* Flags */
+    set_reset_reg_value('h418, LENGTH_ALIGN_MASK); /* Length alignment */
 
     set_reset_reg_value('h434, VAL_DBG_DEST_ADDR);
     set_reset_reg_value('h438, VAL_DBG_SRC_ADDR);
@@ -250,19 +254,19 @@ module dmac_regmap_tb;
     write_reg_and_update('h414, ADDR_MASK);
     write_reg_and_update('h418, LENGTH_MASK);
     write_reg_and_update('h41c, LENGTH_MASK);
-    write_reg_and_update('h420, LENGTH_MASK);
-    write_reg_and_update('h424, LENGTH_MASK);
+    write_reg_and_update('h420, STRIDE_MASK);
+    write_reg_and_update('h424, STRIDE_MASK);
 
     check_all_registers("Transfer setup 1");
 
     /* Check transfer registers */
     write_reg_and_update('h40c, {$random} & 'h3);
     write_reg_and_update('h410, {$random} & ADDR_MASK);
     write_reg_and_update('h414, {$random} & ADDR_MASK);
-    write_reg_and_update('h418, {$random} & LENGTH_MASK);
+    write_reg_and_update('h418, {$random} & LENGTH_MASK | LENGTH_ALIGN_MASK);
     write_reg_and_update('h41c, {$random} & LENGTH_MASK);
-    write_reg_and_update('h420, {$random} & LENGTH_MASK);
-    write_reg_and_update('h424, {$random} & LENGTH_MASK);
+    write_reg_and_update('h420, {$random} & STRIDE_MASK);
+    write_reg_and_update('h424, {$random} & STRIDE_MASK);
 
     check_all_registers("Transfer setup 2");
 
@@ -329,6 +333,7 @@ module dmac_regmap_tb;
     .BYTES_PER_BEAT_WIDTH_SRC(BYTES_PER_BEAT),
     .DMA_AXI_ADDR_WIDTH(DMA_AXI_ADDR_WIDTH),
     .DMA_LENGTH_WIDTH(DMA_LENGTH_WIDTH),
+    .DMA_LENGTH_ALIGN(DMA_LENGTH_ALIGN),
     .DMA_CYCLIC(1),
     .HAS_DEST_ADDR(1),
     .HAS_SRC_ADDR(1),