Re: [PATCH 4/4] ASoC: DaVinci: pcm, fix underrun by using sram

Re: [PATCH 4/4] ASoC: DaVinci: pcm, fix underrun by using sram

[troy.kisky]

  BODY { font-family:Arial, Helvetica, sans-serif;font-size:12px; } 
 On Tue 13/07/10 6:00 AM , "Nori, Sekhar" nsekhar@ti.com sent:
	Hi Troy,
 On Tue, Nov 17, 2009 at 05:22:34, 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 
 [...]
 > +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 link;
 > + struct davinci_pcm_dma_params *dma_data = prtd->params;
 > +
 > + /* Request ram master channel */
 > + link = prtd->ram_channel = edma_alloc_channel(EDMA_CHANNEL_ANY,
 > + davinci_pcm_dma_irq, substream,
 > + EVENTQ_1);
 What is the reason for choosing EVENTQ_1 for this channel? EVENTQ_0
 is already being used for ASP channel.
 I imagine it will be much easier to tune the queue usage in the
system
 if all of audio data was using the same queue.
 I am working on a patch which lets platform specify the event queues
 for audio DMA. I am not sure if I really need to make a provision
for
 two different queues to be specified - that's why I ask.
 Thanks,
 Sekhar
	The reason is so that the IRAM data can be fetched and used while 

	EVENTQ_1 fetches the next buffer of data from sdram into IRAM. 
	Troy 

	 

RE: RE: [PATCH 4/4] ASoC: DaVinci: pcm, fix underrun by using sram

[Nori, Sekhar]

On Wed, Jul 14, 2010 at 00:31:00, troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org wrote:
>
> On Tue 13/07/10 6:00 AM , "Nori, Sekhar" nsekhar-l0cyMroinI0@public.gmane.org sent:
>
>       [...]
>
>       > +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 link;
>       > + struct davinci_pcm_dma_params *dma_data = prtd->params;
>       > +
>       > + /* Request ram master channel */
>       > + link = prtd->ram_channel =
> edma_alloc_channel(EDMA_CHANNEL_ANY,
>       > + davinci_pcm_dma_irq, substream,
>       > + EVENTQ_1);
>
>       What is the reason for choosing EVENTQ_1 for this channel?
> EVENTQ_0
>       is already being used for ASP channel.
>
>       I imagine it will be much easier to tune the queue usage in the
> system
>       if all of audio data was using the same queue.
>
>       I am working on a patch which lets platform specify the event
> queues
>       for audio DMA. I am not sure if I really need to make a
> provision for
>       two different queues to be specified - that's why I ask.
>
>       Thanks,
>       Sekhar
>
>       The reason is so that the IRAM data can be fetched and used
> while
>
>       EVENTQ_1 fetches the next buffer of data from sdram into IRAM.

Thanks for the explanation. That sounds reasonable.

Just curious as to whether you actually faced an issue
when using the same event queue for both transfers?

I just tested this on DM365 with both transfers on EDMAQ_0
with 16K each of capture and playback IRAM at 48KHz
sampling rate and did not find any issue.

Anyway it makes sense to make provision for platform to choose
different queues for both transfers so will implement my patch
that way.

Thanks,
Sekhar

Re: [PATCH 4/4] ASoC: DaVinci: pcm, fix underrun by using sram

[Troy Kisky]

Nori, Sekhar wrote:
> On Wed, Jul 14, 2010 at 00:31:00, troy.kisky@boundarydevices.com wrote:
>> On Tue 13/07/10 6:00 AM , "Nori, Sekhar" nsekhar@ti.com sent:
>>
>
>>       The reason is so that the IRAM data can be fetched and used
>> while
>>
>>       EVENTQ_1 fetches the next buffer of data from sdram into IRAM.
> 
> Thanks for the explanation. That sounds reasonable.
> 
> Just curious as to whether you actually faced an issue
> when using the same event queue for both transfers?
> 
> I just tested this on DM365 with both transfers on EDMAQ_0
> with 16K each of capture and playback IRAM at 48KHz
> sampling rate and did not find any issue.
> 
> Anyway it makes sense to make provision for platform to choose
> different queues for both transfers so will implement my patch
> that way.
> 
> Thanks,
> Sekhar
> 

