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

Add support for nextpnr-gowin #13

Merged
merged 17 commits into from Jul 17, 2021
Merged

Conversation

pepijndevos
Copy link
Contributor

Apycula seems to have a bit different build process than most other FOSS FPGA tools in that it's a Python package that bundles the chipdb, which is built in CI, so the only sensible way to install it is via pip. This means that bundling Apicula as a YoWasp package would be a huge PITA, but there would not be much benefit because it's already a Python package. So this PR only seeks to enable the nextpnr-gowin binary, which requires Apycula as a build dependency.

This PR is WIP, but just want to put it out there to see all the missing pieces and CI failures.
The first commit only adds gowin to the nextpnr build command, and does not yet bundle it into a PyPi package.

@whitequark
Copy link
Member

This means that bundling Apicula as a YoWasp package would be a huge PITA, but there would not be much benefit because it's already a Python package.

What has to happen here is that yowasp-nextpnr-gowin should require the exact version of Apicula used when building it as a dependency, so that installing it would pull a compatible Apicula package in.

@pepijndevos
Copy link
Contributor Author

Hm, you don't run CI on PRs it seems. I've added a pypi package that "works on my machine"... at least it generates a wheel, I have yet to test it.

Apycula is not a runtime dependency of yowasp-nextpnr-gowin though, so on the one hand it makes sense on the other hand... not. It'd pull in numpy in case someone wants to generate a bitstream in their CI pipeline or whatever. Maybe it could be an extra dependency?

@pepijndevos pepijndevos marked this pull request as ready for review July 17, 2021 12:02
@whitequark
Copy link
Member

Apycula is not a runtime dependency of yowasp-nextpnr-gowin though, so on the one hand it makes sense on the other hand... not. It'd pull in numpy in case someone wants to generate a bitstream in their CI pipeline or whatever. Maybe it could be an extra dependency?

All other YoWASP packages include tools to generate bitstreams, so the Gowin one must, too, and the version mismatches must be prevented.

build.sh Outdated Show resolved Hide resolved
@pepijndevos
Copy link
Contributor Author

Trying to figure out how to do the plumbing for the version. You'd think setuptools would be able to tell me what version of apycula it just installed, but I'm coming up short. ./apycula-prefix/bin/pip freeze | grep Apycula works so I could shell out from the setup.py but feels a bit dirty. Oddly the pip python module does not seem to expose this AFAICT.

@whitequark
Copy link
Member

Give me a few minutes, I'll figure something out for this case.

@whitequark
Copy link
Member

>>> import importlib.metadata
>>> importlib.metadata.version('apycula')
'0.0.1a9'

@whitequark
Copy link
Member

Ideally you would use it with a shim like this.

In that case, you should also add "importlib_metadata; python_version<'3.8'" to setup_requires.

@pepijndevos
Copy link
Contributor Author

Hah, I found this https://pip.pypa.io/en/latest/user_guide/#using-pip-from-your-program eh, up to you if you prefer your solution.

@whitequark
Copy link
Member

whitequark commented Jul 17, 2021

I've updated the CI configuration so that it runs on pull requests. To make it work, you'll need to update the package workflow in your PR.

@pepijndevos
Copy link
Contributor Author

Alright. So I think what remains to be done is to add the pypi-gowin to the CI and test that the flow actually produces working bitstreams.

@pepijndevos
Copy link
Contributor Author

Working bitstream produced!

package-pypi-gowin.sh Outdated Show resolved Hide resolved
Co-authored-by: whitequark <whitequark@whitequark.org>
package-pypi-gowin.sh Outdated Show resolved Hide resolved
Co-authored-by: whitequark <whitequark@whitequark.org>
pypi-gowin/setup.py Outdated Show resolved Hide resolved
pypi-gowin/setup.py Outdated Show resolved Hide resolved
@whitequark
Copy link
Member

Everything seems reasonable now--I'm going to do a few more checks and then merge.

Thank you a lot for the PR and the quick updates on the review comments!

@pepijndevos
Copy link
Contributor Author

Awesome!

My plan is to use this to build the examples in Apicula using this as a way to catch the most dumb mistakes. Probably best I can do short of setting up a hardware-in-the-loop CI.

@whitequark
Copy link
Member

Do you need the find_package(PythonInterp 3.6 REQUIRED) line in nextpnr-gowin, by the way? You never actually use PYTHON_EXECUTABLE as far as I can see, only GOWIN_BBA_EXECUTABLE, which may use a Python completely different from what find_package(PythonInterp) finds.

@pepijndevos
Copy link
Contributor Author

Actually can't really remember... probably not? Might be a historical leftover from the nextpnr-generic heritage or some thing I did for debugging.

@pepijndevos
Copy link
Contributor Author

I think I only added the flag so if you use the Python scripting interface that it ends up using the same interpreter as the virtualenv. I don't think the cmake line is actually needed.

@whitequark
Copy link
Member

Removed it in YosysHQ/nextpnr#765.

@pepijndevos
Copy link
Contributor Author

Lol YosysHQ/nextpnr#766

@whitequark whitequark merged commit 9a5ece7 into YoWASP:develop Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants