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

AXI_XBAR pipeline #116

Closed
wants to merge 9 commits into from
Closed

AXI_XBAR pipeline #116

wants to merge 9 commits into from

Conversation

WRoenninger
Copy link
Collaborator

This adds a change to the axi_demux implementation which has the benefit that the crossing AXI busses inside of axi_xbar can now be pipelined.
The issue was that the axi_demux could forward multiple AWs to different master ports of the module. This then has the effect, that Ws are forwarded in the same sequence. However combined with an axi_mux this could lead to deadlocks, if the AWs where stalled for example due to pipelining.
This PR fixes this issue by preventing AWs to be forwared to a different master port as long as there are still Ws in flight to another.

Changelog:

  • tb_axi_xbar: Add parameters, make more configurable for ci.
  • axi_demux: Remove FIFO between AW and W channel, is now a register plus counter.
    Prevents AWs to be emmitted downstream to a different master port as long as Ws
    are still in flight to another. This prevents deadlock, if there is stalling
    downstream.
  • axi_xbar: Add parameter PipelineStages to axi_pkg::xbar_cfg_t. This adds axi_multicuts
    in the crossed connections in the xbar between the demuxes and muxes.
  • axi_pkg: Add documentation to xbar_cfg_t.

Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

Thanks for this important change, @WRoenninger!

I have one major question on the counter and a few minor comments.

CHANGELOG.md Outdated Show resolved Hide resolved
src/axi_demux.sv Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/axi_pkg.sv Show resolved Hide resolved
test/tb_axi_xbar.sv Show resolved Hide resolved
test/tb_axi_xbar.sv Show resolved Hide resolved
test/tb_axi_xbar.sv Show resolved Hide resolved
src/axi_demux.sv Show resolved Hide resolved
src/axi_demux.sv Outdated Show resolved Hide resolved
@WRoenninger
Copy link
Collaborator Author

WRoenninger commented Jul 2, 2020

Removed now the Cfg struct of axi_xbar and the defintion in axi_pkg.
This resulted in the same removal being done for axi_lite_xbar.

Edit: This was reverted to keep compatibility for existing code for now.

@WRoenninger
Copy link
Collaborator Author

Rebased onto current master, also changed around the Bender dependencies to match current tags.

@WRoenninger
Copy link
Collaborator Author

Rebased onto current master for ci fix by #129.

@WRoenninger
Copy link
Collaborator Author

@accuminium There seems to be an issue with the build and deploy ci, where there is some lock failure between the (push) and (pull_request) scripts.

@andreaskurth
Copy link
Contributor

@accuminium There seems to be an issue with the build and deploy ci, where there is some lock failure between the (push) and (pull_request) scripts.

Thanks, I also noticed that. In a PR, each of the CI jobs should actually only run once (triggered by the pull_request event, and not by push). The push event is relevant only outside PRs. But I have not yet spent the time to figure out how to define this condition. Please feel free to open a PR that fixes this. The workaround for now is to manually restart the build-and-deploy job when it fails due to a conflict with the other instance.

@thommythomaso
Copy link
Collaborator

Replaced by #268. Closed.

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

3 participants