IIRC, the DM365 has a fifo, the 6443/6446 doesn't. So, the 365 doesn't
need to use IRAM and shouldn't use it. Test using the same controller
on a board without a fifo.

Troy

[PATCH 1/4] ASoC: DaVinci: remove requirement that dma_params is 1st in structure

[Troy Kisky]

Remove requirement that dma_params is 1st in the structures
davinci_audio_dev and davinci_mcbsp_dev.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 4ae7070..9e69a4e 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -98,11 +98,6 @@ enum {
 };
 
 struct davinci_mcbsp_dev {
-	/*
-	 * dma_params must be first because rtd->dai->cpu_dai->private_data
-	 * is cast to a pointer of an array of struct davinci_pcm_dma_params in
-	 * davinci_pcm_open.
-	 */
 	struct davinci_pcm_dma_params	dma_params[2];
 	void __iomem			*base;
 #define MOD_DSP_A	0
@@ -547,6 +542,7 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 	dev->dma_params[SNDRV_PCM_STREAM_CAPTURE].channel = res->start;
 
 	davinci_i2s_dai.private_data = dev;
+	davinci_i2s_dai.dma_data = dev->dma_params;
 	ret = snd_soc_register_dai(&davinci_i2s_dai);
 	if (ret != 0)
 		goto err_free_mem;
diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 5d1f98a..5023915 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -907,6 +907,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 
 	dma_data->channel = res->start;
 	davinci_mcasp_dai[pdata->op_mode].private_data = dev;
+	davinci_mcasp_dai[pdata->op_mode].dma_data = dev->dma_params;
 	davinci_mcasp_dai[pdata->op_mode].dev = &pdev->dev;
 	ret = snd_soc_register_dai(&davinci_mcasp_dai[pdata->op_mode]);
 
diff --git a/sound/soc/davinci/davinci-mcasp.h b/sound/soc/davinci/davinci-mcasp.h
index 9d179cc..582c924 100644
--- a/sound/soc/davinci/davinci-mcasp.h
+++ b/sound/soc/davinci/davinci-mcasp.h
@@ -39,11 +39,6 @@ enum {
 };
 
 struct davinci_audio_dev {
-	/*
-	 * dma_params must be first because rtd->dai->cpu_dai->private_data
-	 * is cast to a pointer of an array of struct davinci_pcm_dma_params in
-	 * davinci_pcm_open.
-	 */
 	struct davinci_pcm_dma_params dma_params[2];
 	void __iomem *base;
 	int sample_rate;
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index c73a915..c2c2e85 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -238,10 +238,11 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 	struct davinci_runtime_data *prtd;
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_pcm_dma_params *pa = rtd->dai->cpu_dai->private_data;
-	struct davinci_pcm_dma_params *params = &pa[substream->stream];
-	if (!params)
+	struct davinci_pcm_dma_params *pa = rtd->dai->cpu_dai->dma_data;
+	struct davinci_pcm_dma_params *params;
+	if (!pa)
 		return -ENODEV;
+	params = &pa[substream->stream];
 
 	snd_soc_set_runtime_hwparams(substream, &davinci_pcm_hardware);
 	/* ensure that buffer size is a multiple of period size */
-- 
1.5.6.3

[PATCH 2/4] 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          |   74 ++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h
index 18e4ce3..019c647 100644
--- a/arch/arm/mach-davinci/include/mach/asp.h
+++ b/arch/arm/mach-davinci/include/mach/asp.h
@@ -51,6 +51,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 enable_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 9e69a4e..771cabe 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,
+};
+
 struct davinci_mcbsp_dev {
 	struct davinci_pcm_dma_params	dma_params[2];
 	void __iomem			*base;
@@ -105,6 +122,27 @@ struct davinci_mcbsp_dev {
 	int				mode;
 	u32				pcr;
 	struct clk			*clk;
+	/*
+	 * 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 enable_channel_combine:1;
 };
 
 static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
@@ -344,6 +382,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);
@@ -373,27 +413,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;
 	}
-
-	dma_params->acnt  = dma_params->data_type;
-	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->enable_channel_combine) {
+			element_cnt = 1;
+			fmt = double_fmt[fmt];
+		}
+	}
+	dma_params->acnt = 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);
@@ -508,7 +543,8 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto err_release_region;
 	}
-
+	if (pdata)
+		dev->enable_channel_combine = pdata->enable_channel_combine;
 	dev->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		ret = -ENODEV;
-- 
1.5.6.3

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

[Troy Kisky]

Rename variable master_lch to asp_channel
Rename variable slave_lch to asp_link[0]
Rename local variables:
	lch to link
	count to asp_count
	src to asp_src
	dst to asp_dst

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

diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index c2c2e85..7757010 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_channel;	/* Master DMA channel */
+	int asp_link[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 link = prtd->asp_link[0];
 	unsigned int period_size;
 	unsigned int dma_offset;
 	dma_addr_t dma_pos;
@@ -75,7 +75,7 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	dma_pos = runtime->dma_addr + dma_offset;
 
 	pr_debug("davinci_pcm: audio_set_dma_params_play channel = %d "
-		"dma_ptr = %x period_size=%x\n", lch, dma_pos, period_size);
+		"dma_ptr = %x period_size=%x\n", link, dma_pos, period_size);
 
 	data_type = prtd->params->data_type;
 	count = period_size / data_type;
@@ -93,23 +93,23 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	}
 
 	acnt = prtd->params->acnt;
-	edma_set_src(lch, src, INCR, W8BIT);
-	edma_set_dest(lch, dst, INCR, W8BIT);
-	edma_set_src_index(lch, src_bidx, 0);
-	edma_set_dest_index(lch, dst_bidx, 0);
-	edma_set_transfer_params(lch, acnt, count, 1, 0, ASYNC);
+	edma_set_src(link, src, INCR, W8BIT);
+	edma_set_dest(link, dst, INCR, W8BIT);
+	edma_set_src_index(link, src_bidx, 0);
+	edma_set_dest_index(link, dst_bidx, 0);
+	edma_set_transfer_params(link, acnt, count, 1, 0, ASYNC);
 
 	prtd->period++;
 	if (unlikely(prtd->period >= runtime->periods))
 		prtd->period = 0;
 }
 
-static void davinci_pcm_dma_irq(unsigned lch, u16 ch_status, void *data)
+static void davinci_pcm_dma_irq(unsigned link, u16 ch_status, void *data)
 {
 	struct snd_pcm_substream *substream = data;
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
 
-	pr_debug("davinci_pcm: lch=%d, status=0x%x\n", lch, ch_status);
+	pr_debug("davinci_pcm: link=%d, status=0x%x\n", link, ch_status);
 
 	if (unlikely(ch_status != DMA_COMPLETE))
 		return;
@@ -135,15 +135,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_channel = 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_channel), EDMA_SLOT_ANY);
 	if (ret < 0) {
-		edma_free_channel(prtd->master_lch);
+		edma_free_channel(prtd->asp_channel);
 		return ret;
 	}
-	prtd->slave_lch = ret;
+	prtd->asp_link[0] = ret;
 
 	/* Issue transfer completion IRQ when the channel completes a
 	 * transfer, then always reload from the same slot (by a kind
@@ -154,10 +154,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[0], &p_ram);
+	p_ram.opt |= TCINTEN | EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_channel));
+	p_ram.link_bcntrld = EDMA_CHAN_SLOT(prtd->asp_link[0]) << 5;
+	edma_write_slot(prtd->asp_link[0], &p_ram);
 
 	return 0;
 }
@@ -173,12 +173,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_channel);
 		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_channel);
 		break;
 	default:
 		ret = -EINVAL;
@@ -199,8 +199,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[0], &temp);
+	edma_write_slot(prtd->asp_channel, &temp);
 	davinci_pcm_enqueue_dma(substream);
 
 	return 0;
@@ -212,20 +212,20 @@ 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->master_lch, &src, &dst);
+	edma_get_position(prtd->asp_channel, &asp_src, &asp_dst);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		count = src - runtime->dma_addr;
+		asp_count = asp_src - runtime->dma_addr;
 	else
-		count = dst - runtime->dma_addr;
+		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;
 
@@ -274,10 +274,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[0]);
 
-	edma_free_slot(prtd->slave_lch);
-	edma_free_channel(prtd->master_lch);
+	edma_free_slot(prtd->asp_link[0]);
+	edma_free_channel(prtd->asp_channel);
 
 	kfree(prtd);
 
-- 
1.5.6.3

[PATCH 4/4] ASoC: DaVinci: pcm, fix underrun 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          |    7 +-
 sound/soc/davinci/davinci-pcm.c          |  512 +++++++++++++++++++++++++++---
 sound/soc/davinci/davinci-pcm.h          |    1 +
 4 files changed, 476 insertions(+), 46 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/asp.h b/arch/arm/mach-davinci/include/mach/asp.h
index 019c647..e07f70e 100644
--- a/arch/arm/mach-davinci/include/mach/asp.h
+++ b/arch/arm/mach-davinci/include/mach/asp.h
@@ -57,6 +57,8 @@ struct snd_platform_data {
 	 * when compared to previous behavior.
 	 */
 	unsigned enable_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 771cabe..1d3a678 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -543,8 +543,13 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto err_release_region;
 	}
