All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add pl330 residue support
@ 2013-09-11  6:08 ` Padmavathi Venna
  0 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, padma.v, padma.kvr
  Cc: sbkim73, broonie, vinod.koul, kgene.kim, arnd, dgreid, olofj

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/3] Add pl330 residue support
@ 2013-09-11  6:08 ` Padmavathi Venna
  0 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-09-11  6:08 ` Padmavathi Venna
@ 2013-09-11  6:08   ` Padmavathi Venna
  -1 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, padma.v, padma.kvr
  Cc: sbkim73, broonie, vinod.koul, kgene.kim, arnd, dgreid, olofj

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-09-11  6:08   ` Padmavathi Venna
  0 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/3] ARM: SAMSUNG: Add residue DMA operation.
  2013-09-11  6:08 ` Padmavathi Venna
@ 2013-09-11  6:08   ` Padmavathi Venna
  -1 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, padma.v, padma.kvr
  Cc: sbkim73, broonie, vinod.koul, kgene.kim, arnd, dgreid, olofj

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/3] ARM: SAMSUNG: Add residue DMA operation.
@ 2013-09-11  6:08   ` Padmavathi Venna
  0 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/3] ASoC: Update pointer to account for pending dma transfers.
  2013-09-11  6:08 ` Padmavathi Venna
@ 2013-09-11  6:08   ` Padmavathi Venna
  -1 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, padma.v, padma.kvr
  Cc: sbkim73, broonie, vinod.koul, kgene.kim, arnd, dgreid, olofj

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/3] ASoC: Update pointer to account for pending dma transfers.
@ 2013-09-11  6:08   ` Padmavathi Venna
  0 siblings, 0 replies; 26+ messages in thread
From: Padmavathi Venna @ 2013-09-11  6:08 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* RE: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-09-11  6:08   ` Padmavathi Venna
@ 2013-09-12 11:40     ` Chanho Park
  -1 siblings, 0 replies; 26+ messages in thread
From: Chanho Park @ 2013-09-12 11:40 UTC (permalink / raw)
  To: 'Padmavathi Venna',
	linux-samsung-soc, linux-arm-kernel, padma.kvr
  Cc: kgene.kim, arnd, sbkim73, vinod.koul, broonie, dgreid, olofj

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-09-12 11:40     ` Chanho Park
  0 siblings, 0 replies; 26+ messages in thread
From: Chanho Park @ 2013-09-12 11:40 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-09-12 11:40     ` Chanho Park
@ 2013-09-13  3:03       ` Padma Venkat
  -1 siblings, 0 replies; 26+ messages in thread
From: Padma Venkat @ 2013-09-13  3:03 UTC (permalink / raw)
  To: Chanho Park
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	Kukjin Kim, Arnd Bergmann, Sangbeom Kim, Vinod Koul, broonie,
	dgreid, olofj

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-09-13  3:03       ` Padma Venkat
  0 siblings, 0 replies; 26+ messages in thread
From: Padma Venkat @ 2013-09-13  3:03 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-09-11  6:08   ` Padmavathi Venna
@ 2013-09-17  7:01     ` Vinod Koul
  -1 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-09-17  7:01 UTC (permalink / raw)
  To: Padmavathi Venna
  Cc: linux-samsung-soc, linux-arm-kernel, padma.kvr, sbkim73, broonie,
	kgene.kim, arnd, dgreid, olofj

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
-- 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-09-17  7:01     ` Vinod Koul
  0 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-09-17  7:01 UTC (permalink / raw)
  To: linux-arm-kernel

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
-- 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-09-12 11:40     ` Chanho Park
@ 2013-10-02  4:33       ` Dylan Reid
  -1 siblings, 0 replies; 26+ messages in thread
From: Dylan Reid @ 2013-10-02  4:33 UTC (permalink / raw)
  To: Chanho Park
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel, padma.kvr,
	kgene.kim, arnd, Sangbeom Kim, vinod.koul, Mark Brown,
	Olof Johansson

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-10-02  4:33       ` Dylan Reid
  0 siblings, 0 replies; 26+ messages in thread
From: Dylan Reid @ 2013-10-02  4:33 UTC (permalink / raw)
  To: linux-arm-kernel

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-10-02  4:33       ` Dylan Reid
@ 2013-10-07  1:39         ` Chanho Park
  -1 siblings, 0 replies; 26+ messages in thread
From: Chanho Park @ 2013-10-07  1:39 UTC (permalink / raw)
  To: 'Dylan Reid'
  Cc: 'Padmavathi Venna',
	linux-samsung-soc, linux-arm-kernel, padma.kvr, kgene.kim, arnd,
	'Sangbeom Kim', vinod.koul, 'Mark Brown',
	'Olof Johansson'

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
> >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-10-07  1:39         ` Chanho Park
  0 siblings, 0 replies; 26+ messages in thread
