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

Tiles with no config bits have no FrameData signals #296

Closed
mole99 opened this issue Jan 24, 2025 · 5 comments
Closed

Tiles with no config bits have no FrameData signals #296

mole99 opened this issue Jan 24, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@mole99
Copy link
Contributor

mole99 commented Jan 24, 2025

This may seem like a feature at first: Tiles that do not use any configuration bits have no FrameData/FrameData_O signals. But for the end user this is a actually a bug:

Tiles without any config bits and tiles with config bits can be mixed in a row. This requires all tiles to have FrameData/FrameData_O signals, even those without any config bits. Currently, this will result in a broken fabric.

@IAmMarcelJung
Copy link
Collaborator

Currently, this will result in a broken fabric

This sounds really concerning, can you explain what you exactly mean by "broken" here? Like broken not working or broken with weird side effects or parts of the fabric not working? Or something else? Either way, this shows that we really need to improve our testing infrastructure to catch such things (and for many other reasons ofc).

@mole99
Copy link
Contributor Author

mole99 commented Jan 25, 2025

@IAmMarcelJung it's "broken not working", as in the generated HDL code is not valid.

The reason for that is, if a tile has no config bits, then no FrameData/FrameData_O ports are generated. Thus, when the fabric is generated, the FrameData wires are connected to the non-existent FrameData/FrameData_O ports of those tiles.

See #297 for why this "feature" was added and why I think it's just more trouble than it's worth.

@IAmMarcelJung
Copy link
Collaborator

I read your extensive descriptions in #297 and #161 (thanks!) and things got a bit clearer to me now (I'm honestly not too deep into how the fabric works). Did I get it right that this does not cause an issue if the first and last row are as it is currently in the demo fabric (since I could also not see the issue you described there)

However you are ofc totally right that this should be fixed to work for all kinds of tiles and tile combinations, probably by the suggestions you made :)

@mole99
Copy link
Contributor Author

mole99 commented Jan 26, 2025

Yes, that's exactly what it means :)
I think this feature was implemented this way specifically for the demo fabric, but presents a problem for many other fabrics.

Sounds good, I've already made the necessary changes on my end. Now I just need to clean them up and generalize the handling of NULL tiles before I open a PR 👍

@mole99
Copy link
Contributor Author

mole99 commented Feb 5, 2025

Resolved by #306

@mole99 mole99 closed this as completed Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants