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

Jump to bottom

Add initial support for Symbiflow toolchain for Xilinx 7-series #463

Merged
merged 2 commits into from Aug 24, 2020
Merged

Add initial support for Symbiflow toolchain for Xilinx 7-series #463

merged 2 commits into from Aug 24, 2020

Conversation

mglb
Copy link
Contributor

@mglb mglb commented Aug 3, 2020

The PR adds initial support for Symbiflow toolchain for XC7 devices. To use Symbiflow instead of Vivado, a platform should inherit Xilinx7SeriesSymbiflowPlatform instead of Xilinx7SeriesPlatform.
The example used below is a copy of arty_a7.py from nmigen-boards, where in addition to changing the base class, the device (part) name has also been changed (original part is not present in prjxray/symbiflow database).

Test:

wget 'https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh' -O miniconda.sh
bash miniconda.sh -b -p ./miniconda
source ./miniconda/etc/profile.d/conda.sh
hash -r

conda config --set always_yes yes --set changeps1 no
conda config --add channels conda-forge
conda config --add channels symbiflow
conda update -q conda
conda activate
conda install -c symbiflow yosys yosys-plugins vtr prjxray-tools
conda install lxml simplejson intervaltree python-constraint git pip
pip install git+https://github.com/symbiflow/fasm

git clone https://github.com/SymbiFlow/prjxray.git
git clone https://github.com/SymbiFlow/prjxray-db.git

cd ./prjxray
python setup.py install
cd ..

wget 'https://storage.googleapis.com/symbiflow-arch-defs/artifacts/prod/foss-fpga-tools/symbiflow-arch-defs/presubmit/install/385/20200702-165441/symbiflow-arch-defs-install-3f58546e.tar.xz'
tar -xf symbiflow-arch-defs-install-3f58546e.tar.xz

export PATH=$(realpath ./install/bin/):$PATH
export XRAY_DATABASE_DIR=$(realpath ./prjxray-db)
export XRAY_FASM2FRAMES=$(realpath ./prjxray/utils/fasm2frames.py)
export XRAY_TOOLS_DIR=$(realpath ./miniconda/bin)

# Get PR code
git clone https://github.com/antmicro/nmigen.git -b mglb/new-initial-symbiflow-support
cd nmigen
python setup.py develop
cd ..

# The example uses nmigen-boards
pip install git+https://github.com/nmigen/nmigen-boards.git

# Download and run example target for Arty A7
wget 'https://raw.githubusercontent.com/antmicro/nmigen/mglb/new-initial-symbiflow-support-example/examples/symbiflow/arty_a7.py'
python arty_a7.py

@Ravenslofty
Copy link
Contributor

Ravenslofty commented Aug 3, 2020 •
edited

Considering vendor.lattice_{ice40,ecp5} use the same platform for different toolchains, may I ask why you chose to create a whole new platform here?

whitequark
whitequark requested changes Aug 3, 2020
View reviewed changes
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

@Ravenslofty is correct. The choice of the toolchain lies not with the board file author, but with the board file end user, so there should only be a single platform for Vivado and Symbiflow, similar to what is done for iCE40 and ECP5.

@whitequark whitequark mentioned this pull request Aug 9, 2020
Closed
@mglb mglb force-pushed the mglb/new-initial-symbiflow-support branch from 89567c5 to 7e1fe07 Compare August 14, 2020 14:50
@mglb
Copy link
Contributor Author

mglb commented Aug 14, 2020

I've implemented your suggestions. Now you just have to pass toolchain="Symbiflow" to platform's initializer (and use slightly different device name and speed in Arty example).

@whitequark
Copy link
Member

Thanks, this is much better.

and use slightly different device name and speed in Arty example

Would it be possible to use the same device name for the vendor and the FOSS toolchains? For other platforms that support both, this is handled transparently: either by adjusting the FOSS toolchain to accept the same device names, or by mapping them within nMigen. E.g. you can see this done in the ECP5 platform file.

@mglb mglb force-pushed the mglb/new-initial-symbiflow-support branch from 7e1fe07 to e499ba9 Compare August 24, 2020 12:51
whitequark
whitequark approved these changes Aug 24, 2020
View reviewed changes
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

LGTM

whitequark
whitequark reviewed Aug 24, 2020
View reviewed changes
nmigen/vendor/xilinx_7series.py
elif self.toolchain == "Symbiflow":
cd_sync = ClockDomain("sync", reset_less=self.default_rst is None)
m.domains += cd_sync
m.submodules += Instance("BUFG", i_I=clk_i, o_O=cd_sync.clk)
Copy link
Member

Choose a reason for hiding this comment

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

Does Symbiflow actually need this BUFG, or can it insert one automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple example (blinky) works without this bufg. However, we had some problems with Litex+VexriscV+Linux without it.

@whitequark whitequark merged commit 4e208b0 into amaranth-lang:master Aug 24, 2020
@whitequark
Copy link
Member

Thanks!

@whitequark
Copy link
Member

Have you tested the PR? It currently completely breaks 7-series because if toolchain == "Symbiflow": should be if self.toolchain == "Symbiflow":.

@whitequark
Copy link
Member

Fixed in abaa909.

@nickoe nickoe mentioned this pull request Jan 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whitequark whitequark whitequark approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mglb @Ravenslofty @whitequark