All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/3] Initial stab at converting OMAP ASoC support to DMA engine
@ 2012-09-03 16:58 ` Russell King - ARM Linux
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-03 16:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-omap
  Cc: alsa-devel, Jarkko Nikula, Jaroslav Kysela, Liam Girdwood,
	Mark Brown, Peter Ujfalusi, Takashi Iwai, Santosh Shilimkar

The following series of three patches is an attempt to convert the OMAP
ASoC backend to use the DMA engine support.

I'll bring your attention to the comments in patch 3 which highlight
some of the features lost in this process.  Some questions need
answering there (in particular the one concerning pause/resume)
especially as it seems the present driver could well be buggy wrt
comments recently on the mailing list about ALSA pause/resume
requirements.

This works for me, given the wind is in the right direction and if
it's sunny outside.  I find the audio support on OMAP to be rather
flakey at best, requiring reboots if it doesn't work first time.

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

* [RFC 0/3] Initial stab at converting OMAP ASoC support to DMA engine
@ 2012-09-03 16:58 ` Russell King - ARM Linux
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-03 16:58 UTC (permalink / raw)
  To: linux-arm-kernel

The following series of three patches is an attempt to convert the OMAP
ASoC backend to use the DMA engine support.

I'll bring your attention to the comments in patch 3 which highlight
some of the features lost in this process.  Some questions need
answering there (in particular the one concerning pause/resume)
especially as it seems the present driver could well be buggy wrt
comments recently on the mailing list about ALSA pause/resume
requirements.

This works for me, given the wind is in the right direction and if
it's sunny outside.  I find the audio support on OMAP to be rather
flakey at best, requiring reboots if it doesn't work first time.

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

