[PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Lars-Peter Clausen]

This patch adds a small inline wrapper for the devivce_tx_status callback of a
dma device. This makes the source code of users of this function a bit more
compact and a bit more legible.

E.g.:
-status = chan->device->device_tx_status(chan, cookie, &state)
+status = dmaengine_tx_status(chan, cookie, &state)

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

---
No changes since v1

---
 include/linux/dmaengine.h |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 56377df..cc0756a 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -670,6 +670,12 @@ static inline int dmaengine_resume(struct dma_chan *chan)
 	return dmaengine_device_control(chan, DMA_RESUME, 0);
 }
 
+static inline enum dma_status dmaengine_tx_status(struct dma_chan *chan,
+	dma_cookie_t cookie, struct dma_tx_state *state)
+{
+	return chan->device->device_tx_status(chan, cookie, state);
+}
+
 static inline dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)
 {
 	return desc->tx_submit(desc);
-- 
1.7.10

[PATCH v2 2/3] ASoC: dmaengine-pcm: Rename and deprecate snd_dmaengine_pcm_pointer

[Lars-Peter Clausen]

Currently the sound dmaengine pcm helper functions implement the pcm_pointer
callback by trying to count the number of elapsed periods. This is done by
advancing the stream position in the dmaengine callback by one period.
Unfortunately there is no guarantee that the callback will be called for each
elapsed period. It may be possible that under high system load it is only called
once for multiple elapsed periods. This patch renames the current implementation
and documents its shortcomings and that it should not be used anymore in new
drivers.

The next patch will introduce a new snd_dmaengine_pcm_pointer which will be
implemented based on querying the current stream position from the dma device.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

---
If you are maintaining a pcm driver which use the dmaengine pcm helper please
check if you platform works with the new snd_dmaengine_pcm_pointer
implementation which is added in the next patch (ux500 seems to be good
candidate). And if it does send a follow-up patch to convert your platform to
the new implementation. If it does not please try to fix or add residue
reporting support to your dmaengine driver.

This patch is new in v2 of this series

---
 include/sound/dmaengine_pcm.h |    2 +-
 sound/soc/ep93xx/ep93xx-pcm.c |    2 +-
 sound/soc/fsl/imx-pcm-dma.c   |    2 +-
 sound/soc/mxs/mxs-pcm.c       |    2 +-
 sound/soc/soc-dmaengine-pcm.c |   10 +++++-----
 sound/soc/ux500/ux500_pcm.c   |    2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index a8fcaa6..ea57915 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -38,7 +38,7 @@ void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream);
 int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream,
 	const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config);
 int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd);
-snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream);
+snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream);
 
 int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 	dma_filter_fn filter_fn, void *filter_data);
diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
index 162dbb7..4eea98b 100644
--- a/sound/soc/ep93xx/ep93xx-pcm.c
+++ b/sound/soc/ep93xx/ep93xx-pcm.c
@@ -136,7 +136,7 @@ static struct snd_pcm_ops ep93xx_pcm_ops = {
 	.hw_params	= ep93xx_pcm_hw_params,
 	.hw_free	= ep93xx_pcm_hw_free,
 	.trigger	= snd_dmaengine_pcm_trigger,
-	.pointer	= snd_dmaengine_pcm_pointer,
+	.pointer	= snd_dmaengine_pcm_pointer_no_residue,
 	.mmap		= ep93xx_pcm_mmap,
 };
 
diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
index f3c0a5e..48f9d88 100644
--- a/sound/soc/fsl/imx-pcm-dma.c
+++ b/sound/soc/fsl/imx-pcm-dma.c
@@ -141,7 +141,7 @@ static struct snd_pcm_ops imx_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_imx_pcm_hw_params,
 	.trigger	= snd_dmaengine_pcm_trigger,
-	.pointer	= snd_dmaengine_pcm_pointer,
+	.pointer	= snd_dmaengine_pcm_pointer_no_residue,
 	.mmap		= snd_imx_pcm_mmap,
 };
 
diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c
index 373dec9..f82d766 100644
--- a/sound/soc/mxs/mxs-pcm.c
+++ b/sound/soc/mxs/mxs-pcm.c
@@ -141,7 +141,7 @@ static struct snd_pcm_ops mxs_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_mxs_pcm_hw_params,
 	.trigger	= snd_dmaengine_pcm_trigger,
-	.pointer	= snd_dmaengine_pcm_pointer,
+	.pointer	= snd_dmaengine_pcm_pointer_no_residue,
 	.mmap		= snd_mxs_pcm_mmap,
 };
 
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 4756952..7c0877e 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -200,18 +200,18 @@ int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger);
 
 /**
- * snd_dmaengine_pcm_pointer - dmaengine based PCM pointer implementation
+ * snd_dmaengine_pcm_pointer_no_residue - dmaengine based PCM pointer implementation
  * @substream: PCM substream
  *
- * This function can be used as the PCM pointer callback for dmaengine based PCM
- * driver implementations.
+ * This function is deprecated and should not be used by new drivers, as its
+ * results may be unreliable.
  */
-snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream)
+snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream)
 {
 	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
 	return bytes_to_frames(substream->runtime, prtd->pos);
 }
-EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer);
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue);
 
 static int dmaengine_pcm_request_channel(struct dmaengine_pcm_runtime_data *prtd,
 	dma_filter_fn filter_fn, void *filter_data)
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index 66b080e..f8bb9d3 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -261,7 +261,7 @@ static struct snd_pcm_ops ux500_pcm_ops = {
 	.hw_params	= ux500_pcm_hw_params,
 	.hw_free	= ux500_pcm_hw_free,
 	.trigger	= snd_dmaengine_pcm_trigger,
-	.pointer	= snd_dmaengine_pcm_pointer,
+	.pointer	= snd_dmaengine_pcm_pointer_no_residue,
 	.mmap		= ux500_pcm_mmap
 };
 
-- 
1.7.10

[PATCH v2 3/3] ASoC: dmaengine-pcm: Add support for querying stream position from DMA driver

[Lars-Peter Clausen]

Currently the sound dmaengine pcm helper functions implement the pcm_pointer
callback by trying to count the number of elapsed periods. This is done by
advancing the stream position in the dmaengine callback by one period.
Unfortunately there is no guarantee that the callback will be called for each
elapsed period. It may be possible that under high system load it is only called
once for multiple elapsed periods. This patch addresses the issue by
implementing support for querying the current stream position directly from the
dmaengine driver. Since not all dmaengine drivers support reporting the stream
position yet the old period counting implementation is kept for now.

Furthermore the new mechanism allows to report the stream position with a
sub-period granularity, given that the dmaengine driver supports this.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

---
Changes since v1:
	* Don't implement fallback support in snd_dmaengine_pcm_pointer, instead it
	  is now moved to its own function. This was done to make it more explicit
	  that implementing residue reporting for cyclic is required and that this
	  won't be optional for new drivers.
---
 include/sound/dmaengine_pcm.h |    1 +
 sound/soc/soc-dmaengine-pcm.c |   29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index ea57915..b877334 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -38,6 +38,7 @@ void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream);
 int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream,
 	const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config);
 int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd);
+snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream);
 snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream);
 
 int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 7c0877e..2995334 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -30,6 +30,7 @@
 
 struct dmaengine_pcm_runtime_data {
 	struct dma_chan *dma_chan;
+	dma_cookie_t cookie;
 
 	unsigned int pos;
 
@@ -153,7 +154,7 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
 
 	desc->callback = dmaengine_pcm_dma_complete;
 	desc->callback_param = substream;
-	dmaengine_submit(desc);
+	prtd->cookie = dmaengine_submit(desc);
 
 	return 0;
 }
@@ -213,6 +214,32 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue);
 
+/**
+ * snd_dmaengine_pcm_pointer - dmaengine based PCM pointer implementation
+ * @substream: PCM substream
+ *
+ * This function can be used as the PCM pointer callback for dmaengine based PCM
+ * driver implementations.
+ */
+snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+	struct dma_tx_state state;
+	enum dma_status status;
+	unsigned int buf_size;
+	unsigned int pos = 0;
+
+	status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state);
+	if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) {
+		buf_size = snd_pcm_lib_buffer_bytes(substream);
+		if (state.residue > 0 && state.residue <= buf_size)
+			pos = buf_size - state.residue;
+	}
+
+	return bytes_to_frames(substream->runtime, pos);
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer);
+
 static int dmaengine_pcm_request_channel(struct dmaengine_pcm_runtime_data *prtd,
 	dma_filter_fn filter_fn, void *filter_data)
 {
-- 
1.7.10

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Vinod Koul]

On Mon, 2012-06-11 at 20:11 +0200, Lars-Peter Clausen wrote:
> This patch adds a small inline wrapper for the devivce_tx_status callback of a
> dma device. This makes the source code of users of this function a bit more
> compact and a bit more legible.
Applied, thanks

rest of the series should go thru ASoC tree.

-- 
~Vinod

Re: [PATCH v2 2/3] ASoC: dmaengine-pcm: Rename and deprecate snd_dmaengine_pcm_pointer

[Vinod Koul]

On Mon, 2012-06-11 at 20:11 +0200, Lars-Peter Clausen wrote:
> Currently the sound dmaengine pcm helper functions implement the pcm_pointer
> callback by trying to count the number of elapsed periods. This is done by
> advancing the stream position in the dmaengine callback by one period.
> Unfortunately there is no guarantee that the callback will be called for each
> elapsed period. It may be possible that under high system load it is only called
> once for multiple elapsed periods. This patch renames the current implementation
> and documents its shortcomings and that it should not be used anymore in new
> drivers.
> 
> The next patch will introduce a new snd_dmaengine_pcm_pointer which will be
> implemented based on querying the current stream position from the dma device.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

Re: [PATCH v2 3/3] ASoC: dmaengine-pcm: Add support for querying stream position from DMA driver

[Vinod Koul]

On Mon, 2012-06-11 at 20:11 +0200, Lars-Peter Clausen wrote:
> Currently the sound dmaengine pcm helper functions implement the pcm_pointer
> callback by trying to count the number of elapsed periods. This is done by
> advancing the stream position in the dmaengine callback by one period.
> Unfortunately there is no guarantee that the callback will be called for each
> elapsed period. It may be possible that under high system load it is only called
> once for multiple elapsed periods. This patch addresses the issue by
> implementing support for querying the current stream position directly from the
> dmaengine driver. Since not all dmaengine drivers support reporting the stream
> position yet the old period counting implementation is kept for now.
> 
> Furthermore the new mechanism allows to report the stream position with a
> sub-period granularity, given that the dmaengine driver supports this.
> 
Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Lars-Peter Clausen]

On 06/20/2012 12:54 PM, Vinod Koul wrote:
> On Mon, 2012-06-11 at 20:11 +0200, Lars-Peter Clausen wrote:
>> This patch adds a small inline wrapper for the devivce_tx_status callback of a
>> dma device. This makes the source code of users of this function a bit more
>> compact and a bit more legible.
> Applied, thanks
> 
> rest of the series should go thru ASoC tree.
> 

Thanks.

There is a compile time dependency between the patches, so this patch should
go through the ASoC tree as well, or we'll have to wait another release
before the ASoC patches can be applied.

- Lars

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On Wed, Jun 20, 2012 at 02:13:45PM +0200, Lars-Peter Clausen wrote:

> There is a compile time dependency between the patches, so this patch should
> go through the ASoC tree as well, or we'll have to wait another release
> before the ASoC patches can be applied.

Yes, I was just going to say that - are you OK with me applying these
Vinod?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Vinod Koul]

On Wed, 2012-06-20 at 14:13 +0200, Lars-Peter Clausen wrote:
> On 06/20/2012 12:54 PM, Vinod Koul wrote:
> > On Mon, 2012-06-11 at 20:11 +0200, Lars-Peter Clausen wrote:
> >> This patch adds a small inline wrapper for the devivce_tx_status callback of a
> >> dma device. This makes the source code of users of this function a bit more
> >> compact and a bit more legible.
> > Applied, thanks
> > 
> > rest of the series should go thru ASoC tree.
> > 
> 
> Thanks.
> 
> There is a compile time dependency between the patches, so this patch should
> go through the ASoC tree as well, or we'll have to wait another release
> before the ASoC patches can be applied.
> 
Yes and rest of the two patches depend on components in asoc-next so
they don't apply for me as well.
Mark, it would make sense to take all three to your tree. I will retain
this in my next for others to use.
So, 
Acked-by Vinod Koul <vinod.koul@linux.intel.com>

-- 
~Vinod

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Vinod Koul]

On Wed, 2012-06-20 at 14:09 +0100, Mark Brown wrote:
> On Wed, Jun 20, 2012 at 02:13:45PM +0200, Lars-Peter Clausen wrote:
> 
> > There is a compile time dependency between the patches, so this patch should
> > go through the ASoC tree as well, or we'll have to wait another release
> > before the ASoC patches can be applied.
> 
> Yes, I was just going to say that - are you OK with me applying these
> Vinod?

Ah spoke too soon.

Please go ahead :)

-- 
~Vinod

Re: [alsa-devel] [PATCH v2 2/3] ASoC: dmaengine-pcm: Rename and deprecate snd_dmaengine_pcm_pointer

[Dong Aisheng]

