[PATCH 0/3] Add pl330 residue support

[PATCH 0/3] Add pl330 residue support

[Padmavathi Venna]

This patch set adds support for finding the remaining number of bytes
in the current transfer and fills txstate.residue with the amount of
bytes remaining. Although Samsung specific dma operations will be
removed in future, the residue specific patches are required. So when
we add support for generic dma engine these patches have to be ported
accordingly.

Dylan Reid (3):
  dmaengine: pl330: Set residue in tx_status callback.
  ARM: SAMSUNG: Add residue DMA operation.
  ASoC: Update pointer to account for pending dma transfers.

 arch/arm/plat-samsung/dma-ops.c              |   14 +++++++
 arch/arm/plat-samsung/include/plat/dma-ops.h |    1 +
 drivers/dma/pl330.c                          |   55 +++++++++++++++++++++++++-
 sound/soc/samsung/dma.c                      |   27 +++++++++----
 4 files changed, 88 insertions(+), 9 deletions(-)

-- 
1.7.4.4

[PATCH 0/3] Add pl330 residue support

[Padmavathi Venna]

This patch set adds support for finding the remaining number of bytes
in the current transfer and fills txstate.residue with the amount of
bytes remaining. Although Samsung specific dma operations will be
removed in future, the residue specific patches are required. So when
we add support for generic dma engine these patches have to be ported
accordingly.

Dylan Reid (3):
  dmaengine: pl330: Set residue in tx_status callback.
  ARM: SAMSUNG: Add residue DMA operation.
  ASoC: Update pointer to account for pending dma transfers.

 arch/arm/plat-samsung/dma-ops.c              |   14 +++++++
 arch/arm/plat-samsung/include/plat/dma-ops.h |    1 +
 drivers/dma/pl330.c                          |   55 +++++++++++++++++++++++++-
 sound/soc/samsung/dma.c                      |   27 +++++++++----
 4 files changed, 88 insertions(+), 9 deletions(-)

-- 
1.7.4.4

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Fill txstate.residue with the amount of bytes remaining in the current
transfer if the transfer is not complete.  This will be of particular
use to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 drivers/dma/pl330.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 593827b..7ab9136 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
 	spin_unlock_irqrestore(&pch->lock, flags);
 }
 
+static inline int
+pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
+{
+	return ((desc->px.src_addr <= sar) &&
+		(sar <= (desc->px.src_addr + desc->px.bytes)));
+}
+
+static inline int
+pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
+{
+	return ((desc->px.dst_addr <= dar) &&
+		(dar <= (desc->px.dst_addr + desc->px.bytes)));
+}
+
+static unsigned int pl330_tx_residue(struct dma_chan *chan)
+{
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	void __iomem *regs = pch->dmac->pif.base;
+	struct pl330_thread *thrd = pch->pl330_chid;
+	struct dma_pl330_desc *desc;
+	unsigned int sar, dar;
+	unsigned int residue = 0;
+	unsigned long flags;
+
+	sar = readl(regs + SA(thrd->id));
+	dar = readl(regs + DA(thrd->id));
+
+	spin_lock_irqsave(&pch->lock, flags);
+
+	/* Find the desc related to the current buffer. */
+	list_for_each_entry(desc, &pch->work_list, node) {
+		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, sar)) {
+			residue = desc->px.bytes - (sar - desc->px.src_addr);
+			goto found_unlock;
+		}
+		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, dar)) {
+			residue = desc->px.bytes - (dar - desc->px.dst_addr);
+			goto found_unlock;
+		}
+	}
+
+found_unlock:
+	spin_unlock_irqrestore(&pch->lock, flags);
+
+	return residue;
+}
+
 static enum dma_status
 pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 		 struct dma_tx_state *txstate)
 {
-	return dma_cookie_status(chan, cookie, txstate);
+	enum dma_status ret;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
+		dma_set_residue(txstate, pl330_tx_residue(chan));
+
+	return ret;
 }
 
 static void pl330_issue_pending(struct dma_chan *chan)
-- 
1.7.4.4

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Fill txstate.residue with the amount of bytes remaining in the current
transfer if the transfer is not complete.  This will be of particular
use to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 drivers/dma/pl330.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 593827b..7ab9136 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
 	spin_unlock_irqrestore(&pch->lock, flags);
 }
 
+static inline int
+pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
+{
+	return ((desc->px.src_addr <= sar) &&
+		(sar <= (desc->px.src_addr + desc->px.bytes)));
+}
+
+static inline int
+pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
+{
+	return ((desc->px.dst_addr <= dar) &&
+		(dar <= (desc->px.dst_addr + desc->px.bytes)));
+}
+
+static unsigned int pl330_tx_residue(struct dma_chan *chan)
+{
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	void __iomem *regs = pch->dmac->pif.base;
+	struct pl330_thread *thrd = pch->pl330_chid;
+	struct dma_pl330_desc *desc;
+	unsigned int sar, dar;
+	unsigned int residue = 0;
+	unsigned long flags;
+
+	sar = readl(regs + SA(thrd->id));
+	dar = readl(regs + DA(thrd->id));
+
+	spin_lock_irqsave(&pch->lock, flags);
+
+	/* Find the desc related to the current buffer. */
+	list_for_each_entry(desc, &pch->work_list, node) {
+		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, sar)) {
+			residue = desc->px.bytes - (sar - desc->px.src_addr);
+			goto found_unlock;
+		}
+		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, dar)) {
+			residue = desc->px.bytes - (dar - desc->px.dst_addr);
+			goto found_unlock;
+		}
+	}
+
+found_unlock:
+	spin_unlock_irqrestore(&pch->lock, flags);
+
+	return residue;
+}
+
 static enum dma_status
 pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 		 struct dma_tx_state *txstate)
 {
-	return dma_cookie_status(chan, cookie, txstate);
+	enum dma_status ret;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
+		dma_set_residue(txstate, pl330_tx_residue(chan));
+
+	return ret;
 }
 
 static void pl330_issue_pending(struct dma_chan *chan)
-- 
1.7.4.4

RE: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Chanho Park]

Hi Padmavathi,

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna
> Sent: Wednesday, September 11, 2013 3:08 PM
> To: linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com
> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com;
> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org;
> olofj@chromium.org
> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
> 
> From: Dylan Reid <dgreid@chromium.org>
> 
> Fill txstate.residue with the amount of bytes remaining in the current
> transfer if the transfer is not complete.  This will be of particular use
> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> 
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  drivers/dma/pl330.c |   55
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> 593827b..7ab9136 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
>  	spin_unlock_irqrestore(&pch->lock, flags);  }
> 
> +static inline int
> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
> +	return ((desc->px.src_addr <= sar) &&
> +		(sar <= (desc->px.src_addr + desc->px.bytes))); }
> +
> +static inline int
> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
> +	return ((desc->px.dst_addr <= dar) &&
> +		(dar <= (desc->px.dst_addr + desc->px.bytes))); }
> +
> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> +	struct dma_pl330_chan *pch = to_pchan(chan);
> +	void __iomem *regs = pch->dmac->pif.base;
> +	struct pl330_thread *thrd = pch->pl330_chid;
> +	struct dma_pl330_desc *desc;
> +	unsigned int sar, dar;
> +	unsigned int residue = 0;
> +	unsigned long flags;
> +
> +	sar = readl(regs + SA(thrd->id));
> +	dar = readl(regs + DA(thrd->id));
> +
> +	spin_lock_irqsave(&pch->lock, flags);
> +
> +	/* Find the desc related to the current buffer. */
> +	list_for_each_entry(desc, &pch->work_list, node) {
> +		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> sar)) {
> +			residue = desc->px.bytes - (sar -
desc->px.src_addr);
> +			goto found_unlock;
> +		}
> +		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> dar)) {
> +			residue = desc->px.bytes - (dar -
desc->px.dst_addr);
> +			goto found_unlock;
> +		}
> +	}
> +
> +found_unlock:
> +	spin_unlock_irqrestore(&pch->lock, flags);
> +
> +	return residue;
> +}
> +
>  static enum dma_status
>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  		 struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(chan, cookie, txstate);
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> +		dma_set_residue(txstate, pl330_tx_residue(chan));
> +
> +	return ret;

Why didn't you use a cookie value to track the request?
The cookie is assigned when each transfer is submitted.
If you save the value in the desc, we can find the request easily.

Thanks,

Best  Regards,
Chanho Park

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Chanho Park]