* [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-03 16:58 ` Russell King - ARM Linux
@ 2012-09-03 16:59   ` Liam Girdwood
  -1 siblings, 0 replies; 32+ messages in thread
From: Liam Girdwood @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel, linux-omap
  Cc: Santosh Shilimkar, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
	Mark Brown, alsa-devel

Use a dedicated member to store dmaengine data so that drivers can
use private data for their own purposes.

Signed-off-by: Liam Girdwood <lrg@ti.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/sound/pcm.h           |    2 ++
 sound/soc/soc-dmaengine-pcm.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index cdca2ab..f9e4909 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -269,6 +269,7 @@ struct snd_pcm_hw_constraint_list {
 };
 
 struct snd_pcm_hwptr_log;
+struct dmaengine_pcm_runtime_data;
 
 struct snd_pcm_runtime {
 	/* -- Status -- */
@@ -345,6 +346,7 @@ struct snd_pcm_runtime {
 	unsigned char *dma_area;	/* DMA area */
 	dma_addr_t dma_addr;		/* physical bus address (not accessible from main CPU) */
 	size_t dma_bytes;		/* size of DMA area */
+	struct dmaengine_pcm_runtime_data *dmaengine_data;
 
 	struct snd_dma_buffer *dma_buffer_p;	/* allocated buffer */
 
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 5df529e..27fa5ad 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -40,7 +40,7 @@ struct dmaengine_pcm_runtime_data {
 static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
 	const struct snd_pcm_substream *substream)
 {
-	return substream->runtime->private_data;
+	return substream->runtime->dmaengine_data;
 }
 
 /**
-- 
1.7.4.4


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

* [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-03 16:59   ` Liam Girdwood
  0 siblings, 0 replies; 32+ messages in thread
From: Liam Girdwood @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel

Use a dedicated member to store dmaengine data so that drivers can
use private data for their own purposes.

Signed-off-by: Liam Girdwood <lrg@ti.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 include/sound/pcm.h           |    2 ++
 sound/soc/soc-dmaengine-pcm.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index cdca2ab..f9e4909 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -269,6 +269,7 @@ struct snd_pcm_hw_constraint_list {
 };
 
 struct snd_pcm_hwptr_log;
+struct dmaengine_pcm_runtime_data;
 
 struct snd_pcm_runtime {
 	/* -- Status -- */
@@ -345,6 +346,7 @@ struct snd_pcm_runtime {
 	unsigned char *dma_area;	/* DMA area */
 	dma_addr_t dma_addr;		/* physical bus address (not accessible from main CPU) */
 	size_t dma_bytes;		/* size of DMA area */
+	struct dmaengine_pcm_runtime_data *dmaengine_data;
 
 	struct snd_dma_buffer *dma_buffer_p;	/* allocated buffer */
 
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 5df529e..27fa5ad 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -40,7 +40,7 @@ struct dmaengine_pcm_runtime_data {
 static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
 	const struct snd_pcm_substream *substream)
 {
-	return substream->runtime->private_data;
+	return substream->runtime->dmaengine_data;
 }
 
 /**
-- 
1.7.4.4

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

* [RFC 2/3] Fix "ASoC: dmaengine: Don't use runtime private data for dmaengine data"
  2012-09-03 16:58 ` Russell King - ARM Linux
@ 2012-09-03 16:59   ` Russell King
  -1 siblings, 0 replies; 32+ messages in thread
From: Russell King @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel, linux-omap
  Cc: Santosh Shilimkar, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, alsa-devel

The above commit was not initializing the correct substream runtime
member with the ASoC DMA engine runtime data.  Fix that.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 sound/soc/soc-dmaengine-pcm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 27fa5ad..6fb9a2a 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -293,7 +293,7 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	substream->runtime->private_data = prtd;
+	substream->runtime->dmaengine_data = prtd;
 
 	return 0;
 }
-- 
1.7.4.4


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

* [RFC 2/3] Fix "ASoC: dmaengine: Don't use runtime private data for dmaengine data"
@ 2012-09-03 16:59   ` Russell King
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel

The above commit was not initializing the correct substream runtime
member with the ASoC DMA engine runtime data.  Fix that.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 sound/soc/soc-dmaengine-pcm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 27fa5ad..6fb9a2a 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -293,7 +293,7 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	substream->runtime->private_data = prtd;
+	substream->runtime->dmaengine_data = prtd;
 
 	return 0;
 }
-- 
1.7.4.4

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

* [RFC 3/3] ASoC: first stab at converting OMAP PCM driver to use dmaengine
  2012-09-03 16:58 ` Russell King - ARM Linux
@ 2012-09-03 16:59   ` Russell King
  -1 siblings, 0 replies; 32+ messages in thread
From: Russell King @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel, linux-omap
  Cc: Santosh Shilimkar, Peter Ujfalusi, Jarkko Nikula, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel

Note that certain features of the original driver are not supported:
1. Non-packet mode sync
2. No period wakeup mode
   DMA engine has no way to communicate this information through
   standard channels.

3. Pause
4. Resume
   OMAP DMA engine backend does not support pausing and resuming
   an in-progress transfer.  It is unclear from the specs what
   effect clearing the enable bit has on the DMA position of a
   destination synchronized transfer, and whether the transfer
   can be restarted from the exact point that it was paused (or
   whether the data in the FIFO read from memory is simply
   discarded.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 sound/soc/omap/Kconfig    |    1 +
 sound/soc/omap/omap-pcm.c |  173 ++++++++++++--------------------------------
 2 files changed, 48 insertions(+), 126 deletions(-)

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index 57a2fa7..8a92d5b 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -1,6 +1,7 @@
 config SND_OMAP_SOC
 	tristate "SoC Audio for the Texas Instruments OMAP chips"
 	depends on ARCH_OMAP
+	select SND_SOC_DMAENGINE_PCM
 
 config SND_OMAP_SOC_DMIC
 	tristate
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index f0feb06..50ae048 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -25,21 +25,23 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/omap-dma.h>
 #include <sound/core.h>
+#include <sound/dmaengine_pcm.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 
-#include <plat/dma.h>
+#include <plat/dma.h> /* needed just for OMAP_DMA_SYNC_PACKET */
 #include "omap-pcm.h"
 
 static const struct snd_pcm_hardware omap_pcm_hardware = {
 	.info			= SNDRV_PCM_INFO_MMAP |
 				  SNDRV_PCM_INFO_MMAP_VALID |
-				  SNDRV_PCM_INFO_INTERLEAVED |
+				  SNDRV_PCM_INFO_INTERLEAVED /* |
 				  SNDRV_PCM_INFO_PAUSE |
 				  SNDRV_PCM_INFO_RESUME |
-				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
+				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP */,
 	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 	.period_bytes_min	= 32,
@@ -50,51 +52,9 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
 };
 
 struct omap_runtime_data {
-	spinlock_t			lock;
 	struct omap_pcm_dma_data	*dma_data;
-	int				dma_ch;
-	int				period_index;
 };
 
-static void omap_pcm_dma_irq(int ch, u16 stat, void *data)
-{
-	struct snd_pcm_substream *substream = data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	unsigned long flags;
-
-	if ((cpu_is_omap1510())) {
-		/*
-		 * OMAP1510 doesn't fully support DMA progress counter
-		 * and there is no software emulation implemented yet,
-		 * so have to maintain our own progress counters
-		 * that can be used by omap_pcm_pointer() instead.
-		 */
-		spin_lock_irqsave(&prtd->lock, flags);
-		if ((stat == OMAP_DMA_LAST_IRQ) &&
-				(prtd->period_index == runtime->periods - 1)) {
-			/* we are in sync, do nothing */
-			spin_unlock_irqrestore(&prtd->lock, flags);
-			return;
-		}
-		if (prtd->period_index >= 0) {
-			if (stat & OMAP_DMA_BLOCK_IRQ) {
-				/* end of buffer reached, loop back */
-				prtd->period_index = 0;
-			} else if (stat & OMAP_DMA_LAST_IRQ) {
-				/* update the counter for the last period */
-				prtd->period_index = runtime->periods - 1;
-			} else if (++prtd->period_index >= runtime->periods) {
-				/* end of buffer missed? loop back */
-				prtd->period_index = 0;
-			}
-		}
-		spin_unlock_irqrestore(&prtd->lock, flags);
-	}
-
-	snd_pcm_period_elapsed(substream);
-}
-
 /* this may get called several times by oss emulation */
 static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
@@ -103,6 +63,9 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct omap_runtime_data *prtd = runtime->private_data;
 	struct omap_pcm_dma_data *dma_data;
+	struct dma_slave_config config;
+	struct dma_chan *chan;
+	unsigned req;
 
 	int err = 0;
 
@@ -119,17 +82,36 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 	if (prtd->dma_data)
 		return 0;
 	prtd->dma_data = dma_data;
-	err = omap_request_dma(dma_data->dma_req, dma_data->name,
-			       omap_pcm_dma_irq, substream, &prtd->dma_ch);
-	if (!err) {
-		/*
-		 * Link channel with itself so DMA doesn't need any
-		 * reprogramming while looping the buffer
-		 */
-		omap_dma_link_lch(prtd->dma_ch, prtd->dma_ch);
+
+	/*
+	 * This is the only parameter we don't handle with DMA
+	 * engine - so we insist on OMAP_DMA_SYNC_PACKET here.
+	 */
+	if (dma_data->sync_mode != OMAP_DMA_SYNC_PACKET) {
+		pr_warn("ALSA: omap-dma: DMA using non-packet mode?\n");
+		return -EINVAL;
 	}
 
-	return err;
+	req = dma_data->dma_req;
+	err = snd_dmaengine_pcm_open(substream, omap_dma_filter_fn, &req);
+	if (err)
+		return err;
+
+	chan = snd_dmaengine_pcm_get_chan(substream);
+	if (!chan)
+		return -EINVAL;
+
+	/* fills in addr_width and direction */
+	err = snd_hwparams_to_dma_slave_config(substream, params, &config);
+	if (err)
+		return err;
+
+	config.src_addr = dma_data->port_addr;
+	config.dst_addr = dma_data->port_addr;
+	config.src_maxburst = dma_data->packet_size;
+	config.dst_maxburst = dma_data->packet_size;
+
+	return dmaengine_slave_config(chan, &config);
 }
 
 static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
@@ -140,8 +122,6 @@ static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
 	if (prtd->dma_data == NULL)
 		return 0;
 
-	omap_dma_unlink_lch(prtd->dma_ch, prtd->dma_ch);
-	omap_free_dma(prtd->dma_ch);
 	prtd->dma_data = NULL;
 
 	snd_pcm_set_runtime_buffer(substream, NULL);
@@ -151,48 +131,12 @@ static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
 
 static int omap_pcm_prepare(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	struct omap_pcm_dma_data *dma_data = prtd->dma_data;
-	struct omap_dma_channel_params dma_params;
-	int bytes;
-
-	/* return if this is a bufferless transfer e.g.
-	 * codec <--> BT codec or GSM modem -- lg FIXME */
-	if (!prtd->dma_data)
-		return 0;
-
-	memset(&dma_params, 0, sizeof(dma_params));
-	dma_params.data_type			= dma_data->data_type;
-	dma_params.trigger			= dma_data->dma_req;
-	dma_params.sync_mode			= dma_data->sync_mode;
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		dma_params.src_amode		= OMAP_DMA_AMODE_POST_INC;
-		dma_params.dst_amode		= OMAP_DMA_AMODE_CONSTANT;
-		dma_params.src_or_dst_synch	= OMAP_DMA_DST_SYNC;
-		dma_params.src_start		= runtime->dma_addr;
-		dma_params.dst_start		= dma_data->port_addr;
-		dma_params.dst_port		= OMAP_DMA_PORT_MPUI;
-		dma_params.dst_fi		= dma_data->packet_size;
-	} else {
-		dma_params.src_amode		= OMAP_DMA_AMODE_CONSTANT;
-		dma_params.dst_amode		= OMAP_DMA_AMODE_POST_INC;
-		dma_params.src_or_dst_synch	= OMAP_DMA_SRC_SYNC;
-		dma_params.src_start		= dma_data->port_addr;
-		dma_params.dst_start		= runtime->dma_addr;
-		dma_params.src_port		= OMAP_DMA_PORT_MPUI;
-		dma_params.src_fi		= dma_data->packet_size;
-	}
+#if 0
 	/*
-	 * Set DMA transfer frame size equal to ALSA period size and frame
-	 * count as no. of ALSA periods. Then with DMA frame interrupt enabled,
-	 * we can transfer the whole ALSA buffer with single DMA transfer but
-	 * still can get an interrupt at each period bounary
+	 * These remains serve to document the bits which
+	 * DMA engine doesn't handle.
 	 */
-	bytes = snd_pcm_lib_period_bytes(substream);
-	dma_params.elem_count	= bytes >> dma_data->data_type;
-	dma_params.frame_count	= runtime->periods;
-	omap_set_dma_params(prtd->dma_ch, &dma_params);
+	dma_params.sync_mode			= dma_data->sync_mode;
 
 	if ((cpu_is_omap1510()))
 		omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ |
@@ -207,14 +151,7 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream)
 		 */
 		omap_disable_dma_irq(prtd->dma_ch, OMAP_DMA_BLOCK_IRQ);
 	}
-
-	if (!(cpu_class_is_omap1())) {
-		omap_set_dma_src_burst_mode(prtd->dma_ch,
-						OMAP_DMA_DATA_BURST_16);
-		omap_set_dma_dest_burst_mode(prtd->dma_ch,
-						OMAP_DMA_DATA_BURST_16);
-	}
-
+#endif
 	return 0;
 }
 
@@ -223,56 +160,41 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct omap_runtime_data *prtd = runtime->private_data;
 	struct omap_pcm_dma_data *dma_data = prtd->dma_data;
-	unsigned long flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&prtd->lock, flags);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		prtd->period_index = 0;
 		/* Configure McBSP internal buffer usage */
 		if (dma_data->set_threshold)
 			dma_data->set_threshold(substream);
-
-		omap_start_dma(prtd->dma_ch);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		prtd->period_index = -1;
-		omap_stop_dma(prtd->dma_ch);
 		break;
 	default:
 		ret = -EINVAL;
 	}
-	spin_unlock_irqrestore(&prtd->lock, flags);
+
+	if (ret == 0)
+	    ret = snd_dmaengine_pcm_trigger(substream, cmd);
 
 	return ret;
 }
 
 static snd_pcm_uframes_t omap_pcm_pointer(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	dma_addr_t ptr;
 	snd_pcm_uframes_t offset;
 
 	if (cpu_is_omap1510()) {
-		offset = prtd->period_index * runtime->period_size;
-	} else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		ptr = omap_get_dma_dst_pos(prtd->dma_ch);
-		offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
+		offset = snd_dmaengine_pcm_pointer_no_residue(substream);
 	} else {
-		ptr = omap_get_dma_src_pos(prtd->dma_ch);
-		offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
+		offset = snd_dmaengine_pcm_pointer(substream);
 	}
 
-	if (offset >= runtime->buffer_size)
-		offset = 0;
-
 	return offset;
 }
 
