thesofproject / sof

Sound Open Firmware
Other
548 stars 315 forks source link

[BUG] host-zephyr.c: DMA copy size alignment may result in frames been split between LL cycles #9301

Open serhiy-katsyuba-intel opened 3 months ago

serhiy-katsyuba-intel commented 3 months ago

host_get_copy_bytes_normal() in host-zephyr.c aligns down copy bytes to DMA specific value:

return ALIGN_DOWN(dma_copy_bytes, hd->dma_copy_align);

For some frame sizes, this can lead to a split first and/or last frame: one part of the frame is processed during one LL cycle, while the remaining portion is processed in the subsequent LL cycle. This could be a problem for components that assume the first sample in the buffer belongs to the first channel. Even if such components consume full frames, they could be bound on a fly as additional copier sinks or additional mixin sources or sinks, causing them to start processing from the wrong channel.

lgirdwood commented 2 months ago

@serhiy-katsyuba-intel any update here ? We are going to have jitter in the DMA vs timer, but this should always be leading rather than trailing vs the timer (since timer has to fire IRQ, handler runs, pipeline infra wakes up etc before FW read DMA position).

serhiy-katsyuba-intel commented 2 months ago

No update, nobody is working on it. For typical stereo streams this should not cause any problem. The problem might happen for big frame size, e.g., for less typical streams with many channels.

lgirdwood commented 2 months ago

No update, nobody is working on it. For typical stereo streams this should not cause any problem. The problem might happen for big frame size, e.g., for less typical streams with many channels.

Ok, fwiw @iuliana-prodan came across something similar with a large "frame" size being used for HiFi SIMD, she did encounter a non aligned HiFi frame size mismatch. It could be coming from here...

kv2019i commented 1 month ago

I think by design, the host copier should not push partial frames into the audio pipeline. OTOH, in the audio pipeline, components can express alignment constraints with a separate API, so this is potentially handled there (but depends on the every audio module/component to declare constraints correctly. So I think this is a valid bug, but might be in practise hard to hit (and could be also fixed in the module by implementing constraints correctly). @singalsu any inputs on this?

kv2019i commented 1 month ago

No owner, moving to v2.12.

singalsu commented 1 month ago

I think by design, the host copier should not push partial frames into the audio pipeline. OTOH, in the audio pipeline, components can express alignment constraints with a separate API, so this is potentially handled there (but depends on the every audio module/component to declare constraints correctly. So I think this is a valid bug, but might be in practise hard to hit (and could be also fixed in the module by implementing constraints correctly). @singalsu any inputs on this?

Yep, the components are not capable to process partial frames. Having such buffers passed to components could trigger bugs in components if they don't correctly calculate what is available. We could have a testbench test mode with simulated host/dai-copier (file component) to catch issues like this in components.