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

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

Closed
rachsug opened this issue Dec 10, 2020 · 16 comments
Closed

Comments

@rachsug
Copy link

rachsug commented Dec 10, 2020

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
Copy link
Contributor

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
Copy link

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 whitequark added the bug label Dec 10, 2020
@whitequark
Copy link
Member

This bug is introduced in #523.

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

@nfbraun
Copy link
Contributor

nfbraun commented Dec 10, 2020

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
Copy link
Member

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

@nfbraun
Copy link
Contributor

nfbraun commented Dec 10, 2020

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
Copy link
Member

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

@nfbraun
Copy link
Contributor

nfbraun commented Dec 10, 2020

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

@whitequark
Copy link
Member

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
Copy link
Contributor

nfbraun commented Dec 10, 2020

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
Copy link
Contributor

nfbraun commented Dec 12, 2020

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
Copy link
Member

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
Copy link
Contributor

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

@nfbraun
Copy link
Contributor

nfbraun commented Dec 12, 2020

@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
Copy link
Member

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

@whitequark
Copy link
Member

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

@whitequark whitequark added this to the 0.3 milestone Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants