[RFC 0/7] ASoC: Introduce dmaengine pcm helper functions

[RFC 0/7] ASoC: Introduce dmaengine pcm helper functions

[Lars-Peter Clausen]

Same series as send out yesterday (sans the cleanup patches which already have
been applied), since I forgot to Cc alsa-devel. Sorry for the noice if you got
the series twice.


This patch series introduces a new set of helper functions for dmaengine based
PCM drivers. Currently we have 6 different dmaengine based PCM drivers which are
all more or less similar in structure, but all have also individual requirements
which can't easily be generalized. So that's why this series does not introduce
a generic dmaengine PCM driver but rather a set of helper functions which can be
used to implement the common bits between such dmaengine based PCM drivers.

The helper functions only provide infrastructure code for dmaengines which
provide thed evice_prep_dma_cyclic. While it is possible to add support for
dmaengine, which only provide device_prep_slave_sg, it is in my opinion a better
idea to either implement the missing functionality in the dmaengine driver or
provide a emulation layer for cyclic transfer within the dmaengine framework
itself. This has the advantage that it is also available outside of ALSA.

This series converts the three PCM drivers which already use the circular
dmaengine prepare function. Since I don't have access to hardware using the
converted drivers the patches have only been compile tested. But the helper
functions introduced in this series have been runtime tested with a platform
which is not upstream yet.

The first two patches restructure the iMX and MXS drivers to request their DMA
channel in the PCM open callback instead of the hwparams callback.  This is done
to make their structure more similar to what it will be when using the helper
functions and make the actual patch converting them to the new dmaengine PCM
helper functions less intrusive. The next patch introduces the helper functions
and the last three patches respectively convert the imx, mxs and ep39xx
dmaengine based PCM drivers to use the new set of helper functions.

The set of helper functions consists of a open, a close, a trigger and a pointer
callback and also a function to convert hwparams to a dma_slave_config. The
trigger and pointer callbacks can usually be used directly for the pcm_ops
callbacks. The open callback wants to be called from a driver specific open
function which may do some driver specific setup tasks and provide the helper
open callback with a dma filter function. If the driver specific open functions
allocates resources they need to be freed in a driver specific close function
which has to call the helper close callback. If not the helper close callback
can be assigned directly to the pcm_ops struct.

If a driver needs to keep additional driver specific data around it can be done
by using snd_dmaengine_pcm_{get,set}_data. Normally a driver should not require
to keep additional data around, but all of the converted drivers need this,
because of this horrible abusive pattern where the dma channels private field
gets assigned configuration data in the filter callback. We should hopefully be
able to get rid of this pattern with Guennadi Liakhovetski's patches[1], which
add a slave configuration parameter to dma_request_channel, and with it the need
for storing extra private data.

While the DMA helper functions only use generic ALSA functions and nothing ASoC
specific I've placed the, under ASoC for now since the only users are ASoC
driver.

- Lars

[1] https://lkml.org/lkml/2012/2/1/227

Lars-Peter Clausen (7):
  ASoC: imx-ssi: Set dma data early
  ASoC: imx-pcm: Request DMA channel early
  ASoC: mxs-pcm: Request DMA channel early
  ASoC: Add dmaengine PCM helper functions
  ASoC: imx-pcm-dma: Use dmaengine PCM helper functions
  ASoC: mxs-pcm: Use dmaengine PCM helper functions
  ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

 include/sound/dmaengine_pcm.h   |   49 +++++++
 sound/soc/Kconfig               |    3 +
 sound/soc/Makefile              |    3 +
 sound/soc/ep93xx/Kconfig        |    1 +
 sound/soc/ep93xx/ep93xx-pcm.c   |  148 +++-----------------
 sound/soc/imx/Kconfig           |    1 +
 sound/soc/imx/imx-pcm-dma-mx2.c |  196 ++++----------------------
 sound/soc/imx/imx-ssi.c         |   28 +++-
 sound/soc/mxs/Kconfig           |    1 +
 sound/soc/mxs/mxs-pcm.c         |  152 ++++-----------------
 sound/soc/mxs/mxs-pcm.h         |   12 --
 sound/soc/soc-dmaengine-pcm.c   |  287 +++++++++++++++++++++++++++++++++++++++
 12 files changed, 443 insertions(+), 438 deletions(-)
 create mode 100644 include/sound/dmaengine_pcm.h
 create mode 100644 sound/soc/soc-dmaengine-pcm.c

-- 
1.7.9

[RFC 1/7] ASoC: imx-ssi: Set dma data early

[Lars-Peter Clausen]

Move the call to snd_soc_dai_set_dma_data from the hw_params callback to the
startup callback. This allows us to use the dma data in the pcm driver's open
callback.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/imx-ssi.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/sound/soc/imx/imx-ssi.c b/sound/soc/imx/imx-ssi.c
index 01d1f74..dbf43f5 100644
--- a/sound/soc/imx/imx-ssi.c
+++ b/sound/soc/imx/imx-ssi.c
@@ -233,6 +233,23 @@ static int imx_ssi_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
 	return 0;
 }
 
+static int imx_ssi_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *cpu_dai)
+{
+	struct imx_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
+	struct imx_pcm_dma_params *dma_data;
+
+	/* Tx/Rx config */
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		dma_data = &ssi->dma_params_tx;
+	else
+		dma_data = &ssi->dma_params_rx;
+
+	snd_soc_dai_set_dma_data(cpu_dai, substream, dma_data);
+
+	return 0;
+}
+
 /*
  * Should only be called when port is inactive (i.e. SSIEN = 0),
  * although can be called multiple times by upper layers.
@@ -242,23 +259,17 @@ static int imx_ssi_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *cpu_dai)
 {
 	struct imx_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
-	struct imx_pcm_dma_params *dma_data;
 	u32 reg, sccr;
 
 	/* Tx/Rx config */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		reg = SSI_STCCR;
-		dma_data = &ssi->dma_params_tx;
-	} else {
+	else
 		reg = SSI_SRCCR;
-		dma_data = &ssi->dma_params_rx;
-	}
 
 	if (ssi->flags & IMX_SSI_SYN)
 		reg = SSI_STCCR;
 
-	snd_soc_dai_set_dma_data(cpu_dai, substream, dma_data);
-
 	sccr = readl(ssi->base + reg) & ~SSI_STCCR_WL_MASK;
 
 	/* DAI data (word) size */
@@ -343,6 +354,7 @@ static int imx_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 }
 
 static const struct snd_soc_dai_ops imx_ssi_pcm_dai_ops = {
+	.startup	= imx_ssi_startup,
 	.hw_params	= imx_ssi_hw_params,
 	.set_fmt	= imx_ssi_set_dai_fmt,
 	.set_clkdiv	= imx_ssi_set_dai_clkdiv,
-- 
1.7.9

[RFC 2/7] ASoC: imx-pcm: Request DMA channel early

[Lars-Peter Clausen]

Request the DMA channel in the pcm open callback. This allows us to let open
fail if there is no dma channel available.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/imx-pcm-dma-mx2.c |   78 ++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 46 deletions(-)

diff --git a/sound/soc/imx/imx-pcm-dma-mx2.c b/sound/soc/imx/imx-pcm-dma-mx2.c
index ec13944..f974e61 100644
--- a/sound/soc/imx/imx-pcm-dma-mx2.c
+++ b/sound/soc/imx/imx-pcm-dma-mx2.c
@@ -65,17 +65,13 @@ static bool filter(struct dma_chan *chan, void *param)
         return true;
 }
 
-static int imx_ssi_dma_alloc(struct snd_pcm_substream *substream,
-				struct snd_pcm_hw_params *params)
+static int imx_ssi_dma_alloc(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct imx_pcm_dma_params *dma_params;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-	struct dma_slave_config slave_config;
 	dma_cap_mask_t mask;
-	enum dma_slave_buswidth buswidth;
-	int ret;
 
 	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
@@ -84,13 +80,29 @@ static int imx_ssi_dma_alloc(struct snd_pcm_substream *substream,
 	iprtd->dma_data.dma_request = dma_params->dma;
 
 	/* Try to grab a DMA channel */
-	if (!iprtd->dma_chan) {
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-		iprtd->dma_chan = dma_request_channel(mask, filter, iprtd);
-		if (!iprtd->dma_chan)
-			return -EINVAL;
-	}
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	iprtd->dma_chan = dma_request_channel(mask, filter, iprtd);
+	if (!iprtd->dma_chan)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int snd_imx_pcm_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
+	struct dma_chan *chan = iprtd->dma_chan;
+	struct imx_pcm_dma_params *dma_params;
+	struct dma_slave_config slave_config;
+	enum dma_slave_buswidth buswidth;
+	unsigned long dma_addr;
+	int ret;
+
+	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S16_LE:
@@ -116,29 +128,10 @@ static int imx_ssi_dma_alloc(struct snd_pcm_substream *substream,
 		slave_config.src_maxburst = dma_params->burstsize;
 	}
 
-	ret = dmaengine_slave_config(iprtd->dma_chan, &slave_config);
+	ret = dmaengine_slave_config(chan, &slave_config);
 	if (ret)
 		return ret;
 
-	return 0;
-}
-
-static int snd_imx_pcm_hw_params(struct snd_pcm_substream *substream,
-				struct snd_pcm_hw_params *params)
-{
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-	unsigned long dma_addr;
-	struct dma_chan *chan;
-	struct imx_pcm_dma_params *dma_params;
-	int ret;
-
-	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
-	ret = imx_ssi_dma_alloc(substream, params);
-	if (ret)
-		return ret;
-	chan = iprtd->dma_chan;
 
 	iprtd->periods = params_periods(params);
 	iprtd->period_bytes = params_period_bytes(params);
@@ -164,19 +157,6 @@ static int snd_imx_pcm_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int snd_imx_pcm_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-
-	if (iprtd->dma_chan) {
-		dma_release_channel(iprtd->dma_chan);
-		iprtd->dma_chan = NULL;
-	}
-
-	return 0;
-}
-
 static int snd_imx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -251,6 +231,12 @@ static int snd_imx_open(struct snd_pcm_substream *substream)
 		return ret;
 	}
 
+	ret = imx_ssi_dma_alloc(substream);
+	if (ret < 0) {
+		kfree(iprtd);
+		return ret;
+	}
+
 	snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
 
 	return 0;
@@ -261,6 +247,7 @@ static int snd_imx_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
 
