[PATCH 1/5] ASoC: DaVinci: i2s: don't bounce through rtd to get dai

[PATCH 1/5] ASoC: DaVinci: i2s: don't bounce through rtd to get dai

[Troy Kisky]

dai is a parameter to the functions, so use it instead of
looking it up.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 sound/soc/davinci/davinci-i2s.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index e5cd97b..2a56fb7 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -353,9 +353,8 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 				 struct snd_pcm_hw_params *params,
 				 struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_pcm_dma_params *dma_params = rtd->dai->cpu_dai->dma_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
+	struct davinci_pcm_dma_params *dma_params = dai->dma_data;
+	struct davinci_mcbsp_dev *dev = dai->private_data;
 	struct snd_interval *i = NULL;
 	int mcbsp_word_length;
 	unsigned int rcr, xcr, srgr;
@@ -425,8 +424,7 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 static int davinci_i2s_prepare(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
+	struct davinci_mcbsp_dev *dev = dai->private_data;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	davinci_mcbsp_stop(dev, playback);
 	if ((dev->pcr & DAVINCI_MCBSP_PCR_FSXM) == 0) {
@@ -439,8 +437,7 @@ static int davinci_i2s_prepare(struct snd_pcm_substream *substream,
 static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 			       struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
+	struct davinci_mcbsp_dev *dev = dai->private_data;
 	int ret = 0;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	if ((dev->pcr & DAVINCI_MCBSP_PCR_FSXM) == 0)
@@ -466,8 +463,7 @@ static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 static void davinci_i2s_shutdown(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
+	struct davinci_mcbsp_dev *dev = dai->private_data;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	davinci_mcbsp_stop(dev, playback);
 }
-- 
1.5.6.3

[PATCH 2/5] ASoC: DaVinci: pcm, constrain buffer size to multiple of period

[Troy Kisky]

The dma setup code assumes that the buffer size is a multiple
of the period size.

Signed-off-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 sound/soc/davinci/davinci-pcm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index fbda8ef..a294711 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -244,6 +244,11 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 	int ret = 0;
 
 	snd_soc_set_runtime_hwparams(substream, &davinci_pcm_hardware);
+	/* ensure that buffer size is a multiple of period size */
+	ret = snd_pcm_hw_constraint_integer(runtime,
+						SNDRV_PCM_HW_PARAM_PERIODS);
+	if (ret < 0)
+		return ret;
 
 	prtd = kzalloc(sizeof(struct davinci_runtime_data), GFP_KERNEL);
 	if (prtd == NULL)
-- 
1.5.6.3

[PATCH 3/5] ASoC: DaVinci: i2s, reduce underruns by combining into 1 element

[Troy Kisky]

Allow the left and right 16 bit samples to be shifted out as 1
32 bit sample.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/mach-davinci/include/mach/asp.h |    6 +++
 sound/soc/davinci/davinci-i2s.c          |   73 ++++++++++++++++++++++-------
 2 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h
index cdf1f44..73b60b6 100644
--- a/arch/arm/mach-davinci/include/mach/asp.h
+++ b/arch/arm/mach-davinci/include/mach/asp.h
@@ -37,6 +37,12 @@ struct snd_platform_data {
 	u32 rx_dma_offset;
 	enum dma_event_q eventq_no;	/* event queue number */
 	unsigned int codec_fmt;
+	/*
+	 * Allowing this is more efficient and eliminates left and right swaps
+	 * caused by underruns, but will swap the left and right channels
+	 * when compared to previous behavior.
+	 */
+	unsigned disable_channel_combine:1;
 
 	/* McASP specific fields */
 	int tdm_slots;
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 2a56fb7..9587e00 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -97,6 +97,23 @@ enum {
 	DAVINCI_MCBSP_WORD_32,
 };
 
+static const unsigned char data_type[SNDRV_PCM_FORMAT_S32_LE + 1] = {
+	[SNDRV_PCM_FORMAT_S8]		= 1,
+	[SNDRV_PCM_FORMAT_S16_LE]	= 2,
+	[SNDRV_PCM_FORMAT_S32_LE]	= 4,
+};
+
+static const unsigned char asp_word_length[SNDRV_PCM_FORMAT_S32_LE + 1] = {
+	[SNDRV_PCM_FORMAT_S8]		= DAVINCI_MCBSP_WORD_8,
+	[SNDRV_PCM_FORMAT_S16_LE]	= DAVINCI_MCBSP_WORD_16,
+	[SNDRV_PCM_FORMAT_S32_LE]	= DAVINCI_MCBSP_WORD_32,
+};
+
+static const unsigned char double_fmt[SNDRV_PCM_FORMAT_S32_LE + 1] = {
+	[SNDRV_PCM_FORMAT_S8]		= SNDRV_PCM_FORMAT_S16_LE,
+	[SNDRV_PCM_FORMAT_S16_LE]	= SNDRV_PCM_FORMAT_S32_LE,
+};
+
 static struct davinci_pcm_dma_params davinci_i2s_pcm_out = {
 	.name = "I2S PCM Stereo out",
 };
@@ -113,6 +130,27 @@ struct davinci_mcbsp_dev {
 	u32				pcr;
 	struct clk			*clk;
 	struct davinci_pcm_dma_params	*dma_params[2];
+	/*
+	 * Combining both channels into 1 element will at least double the
+	 * amount of time between servicing the dma channel, increase
+	 * effiency, and reduce the chance of overrun/underrun. But,
+	 * it will result in the left & right channels being swapped.
+	 *
+	 * If relabeling the left and right channels is not possible,
+	 * you may want to let the codec know to swap them back.
+	 *
+	 * It may allow x10 the amount of time to service dma requests,
+	 * if the codec is master and is using an unnecessarily fast bit clock
+	 * (ie. tlvaic23b), independent of the sample rate. So, having an
+	 * entire frame at once means it can be serviced at the sample rate
+	 * instead of the bit clock rate.
+	 *
+	 * In the now unlikely case that an underrun still
+	 * occurs, both the left and right samples will be repeated
+	 * so that no pops are heard, and the left and right channels
+	 * won't end up being swapped because of the underrun.
+	 */
+	unsigned disable_channel_combine:1;
 };
 
 static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
@@ -359,6 +397,8 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	int mcbsp_word_length;
 	unsigned int rcr, xcr, srgr;
 	u32 spcr;
+	snd_pcm_format_t fmt;
+	unsigned element_cnt = 1;
 
 	/* general line settings */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
@@ -388,26 +428,22 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
 	}
 	/* Determine xfer data type */
-	switch (params_format(params)) {
-	case SNDRV_PCM_FORMAT_S8:
-		dma_params->data_type = 1;
-		mcbsp_word_length = DAVINCI_MCBSP_WORD_8;
-		break;
-	case SNDRV_PCM_FORMAT_S16_LE:
-		dma_params->data_type = 2;
-		mcbsp_word_length = DAVINCI_MCBSP_WORD_16;
-		break;
-	case SNDRV_PCM_FORMAT_S32_LE:
-		dma_params->data_type = 4;
-		mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
-		break;
-	default:
+	fmt = params_format(params);
+	if ((fmt > SNDRV_PCM_FORMAT_S32_LE) || !data_type[fmt]) {
 		printk(KERN_WARNING "davinci-i2s: unsupported PCM format\n");
 		return -EINVAL;
 	}
-
-	rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(1);
-	xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(1);
+	if (params_channels(params) == 2) {
+		element_cnt = 2;
+		if (double_fmt[fmt] && !dev->disable_channel_combine) {
+			element_cnt = 1;
+			fmt = double_fmt[fmt];
+		}
+	}
+	dma_params->data_type = data_type[fmt];
+	mcbsp_word_length = asp_word_length[fmt];
+	rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(element_cnt - 1);
+	xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(element_cnt - 1);
 
 	rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
 		DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
@@ -523,7 +559,8 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto err_release_region;
 	}
-
+	if (pdata)
+		dev->disable_channel_combine = pdata->disable_channel_combine;
 	dev->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		ret = -ENODEV;
-- 
1.5.6.3

[PATCH 4/5] ASoC: DaVinci: pcm, rename variables in prep for ping/pong

[Troy Kisky]

Rename variable master_lch to asp_master_lch
Rename variable slave_lch to asp_link_lch[0]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 sound/soc/davinci/davinci-pcm.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index a294711..3489f18 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -51,8 +51,8 @@ static struct snd_pcm_hardware davinci_pcm_hardware = {
 struct davinci_runtime_data {
 	spinlock_t lock;
 	int period;		/* current DMA period */
-	int master_lch;		/* Master DMA channel */
-	int slave_lch;		/* linked parameter RAM reload slot */
+	int asp_master_lch;	/* Master DMA channel */
+	int asp_link_lch[2];	/* asp parameter link channel, ping/pong */
 	struct davinci_pcm_dma_params *params;	/* DMA params */
 };
 
@@ -60,7 +60,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 {
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	int lch = prtd->slave_lch;
+	int lch = prtd->asp_link_lch[0];
 	unsigned int period_size;
 	unsigned int dma_offset;
 	dma_addr_t dma_pos;
@@ -140,15 +140,15 @@ static int davinci_pcm_dma_request(struct snd_pcm_substream *substream)
 				  EVENTQ_0);
 	if (ret < 0)
 		return ret;
-	prtd->master_lch = ret;
+	prtd->asp_master_lch = ret;
 
 	/* Request parameter RAM reload slot */
-	ret = edma_alloc_slot(EDMA_CTLR(prtd->master_lch), EDMA_SLOT_ANY);
+	ret = edma_alloc_slot(EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
 	if (ret < 0) {
-		edma_free_channel(prtd->master_lch);
+		edma_free_channel(prtd->asp_master_lch);
 		return ret;
 	}
-	prtd->slave_lch = ret;
+	prtd->asp_link_lch[0] = ret;
 
 	/* Issue transfer completion IRQ when the channel completes a
 	 * transfer, then always reload from the same slot (by a kind
@@ -159,10 +159,10 @@ static int davinci_pcm_dma_request(struct snd_pcm_substream *substream)
 	 * the buffer and its length (ccnt) ... use it as a template
 	 * so davinci_pcm_enqueue_dma() takes less time in IRQ.
 	 */
-	edma_read_slot(prtd->slave_lch, &p_ram);
-	p_ram.opt |= TCINTEN | EDMA_TCC(EDMA_CHAN_SLOT(prtd->master_lch));
-	p_ram.link_bcntrld = EDMA_CHAN_SLOT(prtd->slave_lch) << 5;
-	edma_write_slot(prtd->slave_lch, &p_ram);
+	edma_read_slot(prtd->asp_link_lch[0], &p_ram);
+	p_ram.opt |= TCINTEN | EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_master_lch));
+	p_ram.link_bcntrld = EDMA_CHAN_SLOT(prtd->asp_link_lch[0]) << 5;
+	edma_write_slot(prtd->asp_link_lch[0], &p_ram);
 
 	return 0;
 }
@@ -178,12 +178,12 @@ static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		edma_start(prtd->master_lch);
+		edma_start(prtd->asp_master_lch);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		edma_stop(prtd->master_lch);
+		edma_stop(prtd->asp_master_lch);
 		break;
 	default:
 		ret = -EINVAL;
@@ -204,8 +204,8 @@ static int davinci_pcm_prepare(struct snd_pcm_substream *substream)
 	davinci_pcm_enqueue_dma(substream);
 
 	/* Copy self-linked parameter RAM entry into master channel */
-	edma_read_slot(prtd->slave_lch, &temp);
-	edma_write_slot(prtd->master_lch, &temp);
+	edma_read_slot(prtd->asp_link_lch[0], &temp);
+	edma_write_slot(prtd->asp_master_lch, &temp);
 	davinci_pcm_enqueue_dma(substream);
 
 	return 0;
@@ -222,7 +222,7 @@ davinci_pcm_pointer(struct snd_pcm_substream *substream)
 
 	spin_lock(&prtd->lock);
 
-	edma_get_position(prtd->master_lch, &src, &dst);
+	edma_get_position(prtd->asp_master_lch, &src, &dst);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		count = src - runtime->dma_addr;
 	else
@@ -272,10 +272,10 @@ static int davinci_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct davinci_runtime_data *prtd = runtime->private_data;
 
-	edma_unlink(prtd->slave_lch);
+	edma_unlink(prtd->asp_link_lch[0]);
 
-	edma_free_slot(prtd->slave_lch);
-	edma_free_channel(prtd->master_lch);
+	edma_free_slot(prtd->asp_link_lch[0]);
+	edma_free_channel(prtd->asp_master_lch);
 
 	kfree(prtd);
 
-- 
1.5.6.3

[PATCH 5/5] ASoC: DaVinci: pcm, fix underruns by using sram

[Troy Kisky]

Fix underruns by using dma to copy 1st to sram
in a ping/pong buffer style and then copying from
the sram to the ASP. This also has the advantage
of tolerating very long interrupt latency on dma
completion.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/mach-davinci/include/mach/asp.h |    2 +
 sound/soc/davinci/davinci-i2s.c          |    5 +-
 sound/soc/davinci/davinci-pcm.c          |  471 +++++++++++++++++++++++++++---
 sound/soc/davinci/davinci-pcm.h          |    1 +
 4 files changed, 435 insertions(+), 44 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h
index 73b60b6..0fdbf45 100644
--- a/arch/arm/mach-davinci/include/mach/asp.h
+++ b/arch/arm/mach-davinci/include/mach/asp.h
@@ -43,6 +43,8 @@ struct snd_platform_data {
 	 * when compared to previous behavior.
 	 */
 	unsigned disable_channel_combine:1;
+	unsigned sram_size_playback;
+	unsigned sram_size_capture;
 
 	/* McASP specific fields */
 	int tdm_slots;
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 9587e00..129684d 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -559,8 +559,11 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto err_release_region;
 	}
-	if (pdata)
+	if (pdata) {
 		dev->disable_channel_combine = pdata->disable_channel_combine;
+		davinci_i2s_pcm_out.sram_size = pdata->sram_size_playback;
+		davinci_i2s_pcm_in.sram_size = pdata->sram_size_capture;
+	}
 	dev->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		ret = -ENODEV;
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index 3489f18..191cf4f 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -3,6 +3,7 @@
  *
  * Author:      Vladimir Barinov, <vbarinov@embeddedalley.com>
  * Copyright:   (C) 2007 MontaVista Software, Inc., <source@mvista.com>
+ * added SRAM ping/pong (C) 2008 Troy Kisky <troy.kisky@boundarydevices.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -23,9 +24,28 @@
 
 #include <asm/dma.h>
 #include <mach/edma.h>
+#include <mach/sram.h>
 
 #include "davinci-pcm.h"
 
+#ifdef DEBUG
+static void print_buf_info(int lch, char *name)
+{
+	struct edmacc_param p;
+	if (lch < 0)
+		return;
+	edma_read_slot(lch, &p);
+	printk(KERN_DEBUG "%s: 0x%x, opt=%x, src=%x, a_b_cnt=%x dst=%x\n",
+			name, lch, p.opt, p.src, p.a_b_cnt, p.dst);
+	printk(KERN_DEBUG "    src_dst_bidx=%x link_bcntrld=%x src_dst_cidx=%x ccnt=%x\n",
+			p.src_dst_bidx, p.link_bcntrld, p.src_dst_cidx, p.ccnt);
+}
+#else
+static void print_buf_info(int lch, char *name)
+{
+}
+#endif
+
 static struct snd_pcm_hardware davinci_pcm_hardware = {
 	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
@@ -48,14 +68,58 @@ static struct snd_pcm_hardware davinci_pcm_hardware = {
 	.fifo_size = 0,
 };
 
+/*
+ * How ping/pong works....
+ *
+ * Playback:
+ * ram_params - copys 2*ping_size from start of SDRAM to iram,
+ * 	links to ram_link_lch2
+ * ram_link_lch2 - copys rest of SDRAM to iram in ping_size units,
+ * 	links to ram_link_lch
+ * ram_link_lch - copys entire SDRAM to iram in ping_size uints,
+ * 	links to self
+ *
+ * asp_params - same as asp_link_lch[0]
+ * asp_link_lch[0] - copys from lower half of iram to asp port
+ * 	links to asp_link_lch[1], triggers iram copy event on completion
+ * asp_link_lch[1] - copys from upper half of iram to asp port
+ * 	links to asp_link_lch[0], triggers iram copy event on completion
+ * 	triggers interrupt only needed to let upper SOC levels update position
+ * 	in stream on completion
+ *
+ * When playback is started:
+ * 	ram_params started
+ * 	asp_params started
+ *
+ * Capture:
+ * ram_params - same as ram_link_lch,
+ * 	links to ram_link_lch
+ * ram_link_lch - same as playback
+ * 	links to self
+ *
+ * asp_params - same as playback
+ * asp_link_lch[0] - same as playback
+ * asp_link_lch[1] - same as playback
+ *
+ * When capture is started:
+ * 	asp_params started
+ */
 struct davinci_runtime_data {
 	spinlock_t lock;
 	int period;		/* current DMA period */
 	int asp_master_lch;	/* Master DMA channel */
 	int asp_link_lch[2];	/* asp parameter link channel, ping/pong */
 	struct davinci_pcm_dma_params *params;	/* DMA params */
+	int ram_master_lch;
+	int ram_link_lch;
+	int ram_link_lch2;
+	struct edmacc_param asp_params;
+	struct edmacc_param ram_params;
 };
 
+/*
+ * Not used with ping/pong
+ */
 static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 {
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
@@ -107,48 +171,280 @@ static void davinci_pcm_dma_irq(unsigned lch, u16 ch_status, void *data)
 	struct snd_pcm_substream *substream = data;
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
 
+	print_buf_info(prtd->ram_master_lch, "i ram_master_lch");
 	pr_debug("davinci_pcm: lch=%d, status=0x%x\n", lch, ch_status);
 
 	if (unlikely(ch_status != DMA_COMPLETE))
 		return;
 
 	if (snd_pcm_running(substream)) {
+		if (prtd->ram_master_lch < 0) {
+			/* No ping/pong must fix up link dma data*/
+			spin_lock(&prtd->lock);
+			davinci_pcm_enqueue_dma(substream);
+			spin_unlock(&prtd->lock);
+		}
 		snd_pcm_period_elapsed(substream);
+	}
+}
+
+static int allocate_sram(struct snd_pcm_substream *substream, unsigned size)
+{
+	struct snd_dma_buffer *buf = &substream->dma_buffer;
+	struct snd_dma_buffer *iram_dma = NULL;
+	dma_addr_t iram_phys = 0;
+	void *iram_virt = NULL;
+
+	if (buf->private_data || !size)
+		return 0;
+
+	davinci_pcm_hardware.period_bytes_max = size;
+	iram_virt = sram_alloc(size, &iram_phys);
+	if (!iram_virt)
+		goto exit1;
+	iram_dma = kzalloc(sizeof(*iram_dma), GFP_KERNEL);
+	if (!iram_dma)
+		goto exit2;
+	iram_dma->area = iram_virt;
+	iram_dma->addr = iram_phys;
+	memset(iram_dma->area, 0, size);
+	iram_dma->bytes = size;
+	buf->private_data = iram_dma;
+	return 0;
+exit2:
+	if (iram_virt)
+		sram_free(iram_virt, size);
+exit1:
+	return -ENOMEM;
+}
+
+/*
+ * Only used with ping/pong.
+ * This is called after runtime->dma_addr, period_bytes and data_type are valid
+ */
+static int ping_pong_dma_setup(struct snd_pcm_substream *substream)
+{
+	unsigned short ram_src_cidx, ram_dst_cidx;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct davinci_runtime_data *prtd = runtime->private_data;
+	struct snd_dma_buffer *iram_dma =
+		(struct snd_dma_buffer *)substream->dma_buffer.private_data;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct davinci_pcm_dma_params *dma_data = rtd->dai->cpu_dai->dma_data;
+	unsigned int data_type = dma_data->data_type;
+	/* divide by 2 for ping/pong */
+	unsigned int ping_size = snd_pcm_lib_period_bytes(substream) >> 1;
+	int lch = prtd->asp_link_lch[1];
+	if ((data_type == 0) || (data_type > 4)) {
+		printk(KERN_ERR "%s: data_type=%i\n", __func__, data_type);
+		return -EINVAL;
+	}
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		dma_addr_t asp_src_pong = iram_dma->addr + ping_size;
+		ram_src_cidx = ping_size;
+		ram_dst_cidx = -ping_size;
+		edma_set_src(lch, asp_src_pong, INCR, W8BIT);
+
+		lch = prtd->asp_link_lch[0];
+		edma_set_src_index(lch, data_type, 0);
+		lch = prtd->asp_link_lch[1];
+		edma_set_src_index(lch, data_type, 0);
+
+		lch = prtd->ram_link_lch;
+		edma_set_src(lch, runtime->dma_addr, INCR, W32BIT);
+	} else {
+		dma_addr_t asp_dst_pong = iram_dma->addr + ping_size;
+		ram_src_cidx = -ping_size;
+		ram_dst_cidx = ping_size;
+		edma_set_dest(lch, asp_dst_pong, INCR, W8BIT);
+
+		lch = prtd->asp_link_lch[0];
+		edma_set_dest_index(lch, data_type, 0);
+		lch = prtd->asp_link_lch[1];
+		edma_set_dest_index(lch, data_type, 0);
+
+		lch = prtd->ram_link_lch;
+		edma_set_dest(lch, runtime->dma_addr, INCR, W32BIT);
+	}
 
-		spin_lock(&prtd->lock);
-		davinci_pcm_enqueue_dma(substream);
-		spin_unlock(&prtd->lock);
+	lch = prtd->asp_link_lch[0];
+	edma_set_transfer_params(lch, data_type,
+			ping_size/data_type, 1, 0, ASYNC);
+	lch = prtd->asp_link_lch[1];
+	edma_set_transfer_params(lch, data_type,
+			ping_size/data_type, 1, 0, ASYNC);
+
+	lch = prtd->ram_link_lch;
+	edma_set_src_index(lch, ping_size, ram_src_cidx);
+	edma_set_dest_index(lch, ping_size, ram_dst_cidx);
+	edma_set_transfer_params(lch, ping_size, 2,
+			runtime->periods, 2, ASYNC);
+
+	/* init master params */
+	edma_read_slot(prtd->asp_link_lch[0], &prtd->asp_params);
+	edma_read_slot(prtd->ram_link_lch, &prtd->ram_params);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		struct edmacc_param p_ram;
+		/* Copy entire iram buffer before playback started */
+		prtd->ram_params.a_b_cnt = (1 << 16) | (ping_size << 1);
+		/* 0 dst_bidx */
+		prtd->ram_params.src_dst_bidx = (ping_size << 1);
+		/* 0 dst_cidx */
+		prtd->ram_params.src_dst_cidx = (ping_size << 1);
+		prtd->ram_params.ccnt = 1;
+
+		/* Skip 1st period */
+		edma_read_slot(prtd->ram_link_lch, &p_ram);
+		p_ram.src += (ping_size << 1);
+		p_ram.ccnt -= 1;
+		edma_write_slot(prtd->ram_link_lch2, &p_ram);
+		/*
+		 * When 1st started, ram -> iram dma channel will fill the
+		 * entire iram.  Then, whenever a ping/pong asp buffer finishes,
+		 * 1/2 iram will be filled.
+		 */
+		prtd->ram_params.link_bcntrld =
+			EDMA_CHAN_SLOT(prtd->ram_link_lch2) << 5;
 	}
+	return 0;
+}
+
+/* 1 asp tx or rx channel using 2 parameter channels
+ * 1 ram to/from iram channel using 1 parameter channel
+ *
+ * Playback
+ * ram copy channel kicks off first,
+ * 1st ram copy of entire iram buffer completion kicks off asp channel
+ * asp tcc always kicks off ram copy of 1/2 iram buffer
+ *
+ * Record
+ * asp channel starts, tcc kicks off ram copy
+ */
+static int request_ping_pong(struct snd_pcm_substream *substream,
+		struct davinci_runtime_data *prtd,
+		struct snd_dma_buffer *iram_dma)
+{
+	dma_addr_t asp_src_ping;
+	dma_addr_t asp_dst_ping;
+	int lch;
+	struct davinci_pcm_dma_params *dma_data = prtd->params;
+
+	/* Request ram master channel */
+	lch = prtd->ram_master_lch = edma_alloc_channel(EDMA_CHANNEL_ANY,
+				  davinci_pcm_dma_irq, substream,
+				  EVENTQ_1);
+	if (lch < 0)
+		goto exit1;
+
+	/* Request ram link channel */
+	lch = prtd->ram_link_lch = edma_alloc_slot(
+			EDMA_CTLR(prtd->ram_master_lch), EDMA_SLOT_ANY);
+	if (lch < 0)
+		goto exit2;
+
+	lch = prtd->asp_link_lch[1] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
+	if (lch < 0)
+		goto exit3;
+
+	prtd->ram_link_lch2 = -1;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		lch = prtd->ram_link_lch2 = edma_alloc_slot(
+			EDMA_CTLR(prtd->ram_master_lch), EDMA_SLOT_ANY);
+		if (lch < 0)
+			goto exit4;
+	}
+	/* circle ping-pong buffers */
+	edma_link(prtd->asp_link_lch[0], prtd->asp_link_lch[1]);
+	edma_link(prtd->asp_link_lch[1], prtd->asp_link_lch[0]);
+	/* circle ram buffers */
+	edma_link(prtd->ram_link_lch, prtd->ram_link_lch);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		asp_src_ping = iram_dma->addr;
+		asp_dst_ping = dma_data->dma_addr;	/* fifo */
+	} else {
+		asp_src_ping = dma_data->dma_addr;	/* fifo */
+		asp_dst_ping = iram_dma->addr;
+	}
+	/* ping */
+	lch = prtd->asp_link_lch[0];
+	edma_set_src(lch, asp_src_ping, INCR, W16BIT);
+	edma_set_dest(lch, asp_dst_ping, INCR, W16BIT);
+	edma_set_src_index(lch, 0, 0);
+	edma_set_dest_index(lch, 0, 0);
+
+	edma_read_slot(lch, &prtd->asp_params);
+	prtd->asp_params.opt &= ~(TCCMODE | EDMA_TCC(0x3f) | TCINTEN);
+	prtd->asp_params.opt |= TCCHEN | EDMA_TCC(prtd->ram_master_lch & 0x3f);
+	edma_write_slot(lch, &prtd->asp_params);
+
+	/* pong */
+	lch = prtd->asp_link_lch[1];
+	edma_set_src(lch, asp_src_ping, INCR, W16BIT);
+	edma_set_dest(lch, asp_dst_ping, INCR, W16BIT);
+	edma_set_src_index(lch, 0, 0);
+	edma_set_dest_index(lch, 0, 0);
+
+	edma_read_slot(lch, &prtd->asp_params);
+	prtd->asp_params.opt &= ~(TCCMODE | EDMA_TCC(0x3f));
+	/* interrupt after every pong completion */
+	prtd->asp_params.opt |= TCINTEN | TCCHEN |
+		EDMA_TCC(EDMA_CHAN_SLOT(prtd->ram_master_lch));
+	edma_write_slot(lch, &prtd->asp_params);
+
+	/* ram */
+	lch = prtd->ram_link_lch;
+	edma_set_src(lch, iram_dma->addr, INCR, W32BIT);
+	edma_set_dest(lch, iram_dma->addr, INCR, W32BIT);
+	pr_debug("%s: audio dma channels/slots in use for ram:%u %u %u,"
+		"for asp:%u %u %u\n", __func__,
+		prtd->ram_master_lch, prtd->ram_link_lch, prtd->ram_link_lch2,
+		prtd->asp_master_lch, prtd->asp_link_lch[0],
+		prtd->asp_link_lch[1]);
+	return 0;
+exit4:
+	edma_free_channel(prtd->asp_link_lch[1]);
+	prtd->asp_link_lch[1] = -1;
+exit3:
+	edma_free_channel(prtd->ram_link_lch);
+	prtd->ram_link_lch = -1;
+exit2:
+	edma_free_channel(prtd->ram_master_lch);
+	prtd->ram_master_lch = -1;
+exit1:
+	return lch;
 }
 
 static int davinci_pcm_dma_request(struct snd_pcm_substream *substream)
 {
+	struct snd_dma_buffer *iram_dma;
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct davinci_pcm_dma_params *dma_data = rtd->dai->cpu_dai->dma_data;
-	struct edmacc_param p_ram;
-	int ret;
+	int lch;
 
 	if (!dma_data)
 		return -ENODEV;
 
 	prtd->params = dma_data;
 
-	/* Request master DMA channel */
-	ret = edma_alloc_channel(prtd->params->channel,
-				  davinci_pcm_dma_irq, substream,
-				  EVENTQ_0);
-	if (ret < 0)
-		return ret;
-	prtd->asp_master_lch = ret;
+	/* Request asp master DMA channel */
+	lch = prtd->asp_master_lch = edma_alloc_channel(dma_data->channel,
+			davinci_pcm_dma_irq, substream, EVENTQ_0);
+	if (lch < 0)
+		goto exit1;
 
-	/* Request parameter RAM reload slot */
-	ret = edma_alloc_slot(EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
-	if (ret < 0) {
-		edma_free_channel(prtd->asp_master_lch);
-		return ret;
-	}
-	prtd->asp_link_lch[0] = ret;
+	/* Request asp link channels */
+	lch = prtd->asp_link_lch[0] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
+	if (lch < 0)
+		goto exit2;
+
+	allocate_sram(substream, dma_data->sram_size);
+	iram_dma = (struct snd_dma_buffer *)substream->dma_buffer.private_data;
+	if (iram_dma)
+		return request_ping_pong(substream, prtd, iram_dma);
 
 	/* Issue transfer completion IRQ when the channel completes a
 	 * transfer, then always reload from the same slot (by a kind
@@ -159,12 +455,17 @@ static int davinci_pcm_dma_request(struct snd_pcm_substream *substream)
 	 * the buffer and its length (ccnt) ... use it as a template
 	 * so davinci_pcm_enqueue_dma() takes less time in IRQ.
 	 */
-	edma_read_slot(prtd->asp_link_lch[0], &p_ram);
-	p_ram.opt |= TCINTEN | EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_master_lch));
-	p_ram.link_bcntrld = EDMA_CHAN_SLOT(prtd->asp_link_lch[0]) << 5;
-	edma_write_slot(prtd->asp_link_lch[0], &p_ram);
-
+	edma_read_slot(lch, &prtd->asp_params);
+	prtd->asp_params.opt |= TCINTEN |
+		EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_master_lch));
+	prtd->asp_params.link_bcntrld = EDMA_CHAN_SLOT(lch) << 5;
+	edma_write_slot(lch, &prtd->asp_params);
 	return 0;