-	if (pdata)
+	if (pdata) {
 		dev->enable_channel_combine = pdata->enable_channel_combine;
+		dev->dma_params[SNDRV_PCM_STREAM_PLAYBACK].sram_size =
+			pdata->sram_size_playback;
+		dev->dma_params[SNDRV_PCM_STREAM_CAPTURE].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 7757010..ae9fe75 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,10 +24,51 @@
 
 #include <asm/dma.h>
 #include <mach/edma.h>
+#include <mach/sram.h>
 
 #include "davinci-pcm.h"
 
-static struct snd_pcm_hardware davinci_pcm_hardware = {
+#ifdef DEBUG
+static void print_buf_info(int slot, char *name)
+{
+	struct edmacc_param p;
+	if (slot < 0)
+		return;
+	edma_read_slot(slot, &p);
+	printk(KERN_DEBUG "%s: 0x%x, opt=%x, src=%x, a_b_cnt=%x dst=%x\n",
+			name, slot, 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 slot, char *name)
+{
+}
+#endif
+
+static struct snd_pcm_hardware pcm_hardware_playback = {
+	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
+		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
+		 SNDRV_PCM_INFO_PAUSE),
+	.formats = (SNDRV_PCM_FMTBIT_S16_LE),
+	.rates = (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
+		  SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 |
+		  SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
+		  SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |
+		  SNDRV_PCM_RATE_KNOT),
+	.rate_min = 8000,
+	.rate_max = 96000,
+	.channels_min = 2,
+	.channels_max = 2,
+	.buffer_bytes_max = 128 * 1024,
+	.period_bytes_min = 32,
+	.period_bytes_max = 8 * 1024,
+	.periods_min = 16,
+	.periods_max = 255,
+	.fifo_size = 0,
+};
+
+static struct snd_pcm_hardware pcm_hardware_capture = {
 	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		 SNDRV_PCM_INFO_PAUSE),
@@ -48,14 +90,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_link2
+ * ram_link2 - copys rest of SDRAM to iram in ping_size units,
+ * 	links to ram_link
+ * ram_link - copys entire SDRAM to iram in ping_size uints,
+ * 	links to self
+ *
+ * asp_params - same as asp_link[0]
+ * asp_link[0] - copys from lower half of iram to asp port
+ * 	links to asp_link[1], triggers iram copy event on completion
+ * asp_link[1] - copys from upper half of iram to asp port
+ * 	links to asp_link[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,
+ * 	links to ram_link
+ * ram_link - same as playback
+ * 	links to self
+ *
+ * asp_params - same as playback
+ * asp_link[0] - same as playback
+ * asp_link[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_channel;	/* Master DMA channel */
 	int asp_link[2];	/* asp parameter link channel, ping/pong */
 	struct davinci_pcm_dma_params *params;	/* DMA params */
+	int ram_channel;
+	int ram_link;
+	int ram_link2;
+	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;
@@ -109,41 +195,282 @@ static void davinci_pcm_dma_irq(unsigned link, 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_channel, "i ram_channel");
 	pr_debug("davinci_pcm: link=%d, status=0x%x\n", link, ch_status);
 
 	if (unlikely(ch_status != DMA_COMPLETE))
 		return;
 
 	if (snd_pcm_running(substream)) {
+		if (prtd->ram_channel < 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_pcm_hardware *ppcm)
+{
+	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;
+
+	ppcm->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;
+}
 
-		spin_lock(&prtd->lock);
-		davinci_pcm_enqueue_dma(substream);
-		spin_unlock(&prtd->lock);
+/*
+ * 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 davinci_pcm_dma_params *dma_data = prtd->params;
+	unsigned int data_type = dma_data->data_type;
+	unsigned int acnt = dma_data->acnt;
+	/* divide by 2 for ping/pong */
+	unsigned int ping_size = snd_pcm_lib_period_bytes(substream) >> 1;
+	int link = prtd->asp_link[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(link, asp_src_pong, INCR, W8BIT);
+
+		link = prtd->asp_link[0];
+		edma_set_src_index(link, data_type, 0);
+		link = prtd->asp_link[1];
+		edma_set_src_index(link, data_type, 0);
+
+		link = prtd->ram_link;
+		edma_set_src(link, 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(link, asp_dst_pong, INCR, W8BIT);
+
+		link = prtd->asp_link[0];
+		edma_set_dest_index(link, data_type, 0);
+		link = prtd->asp_link[1];
+		edma_set_dest_index(link, data_type, 0);
+
+		link = prtd->ram_link;
+		edma_set_dest(link, runtime->dma_addr, INCR, W32BIT);
+	}
+
+	link = prtd->asp_link[0];
+	edma_set_transfer_params(link, acnt,
+			ping_size/data_type, 1, 0, ASYNC);
+	link = prtd->asp_link[1];
+	edma_set_transfer_params(link, acnt,
+			ping_size/data_type, 1, 0, ASYNC);
+
+
+	link = prtd->ram_link;
+	edma_set_src_index(link, ping_size, ram_src_cidx);
+	edma_set_dest_index(link, ping_size, ram_dst_cidx);
+	edma_set_transfer_params(link, ping_size, 2,
+			runtime->periods, 2, ASYNC);
+
+	/* init master params */
+	edma_read_slot(prtd->asp_link[0], &prtd->asp_params);
+	edma_read_slot(prtd->ram_link, &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, &p_ram);
+		p_ram.src += (ping_size << 1);
+		p_ram.ccnt -= 1;
+		edma_write_slot(prtd->ram_link2, &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_link2) << 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 link;
+	struct davinci_pcm_dma_params *dma_data = prtd->params;
+
+	/* Request ram master channel */
+	link = prtd->ram_channel = edma_alloc_channel(EDMA_CHANNEL_ANY,
+				  davinci_pcm_dma_irq, substream,
+				  EVENTQ_1);
+	if (link < 0)
+		goto exit1;
+
+	/* Request ram link channel */
+	link = prtd->ram_link = edma_alloc_slot(
+			EDMA_CTLR(prtd->ram_channel), EDMA_SLOT_ANY);
+	if (link < 0)
+		goto exit2;
+
+	link = prtd->asp_link[1] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_channel), EDMA_SLOT_ANY);
+	if (link < 0)
+		goto exit3;
+
+	prtd->ram_link2 = -1;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		link = prtd->ram_link2 = edma_alloc_slot(
+			EDMA_CTLR(prtd->ram_channel), EDMA_SLOT_ANY);
+		if (link < 0)
+			goto exit4;
+	}
+	/* circle ping-pong buffers */
+	edma_link(prtd->asp_link[0], prtd->asp_link[1]);
+	edma_link(prtd->asp_link[1], prtd->asp_link[0]);
+	/* circle ram buffers */
+	edma_link(prtd->ram_link, prtd->ram_link);
+
+	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 */
+	link = prtd->asp_link[0];
+	edma_set_src(link, asp_src_ping, INCR, W16BIT);
+	edma_set_dest(link, asp_dst_ping, INCR, W16BIT);
+	edma_set_src_index(link, 0, 0);
+	edma_set_dest_index(link, 0, 0);
+
+	edma_read_slot(link, &prtd->asp_params);
+	prtd->asp_params.opt &= ~(TCCMODE | EDMA_TCC(0x3f) | TCINTEN);
+	prtd->asp_params.opt |= TCCHEN | EDMA_TCC(prtd->ram_channel & 0x3f);
+	edma_write_slot(link, &prtd->asp_params);
+
+	/* pong */
+	link = prtd->asp_link[1];
+	edma_set_src(link, asp_src_ping, INCR, W16BIT);
+	edma_set_dest(link, asp_dst_ping, INCR, W16BIT);
+	edma_set_src_index(link, 0, 0);
+	edma_set_dest_index(link, 0, 0);
+
+	edma_read_slot(link, &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_channel));
+	edma_write_slot(link, &prtd->asp_params);
+
+	/* ram */
+	link = prtd->ram_link;
+	edma_set_src(link, iram_dma->addr, INCR, W32BIT);
+	edma_set_dest(link, 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_channel, prtd->ram_link, prtd->ram_link2,
+		prtd->asp_channel, prtd->asp_link[0],
+		prtd->asp_link[1]);
+	return 0;
+exit4:
+	edma_free_channel(prtd->asp_link[1]);
+	prtd->asp_link[1] = -1;
+exit3:
+	edma_free_channel(prtd->ram_link);
+	prtd->ram_link = -1;
+exit2:
+	edma_free_channel(prtd->ram_channel);
+	prtd->ram_channel = -1;
+exit1:
+	return link;
 }
 
 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 edmacc_param p_ram;
-	int ret;
+	struct davinci_pcm_dma_params *dma_data = prtd->params;
+	int link;
 
-	/* 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_channel = ret;
+	if (!dma_data)
+		return -ENODEV;
 
-	/* Request parameter RAM reload slot */
-	ret = edma_alloc_slot(EDMA_CTLR(prtd->asp_channel), EDMA_SLOT_ANY);
-	if (ret < 0) {
-		edma_free_channel(prtd->asp_channel);
-		return ret;
+	/* Request asp master DMA channel */
+	link = prtd->asp_channel = edma_alloc_channel(dma_data->channel,
+			davinci_pcm_dma_irq, substream, EVENTQ_0);
+	if (link < 0)
+		goto exit1;
+
+	/* Request asp link channels */
+	link = prtd->asp_link[0] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_channel), EDMA_SLOT_ANY);
+	if (link < 0)
+		goto exit2;
+
+	iram_dma = (struct snd_dma_buffer *)substream->dma_buffer.private_data;
+	if (iram_dma) {
+		if (request_ping_pong(substream, prtd, iram_dma) == 0)
+			return 0;
+		printk(KERN_WARNING "%s: dma channel allocation failed,"
+				"not using sram\n", __func__);
 	}