+	dma_release_channel(iprtd->dma_chan);
 	kfree(iprtd);
 
 	return 0;
@@ -271,7 +258,6 @@ static struct snd_pcm_ops imx_pcm_ops = {
 	.close		= snd_imx_close,
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_imx_pcm_hw_params,
-	.hw_free	= snd_imx_pcm_hw_free,
 	.trigger	= snd_imx_pcm_trigger,
 	.pointer	= snd_imx_pcm_pointer,
 	.mmap		= snd_imx_pcm_mmap,
-- 
1.7.9

[RFC 3/7] ASoC: mxs-pcm: Request DMA channel early

[Lars-Peter Clausen]

Request the DMA channel in the PCM open callback instead of the hwparams
callback, this allows us to let open fail if no dma channel is available. This
also fixes a bug where the channel will be requested multiple times if hwparams
is called multiple times.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/mxs/mxs-pcm.c |   28 ++++++++--------------------
 1 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c
index 06c18ec..5b8c8d3 100644
--- a/sound/soc/mxs/mxs-pcm.c
+++ b/sound/soc/mxs/mxs-pcm.c
@@ -85,8 +85,7 @@ static bool filter(struct dma_chan *chan, void *param)
 	return true;
 }
 
-static int mxs_dma_alloc(struct snd_pcm_substream *substream,
-				struct snd_pcm_hw_params *params)
+static int mxs_dma_alloc(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -112,11 +111,7 @@ static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream,
 	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
 	unsigned long dma_addr;
 	struct dma_chan *chan;
-	int ret;
 
-	ret = mxs_dma_alloc(substream, params);
-	if (ret)
-		return ret;
 	chan = iprtd->dma_chan;
 
 	iprtd->periods = params_periods(params);
@@ -143,19 +138,6 @@ static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int snd_mxs_pcm_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-
-	if (iprtd->dma_chan) {
-		dma_release_channel(iprtd->dma_chan);
-		iprtd->dma_chan = NULL;
-	}
-
-	return 0;
-}
-
 static int snd_mxs_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -208,6 +190,12 @@ static int snd_mxs_open(struct snd_pcm_substream *substream)
 		return ret;
 	}
 
+	ret = mxs_dma_alloc(substream);
+	if (ret) {
+		kfree(iprtd);
+		return ret;
+	}
+
 	snd_soc_set_runtime_hwparams(substream, &snd_mxs_hardware);
 
 	return 0;
@@ -218,6 +206,7 @@ static int snd_mxs_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
 
+	dma_release_channel(iprtd->dma_chan);
 	kfree(iprtd);
 
 	return 0;
@@ -239,7 +228,6 @@ static struct snd_pcm_ops mxs_pcm_ops = {
 	.close		= snd_mxs_close,
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_mxs_pcm_hw_params,
-	.hw_free	= snd_mxs_pcm_hw_free,
 	.trigger	= snd_mxs_pcm_trigger,
 	.pointer	= snd_mxs_pcm_pointer,
 	.mmap		= snd_mxs_pcm_mmap,
-- 
1.7.9

[RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Lars-Peter Clausen]

This patch adds a set of functions which are intended to be used when
implementing a dmaengine based sound PCM driver.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/sound/dmaengine_pcm.h |   49 +++++++
 sound/soc/Kconfig             |    3 +
 sound/soc/Makefile            |    3 +
 sound/soc/soc-dmaengine-pcm.c |  287 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 342 insertions(+), 0 deletions(-)
 create mode 100644 include/sound/dmaengine_pcm.h
 create mode 100644 sound/soc/soc-dmaengine-pcm.c

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
new file mode 100644
index 0000000..a8fcaa6
--- /dev/null
+++ b/include/sound/dmaengine_pcm.h
@@ -0,0 +1,49 @@
+/*
+ *  Copyright (C) 2012, Analog Devices Inc.
+ *	Author: Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under  the terms of the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the License, or (at your
+ *  option) any later version.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+#ifndef __SOUND_DMAENGINE_PCM_H__
+#define __SOUND_DMAENGINE_PCM_H__
+
+#include <sound/pcm.h>
+#include <linux/dmaengine.h>
+
+/**
+ * snd_pcm_substream_to_dma_direction - Get dma_transfer_direction for a PCM
+ *   substream
+ * @substream: PCM substream
+ */
+static inline enum dma_transfer_direction
+snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream)
+{
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		return DMA_MEM_TO_DEV;
+	else
+		return DMA_DEV_TO_MEM;
+}
+
+void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data);
+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);
+
+int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
+	dma_filter_fn filter_fn, void *filter_data);
+int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
+
+struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
+
+#endif
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 35e662d..91c9855 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -25,6 +25,9 @@ if SND_SOC
 config SND_SOC_AC97_BUS
 	bool
 
+config SND_SOC_DMAENGINE_PCM
+	bool
+
 # All the supported SoCs
 source "sound/soc/atmel/Kconfig"
 source "sound/soc/au1x/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 9ea8ac8..2feaf37 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -1,6 +1,9 @@
 snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o
 snd-soc-core-objs += soc-pcm.o soc-io.o
 
+snd-soc-dmaengine-pcm-objs := soc-dmaengine-pcm.o
+obj-$(CONFIG_SND_SOC_DMAENGINE_PCM) += snd-soc-dmaengine-pcm.o
+
 obj-$(CONFIG_SND_SOC)	+= snd-soc-core.o
 obj-$(CONFIG_SND_SOC)	+= codecs/
 obj-$(CONFIG_SND_SOC)	+= atmel/
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
new file mode 100644
index 0000000..0526cf8
--- /dev/null
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -0,0 +1,287 @@
+/*
+ *  Copyright (C) 2012, Analog Devices Inc.
+ *	Author: Lars-Peter Clausen <lars@metafoo.de>
+ *
+ *  Based on:
+ *	imx-pcm-dma-mx2.c, Copyright 2009 Sascha Hauer <s.hauer@pengutronix.de>
+ *	mxs-pcm.c, Copyright (C) 2011 Freescale Semiconductor, Inc.
+ *	ep93xx-pcm.c, Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ *		      Copyright (C) 2006 Applied Data Systems
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under  the terms of the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the License, or (at your
+ *  option) any later version.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/dmaengine.h>
+#include <linux/slab.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include <sound/dmaengine_pcm.h>
+
+struct dmaengine_pcm_runtime_data {
+	struct dma_chan *dma_chan;
+
+	unsigned int pos;
+
+	void *data;
+};
+
+static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
+	const struct snd_pcm_substream *substream)
+{
+	return substream->runtime->private_data;
+}
+
+/**
+ * snd_dmaengine_pcm_set_data - Set dmaengine substream private data
+ * @substream: PCM substream
+ * @data: Data to set
+ */
+void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+
+	prtd->data = data;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_set_data);
+
+/**
+ * snd_dmaengine_pcm_get_data - Get dmaeinge substream private data
+ * @substream: PCM substream
+ *
+ * Returns the data previously set with snd_dmaengine_pcm_set_data
+ */
+void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+
+	return prtd->data;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_get_data);
+
+struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+
+	return prtd->dma_chan;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_get_chan);
+
+/**
+ * snd_hwparams_to_dma_slave_config - Convert hw_params to dma_slave_config
+ * @substream: PCM substream
+ * @params: hw_params
+ * @slave_config: DMA slave config
+ *
+ * This function can be used to initialize a dma_slave_config from a substream
+ * and hw_params in a dmaengine based PCM driver implementation.
+ */
+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)
+{
+	enum dma_slave_buswidth buswidth;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S8:
+		buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		break;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		break;
+	case SNDRV_PCM_FORMAT_S18_3LE:
+	case SNDRV_PCM_FORMAT_S20_3LE:
+	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S32_LE:
+		buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		slave_config->direction = DMA_MEM_TO_DEV;
+		slave_config->dst_addr_width = buswidth;
+	} else {
+		slave_config->direction = DMA_DEV_TO_MEM;
+		slave_config->src_addr_width = buswidth;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hwparams_to_dma_slave_config);
+
+static void dmaengine_pcm_dma_complete(void *arg)
+{
+	struct snd_pcm_substream *substream = arg;
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+
+	prtd->pos += snd_pcm_lib_period_bytes(substream);
+	if (prtd->pos >= snd_pcm_lib_buffer_bytes(substream))
+		prtd->pos = 0;
+
+	snd_pcm_period_elapsed(substream);
+}
+
+static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+	struct dma_chan *chan = prtd->dma_chan;
+	struct dma_async_tx_descriptor *desc;
+	enum dma_transfer_direction direction;
+
+	direction = snd_pcm_substream_to_dma_direction(substream);
+
+	desc = chan->device->device_prep_dma_cyclic(chan,
+		substream->runtime->dma_addr,
+		snd_pcm_lib_buffer_bytes(substream),
+		snd_pcm_lib_period_bytes(substream), direction);
+
+	if (!desc)
+		return -ENOMEM;
+
+	desc->callback = dmaengine_pcm_dma_complete;
+	desc->callback_param = substream;
+	dmaengine_submit(desc);
+
+	return 0;
+}
+
+/**
+ * snd_dmaengine_pcm_trigger - dmaengine based PCM trigger implementation
+ * @substream: PCM substream
+ * @cmd: Trigger command
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ *
+ * This function can be used as the PCM trigger callback for dmaengine based PCM
+ * driver implementations.
+ */
+int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+	int ret;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		ret = dmaengine_pcm_prepare_and_submit(substream);
+		if (ret)
+			return ret;
+		dma_async_issue_pending(prtd->dma_chan);
+		break;
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		dmaengine_resume(prtd->dma_chan);
+		break;
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		dmaengine_pause(prtd->dma_chan);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		dmaengine_terminate_all(prtd->dma_chan);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger);
+
+/**
+ * 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);
+	return bytes_to_frames(substream->runtime, prtd->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)
+{
+	dma_cap_mask_t mask;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+	dma_cap_set(DMA_CYCLIC, mask);
+	prtd->dma_chan = dma_request_channel(mask, filter_fn, filter_data);
+
+	if (!prtd->dma_chan)
+		return -ENXIO;
+
+	return 0;
+}
+
+/**
+ * snd_dmaengine_pcm_open - Open a dmaengine based PCM substream
+ * @substream: PCM substream
+ * @filter_fn: Filter function used to request the DMA channel
+ * @filter_data: Data passed to the DMA filter function
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ *
+ * This function will request a DMA channel using the passed filter function and
+ * data. The function should usually be called from the pcm open callback.
+ *
+ * Note that this function will use private_data field of the substream's
+ * runtime. So it is not availabe to your pcm driver implementation. If you need
+ * to keep additional data attached to a substream use
+ * snd_dmaeinge_pcm_{set,get}_data.
+ */
+int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
+	dma_filter_fn filter_fn, void *filter_data)
+{
+	struct dmaengine_pcm_runtime_data *prtd;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_integer(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		return ret;
+
+	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
+	if (!prtd)
+		return -ENOMEM;
+
+	ret = dmaengine_pcm_request_channel(prtd, filter_fn, filter_data);
+	if (ret < 0) {
+		kfree(prtd);
+		return ret;
+	}
+
+	substream->runtime->private_data = prtd;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
+
+/**
+ * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
+ * @substream: PCM substream
+ */
+int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream)
+{
+	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
+
+	dma_release_channel(prtd->dma_chan);
+	kfree(prtd);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close);
-- 
1.7.9

[RFC 5/7] ASoC: imx-pcm-dma: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/imx/Kconfig           |    1 +
 sound/soc/imx/imx-pcm-dma-mx2.c |  176 ++++++---------------------------------
 2 files changed, 28 insertions(+), 149 deletions(-)

diff --git a/sound/soc/imx/Kconfig b/sound/soc/imx/Kconfig
index 91b66ef..08c4b9b 100644
--- a/sound/soc/imx/Kconfig
+++ b/sound/soc/imx/Kconfig
@@ -14,6 +14,7 @@ config SND_MXC_SOC_FIQ
 	tristate
 
 config SND_MXC_SOC_MX2
+	select SND_SOC_DMAENGINE_PCM
 	tristate
 
 config SND_MXC_SOC_WM1133_EV1
diff --git a/sound/soc/imx/imx-pcm-dma-mx2.c b/sound/soc/imx/imx-pcm-dma-mx2.c
index f974e61..4cd5462 100644
--- a/sound/soc/imx/imx-pcm-dma-mx2.c
+++ b/sound/soc/imx/imx-pcm-dma-mx2.c
@@ -27,104 +27,42 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
 
 #include <mach/dma.h>
 
 #include "imx-ssi.h"
 
-struct imx_pcm_runtime_data {
-	int period_bytes;
-	int periods;
-	unsigned long offset;
-	struct dma_async_tx_descriptor *desc;
-	struct dma_chan *dma_chan;
-	struct imx_dma_data dma_data;
-};
-
-static void audio_dma_irq(void *data)
-{
-	struct snd_pcm_substream *substream = (struct snd_pcm_substream *)data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-
-	iprtd->offset += iprtd->period_bytes;
-	iprtd->offset %= iprtd->period_bytes * iprtd->periods;
-
-	snd_pcm_period_elapsed(substream);
-}
-
 static bool filter(struct dma_chan *chan, void *param)
 {
-	struct imx_pcm_runtime_data *iprtd = param;
-
 	if (!imx_dma_is_general_purpose(chan))
 		return false;
 
-        chan->private = &iprtd->dma_data;
+	chan->private = param;
 
-        return true;
-}
-
-static int imx_ssi_dma_alloc(struct snd_pcm_substream *substream)
-{
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct imx_pcm_dma_params *dma_params;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-	dma_cap_mask_t mask;
-
-	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
-
-	iprtd->dma_data.peripheral_type = IMX_DMATYPE_SSI;
-	iprtd->dma_data.priority = DMA_PRIO_HIGH;
-	iprtd->dma_data.dma_request = dma_params->dma;
-
-	/* Try to grab a DMA channel */
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
-	iprtd->dma_chan = dma_request_channel(mask, filter, iprtd);
-	if (!iprtd->dma_chan)
-		return -EINVAL;
-
-	return 0;
+	return true;
 }
 
 static int snd_imx_pcm_hw_params(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-	struct dma_chan *chan = iprtd->dma_chan;
+	struct dma_chan *chan = snd_dmaengine_pcm_get_chan(substream);
 	struct imx_pcm_dma_params *dma_params;
 	struct dma_slave_config slave_config;
-	enum dma_slave_buswidth buswidth;
-	unsigned long dma_addr;
 	int ret;
 
 	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
-	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S16_LE:
-		buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES;
-		break;
-	case SNDRV_PCM_FORMAT_S20_3LE:
-	case SNDRV_PCM_FORMAT_S24_LE:
-		buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES;
-		break;
-	default:
-		return 0;
-	}
+	ret = snd_hwparams_to_dma_slave_config(substream, params, &slave_config);
+	if (ret)
+		return ret;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		slave_config.direction = DMA_MEM_TO_DEV;
 		slave_config.dst_addr = dma_params->dma_addr;
-		slave_config.dst_addr_width = buswidth;
 		slave_config.dst_maxburst = dma_params->burstsize;
 	} else {
-		slave_config.direction = DMA_DEV_TO_MEM;
 		slave_config.src_addr = dma_params->dma_addr;
-		slave_config.src_addr_width = buswidth;
 		slave_config.src_maxburst = dma_params->burstsize;
 	}
 