Hi Padmavathi,

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna
> Sent: Wednesday, September 11, 2013 3:08 PM
> To: linux-samsung-soc at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com
> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com;
> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org;
> olofj at chromium.org
> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
> 
> From: Dylan Reid <dgreid@chromium.org>
> 
> Fill txstate.residue with the amount of bytes remaining in the current
> transfer if the transfer is not complete.  This will be of particular use
> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> 
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  drivers/dma/pl330.c |   55
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> 593827b..7ab9136 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> dma_chan *chan)
>  	spin_unlock_irqrestore(&pch->lock, flags);  }
> 
> +static inline int
> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
> +	return ((desc->px.src_addr <= sar) &&
> +		(sar <= (desc->px.src_addr + desc->px.bytes))); }
> +
> +static inline int
> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
> +	return ((desc->px.dst_addr <= dar) &&
> +		(dar <= (desc->px.dst_addr + desc->px.bytes))); }
> +
> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> +	struct dma_pl330_chan *pch = to_pchan(chan);
> +	void __iomem *regs = pch->dmac->pif.base;
> +	struct pl330_thread *thrd = pch->pl330_chid;
> +	struct dma_pl330_desc *desc;
> +	unsigned int sar, dar;
> +	unsigned int residue = 0;
> +	unsigned long flags;
> +
> +	sar = readl(regs + SA(thrd->id));
> +	dar = readl(regs + DA(thrd->id));
> +
> +	spin_lock_irqsave(&pch->lock, flags);
> +
> +	/* Find the desc related to the current buffer. */
> +	list_for_each_entry(desc, &pch->work_list, node) {
> +		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> sar)) {
> +			residue = desc->px.bytes - (sar -
desc->px.src_addr);
> +			goto found_unlock;
> +		}
> +		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> dar)) {
> +			residue = desc->px.bytes - (dar -
desc->px.dst_addr);
> +			goto found_unlock;
> +		}
> +	}
> +
> +found_unlock:
> +	spin_unlock_irqrestore(&pch->lock, flags);
> +
> +	return residue;
> +}
> +
>  static enum dma_status
>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  		 struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(chan, cookie, txstate);
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> +		dma_set_residue(txstate, pl330_tx_residue(chan));
> +
> +	return ret;

Why didn't you use a cookie value to track the request?
The cookie is assigned when each transfer is submitted.
If you save the value in the desc, we can find the request easily.

Thanks,

Best  Regards,
Chanho Park

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Padma Venkat]

Hi Chanho,

On Thu, Sep 12, 2013 at 5:10 PM, Chanho Park <chanho61.park@samsung.com> wrote:
> Hi Padmavathi,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-
>> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna
>> Sent: Wednesday, September 11, 2013 3:08 PM
>> To: linux-samsung-soc@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com
>> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com;
>> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org;
>> olofj@chromium.org
>> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
>>
>> From: Dylan Reid <dgreid@chromium.org>
>>
>> Fill txstate.residue with the amount of bytes remaining in the current
>> transfer if the transfer is not complete.  This will be of particular use
>> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
>>
>> Signed-off-by: Dylan Reid <dgreid@chromium.org>
>> Reviewed-by: Olof Johansson <olofj@chromium.org>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  drivers/dma/pl330.c |   55
>> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 54 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
>> 593827b..7ab9136 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
>> dma_chan *chan)
>>       spin_unlock_irqrestore(&pch->lock, flags);  }
>>
>> +static inline int
>> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
>> +     return ((desc->px.src_addr <= sar) &&
>> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
>> +
>> +static inline int
>> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
>> +     return ((desc->px.dst_addr <= dar) &&
>> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
>> +
>> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
>> +     struct dma_pl330_chan *pch = to_pchan(chan);
>> +     void __iomem *regs = pch->dmac->pif.base;
>> +     struct pl330_thread *thrd = pch->pl330_chid;
>> +     struct dma_pl330_desc *desc;
>> +     unsigned int sar, dar;
>> +     unsigned int residue = 0;
>> +     unsigned long flags;
>> +
>> +     sar = readl(regs + SA(thrd->id));
>> +     dar = readl(regs + DA(thrd->id));
>> +
>> +     spin_lock_irqsave(&pch->lock, flags);
>> +
>> +     /* Find the desc related to the current buffer. */
>> +     list_for_each_entry(desc, &pch->work_list, node) {
>> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
>> sar)) {
>> +                     residue = desc->px.bytes - (sar -
> desc->px.src_addr);
>> +                     goto found_unlock;
>> +             }
>> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
>> dar)) {
>> +                     residue = desc->px.bytes - (dar -
> desc->px.dst_addr);
>> +                     goto found_unlock;
>> +             }
>> +     }
>> +
>> +found_unlock:
>> +     spin_unlock_irqrestore(&pch->lock, flags);
>> +
>> +     return residue;
>> +}
>> +
>>  static enum dma_status
>>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>>                struct dma_tx_state *txstate)
>>  {
>> -     return dma_cookie_status(chan, cookie, txstate);
>> +     enum dma_status ret;
>> +
>> +     ret = dma_cookie_status(chan, cookie, txstate);
>> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
>> +             dma_set_residue(txstate, pl330_tx_residue(chan));
>> +
>> +     return ret;
>
> Why didn't you use a cookie value to track the request?
> The cookie is assigned when each transfer is submitted.
> If you save the value in the desc, we can find the request easily.

Ok. I will check this and modify the code in the next version of patches.

Thanks for the suggestion.

>
> Thanks,
>
> Best  Regards,
> Chanho Park
>

Best Regards
Padma

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Padma Venkat]

Hi Chanho,

On Thu, Sep 12, 2013 at 5:10 PM, Chanho Park <chanho61.park@samsung.com> wrote:
> Hi Padmavathi,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-
>> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna
>> Sent: Wednesday, September 11, 2013 3:08 PM
>> To: linux-samsung-soc at vger.kernel.org; linux-arm-
>> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com
>> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com;
>> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org;
>> olofj at chromium.org
>> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
>>
>> From: Dylan Reid <dgreid@chromium.org>
>>
>> Fill txstate.residue with the amount of bytes remaining in the current
>> transfer if the transfer is not complete.  This will be of particular use
>> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
>>
>> Signed-off-by: Dylan Reid <dgreid@chromium.org>
>> Reviewed-by: Olof Johansson <olofj@chromium.org>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  drivers/dma/pl330.c |   55
>> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 54 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
>> 593827b..7ab9136 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
>> dma_chan *chan)
>>       spin_unlock_irqrestore(&pch->lock, flags);  }
>>
>> +static inline int
>> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
>> +     return ((desc->px.src_addr <= sar) &&
>> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
>> +
>> +static inline int
>> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
>> +     return ((desc->px.dst_addr <= dar) &&
>> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
>> +
>> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
>> +     struct dma_pl330_chan *pch = to_pchan(chan);
>> +     void __iomem *regs = pch->dmac->pif.base;
>> +     struct pl330_thread *thrd = pch->pl330_chid;
>> +     struct dma_pl330_desc *desc;
>> +     unsigned int sar, dar;
>> +     unsigned int residue = 0;
>> +     unsigned long flags;
>> +
>> +     sar = readl(regs + SA(thrd->id));
>> +     dar = readl(regs + DA(thrd->id));
>> +
>> +     spin_lock_irqsave(&pch->lock, flags);
>> +
>> +     /* Find the desc related to the current buffer. */
>> +     list_for_each_entry(desc, &pch->work_list, node) {
>> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
>> sar)) {
>> +                     residue = desc->px.bytes - (sar -
> desc->px.src_addr);
>> +                     goto found_unlock;
>> +             }
>> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
>> dar)) {
>> +                     residue = desc->px.bytes - (dar -
> desc->px.dst_addr);
>> +                     goto found_unlock;
>> +             }
>> +     }
>> +
>> +found_unlock:
>> +     spin_unlock_irqrestore(&pch->lock, flags);
>> +
>> +     return residue;
>> +}
>> +
>>  static enum dma_status
>>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>>                struct dma_tx_state *txstate)
>>  {
>> -     return dma_cookie_status(chan, cookie, txstate);
>> +     enum dma_status ret;
>> +
>> +     ret = dma_cookie_status(chan, cookie, txstate);
>> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
>> +             dma_set_residue(txstate, pl330_tx_residue(chan));
>> +
>> +     return ret;
>
> Why didn't you use a cookie value to track the request?
> The cookie is assigned when each transfer is submitted.
> If you save the value in the desc, we can find the request easily.

Ok. I will check this and modify the code in the next version of patches.

Thanks for the suggestion.

>
> Thanks,
>
> Best  Regards,
> Chanho Park
>

Best Regards
Padma

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Dylan Reid]

