Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/audio/chain_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ static enum task_state chain_task_run(void *data)
if (!cd->first_data_received && host_avail_bytes > half_buff_size) {
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));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

if (ret < 0) {
tr_err(&chain_dma_tr,
"dma_reload() link error, ret = %d", ret);
Expand Down
Loading