Re: [PATCH] dma: mmp-tdma: refine dma disable and dma-pos update

[Vinod Koul]

On Sat, Feb 28, 2015 at 05:39:53PM +0800, Qiao Zhou wrote:
> On 02/23/2015 07:23 PM, Vinod Koul wrote:
> >On Thu, Feb 05, 2015 at 08:54:19PM +0800, Qiao Zhou wrote:
> >>Below are the refinements.
> >>1. Set DMA abort bit when disabling dma channel. This will clear
> >>the remaining data in dma FIFO, to fix channel-swap issue.
> >>2. Read DMA HW pointer when updating DMA status. Previously dma
> >>position is calculated by adding one period size in dma interrupt.
> >>This is inaccurate/insufficient for some high-quality audio APP.
> >>Since interrupt bottom half handler has variable schedule delay,
> >>it causes big error when calculating sample delay. Read the actual
> >>HW pointer and feedback can improve the accuracy.
> >>3. Do some minor code clean.
> >This fails to apply for me, care to rebase pls
> >
> Rebased. Thanks.
And that is why people use git-send email and don't copy patches. Pls resend
again, as your MUA has wrapped this so cant apply

-- 
~Vinod

> 
> From b9cd9aca9c7700423276ba7fb49f267e00e63792 Mon Sep 17 00:00:00 2001
> From: Qiao Zhou <zhouqiao@xxxxxxxxxxx>
> Date: Sat, 28 Feb 2015 17:05:21 +0800
> Subject: [PATCH v2] dma: mmp-tdma: refine dma disable and dma-pos update
> 
> Below are the refinements.
> 1. Set DMA abort bit when disabling dma channel. This will clear
> the remaining data in dma FIFO, to fix channel-swap issue.
> 2. Read DMA HW pointer when updating DMA status. Previously dma
> position is calculated by adding one period size in dma interrupt.
> This is inaccurate/insufficient for some high-quality audio APP.
> Since interrupt bottom half handler has variable schedule delay,
> it causes big error when calculating sample delay. Read the actual
> HW pointer and feedback can improve the accuracy.
> 3. Do some minor code clean.
> 
> Signed-off-by: Qiao Zhou <zhouqiao@xxxxxxxxxxx>
> ---
>  drivers/dma/mmp_tdma.c |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
> index 70c2fa9..b6f4e1f 100644
> --- a/drivers/dma/mmp_tdma.c
> +++ b/drivers/dma/mmp_tdma.c
> @@ -110,7 +110,7 @@ struct mmp_tdma_chan {
>  	struct tasklet_struct		tasklet;
> 
>  	struct mmp_tdma_desc		*desc_arr;
> -	phys_addr_t			desc_arr_phys;
> +	dma_addr_t			desc_arr_phys;
>  	int				desc_num;
>  	enum dma_transfer_direction	dir;
>  	dma_addr_t			dev_addr;
> @@ -166,9 +166,12 @@ static void mmp_tdma_enable_chan(struct
> mmp_tdma_chan *tdmac)
>  static int mmp_tdma_disable_chan(struct dma_chan *chan)
>  {
>  	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
> +	u32 tdcr;
> 
> -	writel(readl(tdmac->reg_base + TDCR) & ~TDCR_CHANEN,
> -					tdmac->reg_base + TDCR);
> +	tdcr = readl(tdmac->reg_base + TDCR);
> +	tdcr |= TDCR_ABR;
> +	tdcr &= ~TDCR_CHANEN;
> +	writel(tdcr, tdmac->reg_base + TDCR);
> 
>  	tdmac->status = DMA_COMPLETE;
> 
> @@ -296,12 +299,27 @@ static int mmp_tdma_clear_chan_irq(struct
> mmp_tdma_chan *tdmac)
>  	return -EAGAIN;
>  }
> 
> +static size_t mmp_tdma_get_pos(struct mmp_tdma_chan *tdmac)
> +{
> +	size_t reg;
> +
> +	if (tdmac->idx == 0) {
> +		reg = __raw_readl(tdmac->reg_base + TDSAR);
> +		reg -= tdmac->desc_arr[0].src_addr;
> +	} else if (tdmac->idx == 1) {
> +		reg = __raw_readl(tdmac->reg_base + TDDAR);
> +		reg -= tdmac->desc_arr[0].dst_addr;
> +	} else
> +		return -EINVAL;
> +
> +	return reg;
> +}
> +
>  static irqreturn_t mmp_tdma_chan_handler(int irq, void *dev_id)
>  {
>  	struct mmp_tdma_chan *tdmac = dev_id;
> 
>  	if (mmp_tdma_clear_chan_irq(tdmac) == 0) {
> -		tdmac->pos = (tdmac->pos + tdmac->period_len) % tdmac->buf_len;
>  		tasklet_schedule(&tdmac->tasklet);
>  		return IRQ_HANDLED;
>  	} else
> @@ -343,7 +361,7 @@ static void mmp_tdma_free_descriptor(struct
> mmp_tdma_chan *tdmac)
>  	int size = tdmac->desc_num * sizeof(struct mmp_tdma_desc);
> 
>  	gpool = tdmac->pool;
> -	if (tdmac->desc_arr)
> +	if (gpool && tdmac->desc_arr)
>  		gen_pool_free(gpool, (unsigned long)tdmac->desc_arr,
>  				size);
>  	tdmac->desc_arr = NULL;
> @@ -499,6 +517,7 @@ static enum dma_status mmp_tdma_tx_status(struct
> dma_chan *chan,
>  {
>  	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
> 
> +	tdmac->pos = mmp_tdma_get_pos(tdmac);
>  	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
>  			 tdmac->buf_len - tdmac->pos);
> 
> @@ -610,7 +629,7 @@ static int mmp_tdma_probe(struct platform_device *pdev)
>  	int i, ret;
>  	int irq = 0, irq_num = 0;
>  	int chan_num = TDMA_CHANNEL_NUM;
> -	struct gen_pool *pool;
> +	struct gen_pool *pool = NULL;
> 
>  	of_id = of_match_device(mmp_tdma_dt_ids, &pdev->dev);
>  	if (of_id)
> -- 
> 1.7.0.4
> 
> 
> -- 
> 
> Best Regards
> Qiao
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/