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

From: Vinod Koul
Date: Mon Mar 02 2015 - 11:59:52 EST


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/