idma: Streamline TCDM connection, enable multi-channel operation by gbellocchi · Pull Request #330 · pulp-platform/snitch_cluster

gbellocchi · 2026-08-04T13:21:55Z

Description

This PR updates the iDMA integration within the Snitch cluster, as shown in Figures 1 and 2.
It is a refreshed version of PR #238 , which had fallen behind the main snitch cluster branch due to being open for a long period, and a continuation of PR #322.

New features include:

The wide SoC port directly connects to the TCDM subsystem, allowing external data access while DMA transfers run.
The wide DMA XBAR has been simplified.
The DMA uses the OBI protocol to connect to the TCDM subsystem.
Arbitration between the channels, the superbanks, and the SoC port happens now in TCDM, increasing flexibility and throughput. With minimal changes, it is possible to have multiple DMA cores in the cluster.
Add DMINIT support in Snitch cluster.
Add support for OBI events and performance counters.
Add L1-to-L1 and TLS tests to investigate TCDM deadlock.
Support iDMA after common-cells-v2 update.

Tasks

I have collected the tasks and reviewer comments/suggestions of PR #88:

Figure 1: Old integration of `idma`. The wide AXI4 XBAR interconnect is used for: (i) DMA interfaces to TCDM and SoC; (ii) NoC wide in/out; (iii) I$; (iv) Zero memory; and (v) BootROM.

Figure 2: New integration of `idma`. The wide AXI4 XBAR interconnect is simplified: (i) DMA interfaces to TCDM via OBI and wide AXI4 requests are transmitted to other clusters via the AXI4 XBAR (NoC wide out); (ii) Remove AXI4 TCDM port and directly interface the NoC wide inputs of external DMAs to the TCDM subsystem (bypassing the XBAR); (iii) Remove zero memory as the iDMA now supports memset initialization of TCDM.

* `axi_zero_mem` is removed because the idma can now initialize the memory to a desired value.

* Modify the wide cluster xbar address map based on the removal of the zero memory and the new idma-tcdm integration. * Update the cluster and dma enums in the `snitch_pkg`.

The wide `soc_in_axi_req` is directly interfaced with the tcdm subsystem and bypasses the wide axi cluster xbar.

* `snitch_cluster`: Add memory and obi typedefs. * `snitch_cluster`: Add obi-to-tcdm protocol conversion for dma requests toward the tcdm subsystem. * `snitch_cluster`: Update interface of `snitch_cc` instance. * `snitch_cc`: Instantiate `idma` with obi interfaces.

idma: Fix dminit opcode encoding and add TCDM tests

* Avoid the flattening of arrays in the tcdm dma interconnect.

* Fix deadlock in tcdm-to-tcdm idma transfers, which originates from the absence of p_valid for write transactions. * Add write pipeline shift register (mirroring id_pipeline) for tracking whether an in-flight slot is a write.

* Add GUI version of `vsim` among the `run.py` simulators. * Add `wave-file` argument to specify a corresponding wave file to automatically source when launching the `vsim-gui` simulator. * Add support for wave argument in the generated `snitch_cluster.vsim.gui` script.

* Tie off the `obi_dma_req_o` when no DMA is instantiated in the `snitch_cc`.

* Tie off undriven signals to avoid having undefined behaviors. * Add documentation for the obi-to-tcdm bridge.

* hw: Update hw configuration files and templates. * hw: Remove commented lines in `snitch_cluster.sv` for address remapping after zeromem removal. * sw: Update experiment json configuration files.

* Remove hardcoded reference to DM core as core 8. * Update dma wait api.

* Map arrays to l1 in order to test l1-to-l1 dma transfers. To this end, use `snrt_l1_alloc()` to initialize `src` and `dst` arrays. * Extend the range of traffic sizes to trigger the l1-to-l1 deadlock experienced with other kernels (exp).

* This test assesses proper tls initialization at runtime (sanity check) and modification at application-time (core isolation).