-	prtd->asp_link[0] = ret;
 
 	/* Issue transfer completion IRQ when the channel completes a
 	 * transfer, then always reload from the same slot (by a kind
@@ -154,12 +481,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[0], &p_ram);
-	p_ram.opt |= TCINTEN | EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_channel));
-	p_ram.link_bcntrld = EDMA_CHAN_SLOT(prtd->asp_link[0]) << 5;
-	edma_write_slot(prtd->asp_link[0], &p_ram);
-
+	edma_read_slot(link, &prtd->asp_params);
+	prtd->asp_params.opt |= TCINTEN |
+		EDMA_TCC(EDMA_CHAN_SLOT(prtd->asp_channel));
+	prtd->asp_params.link_bcntrld = EDMA_CHAN_SLOT(link) << 5;
+	edma_write_slot(link, &prtd->asp_params);
 	return 0;
+exit2:
+	edma_free_channel(prtd->asp_channel);
+	prtd->asp_channel = -1;
+exit1:
+	return link;
 }
 
 static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
@@ -173,12 +505,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_channel);
+		edma_resume(prtd->asp_channel);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		edma_stop(prtd->asp_channel);
+		edma_pause(prtd->asp_channel);
 		break;
 	default:
 		ret = -EINVAL;
@@ -193,14 +525,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_channel >= 0) {
+		int ret = ping_pong_dma_setup(substream);
+		if (ret < 0)
+			return ret;
+
+		edma_write_slot(prtd->ram_channel, &prtd->ram_params);
+		edma_write_slot(prtd->asp_channel, &prtd->asp_params);
+
+		print_buf_info(prtd->ram_channel, "ram_channel");
+		print_buf_info(prtd->ram_link, "ram_link");
+		print_buf_info(prtd->ram_link2, "ram_link2");
+		print_buf_info(prtd->asp_channel, "asp_channel");
+		print_buf_info(prtd->asp_link[0], "asp_link[0]");
+		print_buf_info(prtd->asp_link[1], "asp_link[1]");
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			/* copy 1st iram buffer */
+			edma_start(prtd->ram_channel);
+		}
+		edma_start(prtd->asp_channel);
+		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[0], &temp);
-	edma_write_slot(prtd->asp_channel, &temp);
+	edma_read_slot(prtd->asp_link[0], &prtd->asp_params);
+	edma_write_slot(prtd->asp_channel, &prtd->asp_params);
 	davinci_pcm_enqueue_dma(substream);
 
 	return 0;
