Skip to content

Commit bbe1490

Browse files
zephyr-dma: Fix HW DMA position regs going out of sync with w_ptr/r_ptr.
The DMA hardware keeps track of data in the DMA buffer using hardware Read and Write Position registers. The software uses the struct audio_stream with w_ptr and r_ptr for a similar purpose. If the software w_ptr and r_ptr do not point to the same location as the hardware Write and Read Position registers, the problem occurs. Such desynchronization happens upon a pipeline reset. The dma_buffer is freed during a reset in dai-zephyr or host-zephyr and reallocated in prepare() after the pipeline resume. The reallocated dma_buffer has w_ptr and r_ptr set to NULL, while the hardware Read and Write Position registers retain their values. If, for example, in the DAI playback case, the difference between the Write Position register and w_ptr is more than one period, the problem could easily go unnoticed as the hardware simply copies older data. In case where the difference between the Write Position and w_ptr is more than 0 but less than one period, the DMA copies part of the new data and part of the old data, resulting in glitches. This fix ensures that the software w_ptr and r_ptr stay in sync with the hardware Write and Read Position registers. Signed-off-by: Serhiy Katsyuba <serhiy.katsyuba@intel.com>
1 parent b193c48 commit bbe1490

File tree

3 files changed

+42
-0
lines changed

3 files changed

+42
-0
lines changed

src/audio/dai-zephyr.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,6 +1516,8 @@ int dai_zephyr_multi_endpoint_copy(struct dai_data **dd, struct comp_dev *dev,
15161516
return ret;
15171517
}
15181518

1519+
audio_stream_sync_to_hw(&dd[i]->dma_buffer->stream, &stat);
1520+
15191521
avail_bytes = MIN(avail_bytes, stat.pending_length);
15201522
free_bytes = MIN(free_bytes, stat.free);
15211523
}
@@ -1647,6 +1649,8 @@ int dai_common_copy(struct dai_data *dd, struct comp_dev *dev, pcm_converter_fun
16471649
return ret;
16481650
}
16491651

1652+
audio_stream_sync_to_hw(&dd->dma_buffer->stream, &stat);
1653+
16501654
avail_bytes = stat.pending_length;
16511655
free_bytes = stat.free;
16521656

src/audio/host-zephyr.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,14 @@ static uint32_t host_get_copy_bytes_normal(struct host_data *hd, struct comp_dev
386386
return 0;
387387
}
388388

389+
/* For deep buffers, dma_reload() is called less frequently than consume/produce
390+
* on dma_buffer. Replicate the hardware state of the DMA buffer to the dma_buffer
391+
* struct only when no consume/produce operations on dma_buffer have been called
392+
* since the last dma_reload().
393+
*/
394+
if (hd->partial_size == 0)
395+
audio_stream_sync_to_hw(&hd->dma_buffer->stream, &dma_stat);
396+
389397
dma_sample_bytes = hd->config.src_width;
390398

391399
/* calculate minimum size to copy */

src/include/sof/audio/audio_stream.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <sof/compiler_attributes.h>
2121
#include <rtos/panic.h>
2222
#include <sof/math/numbers.h>
23+
#include <sof/lib/dma.h>
2324
#include <rtos/alloc.h>
2425
#include <rtos/cache.h>
2526
#include <ipc/stream.h>
@@ -661,6 +662,35 @@ static inline void audio_stream_consume(struct audio_stream *buffer, uint32_t by
661662
buffer->free = buffer->size - buffer->avail;
662663
}
663664

665+
#ifdef __ZEPHYR__
666+
/**
667+
* Replicates the hardware state of the DMA buffer as a struct audio_stream.
668+
* @param stream Stream to update.
669+
* @param dma_status DMA buffer hardware state (struct dma_status from Zephyr).
670+
*/
671+
static inline void audio_stream_sync_to_hw(struct audio_stream *stream,
672+
const struct dma_status *dma_status)
673+
{
674+
uintptr_t buf_start = (uintptr_t)audio_stream_get_addr(stream);
675+
676+
/* Note: It is assumed here that dma_status values are reported as bytes. However,
677+
* this is actually platform-specific. Although unlikely, they could be, for example,
678+
* words on some particular platform, and changes should be made here to address
679+
* such case.
680+
*/
681+
682+
assert(dma_status->write_position < audio_stream_get_size(stream));
683+
assert(dma_status->read_position < audio_stream_get_size(stream));
684+
assert(dma_status->pending_length <= audio_stream_get_size(stream));
685+
assert(dma_status->free <= audio_stream_get_size(stream));
686+
687+
audio_stream_set_wptr(stream, (void *)(buf_start + dma_status->write_position));
688+
audio_stream_set_rptr(stream, (void *)(buf_start + dma_status->read_position));
689+
audio_stream_set_avail(stream, dma_status->pending_length);
690+
audio_stream_set_free(stream, dma_status->free);
691+
}
692+
#endif /* __ZEPHYR__ */
693+
664694
/**
665695
* Resets the buffer.
666696
* @param buffer Buffer to reset.

0 commit comments

Comments
 (0)