@@ -132,68 +70,11 @@ static int snd_imx_pcm_hw_params(struct snd_pcm_substream *substream,
 	if (ret)
 		return ret;
 
-
-	iprtd->periods = params_periods(params);
-	iprtd->period_bytes = params_period_bytes(params);
-	iprtd->offset = 0;
-
 	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
 
-	dma_addr = runtime->dma_addr;
-
-	iprtd->desc = chan->device->device_prep_dma_cyclic(chan, dma_addr,
-			iprtd->period_bytes * iprtd->periods,
-			iprtd->period_bytes,
-			substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
-			DMA_MEM_TO_DEV : DMA_DEV_TO_MEM);
-	if (!iprtd->desc) {
-		dev_err(&chan->dev->device, "cannot prepare slave dma\n");
-		return -EINVAL;
-	}
-
-	iprtd->desc->callback = audio_dma_irq;
-	iprtd->desc->callback_param = substream;
-
-	return 0;
-}
-
-static int snd_imx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		dmaengine_submit(iprtd->desc);
-
-		break;
-
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		dmaengine_terminate_all(iprtd->dma_chan);
-
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
-static snd_pcm_uframes_t snd_imx_pcm_pointer(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
-
-	pr_debug("%s: %ld %ld\n", __func__, iprtd->offset,
-			bytes_to_frames(substream->runtime, iprtd->offset));
-
-	return bytes_to_frames(substream->runtime, iprtd->offset);
-}
-
 static struct snd_pcm_hardware snd_imx_hardware = {
 	.info = SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_BLOCK_TRANSFER |
@@ -215,40 +96,37 @@ static struct snd_pcm_hardware snd_imx_hardware = {
 
 static int snd_imx_open(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct imx_pcm_dma_params *dma_params;
+	struct imx_dma_data *dma_data;
 	int ret;
 
-	iprtd = kzalloc(sizeof(*iprtd), GFP_KERNEL);
-	if (iprtd == NULL)
-		return -ENOMEM;
-	runtime->private_data = iprtd;
+	snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
 
-	ret = snd_pcm_hw_constraint_integer(substream->runtime,
-			SNDRV_PCM_HW_PARAM_PERIODS);
-	if (ret < 0) {
-		kfree(iprtd);
-		return ret;
-	}
+	dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
-	ret = imx_ssi_dma_alloc(substream);
-	if (ret < 0) {
-		kfree(iprtd);
-		return ret;
+	dma_data = kzalloc(sizeof(*dma_data), GFP_KERNEL);
+	dma_data->peripheral_type = IMX_DMATYPE_SSI;
+	dma_data->priority = DMA_PRIO_HIGH;
+	dma_data->dma_request = dma_params->dma;
+
+	ret = snd_dmaengine_pcm_open(substream, filter, dma_data);
+	if (ret) {
+		kfree(dma_data);
+		return 0;
 	}
 
-	snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
+	snd_dmaengine_pcm_set_data(substream, dma_data);
 
 	return 0;
 }
 
 static int snd_imx_close(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct imx_pcm_runtime_data *iprtd = runtime->private_data;
+	struct imx_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
 
-	dma_release_channel(iprtd->dma_chan);
-	kfree(iprtd);
+	snd_dmaengine_pcm_close(substream);
+	kfree(dma_data);
 
 	return 0;
 }
@@ -258,8 +136,8 @@ static struct snd_pcm_ops imx_pcm_ops = {
 	.close		= snd_imx_close,
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_imx_pcm_hw_params,
-	.trigger	= snd_imx_pcm_trigger,
-	.pointer	= snd_imx_pcm_pointer,
+	.trigger	= snd_dmaengine_pcm_trigger,
+	.pointer	= snd_dmaengine_pcm_pointer,
 	.mmap		= snd_imx_pcm_mmap,
 };
 
-- 
1.7.9

[RFC 6/7] ASoC: mxs-pcm: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
---
 sound/soc/mxs/Kconfig   |    1 +
 sound/soc/mxs/mxs-pcm.c |  136 ++++++++--------------------------------------
 sound/soc/mxs/mxs-pcm.h |   12 ----
 3 files changed, 25 insertions(+), 124 deletions(-)

diff --git a/sound/soc/mxs/Kconfig b/sound/soc/mxs/Kconfig
index 21d20f3..99a997f 100644
--- a/sound/soc/mxs/Kconfig
+++ b/sound/soc/mxs/Kconfig
@@ -1,6 +1,7 @@
 menuconfig SND_MXS_SOC
 	tristate "SoC Audio for Freescale MXS CPUs"
 	depends on ARCH_MXS
+	select SND_SOC_DMAENGINE_PCM
 	help
 	  Say Y or M if you want to add support for codecs attached to
 	  the MXS SAIF interface.
diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c
index 5b8c8d3..6ca1f46 100644
--- a/sound/soc/mxs/mxs-pcm.c
+++ b/sound/soc/mxs/mxs-pcm.c
@@ -34,10 +34,16 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
 
 #include <mach/dma.h>
 #include "mxs-pcm.h"
 
+struct mxs_pcm_dma_data {
+	struct mxs_dma_data dma_data;
+	struct mxs_pcm_dma_params *dma_params;
+};
+
 static struct snd_pcm_hardware snd_mxs_hardware = {
 	.info			= SNDRV_PCM_INFO_MMAP |
 				  SNDRV_PCM_INFO_MMAP_VALID |
@@ -58,21 +64,10 @@ static struct snd_pcm_hardware snd_mxs_hardware = {
 
 };
 
-static void audio_dma_irq(void *data)
-{
-	struct snd_pcm_substream *substream = (struct snd_pcm_substream *)data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-
-	iprtd->offset += iprtd->period_bytes;
-	iprtd->offset %= iprtd->period_bytes * iprtd->periods;
-	snd_pcm_period_elapsed(substream);
-}
-
 static bool filter(struct dma_chan *chan, void *param)
 {
-	struct mxs_pcm_runtime_data *iprtd = param;
-	struct mxs_pcm_dma_params *dma_params = iprtd->dma_params;
+	struct mxs_pcm_dma_data *pcm_dma_data = param;
+	struct mxs_pcm_dma_params *dma_params = pcm_dma_data->dma_params;
 
 	if (!mxs_dma_is_apbx(chan))
 		return false;
@@ -80,134 +75,51 @@ static bool filter(struct dma_chan *chan, void *param)
 	if (chan->chan_id != dma_params->chan_num)
 		return false;
 
-	chan->private = &iprtd->dma_data;
+	chan->private = &pcm_dma_data->dma_data;
 
 	return true;
 }
 
-static int mxs_dma_alloc(struct snd_pcm_substream *substream)
-{
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-	dma_cap_mask_t mask;
-
-	iprtd->dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
-
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
-	iprtd->dma_data.chan_irq = iprtd->dma_params->chan_irq;
-	iprtd->dma_chan = dma_request_channel(mask, filter, iprtd);
-	if (!iprtd->dma_chan)
-		return -EINVAL;
-
-	return 0;
-}
-
 static int snd_mxs_pcm_hw_params(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-	unsigned long dma_addr;
-	struct dma_chan *chan;
-
-	chan = iprtd->dma_chan;
-
-	iprtd->periods = params_periods(params);
-	iprtd->period_bytes = params_period_bytes(params);
-	iprtd->offset = 0;
-
 	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
 
-	dma_addr = runtime->dma_addr;
-
-	iprtd->desc = chan->device->device_prep_dma_cyclic(chan, dma_addr,
-			iprtd->period_bytes * iprtd->periods,
-			iprtd->period_bytes,
-			substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
-			DMA_MEM_TO_DEV : DMA_DEV_TO_MEM);
-	if (!iprtd->desc) {
-		dev_err(&chan->dev->device, "cannot prepare slave dma\n");
-		return -EINVAL;
-	}
-
-	iprtd->desc->callback = audio_dma_irq;
-	iprtd->desc->callback_param = substream;
-
 	return 0;
 }
 
-static int snd_mxs_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		dmaengine_submit(iprtd->desc);
-
-		break;
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		dmaengine_terminate_all(iprtd->dma_chan);
-
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static snd_pcm_uframes_t snd_mxs_pcm_pointer(
-		struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
-
-	return bytes_to_frames(substream->runtime, iprtd->offset);
-}
-
 static int snd_mxs_open(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct mxs_pcm_dma_data *pcm_dma_data;
 	int ret;
 
-	iprtd = kzalloc(sizeof(*iprtd), GFP_KERNEL);
-	if (iprtd == NULL)
+	pcm_dma_data = kzalloc(sizeof(*pcm_dma_data), GFP_KERNEL);
+	if (pcm_dma_data == NULL)
 		return -ENOMEM;
-	runtime->private_data = iprtd;
 
-	ret = snd_pcm_hw_constraint_integer(substream->runtime,
-			SNDRV_PCM_HW_PARAM_PERIODS);
-	if (ret < 0) {
-		kfree(iprtd);
-		return ret;
-	}
+	pcm_dma_data->dma_params = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
+	pcm_dma_data->dma_data.chan_irq = pcm_dma_data->dma_params->chan_irq;
 
-	ret = mxs_dma_alloc(substream);
+	ret = snd_dmaengine_pcm_open(substream, filter, pcm_dma_data);
 	if (ret) {
-		kfree(iprtd);
+		kfree(pcm_dma_data);
 		return ret;
 	}
 
 	snd_soc_set_runtime_hwparams(substream, &snd_mxs_hardware);
 
+	snd_dmaengine_pcm_set_data(substream, pcm_dma_data);
+
 	return 0;
 }
 
 static int snd_mxs_close(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct mxs_pcm_runtime_data *iprtd = runtime->private_data;
+	struct mxs_pcm_dma_data *pcm_dma_data = snd_dmaengine_pcm_get_data(substream);
 
-	dma_release_channel(iprtd->dma_chan);
-	kfree(iprtd);
+	snd_dmaengine_pcm_close(substream);
+	kfree(pcm_dma_data);
 
 	return 0;
 }
@@ -228,8 +140,8 @@ static struct snd_pcm_ops mxs_pcm_ops = {
 	.close		= snd_mxs_close,
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= snd_mxs_pcm_hw_params,
-	.trigger	= snd_mxs_pcm_trigger,
-	.pointer	= snd_mxs_pcm_pointer,
+	.trigger	= snd_dmaengine_pcm_trigger,
+	.pointer	= snd_dmaengine_pcm_pointer,
 	.mmap		= snd_mxs_pcm_mmap,
 };
 
diff --git a/sound/soc/mxs/mxs-pcm.h b/sound/soc/mxs/mxs-pcm.h
index ba75e10..5f01a91 100644
--- a/sound/soc/mxs/mxs-pcm.h
+++ b/sound/soc/mxs/mxs-pcm.h
@@ -19,21 +19,9 @@
 #ifndef _MXS_PCM_H
 #define _MXS_PCM_H
 
-#include <mach/dma.h>
-
 struct mxs_pcm_dma_params {
 	int chan_irq;
 	int chan_num;
 };
 
-struct mxs_pcm_runtime_data {
-	int period_bytes;
-	int periods;
-	unsigned long offset;
-	struct dma_async_tx_descriptor *desc;
-	struct dma_chan *dma_chan;
-	struct mxs_dma_data dma_data;
-	struct mxs_pcm_dma_params *dma_params;
-};
-
 #endif
-- 
1.7.9

[RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/ep93xx/Kconfig      |    1 +
 sound/soc/ep93xx/ep93xx-pcm.c |  148 ++++++-----------------------------------
 2 files changed, 23 insertions(+), 126 deletions(-)

diff --git a/sound/soc/ep93xx/Kconfig b/sound/soc/ep93xx/Kconfig
index 91a28de..88143db 100644
--- a/sound/soc/ep93xx/Kconfig
+++ b/sound/soc/ep93xx/Kconfig
@@ -1,6 +1,7 @@
 config SND_EP93XX_SOC
 	tristate "SoC Audio support for the Cirrus Logic EP93xx series"
 	depends on ARCH_EP93XX && SND_SOC
+	select SND_SOC_DMAENGINE_PCM
 	help
 	  Say Y or M if you want to add support for codecs attached to
 	  the EP93xx I2S or AC97 interfaces.
diff --git a/sound/soc/ep93xx/ep93xx-pcm.c b/sound/soc/ep93xx/ep93xx-pcm.c
index 32adca3..162dbb7 100644
--- a/sound/soc/ep93xx/ep93xx-pcm.c
+++ b/sound/soc/ep93xx/ep93xx-pcm.c
@@ -23,6 +23,7 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
+#include <sound/dmaengine_pcm.h>
 
 #include <mach/dma.h>
 #include <mach/hardware.h>
@@ -52,26 +53,6 @@ static const struct snd_pcm_hardware ep93xx_pcm_hardware = {
 	.fifo_size		= 32,
 };
 
-struct ep93xx_runtime_data
-{
-	int				pointer_bytes;
-	int				periods;
-	int				period_bytes;
-	struct dma_chan			*dma_chan;
-	struct ep93xx_dma_data		dma_data;
-};
-
-static void ep93xx_pcm_dma_callback(void *data)
-{
-	struct snd_pcm_substream *substream = data;
-	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
-
-	rtd->pointer_bytes += rtd->period_bytes;
-	rtd->pointer_bytes %= rtd->period_bytes * rtd->periods;
-
-	snd_pcm_period_elapsed(substream);
-}
-
 static bool ep93xx_pcm_dma_filter(struct dma_chan *chan, void *filter_param)
 {
 	struct ep93xx_dma_data *data = filter_param;
@@ -86,98 +67,48 @@ static bool ep93xx_pcm_dma_filter(struct dma_chan *chan, void *filter_param)
 
 static int ep93xx_pcm_open(struct snd_pcm_substream *substream)
 {
-	struct snd_soc_pcm_runtime *soc_rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai = soc_rtd->cpu_dai;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct ep93xx_pcm_dma_params *dma_params;
-	struct ep93xx_runtime_data *rtd;    
-	dma_cap_mask_t mask;
+	struct ep93xx_dma_data *dma_data;
 	int ret;
 
-	ret = snd_pcm_hw_constraint_integer(substream->runtime,
-					    SNDRV_PCM_HW_PARAM_PERIODS);
-	if (ret < 0)
-		return ret;
-
 	snd_soc_set_runtime_hwparams(substream, &ep93xx_pcm_hardware);
 
-	rtd = kmalloc(sizeof(*rtd), GFP_KERNEL);
-	if (!rtd) 
+	dma_data = kmalloc(sizeof(*dma_data), GFP_KERNEL);
+	if (!dma_data)
 		return -ENOMEM;
 
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
-	dma_cap_set(DMA_CYCLIC, mask);
-
 	dma_params = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	rtd->dma_data.port = dma_params->dma_port;
-	rtd->dma_data.name = dma_params->name;
-
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		rtd->dma_data.direction = DMA_MEM_TO_DEV;
-	else
-		rtd->dma_data.direction = DMA_DEV_TO_MEM;
-
-	rtd->dma_chan = dma_request_channel(mask, ep93xx_pcm_dma_filter,
-					    &rtd->dma_data);
-	if (!rtd->dma_chan) {
-		kfree(rtd);
-		return -EINVAL;
-	}
-	
-	substream->runtime->private_data = rtd;
-	return 0;
-}
+	dma_data->port = dma_params->dma_port;
+	dma_data->name = dma_params->name;
+	dma_data->direction = snd_pcm_substream_to_dma_direction(substream);
 
-static int ep93xx_pcm_close(struct snd_pcm_substream *substream)
-{
-	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
+	ret = snd_dmaengine_pcm_open(substream, ep93xx_pcm_dma_filter, dma_data);
+	if (ret) {
+		kfree(dma_data);
+		return ret;
+	}
 
-	dma_release_channel(rtd->dma_chan);
-	kfree(rtd);
-	return 0;
-}
+	snd_dmaengine_pcm_set_data(substream, dma_data);
 
-static int ep93xx_pcm_dma_submit(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct ep93xx_runtime_data *rtd = runtime->private_data;
-	struct dma_chan *chan = rtd->dma_chan;
-	struct dma_device *dma_dev = chan->device;
-	struct dma_async_tx_descriptor *desc;
-
-	rtd->pointer_bytes = 0;
-	desc = dma_dev->device_prep_dma_cyclic(chan, runtime->dma_addr,
-					       rtd->period_bytes * rtd->periods,
-					       rtd->period_bytes,
-					       rtd->dma_data.direction);
-	if (!desc)
-		return -EINVAL;
-
-	desc->callback = ep93xx_pcm_dma_callback;
-	desc->callback_param = substream;
-
-	dmaengine_submit(desc);
 	return 0;
 }
 
-static void ep93xx_pcm_dma_flush(struct snd_pcm_substream *substream)
+static int ep93xx_pcm_close(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct ep93xx_runtime_data *rtd = runtime->private_data;
+	struct dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
 
-	dmaengine_terminate_all(rtd->dma_chan);
+	snd_dmaengine_pcm_close(substream);
+	kfree(dma_data);
+	return 0;
 }
 
 static int ep93xx_pcm_hw_params(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct ep93xx_runtime_data *rtd = runtime->private_data;
-
 	snd_pcm_set_runtime_buffer(substream, &substream->dma_buffer);
 
-	rtd->periods = params_periods(params);
-	rtd->period_bytes = params_period_bytes(params);
 	return 0;
 }
 
@@ -187,41 +118,6 @@ static int ep93xx_pcm_hw_free(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int ep93xx_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	int ret;
-
-	ret = 0;
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_RESUME:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		ret = ep93xx_pcm_dma_submit(substream);
-		break;
-
-	case SNDRV_PCM_TRIGGER_STOP:
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		ep93xx_pcm_dma_flush(substream);
-		break;
-
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-static snd_pcm_uframes_t ep93xx_pcm_pointer(struct snd_pcm_substream *substream)
-{
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct ep93xx_runtime_data *rtd = substream->runtime->private_data;
-
-	/* FIXME: implement this with sub-period granularity */
-	return bytes_to_frames(runtime, rtd->pointer_bytes);
-}
-
 static int ep93xx_pcm_mmap(struct snd_pcm_substream *substream,
 			   struct vm_area_struct *vma)
 {
@@ -239,8 +135,8 @@ static struct snd_pcm_ops ep93xx_pcm_ops = {
 	.ioctl		= snd_pcm_lib_ioctl,
 	.hw_params	= ep93xx_pcm_hw_params,
 	.hw_free	= ep93xx_pcm_hw_free,
-	.trigger	= ep93xx_pcm_trigger,
-	.pointer	= ep93xx_pcm_pointer,
+	.trigger	= snd_dmaengine_pcm_trigger,
+	.pointer	= snd_dmaengine_pcm_pointer,
 	.mmap		= ep93xx_pcm_mmap,
 };
 
-- 
1.7.9

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Russell King - ARM Linux]

On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> This patch adds a set of functions which are intended to be used when
> implementing a dmaengine based sound PCM driver.

This isn't going to be usable on sa11x0 platforms, because we need to
know the struct device underlying the DMA device before we allocate
the buffers.

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Lars-Peter Clausen]

On 02/22/2012 11:01 AM, Russell King - ARM Linux wrote:
> On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
>> This patch adds a set of functions which are intended to be used when
>> implementing a dmaengine based sound PCM driver.
> 
> This isn't going to be usable on sa11x0 platforms, because we need to
> know the struct device underlying the DMA device before we allocate
> the buffers.

Which means the dma channel has to be requested before allocating the
buffers. Which can be done. Not with the current code but it can be
implemented on top of it.

I had a look at your 'generic' dmaengine PCM driver patch. And it looks like
you went to opposite way as I did in this patch. You implemented one driver
which is supposed to work for all cases, while my patch provides a set of
helper functions which can be used to implement a PCM driver. Your patch
only supports non-cyclic transfers, mine only supports cyclic transfers.

Your driver emulates cyclic transfers using non-cyclic transfers, in my
opinion it is better to add such a emulation layer to the dmaengine core
itself. This will allow other users to benefit from this as well and it
doesn't have to be reimplemented in other subsystems/driver. Also users
don't have to be updated if a dmaengine driver gets native support for
cyclic transfers. But, well, the code exists and it is a step in the right
direction so we should probably use it.

I suppose we could rearrange the code so that we can share most of it
between the cyclic and non-cyclic case. The non-cyclic case needs special
handling everywhere, which can be made conditional.

- Lars

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 13377 bytes --]

On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> This patch adds a set of functions which are intended to be used when
> implementing a dmaengine based sound PCM driver.

Adding Morimoto-san as he has just implemented dmaengine support for
fsi which should also be able to use the same framework.  Not cutting
any of the text so he can see it.

> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Tested-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  include/sound/dmaengine_pcm.h |   49 +++++++
>  sound/soc/Kconfig             |    3 +
>  sound/soc/Makefile            |    3 +
>  sound/soc/soc-dmaengine-pcm.c |  287 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 342 insertions(+), 0 deletions(-)
>  create mode 100644 include/sound/dmaengine_pcm.h
>  create mode 100644 sound/soc/soc-dmaengine-pcm.c
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> new file mode 100644
> index 0000000..a8fcaa6
> --- /dev/null
> +++ b/include/sound/dmaengine_pcm.h
> @@ -0,0 +1,49 @@
> +/*
> + *  Copyright (C) 2012, Analog Devices Inc.
> + *	Author: Lars-Peter Clausen <lars@metafoo.de>
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under  the terms of the GNU General  Public License as published by the
> + *  Free Software Foundation;  either version 2 of the License, or (at your
> + *  option) any later version.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  675 Mass Ave, Cambridge, MA 02139, USA.
> + *
> + */
> +#ifndef __SOUND_DMAENGINE_PCM_H__
> +#define __SOUND_DMAENGINE_PCM_H__
> +
> +#include <sound/pcm.h>
> +#include <linux/dmaengine.h>
> +
> +/**
> + * snd_pcm_substream_to_dma_direction - Get dma_transfer_direction for a PCM
> + *   substream
> + * @substream: PCM substream
> + */
> +static inline enum dma_transfer_direction
> +snd_pcm_substream_to_dma_direction(const struct snd_pcm_substream *substream)
> +{
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +		return DMA_MEM_TO_DEV;
> +	else
> +		return DMA_DEV_TO_MEM;
> +}
> +
> +void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data);
> +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);
> +
> +int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> +	dma_filter_fn filter_fn, void *filter_data);
> +int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
> +
> +struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> +
> +#endif
> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
> index 35e662d..91c9855 100644
> --- a/sound/soc/Kconfig
> +++ b/sound/soc/Kconfig
> @@ -25,6 +25,9 @@ if SND_SOC
>  config SND_SOC_AC97_BUS
>  	bool
>  
> +config SND_SOC_DMAENGINE_PCM
> +	bool
> +
>  # All the supported SoCs
>  source "sound/soc/atmel/Kconfig"
>  source "sound/soc/au1x/Kconfig"
> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
> index 9ea8ac8..2feaf37 100644
> --- a/sound/soc/Makefile
> +++ b/sound/soc/Makefile
> @@ -1,6 +1,9 @@
>  snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o
>  snd-soc-core-objs += soc-pcm.o soc-io.o
>  
> +snd-soc-dmaengine-pcm-objs := soc-dmaengine-pcm.o
> +obj-$(CONFIG_SND_SOC_DMAENGINE_PCM) += snd-soc-dmaengine-pcm.o
> +
>  obj-$(CONFIG_SND_SOC)	+= snd-soc-core.o
>  obj-$(CONFIG_SND_SOC)	+= codecs/
>  obj-$(CONFIG_SND_SOC)	+= atmel/
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> new file mode 100644
> index 0000000..0526cf8
> --- /dev/null
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -0,0 +1,287 @@
> +/*
> + *  Copyright (C) 2012, Analog Devices Inc.
> + *	Author: Lars-Peter Clausen <lars@metafoo.de>
> + *
> + *  Based on:
> + *	imx-pcm-dma-mx2.c, Copyright 2009 Sascha Hauer <s.hauer@pengutronix.de>
> + *	mxs-pcm.c, Copyright (C) 2011 Freescale Semiconductor, Inc.
> + *	ep93xx-pcm.c, Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
> + *		      Copyright (C) 2006 Applied Data Systems
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under  the terms of the GNU General  Public License as published by the
> + *  Free Software Foundation;  either version 2 of the License, or (at your
> + *  option) any later version.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  675 Mass Ave, Cambridge, MA 02139, USA.
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/dmaengine.h>
> +#include <linux/slab.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#include <sound/dmaengine_pcm.h>
> +
> +struct dmaengine_pcm_runtime_data {
> +	struct dma_chan *dma_chan;
> +
> +	unsigned int pos;
> +
> +	void *data;
> +};
> +
> +static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
> +	const struct snd_pcm_substream *substream)
> +{
> +	return substream->runtime->private_data;
> +}
> +
> +/**
> + * snd_dmaengine_pcm_set_data - Set dmaengine substream private data
> + * @substream: PCM substream
> + * @data: Data to set
> + */
> +void snd_dmaengine_pcm_set_data(struct snd_pcm_substream *substream, void *data)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	prtd->data = data;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_set_data);
> +
> +/**
> + * snd_dmaengine_pcm_get_data - Get dmaeinge substream private data
> + * @substream: PCM substream
> + *
> + * Returns the data previously set with snd_dmaengine_pcm_set_data
> + */
> +void *snd_dmaengine_pcm_get_data(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	return prtd->data;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_get_data);
> +
> +struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	return prtd->dma_chan;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_get_chan);
> +
> +/**
> + * snd_hwparams_to_dma_slave_config - Convert hw_params to dma_slave_config
> + * @substream: PCM substream
> + * @params: hw_params
> + * @slave_config: DMA slave config
> + *
> + * This function can be used to initialize a dma_slave_config from a substream
> + * and hw_params in a dmaengine based PCM driver implementation.
> + */
> +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)
> +{
> +	enum dma_slave_buswidth buswidth;
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S8:
> +		buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
> +		break;
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +		break;
> +	case SNDRV_PCM_FORMAT_S18_3LE:
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +	case SNDRV_PCM_FORMAT_S32_LE:
> +		buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		slave_config->direction = DMA_MEM_TO_DEV;
> +		slave_config->dst_addr_width = buswidth;
> +	} else {
> +		slave_config->direction = DMA_DEV_TO_MEM;
> +		slave_config->src_addr_width = buswidth;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_hwparams_to_dma_slave_config);
> +
> +static void dmaengine_pcm_dma_complete(void *arg)
> +{
> +	struct snd_pcm_substream *substream = arg;
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	prtd->pos += snd_pcm_lib_period_bytes(substream);
> +	if (prtd->pos >= snd_pcm_lib_buffer_bytes(substream))
> +		prtd->pos = 0;
> +
> +	snd_pcm_period_elapsed(substream);
> +}
> +
> +static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +	struct dma_chan *chan = prtd->dma_chan;
> +	struct dma_async_tx_descriptor *desc;
> +	enum dma_transfer_direction direction;
> +
> +	direction = snd_pcm_substream_to_dma_direction(substream);
> +
> +	desc = chan->device->device_prep_dma_cyclic(chan,
> +		substream->runtime->dma_addr,
> +		snd_pcm_lib_buffer_bytes(substream),
> +		snd_pcm_lib_period_bytes(substream), direction);
> +
> +	if (!desc)
> +		return -ENOMEM;
> +
> +	desc->callback = dmaengine_pcm_dma_complete;
> +	desc->callback_param = substream;
> +	dmaengine_submit(desc);
> +
> +	return 0;
> +}
> +
> +/**
> + * snd_dmaengine_pcm_trigger - dmaengine based PCM trigger implementation
> + * @substream: PCM substream
> + * @cmd: Trigger command
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function can be used as the PCM trigger callback for dmaengine based PCM
> + * driver implementations.
> + */
> +int snd_dmaengine_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +	int ret;
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +		ret = dmaengine_pcm_prepare_and_submit(substream);
> +		if (ret)
> +			return ret;
> +		dma_async_issue_pending(prtd->dma_chan);
> +		break;
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		dmaengine_resume(prtd->dma_chan);
> +		break;
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		dmaengine_pause(prtd->dma_chan);
> +		break;
> +	case SNDRV_PCM_TRIGGER_STOP:
> +		dmaengine_terminate_all(prtd->dma_chan);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_trigger);
> +
> +/**
> + * 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);
> +	return bytes_to_frames(substream->runtime, prtd->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)
> +{
> +	dma_cap_mask_t mask;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +	dma_cap_set(DMA_CYCLIC, mask);
> +	prtd->dma_chan = dma_request_channel(mask, filter_fn, filter_data);
> +
> +	if (!prtd->dma_chan)
> +		return -ENXIO;
> +
> +	return 0;
> +}
> +
> +/**
> + * snd_dmaengine_pcm_open - Open a dmaengine based PCM substream
> + * @substream: PCM substream
> + * @filter_fn: Filter function used to request the DMA channel
> + * @filter_data: Data passed to the DMA filter function
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function will request a DMA channel using the passed filter function and
> + * data. The function should usually be called from the pcm open callback.
> + *
> + * Note that this function will use private_data field of the substream's
> + * runtime. So it is not availabe to your pcm driver implementation. If you need
> + * to keep additional data attached to a substream use
> + * snd_dmaeinge_pcm_{set,get}_data.
> + */
> +int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
> +	dma_filter_fn filter_fn, void *filter_data)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_integer(substream->runtime,
> +					    SNDRV_PCM_HW_PARAM_PERIODS);
> +	if (ret < 0)
> +		return ret;
> +
> +	prtd = kzalloc(sizeof(*prtd), GFP_KERNEL);
> +	if (!prtd)
> +		return -ENOMEM;
> +
> +	ret = dmaengine_pcm_request_channel(prtd, filter_fn, filter_data);
> +	if (ret < 0) {
> +		kfree(prtd);
> +		return ret;
> +	}
> +
> +	substream->runtime->private_data = prtd;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_open);
> +
> +/**
> + * snd_dmaengine_pcm_close - Close a dmaengine based PCM substream
> + * @substream: PCM substream
> + */
> +int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream)
> +{
> +	struct dmaengine_pcm_runtime_data *prtd = substream_to_prtd(substream);
> +
> +	dma_release_channel(prtd->dma_chan);
> +	kfree(prtd);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close);
> -- 
> 1.7.9
> 

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 998 bytes --]