@@ -295,7 +217,6 @@ static int omap_pcm_open(struct snd_pcm_substream *substream)
 		ret = -ENOMEM;
 		goto out;
 	}
-	spin_lock_init(&prtd->lock);
 	runtime->private_data = prtd;
 
 out:
@@ -307,7 +228,7 @@ static int omap_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	kfree(runtime->private_data);
-	return 0;
+	return snd_dmaengine_pcm_close(substream);
 }
 
 static int omap_pcm_mmap(struct snd_pcm_substream *substream,
-- 
1.7.4.4


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

* [RFC 3/3] ASoC: first stab at converting OMAP PCM driver to use dmaengine
@ 2012-09-03 16:59   ` Russell King
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King @ 2012-09-03 16:59 UTC (permalink / raw)
  To: linux-arm-kernel

Note that certain features of the original driver are not supported:
1. Non-packet mode sync
2. No period wakeup mode
   DMA engine has no way to communicate this information through
   standard channels.

3. Pause
4. Resume
   OMAP DMA engine backend does not support pausing and resuming
   an in-progress transfer.  It is unclear from the specs what
   effect clearing the enable bit has on the DMA position of a
   destination synchronized transfer, and whether the transfer
   can be restarted from the exact point that it was paused (or
   whether the data in the FIFO read from memory is simply
   discarded.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 sound/soc/omap/Kconfig    |    1 +
 sound/soc/omap/omap-pcm.c |  173 ++++++++++++--------------------------------
 2 files changed, 48 insertions(+), 126 deletions(-)

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index 57a2fa7..8a92d5b 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -1,6 +1,7 @@
 config SND_OMAP_SOC
 	tristate "SoC Audio for the Texas Instruments OMAP chips"
 	depends on ARCH_OMAP
+	select SND_SOC_DMAENGINE_PCM
 
 config SND_OMAP_SOC_DMIC
 	tristate
diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index f0feb06..50ae048 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -25,21 +25,23 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/omap-dma.h>
 #include <sound/core.h>
+#include <sound/dmaengine_pcm.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 
-#include <plat/dma.h>
+#include <plat/dma.h> /* needed just for OMAP_DMA_SYNC_PACKET */
 #include "omap-pcm.h"
 
 static const struct snd_pcm_hardware omap_pcm_hardware = {
 	.info			= SNDRV_PCM_INFO_MMAP |
 				  SNDRV_PCM_INFO_MMAP_VALID |
-				  SNDRV_PCM_INFO_INTERLEAVED |
+				  SNDRV_PCM_INFO_INTERLEAVED /* |
 				  SNDRV_PCM_INFO_PAUSE |
 				  SNDRV_PCM_INFO_RESUME |
-				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
+				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP */,
 	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
 				  SNDRV_PCM_FMTBIT_S32_LE,
 	.period_bytes_min	= 32,
@@ -50,51 +52,9 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
 };
 
 struct omap_runtime_data {
-	spinlock_t			lock;
 	struct omap_pcm_dma_data	*dma_data;
-	int				dma_ch;
-	int				period_index;
 };
 
-static void omap_pcm_dma_irq(int ch, u16 stat, void *data)
-{
-	struct snd_pcm_substream *substream = data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	unsigned long flags;
-
-	if ((cpu_is_omap1510())) {
-		/*
-		 * OMAP1510 doesn't fully support DMA progress counter
-		 * and there is no software emulation implemented yet,
-		 * so have to maintain our own progress counters
-		 * that can be used by omap_pcm_pointer() instead.
-		 */
-		spin_lock_irqsave(&prtd->lock, flags);
-		if ((stat == OMAP_DMA_LAST_IRQ) &&
-				(prtd->period_index == runtime->periods - 1)) {
-			/* we are in sync, do nothing */
-			spin_unlock_irqrestore(&prtd->lock, flags);
-			return;
-		}
-		if (prtd->period_index >= 0) {
-			if (stat & OMAP_DMA_BLOCK_IRQ) {
-				/* end of buffer reached, loop back */
-				prtd->period_index = 0;
-			} else if (stat & OMAP_DMA_LAST_IRQ) {
-				/* update the counter for the last period */
-				prtd->period_index = runtime->periods - 1;
-			} else if (++prtd->period_index >= runtime->periods) {
-				/* end of buffer missed? loop back */
-				prtd->period_index = 0;
-			}
-		}
-		spin_unlock_irqrestore(&prtd->lock, flags);
-	}
-
-	snd_pcm_period_elapsed(substream);
-}
-
 /* this may get called several times by oss emulation */
 static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
@@ -103,6 +63,9 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct omap_runtime_data *prtd = runtime->private_data;
 	struct omap_pcm_dma_data *dma_data;
+	struct dma_slave_config config;
+	struct dma_chan *chan;
+	unsigned req;
 
 	int err = 0;
 
@@ -119,17 +82,36 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 	if (prtd->dma_data)
 		return 0;
 	prtd->dma_data = dma_data;
-	err = omap_request_dma(dma_data->dma_req, dma_data->name,
-			       omap_pcm_dma_irq, substream, &prtd->dma_ch);
-	if (!err) {
-		/*
-		 * Link channel with itself so DMA doesn't need any
-		 * reprogramming while looping the buffer
-		 */
-		omap_dma_link_lch(prtd->dma_ch, prtd->dma_ch);
+
+	/*
+	 * This is the only parameter we don't handle with DMA
+	 * engine - so we insist on OMAP_DMA_SYNC_PACKET here.
+	 */
+	if (dma_data->sync_mode != OMAP_DMA_SYNC_PACKET) {
+		pr_warn("ALSA: omap-dma: DMA using non-packet mode?\n");
+		return -EINVAL;
 	}
 
-	return err;
+	req = dma_data->dma_req;
+	err = snd_dmaengine_pcm_open(substream, omap_dma_filter_fn, &req);
+	if (err)
+		return err;
+
+	chan = snd_dmaengine_pcm_get_chan(substream);
+	if (!chan)
+		return -EINVAL;
+
+	/* fills in addr_width and direction */
+	err = snd_hwparams_to_dma_slave_config(substream, params, &config);
+	if (err)
+		return err;
+
+	config.src_addr = dma_data->port_addr;
+	config.dst_addr = dma_data->port_addr;
+	config.src_maxburst = dma_data->packet_size;
+	config.dst_maxburst = dma_data->packet_size;
+
+	return dmaengine_slave_config(chan, &config);
 }
 
 static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
@@ -140,8 +122,6 @@ static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
 	if (prtd->dma_data == NULL)
 		return 0;
 
-	omap_dma_unlink_lch(prtd->dma_ch, prtd->dma_ch);
-	omap_free_dma(prtd->dma_ch);
 	prtd->dma_data = NULL;
 
 	snd_pcm_set_runtime_buffer(substream, NULL);
@@ -151,48 +131,12 @@ static int omap_pcm_hw_free(struct snd_pcm_substream *substream)
 
 static int omap_pcm_prepare(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	struct omap_pcm_dma_data *dma_data = prtd->dma_data;
-	struct omap_dma_channel_params dma_params;
-	int bytes;
-
-	/* return if this is a bufferless transfer e.g.
-	 * codec <--> BT codec or GSM modem -- lg FIXME */
-	if (!prtd->dma_data)
-		return 0;
-
-	memset(&dma_params, 0, sizeof(dma_params));
-	dma_params.data_type			= dma_data->data_type;
-	dma_params.trigger			= dma_data->dma_req;
-	dma_params.sync_mode			= dma_data->sync_mode;
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		dma_params.src_amode		= OMAP_DMA_AMODE_POST_INC;
-		dma_params.dst_amode		= OMAP_DMA_AMODE_CONSTANT;
-		dma_params.src_or_dst_synch	= OMAP_DMA_DST_SYNC;
-		dma_params.src_start		= runtime->dma_addr;
-		dma_params.dst_start		= dma_data->port_addr;
-		dma_params.dst_port		= OMAP_DMA_PORT_MPUI;
-		dma_params.dst_fi		= dma_data->packet_size;
-	} else {
-		dma_params.src_amode		= OMAP_DMA_AMODE_CONSTANT;
-		dma_params.dst_amode		= OMAP_DMA_AMODE_POST_INC;
-		dma_params.src_or_dst_synch	= OMAP_DMA_SRC_SYNC;
-		dma_params.src_start		= dma_data->port_addr;
-		dma_params.dst_start		= runtime->dma_addr;
-		dma_params.src_port		= OMAP_DMA_PORT_MPUI;
-		dma_params.src_fi		= dma_data->packet_size;
-	}
+#if 0
 	/*
-	 * Set DMA transfer frame size equal to ALSA period size and frame
-	 * count as no. of ALSA periods. Then with DMA frame interrupt enabled,
-	 * we can transfer the whole ALSA buffer with single DMA transfer but
-	 * still can get an interrupt at each period bounary
+	 * These remains serve to document the bits which
+	 * DMA engine doesn't handle.
 	 */
-	bytes = snd_pcm_lib_period_bytes(substream);
-	dma_params.elem_count	= bytes >> dma_data->data_type;
-	dma_params.frame_count	= runtime->periods;
-	omap_set_dma_params(prtd->dma_ch, &dma_params);
+	dma_params.sync_mode			= dma_data->sync_mode;
 
 	if ((cpu_is_omap1510()))
 		omap_enable_dma_irq(prtd->dma_ch, OMAP_DMA_FRAME_IRQ |
@@ -207,14 +151,7 @@ static int omap_pcm_prepare(struct snd_pcm_substream *substream)
 		 */
 		omap_disable_dma_irq(prtd->dma_ch, OMAP_DMA_BLOCK_IRQ);
 	}
-
-	if (!(cpu_class_is_omap1())) {
-		omap_set_dma_src_burst_mode(prtd->dma_ch,
-						OMAP_DMA_DATA_BURST_16);
-		omap_set_dma_dest_burst_mode(prtd->dma_ch,
-						OMAP_DMA_DATA_BURST_16);
-	}
-
+#endif
 	return 0;
 }
 
@@ -223,56 +160,41 @@ static int omap_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct omap_runtime_data *prtd = runtime->private_data;
 	struct omap_pcm_dma_data *dma_data = prtd->dma_data;
-	unsigned long flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&prtd->lock, flags);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		prtd->period_index = 0;
 		/* Configure McBSP internal buffer usage */
 		if (dma_data->set_threshold)
 			dma_data->set_threshold(substream);
-
-		omap_start_dma(prtd->dma_ch);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		prtd->period_index = -1;
-		omap_stop_dma(prtd->dma_ch);
 		break;
 	default:
 		ret = -EINVAL;
 	}
-	spin_unlock_irqrestore(&prtd->lock, flags);
+
+	if (ret == 0)
+	    ret = snd_dmaengine_pcm_trigger(substream, cmd);
 
 	return ret;
 }
 
 static snd_pcm_uframes_t omap_pcm_pointer(struct snd_pcm_substream *substream)
 {
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct omap_runtime_data *prtd = runtime->private_data;
-	dma_addr_t ptr;
 	snd_pcm_uframes_t offset;
 
 	if (cpu_is_omap1510()) {
-		offset = prtd->period_index * runtime->period_size;
-	} else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		ptr = omap_get_dma_dst_pos(prtd->dma_ch);
-		offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
+		offset = snd_dmaengine_pcm_pointer_no_residue(substream);
 	} else {
-		ptr = omap_get_dma_src_pos(prtd->dma_ch);
-		offset = bytes_to_frames(runtime, ptr - runtime->dma_addr);
+		offset = snd_dmaengine_pcm_pointer(substream);
 	}
 
-	if (offset >= runtime->buffer_size)
-		offset = 0;
-
 	return offset;
 }
 
@@ -295,7 +217,6 @@ static int omap_pcm_open(struct snd_pcm_substream *substream)
 		ret = -ENOMEM;
 		goto out;
 	}
-	spin_lock_init(&prtd->lock);
 	runtime->private_data = prtd;
 
 out:
@@ -307,7 +228,7 @@ static int omap_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	kfree(runtime->private_data);
-	return 0;
+	return snd_dmaengine_pcm_close(substream);
 }
 
 static int omap_pcm_mmap(struct snd_pcm_substream *substream,
-- 
1.7.4.4

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

* Re: [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-03 16:59   ` Liam Girdwood
@ 2012-09-03 20:25     ` Lars-Peter Clausen
  -1 siblings, 0 replies; 32+ messages in thread
From: Lars-Peter Clausen @ 2012-09-03 20:25 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: alsa-devel, Takashi Iwai, Mark Brown, Santosh Shilimkar,
	linux-omap, linux-arm-kernel

On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> Use a dedicated member to store dmaengine data so that drivers can
> use private data for their own purposes.
> 

The idea was that we'll eventually get to a point where we won't need private
data for the drivers using the generic dmaengine code. But for the transitional
period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
private data to the dmaengine pcm. For an example see how the other users of
dmaengine pcm handle this.

> Signed-off-by: Liam Girdwood <lrg@ti.com>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  include/sound/pcm.h           |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |    2 +-
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index cdca2ab..f9e4909 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -269,6 +269,7 @@ struct snd_pcm_hw_constraint_list {
>  };
>  
>  struct snd_pcm_hwptr_log;
> +struct dmaengine_pcm_runtime_data;
>  
>  struct snd_pcm_runtime {
>  	/* -- Status -- */
> @@ -345,6 +346,7 @@ struct snd_pcm_runtime {
>  	unsigned char *dma_area;	/* DMA area */
>  	dma_addr_t dma_addr;		/* physical bus address (not accessible from main CPU) */
>  	size_t dma_bytes;		/* size of DMA area */
> +	struct dmaengine_pcm_runtime_data *dmaengine_data;
>  
>  	struct snd_dma_buffer *dma_buffer_p;	/* allocated buffer */
>  
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 5df529e..27fa5ad 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -40,7 +40,7 @@ struct dmaengine_pcm_runtime_data {
>  static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
>  	const struct snd_pcm_substream *substream)
>  {
> -	return substream->runtime->private_data;
> +	return substream->runtime->dmaengine_data;
>  }
>  
>  /**

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-03 20:25     ` Lars-Peter Clausen
  0 siblings, 0 replies; 32+ messages in thread
From: Lars-Peter Clausen @ 2012-09-03 20:25 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> Use a dedicated member to store dmaengine data so that drivers can
> use private data for their own purposes.
> 

The idea was that we'll eventually get to a point where we won't need private
data for the drivers using the generic dmaengine code. But for the transitional
period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
private data to the dmaengine pcm. For an example see how the other users of
dmaengine pcm handle this.

> Signed-off-by: Liam Girdwood <lrg@ti.com>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  include/sound/pcm.h           |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |    2 +-
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index cdca2ab..f9e4909 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -269,6 +269,7 @@ struct snd_pcm_hw_constraint_list {
>  };
>  
>  struct snd_pcm_hwptr_log;
> +struct dmaengine_pcm_runtime_data;
>  
>  struct snd_pcm_runtime {
>  	/* -- Status -- */
> @@ -345,6 +346,7 @@ struct snd_pcm_runtime {
>  	unsigned char *dma_area;	/* DMA area */
>  	dma_addr_t dma_addr;		/* physical bus address (not accessible from main CPU) */
>  	size_t dma_bytes;		/* size of DMA area */
> +	struct dmaengine_pcm_runtime_data *dmaengine_data;
>  
>  	struct snd_dma_buffer *dma_buffer_p;	/* allocated buffer */
>  
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 5df529e..27fa5ad 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -40,7 +40,7 @@ struct dmaengine_pcm_runtime_data {
>  static inline struct dmaengine_pcm_runtime_data *substream_to_prtd(
>  	const struct snd_pcm_substream *substream)
>  {
> -	return substream->runtime->private_data;
> +	return substream->runtime->dmaengine_data;
>  }
>  
>  /**

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

* Re: [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-03 20:25     ` [alsa-devel] " Lars-Peter Clausen
@ 2012-09-03 20:43       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-03 20:43 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: alsa-devel, Takashi Iwai, Mark Brown, Santosh Shilimkar,
	linux-omap, Liam Girdwood, linux-arm-kernel

On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> > Use a dedicated member to store dmaengine data so that drivers can
> > use private data for their own purposes.
> > 
> 
> The idea was that we'll eventually get to a point where we won't need private
> data for the drivers using the generic dmaengine code. But for the transitional
> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
> private data to the dmaengine pcm. For an example see how the other users of
> dmaengine pcm handle this.

That's fine if you are writing new drivers from scatch, or know the
driver you're converting inside-out.  Neither applies here (I've
struggled to do anything with the OMAP audio stuff for many many
reasons.)

I rather wish that people who did know the OMAP ASoC driver had stepped
up to this conversion, but alas they haven't.

In any case, if you want people to use the this soc-dmaengine helper
then you have to make the conversion to it simple, and requiring
everyone to totally restructure their drivers to use it does not make
that process simple.

What you have here is the result of several transformations to the
driver, which would _not_ have been possible without this first patch
from Liam.

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-03 20:43       ` Russell King - ARM Linux
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-03 20:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> > Use a dedicated member to store dmaengine data so that drivers can
> > use private data for their own purposes.
> > 
> 
> The idea was that we'll eventually get to a point where we won't need private
> data for the drivers using the generic dmaengine code. But for the transitional
> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
> private data to the dmaengine pcm. For an example see how the other users of
> dmaengine pcm handle this.

That's fine if you are writing new drivers from scatch, or know the
driver you're converting inside-out.  Neither applies here (I've
struggled to do anything with the OMAP audio stuff for many many
reasons.)

I rather wish that people who did know the OMAP ASoC driver had stepped
up to this conversion, but alas they haven't.

In any case, if you want people to use the this soc-dmaengine helper
then you have to make the conversion to it simple, and requiring
everyone to totally restructure their drivers to use it does not make
that process simple.

What you have here is the result of several transformations to the
driver, which would _not_ have been possible without this first patch
from Liam.

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

* Re: [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-03 20:43       ` [alsa-devel] " Russell King - ARM Linux
@ 2012-09-03 20:59         ` Lars-Peter Clausen
  -1 siblings, 0 replies; 32+ messages in thread
From: Lars-Peter Clausen @ 2012-09-03 20:59 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: alsa-devel, Takashi Iwai, Mark Brown, Santosh Shilimkar,
	linux-omap, Liam Girdwood, linux-arm-kernel

On 09/03/2012 10:43 PM, Russell King - ARM Linux wrote:
> On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
>> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
>>> Use a dedicated member to store dmaengine data so that drivers can
>>> use private data for their own purposes.
>>>
>>
>> The idea was that we'll eventually get to a point where we won't need private
>> data for the drivers using the generic dmaengine code. But for the transitional
>> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
>> private data to the dmaengine pcm. For an example see how the other users of
>> dmaengine pcm handle this.
> 
> That's fine if you are writing new drivers from scatch, or know the
> driver you're converting inside-out.  Neither applies here (I've
> struggled to do anything with the OMAP audio stuff for many many
> reasons.)
> 
> I rather wish that people who did know the OMAP ASoC driver had stepped
> up to this conversion, but alas they haven't.
> 
> In any case, if you want people to use the this soc-dmaengine helper
> then you have to make the conversion to it simple, and requiring
> everyone to totally restructure their drivers to use it does not make
> that process simple.
> 
> What you have here is the result of several transformations to the
> driver, which would _not_ have been possible without this first patch
> from Liam.

Ok, it might have been helpful in the conversion process, but for the final
patch it would be nice if you could replace

struct snd_pcm_runtime *runtime = substream->runtime;
struct omap_runtime_data *prtd = runtime->private_data;
struct omap_pcm_dma_data *dma_data = prtd->dma_data;
with
struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);

and in the hwparams callback use

snd_dmaengine_pcm_set_data(substream, dma_data);

and then drop patch 1 and 2 from the series.

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-03 20:59         ` Lars-Peter Clausen
  0 siblings, 0 replies; 32+ messages in thread
From: Lars-Peter Clausen @ 2012-09-03 20:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/03/2012 10:43 PM, Russell King - ARM Linux wrote:
> On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
>> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
>>> Use a dedicated member to store dmaengine data so that drivers can
>>> use private data for their own purposes.
>>>
>>
>> The idea was that we'll eventually get to a point where we won't need private
>> data for the drivers using the generic dmaengine code. But for the transitional
>> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
>> private data to the dmaengine pcm. For an example see how the other users of
>> dmaengine pcm handle this.
> 
> That's fine if you are writing new drivers from scatch, or know the
> driver you're converting inside-out.  Neither applies here (I've
> struggled to do anything with the OMAP audio stuff for many many
> reasons.)
> 
> I rather wish that people who did know the OMAP ASoC driver had stepped
> up to this conversion, but alas they haven't.
> 
> In any case, if you want people to use the this soc-dmaengine helper
> then you have to make the conversion to it simple, and requiring
> everyone to totally restructure their drivers to use it does not make
> that process simple.
> 
> What you have here is the result of several transformations to the
> driver, which would _not_ have been possible without this first patch
> from Liam.

Ok, it might have been helpful in the conversion process, but for the final
patch it would be nice if you could replace

struct snd_pcm_runtime *runtime = substream->runtime;
struct omap_runtime_data *prtd = runtime->private_data;
struct omap_pcm_dma_data *dma_data = prtd->dma_data;
with
struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);

and in the hwparams callback use

snd_dmaengine_pcm_set_data(substream, dma_data);

and then drop patch 1 and 2 from the series.

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

* [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
  2012-09-03 16:59   ` Russell King
@ 2012-09-04 12:08     ` Peter Ujfalusi
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: Russell King, Janusz Krzysztofik
  Cc: Liam Girdwood, Mark Brown, Jarkko Nikula, alsa-devel, linux-omap,
	linux-arm-kernel, Santosh Shilimkar

Hi Russell,

Enable the element mode (thus allowing mono playback and probably unblocking
OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.

Janusz: would it be possible for you to test Russell's series plus this on
OMAP1 to make sure that we do not broke it?

Russell: we should wait for Janusz to test this on OMAP1. Based on the feedback
we can plan on how to proceed with the dmaengine for OMAP audio.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: omap: Support for element mode in cyclic DMA
  ASoC: omap-pcm: Do not check DMA sync_mode

 drivers/dma/omap-dma.c    |  5 ++++-
 sound/soc/omap/omap-pcm.c | 10 ----------
 2 files changed, 4 insertions(+), 11 deletions(-)

-- 
1.7.12


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

* [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
@ 2012-09-04 12:08     ` Peter Ujfalusi
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

Enable the element mode (thus allowing mono playback and probably unblocking
OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.

Janusz: would it be possible for you to test Russell's series plus this on
OMAP1 to make sure that we do not broke it?

Russell: we should wait for Janusz to test this on OMAP1. Based on the feedback
we can plan on how to proceed with the dmaengine for OMAP audio.

Regards,
Peter
---
Peter Ujfalusi (2):
  dmaengine: omap: Support for element mode in cyclic DMA
  ASoC: omap-pcm: Do not check DMA sync_mode

 drivers/dma/omap-dma.c    |  5 ++++-
 sound/soc/omap/omap-pcm.c | 10 ----------
 2 files changed, 4 insertions(+), 11 deletions(-)

-- 
1.7.12

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

* [RFC update 1/2] dmaengine: omap: Support for element mode in cyclic DMA
  2012-09-04 12:08     ` Peter Ujfalusi
@ 2012-09-04 12:08       ` Peter Ujfalusi
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: Russell King, Janusz Krzysztofik
  Cc: Liam Girdwood, Mark Brown, Jarkko Nikula, alsa-devel, linux-omap,
	linux-arm-kernel, Santosh Shilimkar

When src_maxburst/dst_maxburst is set to 0 by the users of cyclic DMA
(mostly audio) indicates that we should configure the omap DMA to element
sync mode instead of packet mode.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/omap-dma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index ae05618..b77a40d 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -413,7 +413,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic(
 	d->dev_addr = dev_addr;
 	d->fi = burst;
 	d->es = es;
-	d->sync_mode = OMAP_DMA_SYNC_PACKET;
+	if (burst)
+		d->sync_mode = OMAP_DMA_SYNC_PACKET;
+	else
+		d->sync_mode = OMAP_DMA_SYNC_ELEMENT;
 	d->sync_type = sync_type;
 	d->periph_port = OMAP_DMA_PORT_MPUI;
 	d->sg[0].addr = buf_addr;
-- 
1.7.12


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

* [RFC update 1/2] dmaengine: omap: Support for element mode in cyclic DMA
@ 2012-09-04 12:08       ` Peter Ujfalusi
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

When src_maxburst/dst_maxburst is set to 0 by the users of cyclic DMA
(mostly audio) indicates that we should configure the omap DMA to element
sync mode instead of packet mode.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/omap-dma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index ae05618..b77a40d 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -413,7 +413,10 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic(
 	d->dev_addr = dev_addr;
 	d->fi = burst;
 	d->es = es;
-	d->sync_mode = OMAP_DMA_SYNC_PACKET;
+	if (burst)
+		d->sync_mode = OMAP_DMA_SYNC_PACKET;
+	else
+		d->sync_mode = OMAP_DMA_SYNC_ELEMENT;
 	d->sync_type = sync_type;
 	d->periph_port = OMAP_DMA_PORT_MPUI;
 	d->sg[0].addr = buf_addr;
-- 
1.7.12

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

* [RFC update 2/2] ASoC: omap-pcm: Do not check DMA sync_mode
  2012-09-04 12:08     ` Peter Ujfalusi
@ 2012-09-04 12:08       ` Peter Ujfalusi
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: Russell King, Janusz Krzysztofik
  Cc: Liam Girdwood, Mark Brown, Jarkko Nikula, alsa-devel, linux-omap,
	linux-arm-kernel, Santosh Shilimkar

OMAP dmaengine now supports element mode also in cyclic DMA so we do not
need to block non PACKET modes.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/omap-pcm.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index 50ae048..0f88db3 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -32,7 +32,6 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 
-#include <plat/dma.h> /* needed just for OMAP_DMA_SYNC_PACKET */
 #include "omap-pcm.h"
 
 static const struct snd_pcm_hardware omap_pcm_hardware = {
@@ -83,15 +82,6 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 		return 0;
 	prtd->dma_data = dma_data;
 
-	/*
-	 * This is the only parameter we don't handle with DMA
-	 * engine - so we insist on OMAP_DMA_SYNC_PACKET here.
-	 */
-	if (dma_data->sync_mode != OMAP_DMA_SYNC_PACKET) {
-		pr_warn("ALSA: omap-dma: DMA using non-packet mode?\n");
-		return -EINVAL;
-	}
-
 	req = dma_data->dma_req;
 	err = snd_dmaengine_pcm_open(substream, omap_dma_filter_fn, &req);
 	if (err)
-- 
1.7.12


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

* [RFC update 2/2] ASoC: omap-pcm: Do not check DMA sync_mode
@ 2012-09-04 12:08       ` Peter Ujfalusi
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 12:08 UTC (permalink / raw)
  To: linux-arm-kernel

OMAP dmaengine now supports element mode also in cyclic DMA so we do not
need to block non PACKET modes.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/omap-pcm.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index 50ae048..0f88db3 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -32,7 +32,6 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 
-#include <plat/dma.h> /* needed just for OMAP_DMA_SYNC_PACKET */
 #include "omap-pcm.h"
 
 static const struct snd_pcm_hardware omap_pcm_hardware = {
@@ -83,15 +82,6 @@ static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 		return 0;
 	prtd->dma_data = dma_data;
 
-	/*
-	 * This is the only parameter we don't handle with DMA
-	 * engine - so we insist on OMAP_DMA_SYNC_PACKET here.
-	 */
-	if (dma_data->sync_mode != OMAP_DMA_SYNC_PACKET) {
-		pr_warn("ALSA: omap-dma: DMA using non-packet mode?\n");
-		return -EINVAL;
-	}
-
 	req = dma_data->dma_req;
 	err = snd_dmaengine_pcm_open(substream, omap_dma_filter_fn, &req);
 	if (err)
-- 
1.7.12

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

* Re: [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-03 20:59         ` [alsa-devel] " Lars-Peter Clausen
@ 2012-09-04 13:14           ` Takashi Iwai
  -1 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-09-04 13:14 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Russell King - ARM Linux, alsa-devel, Mark Brown,
	Santosh Shilimkar, linux-omap, Liam Girdwood, linux-arm-kernel

At Mon, 03 Sep 2012 22:59:54 +0200,
Lars-Peter Clausen wrote:
> 
> On 09/03/2012 10:43 PM, Russell King - ARM Linux wrote:
> > On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
> >> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> >>> Use a dedicated member to store dmaengine data so that drivers can
> >>> use private data for their own purposes.
> >>>
> >>
> >> The idea was that we'll eventually get to a point where we won't need private
> >> data for the drivers using the generic dmaengine code. But for the transitional
> >> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
> >> private data to the dmaengine pcm. For an example see how the other users of
> >> dmaengine pcm handle this.
> > 
> > That's fine if you are writing new drivers from scatch, or know the
> > driver you're converting inside-out.  Neither applies here (I've
> > struggled to do anything with the OMAP audio stuff for many many
> > reasons.)
> > 
> > I rather wish that people who did know the OMAP ASoC driver had stepped
> > up to this conversion, but alas they haven't.
> > 
> > In any case, if you want people to use the this soc-dmaengine helper
> > then you have to make the conversion to it simple, and requiring
> > everyone to totally restructure their drivers to use it does not make
> > that process simple.
> > 
> > What you have here is the result of several transformations to the
> > driver, which would _not_ have been possible without this first patch
> > from Liam.
> 
> Ok, it might have been helpful in the conversion process, but for the final
> patch it would be nice if you could replace
> 
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct omap_runtime_data *prtd = runtime->private_data;
> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
> with
> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
> 
> and in the hwparams callback use
> 
> snd_dmaengine_pcm_set_data(substream, dma_data);
> 
> and then drop patch 1 and 2 from the series.

We discussed with Liam about the addition of new field in ALSA core,
and concluded that a bit different approach, at least, more generic
name is preferred, even if a new field is inevitably needed.

So, eventually some change may happen in near future in ALSA core
side, but still it'd be really helpful if the callers have been
standardized beforehand with a helper function like above.


thanks,

Takashi

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-04 13:14           ` Takashi Iwai
  0 siblings, 0 replies; 32+ messages in thread
From: Takashi Iwai @ 2012-09-04 13:14 UTC (permalink / raw)
  To: linux-arm-kernel

At Mon, 03 Sep 2012 22:59:54 +0200,
Lars-Peter Clausen wrote:
> 
> On 09/03/2012 10:43 PM, Russell King - ARM Linux wrote:
> > On Mon, Sep 03, 2012 at 10:25:49PM +0200, Lars-Peter Clausen wrote:
> >> On 09/03/2012 06:59 PM, Liam Girdwood wrote:
> >>> Use a dedicated member to store dmaengine data so that drivers can
> >>> use private data for their own purposes.
> >>>
> >>
> >> The idea was that we'll eventually get to a point where we won't need private
> >> data for the drivers using the generic dmaengine code. But for the transitional
> >> period there is snd_dmaengine_pcm_{set,get}_data which allows to attach driver
> >> private data to the dmaengine pcm. For an example see how the other users of
> >> dmaengine pcm handle this.
> > 
> > That's fine if you are writing new drivers from scatch, or know the
> > driver you're converting inside-out.  Neither applies here (I've
> > struggled to do anything with the OMAP audio stuff for many many
> > reasons.)
> > 
> > I rather wish that people who did know the OMAP ASoC driver had stepped
> > up to this conversion, but alas they haven't.
> > 
> > In any case, if you want people to use the this soc-dmaengine helper
> > then you have to make the conversion to it simple, and requiring
> > everyone to totally restructure their drivers to use it does not make
> > that process simple.
> > 
> > What you have here is the result of several transformations to the
> > driver, which would _not_ have been possible without this first patch
> > from Liam.
> 
> Ok, it might have been helpful in the conversion process, but for the final
> patch it would be nice if you could replace
> 
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct omap_runtime_data *prtd = runtime->private_data;
> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
> with
> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
> 
> and in the hwparams callback use
> 
> snd_dmaengine_pcm_set_data(substream, dma_data);
> 
> and then drop patch 1 and 2 from the series.

We discussed with Liam about the addition of new field in ALSA core,
and concluded that a bit different approach, at least, more generic
name is preferred, even if a new field is inevitably needed.

So, eventually some change may happen in near future in ALSA core
side, but still it'd be really helpful if the callers have been
standardized beforehand with a helper function like above.


thanks,

Takashi

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

* Re: [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-04 13:14           ` Takashi Iwai
@ 2012-09-04 13:26             ` Peter Ujfalusi
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 13:26 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Lars-Peter Clausen, alsa-devel, Russell King - ARM Linux,
	Mark Brown, Santosh Shilimkar, linux-omap, Liam Girdwood,
	linux-arm-kernel

Hi Takashi,

On 09/04/2012 04:14 PM, Takashi Iwai wrote:
>> Ok, it might have been helpful in the conversion process, but for the final
>> patch it would be nice if you could replace
>>
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> struct omap_runtime_data *prtd = runtime->private_data;
>> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
>> with
>> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
>>
>> and in the hwparams callback use
>>
>> snd_dmaengine_pcm_set_data(substream, dma_data);
>>
>> and then drop patch 1 and 2 from the series.
> 
> We discussed with Liam about the addition of new field in ALSA core,
> and concluded that a bit different approach, at least, more generic
> name is preferred, even if a new field is inevitably needed.
> 
> So, eventually some change may happen in near future in ALSA core
> side, but still it'd be really helpful if the callers have been
> standardized beforehand with a helper function like above.

If the omap-pcm dmaengine conversion works on all OMAP versions (from OMAP1 to
OMAP5) it is possible to avoid the additional field.
My main concern at the moment is if we will need two sets of drivers to
support OMAP1 and OMAP2/3/4/5.
In all case we use the same omap-mcbsp driver which would need deal with two
different type of ASoC platform driver (dmaengine and non-dmaengine).
I hope we get confirmation from Janusz soon regarding to OMAP1 with dmaengine
so we can plan on how to move forward.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-04 13:26             ` Peter Ujfalusi
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-04 13:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Takashi,

On 09/04/2012 04:14 PM, Takashi Iwai wrote:
>> Ok, it might have been helpful in the conversion process, but for the final
>> patch it would be nice if you could replace
>>
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> struct omap_runtime_data *prtd = runtime->private_data;
>> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
>> with
>> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
>>
>> and in the hwparams callback use
>>
>> snd_dmaengine_pcm_set_data(substream, dma_data);
>>
>> and then drop patch 1 and 2 from the series.
> 
> We discussed with Liam about the addition of new field in ALSA core,
> and concluded that a bit different approach, at least, more generic
> name is preferred, even if a new field is inevitably needed.
> 
> So, eventually some change may happen in near future in ALSA core
> side, but still it'd be really helpful if the callers have been
> standardized beforehand with a helper function like above.

If the omap-pcm dmaengine conversion works on all OMAP versions (from OMAP1 to
OMAP5) it is possible to avoid the additional field.
My main concern at the moment is if we will need two sets of drivers to
support OMAP1 and OMAP2/3/4/5.
In all case we use the same omap-mcbsp driver which would need deal with two
different type of ASoC platform driver (dmaengine and non-dmaengine).
I hope we get confirmation from Janusz soon regarding to OMAP1 with dmaengine
so we can plan on how to move forward.

-- 
P?ter

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

* Re: [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
  2012-09-04 13:26             ` Peter Ujfalusi
@ 2012-09-04 18:14               ` Russell King - ARM Linux
  -1 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-04 18:14 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Takashi Iwai, Lars-Peter Clausen, alsa-devel, Mark Brown,
	Santosh Shilimkar, linux-omap, Liam Girdwood, linux-arm-kernel

On Tue, Sep 04, 2012 at 04:26:28PM +0300, Peter Ujfalusi wrote:
> Hi Takashi,
> 
> On 09/04/2012 04:14 PM, Takashi Iwai wrote:
> >> Ok, it might have been helpful in the conversion process, but for the final
> >> patch it would be nice if you could replace
> >>
> >> struct snd_pcm_runtime *runtime = substream->runtime;
> >> struct omap_runtime_data *prtd = runtime->private_data;
> >> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
> >> with
> >> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
> >>
> >> and in the hwparams callback use
> >>
> >> snd_dmaengine_pcm_set_data(substream, dma_data);
> >>
> >> and then drop patch 1 and 2 from the series.
> > 
> > We discussed with Liam about the addition of new field in ALSA core,
> > and concluded that a bit different approach, at least, more generic
> > name is preferred, even if a new field is inevitably needed.
> > 
> > So, eventually some change may happen in near future in ALSA core
> > side, but still it'd be really helpful if the callers have been
> > standardized beforehand with a helper function like above.
> 
> If the omap-pcm dmaengine conversion works on all OMAP versions (from OMAP1 to
> OMAP5) it is possible to avoid the additional field.
> My main concern at the moment is if we will need two sets of drivers to
> support OMAP1 and OMAP2/3/4/5.
> In all case we use the same omap-mcbsp driver which would need deal with two
> different type of ASoC platform driver (dmaengine and non-dmaengine).
> I hope we get confirmation from Janusz soon regarding to OMAP1 with dmaengine
> so we can plan on how to move forward.

As the target for the DMA engine work is to kill off the OMAP private DMA
APIs, then if OMAP1 doesn't work with the OMAP DMA engine driver, that's
what needs fixing, rather than having two ASoC platform drivers.

Note that time is ticking for the removal of the OMAP private DMA APIs (see
feature-removal-schedule.txt) and it has to happen so that the next stage
of the OMAP DMA engine conversion can happen - that is, to make the OMAP
DMA engine support be a proper driver rather than just a DMA Engine to OMAP
private DMA API conversion shim.

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

* [alsa-devel] [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data
@ 2012-09-04 18:14               ` Russell King - ARM Linux
  0 siblings, 0 replies; 32+ messages in thread
From: Russell King - ARM Linux @ 2012-09-04 18:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 04, 2012 at 04:26:28PM +0300, Peter Ujfalusi wrote:
> Hi Takashi,
> 
> On 09/04/2012 04:14 PM, Takashi Iwai wrote:
> >> Ok, it might have been helpful in the conversion process, but for the final
> >> patch it would be nice if you could replace
> >>
> >> struct snd_pcm_runtime *runtime = substream->runtime;
> >> struct omap_runtime_data *prtd = runtime->private_data;
> >> struct omap_pcm_dma_data *dma_data = prtd->dma_data;
> >> with
> >> struct omap_pcm_dma_data *dma_data = snd_dmaengine_pcm_get_data(substream);
> >>
> >> and in the hwparams callback use
> >>
> >> snd_dmaengine_pcm_set_data(substream, dma_data);
> >>
> >> and then drop patch 1 and 2 from the series.
> > 
> > We discussed with Liam about the addition of new field in ALSA core,
> > and concluded that a bit different approach, at least, more generic
> > name is preferred, even if a new field is inevitably needed.
> > 
> > So, eventually some change may happen in near future in ALSA core
> > side, but still it'd be really helpful if the callers have been
> > standardized beforehand with a helper function like above.
> 
> If the omap-pcm dmaengine conversion works on all OMAP versions (from OMAP1 to
> OMAP5) it is possible to avoid the additional field.
> My main concern at the moment is if we will need two sets of drivers to
> support OMAP1 and OMAP2/3/4/5.
> In all case we use the same omap-mcbsp driver which would need deal with two
> different type of ASoC platform driver (dmaengine and non-dmaengine).
> I hope we get confirmation from Janusz soon regarding to OMAP1 with dmaengine
> so we can plan on how to move forward.

As the target for the DMA engine work is to kill off the OMAP private DMA
APIs, then if OMAP1 doesn't work with the OMAP DMA engine driver, that's
what needs fixing, rather than having two ASoC platform drivers.

Note that time is ticking for the removal of the OMAP private DMA APIs (see
feature-removal-schedule.txt) and it has to happen so that the next stage
of the OMAP DMA engine conversion can happen - that is, to make the OMAP
DMA engine support be a proper driver rather than just a DMA Engine to OMAP
private DMA API conversion shim.

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

* Re: [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
  2012-09-04 12:08     ` Peter Ujfalusi
@ 2012-09-04 22:37       ` Janusz Krzysztofik
  -1 siblings, 0 replies; 32+ messages in thread
From: Janusz Krzysztofik @ 2012-09-04 22:37 UTC (permalink / raw)
  To: alsa-devel
  Cc: Mark Brown, Peter Ujfalusi, Santosh Shilimkar, linux-arm-kernel,
	Russell King, linux-omap, Liam Girdwood, Jarkko Nikula

On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
> Hi Russell,
> 
> Enable the element mode (thus allowing mono playback and probably unblocking
> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
> 
> Janusz: would it be possible for you to test Russell's series plus this on
> OMAP1 to make sure that we do not broke it?

Hi Peter, Russell,
I'll be happy to make this test for you. This will take some time (I 
work away from home), but I expect to have it done by Monday.

BTW, I haven't been following OMAP development very closely last weeks, 
and I didn't even know about the OMAP DMA engine availability. Which 
Linux version should I base my test on? Would 3.6-rc be OK? Or l-o 
master?

Thanks,
Janusz

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

* [alsa-devel] [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
@ 2012-09-04 22:37       ` Janusz Krzysztofik
  0 siblings, 0 replies; 32+ messages in thread
From: Janusz Krzysztofik @ 2012-09-04 22:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
> Hi Russell,
> 
> Enable the element mode (thus allowing mono playback and probably unblocking
> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
> 
> Janusz: would it be possible for you to test Russell's series plus this on
> OMAP1 to make sure that we do not broke it?

Hi Peter, Russell,
I'll be happy to make this test for you. This will take some time (I 
work away from home), but I expect to have it done by Monday.

BTW, I haven't been following OMAP development very closely last weeks, 
and I didn't even know about the OMAP DMA engine availability. Which 
Linux version should I base my test on? Would 3.6-rc be OK? Or l-o 
master?

Thanks,
Janusz

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

* Re: [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
  2012-09-04 12:08     ` Peter Ujfalusi
@ 2012-09-09 19:57       ` Janusz Krzysztofik
  -1 siblings, 0 replies; 32+ messages in thread
From: Janusz Krzysztofik @ 2012-09-09 19:57 UTC (permalink / raw)
  To: alsa-devel
  Cc: Mark Brown, Peter Ujfalusi, Santosh Shilimkar, linux-arm-kernel,
	Russell King, linux-omap, Liam Girdwood, Jarkko Nikula

On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
> 
> Enable the element mode (thus allowing mono playback and probably 
unblocking
> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
> 
> Janusz: would it be possible for you to test Russell's series plus 
this on
> OMAP1 to make sure that we do not broke it?

Hi,
I can confirm that sound works for me as before, both capture and 
playback, on my OMAP1 Amstrad Delta with Russell's and Peter's patches 
applied on top of linux-3.6-rc3, with the OMAP DMA engine driver built 
in. I was not able make audible tests with applications other than a 
soft phone as I didn't get back home for this weekend, but I think that 
the asterisk soft phone is quite a demanding use case.

The only thing I'm not sure about is why the sysfs provided 
bytes_transferred values never change from their initial zeros.

For OMAP1:
Tested-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Thanks,
Janusz

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

* [alsa-devel] [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
@ 2012-09-09 19:57       ` Janusz Krzysztofik
  0 siblings, 0 replies; 32+ messages in thread
From: Janusz Krzysztofik @ 2012-09-09 19:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
> 
> Enable the element mode (thus allowing mono playback and probably 
unblocking
> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
> 
> Janusz: would it be possible for you to test Russell's series plus 
this on
> OMAP1 to make sure that we do not broke it?

Hi,
I can confirm that sound works for me as before, both capture and 
playback, on my OMAP1 Amstrad Delta with Russell's and Peter's patches 
applied on top of linux-3.6-rc3, with the OMAP DMA engine driver built 
in. I was not able make audible tests with applications other than a 
soft phone as I didn't get back home for this weekend, but I think that 
the asterisk soft phone is quite a demanding use case.

The only thing I'm not sure about is why the sysfs provided 
bytes_transferred values never change from their initial zeros.

For OMAP1:
Tested-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Thanks,
Janusz

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

* Re: [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
  2012-09-09 19:57       ` [alsa-devel] " Janusz Krzysztofik
@ 2012-09-10  8:21         ` Peter Ujfalusi
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-10  8:21 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: alsa-devel, Mark Brown, Santosh Shilimkar, linux-arm-kernel,
	Russell King, linux-omap, Liam Girdwood, Jarkko Nikula

Hi Janusz,

On 09/09/2012 10:57 PM, Janusz Krzysztofik wrote:
> On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
>>
>> Enable the element mode (thus allowing mono playback and probably 
> unblocking
>> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
>>
>> Janusz: would it be possible for you to test Russell's series plus 
> this on
>> OMAP1 to make sure that we do not broke it?
> 
> Hi,
> I can confirm that sound works for me as before, both capture and 
> playback, on my OMAP1 Amstrad Delta with Russell's and Peter's patches 
> applied on top of linux-3.6-rc3, with the OMAP DMA engine driver built 
> in. I was not able make audible tests with applications other than a 
> soft phone as I didn't get back home for this weekend, but I think that 
> the asterisk soft phone is quite a demanding use case.

Thank you very much for taking time to test this! This is indeed a good news.

> The only thing I'm not sure about is why the sysfs provided 
> bytes_transferred values never change from their initial zeros.

I have not looked at those files in sysfs, but if the same applies for
OMAP3/4/5 I can look at it and fix it which should correct OMAP1 at the same time.

> 
> For OMAP1:
> Tested-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Again, thanks for the testing,
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

* [alsa-devel] [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA
@ 2012-09-10  8:21         ` Peter Ujfalusi
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Ujfalusi @ 2012-09-10  8:21 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Janusz,

On 09/09/2012 10:57 PM, Janusz Krzysztofik wrote:
> On Tue, 4 Sep 2012 15:08:00 Peter Ujfalusi wrote:
>>
>> Enable the element mode (thus allowing mono playback and probably 
> unblocking
>> OMAP1, OMAP2420) in OMAP dmaengine and omap-pcm.
>>
>> Janusz: would it be possible for you to test Russell's series plus 
> this on
>> OMAP1 to make sure that we do not broke it?
> 
> Hi,
> I can confirm that sound works for me as before, both capture and 
> playback, on my OMAP1 Amstrad Delta with Russell's and Peter's patches 
> applied on top of linux-3.6-rc3, with the OMAP DMA engine driver built 
> in. I was not able make audible tests with applications other than a 
> soft phone as I didn't get back home for this weekend, but I think that 
> the asterisk soft phone is quite a demanding use case.

Thank you very much for taking time to test this! This is indeed a good news.

> The only thing I'm not sure about is why the sysfs provided 
> bytes_transferred values never change from their initial zeros.

I have not looked at those files in sysfs, but if the same applies for
OMAP3/4/5 I can look at it and fix it which should correct OMAP1 at the same time.

> 
> For OMAP1:
> Tested-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

Again, thanks for the testing,
P?ter

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

end of thread, other threads:[~2012-09-10  8:21 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-03 16:58 [RFC 0/3] Initial stab at converting OMAP ASoC support to DMA engine Russell King - ARM Linux
2012-09-03 16:58 ` Russell King - ARM Linux
2012-09-03 16:59 ` [RFC 1/3] ASoC: dmaengine: Don't use runtime private data for dmaengine data Liam Girdwood
2012-09-03 16:59   ` Liam Girdwood
2012-09-03 20:25   ` Lars-Peter Clausen
2012-09-03 20:25     ` [alsa-devel] " Lars-Peter Clausen
2012-09-03 20:43     ` Russell King - ARM Linux
2012-09-03 20:43       ` [alsa-devel] " Russell King - ARM Linux
2012-09-03 20:59       ` Lars-Peter Clausen
2012-09-03 20:59         ` [alsa-devel] " Lars-Peter Clausen
2012-09-04 13:14         ` Takashi Iwai
2012-09-04 13:14           ` Takashi Iwai
2012-09-04 13:26           ` Peter Ujfalusi
2012-09-04 13:26             ` Peter Ujfalusi
2012-09-04 18:14             ` Russell King - ARM Linux
2012-09-04 18:14               ` Russell King - ARM Linux
2012-09-03 16:59 ` [RFC 2/3] Fix "ASoC: dmaengine: Don't use runtime private data for dmaengine data" Russell King
2012-09-03 16:59   ` Russell King
2012-09-03 16:59 ` [RFC 3/3] ASoC: first stab at converting OMAP PCM driver to use dmaengine Russell King
2012-09-03 16:59   ` Russell King
2012-09-04 12:08   ` [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA Peter Ujfalusi
2012-09-04 12:08     ` Peter Ujfalusi
2012-09-04 12:08     ` [RFC update 1/2] dmaengine: omap: Support for " Peter Ujfalusi
2012-09-04 12:08       ` Peter Ujfalusi
2012-09-04 12:08     ` [RFC update 2/2] ASoC: omap-pcm: Do not check DMA sync_mode Peter Ujfalusi
2012-09-04 12:08       ` Peter Ujfalusi
2012-09-04 22:37     ` [RFC update 0/2] dmaengine/ASoC: omap: Enable element mode in cyclic DMA Janusz Krzysztofik
2012-09-04 22:37       ` [alsa-devel] " Janusz Krzysztofik
2012-09-09 19:57     ` Janusz Krzysztofik
2012-09-09 19:57       ` [alsa-devel] " Janusz Krzysztofik
2012-09-10  8:21       ` Peter Ujfalusi
2012-09-10  8:21         ` [alsa-devel] " Peter Ujfalusi

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