+exit2:
+	edma_free_channel(prtd->asp_master_lch);
+	prtd->asp_master_lch = -1;
+exit1:
+	return lch;
 }
 
 static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
@@ -178,12 +479,12 @@ static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		edma_start(prtd->asp_master_lch);
+		edma_resume(prtd->asp_master_lch);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		edma_stop(prtd->asp_master_lch);
+		edma_pause(prtd->asp_master_lch);
 		break;
 	default:
 		ret = -EINVAL;
@@ -198,14 +499,35 @@ static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 static int davinci_pcm_prepare(struct snd_pcm_substream *substream)
 {
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
-	struct edmacc_param temp;
 
+	if (prtd->ram_master_lch >= 0) {
+		int ret = ping_pong_dma_setup(substream);
+		if (ret < 0)
+			return ret;
+
+		edma_write_slot(prtd->ram_master_lch, &prtd->ram_params);
+		edma_write_slot(prtd->asp_master_lch, &prtd->asp_params);
+
+		print_buf_info(prtd->ram_master_lch, "ram_master_lch");
+		print_buf_info(prtd->ram_link_lch, "ram_link_lch");
+		print_buf_info(prtd->ram_link_lch2, "ram_link_lch2");
+		print_buf_info(prtd->asp_master_lch, "asp_master_lch");
+		print_buf_info(prtd->asp_link_lch[0], "asp_link_lch[0]");
+		print_buf_info(prtd->asp_link_lch[1], "asp_link_lch[1]");
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			/* copy 1st iram buffer */
+			edma_start(prtd->ram_master_lch);
+		}
+		edma_start(prtd->asp_master_lch);
+		return 0;
+	}
 	prtd->period = 0;
 	davinci_pcm_enqueue_dma(substream);
 
 	/* Copy self-linked parameter RAM entry into master channel */
