Skip to content
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

[topgen] Add support for instantiating uniquified ipgen IPs #25966

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Jan 21, 2025

This PR adds basic support to render and instantiate uniquified IPs from ipgen. The change is done in the following way:

  1. It is assumed that all ipgen IP templates support the module_instance_name parameter. This is not true for all IPs there. Though this parameter was added to all *.tpldesc files, even it is not used.
  2. In topgen, when you instantiate an ipgen'ed (attr: "ipen"), you specify three different fields:
    1. template_type: This is the basic template type that is used to render the IP from.
    2. type: This is the uniquified type.
    3. name: This is the actual instance name used in the top-level SystemVerilog file.

Let's make an example: To render a uniquified rv_plic you would set the following parameters in the HJSON:

{
  template_type: "rv_plic"
  type:          "my_fancy_plic"
  name:          "my_plic_name"
  ...
}

This would render and create new module my_fancy_plic and the instantiation would look like:

my_fancy_plic #(
   ...
 ) u_my_plic_name (
   ...
 );

Note, currently, only the AC ranges, RACL Ctrl, and the PLIC support this kind of uniquification. The alert handler would support it in theory. However, here, the template needs a few modifications as the uniquication there does create wrong file names.

@Razer6 Razer6 requested a review from msfschaffner as a code owner January 21, 2025 21:06
@Razer6 Razer6 requested review from a-will, matutem, rswarbrick and vogelpi and removed request for msfschaffner January 21, 2025 21:06
@Razer6 Razer6 force-pushed the topgen-uniquified-instances branch from a265065 to f051d9e Compare January 21, 2025 21:13
Copy link
Contributor

@a-will a-will left a comment

Choose a reason for hiding this comment

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

LGTM

util/topgen.py Show resolved Hide resolved
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

LGTM

util/topgen.py Show resolved Hide resolved
@vogelpi
Copy link
Contributor

vogelpi commented Jan 23, 2025

CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson

This PR adds some now keys to the Earlgrey hjson but there is no RTL or functional impact of this. This is fine.

Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

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

A couple of notes about improvements that are probably worth doing, but they can be in a follow-up so shouldn't block this PR.

util/topgen/lib.py Show resolved Hide resolved
@rswarbrick
Copy link
Contributor

CHANGE AUTHORIZED: hw/top_earlgrey/data/top_earlgrey.hjson

We gain some new keys in the hjson file, but these won't have any effect on the Earlgrey configuration.

@vogelpi
Copy link
Contributor

vogelpi commented Jan 23, 2025

Thanks for your reviews, I am merging this to unblock follow-up work but it would still be good if @Razer6 could address the points you've raised.

@vogelpi vogelpi merged commit 27932fe into lowRISC:master Jan 23, 2025
38 checks passed
@Razer6 Razer6 deleted the topgen-uniquified-instances branch January 23, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants