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

Python code for generating and writing test architecture #85

Merged
merged 14 commits into from
Jun 23, 2021

Conversation

mkurc-ant
Copy link
Collaborator

This is a very WIP code that in the end should allow to easily define a test FPGA architecture with timings. Currently it generates and writes site types and tile types, no alternative site types supported.

@mkurc-ant mkurc-ant marked this pull request as draft May 19, 2021 14:53
self.bels[bel.name] = bel

# Add BEL pin
bel.add_pin(name, direction) # FIXME: Opposite direction ?!?!?!
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - an input to the site from the tile is an output from the site port bel pin within the tile (think of it a bit like a 'buffer' that brings the signal into the tile)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Allright, thanks.

@mtdudek
Copy link
Contributor

mtdudek commented Jun 9, 2021

Testarch generated right now can be combined with gen_device_config.yaml in nextpnr_emit to emit testarch database, but when trying to run wire test, nextpnr fails on assertion in line 150 in arch_pack_io.cc


w = site_type.add_wire("MUX_O")
w.connect_to_bel_pin("FFMUX", "O")
w.connect_to_bel_pin("FF", "D")

w = site_type.add_wire("LUT_OUT", [("LUT", "O"), ("O", "O")])
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to remove wire "LUT_OUT" as there cannot be 2 wires driven by the same output.
This caused nextpnr_emit to fail

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this looks like a typo in any case as the O site pin is being correctly driven by the LUT_O wire anyway: https://github.com/antmicro/python-fpga-interchange/blob/399355e63c3edf9202ca373d371cfa7d1bfa92a4/fpga_interchange/testarch_generators/generate_testarch.py#L82

is_perimeter = y in [0, self.grid_size[1] - 1] or \
x in [0, self.grid_size[0] - 1]
is_centre = y == self.grid_size[1] // 2 and x == self.grid_size[0] // 2
Copy link
Contributor

@mtdudek mtdudek Jun 9, 2021

Choose a reason for hiding this comment

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

Nextpnr_emit expects tile 0,0 to be dummy/null tile.
Also nextpnr_emmit needs some way to inject GND and VCC signals, this is now done by PWR tile

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this is fair for now - eventually we do want to find a better solution to that

@mtdudek
Copy link
Contributor

mtdudek commented Jun 14, 2021

So far this works, testarch capnp is generated and is valid, I'm able to compile it to chip database, and with custom yosys techmaps and flow testarch can pass wire test, but const_wire is still not working.

top_instance_name=None,
top_instance=None,
libraries=libraries)
netlist = LogicalNetlist(name=args.library,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for what appear to be just format changes in existing code that should have been auto-formatted already?

Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

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

A few comments in

.github/workflows/testarch_device.sh Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
fpga_interchange/fasm_generators/xc7.py Outdated Show resolved Hide resolved
fpga_interchange/populate_chip_info.py Outdated Show resolved Hide resolved
@mtdudek mtdudek force-pushed the testarch-writer branch 3 times, most recently from 1e2ad36 to 86fc80b Compare June 16, 2021 09:46
@mkurc-ant mkurc-ant changed the title [WIP] Python code for generating and writing test architecture Python code for generating and writing test architecture Jun 16, 2021
@mkurc-ant mkurc-ant marked this pull request as ready for review June 16, 2021 09:55
Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -434,7 +437,8 @@ def from_reader(message,
continue

if field_proto_data.ref_annotation is not None:
reference_fun = lambda value: reader.reference_value(field_proto_data.ref_annotation, value, root, parent)
reference_fun = lambda value: reader.reference_value(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still slightly confused by these autoformatting changes, but it' passing CI so presumably this format is OK too?

features=["IN_USE", "ZINV_CE"],
callback=lambda m: "BUFHCE.BUFHCE_X{}Y{}".format(0 if m.group(1) == "L" else 1, m.group(2))
))
site_thru_features.append(
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this seems to be just a formatting change unrelated to testarch?

bel_ib.add_pin("I", Direction.Output)
bel_ib.add_pin("P", Direction.Input)

bel_ipad = site_type.add_bel("IPAD", "IPAD", BelCategory.LOGIC)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not critical for now, but more realistic longer term would be to have one PAD bel like Xilinx rather than seperate IPAD and OPAD bels. This would be useful for testing bidirectional IO away from the complexity of real Xilinx IOBs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, for now, a TODO comment to not lose track of this might be enough.

@acomodi
Copy link
Contributor

acomodi commented Jun 21, 2021

This requires a rebase, and I think we can go ahead and merge once CI is green

mkurc-ant and others added 13 commits June 21, 2021 13:30
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Add dummy tile on 0,0 location in generate_testarch as populate_chip needs this tile to be empty
Add gen_device_config.yaml for testarch

Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Fix primitives names to use global names

Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Const wire now passes

Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@antmicro.com>
@acomodi acomodi merged commit fade094 into chipsalliance:master Jun 23, 2021
@acomodi acomodi deleted the testarch-writer branch June 23, 2021 16:12
@acomodi acomodi restored the testarch-writer branch June 23, 2021 16:12
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.

None yet

4 participants