-	edma_read_slot(prtd->asp_link_lch[0], &temp);
-	edma_write_slot(prtd->asp_master_lch, &temp);
+	edma_read_slot(prtd->asp_link_lch[0], &prtd->asp_params);
+	edma_write_slot(prtd->asp_master_lch, &prtd->asp_params);
 	davinci_pcm_enqueue_dma(substream);
 
 	return 0;
@@ -217,20 +539,53 @@ davinci_pcm_pointer(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct davinci_runtime_data *prtd = runtime->private_data;
 	unsigned int offset;
-	dma_addr_t count;
-	dma_addr_t src, dst;
+	int asp_count;
+	dma_addr_t asp_src, asp_dst;
 
 	spin_lock(&prtd->lock);
-
-	edma_get_position(prtd->asp_master_lch, &src, &dst);
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		count = src - runtime->dma_addr;
-	else
-		count = dst - runtime->dma_addr;
-
+	if (prtd->ram_master_lch >= 0) {
+		int ram_count;
+		int mod_ram;
+		dma_addr_t ram_src, ram_dst;
+		unsigned int period_size = snd_pcm_lib_period_bytes(substream);
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			/* reading ram before asp should be safe
+			 * as long as the asp transfers less than a ping size
+			 * of bytes between the 2 reads
+			 */
+			edma_get_position(prtd->ram_master_lch,
+					&ram_src, &ram_dst);
+			edma_get_position(prtd->asp_master_lch,
+					&asp_src, &asp_dst);
+			asp_count = asp_src - prtd->asp_params.src;
+			ram_count = ram_src - prtd->ram_params.src;
+			mod_ram = ram_count % period_size;
+			mod_ram -= asp_count;
+			if (mod_ram < 0)
+				mod_ram += period_size;
+			else if (mod_ram == 0) {
+				if (snd_pcm_running(substream))
+					mod_ram += period_size;
+			}
+			ram_count -= mod_ram;
+			if (ram_count < 0)
+				ram_count += period_size * runtime->periods;
+		} else {
+			edma_get_position(prtd->ram_master_lch,
+					&ram_src, &ram_dst);
+			ram_count = ram_dst - prtd->ram_params.dst;
+		}
+		asp_count = ram_count;
+	} else {
+		edma_get_position(prtd->asp_master_lch, &asp_src, &asp_dst);
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			asp_count = asp_src - runtime->dma_addr;
+		else
+			asp_count = asp_dst - runtime->dma_addr;
+	}
 	spin_unlock(&prtd->lock);
 
