Missing case termination in simulation of psi_ms_daq_daq_sm.vhd

[elmarschmid]

I had trouble simulating the psi_ms_daq_daq_sm.vhd with the corresponding testbench. I reduced the subcases by commenting out the unused ones. This lead to failing simulations. I found out that this happend e.g. when I simulated case 1 subcase 2 and case 2 subcase 2 subsequently (same subcase).

I found out, that the procedure "FinishTestCase" is not called at the end of the "control" procedure in the _tbcase.vhd files. This lead to a failing initialization of the subcase and therefore to error reports in the simulation.

Calling the "FinishTestCase" procedure at the end of each "control" procedure cures the problem.

I did not check if there is a similar problem in the simulation of other components.

[elmarschmid]

To reproduce the error do the following:

in psi_ms_daq_daq_sm_tb_case_single_window.vhd: remove (comment) all subcases except -- Linear without overwrite, no trigger

in psi_ms_daq_daq_sm_tb_case_multi_window.vhd: remove (comment) all subcases except -- Linear without overwrite, no trigger

Run the simulation.

[obruendl]

Hi Elmar

When writing the testbench, I did not spend any thoughts on supporting the scenario that subcases are commented out, so it is well possible that this fails.

What I usually did was: adding a "report XXX" statement at the beginning of the subcase I was interested in. In Modelsim you can then jump to the beginning of this subcase by clicking on the timestamp in the console.

[elmarschmid]

Hi Oliver

I'm absolutely aware that this is no case you encounter when designing and testing the core. Since the "FinishTestCase" procedure existed and was added in some of the control procedures, I thought the intention was probably to add it in all of them. That's why I reported this even though it's actually not a real issue.

By the way, thanks for that core. It's great.

[obruendl]

Did it work out of the box without any issues?

[elmarschmid]

Well, ok, I haven't built the design yet. So the comment I made only refers to the code, simulation and documentation. Instead of writing a bunch of streams to memory we collect packets delivered from several boards in the memory. I will adapt the core correspondingly. We will not need the overwrite and ringbuffer functionality. I removed these things which is why I also removed the corresponding test cases ending up with the simulation stopping somewhere in the middle due to the problem I described in the issue.

I really like the concept of the (fancy) selfchecking testbenches. I could do the simulation right away - out of the box - by just calling the scripts. It's all well described in the readme files so no big effort to do this as a start. It took me a while to dive into the test procedures through all levels and see how it all works. But I definitely think it's worth the effort.

[obruendl]

Are you asure adaptation of the core is needed? I took care designing it in a way, that it can be used out-of-the-box to record AXI-Stream packets. Ring-buffer and overwrite behavior can be configured through the register bank, so they do not have to be removed from HDL.

[elmarschmid]

I'm aware that the core is very fexible and that features like ringbuffer and overwrite are configurable (even at runtime).

The reason I adapt the core is actually to keep packets belonging to the same event together. Some inputs might deliver more or larger packets in an event than others do. So if I the system is currently processing event n, I have to postpone processing of inputs that already deliver data of event n+1. I did not see how to map this behavior to the configuration options of the library core. And since I had to adapt the core anyway, I decided remove features I do not deed (ringbuffer, overwrite, priorities) to save resources.

[obruendl]

I assume you have one AXI-Stream packet per event on each stream (each input). Is this correct? If so, I think no changes are required:

You can configure buffer and window sizes per stream. You just have to make sure that the largest packet arriving on one stream fits into a window size of this stream. You can then use multiple windows, so that data of multiple events can be buffered for each stream. This way you can wait until the data of all stream for a certain event arrived and then process it.

Because the core is able of recording data for multiple events in separate windows, you can record data for the next event even if data from the last event is not yet read.

[elmarschmid]

I assume you have one AXI-Stream packet per event on each stream (each input).

No, there are multiple packets per event and stream and there can be a different number of packets for each stream. I can define the maximum packet size per stream but the number of packets depends on many parameters.

You can configure buffer and window sizes per stream. You just have to make sure that the largest packet arriving on one stream fits into a window size of this stream.

I configure for one packet per window i.e. one event can be several windows per stream

You can then use multiple windows, so that data of multiple events can be buffered for each stream. This way you can wait until the data of all stream for a certain event arrived and then process it.

The software can only process an event when all packets for the event arrived. The corresponding information is taken from the packet headers. If there are packets of a new event when the previous event is not complete, the software throws an error. So it's a requirement that I keep the packets from one event together.

Because the core is able of recording data for multiple events in separate windows, you can record data for the next event even if data from the last event is not yet read.

In this case I would have to put all packets of an event in one window. But in order not to delay event processing I would still have to make sure all packets of a specific event are processed first.

[obruendl]

What is the maximum number of packets per stream per event? In my opinion you can still use one window per packet as long as you have enough windows available to store all packets until they are processed.

The software can then read each packet from its own window.

Currently the maximum number of windows supported is 32. Maybe this is the limitting point.

[elmarschmid]

There can be much more than 32 packets per stream per event. I'm just wondering if going for a low window count is an option. If I only configure one or two windows per stream, I could control event buffering by the software. I.e. if the software stops reading windows of a specific stream because they already contain a packet of the next event the firmware automatically stops storing packets of the corresponding stream because the windows are occupied. In the meanwhile packets of the current event still get stored. Maybe this would be an option.

[obruendl]

You are absolutely free to only use one window and control the buffering from SW. You can access the current write pointer of the ring-buffer, so you are free to use the core as "large FIFO".

Another option that crossed my mind: If the packets have a protocol that allows parsing them without the "TLAST" information being available (i.e. if either the size is known or they contain a header informaing about the size), you could also do the following: Use two windows. Assert the trigger when all packets of a given event are completed. This way, all packets to a given event are recorded into one window. Packets for the next event can be recorded (into the next window) before data from the last event is read out. So this would work as double-buffer.

[elmarschmid]

That's a good option indeed. I'm a bit in two minds about about how to proceed now. I think since I'm almost done with the adaptions I made, I'll go ahead with the adapted core. Anyway, it's good to know that there are ways to solve the problem with the original core too. Thanks a lot for your support and the ideas you provided.

[obruendl]

Of course you are free to go with your changes.

Generally for future projects, I would suggest trying to keep the unchaged IP, just because it is maintained and you get bug-fixes. If you use a modified version, this is not the case.