On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com> wrote:
> Hi Padmavathi,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-
>> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna
>> Sent: Wednesday, September 11, 2013 3:08 PM
>> To: linux-samsung-soc@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com
>> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com;
>> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org;
>> olofj@chromium.org
>> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
>>
>> From: Dylan Reid <dgreid@chromium.org>
>>
>> Fill txstate.residue with the amount of bytes remaining in the current
>> transfer if the transfer is not complete.  This will be of particular use
>> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
>>
>> Signed-off-by: Dylan Reid <dgreid@chromium.org>
>> Reviewed-by: Olof Johansson <olofj@chromium.org>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  drivers/dma/pl330.c |   55
>> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 54 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
>> 593827b..7ab9136 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
>> dma_chan *chan)
>>       spin_unlock_irqrestore(&pch->lock, flags);  }
>>
>> +static inline int
>> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
>> +     return ((desc->px.src_addr <= sar) &&
>> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
>> +
>> +static inline int
>> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
>> +     return ((desc->px.dst_addr <= dar) &&
>> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
>> +
>> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
>> +     struct dma_pl330_chan *pch = to_pchan(chan);
>> +     void __iomem *regs = pch->dmac->pif.base;
>> +     struct pl330_thread *thrd = pch->pl330_chid;
>> +     struct dma_pl330_desc *desc;
>> +     unsigned int sar, dar;
>> +     unsigned int residue = 0;
>> +     unsigned long flags;
>> +
>> +     sar = readl(regs + SA(thrd->id));
>> +     dar = readl(regs + DA(thrd->id));
>> +
>> +     spin_lock_irqsave(&pch->lock, flags);
>> +
>> +     /* Find the desc related to the current buffer. */
>> +     list_for_each_entry(desc, &pch->work_list, node) {
>> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
>> sar)) {
>> +                     residue = desc->px.bytes - (sar -
> desc->px.src_addr);
>> +                     goto found_unlock;
>> +             }
>> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
>> dar)) {
>> +                     residue = desc->px.bytes - (dar -
> desc->px.dst_addr);
>> +                     goto found_unlock;
>> +             }
>> +     }
>> +
>> +found_unlock:
>> +     spin_unlock_irqrestore(&pch->lock, flags);
>> +
>> +     return residue;
>> +}
>> +
>>  static enum dma_status
>>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>>                struct dma_tx_state *txstate)
>>  {
>> -     return dma_cookie_status(chan, cookie, txstate);
>> +     enum dma_status ret;
>> +
>> +     ret = dma_cookie_status(chan, cookie, txstate);
>> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
>> +             dma_set_residue(txstate, pl330_tx_residue(chan));
>> +
>> +     return ret;
>
> Why didn't you use a cookie value to track the request?
> The cookie is assigned when each transfer is submitted.
> If you save the value in the desc, we can find the request easily.

If there are several cyclic desc in the work list, is there a better
way to find the "current" one?  The chan struct tracks the last
completed and last submitted cookies, but these will be the first and
last respectively as long as the cyclic transfer is active.  Is there
an "active" cookie stored somewhere that I missed?

Looking for the first buffer with status == BUSY is an improvement
I'll make.  Any way to avoid looking through the list?

Thanks,

Dylan

>
> Thanks,
>
> Best  Regards,
> Chanho Park
>

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Dylan Reid]

On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com> wrote:
> Hi Padmavathi,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-
>> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna
>> Sent: Wednesday, September 11, 2013 3:08 PM
>> To: linux-samsung-soc at vger.kernel.org; linux-arm-
>> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com
>> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com;
>> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org;
>> olofj at chromium.org
>> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
>>
>> From: Dylan Reid <dgreid@chromium.org>
>>
>> Fill txstate.residue with the amount of bytes remaining in the current
>> transfer if the transfer is not complete.  This will be of particular use
>> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
>>
>> Signed-off-by: Dylan Reid <dgreid@chromium.org>
>> Reviewed-by: Olof Johansson <olofj@chromium.org>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  drivers/dma/pl330.c |   55
>> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 files changed, 54 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
>> 593827b..7ab9136 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
>> dma_chan *chan)
>>       spin_unlock_irqrestore(&pch->lock, flags);  }
>>
>> +static inline int
>> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
>> +     return ((desc->px.src_addr <= sar) &&
>> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
>> +
>> +static inline int
>> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
>> +     return ((desc->px.dst_addr <= dar) &&
>> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
>> +
>> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
>> +     struct dma_pl330_chan *pch = to_pchan(chan);
>> +     void __iomem *regs = pch->dmac->pif.base;
>> +     struct pl330_thread *thrd = pch->pl330_chid;
>> +     struct dma_pl330_desc *desc;
>> +     unsigned int sar, dar;
>> +     unsigned int residue = 0;
>> +     unsigned long flags;
>> +
>> +     sar = readl(regs + SA(thrd->id));
>> +     dar = readl(regs + DA(thrd->id));
>> +
>> +     spin_lock_irqsave(&pch->lock, flags);
>> +
>> +     /* Find the desc related to the current buffer. */
>> +     list_for_each_entry(desc, &pch->work_list, node) {
>> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
>> sar)) {
>> +                     residue = desc->px.bytes - (sar -
> desc->px.src_addr);
>> +                     goto found_unlock;
>> +             }
>> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
>> dar)) {
>> +                     residue = desc->px.bytes - (dar -
> desc->px.dst_addr);
>> +                     goto found_unlock;
>> +             }
>> +     }
>> +
>> +found_unlock:
>> +     spin_unlock_irqrestore(&pch->lock, flags);
>> +
>> +     return residue;
>> +}
>> +
>>  static enum dma_status
>>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>>                struct dma_tx_state *txstate)
>>  {
>> -     return dma_cookie_status(chan, cookie, txstate);
>> +     enum dma_status ret;
>> +
>> +     ret = dma_cookie_status(chan, cookie, txstate);
>> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
>> +             dma_set_residue(txstate, pl330_tx_residue(chan));
>> +
>> +     return ret;
>
> Why didn't you use a cookie value to track the request?
> The cookie is assigned when each transfer is submitted.
> If you save the value in the desc, we can find the request easily.

If there are several cyclic desc in the work list, is there a better
way to find the "current" one?  The chan struct tracks the last
completed and last submitted cookies, but these will be the first and
last respectively as long as the cyclic transfer is active.  Is there
an "active" cookie stored somewhere that I missed?

Looking for the first buffer with status == BUSY is an improvement
I'll make.  Any way to avoid looking through the list?

Thanks,

Dylan

>
> Thanks,
>
> Best  Regards,
> Chanho Park
>

RE: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Chanho Park]

Hi Dylan,

> -----Original Message-----
> From: dgreid@google.com [mailto:dgreid@google.com] On Behalf Of Dylan
> Reid
> Sent: Wednesday, October 02, 2013 1:34 PM
> To: Chanho Park
> Cc: Padmavathi Venna; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; padma.kvr@gmail.com; kgene.kim@samsung.com;
> arnd@arndb.de; Sangbeom Kim; vinod.koul@intel.com; Mark Brown; Olof
> Johansson
> Subject: Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status
> callback.
> 
> On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com>
> wrote:
> > Hi Padmavathi,
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel [mailto:linux-arm-kernel-
> >> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna
> >> Sent: Wednesday, September 11, 2013 3:08 PM
> >> To: linux-samsung-soc@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com
> >> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com;
> >> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org;
> >> olofj@chromium.org
> >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status
> callback.
> >>
> >> From: Dylan Reid <dgreid@chromium.org>
> >>
> >> Fill txstate.residue with the amount of bytes remaining in the
> >> current transfer if the transfer is not complete.  This will be of
> >> particular use to i2s DMA transfers, providing more accurate hw_ptr
> values to ASoC.
> >>
> >> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> >> Reviewed-by: Olof Johansson <olofj@chromium.org>
> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> >> ---
> >>  drivers/dma/pl330.c |   55
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 files changed, 54 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> >> 593827b..7ab9136 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> >> dma_chan *chan)
> >>       spin_unlock_irqrestore(&pch->lock, flags);  }
> >>
> >> +static inline int
> >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
> {
> >> +     return ((desc->px.src_addr <= sar) &&
> >> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
> >> +
> >> +static inline int
> >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
> {
> >> +     return ((desc->px.dst_addr <= dar) &&
> >> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
> >> +
> >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> >> +     struct dma_pl330_chan *pch = to_pchan(chan);
> >> +     void __iomem *regs = pch->dmac->pif.base;
> >> +     struct pl330_thread *thrd = pch->pl330_chid;
> >> +     struct dma_pl330_desc *desc;
> >> +     unsigned int sar, dar;
> >> +     unsigned int residue = 0;
> >> +     unsigned long flags;
> >> +
> >> +     sar = readl(regs + SA(thrd->id));
> >> +     dar = readl(regs + DA(thrd->id));
> >> +
> >> +     spin_lock_irqsave(&pch->lock, flags);
> >> +
> >> +     /* Find the desc related to the current buffer. */
> >> +     list_for_each_entry(desc, &pch->work_list, node) {
> >> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> >> sar)) {
> >> +                     residue = desc->px.bytes - (sar -
> > desc->px.src_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> >> dar)) {
> >> +                     residue = desc->px.bytes - (dar -
> > desc->px.dst_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +     }
> >> +
> >> +found_unlock:
> >> +     spin_unlock_irqrestore(&pch->lock, flags);
> >> +
> >> +     return residue;
> >> +}
> >> +
> >>  static enum dma_status
> >>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> >>                struct dma_tx_state *txstate)  {
> >> -     return dma_cookie_status(chan, cookie, txstate);
> >> +     enum dma_status ret;
> >> +
> >> +     ret = dma_cookie_status(chan, cookie, txstate);
> >> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> >> +             dma_set_residue(txstate, pl330_tx_residue(chan));
> >> +
> >> +     return ret;
> >
> > Why didn't you use a cookie value to track the request?
> > The cookie is assigned when each transfer is submitted.
> > If you save the value in the desc, we can find the request easily.
> 
> If there are several cyclic desc in the work list, is there a better way
> to find the "current" one?  The chan struct tracks the last completed and
> last submitted cookies, but these will be the first and last
> respectively as long as the cyclic transfer is active.  Is there an
> "active" cookie stored somewhere that I missed?

Assume there are three cookies. If you want to get the second cookie not
latest cookie, your way can be also correct in such case?
I think tx_status API is to get dma status of the given cookie.
You are only considering a cyclic case.

> 
> Looking for the first buffer with status == BUSY is an improvement I'll
> make.  Any way to avoid looking through the list?
> 
> Thanks,
> 
> Dylan
> 
> >
> > Thanks,
> >
> > Best  Regards,
> > Chanho Park
> >

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Chanho Park]

Hi Dylan,

> -----Original Message-----
> From: dgreid at google.com [mailto:dgreid at google.com] On Behalf Of Dylan
> Reid
> Sent: Wednesday, October 02, 2013 1:34 PM
> To: Chanho Park
> Cc: Padmavathi Venna; linux-samsung-soc at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; padma.kvr at gmail.com; kgene.kim at samsung.com;
> arnd at arndb.de; Sangbeom Kim; vinod.koul at intel.com; Mark Brown; Olof
> Johansson
> Subject: Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status
> callback.
> 
> On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com>
> wrote:
> > Hi Padmavathi,
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel [mailto:linux-arm-kernel-
> >> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna
> >> Sent: Wednesday, September 11, 2013 3:08 PM
> >> To: linux-samsung-soc at vger.kernel.org; linux-arm-
> >> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com
> >> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com;
> >> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org;
> >> olofj at chromium.org
> >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status
> callback.
> >>
> >> From: Dylan Reid <dgreid@chromium.org>
> >>
> >> Fill txstate.residue with the amount of bytes remaining in the
> >> current transfer if the transfer is not complete.  This will be of
> >> particular use to i2s DMA transfers, providing more accurate hw_ptr
> values to ASoC.
> >>
> >> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> >> Reviewed-by: Olof Johansson <olofj@chromium.org>
> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> >> ---
> >>  drivers/dma/pl330.c |   55
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 files changed, 54 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> >> 593827b..7ab9136 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> >> dma_chan *chan)
> >>       spin_unlock_irqrestore(&pch->lock, flags);  }
> >>
> >> +static inline int
> >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
> {
> >> +     return ((desc->px.src_addr <= sar) &&
> >> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
> >> +
> >> +static inline int
> >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
> {
> >> +     return ((desc->px.dst_addr <= dar) &&
> >> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
> >> +
> >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> >> +     struct dma_pl330_chan *pch = to_pchan(chan);
> >> +     void __iomem *regs = pch->dmac->pif.base;
> >> +     struct pl330_thread *thrd = pch->pl330_chid;
> >> +     struct dma_pl330_desc *desc;
> >> +     unsigned int sar, dar;
> >> +     unsigned int residue = 0;
> >> +     unsigned long flags;
> >> +
> >> +     sar = readl(regs + SA(thrd->id));
> >> +     dar = readl(regs + DA(thrd->id));
> >> +
> >> +     spin_lock_irqsave(&pch->lock, flags);
> >> +
> >> +     /* Find the desc related to the current buffer. */
> >> +     list_for_each_entry(desc, &pch->work_list, node) {
> >> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> >> sar)) {
> >> +                     residue = desc->px.bytes - (sar -
> > desc->px.src_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> >> dar)) {
> >> +                     residue = desc->px.bytes - (dar -
> > desc->px.dst_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +     }
> >> +
> >> +found_unlock:
> >> +     spin_unlock_irqrestore(&pch->lock, flags);
> >> +
> >> +     return residue;
> >> +}
> >> +
> >>  static enum dma_status
> >>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> >>                struct dma_tx_state *txstate)  {
> >> -     return dma_cookie_status(chan, cookie, txstate);
> >> +     enum dma_status ret;
> >> +
> >> +     ret = dma_cookie_status(chan, cookie, txstate);
> >> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> >> +             dma_set_residue(txstate, pl330_tx_residue(chan));
> >> +
> >> +     return ret;
> >
> > Why didn't you use a cookie value to track the request?
> > The cookie is assigned when each transfer is submitted.
> > If you save the value in the desc, we can find the request easily.
> 
> If there are several cyclic desc in the work list, is there a better way
> to find the "current" one?  The chan struct tracks the last completed and
> last submitted cookies, but these will be the first and last
> respectively as long as the cyclic transfer is active.  Is there an
> "active" cookie stored somewhere that I missed?

Assume there are three cookies. If you want to get the second cookie not
latest cookie, your way can be also correct in such case?
I think tx_status API is to get dma status of the given cookie.
You are only considering a cyclic case.

> 
> Looking for the first buffer with status == BUSY is an improvement I'll
> make.  Any way to avoid looking through the list?
> 
> Thanks,
> 
> Dylan
> 
> >
> > Thanks,
> >
> > Best  Regards,
> > Chanho Park
> >

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
> > > Why didn't you use a cookie value to track the request?
> > > The cookie is assigned when each transfer is submitted.
> > > If you save the value in the desc, we can find the request easily.
> > 
> > If there are several cyclic desc in the work list, is there a better way
> > to find the "current" one?  The chan struct tracks the last completed and
> > last submitted cookies, but these will be the first and last
> > respectively as long as the cyclic transfer is active.  Is there an
> > "active" cookie stored somewhere that I missed?
> 
> Assume there are three cookies. If you want to get the second cookie not
> latest cookie, your way can be also correct in such case?
> I think tx_status API is to get dma status of the given cookie.
> You are only considering a cyclic case.
For cyclic case you would have possible same descriptor running till you
terminate.

For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
7, 8 and 9. If you query the status of any descriptor and pass the last optional
txstate arg then you will know the last cookie completed. so if txstate->last is
7, then 7th was completed and 8 should be running and 9 in queue!
> > Looking for the first buffer with status == BUSY is an improvement I'll
> > make.  Any way to avoid looking through the list?
Its already there :)

-- 
~Vinod

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
> > > Why didn't you use a cookie value to track the request?
> > > The cookie is assigned when each transfer is submitted.
> > > If you save the value in the desc, we can find the request easily.
> > 
> > If there are several cyclic desc in the work list, is there a better way
> > to find the "current" one?  The chan struct tracks the last completed and
> > last submitted cookies, but these will be the first and last
> > respectively as long as the cyclic transfer is active.  Is there an
> > "active" cookie stored somewhere that I missed?
> 
> Assume there are three cookies. If you want to get the second cookie not
> latest cookie, your way can be also correct in such case?
> I think tx_status API is to get dma status of the given cookie.
> You are only considering a cyclic case.
For cyclic case you would have possible same descriptor running till you
terminate.

For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
7, 8 and 9. If you query the status of any descriptor and pass the last optional
txstate arg then you will know the last cookie completed. so if txstate->last is
7, then 7th was completed and 8 should be running and 9 in queue!
> > Looking for the first buffer with status == BUSY is an improvement I'll
> > make.  Any way to avoid looking through the list?
Its already there :)

-- 
~Vinod

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Dylan Reid]

On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
>> > > Why didn't you use a cookie value to track the request?
>> > > The cookie is assigned when each transfer is submitted.
>> > > If you save the value in the desc, we can find the request easily.
>> >
>> > If there are several cyclic desc in the work list, is there a better way
>> > to find the "current" one?  The chan struct tracks the last completed and
>> > last submitted cookies, but these will be the first and last
>> > respectively as long as the cyclic transfer is active.  Is there an
>> > "active" cookie stored somewhere that I missed?
>>
>> Assume there are three cookies. If you want to get the second cookie not
>> latest cookie, your way can be also correct in such case?
>> I think tx_status API is to get dma status of the given cookie.
>> You are only considering a cyclic case.
> For cyclic case you would have possible same descriptor running till you
> terminate.

The cyclic case that makes this interesting is when there are multiple
cyclic descriptors in the list. The cookie and completed_cookie
markers don't help to determine which of the descriptors in the list
is currently active.  dma_cookie_complete isn't called for a cyclic
desc, the desc is just pushed to the end of the list.

>
> For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
> 7, 8 and 9. If you query the status of any descriptor and pass the last optional
> txstate arg then you will know the last cookie completed. so if txstate->last is
> 7, then 7th was completed and 8 should be running and 9 in queue!

Got it, but the correct desc for cookie 8 will still have to be
searched for, correct?

Thanks for the advise.

Dylan

>> > Looking for the first buffer with status == BUSY is an improvement I'll
>> > make.  Any way to avoid looking through the list?
> Its already there :)
>
> --
> ~Vinod

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Dylan Reid]

On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
>> > > Why didn't you use a cookie value to track the request?
>> > > The cookie is assigned when each transfer is submitted.
>> > > If you save the value in the desc, we can find the request easily.
>> >
>> > If there are several cyclic desc in the work list, is there a better way
>> > to find the "current" one?  The chan struct tracks the last completed and
>> > last submitted cookies, but these will be the first and last
>> > respectively as long as the cyclic transfer is active.  Is there an
>> > "active" cookie stored somewhere that I missed?
>>
>> Assume there are three cookies. If you want to get the second cookie not
>> latest cookie, your way can be also correct in such case?
>> I think tx_status API is to get dma status of the given cookie.
>> You are only considering a cyclic case.
> For cyclic case you would have possible same descriptor running till you
> terminate.

