[Darjeeling] Instantiate Darjeeling #25283

Razer6 · 2024-11-21T09:03:03Z

No description provided.

Signed-off-by: Robert Schilling <[email protected]>

Add jtag_id_pkg.sv, padring.sv, scan_role_pkg, ibex_pmp_reset_pkg.sv padring.sv, top_pkg.sv Signed-off-by: Robert Schilling <[email protected]>

The mi/dio signals are used in both cases, hardware strap sampling enabled an not. Signed-off-by: Robert Schilling <[email protected]>

andreaskurth

This PR is huge because files for Darjeeling get autogenerated and added. This is nicely separated into one commit, however, which makes it possible to manually review the other commits.

Overall LGTM, just a few minor suggestions/requests.

Note 1: hw/top_darjeeling still has a FUSESOC_IGNORE file, which is necessary to not disturb top_earlgrey until the FuseSoC update and virtual cores transition has happened. Those working with top_darjeeling need to

mv hw/top_{darjeeling,earlgrey}/FUSESOC_IGNORE
touch hw/top_englishbreakfast/FUSESOC_IGNORE

for now.

Note 2: Top-level simulations (even those without SW) don't yet work because chip_sim_cfg.hjson hasn't been added yet. I think it's okay to do this in a follow-up PR, though that could uncover a few more changes necessary to be able to elaborate the top-level design.

Note 3: Some IPs, including the DMA, can already be simulated through Darjeeling's top sim config (util/dvsim/dvsim.py hw/top_darjeeling/dv/top_darjeeling_sim_cfgs.hjson --select-cfgs dma ...), while others are still uncommented in top_darjeeling_sim_cfgs.hjson and of those some (such as clkmgr) don't elaborate yet. Also this, I think, is okay to resolve in a follow-up PR.

Thanks @Razer6!

andreaskurth · 2024-11-21T13:46:44Z

hw/top_earlgrey/ip_autogen/pinmux/lint/pinmux.waiver

-waive -rules RESET_USE -location {pinmux_strap_sampling.sv} -regexp {'rst_ni' is connected to 'prim_clock_mux2' port 'clk1_i
-', and used as an asynchronous reset or set at pinmux_strap_sampling} \
+waive -rules RESET_USE -location {pinmux_strap_sampling.sv} -regexp {'rst_ni' is connected to 'prim_clock_mux2' port 'clk1_i', and used as an asynchronous reset or set at pinmux_strap_sampling} \


This is just a line break change that shouldn't semantically affect the waiver used for Earlgrey.

(Just a note; no action required.)

andreaskurth · 2024-11-21T13:49:05Z

hw/ip/rv_dm/rtl/rv_dm.sv

@@ -573,7 +575,6 @@ module rv_dm
    // Tied-off and ignore signals from the DMI interface
    assign dmi_intg_error      = 1'b0;
    assign dbg_intg_error      = 1'b0;
-    assign dmi_gate_intg_error = 1'b0;


This is an unused signal; OK to remove.

(Just a note; no action required.)

andreaskurth · 2024-11-21T13:56:32Z

hw/top_darjeeling/physical_pads.core

+# Copyright lowRISC contributors (OpenTitan project).
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+name: "lowrisc:systems:physical_pads:0.1"


Suggested change

name: "lowrisc:systems:physical_pads:0.1"

name: "lowrisc:systems:top_darjeeling_physical_pads:0.1"

Same as below.

andreaskurth · 2024-11-21T13:58:04Z

hw/top_darjeeling/top_pkg.core

+# XXX: This name is currently required as global identifier until we have
+# support for "interfaces" or a similar concept.
+# Tracked in https://github.com/olofk/fusesoc/issues/235
+name: "lowrisc:constants:top_pkg"


Suggested change

name: "lowrisc:constants:top_pkg"

name: "lowrisc:constants:top_darjeeling_top_pkg"

Do those core files need to specify a virtual core?

Right, with virtual core support this core file would have the name suggested above and specify lowrisc:constants:top_pkg as virtual core.

Dependent cores then either have lowrisc:constants:top_pkg -- if they don't depend on a specific top -- or lowrisc:constants:top_darjeeling_top_pkg -- if they do depend on a specific top -- in their dependencies.

We don't have proper support for virtual cores just yet, so I leave it to you whether you want to do this in this or in a follow-up PR.

Let's do that (and the other Todo) in a follow up-PR to unblock multitop SW, which depends on that change. CI is green (so far it can), so let's get that in :)

andreaskurth · 2024-11-21T15:29:57Z

hw/top_earlgrey/ip_autogen/pinmux/rtl/pinmux.sv

-  // Local versions of the input signals
-  logic [NMioPads-1:0] mio_out, mio_oe, mio_in;
-  logic [NDioPads-1:0] dio_out, dio_oe, dio_in;
-


These lines get moved further up due to a change in the template. No functional change.

(This is just a note; no action required.)

hw/ip/prim/lint/prim_ram_1r1w.waiver

andreaskurth · 2024-11-21T15:35:32Z

util/topgen/templates/toplevel_pkg.sv.tpl

@@ -37,6 +37,26 @@ package top_${top["name"]}_pkg;
   * Memory size for ${name} in top ${top["name"]}.
   */
  parameter int unsigned ${region.size_bytes_name().as_c_define()} = ${hex_size_bytes};
+  ## TODO: we need a more holistic approach to declare memories and IPs sitting in the


Please create an issue and reference it here (TODO(#....)) so we can track this TODO item

Issue: #25313

I'll link to that in a follow up PR

andreaskurth · 2024-11-21T15:57:56Z

CI checks Verible lint and Check for and block unauthorized changes are failing due to the limitation in number of files. I'll run those checks locally before this PR gets merged.

…onfigs Signed-off-by: Robert Schilling <[email protected]>

…lude CTN Signed-off-by: Robert Schilling <[email protected]>

Signed-off-by: Robert Schilling <[email protected]>

Add top_darjeeling.hjson and all xbar.hjson files. Signed-off-by: Robert Schilling <[email protected]>

Signed-off-by: Robert Schilling <[email protected]>

Razer6 · 2024-11-21T22:11:24Z

Thanks for the review @andreaskurth.

Note 2: Top-level simulations (even those without SW) don't yet work because chip_sim_cfg.hjson hasn't been added yet. I think it's okay to do this in a follow-up PR, though that could uncover a few more changes necessary to be able to elaborate the top-level design.

I think it's ok for now. Adding the chip_sim_cfg.hjson would pull in much more files. I would do that in a separate PR.

Note 3: Some IPs, including the DMA, can already be simulated through Darjeeling's top sim config (util/dvsim/dvsim.py hw/top_darjeeling/dv/top_darjeeling_sim_cfgs.hjson --select-cfgs dma ...), while others are still uncommented in top_darjeeling_sim_cfgs.hjson and of those some (such as clkmgr) don't elaborate yet. Also this, I think, is okay to resolve in a follow-up PR.

I added all IPs.

andreaskurth · 2024-11-22T12:29:38Z

CHANGE AUTHORIZED: hw/ip/keymgr/rtl/keymgr.sv
CHANGE AUTHORIZED: hw/ip/keymgr/rtl/keymgr_input_checks.sv
CHANGE AUTHORIZED: hw/ip/rv_dm/rtl/rv_dm.sv
CHANGE AUTHORIZED: hw/top_earlgrey/ip_autogen/pinmux/rtl/pinmux.sv

The CI check cannot run on this PR (too many files), but I ran it locally. The files above need to be authorized. None of the changes has a functional impact on Earlgrey.

andreaskurth · 2024-11-22T12:38:18Z

For the Verible lint check, which CI also cannot run due to the file limit, the report includes

hw/top_darjeeling/rtl/autogen/top_darjeeling.sv:71:101-106: Line length exceeds max: 100; is: 106 [Style: line-length] [line-length]

@Razer6 This is a generated line; would it be simple to insert line breaks?

vogelpi · 2024-11-22T12:48:28Z

CHANGE AUTHORIZED: hw/ip/keymgr/rtl/keymgr.sv
CHANGE AUTHORIZED: hw/ip/keymgr/rtl/keymgr_input_checks.sv
CHANGE AUTHORIZED: hw/ip/rv_dm/rtl/rv_dm.sv
CHANGE AUTHORIZED: hw/top_earlgrey/ip_autogen/pinmux/rtl/pinmux.sv

None of the changes in this PR have a functional impact on Earlgrey. This is fine.

Razer6 added 2 commits November 21, 2024 00:27

[hw,pinmux,lint] Correctly gate all unused signals

3c6a793

Signed-off-by: Robert Schilling <[email protected]>

[hw,rv_dm,lint] Remove unused signals and tie-offs

1868259

Signed-off-by: Robert Schilling <[email protected]>

Razer6 requested review from a team, msfschaffner and vogelpi as code owners November 21, 2024 09:03

Razer6 requested review from eshapira, timothytrippel and andreaskurth and removed request for a team, msfschaffner, eshapira and timothytrippel November 21, 2024 09:03

Razer6 force-pushed the darjeeling-and-lint branch 2 times, most recently from 5c661cf to d24efeb Compare November 21, 2024 09:33

Razer6 added 6 commits November 21, 2024 01:51

[hw,kemgr(_dpe),rtl] Parametrize rom_digets for input checks

b09160f

Signed-off-by: Robert Schilling <[email protected]>

[hw,dma,lint] Update SHA message size for new prim

c4fea45

Signed-off-by: Robert Schilling <[email protected]>

[hw,pinmux] Gate countermeasures only used for HW strampling

0309647

Signed-off-by: Robert Schilling <[email protected]>

[darjeeling] Add core and linter files for Darjeeling

7e4ddd0

Signed-off-by: Robert Schilling <[email protected]>

[darjeeling] Add top-dependend RTL files

26a3f45

Add jtag_id_pkg.sv, padring.sv, scan_role_pkg, ibex_pmp_reset_pkg.sv padring.sv, top_pkg.sv Signed-off-by: Robert Schilling <[email protected]>

[hw,pinmux,rtl] Move signal definition outside of param block

26e8cea

The mi/dio signals are used in both cases, hardware strap sampling enabled an not. Signed-off-by: Robert Schilling <[email protected]>

Razer6 force-pushed the darjeeling-and-lint branch from d24efeb to 015ee1d Compare November 21, 2024 09:52

andreaskurth reviewed Nov 21, 2024

View reviewed changes

Razer6 added 3 commits November 21, 2024 23:02

[hw,prim_ram_1r1w,lint] rst_a_ni is only used in assertions in some c…

c4ec9b6

…onfigs Signed-off-by: Robert Schilling <[email protected]>

[darjeeling,templates] Update Darjeeling templates and top_pkg to inc…

171d1e5

…lude CTN Signed-off-by: Robert Schilling <[email protected]>

[darjeeling,ast,rtl] Remove devmode_i and fix core files

0cdd94d

Signed-off-by: Robert Schilling <[email protected]>

Razer6 added 5 commits November 21, 2024 23:02

[darjeeling,sensor_ctrl,rtl] Remove devmode_i

1d6cb3a

Signed-off-by: Robert Schilling <[email protected]>

[darjeeling] Add HJSON files to define Darjeeling

ef273fb

Add top_darjeeling.hjson and all xbar.hjson files. Signed-off-by: Robert Schilling <[email protected]>

[darjeeling] Generate Darjeeling from HJSON files

1fe0cc5

Signed-off-by: Robert Schilling <[email protected]>

[darjeeling,lint] Re-enable uncommented linting rules

d4d27e4

Signed-off-by: Robert Schilling <[email protected]>

[darjeeling,sim] Enable top-dependent SIM config

a346722

Signed-off-by: Robert Schilling <[email protected]>

Razer6 force-pushed the darjeeling-and-lint branch from 015ee1d to a346722 Compare November 21, 2024 22:06

andreaskurth approved these changes Nov 22, 2024

View reviewed changes

andreaskurth merged commit 32a974b into lowRISC:master Nov 22, 2024
37 of 40 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Darjeeling] Instantiate Darjeeling #25283

[Darjeeling] Instantiate Darjeeling #25283

Razer6 commented Nov 21, 2024

andreaskurth left a comment

andreaskurth Nov 21, 2024

andreaskurth Nov 21, 2024

andreaskurth Nov 21, 2024

Razer6 Nov 21, 2024

andreaskurth Nov 21, 2024

Razer6 Nov 21, 2024

andreaskurth Nov 22, 2024

Razer6 Nov 22, 2024

andreaskurth Nov 21, 2024

andreaskurth Nov 21, 2024

andreaskurth Nov 22, 2024

Razer6 Nov 22, 2024

andreaskurth commented Nov 21, 2024

Razer6 commented Nov 21, 2024

andreaskurth commented Nov 22, 2024

andreaskurth commented Nov 22, 2024

vogelpi commented Nov 22, 2024

	name: "lowrisc:systems:physical_pads:0.1"
	name: "lowrisc:systems:top_darjeeling_physical_pads:0.1"

	name: "lowrisc:constants:top_pkg"
	name: "lowrisc:constants:top_darjeeling_top_pkg"

[Darjeeling] Instantiate Darjeeling #25283

[Darjeeling] Instantiate Darjeeling #25283

Conversation

Razer6 commented Nov 21, 2024

andreaskurth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreaskurth commented Nov 21, 2024

Razer6 commented Nov 21, 2024

andreaskurth commented Nov 22, 2024

andreaskurth commented Nov 22, 2024

vogelpi commented Nov 22, 2024