Fix the free/avail buffer calculations to make sure any overrun or underrun is reported in trace and returned as a negative error in copy().
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/audio/dai.c | 12 ++++++++++ src/audio/eq_fir.c | 22 +++++++++--------- src/audio/eq_iir.c | 22 +++++++++--------- src/audio/host.c | 20 +++++++++++++---- src/audio/mixer.c | 27 ++++++++++++----------- src/audio/pipeline.c | 6 +++-- src/audio/src.c | 17 +++++++++++++- src/audio/tone.c | 9 ++++++-- src/audio/volume.c | 22 +++++++++--------- src/include/reef/audio/buffer.h | 49 ++++++++++++++--------------------------- 10 files changed, 118 insertions(+), 88 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c index 96b37ba..afa4aa2 100644 --- a/src/audio/dai.c +++ b/src/audio/dai.c @@ -117,6 +117,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) dma_buffer->r_ptr - dma_buffer->addr; }
+ /* make sure there is availble bytes for next period */ + if (dma_buffer->avail < dd->period_bytes) { + trace_dai_error("xru"); + comp_underrun(dev, dma_buffer, copied_size, 0); + } + } else { dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); @@ -134,6 +140,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next) *dd->dai_pos = dd->dai_pos_blks + dma_buffer->w_ptr - dma_buffer->addr; } + + /* make sure there is free bytes for next period */ + if (dma_buffer->free < dd->period_bytes) { + trace_dai_error("xro"); + comp_overrun(dev, dma_buffer, dd->period_bytes, 0); + } }
/* notify pipeline that DAI needs its buffer processed */ diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index e05920f..2dfa11c 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -448,7 +448,7 @@ static int eq_fir_copy(struct comp_dev *dev) struct comp_data *sd = comp_get_drvdata(dev); struct comp_buffer *source; struct comp_buffer *sink; - uint32_t copy_bytes; + int res;
trace_comp("EqF");
@@ -458,20 +458,20 @@ static int eq_fir_copy(struct comp_dev *dev) sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* Check that source has enough frames available and sink enough - * frames free. - */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run EQ if buffers have enough room */ - if (copy_bytes < sd->period_bytes) - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, sd->period_bytes); + if (res) { + trace_eq_error("xrn"); + return -EIO; /* xrun */ + }
sd->eq_fir_func(dev, source, sink, dev->frames);
/* calc new free and available */ - comp_update_buffer_consume(source, copy_bytes); - comp_update_buffer_produce(sink, copy_bytes); + comp_update_buffer_consume(source, sd->period_bytes); + comp_update_buffer_produce(sink, sd->period_bytes);
return dev->frames; } diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 1ac8c23..eb464af 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -446,7 +446,7 @@ static int eq_iir_copy(struct comp_dev *dev) struct comp_data *cd = comp_get_drvdata(dev); struct comp_buffer *source; struct comp_buffer *sink; - uint32_t copy_bytes; + int res;
trace_comp("EqI");
@@ -456,20 +456,20 @@ static int eq_iir_copy(struct comp_dev *dev) sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* Check that source has enough frames available and sink enough - * frames free. - */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run EQ if buffers have enough room */ - if (copy_bytes < cd->period_bytes) - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, cd->period_bytes); + if (res) { + trace_eq_iir_error("xrn"); + return -EIO; /* xrun */ + }
cd->eq_iir_func(dev, source, sink, dev->frames);
/* calc new free and available */ - comp_update_buffer_consume(source, copy_bytes); - comp_update_buffer_produce(sink, copy_bytes); + comp_update_buffer_consume(source, cd->period_bytes); + comp_update_buffer_produce(sink, cd->period_bytes);
return dev->frames; } diff --git a/src/audio/host.c b/src/audio/host.c index f542eec..88d45e4 100644 --- a/src/audio/host.c +++ b/src/audio/host.c @@ -610,15 +610,27 @@ static int host_copy(struct comp_dev *dev)
/* enough free or avail to copy ? */ if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) { + dma_buffer = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - if (dma_buffer->free < local_elem->size) - return 0; + + if (dma_buffer->free < local_elem->size) { + /* make sure there is free bytes for next period */ + trace_host_error("xro"); + comp_overrun(dev, dma_buffer, local_elem->size, 0); + return -EIO; + } } else { + dma_buffer = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); - if (dma_buffer->avail < local_elem->size) - return 0; + + if (dma_buffer->avail < local_elem->size) { + /* make sure there is avail bytes for next period */ + trace_host_error("xru"); + comp_underrun(dev, dma_buffer, local_elem->size, 0); + return -EIO; + } }
/* do DMA transfer */ diff --git a/src/audio/mixer.c b/src/audio/mixer.c index e72f1f2..e3611d4 100644 --- a/src/audio/mixer.c +++ b/src/audio/mixer.c @@ -220,10 +220,12 @@ static int mixer_copy(struct comp_dev *dev) struct list_item *blist; int32_t i = 0; int32_t num_mix_sources = 0; - int32_t xru = 0; + int res;
tracev_mixer("cpy");
+ sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); + /* calculate the highest runtime component status between input streams */ list_for_item(blist, &dev->bsource_list) { source = container_of(blist, struct comp_buffer, sink_list); @@ -243,22 +245,21 @@ static int mixer_copy(struct comp_dev *dev)
/* make sure no sources have underruns */ for (i = 0; i < num_mix_sources; i++) { - if (sources[i]->avail < md->period_bytes) { + + /* make sure source component buffer has enough data available + * and that the sink component buffer has enough free bytes + * for copy. Also check for XRUNs */ + res = comp_buffer_can_copy_bytes(sources[i], sink, md->period_bytes); + if (res < 0) { + trace_mixer_error("xru"); comp_underrun(dev, sources[i], sources[i]->avail, md->period_bytes); - xru = 1; + } else if (res > 0) { + trace_mixer_error("xru"); + comp_overrun(dev, sources[i], sink->free, + md->period_bytes); } } - /* underrun ? */ - if (xru) - return 0; - - /* make sure sink has no overruns */ - sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - if (sink->free < md->period_bytes) { - comp_overrun(dev, sink, sink->free, md->period_bytes); - return 0; - }
/* mix streams */ md->mix_func(dev, sink, sources, i, dev->frames); diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 947b64b..bec7519 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -742,7 +742,8 @@ static int pipeline_copy_from_upstream(struct comp_dev *start, err = pipeline_copy_from_upstream(start, buffer->source); if (err < 0) { trace_pipe_error("ePU"); - break; + trace_value(current->comp.id); + return err; } }
@@ -800,7 +801,8 @@ static int pipeline_copy_to_downstream(struct comp_dev *start, err = pipeline_copy_to_downstream(start, buffer->sink); if (err < 0) { trace_pipe_error("ePD"); - break; + trace_value(current->comp.id); + return err; } }
diff --git a/src/audio/src.c b/src/audio/src.c index c617038..3378065 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -496,8 +496,9 @@ static int src_copy(struct comp_dev *dev) int need_sink; size_t consumed = 0; size_t produced = 0; + int res;
- trace_comp("SRC"); + trace_src("SRC");
/* src component needs 1 source and 1 sink buffer */ source = list_first_item(&dev->bsource_list, struct comp_buffer, @@ -515,6 +516,20 @@ static int src_copy(struct comp_dev *dev) need_source = cd->param.blk_in * dev->frame_bytes; need_sink = cd->param.blk_out * dev->frame_bytes;
+ /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + res = comp_buffer_can_copy_bytes(source, sink, need_source); + if (res) { + trace_src_error("xru"); + return -EIO; /* xrun */ + } + res = comp_buffer_can_copy_bytes(source, sink, need_sink); + if (res) { + trace_src_error("xro"); + return -EIO; /* xrun */ + } + /* Run SRC function if buffers avail and free allow */ if (((int) source->avail >= need_source) && ((int) sink->free >= need_sink)) { cd->src_func(dev, source, sink, &consumed, &produced); diff --git a/src/audio/tone.c b/src/audio/tone.c index 8fc513d..581f87e 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -613,9 +613,14 @@ static int tone_copy(struct comp_dev * dev)
/* calc new free and available */ comp_update_buffer_produce(sink, cd->period_bytes); - }
- return dev->frames; + return dev->frames; + } else { + /* XRUN */ + trace_tone_error("xrn"); + comp_overrun(dev, sink, cd->period_bytes, sink->free); + return -EIO; + } }
static int tone_prepare(struct comp_dev * dev) diff --git a/src/audio/volume.c b/src/audio/volume.c index 3671046..e741b1a 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -561,7 +561,6 @@ static int volume_copy(struct comp_dev *dev) struct comp_data *cd = comp_get_drvdata(dev); struct comp_buffer *sink; struct comp_buffer *source; - uint32_t copy_bytes;
tracev_volume("cpy");
@@ -569,17 +568,18 @@ static int volume_copy(struct comp_dev *dev) source = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list); sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* get max number of bytes that can be copied */ - copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink); - - /* Run volume if buffers have enough room */ - if (copy_bytes < cd->source_period_bytes) { - comp_underrun(dev, source, copy_bytes, cd->source_period_bytes); - return 0; + /* make sure source component buffer has enough data available and that + * the sink component buffer has enough free bytes for copy. Also + * check for XRUNs */ + if (source->avail < cd->source_period_bytes) { + trace_volume_error("xru"); + comp_overrun(dev, source, cd->source_period_bytes, 0); + return -EIO; /* xrun */ } - if (copy_bytes < cd->sink_period_bytes) { - comp_overrun(dev, sink, copy_bytes, cd->sink_period_bytes); - return 0; + if (sink->free < cd->sink_period_bytes) { + trace_volume_error("xro"); + comp_underrun(dev, sink, cd->sink_period_bytes, 0); + return -EIO; /* xrun */ }
/* copy and scale volume */ diff --git a/src/include/reef/audio/buffer.h b/src/include/reef/audio/buffer.h index 179a84f..2bb2033 100644 --- a/src/include/reef/audio/buffer.h +++ b/src/include/reef/audio/buffer.h @@ -81,14 +81,6 @@ void buffer_free(struct comp_buffer *buffer); static inline void comp_update_buffer_produce(struct comp_buffer *buffer, uint32_t bytes) { - /* complain loudly if component tries to overrun buffer - * components MUST check for free space first !! */ - if (bytes > buffer->free) { - trace_buffer_error("Xxo"); - trace_value(buffer->ipc_buffer.comp.id); - return; - } - buffer->w_ptr += bytes;
/* check for pointer wrap */ @@ -116,14 +108,6 @@ static inline void comp_update_buffer_produce(struct comp_buffer *buffer, static inline void comp_update_buffer_consume(struct comp_buffer *buffer, uint32_t bytes) { - /* complain loudly if component tries to underrun buffer - * components MUST check for avail space first !! */ - if (buffer->avail < bytes) { - trace_buffer_error("Xxu"); - trace_value(buffer->ipc_buffer.comp.id); - return; - } - buffer->r_ptr += bytes;
/* check for pointer wrap */ @@ -148,29 +132,28 @@ static inline void comp_update_buffer_consume(struct comp_buffer *buffer, }
/* get the max number of bytes that can be copied between sink and source */ -static inline uint32_t comp_buffer_get_copy_bytes(struct comp_dev *dev, - struct comp_buffer *source, struct comp_buffer *sink) +static inline int comp_buffer_can_copy_bytes(struct comp_buffer *source, + struct comp_buffer *sink, uint32_t bytes) { - uint32_t copy_bytes; + /* check for underrun */ + if (source->avail < bytes) + return -1;
- /* Check that source has enough frames available and sink enough - * frames free. - */ - if (source->avail > sink->free) - copy_bytes = sink->free; - else - copy_bytes = source->avail; + /* check for overrun */ + if (sink->free < bytes) + return 1;
- return copy_bytes; + /* we are good to copy */ + return 0; }
-static inline void buffer_reset_pos(struct comp_buffer *buffer, - uint32_t sink_period_bytes) +static inline uint32_t comp_buffer_get_copy_bytes(struct comp_buffer *source, + struct comp_buffer *sink) { - buffer->r_ptr = buffer->addr + buffer->size - sink_period_bytes; - buffer->w_ptr = buffer->addr; - buffer->free = buffer->size; - buffer->avail = 0; + if (source->avail < sink->free) + return sink->free; + else + return source->avail; }
static inline void buffer_reset_pos(struct comp_buffer *buffer)