-
Notifications
You must be signed in to change notification settings - Fork 22
SystemRDL-based register interface for HWPE controller #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…erface description
…e hwpe_ctrl_target as companion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request modernizes the HWPE controller register interface by introducing a SystemRDL-based approach that replaces the legacy manual register file implementation. The changes include a reference SystemRDL description defining mandatory and customizable registers, a code generation script using PeakRDL, and a new hwpe_ctrl_target module compatible with generated register interfaces. Legacy modules are preserved in a deprecated directory for backward compatibility.
Key changes:
- Added SystemRDL-based register interface generation workflow (
.rdlfile + generation script) - Introduced new
hwpe_ctrl_target.svmodule for SystemRDL-generated register file integration - Moved legacy register file modules (
hwpe_ctrl_slave,hwpe_ctrl_regfile*) todeprecated/directory
Reviewed changes
Copilot reviewed 4 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| rtl/rdl.sh | Shell script for generating SystemVerilog, HTML, and C header files from SystemRDL using PeakRDL |
| rtl/hwpe_ctrl_regif_example.rdl | Reference SystemRDL description defining mandatory HWPE control registers and example register sets |
| rtl/hwpe_ctrl_target.sv | New target module implementing job queueing and soft clear with SystemRDL register interface |
| rtl/deprecated/hwpe_ctrl_slave.sv | Legacy slave module moved to deprecated directory |
| rtl/deprecated/hwpe_ctrl_regfile*.sv | Legacy register file implementation modules moved to deprecated directory |
| Bender.yml | Updated build configuration to reference new files and deprecated directory paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool to see SystemRDL getting more adoption🤓
I can't comment on the logic itself, but I can maybe give some feedback on SystemRDL. There is an official SystemRDL style guide, which you are already following mostly, except for the prefixes. The nice thing about SystemRDL is that it is a hierarchical language and its not possible to create naming collisions! So you can generally lose the hpwe_ prefixes everywhere inside the addrmap (which already has it). Then, the names and types also become a lot shorter, which makes the SystemVerilog code a bit nicer.
But other than that, it looks very clean!
| desc = "Control register map for the HWPE, including mandatory control/status registers and example job-independent and job-dependent configuration registers."; | ||
|
|
||
| // Mandatory COMMIT_TRIGGER register. Not to be updated inside HWPEs. | ||
| reg hwpe_commit_trigger { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the hwpe_ prefix here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I will fix this.
| hw = w; | ||
| sw = r; | ||
| swacc = true; | ||
| } acquire[31:0] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit annoying in SystemRDL that you need to create a field container in reg, even if it uses the full regwidth. Because then accessing it requires hwpe_acquire.acquire, which feels a bit redundant. One way to (slightly) improve this is to call the the reg by its name i.e. acquire and then the field just value or val.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I will do that.
This pull request introduces a new SystemRDL-based register interface and target module for the HWPE controller, modernizing the register file integration and deprecating the legacy register file modules (which are kept available). The changes provide a reference SystemRDL description, a script for generating register interface artifacts, and a new
hwpe_ctrl_targetmodule designed to work with the generated interface. The Bender configuration is updated to use these new files and move legacy files to adeprecateddirectory.This new register interface has been tested in NEureka (feature branch here: https://github.com/pulp-platform/neureka/tree/fc/rdl).
SystemRDL-based Register Interface Integration:
hwpe_ctrl_regif_example.rdlto define the HWPE control port's mandatory and customizable registers.rdl.shto generate SystemVerilog, HTML, and C header files from the SystemRDL file using PeakRDL, and to patch the generated structs for compatibility.New Target Module:
hwpe_ctrl_target.svmodule, which implements a target port compatible with SystemRDL-generated register files, replacing the deprecatedhwpe_ctrl_slave. This module handles job queueing, soft clear, and register interfacing, and is designed for easy integration with custom register sets.Project Configuration Updates:
Bender.ymlto reference the new SystemRDL-based files and move legacy register file modules to adeprecateddirectory, ensuring new designs use the modernized flow.