From: Chanho Park @ 2013-10-07  1:39 UTC (permalink / raw)
  To: linux-arm-kernel

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
> >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-10-07  1:39         ` Chanho Park
@ 2013-10-07  3:48           ` Vinod Koul
  -1 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-10-07  3:48 UTC (permalink / raw)
  To: Chanho Park, 'Dylan Reid'
  Cc: 'Padmavathi Venna',
	linux-samsung-soc, linux-arm-kernel, padma.kvr, kgene.kim, arnd,
	'Sangbeom Kim', 'Mark Brown',
	'Olof Johansson'

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-10-07  3:48           ` Vinod Koul
  0 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-10-07  3:48 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-10-07  3:48           ` Vinod Koul
@ 2013-10-09 20:37             ` Dylan Reid
  -1 siblings, 0 replies; 26+ messages in thread
From: Dylan Reid @ 2013-10-09 20:37 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Chanho Park, Padmavathi Venna, linux-samsung-soc,
	linux-arm-kernel, Padma Venkat, kgene.kim, Arnd Bergmann,
	Sangbeom Kim, Mark Brown, Olof Johansson

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-10-09 20:37             ` Dylan Reid
  0 siblings, 0 replies; 26+ messages in thread
From: Dylan Reid @ 2013-10-09 20:37 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-10-09 20:37             ` Dylan Reid
@ 2013-10-10 15:58               ` Vinod Koul
  -1 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-10-10 15:58 UTC (permalink / raw)
  To: Dylan Reid
  Cc: Chanho Park, Padmavathi Venna, linux-samsung-soc,
	linux-arm-kernel, Padma Venkat, kgene.kim, Arnd Bergmann,
	Sangbeom Kim, Mark Brown, Olof Johansson

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-10-10 15:58               ` Vinod Koul
  0 siblings, 0 replies; 26+ messages in thread
From: Vinod Koul @ 2013-10-10 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
  2013-10-02  4:33       ` Dylan Reid
@ 2013-12-03 23:56         ` Alban Browaeys
  -1 siblings, 0 replies; 26+ messages in thread
From: Alban Browaeys @ 2013-12-03 23:56 UTC (permalink / raw)
  To: Dylan Reid
  Cc: kgene.kim, arnd, Padmavathi Venna, Sangbeom Kim, vinod.koul,
	Mark Brown, linux-arm-kernel, padma.kvr, Chanho Park,
	Olof Johansson, linux-samsung-soc


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback.
@ 2013-12-03 23:56         ` Alban Browaeys
  0 siblings, 0 replies; 26+ messages in thread
From: Alban Browaeys @ 2013-12-03 23:56 UTC (permalink / raw)
  To: linux-arm-kernel


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2013-12-03 23:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-11  6:08 [PATCH 0/3] Add pl330 residue support Padmavathi Venna
2013-09-11  6:08 ` Padmavathi Venna
2013-09-11  6:08 ` [PATCH 1/3] dmaengine: pl330: Set residue in tx_status callback Padmavathi Venna
2013-09-11  6:08   ` Padmavathi Venna
2013-09-12 11:40   ` Chanho Park
2013-09-12 11:40     ` Chanho Park
2013-09-13  3:03     ` Padma Venkat
2013-09-13  3:03       ` Padma Venkat
2013-10-02  4:33     ` Dylan Reid
2013-10-02  4:33       ` Dylan Reid
2013-10-07  1:39       ` Chanho Park
2013-10-07  1:39         ` Chanho Park
2013-10-07  3:48         ` Vinod Koul
2013-10-07  3:48           ` Vinod Koul
2013-10-09 20:37           ` Dylan Reid
2013-10-09 20:37             ` Dylan Reid
2013-10-10 15:58             ` Vinod Koul
2013-10-10 15:58               ` Vinod Koul
2013-12-03 23:56       ` Alban Browaeys
2013-12-03 23:56         ` Alban Browaeys
2013-09-17  7:01   ` Vinod Koul
2013-09-17  7:01     ` Vinod Koul
2013-09-11  6:08 ` [PATCH 2/3] ARM: SAMSUNG: Add residue DMA operation Padmavathi Venna
2013-09-11  6:08   ` Padmavathi Venna
2013-09-11  6:08 ` [PATCH 3/3] ASoC: Update pointer to account for pending dma transfers Padmavathi Venna
2013-09-11  6:08   ` Padmavathi Venna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.