From fa0c7b681d0bd149ca9001324d98389a130c30b8 Mon Sep 17 00:00:00 2001 From: Daniel Keller Date: Tue, 16 Jun 2026 14:10:30 +0200 Subject: [PATCH 1/2] frontend: Expose the reg frontend as a native APB slave The PeakRDL register block (#73) is already APB4 internally, but the reg frontend wrapped it behind a register_interface reg_bus slave and a reg_to_apb shim. Drop the shim and present APB directly, matching the desc64 frontend, so every iDMA register interface is PeakRDL-native APB. The launch-on-read backpressure moves from the reg_bus .ready trick onto the external next_id register's rd_ack (gated by the arbiter grant); the PeakRDL APB CPUIF holds s_apb_pready low until the transfer is accepted. This also removes the last register_interface user, so the dependency is dropped from Bender.yml. --- Bender.lock | 10 --- Bender.yml | 1 - .../content/docs/guides/system-integration.md | 10 +-- src/frontend/reg/tpl/idma_reg.sv.tpl | 72 ++++++------------- 4 files changed, 25 insertions(+), 68 deletions(-) diff --git a/Bender.lock b/Bender.lock index caa3519b..dc2bc91e 100644 --- a/Bender.lock +++ b/Bender.lock @@ -44,16 +44,6 @@ packages: dependencies: - common_cells - common_verification - register_interface: - revision: d6e1d4cdaab7870f4faf3f88a1c788eaf5ac129d - version: 0.4.7 - source: - Git: https://github.com/pulp-platform/register_interface.git - dependencies: - - apb - - axi - - common_cells - - common_verification tech_cells_generic: revision: 7968dd6e6180df2c644636bc6d2908a49f2190cf version: 0.2.13 diff --git a/Bender.yml b/Bender.yml index 6f16b87e..167007c4 100644 --- a/Bender.yml +++ b/Bender.yml @@ -18,7 +18,6 @@ dependencies: axi_stream: { git: "https://github.com/pulp-platform/axi_stream.git", version: 0.1.1 } common_cells: { git: "https://github.com/pulp-platform/common_cells.git", version: 1.39.0 } common_verification: { git: "https://github.com/pulp-platform/common_verification.git", version: 0.2.5 } - register_interface: { git: "https://github.com/pulp-platform/register_interface.git", version: 0.4.7 } apb: { git: "https://github.com/pulp-platform/apb.git", version: 0.2.4 } obi: { git: "https://github.com/pulp-platform/obi.git", version: 0.1.7 } diff --git a/doc/site/src/content/docs/guides/system-integration.md b/doc/site/src/content/docs/guides/system-integration.md index 7c7b8898..30439c3a 100644 --- a/doc/site/src/content/docs/guides/system-integration.md +++ b/doc/site/src/content/docs/guides/system-integration.md @@ -63,14 +63,14 @@ The following skeleton shows how the three layers connect. Signal types come fro idma_reg64_2d #( .NumRegs ( 1 ), .NumStreams ( 1 ), - .reg_req_t ( reg_req_t ), - .reg_rsp_t ( reg_rsp_t ), + .apb_req_t ( apb_req_t ), + .apb_rsp_t ( apb_rsp_t ), .dma_req_t ( idma_nd_req_t ) ) i_frontend ( .clk_i, .rst_ni, - .reg_req_i ( dma_reg_req ), - .reg_rsp_o ( dma_reg_rsp ), + .dma_ctrl_req_i ( dma_apb_req ), + .dma_ctrl_rsp_o ( dma_apb_rsp ), .dma_req_o ( fe_req ), .req_valid_o ( fe_valid ), .req_ready_i ( fe_ready ), @@ -208,4 +208,4 @@ Shows multi-core DMA sharing and the mchan/iDMA selection mechanism. The PULP cl ## Dependency Management -iDMA uses [Bender](https://github.com/pulp-platform/bender) for RTL dependency management. Add iDMA to your `Bender.yml` and run `bender update` to pull it and its transitive dependencies (`axi`, `common_cells`, `register_interface`, `obi`). Bender resolves the source file list for your build system — use `bender script vsim` for Questa or `bender script vcs` for VCS. +iDMA uses [Bender](https://github.com/pulp-platform/bender) for RTL dependency management. Add iDMA to your `Bender.yml` and run `bender update` to pull it and its transitive dependencies (`axi`, `common_cells`, `apb`, `obi`). Bender resolves the source file list for your build system — use `bender script vsim` for Questa or `bender script vcs` for VCS. diff --git a/src/frontend/reg/tpl/idma_reg.sv.tpl b/src/frontend/reg/tpl/idma_reg.sv.tpl index 9b878a6c..cebd4269 100644 --- a/src/frontend/reg/tpl/idma_reg.sv.tpl +++ b/src/frontend/reg/tpl/idma_reg.sv.tpl @@ -6,8 +6,6 @@ // - Michael Rogenmoser // - Thomas Benz -`include "apb/typedef.svh" - /// Description: Register-based front-end for iDMA module idma_${identifier} #( /// Number of configuration register ports @@ -18,10 +16,10 @@ module idma_${identifier} #( parameter int unsigned IdCounterWidth = 32'd32, /// Dependent parameter: Stream Idx parameter int unsigned StreamWidth = cf_math_pkg::idx_width(NumStreams), - /// Register_interface request type - parameter type reg_req_t = logic, - /// Register_interface response type - parameter type reg_rsp_t = logic, + /// APB request type. Use the APB_TYPEDEF macros to define it + parameter type apb_req_t = logic, + /// APB response type. Use the APB_TYPEDEF macros to define it + parameter type apb_rsp_t = logic, /// DMA 1d or ND burst request type parameter type dma_req_t = logic, /// Dependent type for IdCounterWidth @@ -31,9 +29,9 @@ module idma_${identifier} #( ) ( input logic clk_i, input logic rst_ni, - /// Register interface control slave - input reg_req_t [NumRegs-1:0] dma_ctrl_req_i, - output reg_rsp_t [NumRegs-1:0] dma_ctrl_rsp_o, + /// APB control slave + input apb_req_t [NumRegs-1:0] dma_ctrl_req_i, + output apb_rsp_t [NumRegs-1:0] dma_ctrl_rsp_o, /// Request signals output dma_req_t dma_req_o, output logic req_valid_o, @@ -51,11 +49,6 @@ module idma_${identifier} #( localparam int unsigned MaxNumStreams = 32'd16; localparam int unsigned RegAddrWidth = idma_${identifier}_reg_pkg::IDMA_${identifier.upper()}_REG_TOP_MIN_ADDR_WIDTH; - `APB_TYPEDEF_ALL(apb, logic[31:0], logic[31:0], logic[3:0]) - apb_req_t [NumRegs-1:0] apb_req; - apb_resp_t [NumRegs-1:0] apb_rsp; - - // register connections idma_${identifier}_reg_pkg::idma_reg__out_t dma_reg2hw [NumRegs-1:0]; idma_${identifier}_reg_pkg::idma_reg__in_t dma_hw2reg [NumRegs-1:0]; @@ -65,9 +58,6 @@ module idma_${identifier} #( logic [NumRegs-1:0] arb_valid; logic [NumRegs-1:0] arb_ready; - // register signals - reg_rsp_t [NumRegs-1:0] dma_ctrl_rsp; - always_comb begin stream_idx_o = '0; for (int r = 0; r < NumRegs; r++) begin @@ -82,50 +72,28 @@ module idma_${identifier} #( // generate the registers for (genvar i = 0; i < NumRegs; i++) begin : gen_core_regs - - reg_to_apb #( - .reg_req_t ( reg_req_t ), - .reg_rsp_t ( reg_rsp_t ), - .apb_req_t ( apb_req_t ), - .apb_rsp_t ( apb_resp_t ) - ) chs_regs_reg_to_apb ( - .clk_i, - .rst_ni, - .reg_req_i ( dma_ctrl_req_i [i] ), - .reg_rsp_o ( dma_ctrl_rsp [i] ), - .apb_req_o ( apb_req [i] ), - .apb_rsp_i ( apb_rsp [i] ) - ); - idma_${identifier}_reg_top i_idma_${identifier}_reg_top ( .clk ( clk_i ), .arst_n ( rst_ni ), - .s_apb_psel (apb_req[i].psel), - .s_apb_penable (apb_req[i].penable), - .s_apb_pwrite (apb_req[i].pwrite), - .s_apb_pprot (apb_req[i].pprot), - .s_apb_paddr (apb_req[i].paddr[RegAddrWidth-1:0]), - .s_apb_pwdata (apb_req[i].pwdata), - .s_apb_pstrb (apb_req[i].pstrb), - .s_apb_pready (apb_rsp[i].pready), - .s_apb_prdata (apb_rsp[i].prdata), - .s_apb_pslverr (apb_rsp[i].pslverr), + .s_apb_psel (dma_ctrl_req_i[i].psel), + .s_apb_penable (dma_ctrl_req_i[i].penable), + .s_apb_pwrite (dma_ctrl_req_i[i].pwrite), + .s_apb_pprot (dma_ctrl_req_i[i].pprot), + .s_apb_paddr (dma_ctrl_req_i[i].paddr[RegAddrWidth-1:0]), + .s_apb_pwdata (dma_ctrl_req_i[i].pwdata), + .s_apb_pstrb (dma_ctrl_req_i[i].pstrb), + .s_apb_pready (dma_ctrl_rsp_o[i].pready), + .s_apb_prdata (dma_ctrl_rsp_o[i].prdata), + .s_apb_pslverr (dma_ctrl_rsp_o[i].pslverr), .hwif_out ( dma_reg2hw [i] ), .hwif_in ( dma_hw2reg [i] ) ); + // A read of `next_id` launches a transfer; stall the APB read (via the + // external register `rd_ack`) until the arbiter accepts the request. logic read_happens; - // DMA backpressure - always_comb begin : proc_dma_backpressure - // ready signal - dma_ctrl_rsp_o[i] = dma_ctrl_rsp[i]; - dma_ctrl_rsp_o[i].ready = read_happens ? arb_ready[i] : dma_ctrl_rsp[i]; - end - - // valid signals - always_comb begin : proc_launch read_happens = 1'b0; for (int c = 0; c < NumStreams; c++) begin @@ -207,7 +175,7 @@ module idma_${identifier} #( assign dma_hw2reg[i].status[c].rd_data.busy = {midend_busy_i[c], busy_i[c]}; assign dma_hw2reg[i].status[c].rd_ack = 1'b1; assign dma_hw2reg[i].next_id[c].rd_data.next_id = next_id_i; - assign dma_hw2reg[i].next_id[c].rd_ack = 1'b1; + assign dma_hw2reg[i].next_id[c].rd_ack = arb_ready[i]; assign dma_hw2reg[i].done_id[c].rd_data.done_id = done_id_i[c]; assign dma_hw2reg[i].done_id[c].rd_ack = 1'b1; end From de385b20c9b3fa7d051f38a46be9adb3cb76547c Mon Sep 17 00:00:00 2001 From: Daniel Keller Date: Thu, 18 Jun 2026 10:38:17 +0200 Subject: [PATCH 2/2] frontend: Gate external register read-ack Acking the external status/next_id/done_id reads unconditionally (or next_id on bare arb_ready) violates the PeakRDL external-read handshake and trips assert_bad_ext_rd_ack every idle cycle. Ack only when the block requests the read; next_id holds the ack until the arbiter accepts. --- src/frontend/reg/tpl/idma_reg.sv.tpl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/frontend/reg/tpl/idma_reg.sv.tpl b/src/frontend/reg/tpl/idma_reg.sv.tpl index cebd4269..aeeea2e7 100644 --- a/src/frontend/reg/tpl/idma_reg.sv.tpl +++ b/src/frontend/reg/tpl/idma_reg.sv.tpl @@ -173,11 +173,15 @@ module idma_${identifier} #( // observational registers for (genvar c = 0; c < NumStreams; c++) begin : gen_hw2reg_connections assign dma_hw2reg[i].status[c].rd_data.busy = {midend_busy_i[c], busy_i[c]}; - assign dma_hw2reg[i].status[c].rd_ack = 1'b1; + assign dma_hw2reg[i].status[c].rd_ack = dma_reg2hw[i].status[c].req + & ~dma_reg2hw[i].status[c].req_is_wr; assign dma_hw2reg[i].next_id[c].rd_data.next_id = next_id_i; - assign dma_hw2reg[i].next_id[c].rd_ack = arb_ready[i]; + assign dma_hw2reg[i].next_id[c].rd_ack = dma_reg2hw[i].next_id[c].req + & ~dma_reg2hw[i].next_id[c].req_is_wr + & arb_ready[i]; assign dma_hw2reg[i].done_id[c].rd_data.done_id = done_id_i[c]; - assign dma_hw2reg[i].done_id[c].rd_ack = 1'b1; + assign dma_hw2reg[i].done_id[c].rd_ack = dma_reg2hw[i].done_id[c].req + & ~dma_reg2hw[i].done_id[c].req_is_wr; end // tie-off unused channels