The cyclic case that makes this interesting is when there are multiple
cyclic descriptors in the list. The cookie and completed_cookie
markers don't help to determine which of the descriptors in the list
is currently active.  dma_cookie_complete isn't called for a cyclic
desc, the desc is just pushed to the end of the list.

>
> For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
> 7, 8 and 9. If you query the status of any descriptor and pass the last optional
> txstate arg then you will know the last cookie completed. so if txstate->last is
> 7, then 7th was completed and 8 should be running and 9 in queue!

Got it, but the correct desc for cookie 8 will still have to be
searched for, correct?

Thanks for the advise.

Dylan

>> > Looking for the first buffer with status == BUSY is an improvement I'll
>> > make.  Any way to avoid looking through the list?
> Its already there :)
>
> --
> ~Vinod

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Wed, Oct 09, 2013 at 01:37:56PM -0700, Dylan Reid wrote:
> On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
> >> > > Why didn't you use a cookie value to track the request?
> >> > > The cookie is assigned when each transfer is submitted.
> >> > > If you save the value in the desc, we can find the request easily.
> >> >
> >> > If there are several cyclic desc in the work list, is there a better way
> >> > to find the "current" one?  The chan struct tracks the last completed and
> >> > last submitted cookies, but these will be the first and last
> >> > respectively as long as the cyclic transfer is active.  Is there an
> >> > "active" cookie stored somewhere that I missed?
> >>
> >> Assume there are three cookies. If you want to get the second cookie not
> >> latest cookie, your way can be also correct in such case?
> >> I think tx_status API is to get dma status of the given cookie.
> >> You are only considering a cyclic case.
> > For cyclic case you would have possible same descriptor running till you
> > terminate.
> 
> The cyclic case that makes this interesting is when there are multiple
> cyclic descriptors in the list. The cookie and completed_cookie
> markers don't help to determine which of the descriptors in the list
> is currently active.  dma_cookie_complete isn't called for a cyclic
> desc, the desc is just pushed to the end of the list.
Yes the cyclic is a very different case. I think driver can still return for
cyclic case which was txstate->last and that will give clue to client which is
getting processed now!

> > For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
> > 7, 8 and 9. If you query the status of any descriptor and pass the last optional
> > txstate arg then you will know the last cookie completed. so if txstate->last is
> > 7, then 7th was completed and 8 should be running and 9 in queue!
> 
> Got it, but the correct desc for cookie 8 will still have to be
> searched for, correct?
Yes

-- 
~Vinod

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Wed, Oct 09, 2013 at 01:37:56PM -0700, Dylan Reid wrote:
> On Sun, Oct 6, 2013 at 8:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Mon, Oct 07, 2013 at 10:39:34AM +0900, Chanho Park wrote:
> >> > > Why didn't you use a cookie value to track the request?
> >> > > The cookie is assigned when each transfer is submitted.
> >> > > If you save the value in the desc, we can find the request easily.
> >> >
> >> > If there are several cyclic desc in the work list, is there a better way
> >> > to find the "current" one?  The chan struct tracks the last completed and
> >> > last submitted cookies, but these will be the first and last
> >> > respectively as long as the cyclic transfer is active.  Is there an
> >> > "active" cookie stored somewhere that I missed?
> >>
> >> Assume there are three cookies. If you want to get the second cookie not
> >> latest cookie, your way can be also correct in such case?
> >> I think tx_status API is to get dma status of the given cookie.
> >> You are only considering a cyclic case.
> > For cyclic case you would have possible same descriptor running till you
> > terminate.
> 
> The cyclic case that makes this interesting is when there are multiple
> cyclic descriptors in the list. The cookie and completed_cookie
> markers don't help to determine which of the descriptors in the list
> is currently active.  dma_cookie_complete isn't called for a cyclic
> desc, the desc is just pushed to the end of the list.
Yes the cyclic is a very different case. I think driver can still return for
cyclic case which was txstate->last and that will give clue to client which is
getting processed now!

> > For non cyclic, if you have 3 descriptors submitted, the cookie value can be, say
> > 7, 8 and 9. If you query the status of any descriptor and pass the last optional
> > txstate arg then you will know the last cookie completed. so if txstate->last is
> > 7, then 7th was completed and 8 should be running and 9 in queue!
> 
> Got it, but the correct desc for cookie 8 will still have to be
> searched for, correct?
Yes

-- 
~Vinod

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Alban Browaeys]

Hi Dylan,

Le mardi 01 octobre 2013 à 21:33 -0700, Dylan Reid a écrit :
> On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com> wrote:
> > Hi Padmavathi,
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel [mailto:linux-arm-kernel-
> >> bounces@lists.infradead.org] On Behalf Of Padmavathi Venna
> >> Sent: Wednesday, September 11, 2013 3:08 PM
> >> To: linux-samsung-soc@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; padma.v@samsung.com; padma.kvr@gmail.com
> >> Cc: kgene.kim@samsung.com; arnd@arndb.de; sbkim73@samsung.com;
> >> vinod.koul@intel.com; broonie@kernel.org; dgreid@chromium.org;
> >> olofj@chromium.org
> >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
> >>
> >> From: Dylan Reid <dgreid@chromium.org>
> >>
> >> Fill txstate.residue with the amount of bytes remaining in the current
> >> transfer if the transfer is not complete.  This will be of particular use
> >> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> >>
> >> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> >> Reviewed-by: Olof Johansson <olofj@chromium.org>
> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> >> ---
> >>  drivers/dma/pl330.c |   55
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 files changed, 54 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> >> 593827b..7ab9136 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> >> dma_chan *chan)
> >>       spin_unlock_irqrestore(&pch->lock, flags);  }
> >>
> >> +static inline int
> >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
> >> +     return ((desc->px.src_addr <= sar) &&
> >> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
> >> +
> >> +static inline int
> >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
> >> +     return ((desc->px.dst_addr <= dar) &&
> >> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
> >> +
> >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> >> +     struct dma_pl330_chan *pch = to_pchan(chan);
> >> +     void __iomem *regs = pch->dmac->pif.base;
> >> +     struct pl330_thread *thrd = pch->pl330_chid;
> >> +     struct dma_pl330_desc *desc;
> >> +     unsigned int sar, dar;
> >> +     unsigned int residue = 0;
> >> +     unsigned long flags;
> >> +
> >> +     sar = readl(regs + SA(thrd->id));
> >> +     dar = readl(regs + DA(thrd->id));
> >> +
> >> +     spin_lock_irqsave(&pch->lock, flags);
> >> +
> >> +     /* Find the desc related to the current buffer. */
> >> +     list_for_each_entry(desc, &pch->work_list, node) {
> >> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> >> sar)) {
> >> +                     residue = desc->px.bytes - (sar -
> > desc->px.src_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> >> dar)) {
> >> +                     residue = desc->px.bytes - (dar -
> > desc->px.dst_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +     }
> >> +
> >> +found_unlock:
> >> +     spin_unlock_irqrestore(&pch->lock, flags);
> >> +
> >> +     return residue;
> >> +}
> >> +
> >>  static enum dma_status
> >>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> >>                struct dma_tx_state *txstate)
> >>  {
> >> -     return dma_cookie_status(chan, cookie, txstate);
> >> +     enum dma_status ret;
> >> +
> >> +     ret = dma_cookie_status(chan, cookie, txstate);
> >> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> >> +             dma_set_residue(txstate, pl330_tx_residue(chan));
> >> +
> >> +     return ret;
> >
> > Why didn't you use a cookie value to track the request?
> > The cookie is assigned when each transfer is submitted.
> > If you save the value in the desc, we can find the request easily.
> 
> If there are several cyclic desc in the work list, is there a better
> way to find the "current" one?  The chan struct tracks the last
> completed and last submitted cookies, but these will be the first and
> last respectively as long as the cyclic transfer is active.  Is there
> an "active" cookie stored somewhere that I missed?
> 
> Looking for the first buffer with status == BUSY is an improvement
> I'll make.  Any way to avoid looking through the list?
> 
> Thanks,
> 
> Dylan
> 
> >
> > Thanks,
> >
> > Best  Regards,
> > Chanho Park
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> !DSPAM:524ba462143697141112574!
> 


I phased out following hack to get asoc working (using a driver not yet
submitted but derived from hardkernel tree driver hkdk-max98090) with
dmaengine pcm. Highly likely it will only work for cyclic; it is aimed at
raising awareness of the current asoc dmaengine + pl330 breakage. I had to
include the transfer up to "used" to compute the residue to the end of
the buffer :

From 1199129e9a067e32f5aa4e9bc63f9527590b4c92 Mon Sep 17 00:00:00 2001
From: Dylan Reid <dgreid@chromium.org>
Date: Wed, 11 Sep 2013 11:38:03 +0530
Subject: [PATCH] dmaengine: pl330: Set residue in tx_status callback.

Fill txstate.residue with the amount of bytes remaining in the
transfers. This is required by alsa core dmaengine pcm pointer.

Based on patch from Dylan Reid <dgreid@chromium.org> but compute the
the residue in all transfers instead of the current one.

Signed-off-by: Alban Browaeys <prahal@yahoo.com>
---
 drivers/dma/pl330.c | 91
++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index ada7650..58df9ec 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2447,11 +2447,100 @@ static void pl330_free_chan_resources(struct
dma_chan *chan)
 	spin_unlock_irqrestore(&pch->lock, flags);
 }
 
