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

tests: features: add obufds test #42

Merged
merged 9 commits into from
Aug 5, 2021

Conversation

acomodi
Copy link
Contributor

@acomodi acomodi commented Jun 25, 2021

Signed-off-by: Alessandro Comodi acomodi@antmicro.com

This PR adds a basic test for the OBUFDS. Longer term this test might be expanded to include all diff IO tests (e.g. OBUFTDS, IBUFDS, IOBUFDS, etc.).

There is an issue at the moment that concerns the DCP generation from the physical netlist provided by nextpnr. Here is a tar containing the relevant outputs: obufds tar.

The problem seems to be that there is no macro expansion (probably due to a lack of data from the device database) happening for the OBUFDS cell. In fact, the nextpnr-generated physical netlist, there is only one OUTBUF used, and the DIFF_OUT wire is used.
Instead, when building with Vivado, the OBUFDS cell seem to be expanded to include the two OUTBUF bels, alongside with the inverter for the negative output.

I believe that the correct solution would be to provide the right data to correctly expand the OBUFDS in two OBUFT and an INV cell

@gatecat
Copy link
Contributor

gatecat commented Jun 25, 2021

The problem is that there are two possible OBUFDS expansions: one for true differential standards like LVDS (which is the one being picked here) and one for fake differential standards like SSTL here (which requires the two OBFUTs and INV as you've seen).

We will need to pick the right expansion based on the IO standard, see the discussion about this in chipsalliance/fpga-interchange-schema#40 (the expansion in this case is actually called OBUFDS_DUAL_BUF).

@acomodi
Copy link
Contributor Author

acomodi commented Jun 25, 2021

@gatecat All right, thanks for the info.

I have added some debug lines and noticed that the expansion rules may not be complete, in fact there is only a rule for the OBUFTDS:

Info: createContext time 1.90s
Info: Primitive OBUFTDS - Macro name OBUFTDS_DUAL_BUF

Info: Annotating ports with timing budgets for target frequency 12.00 MHz
Info: Checksum: 0xff5cf696

I have checked also upon the chipdb generation and indeed, the only rule present is the OBUFTDS one. I wonder whether there might be a bug when filling the exceptionMap list from RapidWright.

@acomodi acomodi force-pushed the add-diff-io-test branch 2 times, most recently from 185babc to 005d500 Compare July 7, 2021 09:19
@acomodi acomodi changed the title WIP: tests: features: add obufds test tests: features: add obufds test Jul 7, 2021
@mkurc-ant
Copy link
Collaborator

mkurc-ant commented Jul 23, 2021

I've extended the CMake scripts with support for XDC name prefix - this allows to have multiple tests with different XDCs in the same folder.

I also added tests for the rest of differential IO buffers:

Buffer Status GH Issue
OBUFTDS ok N/A
OBUFDS fail YosysHQ/nextpnr#773
IBUFDS fail YosysHQ/nextpnr#774
IOBUFDS fail YosysHQ/nextpnr#774

Those issues have to be solved for this PR to pass the CI

Copy link
Contributor

@gatecat gatecat left a comment

Choose a reason for hiding this comment

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

LGTM once we get CI passing

acomodi and others added 9 commits August 5, 2021 10:41
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
…f it to the Makefile

Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi acomodi merged commit 576534f into chipsalliance:master Aug 5, 2021
@acomodi acomodi deleted the add-diff-io-test branch August 5, 2021 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants