Re: [PATCH 1/1] dmaengine: imx-sdma - correct the dma transfer residue calculation

From: Peter Senna Tschudin
Date: Tue Oct 11 2016 - 07:45:45 EST



On Tuesday, October 11, 2016 13:13 CEST, Nandor Han <nandor.han@xxxxxx> wrote:

> The residue calculation was taking in consideration that dma
> transaction status will be always retrieved in the dma callback
> used to inform that dma transfer is complete. However this is not
> the case for all subsystems that use dma. Some subsystems use a
> timer to check the dma status periodically.
>
> Therefore the calculation was updated and residue is calculated
> accordingly by a) update the residue calculation taking in
> consideration the last used buffer index by using *buf_ptail* variable
> and b) chn_real_count (number of bytes transferred) is initialized to
> zero, when dma channel is created, to avoid using an uninitialized
> value in residue calculation when dma status is checked without
> waiting dma complete event.

The previous patch* apparently introduced an issue that was detectable when using the fsl_ssi audio driver that produced non smooth audio output. This patch fix the issue.

I tested this patch by booting it in two different devices and checked that the sound issues related to fsl_ssi driver were fixed.

* - https://patchwork.kernel.org/patch/9268463/

>
> Signed-off-by: Nandor Han <nandor.han@xxxxxx>
Acked-by: Peter Senna Tschudin <peter.senna@xxxxxxxxxxxxx>
Tested-by: Peter Senna Tschudin <peter.senna@xxxxxxxxxxxxx>

> ---
> drivers/dma/imx-sdma.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index b9629b2..d1651a5 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -298,6 +298,7 @@ struct sdma_engine;
> * @event_id1 for channels that use 2 events
> * @word_size peripheral access size
> * @buf_tail ID of the buffer that was processed
> + * @buf_ptail ID of the previous buffer that was processed
> * @num_bd max NUM_BD. number of descriptors currently handling
> */
> struct sdma_channel {
> @@ -309,6 +310,7 @@ struct sdma_channel {
> unsigned int event_id1;
> enum dma_slave_buswidth word_size;
> unsigned int buf_tail;
> + unsigned int buf_ptail;
> unsigned int num_bd;
> unsigned int period_len;
> struct sdma_buffer_descriptor *bd;
> @@ -700,6 +702,8 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
> sdmac->chn_real_count = bd->mode.count;
> bd->mode.status |= BD_DONE;
> bd->mode.count = sdmac->period_len;
> + sdmac->buf_ptail = sdmac->buf_tail;
> + sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
>
> /*
> * The callback is called from the interrupt context in order
> @@ -710,9 +714,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
>
> dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
>
> - sdmac->buf_tail++;
> - sdmac->buf_tail %= sdmac->num_bd;
> -
> if (error)
> sdmac->status = old_status;
> }
> @@ -1186,6 +1187,8 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
> sdmac->flags = 0;
>
> sdmac->buf_tail = 0;
> + sdmac->buf_ptail = 0;
> + sdmac->chn_real_count = 0;
>
> dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
> sg_len, channel);
> @@ -1288,6 +1291,8 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
> sdmac->status = DMA_IN_PROGRESS;
>
> sdmac->buf_tail = 0;
> + sdmac->buf_ptail = 0;
> + sdmac->chn_real_count = 0;
> sdmac->period_len = period_len;
>
> sdmac->flags |= IMX_DMA_SG_LOOP;
> @@ -1385,7 +1390,7 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
> u32 residue;
>
> if (sdmac->flags & IMX_DMA_SG_LOOP)
> - residue = (sdmac->num_bd - sdmac->buf_tail) *
> + residue = (sdmac->num_bd - sdmac->buf_ptail) *
> sdmac->period_len - sdmac->chn_real_count;
> else
> residue = sdmac->chn_count - sdmac->chn_real_count;
> --
> 2.7.1
>