<modwizcode>
So this is a (hastily coded from memory) port of some logic I encountered in verilog into nmigen
<modwizcode>
it's supposed to align decoded 8b10b input data based on the position of comma characters
<modwizcode>
I think it actually works, but during testing I got pretty confused about when the output data would be valid, and the way it appears in gtkwave conflicts with if I print from within the simulation
<agg>
There's an open issue about how it would be less confusing if the simulator inputs changed on the falling clock edges instead, and sync assignments on rising
<modwizcode>
Also I'm curious if there's a better way to do what I wrote, one idea is to replace the switch with just concatenating both 32bit words and extracting a 32 bit word from the combination using the offset to select the starting word
<modwizcode>
I'm trying to understand what's actually happening
<agg>
you might find that using some yield Settle() helps clarify the timing
<modwizcode>
How? (I might misremember what Settle does)
<modwizcode>
Right now this accepts data that's presented before the rising edge, on that edge it should figure out the offset and register that, and also register the data present on that edge.
<agg>
I think (without yet having read your code in detail) what's happening is basically you write yield input.eq(1), and nothing changes in the logic until you write yield, advancing the clock one cycle and applying your inputs; in this new cycle your logic reacts and your sync assignments become live, but won't take effect until the next clock cycle after that, so you call yield again in sim and only now can you
<agg>
(yield output) and get valid data
<modwizcode>
The next edge will have all the data to do the alignment and should be able to do the output. Right now I register the output because I wasn't sure if that was safe not to do (which was another question)
<agg>
If that first yield was a yield settle it could process the changed input in the same clock cycle (aiui)
<modwizcode>
So the whole pipeline of sorts is 3 clocks long, on the 3rd clock you get your output (at least I think so)
<modwizcode>
oh
<modwizcode>
I will try that and see if that has the desired effect
<agg>
On mobile otherwise I'd try it out but I've had this sort of confusion before
<modwizcode>
I'll let you know what happens it's a quick change to test here
<modwizcode>
I'd definitely still like some insight as to if I can avoid registering the final output and save a cycle of latency and if there's a cleaner way to do this code (since it seems like a ton of code for a simple idea)
<agg>
Registering the output is usually more about meeting timing and isolating modules from each other
<agg>
If the output comes from a bunch of comb logic in this module and drives a bunch of comb logic in the sink it's going to have a long critical path
<agg>
Sorry if you already know all this
<modwizcode>
Yeah what I'm worried about here is that I don't know for sure if my input is going to be stable such that a combinatorial path to the output is safe
<modwizcode>
No no I'm pretty much a moron with this stuff it's hard to conceptualize it and I get turned around a lot
<modwizcode>
The input data is the output of a 8b10b decoder attached to a serdes, the clocks come out of that too
<modwizcode>
Actually if what I'm worried about is a problem I think the whole thing is screwy regardless
<modwizcode>
I'd need to register it on the input end to be safe I think?
<agg>
Yea, often you'd have like a valid output alongside data from the decoder and you'd register the data whenever valid was high at this module's input
<modwizcode>
Yeah the valid stuff is handled upstream of this iirc
<agg>
I think usually with 8b10b decoders you detect comma based on a smaller set of bits than the entire symbol and so it's done inside the deserialiser
<agg>
Like the comma is a 7(?) bit substring of the 10 bit symbol
<modwizcode>
Yeah what happens here is that the decoder outputs a vectory of "which ones are K chars" and then there's also the decoded 8 bit things
<modwizcode>
the K char stuff is handled out of band
<modwizcode>
so the code matches both "is K char" and "data value for K-char == alignment comma" to do alignment
<modwizcode>
I heard that the decoder also actually might do this aligner's job internally but if so it's an undocumented feature
<agg>
ah, didn't realise you were using an existing decoder block, but yes I'd usually expect it to do comma alignment
<agg>
in 8b/10b the comma is 0011111 or 1100000, which can only occur inside control code-groups
<agg>
but you wouldn't usually search for the specific code-groups that you know contain commas when encoded
<agg>
instead the decoder searches for the 7-bit comma sequence (sometimes just one of the two) to align to
<agg>
if the decoder wasn't already aligned to the ten-bit boundaries, it would not be able to decode into bytes correctly
<agg>
so it doesn't seem like your logic could work, it's sort of at a stage that's too late to detect commas
<modwizcode>
I should be more specific, the rest of the code (the packet decoders) are dumb and use the comma (and another bit) to indiciate start-of-packet/frame, but they only look at the low word (kchar[0], rx_data[0:8])
<modwizcode>
we're only aligning within the decoded word essentially
<agg>
oh, so your decoder has already detected commas for bit alignment, and you're now trying to detect basically a frame delimiter?
<agg>
(which is totally fair to also call a comma, but is confusable with the 8b/10b comma sequence used for bit alignment)
<modwizcode>
Yeah
<agg>
got it, yea, that makes more sense
<modwizcode>
Well my understanding is that both types of alignment use the term comma and the same set of potential symbols so it's nice and confusing :)
<agg>
indeed!
<agg>
i've usually called a particular special code-group a "start of frame delimiter" and it incidentally contains an 8b/10b comma
<modwizcode>
yeah
<agg>
like K.28.1 or whatever, in a lot of standards they then form ordered sets from that
<modwizcode>
I heard maybe that the hardware IP that does the bit alignment might also align the words, but that's like unspecified.
<modwizcode>
It'd kinda make sense though to some extent
<modwizcode>
I'm like nearly positive the hardware to do so has to be there for several other modes the decoder hardware can be used in
<agg>
ooi why are you doing "self.last_rx_data = last_rx_data = Signal(...)"?