[Sound-open-firmware] [RFC/RFT] dai: Make system stable by moving DAI buffer pointer update location

[Ranjani Sridharan]

On Sat, 2018-03-31 at 00:31 +0800, Keyon Jie wrote:
> From: Wu Zhigang <zhigang.wu at linux.intel.com>
> 
> Move the DAI's buffer pointer update location from dai_dma_cb()
> to dai_copy(). This will make pipeline more stable. The xrun
> possibility
> will be lower.Even in xrun it will recover, it will not have dsp
> oops.
> DAI's buffer pointer will be updated in the pipeline copy sequence
> as other components did. Then the DMA will not update the buffer
> pointer
> by itself. in HOST--->VOL--->DAI pipeline, when DMA finished, it has
> to
> reschedule the pipeline to let the pointer update. this could avoid
> the xrun when adjust the read and write pointers by different
> componets.

I may be wrong here but how do we make sure we are always in sync with
the clock rate in this case? 

> 
> Signed-off-by: Wu Zhigang <zhigang.wu at linux.intel.com>
> Tested-by: Keyon Jie <yang.jie at linux.intel.com>
> ---
> Tested on APL-GPMRB
> kernel: sof-v4.14   commit <c33cfed51fff5eb156dcf70ba849e6ffb3008377>
> sof:    1.1-stable  commit <949de92142aa9cdd4a31fda68f05d29212683cc1>
> tools:  1.1-stable  commit <b799e3fa3bab7f5df70f2458bda42de167e65fb3>
> 
> RFT: can someone help test it on byt also?
> 
>  src/audio/dai.c | 108 +++++++++++++++++++++++++++++-----------------
> ----------
>  1 file changed, 56 insertions(+), 52 deletions(-)
> 
> diff --git a/src/audio/dai.c b/src/audio/dai.c
> index e1ea9ab..6d0c1fa 100644
> --- a/src/audio/dai.c
> +++ b/src/audio/dai.c
> @@ -78,7 +78,6 @@ static void dai_dma_cb(void *data, uint32_t type,
> struct dma_sg_elem *next)
>  	struct comp_dev *dev = (struct comp_dev *)data;
>  	struct dai_data *dd = comp_get_drvdata(dev);
>  	struct comp_buffer *dma_buffer;
> -	uint32_t copied_size;
>  
>  	trace_dai("irq");
>  
> @@ -114,57 +113,6 @@ static void dai_dma_cb(void *data, uint32_t
> type, struct dma_sg_elem *next)
>  		return;
>  	}
>  
> -	if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) {
> -		dma_buffer = list_first_item(&dev->bsource_list,
> -			struct comp_buffer, sink_list);
> -
> -		copied_size = dd->last_bytes ? dd->last_bytes : dd-
> >period_bytes;
> -
> -		/* recalc available buffer space */
> -		comp_update_buffer_consume(dma_buffer, copied_size);
> -
> -		/* writeback buffer contents from cache */
> -		dcache_writeback_region(dma_buffer->r_ptr,
> copied_size);
> -
> -		/* update host position(in bytes offset) for drivers
> */
> -		dev->position += copied_size;
> -		if (dd->dai_pos) {
> -			dd->dai_pos_blks += copied_size;
> -			*dd->dai_pos = dd->dai_pos_blks +
> -				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);
> -
> -		/* invalidate buffer contents */
> -		dcache_invalidate_region(dma_buffer->w_ptr, dd-
> >period_bytes);
> -
> -		/* recalc available buffer space */
> -		comp_update_buffer_produce(dma_buffer, dd-
> >period_bytes);
> -
> -		/* update positions */
> -		dev->position += dd->period_bytes;
> -		if (dd->dai_pos) {
> -			dd->dai_pos_blks += dd->period_bytes;
> -			*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 */
>  	if (dev->state == COMP_STATE_ACTIVE)
>  		pipeline_schedule_copy(dev->pipeline, 0);
> @@ -607,6 +555,62 @@ static int dai_comp_trigger(struct comp_dev
> *dev, int cmd)
>  /* copy and process stream data from source to sink buffers */
>  static int dai_copy(struct comp_dev *dev)
>  {
> +	struct dai_data *dd = comp_get_drvdata(dev);
> +	struct comp_buffer *dma_buffer;
> +	uint32_t copied_size;
> +
> +	if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) {
> +		dma_buffer = list_first_item(&dev->bsource_list,
> +					     struct comp_buffer,
> sink_list);
> +
> +		copied_size = dd->last_bytes ?
> +			dd->last_bytes : dd->period_bytes;
> +
> +		/* recalc available buffer space */
> +		comp_update_buffer_consume(dma_buffer, copied_size);
> +
> +		/* writeback buffer contents from cache */
> +		dcache_writeback_region(dma_buffer->r_ptr,
> copied_size);
> +
> +		/* update host position(in bytes offset) for drivers
> */
> +		dev->position += copied_size;
> +		if (dd->dai_pos) {
> +			dd->dai_pos_blks += copied_size;
> +			*dd->dai_pos = dd->dai_pos_blks +
> +				dma_buffer->r_ptr - dma_buffer-
> >addr;
> +		}
> +
> +		/* make sure there is available 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);
> +
> +		/* invalidate buffer contents */
> +		dcache_invalidate_region(dma_buffer->w_ptr, dd-
> >period_bytes);
> +
> +		/* recalc available buffer space */
> +		comp_update_buffer_produce(dma_buffer, dd-
> >period_bytes);
> +
> +		/* update positions */
> +		dev->position += dd->period_bytes;
> +		if (dd->dai_pos) {
> +			dd->dai_pos_blks += dd->period_bytes;
> +			*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);
> +		}
> +	}
> +
>  	return 0;
>  }
>