Closed cr1901 closed 6 years ago
@cr1901 @mithro As a more general comment,having caught up on IRC, it's great to see this working. If you'd rather merge this as is and clean up the Makefile separately to make it more readable, we could perhaps turn my comments into an issue instead, and look at it later. Because it looks like the current version should work on existing/new boards as is.
Ewen
@ewenmcneill Did you forget to publish your comments?
@ewenmcneill Did you forget to publish your comments?
It would appear so, yes. I hadn't used that GitHub "Review" feature that it prompted me to use before, so I didn't realise the "pending" badge meant the comments weren't visible to anyone else (rather than, eg, not replied to). I believe I've managed to publish the comments now. Thanks for the heads up!
Ewen
Just FYI. On the ice40up5k the single port RAM is not initialised from the bitstream, so if you want to execute code from that we will need to copy into it.
Another thought, execute in place from block ram / block rom does kind of makes sense. The block ram / block rom is already initialised from the bitstream, so copying the data from another region seems wasteful. Specially if there is only block ram / block rom in the design (you don't want to copy from block rom to block ram just to leave the block rom unused).
The block ram / block rom is already initialised from the bitstream, so copying the data from another region seems wasteful.
Sure, but this comes at the cost of "every time I recompile the firmware I have to recompile the bitstream".
Specially if there is only block ram / block rom in the design (you don't want to copy from block rom to block ram just to leave the block rom unused).
I assume this is rather rare that you only have an SPI flash large enough for the bitstream only, but yes, this is possible :).
Actually;
A small program for swapping the BRAM contents in IceStorm ASCII files. E.g. for changing the firmware image in a SoC design without re-running synthesis and place&route.
There will actually be a similar tool for Xilinx Series 7 bitstreams shortly too.
@cr1901 @mithro FYI, I've marked both the conversations I opened on this as "resolved" since the tidy up changes flowing on from https://github.com/enjoy-digital/litex/pull/109 will get us something that works now and is fairly clean, and can be improved as needed later (in other PRs :-) ) if we end up with more alternatives.
So FTR the plan of @cr1901 LGTM now.
Ewen
@cr1901 Let me/@mithro know when you're done updating the pull request, and one of us can hit the "merge" button. (Travis isn't properly set up here, it's got the .travis.yml
file pulled in from upstream, but I have a feeling not the right setup in the rest of the GitHub account for it to work. And I'm still snowed under with dayjob work so haven't had a chance to test anything.)
Ewen
@ewenmcneill This is ready to merge. It is a complement to this litex-buildenv PR.
@cr1901 Cool, merged. Hopefully the https://github.com/timvideos/litex-buildenv/pull/67 needed to use it can be merged soon (I can't merge that one; needs @mithro or similar).
Ewen
Self explanatory, this PR adds logic to distinguish LiteX-supported boards which run their main program from flash from those who load their program into DRAM. The Makefile queries the recently-added
COPY_TO_MAIN_RAM
Makefile variable to determine which linker script to use.The new behavior shouldn't affect any previously-supported boards.