On Mon, Jun 11, 2012 at 08:11:41PM +0200, Lars-Peter Clausen wrote:
> Currently the sound dmaengine pcm helper functions implement the pcm_pointer
> callback by trying to count the number of elapsed periods. This is done by
> advancing the stream position in the dmaengine callback by one period.
> Unfortunately there is no guarantee that the callback will be called for each
> elapsed period. It may be possible that under high system load it is only called
> once for multiple elapsed periods. This patch renames the current implementation
> and documents its shortcomings and that it should not be used anymore in new
> drivers.
> 
> The next patch will introduce a new snd_dmaengine_pcm_pointer which will be
> implemented based on querying the current stream position from the dma device.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
Acked-by: Dong Aisheng <dong.aisheng@linaro.org>

> ---
> If you are maintaining a pcm driver which use the dmaengine pcm helper please
> check if you platform works with the new snd_dmaengine_pcm_pointer
> implementation which is added in the next patch (ux500 seems to be good
> candidate). And if it does send a follow-up patch to convert your platform to
> the new implementation. If it does not please try to fix or add residue
> reporting support to your dmaengine driver.
> 
Will try it after adding cyclic tx_status support for mxs/imx dma driver.

Regards
Dong Aisheng

Re: [alsa-devel] [PATCH v2 3/3] ASoC: dmaengine-pcm: Add support for querying stream position from DMA driver

[Dong Aisheng]