-	offset = bytes_to_frames(runtime, count);
+	offset = bytes_to_frames(runtime, asp_count);
 	if (offset >= runtime->buffer_size)
 		offset = 0;
 
@@ -255,6 +610,11 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 		return -ENOMEM;
 
 	spin_lock_init(&prtd->lock);
+	prtd->asp_master_lch = -1;
+	prtd->asp_link_lch[0] = prtd->asp_link_lch[1] = -1;
+	prtd->ram_master_lch = -1;
+	prtd->ram_link_lch = -1;
+	prtd->ram_link_lch2 = -1;
 
 	runtime->private_data = prtd;
 
@@ -272,10 +632,29 @@ static int davinci_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct davinci_runtime_data *prtd = runtime->private_data;
 
-	edma_unlink(prtd->asp_link_lch[0]);
-
-	edma_free_slot(prtd->asp_link_lch[0]);
-	edma_free_channel(prtd->asp_master_lch);
+	if (prtd->ram_master_lch >= 0)
+		edma_stop(prtd->ram_master_lch);
+	if (prtd->asp_master_lch >= 0)
+		edma_stop(prtd->asp_master_lch);
+	if (prtd->asp_link_lch[0] >= 0)
+		edma_unlink(prtd->asp_link_lch[0]);
+	if (prtd->asp_link_lch[1] >= 0)
+		edma_unlink(prtd->asp_link_lch[1]);
+	if (prtd->ram_link_lch >= 0)
+		edma_unlink(prtd->ram_link_lch);
+
+	if (prtd->asp_link_lch[0] >= 0)
+		edma_free_slot(prtd->asp_link_lch[0]);
+	if (prtd->asp_link_lch[1] >= 0)
+		edma_free_slot(prtd->asp_link_lch[1]);
+	if (prtd->asp_master_lch >= 0)
+		edma_free_channel(prtd->asp_master_lch);
+	if (prtd->ram_link_lch >= 0)
+		edma_free_slot(prtd->ram_link_lch);
+	if (prtd->ram_link_lch2 >= 0)
+		edma_free_slot(prtd->ram_link_lch2);
+	if (prtd->ram_master_lch >= 0)
+		edma_free_channel(prtd->ram_master_lch);
 
 	kfree(prtd);
 