+static inline int
+pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
+{
+	return (desc->px.src_addr <= sar) &&
+		(sar <= (desc->px.src_addr + desc->px.bytes));
+}
+
+static inline int
+pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
+{
+	return (desc->px.dst_addr <= dar) &&
+		(dar <= (desc->px.dst_addr + desc->px.bytes));
+}
+
+
 static enum dma_status
 pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 		 struct dma_tx_state *txstate)
 {
-	return dma_cookie_status(chan, cookie, txstate);
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	void __iomem *regs = pch->dmac->pif.base;
+	struct pl330_thread *thrd = pch->pl330_chid;
+	struct dma_pl330_desc *desc;
+	unsigned int sar, dar;
+	unsigned int residue = 0;
+	unsigned long flags;
+	bool first = true;
+	bool running = false;
+	dma_cookie_t last;
+	dma_cookie_t used;
+	enum dma_status ret;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	/*
+	 * There's no point calculating the residue if there's
+	 * no txstate to store the value.
+	 */
+	if (!txstate)
+		return ret;
+
+	spin_lock_irqsave(&pch->lock, flags);
+	ret = dma_cookie_status(chan, cookie, txstate);
+	last = txstate->last;
+	used = txstate->used;
+	sar = readl(regs + SA(thrd->id));
+	dar = readl(regs + DA(thrd->id));
+
+	if (ret == DMA_COMPLETE) {
+		spin_unlock_irqrestore(&pch->lock, flags);
+		return ret;
+	}
+
+	list_for_each_entry(desc, &pch->work_list, node) {
+		if (desc->status == BUSY) {
+			if (first) {
+				first = false;
+				running = true;
+			}
+
+			if (!running) {
+				residue += desc->px.bytes;
+				continue;
+			}
+
+			if (desc->rqcfg.src_inc
+			    && pl330_src_addr_in_desc(desc, sar)) {
+				residue += desc->px.bytes;
+				residue -= sar - desc->px.src_addr;
+			} else if (desc->rqcfg.dst_inc
+				   && pl330_dst_addr_in_desc(desc, dar)) {
+				residue += desc->px.bytes;
+				residue -= dar - desc->px.dst_addr;
+			} else
+				WARN_ON(1);
+
+			running = false;
+		} else if (desc->status == PREP)
+			residue += desc->px.bytes;
+
+		if (desc->txd.cookie == used)
+			break;
+	}
+	spin_unlock_irqrestore(&pch->lock, flags);
+
+	/*
+	 * This cookie not complete yet
+	 * Get number of bytes left in the active transactions and queue
+	 */
+	dma_set_residue(txstate, residue);
+
+	return ret;
 }
 
 static void pl330_issue_pending(struct dma_chan *chan)
-- 
1.8.5



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Alban Browaeys]

Hi Dylan,

Le mardi 01 octobre 2013 ? 21:33 -0700, Dylan Reid a ?crit :
> On Thu, Sep 12, 2013 at 4:40 AM, Chanho Park <chanho61.park@samsung.com> wrote:
> > Hi Padmavathi,
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel [mailto:linux-arm-kernel-
> >> bounces at lists.infradead.org] On Behalf Of Padmavathi Venna
> >> Sent: Wednesday, September 11, 2013 3:08 PM
> >> To: linux-samsung-soc at vger.kernel.org; linux-arm-
> >> kernel at lists.infradead.org; padma.v at samsung.com; padma.kvr at gmail.com
> >> Cc: kgene.kim at samsung.com; arnd at arndb.de; sbkim73 at samsung.com;
> >> vinod.koul at intel.com; broonie at kernel.org; dgreid at chromium.org;
> >> olofj at chromium.org
> >> Subject: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
> >>
> >> From: Dylan Reid <dgreid@chromium.org>
> >>
> >> Fill txstate.residue with the amount of bytes remaining in the current
> >> transfer if the transfer is not complete.  This will be of particular use
> >> to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> >>
> >> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> >> Reviewed-by: Olof Johansson <olofj@chromium.org>
> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> >> ---
> >>  drivers/dma/pl330.c |   55
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 files changed, 54 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index
> >> 593827b..7ab9136 100644
> >> --- a/drivers/dma/pl330.c
> >> +++ b/drivers/dma/pl330.c
> >> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct
> >> dma_chan *chan)
> >>       spin_unlock_irqrestore(&pch->lock, flags);  }
> >>
> >> +static inline int
> >> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar) {
> >> +     return ((desc->px.src_addr <= sar) &&
> >> +             (sar <= (desc->px.src_addr + desc->px.bytes))); }
> >> +
> >> +static inline int
> >> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar) {
> >> +     return ((desc->px.dst_addr <= dar) &&
> >> +             (dar <= (desc->px.dst_addr + desc->px.bytes))); }
> >> +
> >> +static unsigned int pl330_tx_residue(struct dma_chan *chan) {
> >> +     struct dma_pl330_chan *pch = to_pchan(chan);
> >> +     void __iomem *regs = pch->dmac->pif.base;
> >> +     struct pl330_thread *thrd = pch->pl330_chid;
> >> +     struct dma_pl330_desc *desc;
> >> +     unsigned int sar, dar;
> >> +     unsigned int residue = 0;
> >> +     unsigned long flags;
> >> +
> >> +     sar = readl(regs + SA(thrd->id));
> >> +     dar = readl(regs + DA(thrd->id));
> >> +
> >> +     spin_lock_irqsave(&pch->lock, flags);
> >> +
> >> +     /* Find the desc related to the current buffer. */
> >> +     list_for_each_entry(desc, &pch->work_list, node) {
> >> +             if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc,
> >> sar)) {
> >> +                     residue = desc->px.bytes - (sar -
> > desc->px.src_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +             if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc,
> >> dar)) {
> >> +                     residue = desc->px.bytes - (dar -
> > desc->px.dst_addr);
> >> +                     goto found_unlock;
> >> +             }
> >> +     }
> >> +
> >> +found_unlock:
> >> +     spin_unlock_irqrestore(&pch->lock, flags);
> >> +
> >> +     return residue;
> >> +}
> >> +
> >>  static enum dma_status
> >>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> >>                struct dma_tx_state *txstate)
> >>  {
> >> -     return dma_cookie_status(chan, cookie, txstate);
> >> +     enum dma_status ret;
> >> +
> >> +     ret = dma_cookie_status(chan, cookie, txstate);
> >> +     if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
> >> +             dma_set_residue(txstate, pl330_tx_residue(chan));
> >> +
> >> +     return ret;
> >
> > Why didn't you use a cookie value to track the request?
> > The cookie is assigned when each transfer is submitted.
> > If you save the value in the desc, we can find the request easily.
> 
> If there are several cyclic desc in the work list, is there a better
> way to find the "current" one?  The chan struct tracks the last
> completed and last submitted cookies, but these will be the first and
> last respectively as long as the cyclic transfer is active.  Is there
> an "active" cookie stored somewhere that I missed?
> 
> Looking for the first buffer with status == BUSY is an improvement
> I'll make.  Any way to avoid looking through the list?
> 
> Thanks,
> 
> Dylan
> 
> >
> > Thanks,
> >
> > Best  Regards,
> > Chanho Park
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> !DSPAM:524ba462143697141112574!
> 


I phased out following hack to get asoc working (using a driver not yet
submitted but derived from hardkernel tree driver hkdk-max98090) with
dmaengine pcm. Highly likely it will only work for cyclic; it is aimed at
raising awareness of the current asoc dmaengine + pl330 breakage. I had to
include the transfer up to "used" to compute the residue to the end of
the buffer :

>From 1199129e9a067e32f5aa4e9bc63f9527590b4c92 Mon Sep 17 00:00:00 2001
From: Dylan Reid <dgreid@chromium.org>
Date: Wed, 11 Sep 2013 11:38:03 +0530
Subject: [PATCH] dmaengine: pl330: Set residue in tx_status callback.

Fill txstate.residue with the amount of bytes remaining in the
transfers. This is required by alsa core dmaengine pcm pointer.

Based on patch from Dylan Reid <dgreid@chromium.org> but compute the
the residue in all transfers instead of the current one.

Signed-off-by: Alban Browaeys <prahal@yahoo.com>
---
 drivers/dma/pl330.c | 91
++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index ada7650..58df9ec 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2447,11 +2447,100 @@ static void pl330_free_chan_resources(struct
dma_chan *chan)
 	spin_unlock_irqrestore(&pch->lock, flags);
 }
 
