openXC7 / nextpnr-xilinx

Experimental flows using nextpnr for Xilinx devices
ISC License
39 stars 14 forks source link

BSCANE2 placement not determined by JTAG_CHAIN parameter #25

Closed hansemro closed 5 months ago

hansemro commented 5 months ago

Issue Description

NextPNR's placer may place BSCANE2 BELs at a different location than expected, and thus require a different USERx instruction to access.

Steps to Reproduce

  1. Clone jtag_led demo with right-shift-fail branch:
git clone https://github.com/hansemro/jtag_led.git -b right-shift-fail
cd jtag_led
  1. Modify USER_PORT/JTAG_CHAIN parameter in src/rtl/jtag_led.sv with any value between 1-4 (inclusive).

  2. Build with make using openXC7 toolchain:

nix develop github:openXC7/toolchain-nix
make
  1. Check if NEXTPNR_BEL attribute for bscane cell is valid or not inside jtag_led.place.json.

Expected placement:

JTAG_CHAIN NEXTPNR_BEL
1 BSCAN_X0Y0/BSCAN
2 BSCAN_X0Y1/BSCAN
3 BSCAN_X0Y2/BSCAN
4 BSCAN_X0Y3/BSCAN

If still valid, try rebuilding with make YOSYS_OPTS=-DLS clean all. For some reasons, changing how jtag_tdi gets shifted into a shift register can affect placement.

Workaround

As a workaround, you can manually correct placement in jtag_led.place.json and let nextpnr route with the corrected json:

# clear build
make clean

# Synthesize, pack, and place design
make jtag_led.place.json

# edit jtag_led.place.json with correct `NEXTPNR_BEL` placement if necessary
...

# let make route and build the bitstream with corrected placement
make
hansfbaier commented 5 months ago

@hansemro Hi, thanks for this perfect report. That made understanding, reproducing and fixing the problem really easy. I implemented a fix here: https://github.com/openXC7/nextpnr-xilinx/commit/46c781f64e638a6fdb1f2f10486ba4e50b5e4648 it is in the pack-bscan branch. Please test it on your hardware, and let me know if it works, then I will merge it.

hansemro commented 5 months ago

Please test it on your hardware, and let me know if it works, then I will merge it.

Currently away, but will test soon. Thanks!

hansemro commented 5 months ago

@hansfbaier pack-bscan branch resolves this issue. Thanks!

hansfbaier commented 5 months ago

OK this is merged. Thanks for the great report!