@@ -346,6 +725,7 @@ static void davinci_pcm_free(struct snd_pcm *pcm)
 	int stream;
 
 	for (stream = 0; stream < 2; stream++) {
+		struct snd_dma_buffer *iram_dma;
 		substream = pcm->streams[stream].substream;
 		if (!substream)
 			continue;
@@ -357,6 +737,11 @@ static void davinci_pcm_free(struct snd_pcm *pcm)
 		dma_free_writecombine(pcm->card->dev, buf->bytes,
 				      buf->area, buf->addr);
 		buf->area = NULL;
+		iram_dma = (struct snd_dma_buffer *)buf->private_data;
+		if (iram_dma) {
+			sram_free(iram_dma->area, iram_dma->bytes);
+			kfree(iram_dma);
+		}
 	}
 }
 
diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index eb4287f..8d66b35 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -20,6 +20,7 @@ struct davinci_pcm_dma_params {
 	char *name;			/* stream identifier */
 	int channel;			/* sync dma channel ID */
 	dma_addr_t dma_addr;		/* device physical address for DMA */
+	unsigned sram_size;
 	enum dma_event_q eventq_no;	/* event queue number */
 	unsigned char data_type;	/* xfer data type */
 	unsigned char convert_mono_stereo;
-- 
1.5.6.3

Re: [PATCH 5/5] ASoC: DaVinci: pcm, fix underruns by using sram

[Troy Kisky]

Troy Kisky wrote:
> Fix underruns by using dma to copy 1st to sram
> in a ping/pong buffer style and then copying from
> the sram to the ASP. This also has the advantage
> of tolerating very long interrupt latency on dma
> completion.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>  static int davinci_pcm_dma_request(struct snd_pcm_substream *substream)
>  {
> +	struct snd_dma_buffer *iram_dma;
>  	struct davinci_runtime_data *prtd = substream->runtime->private_data;
>  	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>  	struct davinci_pcm_dma_params *dma_data = rtd->dai->cpu_dai->dma_data;
> -	struct edmacc_param p_ram;
> -	int ret;
> +	int lch;
>  
>  	if (!dma_data)
>  		return -ENODEV;
>  
>  	prtd->params = dma_data;
>  
> -	/* Request master DMA channel */
> -	ret = edma_alloc_channel(prtd->params->channel,
> -				  davinci_pcm_dma_irq, substream,
> -				  EVENTQ_0);
> -	if (ret < 0)
> -		return ret;
> -	prtd->asp_master_lch = ret;
> +	/* Request asp master DMA channel */
> +	lch = prtd->asp_master_lch = edma_alloc_channel(dma_data->channel,
> +			davinci_pcm_dma_irq, substream, EVENTQ_0);
> +	if (lch < 0)
> +		goto exit1;
>  
> -	/* Request parameter RAM reload slot */
> -	ret = edma_alloc_slot(EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
> -	if (ret < 0) {
> -		edma_free_channel(prtd->asp_master_lch);
> -		return ret;
> -	}
> -	prtd->asp_link_lch[0] = ret;
> +	/* Request asp link channels */
> +	lch = prtd->asp_link_lch[0] = edma_alloc_slot(
> +			EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
> +	if (lch < 0)
> +		goto exit2;
> +
> +	allocate_sram(substream, dma_data->sram_size);

This is allocated too late for the "ensure that buffer size is a multiple of period size"
constraint.

I have a patch after fixing other feedback.

Troy

Re: [PATCH 5/5] ASoC: DaVinci: pcm, fix underruns by using sram

[Mark Brown]

On Thu, Aug 06, 2009 at 07:05:54PM -0700, Troy Kisky wrote:

> This is allocated too late for the "ensure that buffer size is a multiple of period size"
> constraint.

> I have a patch after fixing other feedback.

It looks good to me - I've no issues with the patch except for the one I
mentioned last time about considering ignoring the data in SRAM when
reporting the current position but I'm happy either way.  The patch will
run into the cross tree issues with the platform data like the channel
combining one, probably best to submit patches against Kevin's tree for
now (or wait until after the merge window).

Have you tested with PulseAudio?  If not it'd be worth giving it a spin
- it's one of the more demanding applications.

Re: [PATCH 5/5] ASoC: DaVinci: pcm, fix underruns by using sram

[Troy Kisky]

Mark Brown wrote:
> On Thu, Aug 06, 2009 at 07:05:54PM -0700, Troy Kisky wrote:
> 
>> This is allocated too late for the "ensure that buffer size is a multiple of period size"
>> constraint.
> 
>> I have a patch after fixing other feedback.
> 
> It looks good to me - I've no issues with the patch except for the one I
> mentioned last time about considering ignoring the data in SRAM when
> reporting the current position but I'm happy either way.  The patch will
> run into the cross tree issues with the platform data like the channel
> combining one, probably best to submit patches against Kevin's tree for
> now (or wait until after the merge window).
> 
> Have you tested with PulseAudio?  If not it'd be worth giving it a spin
> - it's one of the more demanding applications.
> 
I haven't tested with PulseAudio, and I don't have time to look into it currently.
Any volunteers?

On question I had concerns davinci_pcm_hardware. It is currently for both playback
and capture. Since allocate_sram contains "davinci_pcm_hardware.period_bytes_max = size;,"
should I change davinci_pcm_hardware to playback_pcm_hardware, capture_pcm_hardware?

Re: [PATCH 5/5] ASoC: DaVinci: pcm, fix underruns by using sram

[Mark Brown]

On Mon, Aug 10, 2009 at 05:32:19PM -0700, Troy Kisky wrote:

> On question I had concerns davinci_pcm_hardware. It is currently for both playback
> and capture. Since allocate_sram contains "davinci_pcm_hardware.period_bytes_max = size;,"
> should I change davinci_pcm_hardware to playback_pcm_hardware, capture_pcm_hardware?

I guess you'll need to, yes.

Re: [PATCH 3/5] ASoC: DaVinci: i2s, reduce underruns by combining into 1 element

[Mark Brown]

On Thu, Aug 06, 2009 at 04:55:33PM -0700, Troy Kisky wrote:

> @@ -37,6 +37,12 @@ struct snd_platform_data {
>  	u32 rx_dma_offset;
>  	enum dma_event_q eventq_no;	/* event queue number */
>  	unsigned int codec_fmt;
> +	/*
> +	 * Allowing this is more efficient and eliminates left and right swaps
> +	 * caused by underruns, but will swap the left and right channels
> +	 * when compared to previous behavior.
> +	 */
> +	unsigned disable_channel_combine:1;

It seems rather surprising to have this be enabled by default given that
it does swap the channels - I'd expect it to catch people out and it's
not really obvious that Linux would do something like this and it's not
quite so discoverable as it might be.  On the other hand, it does sound
like a useful performance win though I'm not sure how bad the problems
are in general and if the use of SRAM won't deal with things well
enough.

I'll apply the patch, copying the comment here into the commit message
to make things more discoverable, but I'd strongly suggest considering a
followup patch which changes the default to the original behaviour.  It
might even be worth making the behaviour runtime configurable.

Re: [PATCH 3/5] ASoC: DaVinci: i2s, reduce underruns by combining into 1 element

[Mark Brown]

On Sat, Aug 08, 2009 at 09:59:11AM +0100, Mark Brown wrote:

> I'll apply the patch, copying the comment here into the commit message
> to make things more discoverable, but I'd strongly suggest considering a
> followup patch which changes the default to the original behaviour.  It
> might even be worth making the behaviour runtime configurable.

...or not, the change to asp.h doesn't apply at all in ASoC - the
platform data is only present in Kevin's tree at the minute.  I'm happy
for it to get merged that way, but best to check for cross tree issues
and I would like to see something about the L/R swap in the commit log
for discoverabiity (or the default changing).

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 1/5] ASoC: DaVinci: i2s: don't bounce through rtd to get dai

[Mark Brown]

On Thu, Aug 06, 2009 at 04:55:31PM -0700, Troy Kisky wrote:
> dai is a parameter to the functions, so use it instead of
> looking it up.

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

I've applied this and patch 1, thanks.  The other patches look OK at
first glance but I've not reviewed them properly yet.