enjoy-digital / litepcie

Small footprint and configurable PCIe core
Other
484 stars 119 forks source link

DMA prog mode status register race condition / early-update #120

Closed smunaut closed 10 months ago

smunaut commented 1 year ago

So, first to be clear this is using a custom DMA driver that uses prog mode rather than the loop mode.

The way it works is that it polls the loop_status register to know which descriptors have been executed. Interrupts are only used/enabled if when polling there is nothing ready, then it enables interrupt and wait until one occurs.

And I hadn't noticed any bad behavior when doing continuous fast streaming, but I've seen tried to use the same driver for something that has a much more "bursty" behavior and those bursts are not even a multiple of the DMA block size.

And what I noticed there is that from time to time, I would get a block of 256 bytes that wasn't right. Upon further observation, it would be :

And what is happening is that the loop_status register is updated too early. And I suspect this happens because of the BufferizeEndpoints inserted at the output of the LitePCIeDMADescriptorSplitter which causes the last "split block" to be taken into the buffer when we're about to start the last block rather than when it's been issued.

And usually in continuous/fast application you might just not notice. But if your bursty data happens to be say 8000 bytes, it will actually issue all the sub-blocks of 256 bytes except for the last one (because not enough data to start it), but since that last sub block split descriptor is already in the Endpoint Buffer, the actually full descriptor will have been accepted and so loop_status reflect the whole descriptor as completed.

smunaut commented 1 year ago

FWIW, disabling the BufferizeEndpoints did indeed fix the issue.

enjoy-digital commented 10 months ago

Thanks @smunaut, use of BufferizeEndpoints has been removed from the DMA modules (timings have been improved directly in the logic), so the issue which was indeed related to the use of BufferizeEndpoints will no longer occur.