write_cfgmem is writing to arty board when not specified to in the code

[rachsug]

Trying to generate a bitstream from nmigen for the arty board. The output finishes with this error:

write_bitstream completed successfully
write_bitstream: Time (s): cpu = 00:00:10 ; elapsed = 00:00:23 . Memory (MB): peak = 3049.016 ; gain = 205.496 ; free physical = 580 ; free virtual = 22043
# write_cfgmem -force -format bin -interface smapx32 -disablebitswap -loadbit "up 0 top.bit" top.bin
Command: write_cfgmem -force -format bin -interface smapx32 -disablebitswap -loadbit {up 0 top.bit} top.bin
Creating config memory files...
INFO: [Writecfgmem 68-23] Start address provided has been multiplied by a factor of 4 due to the use of interface SMAPX32.
Creating bitstream load up from address 0x00000000
Loading bitfile top.bit
ERROR: [Writecfgmem 68-29] write_cfgmem -interface SMAPX32 is not compatible with the top.bit configuration device setting(s):
SPI_buswidth=4
Regenerate the bitstream with the device settings for S_SELECTMAP32 or use the write_cfgmem -interface SPIx4
1 Infos, 0 Warnings, 0 Critical Warnings and 1 Errors encountered.
write_cfgmem failed
ERROR: [Common 17-39] 'write_cfgmem' failed due to earlier errors.

    while executing
"write_cfgmem -force -format bin -interface smapx32 -disablebitswap -loadbit "up 0 top.bit" top.bin"
    (file "top.tcl" line 42)
INFO: [Common 17-206] Exiting Vivado at Thu Dec 10 15:38:52 2020...
Traceback (most recent call last):
  File "edge_top.py", line 24, in <module>
    platform.build(Top())
  File "/home/rsugrono/playground/lib/python3.8/site-packages/nmigen/build/plat.py", line 99, in build
    products = plan.execute_local(build_dir)
  File "/home/rsugrono/playground/lib/python3.8/site-packages/nmigen/build/run.py", line 98, in execute_local
    subprocess.check_call(["sh", "{}.sh".format(self.script)])
  File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sh', 'build_top.sh']' returned non-zero exit status 1. 

Seems to try to write to arty board when it has not been coded to.
Here is the code that produces this error:

from nmigen import *
from nmigen_boards.arty_a7 import *

class Top(Elaboratable):

    def elaborate(self, platform):
        m = Module()

        button = platform.request('switch', 0)
        led = platform.request('led', 0)

        m.d.sync += led.o.eq(button.i)

if __name__ == "__main__":
    platform = ArtyA7Platform()
    platform.build(Top())

[davidlattimore]

I think the issue is that toolchain_prepare unconditionally adds write_cfgmem to script_after_bitstream.

Two possible fixes that come to mind:

1. Write and run a .tcl file containing the write_cfgmem command from toolchain_program.
2. Pass do_program through to toolchain_prepare so that it can conditionally add write_cfgmem.

The first option seems conceptually a bit nicer to me, but would mean writing and running a second .tcl file, so might be more work.

[daveshah1]

afaik, write_cfgmem isn't trying to write anything to the device but instead to write a file suitable for programming into config memory. The real problem here is why the write_cfgmem args don't match the configuration elsewhere.

[whitequark]

This bug is introduced in #523.

@nfbraun Could you please explain the meaning of -interface smapx32 in this context?

[nfbraun]

It is needed to change the endianness of every 4 byte group with respect to what write_bitstream -bin_file (or write_cfgmem without the -interface option) would produce. (Basically, it turns aa 99 55 66 into 66 55 99 aa.) For whatever reason, mainline FPGA Manager insists on that format (for Zynq), and I have not found any other way to produce this from within Vivado.

[whitequark]

@nfbraun Do you have any insight on fixing this bug? I'm not familiar with Xilinx's deployment tools.

[nfbraun]

I guess FPGA Manager is not normally involved in programming non-Zynq 7 Series FPGAs, so maybe restore the old behavior for non-Zynqs?

[whitequark]

Hm. I guess we could make the write_cfgmem arguments an override that the Zyng boards provide in toolchain_prepare.

[nfbraun]

Either that, or just put the whole write_cfgmem command into the existing script_after_bitstream override.

[whitequark]

Either that, or just put the whole write_cfgmem command into the existing script_after_bitstream override.

That would break all existing board definitions, no? Since script_after_bitstream is empty by default and should stay so.

[nfbraun]

As far as I can see, the board definitions for various Artix-7 boards (e.g. https://github.com/nmigen/nmigen-boards/blob/master/nmigen_boards/arty_a7.py) already have a write_cfgmem command (with different paramters) in their script_after_bitstream override, but none of the Zynq boards do.

So I guess I added the write_cfgmem command in a place that is too generic and the Zynq boards should instead follow what the Artix-7 boards do?

[nfbraun]

I would suggest to revert #523. I have opened a PR in nmigen-boards for a ZedBoard config that fixes #519 (for the ZedBoard) without affecting other devices: amaranth-lang/amaranth-boards#135. The board configs for other Zynq boards would have to add something similar.

@rachsug: Can you confirm that reverting commit 14a5c42 fixes the problem?

[whitequark]

I strongly suspect there will come a case where our default write_cfgmem will break things, so I think this should be a new override (that defaults to the pre-#523 behavior).

[davidlattimore]

Yep, @rachsug and I both locally reverted that commit and it did indeed fix the problem. Thanks

[nfbraun]

@whitequark: not sure if I understand correctly... AFAIK, there was no write_cfgmem before #523. It generated the .bin file via the -bin_file option to write_bitstream. Do you propose to make that configurable? I don't think it matters, because a write_cfgmem command introduced via the board config can always just overwrite the .bin file. Or am I missing something?

[whitequark]

@nfbraun You're right; I misunderstood. I reverted #523.

[whitequark]

I believe this is no longer an issue after reverting #523. Please comment if there is anything that still needs to be done here.