Skip to content

PR to merge chips-IT changes to iDMA#109

Merged
DanielKellerM merged 7 commits into
pulp-platform:develfrom
FondazioneChipsIT:rg/pulp_cluster_pr
Jun 16, 2026
Merged

PR to merge chips-IT changes to iDMA#109
DanielKellerM merged 7 commits into
pulp-platform:develfrom
FondazioneChipsIT:rg/pulp_cluster_pr

Conversation

@RiccardoGandolfi

@RiccardoGandolfi RiccardoGandolfi commented May 27, 2026

Copy link
Copy Markdown

Just added a parameter to the iDMA_legalizer_page_splitter module.

Related to this PR: pulp-platform/pulp_cluster#115

@RiccardoGandolfi RiccardoGandolfi self-assigned this May 27, 2026
@yvantor

yvantor commented Jun 1, 2026

Copy link
Copy Markdown

@RiccardoGandolfi make sure you align the parameter definition to the IP name convention (i.s. BurstLen instead of Burst_len). I would also move it before type parameters since it is a pure number (so I would group it with the other int unsigned parameters).

@RiccardoGandolfi

RiccardoGandolfi commented Jun 8, 2026

Copy link
Copy Markdown
Author

@RiccardoGandolfi make sure you align the parameter definition to the IP name convention (i.s. BurstLen instead of Burst_len). I would also move it before type parameters since it is a pure number (so I would group it with the other int unsigned parameters).

Should be done now

@RiccardoGandolfi RiccardoGandolfi requested a review from yvantor June 9, 2026 07:33
@RiccardoGandolfi RiccardoGandolfi force-pushed the rg/pulp_cluster_pr branch 3 times, most recently from 81b2ee6 to 98bd341 Compare June 9, 2026 12:53
@RiccardoGandolfi RiccardoGandolfi changed the base branch from master to devel June 10, 2026 09:20
DanielKellerM
DanielKellerM requested changes Jun 15, 2026
View reviewed changes

@DanielKellerM DanielKellerM left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things:

  1. Rebase first

  2. Default regression: BurstLen defaults to 5, but the splitter was hardcoded to 8, so every existing variant's default burst drops 2048B→256B on a 64-bit bus (8×). The cap belongs to a specific system; keep the default at 8.

  3. These are DB values: 'd8 = log2(max_beats_per_burst=256) and 'd12 = log2(page_size=4096) from idma_axi.yml. The legalizer already derives bursts from these per-protocol; the page splitter should take log2(max_beats_per_burst)/log2(page_size) from the DB (as idma_legalizer_pow2_splitter already does) instead of a hand-set param.

For the 256B bursts themselves, the reg frontend already exposes reduce_len/max_llen - set reduce_len=1, max_llen=5, no RTL change.

DanielKellerM
DanielKellerM reviewed Jun 15, 2026
View reviewed changes
Comment thread src/backend/idma_legalizer_page_splitter.sv Outdated
DanielKellerM
DanielKellerM reviewed Jun 16, 2026
View reviewed changes

@DanielKellerM DanielKellerM left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix + rebase. But the effective default for every generated variant is still 5. Same change needed here.

Comment thread src/backend/tpl/idma_backend.sv.tpl Outdated
Comment thread src/backend/tpl/idma_backend_synth.sv.tpl Outdated
Comment thread src/backend/tpl/idma_legalizer.sv.tpl Outdated
RiccardoGandolfi and others added 3 commits June 16, 2026 09:56
@RiccardoGandolfi @DanielKellerM
Update src/backend/tpl/idma_backend_synth.sv.tpl
2390a57 
Co-authored-by: Daniel Keller <daniel.keller.m@gmail.com>
@RiccardoGandolfi @DanielKellerM
Update src/backend/tpl/idma_backend.sv.tpl
f0efc1e 
Co-authored-by: Daniel Keller <daniel.keller.m@gmail.com>
@RiccardoGandolfi @DanielKellerM
Update src/backend/tpl/idma_legalizer.sv.tpl
c0627e7 
Co-authored-by: Daniel Keller <daniel.keller.m@gmail.com>
DanielKellerM
DanielKellerM approved these changes Jun 16, 2026
View reviewed changes

@DanielKellerM DanielKellerM left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaults restored to 8 across all four files.

@DanielKellerM DanielKellerM merged commit 44a3320 into pulp-platform:devel Jun 16, 2026
12 checks passed
DanielKellerM added a commit that referenced this pull request Jun 16, 2026
@DanielKellerM
frontend: Drop the duplicate reg tie-off and dead hjson template
6f8130a 
PR #109's rebased NumStreams commit resurrected idma_reg.hjson.tpl
(removed by the SystemRDL conversion in #73) and re-added a second
gen_hw2reg_unused generate block in idma_reg.sv.tpl. The duplicate label
breaks elaboration of the reg block on every variant (DC VER-288, VCS
IPD, Questa vlog-2388), failing the devel pipeline. Keep only #109's
BurstLen change.
DanielKellerM added a commit that referenced this pull request Jun 16, 2026
@DanielKellerM
frontend: Remove the duplicate reg tie-off generate block
05d088f 
The second gen_hw2reg_unused block (#109's resurrected NumStreams churn)
duplicates the SystemRDL one in the same scope, breaking elaboration on
every variant (DC VER-288, VCS IPD, Questa vlog-2388). Completes the
devel-build fix started in 6f8130a (which only removed the dead hjson).
@DanielKellerM DanielKellerM mentioned this pull request Jun 16, 2026
Draft
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DanielKellerM DanielKellerM DanielKellerM approved these changes
@thommythomaso thommythomaso Awaiting requested review from thommythomaso thommythomaso is a code owner
@yvantor yvantor Awaiting requested review from yvantor
@micprog micprog Awaiting requested review from micprog micprog is a code owner

Assignees

@RiccardoGandolfi RiccardoGandolfi

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RiccardoGandolfi @yvantor @DanielKellerM