On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> This patch adds a set of functions which are intended to be used when
> implementing a dmaengine based sound PCM driver.

Looks good - if you need to resend then:

> + * Note that this function will use private_data field of the substream's
> + * runtime. So it is not availabe to your pcm driver implementation. If you need
> + * to keep additional data attached to a substream use
> + * snd_dmaeinge_pcm_{set,get}_data.

there's a typo here but no need to resend just for that.

I'd like to see some review from both Morimoto-san as we should convert
fsi over to this too, Vinod I guess you're also pretty much happy given
your comments on the previous version?

For the non-cyclic DMAs the idea of emulating at the dmaengine layer
does seem very sensible but if that's hard then having the code at the
ASoC level and pushing it down later seems fine.  We do have several
platforms with non-cyclic DMA so it's a general need.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 1/7] ASoC: imx-ssi: Set dma data early

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 257 bytes --]

On Wed, Feb 22, 2012 at 10:49:05AM +0100, Lars-Peter Clausen wrote:
> Move the call to snd_soc_dai_set_dma_data from the hw_params callback to the
> startup callback. This allows us to use the dma data in the pcm driver's open
> callback.

Applied, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 2/7] ASoC: imx-pcm: Request DMA channel early

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 210 bytes --]

On Wed, Feb 22, 2012 at 10:49:06AM +0100, Lars-Peter Clausen wrote:
> Request the DMA channel in the pcm open callback. This allows us to let open
> fail if there is no dma channel available.

Applied, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 3/7] ASoC: mxs-pcm: Request DMA channel early

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 352 bytes --]

On Wed, Feb 22, 2012 at 10:49:07AM +0100, Lars-Peter Clausen wrote:
> Request the DMA channel in the PCM open callback instead of the hwparams
> callback, this allows us to let open fail if no dma channel is available. This
> also fixes a bug where the channel will be requested multiple times if hwparams
> is called multiple times.

Applied, thanks.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Vinod Koul]

On Wed, 2012-02-22 at 13:21 +0000, Mark Brown wrote:
> On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> > This patch adds a set of functions which are intended to be used when
> > implementing a dmaengine based sound PCM driver.
> 
> Looks good - if you need to resend then:
> 
> > + * Note that this function will use private_data field of the substream's
> > + * runtime. So it is not availabe to your pcm driver implementation. If you need
> > + * to keep additional data attached to a substream use
> > + * snd_dmaeinge_pcm_{set,get}_data.
> 
> there's a typo here but no need to resend just for that.
> 
> I'd like to see some review from both Morimoto-san as we should convert
> fsi over to this too, Vinod I guess you're also pretty much happy given
> your comments on the previous version?
Sure,
Acked-by :Vinod Koul <vinod.koul@linux.intel.com>
> 
> For the non-cyclic DMAs the idea of emulating at the dmaengine layer
> does seem very sensible but if that's hard then having the code at the
> ASoC level and pushing it down later seems fine.  We do have several
> platforms with non-cyclic DMA so it's a general need.
I think this should be pushed to dmaengine rather than in ASoC. That way
ASoC can always request circular and we emulate the behavior in
dmaengine for dmacs who don't support this


