-
Notifications
You must be signed in to change notification settings - Fork 15
Burst support #15
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
base: master
Are you sure you want to change the base?
Burst support #15
Conversation
| localparam int unsigned NumOutLog2 = $clog2(NumOut); | ||
| localparam int unsigned IniAggDataWidth = 1 + BeWidth + AddrMemWidth + DataWidth; | ||
| localparam int unsigned ReqAggDataWidth = 1 + BeWidth + AddrMemWidth + DataWidth + BurstWidth; | ||
| localparam int unsigned RespAggDataWidth = DataWidth + 32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should define the width based on RspGF from the burst package. Also, the default width should be DataWidth if burst is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we added a wrapper to ensure modular instantiation of the burst function. This should be handled there.
| `else | ||
| assign req_agg_in[j] = {req_wen_i[j], req_be_i[j], req_tgt_addr_i[j][ByteOffWidth + NumOutLog2 +: AddrMemWidth], req_wdata_i[j]}; | ||
| `endif | ||
| assign {resp_rdata_o[j], resp_burst_o[j]} = resp_agg_out[j]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not include resp_burst_o when Burst is not used => signal not found issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we added a wrapper to ensure modular instantiation of the burst function.
| parameter int unsigned NumInLog2 = (NumIn == 1) ? 1 : $clog2(NumIn), | ||
| // Burst response type can be overwritten for DataWidth > 32b | ||
| // This can happen when the DataWidth includes transaction metadata | ||
| parameter type burst_resp_t = tcdm_burst_pkg::burst_gresp_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the package definition from the burst_pkg.sv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
| parameter int unsigned NumInLog2 = (NumIn == 1) ? 1 : $clog2(NumIn), | ||
| // Burst response type can be overwritten for DataWidth > 32b | ||
| // This can happen when the DataWidth includes transaction metadata | ||
| parameter type burst_resp_t = tcdm_burst_pkg::burst_gresp_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the package definition from the burst_pkg.sv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
| // Note that only the full crossbar allows NumIn/NumOut configurations that are not | ||
| // aligned to a power of 2. | ||
|
|
||
| `ifdef USE_BURST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit concerned about the use of a define here. If using the define, it prevents SoCs from using both the bursty and the non-bursty implementations of this interconnect block (e.g., when integrated into different subsystems). My suggestion would be to convert this to a parameter or to create two different modules, each implementing different functions (one could also call the other, tying off some inputs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I added a wrapper.
An "isburst" signal is needed to redistribute grouped responses at the initiator.
| parameter int unsigned AddrWidth = 32, // Address Width on the Initiator Side | ||
| parameter int unsigned DataWidth = 32, // Data Word Width | ||
| parameter int unsigned BeWidth = DataWidth/8, // Byte Strobe Width | ||
| parameter int unsigned ReqDataWidth = 32, // Data Word Width on the Request path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the original DataWidth is replaced by ReqDataWidth and RespDataWidth, which will require all repo using this module to adapt the connection. I am not sure if it is a good idea. Maybe we can keep the parameter int unsigned DataWidth = 32 and let the default values of ReqDataWidth and RespDataWidth equals to the DataWidth. Then the burst version can still overwrite these two values without needs of modification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely, good point.
a76e1a7 to
486a10b
Compare
1837b50 to
1598ff7
Compare
1598ff7 to
8071742
Compare
bc6fa6f to
8529a1a
Compare
This PR adds burst support to the variable latency interconnect.
In systems where multiple interconnects are instantiated hierarchically, the arbitration across multiple initiators at the boundary between hierarchies causes a bottleneck in the request-per-cycle injected into the interconnect. The burst functionality solves the problem. We provide:
burst_req_groupermodule, that groups a parallel valid request in a burst request,burst_cuttermodule, that issues two bursts when the parallel request crosses the boundary between hierarchies and targets different arbitrators,burst_managermodule, that receives a burst request and translates it into a parallel request,burst_rsp_groupermodule, that increases the bandwidth of the response channel to send multiple words from a parallel response with the sameresp_valid, thus reducing congestion to the arbiters also on the response path.More details on the burst protocol applied to a system with hierarchical crossbars and wide memory requests initiators are in the following publication:
TCDM Burst Access: Breaking the Bandwidth Barrier in Shared-L1 RVV Clusters Beyond 1000 FPUs
This paper is available on arXiv:2501.14370 [cs.AR].