* This test concerns the traffic patterns used in `snrt_init_tls` at runtime. * Parameters are currently tuned on the specific case of the `exp` kernel.

The pinned ref __deploy__9785d30__common-cells-v2 was the deploy branch of the old common_cells v2 PR head. That PR was squash-merged as 14dcdf1a and the deploy branch was deleted, so bender could not resolve it. Point at __deploy__ba01de2__devel instead, the deploy branch of the current iDMA devel tip and the one snitch_cluster PR pulp-platform#330 uses. snitch_cc.sv includes obi/typedef.svh but obi was never declared here, so its export_include_dirs never reached the compile line and vcs failed with "Source file obi/typedef.svh cannot be opened".

Bender.yml iDMA -> 0.7.0 (the release carries the merged MX quant/dequant compute, the single-sourced compute_op encoding, and common_cells v2, matching main's common_cells v2 base). The MX tests and compute helpers move to the per-tile macro idiom of the reshaped heterogeneous-tile address map (GW_L2_SPM_*(i), 4 KiB DMA window); the backend instantiation drops testmode_i (cc-v2 iDMA removed the test-mode ports); snitch_cluster is pinned to dk/idma070 (DanielKellerM fork), which threads the obi/init type parameters iDMA 0.7.0 added to idma_inst64_top - an interim shim converging with pulp-platform/snitch_cluster#330, to be dropped when it merges.

colluca · 2026-09-03T09:19:44Z

+      .TCDMAliasEnable  (TCDMAliasEnable),
+      .DMATracing       (1),


What is this needed for?

I think @DanielKellerM is a better catch for this question. :)

.TCDMAliasEnable required for the new address decoder to map OBI / TCDM requests. I would leave this for now like this, but its a bit unnecessary since its the point of this PR to decouple TCDM port from the xbar.

.DMATracing this was already here, so nothing new. I have a improvement to remove the hardcoded 1 and use the debug flag #329

Still not clear why 1 is needed. Isn't passing the addrmap sufficient?

yes it is. This can be removed indeed

colluca · 2026-09-03T12:50:00Z

+  register_interface:
+    git: https://github.com/pulp-platform/register_interface.git
+    rev: common-cells-v2


Where is this needed?

This dependency was resolved through idma. However, this was dropped at a certain point, thus breaking some Snitch components. At that time, snitch_ssr tests were the first to break. Hence, I expressed the dependency directly in the cluster.

Yes we can keep it, but its not required in the iDMA anymore

What do you mean by the snitch_ssr tests would break? What exactly would break and how?

I am referring to hw/snitch_ssr/test/fixture_ssr.sv, which breaks at vsim build time because of the missing reg_test defined in register_interface.

To this end, I removed register_interface from the dependency list, rebuilt the HW, and this is the resulting error log for reference:

# ** Error: hw/snitch_ssr/test/fixture_ssr.sv(216): (vlog-2164) Class or package 'reg_test' not found. # ** Error: (vlog-13069) hw/snitch_ssr/test/fixture_ssr.sv(216): near "::": syntax error, unexpected ::. # ** Error: hw/snitch_ssr/test/fixture_ssr.sv(216): (vlog-13205) Syntax error found in the scope following 'reg_test'. Is there a missing '::'? # ** Error: hw/snitch_ssr/test/fixture_ssr_streamer.sv(284): (vlog-2164) Class or package 'reg_test' not found. # ** Error: (vlog-13069) hw/snitch_ssr/test/fixture_ssr_streamer.sv(284): near "::": syntax error, unexpected ::. # ** Error: hw/snitch_ssr/test/fixture_ssr_streamer.sv(284): (vlog-13205) Syntax error found in the scope following 'reg_test'. Is there a missing '::'?

colluca · 2026-09-03T12:50:55Z

+    parser.add_argument(
+        '--wave-file',
+        default=None,
+        metavar='WAVE_DO',
+        help='Path to a QuestaSim waveform .do file to load at startup (vsim-gui only)')
    return parser


