[tlul/rtl] Fix SRAM timing #22588
[tlul/rtl] Fix SRAM timing #22588
Conversation
Signed-off-by: Greg Chadwick <gac@lowrisc.org>
| @@ -310,7 +328,7 @@ module tlul_adapter_sram | |||
| d_data : d_data, | |||
| d_user : '{default: '0, data_intg: data_intg}, | |||
| d_error : d_valid && d_error, | |||
| a_ready : (gnt_i | error_internal) & reqfifo_wready & sramreqfifo_wready | |||
| a_ready : (gnt_i | missed_err_gnt_q) & reqfifo_wready & sramreqfifo_wready | |||
There was a problem hiding this comment.
One thing I had noticed while working on Sonata is that a missed error causes an a_ready to be sent but the error response never gets written to the request FIFO.
There was a problem hiding this comment.
Odd, the ready shouldn't be signalled unless there's space in the FIFO and the FIFO is written when you have a_ready and a_valid asserted. It would be good to understand the issue you'd be seeing in more detail.
|
Kick off a kokoro rerun, assuming that's all good I will bypass CI for a new version that adds the missing space @andreaskurth highlighted given this is an important fix to get in. |
GregAC
force-pushed
the
sram_timing_fixes_2
branch
from
5e86282
to
de7fc69
Compare
|
Local block level regression runs are all looking good |
Previously there was a combinational path from the incoming tilelink request to the outgoing ready due to the error checking factoring into the ready. This breaks that path by introducing a potential 1 extra cycle of latency when an error response has to be returned. Signed-off-by: Greg Chadwick <gac@lowrisc.org>
vogelpi
force-pushed
the
sram_timing_fixes_2
branch
from
de7fc69
to
a9614de
Compare
Previously the logic of tlul_err_resp was more complex than was required (having seperate flops for tracking a pending request and response where it's sufficient to just track a pending response). This simplifies the logic and cuts a path from the incoming data ready to the outgoing address ready. Signed-off-by: Greg Chadwick <gac@lowrisc.org>
vogelpi
force-pushed
the
sram_timing_fixes_2
branch
from
a9614de
to
f8430d1
Compare
|
I am merging this, the |
With my hacky block synthesis with these fixes and the previous work (#22497) I believe I have eliminated all feedthroughs from the incoming tilelink signals to the outgoing tilelink signals in sram_ctrl.
These fixes will also have a positive effect on timing elsewhere as
tlul_sram_adapterandtlul_err_respare used in various places. In turn this could give us some modest area improvements as buffers are removed and cell drive strengths can be lowered. Though note this also means increases verification risk in more places.Contributes to resolving #22462.
I'm doing a final synthesis to confirm everything now, block level regressions (on
sram_ctrlother blocks are effect, haven't run regressions for them yet) are looking healthy. I've had issues running the sram_ctrl top-level smoke test (not related to these changes) butchip_sw_sram_ctrl_scrambled_accessis passing successfully.I'm going to do some block level regressions of the other blocks that use the altered primitives and check some other sram related top-level tests.