-- 
~Vinod

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 759 bytes --]

On Wed, Feb 22, 2012 at 07:00:09PM +0530, Vinod Koul wrote:
> On Wed, 2012-02-22 at 13:21 +0000, Mark Brown wrote:

> > For the non-cyclic DMAs the idea of emulating at the dmaengine layer
> > does seem very sensible but if that's hard then having the code at the
> > ASoC level and pushing it down later seems fine.  We do have several
> > platforms with non-cyclic DMA so it's a general need.

> I think this should be pushed to dmaengine rather than in ASoC. That way
> ASoC can always request circular and we emulate the behavior in
> dmaengine for dmacs who don't support this

That's what I just said.  I'm just saying that if for some reason it's
going to be difficult to implement I don't mind carrying code at the
ASoC layer while that work is done.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Russell King - ARM Linux]

On Wed, Feb 22, 2012 at 01:21:08PM +0000, Mark Brown wrote:
> On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> > This patch adds a set of functions which are intended to be used when
> > implementing a dmaengine based sound PCM driver.
> 
> Looks good - if you need to resend then:
> 
> > + * Note that this function will use private_data field of the substream's
> > + * runtime. So it is not availabe to your pcm driver implementation. If you need
> > + * to keep additional data attached to a substream use
> > + * snd_dmaeinge_pcm_{set,get}_data.
> 
> there's a typo here but no need to resend just for that.
> 
> I'd like to see some review from both Morimoto-san as we should convert
> fsi over to this too, Vinod I guess you're also pretty much happy given
> your comments on the previous version?
> 
> For the non-cyclic DMAs the idea of emulating at the dmaengine layer
> does seem very sensible but if that's hard then having the code at the
> ASoC level and pushing it down later seems fine.  We do have several
> platforms with non-cyclic DMA so it's a general need.

I think you're making the assumption that other people need cyclic
transfers.  I've seen little evidence that anyone other than sound
needs such things, so I don't think there's justification to push
this code into every DMA engine driver.

Remember, there's no common code to a DMA engine driver at all,
everyone implements their own way with their own bugs.  I would agree
with you if there was some decent DMA engine infrastructure to abstract
out such things, but there isn't.

So what you're asking is for N different ways of doing this, instead of
having one centralized way.

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1297 bytes --]

On Wed, Feb 22, 2012 at 01:32:07PM +0000, Russell King - ARM Linux wrote:

> I think you're making the assumption that other people need cyclic
> transfers.  I've seen little evidence that anyone other than sound
> needs such things, so I don't think there's justification to push
> this code into every DMA engine driver.

The idea was to have the library code to do this in the dmaengine core
rather in drivers.  Doing it in the drivers would be silly, it should be
in library code somewhere.  If we are going to librify it then pushing
the code as far up the stack as we can seems like a smart move.