+static inline int
+pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
+{
+	return (desc->px.src_addr <= sar) &&
+		(sar <= (desc->px.src_addr + desc->px.bytes));
+}
+
+static inline int
+pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
+{
+	return (desc->px.dst_addr <= dar) &&
+		(dar <= (desc->px.dst_addr + desc->px.bytes));
+}
+
+
 static enum dma_status
 pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 		 struct dma_tx_state *txstate)
 {
-	return dma_cookie_status(chan, cookie, txstate);
+	struct dma_pl330_chan *pch = to_pchan(chan);
+	void __iomem *regs = pch->dmac->pif.base;
+	struct pl330_thread *thrd = pch->pl330_chid;
+	struct dma_pl330_desc *desc;
+	unsigned int sar, dar;
+	unsigned int residue = 0;
+	unsigned long flags;
+	bool first = true;
+	bool running = false;
+	dma_cookie_t last;
+	dma_cookie_t used;
+	enum dma_status ret;
+
+	ret = dma_cookie_status(chan, cookie, txstate);
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	/*
+	 * There's no point calculating the residue if there's
+	 * no txstate to store the value.
+	 */
+	if (!txstate)
+		return ret;
+
+	spin_lock_irqsave(&pch->lock, flags);
+	ret = dma_cookie_status(chan, cookie, txstate);
+	last = txstate->last;
+	used = txstate->used;
+	sar = readl(regs + SA(thrd->id));
+	dar = readl(regs + DA(thrd->id));
+
+	if (ret == DMA_COMPLETE) {
+		spin_unlock_irqrestore(&pch->lock, flags);
+		return ret;
+	}
+
+	list_for_each_entry(desc, &pch->work_list, node) {
+		if (desc->status == BUSY) {
+			if (first) {
+				first = false;
+				running = true;
+			}
+
+			if (!running) {
+				residue += desc->px.bytes;
+				continue;
+			}
+
+			if (desc->rqcfg.src_inc
+			    && pl330_src_addr_in_desc(desc, sar)) {
+				residue += desc->px.bytes;
+				residue -= sar - desc->px.src_addr;
+			} else if (desc->rqcfg.dst_inc
+				   && pl330_dst_addr_in_desc(desc, dar)) {
+				residue += desc->px.bytes;
+				residue -= dar - desc->px.dst_addr;
+			} else
+				WARN_ON(1);
+
+			running = false;
+		} else if (desc->status == PREP)
+			residue += desc->px.bytes;
+
+		if (desc->txd.cookie == used)
+			break;
+	}
+	spin_unlock_irqrestore(&pch->lock, flags);
+
+	/*
+	 * This cookie not complete yet
+	 * Get number of bytes left in the active transactions and queue
+	 */
+	dma_set_residue(txstate, residue);
+
+	return ret;
 }
 
 static void pl330_issue_pending(struct dma_chan *chan)
-- 
1.8.5

Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Wed, Sep 11, 2013 at 11:38:03AM +0530, Padmavathi Venna wrote:
> From: Dylan Reid <dgreid@chromium.org>
> 
> Fill txstate.residue with the amount of bytes remaining in the current
> transfer if the transfer is not complete.  This will be of particular
> use to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> 
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  drivers/dma/pl330.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 593827b..7ab9136 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
>  	spin_unlock_irqrestore(&pch->lock, flags);
>  }
>  
> +static inline int
> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
> +{
> +	return ((desc->px.src_addr <= sar) &&
> +		(sar <= (desc->px.src_addr + desc->px.bytes)));
> +}
> +
> +static inline int
> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
> +{
> +	return ((desc->px.dst_addr <= dar) &&
> +		(dar <= (desc->px.dst_addr + desc->px.bytes)));
> +}
> +
> +static unsigned int pl330_tx_residue(struct dma_chan *chan)
> +{
> +	struct dma_pl330_chan *pch = to_pchan(chan);
> +	void __iomem *regs = pch->dmac->pif.base;
> +	struct pl330_thread *thrd = pch->pl330_chid;
> +	struct dma_pl330_desc *desc;
> +	unsigned int sar, dar;
> +	unsigned int residue = 0;
> +	unsigned long flags;
> +
> +	sar = readl(regs + SA(thrd->id));
> +	dar = readl(regs + DA(thrd->id));
> +
> +	spin_lock_irqsave(&pch->lock, flags);
> +
> +	/* Find the desc related to the current buffer. */
> +	list_for_each_entry(desc, &pch->work_list, node) {
> +		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, sar)) {
> +			residue = desc->px.bytes - (sar - desc->px.src_addr);
> +			goto found_unlock;
> +		}
> +		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, dar)) {
> +			residue = desc->px.bytes - (dar - desc->px.dst_addr);
> +			goto found_unlock;
> +		}
> +	}
> +
> +found_unlock:
> +	spin_unlock_irqrestore(&pch->lock, flags);
> +
> +	return residue;
> +}
> +
>  static enum dma_status
>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  		 struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(chan, cookie, txstate);
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
if will return DMA_IN_PROGRESS for two cases
a) when the descriptor is in queue
b) when the descriptor is submitted and running

You need to take different paths for these two cases. For former just return residue
as complete size of descriptor. For latter you cna read sar/dar and check
remaining bytes in sg_list.

Hint: use the cookie values to find the state

> +		dma_set_residue(txstate, pl330_tx_residue(chan));
> +
> +	return ret;
>  }
>  
>  static void pl330_issue_pending(struct dma_chan *chan)

~Vinod
-- 

[PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.

[Vinod Koul]

On Wed, Sep 11, 2013 at 11:38:03AM +0530, Padmavathi Venna wrote:
> From: Dylan Reid <dgreid@chromium.org>
> 
> Fill txstate.residue with the amount of bytes remaining in the current
> transfer if the transfer is not complete.  This will be of particular
> use to i2s DMA transfers, providing more accurate hw_ptr values to ASoC.
> 
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> Reviewed-by: Olof Johansson <olofj@chromium.org>
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  drivers/dma/pl330.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 593827b..7ab9136 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2476,11 +2476,64 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
>  	spin_unlock_irqrestore(&pch->lock, flags);
>  }
>  
> +static inline int
> +pl330_src_addr_in_desc(struct dma_pl330_desc *desc, unsigned int sar)
> +{
> +	return ((desc->px.src_addr <= sar) &&
> +		(sar <= (desc->px.src_addr + desc->px.bytes)));
> +}
> +
> +static inline int
> +pl330_dst_addr_in_desc(struct dma_pl330_desc *desc, unsigned int dar)
> +{
> +	return ((desc->px.dst_addr <= dar) &&
> +		(dar <= (desc->px.dst_addr + desc->px.bytes)));
> +}
> +
> +static unsigned int pl330_tx_residue(struct dma_chan *chan)
> +{
> +	struct dma_pl330_chan *pch = to_pchan(chan);
> +	void __iomem *regs = pch->dmac->pif.base;
> +	struct pl330_thread *thrd = pch->pl330_chid;
> +	struct dma_pl330_desc *desc;
> +	unsigned int sar, dar;
> +	unsigned int residue = 0;
> +	unsigned long flags;
> +
> +	sar = readl(regs + SA(thrd->id));
> +	dar = readl(regs + DA(thrd->id));
> +
> +	spin_lock_irqsave(&pch->lock, flags);
> +
> +	/* Find the desc related to the current buffer. */
> +	list_for_each_entry(desc, &pch->work_list, node) {
> +		if (desc->rqcfg.src_inc && pl330_src_addr_in_desc(desc, sar)) {
> +			residue = desc->px.bytes - (sar - desc->px.src_addr);
> +			goto found_unlock;
> +		}
> +		if (desc->rqcfg.dst_inc && pl330_dst_addr_in_desc(desc, dar)) {
> +			residue = desc->px.bytes - (dar - desc->px.dst_addr);
> +			goto found_unlock;
> +		}
> +	}
> +
> +found_unlock:
> +	spin_unlock_irqrestore(&pch->lock, flags);
> +
> +	return residue;
> +}
> +
>  static enum dma_status
>  pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  		 struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(chan, cookie, txstate);
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(chan, cookie, txstate);
> +	if (ret != DMA_SUCCESS) /* Not complete, check amount left. */
if will return DMA_IN_PROGRESS for two cases
a) when the descriptor is in queue
b) when the descriptor is submitted and running

You need to take different paths for these two cases. For former just return residue
as complete size of descriptor. For latter you cna read sar/dar and check
remaining bytes in sg_list.

Hint: use the cookie values to find the state

> +		dma_set_residue(txstate, pl330_tx_residue(chan));
> +
> +	return ret;
>  }
>  
>  static void pl330_issue_pending(struct dma_chan *chan)

~Vinod
-- 

[PATCH 2/3] ARM: SAMSUNG: Add residue DMA operation.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Add a new dma op, residue.  It returns the count of bytes left to be
transfered by the dma.  This is useful for Audio, the Samsung dma ASoC
layer will use this to provide an accurate update to the hw_ptr that is
exported to user space.  ASoC wants large transfers to reduce the amount
of wakeups, but needs to be able to know how much audio has been played
for latency estimates.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/plat-samsung/dma-ops.c              |   14 ++++++++++++++
 arch/arm/plat-samsung/include/plat/dma-ops.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c
