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] Support for parametrized inter-module signals #23672

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Jun 13, 2024

This PR adds support for parametrized inter-module signals. Previously, when an inter-module signal depended on a parameter, you needed to specify the width value twice. Once for the parameter and once when the value is used for the width of the inter-module signal.

This PR fixes that and adds support for parameters being used for the width of an inter-module signal. When you want to do that, you need to expose the parameter. If local is set, the parameter will be created as localparam in the top-level file, and the interconnection uses that parameter. If local is false, a top-level parameter is created, but also a local parameter within the reg_pkg of the IP.

The PR is separated into 2 parts:

  1. The infrastructure implemetnation
  2. Using that feature for the KMAC NumAppIntf parameter

@Razer6 Razer6 requested a review from msfschaffner as a code owner June 13, 2024 21:06
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from 91bc953 to eaae82c Compare June 13, 2024 21:07
@Razer6 Razer6 changed the title First pass [topgen] Support for parametrized inter-module signals Jun 13, 2024
@Razer6 Razer6 marked this pull request as draft June 13, 2024 21:08
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from eaae82c to 078b17c Compare June 14, 2024 07:21
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from 078b17c to 755e658 Compare November 8, 2024 14:16
@Razer6 Razer6 changed the base branch from integrated_dev to master November 8, 2024 14:16
@Razer6 Razer6 force-pushed the top-parametrized-ports branch 2 times, most recently from 87e5060 to 92bd869 Compare November 8, 2024 16:03
@Razer6 Razer6 requested review from rswarbrick, a-will, andreaskurth and matutem and removed request for msfschaffner November 8, 2024 16:04
@Razer6 Razer6 marked this pull request as ready for review November 8, 2024 16:05
@Razer6 Razer6 requested a review from a team as a code owner November 8, 2024 16:05
@Razer6 Razer6 removed the request for review from a team November 8, 2024 16:05
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from 92bd869 to 0890bc3 Compare November 8, 2024 16:20
hw/ip/kmac/doc/interfaces.md Outdated Show resolved Hide resolved
@Razer6 Razer6 force-pushed the top-parametrized-ports branch 11 times, most recently from f64038f to 78d0a43 Compare November 9, 2024 13:38
@Razer6 Razer6 force-pushed the top-parametrized-ports branch 10 times, most recently from 72776ab to b3e1a0f Compare November 10, 2024 14:08
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from b3e1a0f to d34c78e Compare November 11, 2024 08:26
hw/top_darjeeling/templates/toplevel.sv.tpl Outdated Show resolved Hide resolved
hw/top_darjeeling/templates/toplevel.sv.tpl Outdated Show resolved Hide resolved
util/reggen/inter_signal.py Outdated Show resolved Hide resolved
util/topgen/intermodule.py Outdated Show resolved Hide resolved
util/topgen/intermodule.py Show resolved Hide resolved
hw/ip/kmac/data/kmac.hjson Outdated Show resolved Hide resolved
hw/ip/kmac/dv/env/kmac_env_pkg.sv Outdated Show resolved Hide resolved
@Razer6 Razer6 force-pushed the top-parametrized-ports branch 5 times, most recently from 98e6e7b to 31f553e Compare November 11, 2024 12:24
@Razer6 Razer6 force-pushed the top-parametrized-ports branch 2 times, most recently from e7c2eca to 4e7315f Compare November 11, 2024 14:32
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

LGTM except for one open thread and a formatting comment. Thx @Razer6!

hw/ip/kmac/rtl/kmac_pkg.sv Outdated Show resolved Hide resolved
@Razer6 Razer6 force-pushed the top-parametrized-ports branch from 4e7315f to 83f8307 Compare November 12, 2024 15:31
Comment on lines +87 to +88
parameter kmac_pkg::app_config_t KmacAppCfg[KmacNumAppIntf] =
'{kmac_pkg::AppCfgKeyMgr, kmac_pkg::AppCfgLcCtrl, kmac_pkg::AppCfgRomCtrl},
Copy link
Contributor

Choose a reason for hiding this comment

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

This array is declared in an "up to" (i.e., [0:KmacNumAppIntf-1]) manner, so to my understanding index 0 is for keymgr, index 1 for lc_ctrl, and index 2 for rom_ctrl -- which is equivalent to the previous definition in kmac_pkg. LGTM.

@andreaskurth
Copy link
Contributor

CHANGE AUTHORIZED: hw/ip/kmac/data/kmac.hjson
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac.sv
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac_app.sv
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac_pkg.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

These changes have no functional impact on Earlgrey.

@vogelpi
Copy link
Contributor

vogelpi commented Nov 12, 2024

CHANGE AUTHORIZED: hw/ip/kmac/data/kmac.hjson
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac.sv
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac_app.sv
CHANGE AUTHORIZED: hw/ip/kmac/rtl/kmac_pkg.sv
CHANGE AUTHORIZED: hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

These changes have no functional impact on Earlgrey. This is fine.

@andreaskurth andreaskurth merged commit 9a0ca1d into lowRISC:master Nov 13, 2024
41 checks passed
@Razer6 Razer6 deleted the top-parametrized-ports branch November 13, 2024 09:57
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