> Remember, there's no common code to a DMA engine driver at all,
> everyone implements their own way with their own bugs.  I would agree
> with you if there was some decent DMA engine infrastructure to abstract
> out such things, but there isn't.

> So what you're asking is for N different ways of doing this, instead of
> having one centralized way.

Well, Vinod seemed to also think that dmaengine ought to be able to cope
with emulating this and I really don't see why it shouldn't be.  It does
know the capabilities of the underlying driver and it gets to look at
all the calls going into the driver before the driver does.  If ASoC can
interpose itself I don't see why dmaengine can't.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Russell King - ARM Linux]

On Wed, Feb 22, 2012 at 01:39:39PM +0000, Mark Brown wrote:
> Well, Vinod seemed to also think that dmaengine ought to be able to cope
> with emulating this and I really don't see why it shouldn't be.  It does
> know the capabilities of the underlying driver and it gets to look at
> all the calls going into the driver before the driver does.  If ASoC can
> interpose itself I don't see why dmaengine can't.

Have you ever looked at the DMA engine drivers?  It's a total mess.
Even something as basic as the DMA engine driver assigning a cookie to
the descriptor is implemented in each driver in their own unique way.

Completion of DMA descriptors is a similar thing - every driver implements
this in their own unique way.

The only really common thing in the DMA engine drivers is that they all
share the same API.  Nothing much more than that is shared.

It's something I have been chipping away at for a while, trying to extract
out common parts into a library, but it's really not that easy.

So, there's no way in hell that I want to see any more stuff pushed down
into the DMA engine drivers until we have a proper DMA engine library in
place to stop all this idiotic code duplication throughout every driver.
Especially if it means everyone will be implementing their own cyclic
emulation in their individual drivers.

And lets not forget that it's not just a question of "oh we can just queue
up buffer after buffer".  Doing it properly for audio does need to ensure
that the DMA engine driver is capable of getting the next buffer chained
into the hardware before the previous buffer has completed, so the transfer
continues across a buffer boundary without needing the intervention of an
interrupt handler.  Otherwise you get glitches in the audio.

So, there's a lot more here that DMA engine stuff needs to get fixed than
just a simple "layer on top".

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2122 bytes --]

On Wed, Feb 22, 2012 at 02:22:36PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 22, 2012 at 01:39:39PM +0000, Mark Brown wrote:

> > Well, Vinod seemed to also think that dmaengine ought to be able to cope
> > with emulating this and I really don't see why it shouldn't be.  It does
> > know the capabilities of the underlying driver and it gets to look at
> > all the calls going into the driver before the driver does.  If ASoC can
> > interpose itself I don't see why dmaengine can't.

> Have you ever looked at the DMA engine drivers?  It's a total mess.

Not in enormous detail, no.

> Even something as basic as the DMA engine driver assigning a cookie to
> the descriptor is implemented in each driver in their own unique way.

> Completion of DMA descriptors is a similar thing - every driver implements
> this in their own unique way.

> The only really common thing in the DMA engine drivers is that they all
> share the same API.  Nothing much more than that is shared.

Plus they're going through a common vtable and accessors which is the
main thing here I expect.

> So, there's no way in hell that I want to see any more stuff pushed down
> into the DMA engine drivers until we have a proper DMA engine library in
> place to stop all this idiotic code duplication throughout every driver.
> Especially if it means everyone will be implementing their own cyclic
> emulation in their individual drivers.

I've not seen any suggestion from anyone that that's a good idea.

> So, there's a lot more here that DMA engine stuff needs to get fixed than
> just a simple "layer on top".

Well, if we can't do it with a layer in the dmaengine code then I don't
see how we could do it in ASoC either, presumably the issues that block
doing it in the dmaengine core code would also cause issues doing it in
ASoC?

I'm not saying there's nothing that needs fixing in the dmaengine code,
and like I say I'm not too worried about carrying things in ASoC if the
dmaengine code is intractably difficult, but I'm not sure that the
issues you're rasing aren't orthogonal to the issues with emulating
cyclic transfers.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Russell King - ARM Linux]

On Wed, Feb 22, 2012 at 03:04:56PM +0000, Mark Brown wrote:
> Well, if we can't do it with a layer in the dmaengine code then I don't
> see how we could do it in ASoC either, presumably the issues that block
> doing it in the dmaengine core code would also cause issues doing it in
> ASoC?
> 
> I'm not saying there's nothing that needs fixing in the dmaengine code,
> and like I say I'm not too worried about carrying things in ASoC if the
> dmaengine code is intractably difficult, but I'm not sure that the
> issues you're rasing aren't orthogonal to the issues with emulating
> cyclic transfers.

Look, I spent a number of weeks working on the SA11x0 DMA engine code
and ALSA side to get something which worked reliably, and a simple
solution doesn't work.  I've been looking at the DMA engine code
probably for longer than Vinod has been involved with it.  I've been
working on cleaning up all the DMA engine drivers extracting some of the
common bits from them.  I'm very familiar with level of crap in the DMA
engine stuff.  I know what I'm talking about.

The point that I'm making is that there's more to this than just adding
a layer.  If you think that's all that there is, then you haven't properly
understood the SA11x0 audio support patch set that I have, and the
interactions between the individual patches.  That's not surprising
because I haven't posted the patches yet, and I haven't explained them
either.

And since I'm spending today working on x86 kernel instability (thanks to
Linux for taking out my laptop last night) I've little time to spend
discussing the fine points of this, let alone any other ARM stuff.

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1363 bytes --]

On Wed, Feb 22, 2012 at 03:23:55PM +0000, Russell King - ARM Linux wrote:

> Look, I spent a number of weeks working on the SA11x0 DMA engine code
> and ALSA side to get something which worked reliably, and a simple
> solution doesn't work.  I've been looking at the DMA engine code
> probably for longer than Vinod has been involved with it.  I've been
> working on cleaning up all the DMA engine drivers extracting some of the
> common bits from them.  I'm very familiar with level of crap in the DMA
> engine stuff.  I know what I'm talking about.

OK, fair enough - like I say the thing I'm not getting is why this is
resolvable in a dmaengine client but not in dmaengine itself.  If
nothing else I'd expect the core to be able to insert a proxy client.

> The point that I'm making is that there's more to this than just adding
> a layer.  If you think that's all that there is, then you haven't properly
> understood the SA11x0 audio support patch set that I have, and the
> interactions between the individual patches.  That's not surprising
> because I haven't posted the patches yet, and I haven't explained them
> either.

Indeed, I've not really looked at the code in any great detail at your
code as you didn't post it (I only looked at all because it showed up in
-next) and there were some issues at the subsystem level so I stopped
pretty quickly.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Vinod Koul]

On Wed, 2012-02-22 at 14:22 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 22, 2012 at 01:39:39PM +0000, Mark Brown wrote:
> > Well, Vinod seemed to also think that dmaengine ought to be able to cope
> > with emulating this and I really don't see why it shouldn't be.  It does
> > know the capabilities of the underlying driver and it gets to look at
> > all the calls going into the driver before the driver does.  If ASoC can
> > interpose itself I don't see why dmaengine can't.
> 
> Have you ever looked at the DMA engine drivers?  It's a total mess.
> Even something as basic as the DMA engine driver assigning a cookie to
> the descriptor is implemented in each driver in their own unique way.
> 
> Completion of DMA descriptors is a similar thing - every driver implements
> this in their own unique way.
> 
> The only really common thing in the DMA engine drivers is that they all
> share the same API.  Nothing much more than that is shared.
> 
> It's something I have been chipping away at for a while, trying to extract
> out common parts into a library, but it's really not that easy.
> 
> So, there's no way in hell that I want to see any more stuff pushed down
> into the DMA engine drivers until we have a proper DMA engine library in
> place to stop all this idiotic code duplication throughout every driver.
> Especially if it means everyone will be implementing their own cyclic
> emulation in their individual drivers.
> 
> And lets not forget that it's not just a question of "oh we can just queue
> up buffer after buffer".  Doing it properly for audio does need to ensure
> that the DMA engine driver is capable of getting the next buffer chained
> into the hardware before the previous buffer has completed, so the transfer
> continues across a buffer boundary without needing the intervention of an
> interrupt handler.  Otherwise you get glitches in the audio.
> 
> So, there's a lot more here that DMA engine stuff needs to get fixed than
> just a simple "layer on top".
I would agree with Russell that some of dma drivers are messy, but then
were also a victim of ambiguous API. Now at least we have a clear
direction of what is expected out of dma driver and what is not. Except
for channel mapping stuff, rest has a clarity.

So the new drivers and fixes now are going in the right direction
(hopefully you will agree with me on this) and we are frankly in much
better state then we were one year back.

Said that, we still have lot of work to do to improve :)
And I see this as another opportunity. Having a common library for these
kind of common subsystem operations forces people to write proper driver
and forces them to fix existing issue. Now, new support for DMAengine in
asoc wont come without using this library and if it does work on a
particular driver that is precondition for fixes as well.

Similarly I would really love to see such libraries in other subsystems
which would help us in making the usage of dmaengine and dmac standard
and straight forward for users.

-- 
~Vinod

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Mika Westerberg]

On Wed, Feb 22, 2012 at 10:49:11AM +0100, Lars-Peter Clausen wrote:
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

For some reason, this doesn't work on my ep93xx based Sim.One board. On
playback with mpg123 when I press stop, it continues to play whatever was on
the ring-buffer forever. Without the patches it works fine.

I'll try to find some time to debug this further.

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

On 02/27/2012 09:19 AM, Mika Westerberg wrote:
> On Wed, Feb 22, 2012 at 10:49:11AM +0100, Lars-Peter Clausen wrote:
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> For some reason, this doesn't work on my ep93xx based Sim.One board. On
> playback with mpg123 when I press stop, it continues to play whatever was on
> the ring-buffer forever. Without the patches it works fine.
> 
> I'll try to find some time to debug this further.

Hm, that’s interesting. The original ep93xx pcm driver was almost identical
to what the common helper functions do. The only difference I can spot right
now is, that it doesn't call dma_issue_pending after submitting the
descriptor. Could you try to comment out the dma_issue_pending in
soc-dmaengine-pcm.c and test whether it makes a difference?

Thanks,
- Lars

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Mika Westerberg]