index ec0d731..bfc6c1a 100644
--- a/arch/arm/plat-samsung/dma-ops.c
+++ b/arch/arm/plat-samsung/dma-ops.c
@@ -122,6 +122,19 @@ static inline int samsung_dmadev_flush(unsigned ch)
 	return dmaengine_terminate_all((struct dma_chan *)ch);
 }
 
+static unsigned samsung_dmadev_residue(unsigned ch)
+{
+	struct dma_chan *chan = (struct dma_chan *)ch;
+	enum dma_status ret;
+	struct dma_tx_state state;
+
+	ret = chan->device->device_tx_status(chan, chan->cookie, &state);
+	if (ret == DMA_SUCCESS) /* Transfer complete. */
+		return 0;
+
+	return state.residue;
+}
+
 static struct samsung_dma_ops dmadev_ops = {
 	.request	= samsung_dmadev_request,
 	.release	= samsung_dmadev_release,
@@ -131,6 +144,7 @@ static struct samsung_dma_ops dmadev_ops = {
 	.started	= NULL,
 	.flush		= samsung_dmadev_flush,
 	.stop		= samsung_dmadev_flush,
+	.residue	= samsung_dmadev_residue,
 };
 
 void *samsung_dmadev_get_ops(void)
diff --git a/arch/arm/plat-samsung/include/plat/dma-ops.h b/arch/arm/plat-samsung/include/plat/dma-ops.h
index ce6d763..82eac53 100644
--- a/arch/arm/plat-samsung/include/plat/dma-ops.h
+++ b/arch/arm/plat-samsung/include/plat/dma-ops.h
@@ -47,6 +47,7 @@ struct samsung_dma_ops {
 	int (*started)(unsigned ch);
 	int (*flush)(unsigned ch);
 	int (*stop)(unsigned ch);
+	unsigned (*residue)(unsigned ch);
 };
 
 extern void *samsung_dmadev_get_ops(void);
-- 
1.7.4.4

[PATCH 2/3] ARM: SAMSUNG: Add residue DMA operation.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Add a new dma op, residue.  It returns the count of bytes left to be
transfered by the dma.  This is useful for Audio, the Samsung dma ASoC
layer will use this to provide an accurate update to the hw_ptr that is
exported to user space.  ASoC wants large transfers to reduce the amount
of wakeups, but needs to be able to know how much audio has been played
for latency estimates.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/plat-samsung/dma-ops.c              |   14 ++++++++++++++
 arch/arm/plat-samsung/include/plat/dma-ops.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c
index ec0d731..bfc6c1a 100644
--- a/arch/arm/plat-samsung/dma-ops.c
+++ b/arch/arm/plat-samsung/dma-ops.c
@@ -122,6 +122,19 @@ static inline int samsung_dmadev_flush(unsigned ch)
 	return dmaengine_terminate_all((struct dma_chan *)ch);
 }
 
+static unsigned samsung_dmadev_residue(unsigned ch)
+{
+	struct dma_chan *chan = (struct dma_chan *)ch;
+	enum dma_status ret;
+	struct dma_tx_state state;
+
+	ret = chan->device->device_tx_status(chan, chan->cookie, &state);
+	if (ret == DMA_SUCCESS) /* Transfer complete. */
+		return 0;
+
+	return state.residue;
+}
+
 static struct samsung_dma_ops dmadev_ops = {
 	.request	= samsung_dmadev_request,
 	.release	= samsung_dmadev_release,
@@ -131,6 +144,7 @@ static struct samsung_dma_ops dmadev_ops = {
 	.started	= NULL,
 	.flush		= samsung_dmadev_flush,
 	.stop		= samsung_dmadev_flush,
+	.residue	= samsung_dmadev_residue,
 };
 
 void *samsung_dmadev_get_ops(void)
diff --git a/arch/arm/plat-samsung/include/plat/dma-ops.h b/arch/arm/plat-samsung/include/plat/dma-ops.h
index ce6d763..82eac53 100644
--- a/arch/arm/plat-samsung/include/plat/dma-ops.h
+++ b/arch/arm/plat-samsung/include/plat/dma-ops.h
@@ -47,6 +47,7 @@ struct samsung_dma_ops {
 	int (*started)(unsigned ch);
 	int (*flush)(unsigned ch);
 	int (*stop)(unsigned ch);
+	unsigned (*residue)(unsigned ch);
 };
 
 extern void *samsung_dmadev_get_ops(void);
-- 
1.7.4.4

[PATCH 3/3] ASoC: Update pointer to account for pending dma transfers.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Make dma_pointer check how much of the current dma transfer has
completed.  This improves the accuracy of the pointer operation, which
previously was only updated once a period.  Before this change calling
snd_pcm_avail() right before a dma transfer completed would indicate
that the entire transfer was still pending, now it will indicate the
actual count of free frames.

If the dma being used doesn't support the residue operation to query a
pending transfer, then assume that no bytes have been transfered.  This
leads to the same behavior as before the change.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 sound/soc/samsung/dma.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index 21b7926..6b2e55d 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -271,13 +271,24 @@ dma_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct runtime_data *prtd = runtime->private_data;
-	unsigned long res;
+	unsigned long offset;
+	unsigned long xfd; /* Number of bytes transfered by current dma. */
+	unsigned int ret = 0;
 
 	pr_debug("Entered %s\n", __func__);
 
-	res = prtd->dma_pos - prtd->dma_start;
+	/* If we can inspect how much of the transfer is left, use that for a
+	 * more accurate number.  Otherwise, assume no bytes have been
+	 * transfered.
+	 */
+	if (prtd->params->ops->residue)
+		ret = prtd->params->ops->residue(prtd->params->ch);
+
+	spin_lock(&prtd->lock);
+	xfd = prtd->dma_period - ret;
 
-	pr_debug("Pointer offset: %lu\n", res);
+	offset = prtd->dma_pos + xfd - prtd->dma_start;
+	spin_unlock(&prtd->lock);
 
 	/* we seem to be getting the odd error from the pcm library due
 	 * to out-of-bounds pointers. this is maybe due to the dma engine
@@ -285,12 +296,12 @@ dma_pointer(struct snd_pcm_substream *substream)
 	 * called... (todo - fix )
 	 */
 
-	if (res >= snd_pcm_lib_buffer_bytes(substream)) {
-		if (res == snd_pcm_lib_buffer_bytes(substream))
-			res = 0;
-	}
+	if (offset >= snd_pcm_lib_buffer_bytes(substream))
+		offset = 0;
+
+	pr_debug("Pointer offset: %lu\n", offset);
 
-	return bytes_to_frames(substream->runtime, res);
+	return bytes_to_frames(substream->runtime, offset);
 }
 
 static int dma_open(struct snd_pcm_substream *substream)
-- 
1.7.4.4

[PATCH 3/3] ASoC: Update pointer to account for pending dma transfers.

[Padmavathi Venna]

From: Dylan Reid <dgreid@chromium.org>

Make dma_pointer check how much of the current dma transfer has
completed.  This improves the accuracy of the pointer operation, which
previously was only updated once a period.  Before this change calling
snd_pcm_avail() right before a dma transfer completed would indicate
that the entire transfer was still pending, now it will indicate the
actual count of free frames.

If the dma being used doesn't support the residue operation to query a
pending transfer, then assume that no bytes have been transfered.  This
leads to the same behavior as before the change.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 sound/soc/samsung/dma.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index 21b7926..6b2e55d 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -271,13 +271,24 @@ dma_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct runtime_data *prtd = runtime->private_data;
-	unsigned long res;
+	unsigned long offset;
+	unsigned long xfd; /* Number of bytes transfered by current dma. */
+	unsigned int ret = 0;
 
 	pr_debug("Entered %s\n", __func__);
 
-	res = prtd->dma_pos - prtd->dma_start;
+	/* If we can inspect how much of the transfer is left, use that for a
+	 * more accurate number.  Otherwise, assume no bytes have been
+	 * transfered.
+	 */
+	if (prtd->params->ops->residue)
+		ret = prtd->params->ops->residue(prtd->params->ch);
+
+	spin_lock(&prtd->lock);
+	xfd = prtd->dma_period - ret;
 
-	pr_debug("Pointer offset: %lu\n", res);
+	offset = prtd->dma_pos + xfd - prtd->dma_start;
+	spin_unlock(&prtd->lock);
 
 	/* we seem to be getting the odd error from the pcm library due
 	 * to out-of-bounds pointers. this is maybe due to the dma engine
@@ -285,12 +296,12 @@ dma_pointer(struct snd_pcm_substream *substream)
 	 * called... (todo - fix )
 	 */
 
-	if (res >= snd_pcm_lib_buffer_bytes(substream)) {
-		if (res == snd_pcm_lib_buffer_bytes(substream))
-			res = 0;
-	}
+	if (offset >= snd_pcm_lib_buffer_bytes(substream))
+		offset = 0;
+
+	pr_debug("Pointer offset: %lu\n", offset);
 
-	return bytes_to_frames(substream->runtime, res);
+	return bytes_to_frames(substream->runtime, offset);
 }
 
 static int dma_open(struct snd_pcm_substream *substream)
-- 
1.7.4.4