+def apply_wave_file(args):
+    """Export QUESTA_WAVE_DO so the vsim-gui binary picks it up.
+
+    Nothings happen when --wave-file is not passed or the simulator is not vsim-gui.
+    """
+    if getattr(args, 'wave_file', None) is not None:
+        os.environ['QUESTA_WAVE_DO'] = str(Path(args.wave_file).resolve())
+


Should this be reverted? I don't see the QUESTA_WAVE_DO variable used anywhere.

This was supported via a mk target that was then removed here. I generally like loading custom wave files in RTL simulation, hence why I added support for this at a certain point. Anyway, it is no critical feature.

Oh okay I see. I'm not exactly sure how this is intended to be used though. In general, it might be a valuable addition, but maybe we can move it to a separate PR, where it can be better documented. Otherwise, I would revert it here.

The associated offset parameters become useless after the recent modification to the wide XBAR and additional support for multiple `idma` channels. `ICache` is not used and would break with the use of >1 `idma` channel since its offset is fixed. Thus, the only useful parameter is `SDMAMst` that is now directly handled in the `snitch_cluster` based on the value of `DMANumChannels`.

Remove `zero_mem` dependency from the `snitch_cluster` address map. This leaves 64kB unmapped between `cluster_periph` and `ext_mem`.

Make this aligned with other cluster signal namings.

The `xdma_mask` is removed and substituted with `supports_xdma`, which mirrors the functionality of `supports_xpulp`. `xdma_mask`. The cluster body is then modified accordingly.

Remove 64kB unmapped address region between `cluster_periph` and `ext_mem`. These are now contiguous.

Move the `obi_to_tcdm` converter inside the `snitch_cc` and directly expose the TCDM interface.

The previous function was initializing a 2D array, but `snrt_dma_memset` cannot do this. So loop over rows with it.

Brings the INIT channel and DMA event typedef macros, and the inst64 buffer-stall event fix dropped by iDMA pulp-platform#187.

iDMA owns these types since pulp-platform/iDMA#229. The event struct loses aw_bw, ar_bw and w_bw, which nothing reads; the peripheral derives those counters from aw_len/aw_size, ar_len/ar_size and num_bytes_written.

The branch had fallen 3 commits behind. None of them touch extensions/unratified/rv_xdma, and hw/snitch/src/snitch_riscv_instr.sv is byte-identical to main, so no instruction encoding changes.

colluca

We're almost there 😁

colluca · 2026-09-17T15:04:04Z

+    typedef enum logic {
+    TcdmDma   = 0,
+    ToSoc     = 1
+  } dma_e;



I don't remember if we discussed this anymore now, but ToSoc seems to not be used anywhere. I don't see a value in having this enumeration; the name doesn't have any meaning either.

We can just hardcode 0 in the DMA addrmap, where TcdmDma is used. If we wanted to do this properly, the iDMA should probably define such a type:

typedef enum logic { DmaObi = 0, DmaAxi = 1 } dma_path_e;

colluca · 2026-09-17T15:16:23Z

+ * @note The function passes the @p channel argument as an immediate,
+ *       thus this must be known at compile time. As a consequence, the
+ *       function must always be inlined. This is true also for all functions
+ *       invoking this function, and passing down an argument to @p channel.
 */


This used to be the way channel was handled in most functions, e.g. the dma_start_1d function. But now we are no longer passing channel as an immediate, which is not very robust.

Can you align the new functions you added to this behaviour?

colluca · 2026-09-17T15:22:34Z

+  register_interface:
+    git: https://github.com/pulp-platform/register_interface.git
+    rev: common-cells-v2


What do you mean by the snitch_ssr tests would break? What exactly would break and how?

colluca · 2026-09-17T15:25:01Z

+    parser.add_argument(
+        '--wave-file',
+        default=None,
+        metavar='WAVE_DO',
+        help='Path to a QuestaSim waveform .do file to load at startup (vsim-gui only)')
    return parser


+def apply_wave_file(args):
+    """Export QUESTA_WAVE_DO so the vsim-gui binary picks it up.
+
+    Nothings happen when --wave-file is not passed or the simulator is not vsim-gui.
+    """
+    if getattr(args, 'wave_file', None) is not None:
+        os.environ['QUESTA_WAVE_DO'] = str(Path(args.wave_file).resolve())
+


Oh okay I see. I'm not exactly sure how this is intended to be used though. In general, it might be a valuable addition, but maybe we can move it to a separate PR, where it can be better documented. Otherwise, I would revert it here.

gbellocchi and others added 30 commits June 2, 2026 23:12

hw: Add decode support for dminit instruction

58085fa

sw: Add sw support for dminit tests

77cc080

hw: Remove axi_zero_mem from snitch_cluster

8ceb732

* `axi_zero_mem` is removed because the idma can now initialize the memory to a desired value.

hw: Update cluster and dma address map

ac20e15

* Modify the wide cluster xbar address map based on the removal of the zero memory and the new idma-tcdm integration. * Update the cluster and dma enums in the `snitch_pkg`.

hw: Connect wide soc input to tcdm subsystem

57fb5f4

The wide `soc_in_axi_req` is directly interfaced with the tcdm subsystem and bypasses the wide axi cluster xbar.

hw: Add obi-to-tcdm protocol converter

92503d3

hw: Update NrWideMasters in snitch_cluster_pkg template

9a07527

bender: Add obi dependency

7068914

bender: Change idma deploy reference commit

d3c922b

docs: Add documentation for dminit custom instruction

fdccf94

test: Add L1-to-L1 TCDM iDMA copy regression test

96c613d

sw: Fix dminit opcode encoding (funct7 and rd register)

7710da8

test: Fence buffer init before the DMA in dma_dminit

4d0e8c2

Merge pull request #1 from DanielKellerM/dkeller/idma-dminit-fixes

eebedeb

idma: Fix dminit opcode encoding and add TCDM tests

hw: Refactor arrays in tcdm dma interconnect

85c7778

* Avoid the flattening of arrays in the tcdm dma interconnect.

hw: Fix OBI write response deadlock in obi_to_tcdm bridge

f61240c

* Fix deadlock in tcdm-to-tcdm idma transfers, which originates from the absence of p_valid for write transactions. * Add write pipeline shift register (mirroring id_pipeline) for tracking whether an in-flight slot is a write.

hw: Tie off unused obi_dma_req_o if no dma

3bfdc22

* Tie off the `obi_dma_req_o` when no DMA is instantiated in the `snitch_cc`.

hw: Tie off undriven signals in obi-to-tcdm bridge

d787bbc

* Tie off undriven signals to avoid having undefined behaviors. * Add documentation for the obi-to-tcdm bridge.

hw: Update id width on dma master side

d99ab93

Bump idma deploy

49dc326

Remove occurrences of zeromem

6411a68

* hw: Update hw configuration files and templates. * hw: Remove commented lines in `snitch_cluster.sv` for address remapping after zeromem removal. * sw: Update experiment json configuration files.

sw: Uodate gemm to use dminit memset

26e011a

sw: Move zero_mem test into deprecated

9ed1c71

sw: Improve dma_dminit portability

1063499

* Remove hardcoded reference to DM core as core 8. * Update dma wait api.

sw: Fix dma_l1l1 to allocate buffers in l1

1b7a926

* Map arrays to l1 in order to test l1-to-l1 dma transfers. To this end, use `snrt_l1_alloc()` to initialize `src` and `dst` arrays. * Extend the range of traffic sizes to trigger the l1-to-l1 deadlock experienced with other kernels (exp).

sw: Add dma test for local-to-remote tcdm transfer

7dcbd40

sw: Add tls test for init and update

d1e4164

* This test assesses proper tls initialization at runtime (sanity check) and modification at application-time (core isolation).

sw: Add test to assess dma traffic pattern of snrt_init_tls

c85aefa

* This test concerns the traffic patterns used in `snrt_init_tls` at runtime. * Parameters are currently tuned on the specific case of the `exp` kernel.

colluca requested changes Sep 3, 2026

View reviewed changes

colluca reviewed Sep 3, 2026

View reviewed changes

Comment thread sw/deps/riscv-opcodes

gbellocchi added 17 commits September 3, 2026 15:24

hw: Remove zero_mem from cluster address map

61e7e4f

Remove `zero_mem` dependency from the `snitch_cluster` address map. This leaves 64kB unmapped between `cluster_periph` and `ext_mem`.

hw: Modify soc_in_axi naming

9ab955c

Make this aligned with other cluster signal namings.

sw: Fix comment typo.

9f912b1

hw: Remove suffix for axi_to_mem_split

6f0b4ee

sw: Use alloc_v2 functions for dma tests

60b6d78

hw: Move addr_rule_t before derived parameter list

7d85e69

hw: Remove xdma_mask

ab86799

The `xdma_mask` is removed and substituted with `supports_xdma`, which mirrors the functionality of `supports_xpulp`. `xdma_mask`. The cluster body is then modified accordingly.

hw: Align acronym capitalization to google styleguides

de3f731

hw: Remove unmapped region between cluster_periph and ext_mem

f288081

Remove 64kB unmapped address region between `cluster_periph` and `ext_mem`. These are now contiguous.

hw: Move obi_to_tcdm inside the snitch_cc

2862ff4

Move the `obi_to_tcdm` converter inside the `snitch_cc` and directly expose the TCDM interface.

hw: Tie off dma_events closer to their assignment

11443d9

hw: Move dma interconnect signal assignment before block instance

1d0de3b

hw: Fix gen_dma_events_stub naming

5d5104b

sw: Fix gemm memset initialization

21dc9f5

The previous function was initializing a 2D array, but `snrt_dma_memset` cannot do this. So loop over rows with it.

sw: Fix linting error.

6381265

sw: Align snrt_dma_memset to other dma function signatures

5632348

DanielKellerM mentioned this pull request Sep 16, 2026

hw: Repair the build against current iDMA #337

Closed

DanielKellerM added 3 commits September 17, 2026 11:44

deps: Repin iDMA to the current devel deploy branch

062f15b

Brings the INIT channel and DMA event typedef macros, and the inst64 buffer-stall event fix dropped by iDMA pulp-platform#187.

hw: Use the iDMA typedef macros for the DMA event and INIT structs

b333716

iDMA owns these types since pulp-platform/iDMA#229. The event struct loses aw_bw, ar_bw and w_bw, which nothing reads; the peripheral derives those counters from aw_len/aw_size, ar_len/ar_size and num_bytes_written.

deps: Bump riscv-opcodes forward to the pointer on main

fc630dd

The branch had fallen 3 commits behind. None of them touch extensions/unratified/rv_xdma, and hw/snitch/src/snitch_riscv_instr.sv is byte-identical to main, so no instruction encoding changes.

colluca requested changes Sep 17, 2026

View reviewed changes

This was referenced Sep 21, 2026

frontend: Make the inst64 TCDM and OBI topology a parameter pulp-platform/iDMA#233

Merged

deps: Bump iDMA to 0.7.0 and select the AXI-only inst64 topology #338

Open

gbellocchi added 2 commits September 22, 2026 09:55

test: Fix commented simulator specification

7251be2

chore(gitlab): Remove sim target for perf reports

cadfcb9

Conversation

gbellocchi commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tasks

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielKellerM Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielKellerM Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

colluca left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gbellocchi commented Aug 4, 2026 •

edited

Loading

DanielKellerM Sep 17, 2026 •

edited

Loading

DanielKellerM Sep 21, 2026 •

edited

Loading