On Mon, Feb 27, 2012 at 09:51:57AM +0100, Lars-Peter Clausen wrote:
> On 02/27/2012 09:19 AM, Mika Westerberg wrote:
> > On Wed, Feb 22, 2012 at 10:49:11AM +0100, Lars-Peter Clausen wrote:
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > 
> > For some reason, this doesn't work on my ep93xx based Sim.One board. On
> > playback with mpg123 when I press stop, it continues to play whatever was on
> > the ring-buffer forever. Without the patches it works fine.
> > 
> > I'll try to find some time to debug this further.
> 
> Hm, that’s interesting. The original ep93xx pcm driver was almost identical
> to what the common helper functions do. The only difference I can spot right
> now is, that it doesn't call dma_issue_pending after submitting the
> descriptor. Could you try to comment out the dma_issue_pending in
> soc-dmaengine-pcm.c and test whether it makes a difference?

I did try that but there was no effect, unfortunately.

However, I noticed that prtd->pos was never set to zero as it was in the
original ep93xx-pcm driver. With following addition to your patch, playback
works fine.

diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 0526cf8..4420b70 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -142,6 +142,7 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
 
 	direction = snd_pcm_substream_to_dma_direction(substream);
 
+	prtd->pos = 0;
 	desc = chan->device->device_prep_dma_cyclic(chan,
 		substream->runtime->dma_addr,
 		snd_pcm_lib_buffer_bytes(substream),
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

On 02/27/2012 08:01 PM, Mika Westerberg wrote:
> On Mon, Feb 27, 2012 at 09:51:57AM +0100, Lars-Peter Clausen wrote:
>> On 02/27/2012 09:19 AM, Mika Westerberg wrote:
>>> On Wed, Feb 22, 2012 at 10:49:11AM +0100, Lars-Peter Clausen wrote:
>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>>
>>> For some reason, this doesn't work on my ep93xx based Sim.One board. On
>>> playback with mpg123 when I press stop, it continues to play whatever was on
>>> the ring-buffer forever. Without the patches it works fine.
>>>
>>> I'll try to find some time to debug this further.
>>
>> Hm, that’s interesting. The original ep93xx pcm driver was almost identical
>> to what the common helper functions do. The only difference I can spot right
>> now is, that it doesn't call dma_issue_pending after submitting the
>> descriptor. Could you try to comment out the dma_issue_pending in
>> soc-dmaengine-pcm.c and test whether it makes a difference?
> 
> I did try that but there was no effect, unfortunately.
> 
> However, I noticed that prtd->pos was never set to zero as it was in the
> original ep93xx-pcm driver. With following addition to your patch, playback
> works fine.
> 

Ah, nice :)

Thanks,
- Lars

> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 0526cf8..4420b70 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -142,6 +142,7 @@ static int dmaengine_pcm_prepare_and_submit(struct snd_pcm_substream *substream)
>  
>  	direction = snd_pcm_substream_to_dma_direction(substream);
>  
> +	prtd->pos = 0;
>  	desc = chan->device->device_prep_dma_cyclic(chan,
>  		substream->runtime->dma_addr,
>  		snd_pcm_lib_buffer_bytes(substream),
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 412 bytes --]

On Tue, Feb 28, 2012 at 09:47:50AM +0100, Lars-Peter Clausen wrote:
> On 02/27/2012 08:01 PM, Mika Westerberg wrote:

> > However, I noticed that prtd->pos was never set to zero as it was in the
> > original ep93xx-pcm driver. With following addition to your patch, playback
> > works fine.

> Ah, nice :)

Looking good.  Can you please resend the series with this fix applied
and all the acks you've collected?

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Lars-Peter Clausen]

On 02/28/2012 01:52 PM, Mark Brown wrote:
> On Tue, Feb 28, 2012 at 09:47:50AM +0100, Lars-Peter Clausen wrote:
>> On 02/27/2012 08:01 PM, Mika Westerberg wrote:
> 
>>> However, I noticed that prtd->pos was never set to zero as it was in the
>>> original ep93xx-pcm driver. With following addition to your patch, playback
>>> works fine.
> 
>> Ah, nice :)
> 
> Looking good.  Can you please resend the series with this fix applied
> and all the acks you've collected?

I don't think there were any Acked-bys, just a Tested-by from Shawn Guo.

Mika, can I get a Tested-by or Acked-by for this patch from you?

Thanks,
- Lars

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 340 bytes --]

On Tue, Feb 28, 2012 at 03:17:06PM +0100, Lars-Peter Clausen wrote:
> On 02/28/2012 01:52 PM, Mark Brown wrote:

> > Looking good.  Can you please resend the series with this fix applied
> > and all the acks you've collected?

> I don't think there were any Acked-bys, just a Tested-by from Shawn Guo.

I think Vinod acked it, ICBW though.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 7/7] ASoC: ep93xx-pcm: Use dmaengine PCM helper functions

[Mika Westerberg]

On Tue, Feb 28, 2012 at 03:17:06PM +0100, Lars-Peter Clausen wrote:
> On 02/28/2012 01:52 PM, Mark Brown wrote:
> > On Tue, Feb 28, 2012 at 09:47:50AM +0100, Lars-Peter Clausen wrote:
> >> On 02/27/2012 08:01 PM, Mika Westerberg wrote:
> > 
> >>> However, I noticed that prtd->pos was never set to zero as it was in the
> >>> original ep93xx-pcm driver. With following addition to your patch, playback
> >>> works fine.
> > 
> >> Ah, nice :)
> > 
> > Looking good.  Can you please resend the series with this fix applied
> > and all the acks you've collected?
> 
> I don't think there were any Acked-bys, just a Tested-by from Shawn Guo.
> 
> Mika, can I get a Tested-by or Acked-by for this patch from you?

Sure,

Tested-by: Mika Westerberg <mika.westerberg@iki.fi>

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 779 bytes --]

On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
> This patch adds a set of functions which are intended to be used when
> implementing a dmaengine based sound PCM driver.

Having this framework is a substantial win and I got bored waiting for
the repost of this and the subsequent patches (especially given that the
merge window is drawing nearer).  Since the fix for ep93xx is so simple
and it works on i.MX I went ahead and applied everything except the
ep93xx conversion.  Thanks!

Please resend the ep93xx conversion along with the tweak to the core so
we can get that working.  It would also be good to update FSI to use
this library as it's also using dmaengine (the patch having been merged
just before this one it got missed in the updates I expect).

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Lars-Peter Clausen]

On 03/02/2012 02:59 PM, Mark Brown wrote:
> On Wed, Feb 22, 2012 at 10:49:08AM +0100, Lars-Peter Clausen wrote:
>> This patch adds a set of functions which are intended to be used when
>> implementing a dmaengine based sound PCM driver.
> 
> Having this framework is a substantial win and I got bored waiting for
> the repost of this and the subsequent patches (especially given that the
> merge window is drawing nearer).  Since the fix for ep93xx is so simple
> and it works on i.MX I went ahead and applied everything except the
> ep93xx conversion.  Thanks!
> 
> Please resend the ep93xx conversion along with the tweak to the core so
> we can get that working. 

I was at trade show for the rest of last week, without much time for
anything else. I will submit the missing patches in a moment.

> It would also be good to update FSI to use this library as it's also
> using dmaengine (the patch having been merged just before this one it
> got missed in the updates I expect).

I've had a look at the FSI dmaengine driver and it is not straight forward
to convert. For one it does not use cyclic dma transfers so it is more
something for Russel's patch and on the other hand the FSI PCM driver
introduces another layer of indirection which is used to be able to switch
between PIO and DMA. But as a result the functions the dmaengine PCM
framework provides don't really fit anymore.

- Lars

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1181 bytes --]

On Mon, Mar 05, 2012 at 01:30:42PM +0100, Lars-Peter Clausen wrote:
> On 03/02/2012 02:59 PM, Mark Brown wrote:

> > Please resend the ep93xx conversion along with the tweak to the core so
> > we can get that working. 

> I was at trade show for the rest of last week, without much time for
> anything else. I will submit the missing patches in a moment.

No problem, I figured it was something like that and didn't see any
reason not to get the changes into 3.4 - it's a substantial win all
round and the fixes are pretty minior.

> > It would also be good to update FSI to use this library as it's also
> > using dmaengine (the patch having been merged just before this one it
> > got missed in the updates I expect).

> I've had a look at the FSI dmaengine driver and it is not straight forward
> to convert. For one it does not use cyclic dma transfers so it is more
> something for Russel's patch and on the other hand the FSI PCM driver
> introduces another layer of indirection which is used to be able to switch
> between PIO and DMA. But as a result the functions the dmaengine PCM
> framework provides don't really fit anymore.

OK, fair enough.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Kuninori Morimoto]

Hi Lars, Mark

> > > It would also be good to update FSI to use this library as it's also
> > > using dmaengine (the patch having been merged just before this one it
> > > got missed in the updates I expect).
> 
> > I've had a look at the FSI dmaengine driver and it is not straight forward
> > to convert. For one it does not use cyclic dma transfers so it is more
> > something for Russel's patch and on the other hand the FSI PCM driver
> > introduces another layer of indirection which is used to be able to switch
> > between PIO and DMA. But as a result the functions the dmaengine PCM
> > framework provides don't really fit anymore.
> 
> OK, fair enough.

FSI can switch PIO/DMA, but it will be decided on _probe() timing, not runtime switch.
PIO/DMA is depend on SuperH CPU/platform supporting status.

And yes, it doesn't use cyclic dma transfer since I don't know how to use it :)

I forgot detail, but I used sound/soc/sh/siu_pcm.c and sound/soc/txx9/txx9aclc.c
as sample code when I created FSI DMAEngine support.

If ASoC layer supports dmaengine helper functions, I can switch to it by my incremental patch.

Best regards
---
Kuninori Morimoto

Re: [RFC 4/7] ASoC: Add dmaengine PCM helper functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 633 bytes --]

On Tue, Mar 06, 2012 at 04:38:30PM -0800, Kuninori Morimoto wrote:

> And yes, it doesn't use cyclic dma transfer since I don't know how to use it :)

> I forgot detail, but I used sound/soc/sh/siu_pcm.c and sound/soc/txx9/txx9aclc.c
> as sample code when I created FSI DMAEngine support.

> If ASoC layer supports dmaengine helper functions, I can switch to it by my incremental patch.

OK, great - if the hardware supports cyclic mode then I guess the best
thing is to enable that in dmaengine then switch over to using the new
library once that's merged (unless someone comes along and implements
cyclic emulation support first).

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]