@@ -216,13 +569,46 @@ davinci_pcm_pointer(struct snd_pcm_substream *substream)
 	dma_addr_t asp_src, asp_dst;
 
 	spin_lock(&prtd->lock);
-
-	edma_get_position(prtd->asp_channel, &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;
-
+	if (prtd->ram_channel >= 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_channel,
+					&ram_src, &ram_dst);
+			edma_get_position(prtd->asp_channel,
+					&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_channel,
+					&ram_src, &ram_dst);
+			ram_count = ram_dst - prtd->ram_params.dst;
+		}
+		asp_count = ram_count;
+	} else {
+		edma_get_position(prtd->asp_channel, &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, asp_count);
@@ -236,6 +622,7 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct davinci_runtime_data *prtd;
+	struct snd_pcm_hardware *ppcm;
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct davinci_pcm_dma_params *pa = rtd->dai->cpu_dai->dma_data;
@@ -244,7 +631,10 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 		return -ENODEV;
 	params = &pa[substream->stream];
 
-	snd_soc_set_runtime_hwparams(substream, &davinci_pcm_hardware);
+	ppcm = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+			&pcm_hardware_playback : &pcm_hardware_capture;
+	allocate_sram(substream, params->sram_size, ppcm);
+	snd_soc_set_runtime_hwparams(substream, ppcm);
 	/* ensure that buffer size is a multiple of period size */
 	ret = snd_pcm_hw_constraint_integer(runtime,
 						SNDRV_PCM_HW_PARAM_PERIODS);
@@ -257,6 +647,11 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream)
 
 	spin_lock_init(&prtd->lock);
 	prtd->params = params;
+	prtd->asp_channel = -1;
+	prtd->asp_link[0] = prtd->asp_link[1] = -1;
+	prtd->ram_channel = -1;
+	prtd->ram_link = -1;
+	prtd->ram_link2 = -1;
 
 	runtime->private_data = prtd;
 
@@ -274,10 +669,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[0]);
-
-	edma_free_slot(prtd->asp_link[0]);
-	edma_free_channel(prtd->asp_channel);
+	if (prtd->ram_channel >= 0)
+		edma_stop(prtd->ram_channel);
+	if (prtd->asp_channel >= 0)
+		edma_stop(prtd->asp_channel);
+	if (prtd->asp_link[0] >= 0)
+		edma_unlink(prtd->asp_link[0]);
+	if (prtd->asp_link[1] >= 0)
+		edma_unlink(prtd->asp_link[1]);
+	if (prtd->ram_link >= 0)
+		edma_unlink(prtd->ram_link);
+
+	if (prtd->asp_link[0] >= 0)
+		edma_free_slot(prtd->asp_link[0]);
+	if (prtd->asp_link[1] >= 0)
+		edma_free_slot(prtd->asp_link[1]);
+	if (prtd->asp_channel >= 0)
+		edma_free_channel(prtd->asp_channel);
+	if (prtd->ram_link >= 0)
+		edma_free_slot(prtd->ram_link);
+	if (prtd->ram_link2 >= 0)
+		edma_free_slot(prtd->ram_link2);
+	if (prtd->ram_channel >= 0)
+		edma_free_channel(prtd->ram_channel);
 
 	kfree(prtd);
 
@@ -319,11 +733,11 @@ static struct snd_pcm_ops davinci_pcm_ops = {
 	.mmap = 	davinci_pcm_mmap,
 };
 
-static int davinci_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
+static int davinci_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream,
+		size_t size)
 {
 	struct snd_pcm_substream *substream = pcm->streams[stream].substream;
 	struct snd_dma_buffer *buf = &substream->dma_buffer;
-	size_t size = davinci_pcm_hardware.buffer_bytes_max;
 
 	buf->dev.type = SNDRV_DMA_TYPE_DEV;
 	buf->dev.dev = pcm->card->dev;
@@ -348,6 +762,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;
@@ -359,6 +774,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);
+		}
 	}
 }
 
@@ -376,14 +796,16 @@ static int davinci_pcm_new(struct snd_card *card,
 
 	if (dai->playback.channels_min) {
 		ret = davinci_pcm_preallocate_dma_buffer(pcm,
-			SNDRV_PCM_STREAM_PLAYBACK);
+			SNDRV_PCM_STREAM_PLAYBACK,
+			pcm_hardware_playback.buffer_bytes_max);
 		if (ret)
 			return ret;
 	}
 
 	if (dai->capture.channels_min) {
 		ret = davinci_pcm_preallocate_dma_buffer(pcm,
-			SNDRV_PCM_STREAM_CAPTURE);
+			SNDRV_PCM_STREAM_CAPTURE,
+			pcm_hardware_capture.buffer_bytes_max);
 		if (ret)
 			return ret;
 	}
diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index 8746606..ca995ca 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -20,6 +20,7 @@ struct davinci_pcm_dma_params {
 	int channel;			/* sync dma channel ID */
 	unsigned short acnt;
 	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 4/4] ASoC: DaVinci: pcm, fix underrun by using sram

[Nori, Sekhar]

Hi Troy,

On Tue, Nov 17, 2009 at 05:22:34, 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-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

[...]

> +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 link;
> +     struct davinci_pcm_dma_params *dma_data = prtd->params;
> +
> +     /* Request ram master channel */
> +     link = prtd->ram_channel = edma_alloc_channel(EDMA_CHANNEL_ANY,
> +                               davinci_pcm_dma_irq, substream,
> +                               EVENTQ_1);

What is the reason for choosing EVENTQ_1 for this channel? EVENTQ_0
is already being used for ASP channel.

I imagine it will be much easier to tune the queue usage in the system
if all of audio data was using the same queue.

I am working on a patch which lets platform specify the event queues
for audio DMA. I am not sure if I really need to make a provision for
two different queues to be specified - that's why I ask.

Thanks,
Sekhar