gateware.iostream.IOStreamer: let o_stream transfer if i_stream not rdy #675
Conversation
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
from
cbb7f65
to
5240a4d
Compare
| m.d.sync += skid_at.eq(skid_at - 1) | ||
|
|
||
| m.d.comb += self.i_stream.payload.eq(skid[skid_at]) | ||
| m.d.comb += self.i_stream.valid.eq(i_en | (skid_at != 0)) | ||
| m.d.comb += self.o_stream.ready.eq(self.i_stream.ready & (skid_at == 0)) | ||
| m.d.comb += self.o_stream.ready.eq(self.i_stream.ready | (skid_at + sum(i_en_delays) < latency)) |
There was a problem hiding this comment.
I don't like this change, as it would probably add a (fairly slow on iCE40) carry chain in the critical path.
Also, no tests?
There was a problem hiding this comment.
No tests yet, PR is just for comments for now... Still thinking about how to test all corners.
So this change in the current PR's state basicly counts the number of reads in flight (skid buffer, or in the i_en delay chain, and if there's no more space for it in the skid buffer, then it refuses new transfers.
Alternatively a simpler, different behavior would be: are there any reads in flight? if yes, then refuse transfers.
That would look like:
m.d.comb += self.o_stream.ready.eq(self.i_stream.ready | ~((skid_at!=0) | i_en_delays.any()))It would work the exact same for SDR.
The advantage would be no carry chains for DDR.
The disadvantage would be that it would be less able to smooth out stalls on i_stream.
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
from
5240a4d
to
2cff955
Compare
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
from
2cff955
to
497a6b9
Compare
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
3 times, most recently
from
a579f7d
to
1a7a719
Compare
|
This PR now has a dependency on #678, and includes its changes. |
|
Temporarily setting this to draft, to see what happens to the smaller PRs first. |
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
from
1a7a719
to
5425029
Compare
Added support for additionally delaying when the sample is taken. Normally the samples are taken at a time when the input signals change that have been launched at the same time as i_en, however this allows us to take samples later, specified in number of sync clock cycles. Additionally for ratio=2 iostreamer we can also delay by half a clock cycle.
… DDR inputs" This reverts commit d1cc5d9.
This changes how the sdr input sampling test works. Before only positive edges on clk_out would signify to the testbench that the input has been sampled. The old behavior put a constraint on payload, that i_en must be high only for every second payload. This was less then ideal for developing stronger skid buffer tests. This change slows down the clk_out signal, and now any edge (positive or negative), means that we have sampled something. In the waves only the shape of clk_out changes, the rest of the signals stay the same. (i.e. the same values are sampled at the same times) The DDR input sample test does not require a similar change to it.
There was a corner-case in IOStreamer where a sample could be lost, if it arrives to the skid buffer on the same cycle as when another sample is removed from the skid buffer. This PR also adds many more testcases.
This better relays the intent that we want to sample the "old" value of the inputs in case they change at the same time as clk_out, and may work better if input_generator_tb changes in the future.
…of SimulatableDDRBuffer is glitchless
This change allows o_stream to take at least one transfer, as long as there are no sample transfers in flight. This will prevent deadlocks, for example if i_stream is implemented with something like: ```m.d.comb += ready.eq(valid)``` Which is valid according to stream rules. This commit also adds tests for this.
purdeaandrei
force-pushed
the
iostreamer_skid_buffer_updates
branch
from
5425029
to
0cd5632
Compare
No description provided.