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

Output enable for Intel platform on multi I/O pins #297

Closed
schwigi opened this issue Jan 6, 2020 · 6 comments · Fixed by #300
Closed

Output enable for Intel platform on multi I/O pins #297

schwigi opened this issue Jan 6, 2020 · 6 comments · Fixed by #300

Comments

@schwigi
Copy link
Contributor

schwigi commented Jan 6, 2020

Not sure wether it is a bug or I have done something wrong.

I tried to get a 16 bit wide io pins, but the generated oe wire is only 1 bit. For the Intel altiobuf_bidir output enable signal must be as wide as the data lines.

Attached is a small python script to generate the output.

ioTest.py.zip

@peteut
Copy link
Contributor

peteut commented Jan 6, 2020

@schwigi That looks like a bug to me,oe should be a vector: nmigen.vendor.intel#L286.

self._get_oereg should be employed here as well nmigen.vendor.intel#L223.

image

Refer to ALTIOBUF IP COre User Guide, UG-01024 for details.

@schwigi
Copy link
Contributor Author

schwigi commented Jan 7, 2020

I did some debugging runs and the problem is not in the platform.intel part.

In lib/io.py:43 the oe width is hard coded to 1. For Intel FPGAs it is clear that you must use width here as well. Not sure however how other FPGAs behave though.

@whitequark
Copy link
Contributor

I did some debugging runs and the problem is not in the platform.intel part.

No, the problem is in vendor.intel.

In lib/io.py:43 the oe width is hard coded to 1.

This is deliberate: a wide tristate signal is treated as a single entity for the purposes of output enable. That is how (n)Migen define its behavior, which is completely independent of any platform. Any platform that has output enable with the same width as output should use Repl.

If you want to control direction per-bit, you should instantiate a tristate per bit.

@peteut
Copy link
Contributor

peteut commented Jan 8, 2020

@schwigi: shouldn't this

i_oe=pin.oe,

also read i_oe=self._get_oereg(m, pin),?

@schwigi
Copy link
Contributor Author

schwigi commented Jan 8, 2020

@peteut: You are right, get_tristate should use _get_oereg as well.

@Ravenslofty
Copy link
Contributor

Thanks for correcting my oversight ^.^'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants