audio: chain_dma: fix link DMA reload logic for initial reload #10163
Conversation
Current code waits until host DMA has more than half of the DMA buffer size worth of data available for transfer. The code however does not check whether link DMA has space for all available data yet and can cause the link DMA write pointer to wrap the read pointer. This will break the delay reporting and can lead to link xruns if the wrapped write pointer ends up too close to the link DMA read position (which is moved by DMA hardware in playback case). Tested-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
| ret = dma_reload(cd->chan_link->dma->z_dev, | ||
| cd->chan_link->index, 0, 0, | ||
| half_buff_size); | ||
| MIN(host_avail_bytes, link_free_bytes)); |
There was a problem hiding this comment.
what is an "initial reload?" If this is the beginning of streaming than why wouldn't the "sink side" have enough space for the data - shouldn't it be empty at that time? And why isn't the same logic needed after the first transfer?
There was a problem hiding this comment.
@lyakh The dma_start() is called on link DMA, the buffer is full of data (link_avail_bytes=0, no room to write more). It is filled by zeroes when CHAIN_DMA IPC is sent by host to star the DMAs.
The same physical buffer is used for both host and link DMA, so we cannot call host reload, until link DMA has moved enough to make room.
Same check is done for following dma_reloads on L268-269 in this function. Only place where we had potentially to overwrite the link DMA pointer was in this initial write. It often didn't fail immediately, which made this harder to debug (as the error happened sometime later on).
There was a problem hiding this comment.
@kv2019i ah, so it's referring to the reloading after some space is freed in the initial silence-filled buffer? Ok, if that my understanding is correct, that makes sense
There was a problem hiding this comment.
@lyakh Ack, here's how it looks from DMA register dumps (playback, link dgbrp, host dgbwp):
buffer size 14208 (5ms buffer), link DMA reads from this, host DMA writes to it
0ms link read 640 host write 0 # zeroes played out, no dma_reload() yet on host
1ms link read 3136 host write 0 # zeroes played out, no dma_reload() yet on host
2ms link read 6016 host write 7104 # bug hit, initial 7104 write too much!
3ms link read 8960 host write 6016 # after xrun handling, write is back to ok distance
4ms link read 11712 host write 8896 # normal operation, still (should be) playing zeroes
5ms link read 256 host write 11712 # link DMA wraps around, started playing initial real samples
|
Test run with alsa_conformance_test (Intel internal test plan #56881 ) shows this PR has fixed all cases of: [11511.341336] chain_dma: chain_task_run: dma_get_status() link xrun occurred, ret = -32 These were particularly easy to hit with 44.1 multiple HDMI sampling rates: |
| ret = dma_reload(cd->chan_link->dma->z_dev, | ||
| cd->chan_link->index, 0, 0, | ||
| half_buff_size); | ||
| MIN(host_avail_bytes, link_free_bytes)); |
There was a problem hiding this comment.
@kv2019i ah, so it's referring to the reloading after some space is freed in the initial silence-filled buffer? Ok, if that my understanding is correct, that makes sense
4 participants
Current code waits until host DMA has more than half of the DMA buffer size worth of data available for transfer. The code however does not check whether link DMA has space for all available data yet and can cause the link DMA write pointer to wrap the read pointer. This will break the delay reporting and can lead to link xruns if the wrapped write pointer ends up too close to the link DMA read position (which is moved by DMA hardware in playback case).
Tested-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com