On Mon, Jun 11, 2012 at 08:11:42PM +0200, Lars-Peter Clausen wrote:
> Currently the sound dmaengine pcm helper functions implement the pcm_pointer
> callback by trying to count the number of elapsed periods. This is done by
> advancing the stream position in the dmaengine callback by one period.
> Unfortunately there is no guarantee that the callback will be called for each
> elapsed period. It may be possible that under high system load it is only called
> once for multiple elapsed periods. This patch addresses the issue by
> implementing support for querying the current stream position directly from the
> dmaengine driver. Since not all dmaengine drivers support reporting the stream
> position yet the old period counting implementation is kept for now.
> 
> Furthermore the new mechanism allows to report the stream position with a
> sub-period granularity, given that the dmaengine driver supports this.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> ---
> Changes since v1:
> 	* Don't implement fallback support in snd_dmaengine_pcm_pointer, instead it
> 	  is now moved to its own function. This was done to make it more explicit
> 	  that implementing residue reporting for cyclic is required and that this
> 	  won't be optional for new drivers.
> ---
>  include/sound/dmaengine_pcm.h |    1 +
>  sound/soc/soc-dmaengine-pcm.c |   29 ++++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index ea57915..b877334 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -38,6 +38,7 @@ void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream);
>  int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream,
>  	const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config);
>  int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd);
> +snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream);
>  snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream);
>  
>  int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 7c0877e..2995334 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -30,6 +30,7 @@
>  
>  struct dmaengine_pcm_runtime_data {
>  	struct dma_chan *dma_chan;
> +	dma_cookie_t cookie;
>  
>  	unsigned int pos;
>  
> @@ -153,7 +154,7 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
>  
>  	desc->callback = dmaengine_pcm_dma_complete;
>  	desc->callback_param = substream;
> -	dmaengine_submit(desc);
> +	prtd->cookie = dmaengine_submit(desc);
>  
>  	return 0;
>  }
> @@ -213,6 +214,32 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>  }
>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue);
>  
> +/**
> + * snd_dmaengine_pcm_pointer - dmaengine based PCM pointer implementation
> + * @substream: PCM substream
> + *
> + * This function can be used as the PCM pointer callback for dmaengine based PCM
> + * driver implementations.
> + */
> +snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +	struct dma_tx_state state;
> +	enum dma_status status;
> +	unsigned int buf_size;
> +	unsigned int pos = 0;
> +
> +	status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state);
> +	if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) {
> +		buf_size = snd_pcm_lib_buffer_bytes(substream);
> +		if (state.residue > 0 && state.residue <= buf_size)
> +			pos = buf_size - state.residue;
One question i still not very clear, for the case when one buffer size of cyclic dma
transfer completes, the dma residue may be buf_size( or 0 as Russell said), then
pos here is 0. So is it correct that call bytes_to_frames(substream->runtime, 0)
for this case?

Regards
Dong Aisheng

Re: [alsa-devel] [PATCH v2 3/3] ASoC: dmaengine-pcm: Add support for querying stream position from DMA driver

[Lars-Peter Clausen]

On 06/20/2012 03:48 PM, Dong Aisheng wrote:
> On Mon, Jun 11, 2012 at 08:11:42PM +0200, Lars-Peter Clausen wrote:
>> Currently the sound dmaengine pcm helper functions implement the pcm_pointer
>> callback by trying to count the number of elapsed periods. This is done by
>> advancing the stream position in the dmaengine callback by one period.
>> Unfortunately there is no guarantee that the callback will be called for each
>> elapsed period. It may be possible that under high system load it is only called
>> once for multiple elapsed periods. This patch addresses the issue by
>> implementing support for querying the current stream position directly from the
>> dmaengine driver. Since not all dmaengine drivers support reporting the stream
>> position yet the old period counting implementation is kept for now.
>>
>> Furthermore the new mechanism allows to report the stream position with a
>> sub-period granularity, given that the dmaengine driver supports this.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>
>> ---
>> Changes since v1:
>> 	* Don't implement fallback support in snd_dmaengine_pcm_pointer, instead it
>> 	  is now moved to its own function. This was done to make it more explicit
>> 	  that implementing residue reporting for cyclic is required and that this
>> 	  won't be optional for new drivers.
>> ---
>>  include/sound/dmaengine_pcm.h |    1 +
>>  sound/soc/soc-dmaengine-pcm.c |   29 ++++++++++++++++++++++++++++-
>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
>> index ea57915..b877334 100644
>> --- a/include/sound/dmaengine_pcm.h
>> +++ b/include/sound/dmaengine_pcm.h
>> @@ -38,6 +38,7 @@ void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream);
>>  int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream,
>>  	const struct snd_pcm_hw_params *params, struct dma_slave_config *slave_config);
>>  int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd);
>> +snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream);
>>  snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream *substream);
>>  
>>  int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
>> index 7c0877e..2995334 100644
>> --- a/sound/soc/soc-dmaengine-pcm.c
>> +++ b/sound/soc/soc-dmaengine-pcm.c
>> @@ -30,6 +30,7 @@
>>  
>>  struct dmaengine_pcm_runtime_data {
>>  	struct dma_chan *dma_chan;
>> +	dma_cookie_t cookie;
>>  
>>  	unsigned int pos;
>>  
>> @@ -153,7 +154,7 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
>>  
>>  	desc->callback = dmaengine_pcm_dma_complete;
>>  	desc->callback_param = substream;
>> -	dmaengine_submit(desc);
>> +	prtd->cookie = dmaengine_submit(desc);
>>  
>>  	return 0;
>>  }
>> @@ -213,6 +214,32 @@ snd_pcm_uframes_t snd_dmaengine_pcm_pointer_no_residue(struct snd_pcm_substream
>>  }
>>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_pointer_no_residue);
>>  
>> +/**
>> + * snd_dmaengine_pcm_pointer - dmaengine based PCM pointer implementation
>> + * @substream: PCM substream
>> + *
>> + * This function can be used as the PCM pointer callback for dmaengine based PCM
>> + * driver implementations.
>> + */
>> +snd_pcm_uframes_t snd_dmaengine_pcm_pointer(struct snd_pcm_substream *substream)
>> +{
>> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
>> +	struct dma_tx_state state;
>> +	enum dma_status status;
>> +	unsigned int buf_size;
>> +	unsigned int pos = 0;
>> +
>> +	status = dmaengine_tx_status(prtd->dma_chan, prtd->cookie, &state);
>> +	if (status == DMA_IN_PROGRESS || status == DMA_PAUSED) {
>> +		buf_size = snd_pcm_lib_buffer_bytes(substream);
>> +		if (state.residue > 0 && state.residue <= buf_size)
>> +			pos = buf_size - state.residue;
> One question i still not very clear, for the case when one buffer size of cyclic dma
> transfer completes, the dma residue may be buf_size( or 0 as Russell said), then
> pos here is 0. So is it correct that call bytes_to_frames(substream->runtime, 0)
> for this case?
> 

Yes, a residue of 0 and of buf_size are equivalent. And the fact that
residue may be 0 for cyclic DMA is just leakage through our abstraction
layers when cyclic DMA is emulated using "normal" DMA. For real cyclic DMA
residue should never be zero.

The value which is returned by the pcm_pointer callback needs to less than
buffer_size, otherwise the ALSA core code will report an error. So if
residue is 0 and we'd not do any special handling for this case we'd return
buffer_size here.

- Lars

Re: [PATCH v2 1/3] dmaengine: Add wrapper for device_tx_status callback

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

On Mon, Jun 11, 2012 at 08:11:40PM +0200, Lars-Peter Clausen wrote:
> This patch adds a small inline wrapper for the devivce_tx_status callback of a
> dma device. This makes the source code of users of this function a bit more
> compact and a bit more legible.

Applied all, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]