davinci-i2c,pcm updates

davinci-i2c,pcm updates

[Troy Kisky]

This patch set is against git://git.alsa-project.org/alsa-kernel.git
but the last patch in series won't compile until the sram allocator patch
is available.

[PATCH V1 01/11] ASoC: DaVinci: i2s, remove MOD_REG_BIT macro

[Troy Kisky]

No functional changes. Rename variable w to something
more meaningful. Remove code obfuscating macro MOD_REG_BIT.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index b1ea52f..bf5ec4b 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -85,14 +85,6 @@
 #define DAVINCI_MCBSP_PCR_FSRM		(1 << 10)
 #define DAVINCI_MCBSP_PCR_FSXM		(1 << 11)
 
-#define MOD_REG_BIT(val, mask, set) do { \
-	if (set) { \
-		val |= mask; \
-	} else { \
-		val &= ~mask; \
-	} \
-} while (0)
-
 enum {
 	DAVINCI_MCBSP_WORD_8 = 0,
 	DAVINCI_MCBSP_WORD_12,
@@ -133,13 +125,13 @@ static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_platform *platform = socdev->card->platform;
-	u32 w;
+	u32 spcr;
 	int ret;
 
 	/* Start the sample generator and enable transmitter/receiver */
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-	MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_GRST, 1);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	spcr |= DAVINCI_MCBSP_SPCR_GRST;
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		/* Stop the DMA to avoid data loss */
@@ -152,17 +144,17 @@ static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 		}
 
 		/* Enable the transmitter */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_XRST, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+		spcr |= DAVINCI_MCBSP_SPCR_XRST;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
 		/* wait for any unexpected frame sync error to occur */
 		udelay(100);
 
 		/* Disable the transmitter to clear any outstanding XSYNCERR */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_XRST, 0);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+		spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
 		/* Restart the DMA */
 		if (platform->pcm_ops->trigger) {
@@ -172,40 +164,39 @@ static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 				printk(KERN_DEBUG "Playback DMA start failed\n");
 		}
 		/* Enable the transmitter */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_XRST, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+		spcr |= DAVINCI_MCBSP_SPCR_XRST;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
 	} else {
 
 		/* Enable the reciever */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_RRST, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+		spcr |= DAVINCI_MCBSP_SPCR_RRST;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	}
 
 
 	/* Start frame sync */
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-	MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_FRST, 1);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	spcr |= DAVINCI_MCBSP_SPCR_FRST;
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 }
 
 static void davinci_mcbsp_stop(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
-	u32 w;
+	u32 spcr;
 
 	/* Reset transmitter/receiver and sample rate/frame sync generators */
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-	MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_GRST |
-		       DAVINCI_MCBSP_SPCR_FRST, 0);
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	spcr &= ~(DAVINCI_MCBSP_SPCR_GRST | DAVINCI_MCBSP_SPCR_FRST);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_XRST, 0);
+		spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
 	else
-		MOD_REG_BIT(w, DAVINCI_MCBSP_SPCR_RRST, 0);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr &= ~DAVINCI_MCBSP_SPCR_RRST;
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 }
 
 static int davinci_i2s_startup(struct snd_pcm_substream *substream,
@@ -358,25 +349,26 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	struct snd_interval *i = NULL;
 	int mcbsp_word_length;
-	u32 w;
+	unsigned int rcr, xcr, srgr;
+	u32 spcr;
 
 	/* general line settings */
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		w |= DAVINCI_MCBSP_SPCR_RINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	} else {
-		w |= DAVINCI_MCBSP_SPCR_XINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, w);
+		spcr |= DAVINCI_MCBSP_SPCR_XINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	}
 
 	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
-	w = DAVINCI_MCBSP_SRGR_FSGM;
-	MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1), 1);
+	srgr = DAVINCI_MCBSP_SRGR_FSGM;
+	srgr |= DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1);
 
 	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
-	MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1), 1);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, w);
+	srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 
 	/* Determine xfer data type */
 	switch (params_format(params)) {
@@ -398,16 +390,16 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_RCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
-			       DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length), 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, w);
+		rcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_RCR_REG);
+		rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
+			       DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
 
 	} else {
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_XCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
-			       DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length), 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, w);
+		xcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_XCR_REG);
+		xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
+				DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length);
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
 
 	}
 	return 0;
-- 
1.5.6.3

[PATCH V1 02/11] ASoC: DaVinci: i2s toggle clock to complete reset

[Troy Kisky]

Add toggle_clock function to complete i2s reset earlier.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index bf5ec4b..dd44167 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -104,6 +104,7 @@ static struct davinci_pcm_dma_params davinci_i2s_pcm_in = {
 
 struct davinci_mcbsp_dev {
 	void __iomem			*base;
+	u32				pcr;
 	struct clk			*clk;
 	struct davinci_pcm_dma_params	*dma_params[2];
 };
@@ -119,17 +120,34 @@ static inline u32 davinci_mcbsp_read_reg(struct davinci_mcbsp_dev *dev, int reg)
 	return __raw_readl(dev->base + reg);
 }
 
+static void toggle_clock(struct davinci_mcbsp_dev *dev, int playback)
+{
+	u32 m = playback ? DAVINCI_MCBSP_PCR_CLKXP : DAVINCI_MCBSP_PCR_CLKRP;
+	/* The clock needs to toggle to complete reset.
+	 * So, fake it by toggling the clk polarity.
+	 */
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, dev->pcr ^ m);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, dev->pcr);
+}
+
 static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_platform *platform = socdev->card->platform;
+	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	u32 spcr;
 	int ret;
-
-	/* Start the sample generator and enable transmitter/receiver */
+	u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST;
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	if (spcr & mask) {
+		/* start off disabled */
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG,
+				spcr & ~mask);
+		toggle_clock(dev, playback);
+	}
+	/* Start the sample generator and enable transmitter/receiver */
 	spcr |= DAVINCI_MCBSP_SPCR_GRST;
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
@@ -155,6 +173,7 @@ static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
 		spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+		toggle_clock(dev, playback);
 
 		/* Restart the DMA */
 		if (platform->pcm_ops->trigger) {
@@ -188,15 +207,14 @@ static void davinci_mcbsp_stop(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	u32 spcr;
+	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 
 	/* Reset transmitter/receiver and sample rate/frame sync generators */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
 	spcr &= ~(DAVINCI_MCBSP_SPCR_GRST | DAVINCI_MCBSP_SPCR_FRST);
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		spcr &= ~DAVINCI_MCBSP_SPCR_XRST;
-	else
-		spcr &= ~DAVINCI_MCBSP_SPCR_RRST;
+	spcr &= playback ? ~DAVINCI_MCBSP_SPCR_XRST : ~DAVINCI_MCBSP_SPCR_RRST;
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+	toggle_clock(dev, playback);
 }
 
 static int davinci_i2s_startup(struct snd_pcm_substream *substream,
@@ -334,6 +352,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
+	dev->pcr = pcr;
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
-- 
1.5.6.3

[PATCH V1 03/11] ASoc: DaVinci: i2s, minor cleanup

[Troy Kisky]

Add davinci_mcbsp_dev as argument to davinci_mcbsp_start
and davinci_mcbsp_stop.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index dd44167..f7d7e4c 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -130,10 +130,10 @@ static void toggle_clock(struct davinci_mcbsp_dev *dev, int playback)
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, dev->pcr);
 }
 
-static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
+static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
+		struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	struct snd_soc_device *socdev = rtd->socdev;
 	struct snd_soc_platform *platform = socdev->card->platform;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
@@ -202,12 +202,9 @@ static void davinci_mcbsp_start(struct snd_pcm_substream *substream)
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 }
 
-static void davinci_mcbsp_stop(struct snd_pcm_substream *substream)
+static void davinci_mcbsp_stop(struct davinci_mcbsp_dev *dev, int playback)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	u32 spcr;
-	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 
 	/* Reset transmitter/receiver and sample rate/frame sync generators */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
@@ -427,18 +424,21 @@ static int davinci_i2s_hw_params(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;
 	int ret = 0;
+	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		davinci_mcbsp_start(substream);
+		davinci_mcbsp_start(dev, substream);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		davinci_mcbsp_stop(substream);
+		davinci_mcbsp_stop(dev, playback);
 		break;
 	default:
 		ret = -EINVAL;
-- 
1.5.6.3

[PATCH V1 04/11] ASoC: DaVinci: i2s cleanup

[Troy Kisky]

Move variable declaration closer to use.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index f7d7e4c..73a0fd8 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -138,7 +138,6 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
 	struct snd_soc_platform *platform = socdev->card->platform;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	u32 spcr;
-	int ret;
 	u32 mask = playback ? DAVINCI_MCBSP_SPCR_XRST : DAVINCI_MCBSP_SPCR_RRST;
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
 	if (spcr & mask) {
@@ -155,7 +154,7 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
 		/* Stop the DMA to avoid data loss */
 		/* while the transmitter is out of reset to handle XSYNCERR */
 		if (platform->pcm_ops->trigger) {
-			ret = platform->pcm_ops->trigger(substream,
+			int ret = platform->pcm_ops->trigger(substream,
 				SNDRV_PCM_TRIGGER_STOP);
 			if (ret < 0)
 				printk(KERN_DEBUG "Playback DMA stop failed\n");
@@ -177,7 +176,7 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
 
 		/* Restart the DMA */
 		if (platform->pcm_ops->trigger) {
-			ret = platform->pcm_ops->trigger(substream,
+			int ret = platform->pcm_ops->trigger(substream,
 				SNDRV_PCM_TRIGGER_START);
 			if (ret < 0)
 				printk(KERN_DEBUG "Playback DMA start failed\n");
-- 
1.5.6.3

[PATCH V1 05/11] ASoC: DaVinci: i2s, only start sample generator if needed

[Troy Kisky]

Only start sample generator if needed, and more
cleanup on davinci_mcbsp_start.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 73a0fd8..4260965 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -146,11 +146,14 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
 				spcr & ~mask);
 		toggle_clock(dev, playback);
 	}
-	/* Start the sample generator and enable transmitter/receiver */
-	spcr |= DAVINCI_MCBSP_SPCR_GRST;
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+	if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM |
+			DAVINCI_MCBSP_PCR_CLKXM | DAVINCI_MCBSP_PCR_CLKRM)) {
+		/* Start the sample generator */
+		spcr |= DAVINCI_MCBSP_SPCR_GRST;
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+	}
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+	if (playback) {
 		/* Stop the DMA to avoid data loss */
 		/* while the transmitter is out of reset to handle XSYNCERR */
 		if (platform->pcm_ops->trigger) {
@@ -181,23 +184,16 @@ static void davinci_mcbsp_start(struct davinci_mcbsp_dev *dev,
 			if (ret < 0)
 				printk(KERN_DEBUG "Playback DMA start failed\n");
 		}
-		/* Enable the transmitter */
-		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		spcr |= DAVINCI_MCBSP_SPCR_XRST;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-
-	} else {
-
-		/* Enable the reciever */
-		spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-		spcr |= DAVINCI_MCBSP_SPCR_RRST;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	}
 
-
-	/* Start frame sync */
+	/* Enable transmitter or receiver */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-	spcr |= DAVINCI_MCBSP_SPCR_FRST;
+	spcr |= mask;
+
+	if (dev->pcr & (DAVINCI_MCBSP_PCR_FSXM | DAVINCI_MCBSP_PCR_FSRM)) {
+		/* Start frame sync */
+		spcr |= DAVINCI_MCBSP_SPCR_FRST;
+	}
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 }
 
-- 
1.5.6.3

[PATCH V1 06/11] ASoC: DaVinci: i2s, minor cleanup of davinci_i2s_startup

[Troy Kisky]

Save a few lines of code.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 4260965..5b31b56 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -210,14 +210,10 @@ static void davinci_mcbsp_stop(struct davinci_mcbsp_dev *dev, int playback)
 }
 
 static int davinci_i2s_startup(struct snd_pcm_substream *substream,
-			       struct snd_soc_dai *dai)
+			       struct snd_soc_dai *cpu_dai)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
-	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
-
+	struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
 	cpu_dai->dma_data = dev->dma_params[substream->stream];
-
 	return 0;
 }
 
-- 
1.5.6.3

[PATCH V1 07/11] ASoC: DaVinci: i2s, fix mcbsp_word_length update

[Troy Kisky]

Code previously just "ors" in this field without clearing
first. Fix, by never reading this register.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 5b31b56..6fa1b6a 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -63,6 +63,7 @@
 #define DAVINCI_MCBSP_RCR_RWDLEN1(v)	((v) << 5)
 #define DAVINCI_MCBSP_RCR_RFRLEN1(v)	((v) << 8)
 #define DAVINCI_MCBSP_RCR_RDATDLY(v)	((v) << 16)
+#define DAVINCI_MCBSP_RCR_RFIG		(1 << 18)
 #define DAVINCI_MCBSP_RCR_RWDLEN2(v)	((v) << 21)
 
 #define DAVINCI_MCBSP_XCR_XWDLEN1(v)	((v) << 5)
@@ -104,6 +105,9 @@ static struct davinci_pcm_dma_params davinci_i2s_pcm_in = {
 
 struct davinci_mcbsp_dev {
 	void __iomem			*base;
+#define MOD_DSP_A	0
+#define MOD_DSP_B	1
+	int				mode;
 	u32				pcr;
 	struct clk			*clk;
 	struct davinci_pcm_dma_params	*dma_params[2];
@@ -225,12 +229,11 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
 	unsigned int pcr;
 	unsigned int srgr;
-	unsigned int rcr;
-	unsigned int xcr;
 	srgr = DAVINCI_MCBSP_SRGR_FSGM |
 		DAVINCI_MCBSP_SRGR_FPER(DEFAULT_BITPERSAMPLE * 2 - 1) |
 		DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
 
+	/* set master/slave audio interface */
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 		/* cpu is master */
@@ -255,11 +258,8 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
-	rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1);
-	xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1);
+	/* interface format */
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
-	case SND_SOC_DAIFMT_DSP_B:
-		break;
 	case SND_SOC_DAIFMT_I2S:
 		/* Davinci doesn't support TRUE I2S, but some codecs will have
 		 * the left and right channels contiguous. This allows
@@ -279,8 +279,10 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		 */
 		fmt ^= SND_SOC_DAIFMT_NB_IF;
 	case SND_SOC_DAIFMT_DSP_A:
-		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
-		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
+		dev->mode = MOD_DSP_A;
+		break;
+	case SND_SOC_DAIFMT_DSP_B:
+		dev->mode = MOD_DSP_B;
 		break;
 	default:
 		printk(KERN_ERR "%s:bad format\n", __func__);
@@ -342,8 +344,6 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 	dev->pcr = pcr;
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
 	return 0;
 }
 
@@ -377,6 +377,15 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 
+	rcr = DAVINCI_MCBSP_RCR_RFIG;
+	xcr = DAVINCI_MCBSP_XCR_XFIG;
+	if (dev->mode == MOD_DSP_B) {
+		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(0);
+		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(0);
+	} else {
+		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
+		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
+	}
 	/* Determine xfer data type */
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S8:
@@ -396,19 +405,18 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		rcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_RCR_REG);
-		rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
-			       DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
+	rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(1);
+	xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(1);
 
-	} else {
-		xcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_XCR_REG);
-		xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
-				DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
+	rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
+		DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
+	xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
+		DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length);
 
-	}
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
+	else
+		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
 	return 0;
 }
 
-- 
1.5.6.3

[PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono

[Troy Kisky]

This patch will reduce the number of underruns by
shifting out 32 bit values instead of 16 bit. It also
adds mono support.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 sound/soc/davinci/davinci-i2s.c |  124 +++++++++++++++++++++++++++------------
 sound/soc/davinci/davinci-pcm.c |   31 +++++++---
 sound/soc/davinci/davinci-pcm.h |    3 +-
 3 files changed, 110 insertions(+), 48 deletions(-)

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 6fa1b6a..a2ad53e 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -356,26 +356,29 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	struct snd_interval *i = NULL;
 	int mcbsp_word_length;
+	int bits_per_sample;
+	int bits_per_frame;
 	unsigned int rcr, xcr, srgr;
+	int channels;
+	int format;
+	int element_cnt = 1;
 	u32 spcr;
 
-	/* general line settings */
-	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-	} else {
-		spcr |= DAVINCI_MCBSP_SPCR_XINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
-	}
-
 	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
-	srgr = DAVINCI_MCBSP_SRGR_FSGM;
-	srgr |= DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1);
+	bits_per_sample = snd_interval_value(i);
+	/* always 2 samples/frame, mono will convert to stereo */
+	bits_per_frame = bits_per_sample << 1;
+	srgr = DAVINCI_MCBSP_SRGR_FSGM |
+		DAVINCI_MCBSP_SRGR_FPER(bits_per_frame - 1) |
+		DAVINCI_MCBSP_SRGR_FWID(bits_per_sample - 1);
 
-	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
-	srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
+	/* general line settings */
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	spcr |= DAVINCI_MCBSP_SPCR_FREE;
+	spcr |= (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+			DAVINCI_MCBSP_SPCR_XINTM(3) :
+			DAVINCI_MCBSP_SPCR_RINTM(3);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 
 	rcr = DAVINCI_MCBSP_RCR_RFIG;
 	xcr = DAVINCI_MCBSP_XCR_XFIG;
@@ -386,33 +389,80 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
 	}
+	channels = params_channels(params);
+	format = params_format(params);
 	/* 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:
-		printk(KERN_WARNING "davinci-i2s: unsupported PCM format\n");
-		return -EINVAL;
+	if (channels == 2) {
+		/* Combining both channels into 1 element can allow x10 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.
+		 * So, you may want to let the codec know to swap them back.
+		 *
+		 * It may be x10 instead of x2 because the clock from the codec
+		 * may run at mclk speed (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 mclk speed.
+		 *
+		 * In the now very 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.
+		 */
+		dma_params->convert_mono_stereo = 0;
+		switch (format) {
+		case SNDRV_PCM_FORMAT_S8:
+			dma_params->data_type = 2;	/* 2 byte frame */
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_16;
+			break;
+		case SNDRV_PCM_FORMAT_S16_LE:
+			dma_params->data_type = 4;	/* 4 byte frame */
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
+			break;
+		case SNDRV_PCM_FORMAT_S32_LE:
+			element_cnt = 2;
+			dma_params->data_type = 4;	/* 4 byte element */
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
+			break;
+		default:
+			printk(KERN_WARNING
+					"davinci-i2s: unsupported PCM format");
+			return -EINVAL;
+		}
+	} else {
+		dma_params->convert_mono_stereo = 1;
+		/* 1 element in ram becomes 2 for stereo */
+		element_cnt = 2;
+		switch (format) {
+		case SNDRV_PCM_FORMAT_S8:
+			/* 1 byte frame in ram */
+			dma_params->data_type = 1;
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_8;
+			break;
+		case SNDRV_PCM_FORMAT_S16_LE:
+			/* 2 byte frame in ram */
+			dma_params->data_type = 2;
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_16;
+			break;
+		case SNDRV_PCM_FORMAT_S32_LE:
+			/* 4 byte element */
+			dma_params->data_type = 4;
+			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
+			break;
+		default:
+			printk(KERN_WARNING
+					"davinci-i2s: unsupported PCM format");
+			return -EINVAL;
+		}
 	}
-
-	rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(1);
-	xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(1);
+	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);
 	xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
 		DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length);
-
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
 	else
@@ -542,12 +592,12 @@ struct snd_soc_dai davinci_i2s_dai = {
 	.probe = davinci_i2s_probe,
 	.remove = davinci_i2s_remove,
 	.playback = {
-		.channels_min = 2,
+		.channels_min = 1,
 		.channels_max = 2,
 		.rates = DAVINCI_I2S_RATES,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,},
 	.capture = {
-		.channels_min = 2,
+		.channels_min = 1,
 		.channels_max = 2,
 		.rates = DAVINCI_I2S_RATES,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE,},
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index a059965..2002fdc 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -65,9 +65,9 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	unsigned int dma_offset;
 	dma_addr_t dma_pos;
 	dma_addr_t src, dst;
-	unsigned short src_bidx, dst_bidx;
-	unsigned int data_type;
 	unsigned int count;
+	unsigned int data_type = prtd->params->data_type;
+	unsigned int convert_mono_stereo = prtd->params->convert_mono_stereo;
 
 	period_size = snd_pcm_lib_period_bytes(substream);
 	dma_offset = prtd->period * period_size;
@@ -76,26 +76,37 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
 	pr_debug("davinci_pcm: audio_set_dma_params_play channel = %d "
 		"dma_ptr = %x period_size=%x\n", lch, dma_pos, period_size);
 
-	data_type = prtd->params->data_type;
 	count = period_size / data_type;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		src = dma_pos;
 		dst = prtd->params->dma_addr;
-		src_bidx = data_type;
-		dst_bidx = 0;
+		if (convert_mono_stereo)
+			edma_set_src_index(lch, 0, data_type);
+		else
+			edma_set_src_index(lch, data_type, 0);
+		edma_set_dest_index(lch, 0, 0);
 	} else {
 		src = prtd->params->dma_addr;
 		dst = dma_pos;
-		src_bidx = 0;
-		dst_bidx = data_type;
+		edma_set_src_index(lch, 0, 0);
+		if (convert_mono_stereo)
+			edma_set_dest_index(lch, 0, data_type);
+		else
+			edma_set_dest_index(lch, data_type, 0);
 	}
 
 	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, data_type, count, 1, 0, ASYNC);
+	if (convert_mono_stereo) {
+		/*
+		 * Each byte is sent twice, so
+		 * A_CNT * B_CNT * C_CNT = 2 * period_size
+		 */
+		edma_set_transfer_params(lch, data_type, 2, count, 2, ASYNC);
+	} else {
+		edma_set_transfer_params(lch, data_type, count, 1, 0, ASYNC);
+	}
 
 	prtd->period++;
 	if (unlikely(prtd->period >= runtime->periods))
diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index 62cb4eb..fc70161 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -16,7 +16,8 @@ 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 int data_type;	/* xfer data type */
+	unsigned char data_type;	/* xfer data type */
+	unsigned char convert_mono_stereo;
 };
 
 struct evm_snd_platform_data {
-- 
1.5.6.3

[PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Troy Kisky]

If the codec is master then prepare should call
mcbsp_start, not trigger.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index a2ad53e..f3e7bf8 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -470,6 +470,20 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+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;
+	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+	davinci_mcbsp_stop(dev, playback);
+	if ((dev->pcr & DAVINCI_MCBSP_PCR_FSXM) == 0) {
+		/* codec is master */
+		davinci_mcbsp_start(dev, substream);
+	}
+	return 0;
+}
+
 static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 			       struct snd_soc_dai *dai)
 {
@@ -477,6 +491,8 @@ static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
 	int ret = 0;
 	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+	if ((dev->pcr & DAVINCI_MCBSP_PCR_FSXM) == 0)
+		return 0;	/* return if codec is master */
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -492,10 +508,18 @@ static int davinci_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 	default:
 		ret = -EINVAL;
 	}
-
 	return ret;
 }
 
+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;
+	int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
+	davinci_mcbsp_stop(dev, playback);
+}
+
 static int davinci_i2s_probe(struct platform_device *pdev,
 			     struct snd_soc_dai *dai)
 {
@@ -581,6 +605,8 @@ static void davinci_i2s_remove(struct platform_device *pdev,
 
 static struct snd_soc_dai_ops davinci_i2s_dai_ops = {
 	.startup	= davinci_i2s_startup,
+	.shutdown	= davinci_i2s_shutdown,
+	.prepare	= davinci_i2s_prepare,
 	.trigger	= davinci_i2s_trigger,
 	.hw_params	= davinci_i2s_hw_params,
 	.set_fmt	= davinci_i2s_set_dai_fmt,
-- 
1.5.6.3

[PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates

[Troy Kisky]

If the codec is master, we support anything
that the codec supports.

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

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index f3e7bf8..f4b25ce 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -601,7 +601,8 @@ static void davinci_i2s_remove(struct platform_device *pdev,
 	release_mem_region(mem->start, (mem->end - mem->start) + 1);
 }
 
-#define DAVINCI_I2S_RATES	SNDRV_PCM_RATE_8000_96000
+#define DAVINCI_I2S_RATES	(SNDRV_PCM_RATE_8000_96000 |\
+	SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS)
 
 static struct snd_soc_dai_ops davinci_i2s_dai_ops = {
 	.startup	= davinci_i2s_startup,
-- 
1.5.6.3

[PATCH V1 11/11] 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>
---
 sound/soc/davinci/davinci-pcm.c |  520 ++++++++++++++++++++++++++++++---------
 1 files changed, 403 insertions(+), 117 deletions(-)

diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index 2002fdc..754f0b2 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 IRAM 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,31 @@
 
 #include <asm/dma.h>
 #include <mach/edma.h>
+#include <mach/sram.h>
 
 #include "davinci-pcm.h"
 
+#define DAVINCI_PCM_DEBUG 0
+#if DAVINCI_PCM_DEBUG
+#define DPRINTK(format, arg...) printk(KERN_DEBUG format, ## arg)
+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
+#define DPRINTK(format, arg...) do {} while (0)
+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 |
@@ -42,142 +65,323 @@ static struct snd_pcm_hardware davinci_pcm_hardware = {
 	.channels_max = 2,
 	.buffer_bytes_max = 128 * 1024,
 	.period_bytes_min = 32,
-	.period_bytes_max = 8 * 1024,
+	.period_bytes_max = 7 * 512,	/* This is size of ping + pong buffer*/
 	.periods_min = 16,
 	.periods_max = 255,
 	.fifo_size = 0,
 };
 
+/*
+ * How it 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 master_lch;		/* Master DMA channel */
-	int slave_lch;		/* linked parameter RAM reload slot */
-	struct davinci_pcm_dma_params *params;	/* DMA params */
+	int asp_master_lch;		/* Master DMA channel */
+	int asp_link_lch[2];	/* asp parameter link channel */
+	int ram_master_lch;
+	int ram_link_lch;
+	int ram_link_lch2;
+	struct edmacc_param asp_params;
+	struct edmacc_param ram_params;
 };
 
-static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
+
+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;
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	int lch = prtd->slave_lch;
-	unsigned int period_size;
-	unsigned int dma_offset;
-	dma_addr_t dma_pos;
-	dma_addr_t src, dst;
-	unsigned int count;
-	unsigned int data_type = prtd->params->data_type;
-	unsigned int convert_mono_stereo = prtd->params->convert_mono_stereo;
-
-	period_size = snd_pcm_lib_period_bytes(substream);
-	dma_offset = prtd->period * period_size;
-	dma_pos = runtime->dma_addr + dma_offset;
+	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;
 
-	pr_debug("davinci_pcm: audio_set_dma_params_play channel = %d "
-		"dma_ptr = %x period_size=%x\n", lch, dma_pos, period_size);
+	if (snd_pcm_running(substream))
+		snd_pcm_period_elapsed(substream);
+}
 
-	count = period_size / data_type;
 
+/*
+ * This is called after runtime->dma_addr, period_bytes and data_type are valid
+ */
+static int davinci_pcm_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;
+	unsigned int convert_mono_stereo = dma_data->convert_mono_stereo;
+	/* 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) {
-		src = dma_pos;
-		dst = prtd->params->dma_addr;
-		if (convert_mono_stereo)
+		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];
+		if (convert_mono_stereo) {
 			edma_set_src_index(lch, 0, data_type);
-		else
+			lch = prtd->asp_link_lch[1];
+			edma_set_src_index(lch, 0, data_type);
+		} else {
+			edma_set_src_index(lch, data_type, 0);
+			lch = prtd->asp_link_lch[1];
 			edma_set_src_index(lch, data_type, 0);
-		edma_set_dest_index(lch, 0, 0);
+		}
+
+		lch = prtd->ram_link_lch;
+		edma_set_src(lch, runtime->dma_addr, INCR, W32BIT);
 	} else {
-		src = prtd->params->dma_addr;
-		dst = dma_pos;
-		edma_set_src_index(lch, 0, 0);
-		if (convert_mono_stereo)
+		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];
+		if (convert_mono_stereo) {
+			edma_set_dest_index(lch, 0, data_type);
+			lch = prtd->asp_link_lch[1];
 			edma_set_dest_index(lch, 0, data_type);
-		else
+		} else {
+			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);
 	}
 
-	edma_set_src(lch, src, INCR, W8BIT);
-	edma_set_dest(lch, dst, INCR, W8BIT);
+	lch = prtd->asp_link_lch[0];
 	if (convert_mono_stereo) {
 		/*
 		 * Each byte is sent twice, so
-		 * A_CNT * B_CNT * C_CNT = 2 * period_size
+		 * A_CNT * B_CNT * C_CNT = 2 * ping_size
 		 */
-		edma_set_transfer_params(lch, data_type, 2, count, 2, ASYNC);
+		edma_set_transfer_params(lch, data_type,
+				2, ping_size/data_type, 2, ASYNC);
+		lch = prtd->asp_link_lch[1];
+		edma_set_transfer_params(lch, data_type,
+				2, ping_size/data_type, 2, ASYNC);
 	} else {
-		edma_set_transfer_params(lch, data_type, count, 1, 0, ASYNC);
+		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);
 	}
 
-	prtd->period++;
-	if (unlikely(prtd->period >= runtime->periods))
-		prtd->period = 0;
-}
 
-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;
-
-	pr_debug("davinci_pcm: lch=%d, status=0x%x\n", lch, ch_status);
+	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);
 
-	if (unlikely(ch_status != DMA_COMPLETE))
-		return;
-
-	if (snd_pcm_running(substream)) {
-		snd_pcm_period_elapsed(substream);
-
-		spin_lock(&prtd->lock);
-		davinci_pcm_enqueue_dma(substream);
-		spin_unlock(&prtd->lock);
+	/* 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 davinci_pcm_dma_request(struct snd_pcm_substream *substream)
 {
-	struct davinci_runtime_data *prtd = substream->runtime->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_dma_buffer *iram_dma =
+		(struct snd_dma_buffer *)substream->dma_buffer.private_data;
+	struct davinci_runtime_data *prtd = 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;
+
+	dma_addr_t asp_src_ping;
+	dma_addr_t asp_dst_ping;
+
 	int ret;
+	int lch;
 
-	if (!dma_data)
+	if ((!dma_data) || (!iram_dma))
 		return -ENODEV;
 
-	prtd->params = dma_data;
-
-	/* Request master DMA channel */
-	ret = edma_alloc_channel(prtd->params->channel,
+	/* Request ram master channel */
+	ret = prtd->ram_master_lch = edma_alloc_channel(EDMA_CHANNEL_ANY,
 				  davinci_pcm_dma_irq, substream,
-				  EVENTQ_0);
+				  EVENTQ_1);
 	if (ret < 0)
-		return ret;
-	prtd->master_lch = ret;
+		goto exit1;
 
-	/* Request parameter RAM reload slot */
-	ret = edma_alloc_slot(EDMA_SLOT_ANY);
-	if (ret < 0) {
-		edma_free_channel(prtd->master_lch);
-		return ret;
+	/* Request ram link channel */
+	ret = prtd->ram_link_lch = edma_alloc_slot(
+			EDMA_CTLR(prtd->ram_master_lch), EDMA_SLOT_ANY);
+	if (ret < 0)
+		goto exit2;
+
+	/* Request asp master DMA channel */
+	ret = prtd->asp_master_lch = edma_alloc_channel(dma_data->channel,
+			  davinci_pcm_dma_irq, substream,
+			  EVENTQ_0);
+	if (ret < 0)
+		goto exit3;
+
+	/* Request asp link channels */
+	ret = prtd->asp_link_lch[0] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
+	if (ret < 0)
+		goto exit4;
+	ret = prtd->asp_link_lch[1] = edma_alloc_slot(
+			EDMA_CTLR(prtd->asp_master_lch), EDMA_SLOT_ANY);
+	if (ret < 0)
+		goto exit5;
+
+	prtd->ram_link_lch2 = -1;
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		ret = prtd->ram_link_lch2 = edma_alloc_slot(
+				EDMA_CTLR(prtd->ram_master_lch), EDMA_SLOT_ANY);
+		if (ret < 0)
+			goto exit6;
 	}
-	prtd->slave_lch = ret;
-
-	/* Issue transfer completion IRQ when the channel completes a
-	 * transfer, then always reload from the same slot (by a kind
-	 * of loopback link).  The completion IRQ handler will update
-	 * the reload slot with a new buffer.
-	 *
-	 * REVISIT save p_ram here after setting up everything except
-	 * 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(prtd->master_lch);
-	p_ram.link_bcntrld = prtd->slave_lch << 5;
-	edma_write_slot(prtd->slave_lch, &p_ram);
+	/* 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, &p_ram);
+	p_ram.opt &= ~(TCCMODE | EDMA_TCC(0x3f) | TCINTEN);
+	p_ram.opt |= TCCHEN | EDMA_TCC(prtd->ram_master_lch & 0x3f);
+	edma_write_slot(lch, &p_ram);
+
+	/* 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, &p_ram);
+	p_ram.opt &= ~(TCCMODE | EDMA_TCC(0x3f));
+	/* interrupt after every pong completion */
+	p_ram.opt |= TCINTEN | TCCHEN | EDMA_TCC(
+			EDMA_CHAN_SLOT(prtd->ram_master_lch));
+	edma_write_slot(lch, &p_ram);
+
+	/* 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;
+exit6:
+	edma_free_channel(prtd->ram_link_lch2);
+exit5:
+	edma_free_channel(prtd->asp_link_lch[0]);
+exit4:
+	edma_free_channel(prtd->asp_master_lch);
+exit3:
+	edma_free_channel(prtd->ram_link_lch);
+exit2:
+	edma_free_channel(prtd->ram_master_lch);
+exit1:
+	return ret;
 }
-
+/* I2S master (codec/cpudai) should start/stop dma request,
+ *  we shouldn't ignore them
+ *  codec AIC33 needs fixed
+ */
+#define BROKEN_MASTER 1
+#ifdef BROKEN_MASTER
 static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
@@ -189,12 +393,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_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->master_lch);
+		edma_pause(prtd->asp_master_lch);
 		break;
 	default:
 		ret = -EINVAL;
@@ -205,19 +409,32 @@ static int davinci_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 
 	return ret;
 }
+#endif
 
 static int davinci_pcm_prepare(struct snd_pcm_substream *substream)
 {
+	int ret;
 	struct davinci_runtime_data *prtd = substream->runtime->private_data;
-	struct edmacc_param temp;
 
-	prtd->period = 0;
-	davinci_pcm_enqueue_dma(substream);
+	ret = davinci_pcm_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);
 
-	/* Copy self-linked parameter RAM entry into master channel */
-	edma_read_slot(prtd->slave_lch, &temp);
-	edma_write_slot(prtd->master_lch, &temp);
+	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;
 }
 
@@ -227,20 +444,44 @@ 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 count_asp, count_ram;
+	int mod_ram;
+	dma_addr_t ram_src, ram_dst;
+	dma_addr_t asp_src, asp_dst;
+	unsigned int period_size = snd_pcm_lib_period_bytes(substream);
 
 	spin_lock(&prtd->lock);
-
-	edma_get_position(prtd->master_lch, &src, &dst);
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		count = src - runtime->dma_addr;
-	else
-		count = dst - runtime->dma_addr;
-
+	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);
+		count_asp = asp_src - prtd->asp_params.src;
+		count_ram = ram_src - prtd->ram_params.src;
+		mod_ram = count_ram % period_size;
+		mod_ram -= count_asp;
+		if (mod_ram < 0)
+			mod_ram += period_size;
+		else if (mod_ram == 0) {
+			if (snd_pcm_running(substream))
+				mod_ram += period_size;
+		}
+		count_ram -= mod_ram;
+		if (count_ram < 0)
+			count_ram += period_size * runtime->periods;
+	} else {
+		edma_get_position(prtd->ram_master_lch,
+				&ram_src, &ram_dst);
+		count_ram = ram_dst - prtd->ram_params.dst;
+	}
 	spin_unlock(&prtd->lock);
 
-	offset = bytes_to_frames(runtime, count);
+	offset = bytes_to_frames(runtime, count_ram);
+	DPRINTK("count_ram=0x%x\n", count_ram);
 	if (offset >= runtime->buffer_size)
 		offset = 0;
 
@@ -254,6 +495,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)
@@ -277,13 +523,22 @@ 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_free_slot(prtd->slave_lch);
-	edma_free_channel(prtd->master_lch);
-
+	edma_stop(prtd->ram_master_lch);
+	edma_stop(prtd->asp_master_lch);
+	edma_unlink(prtd->asp_link_lch[0]);
+	edma_unlink(prtd->asp_link_lch[1]);
+	edma_unlink(prtd->ram_link_lch);
+
+	edma_free_slot(prtd->asp_link_lch[0]);
+	edma_free_slot(prtd->asp_link_lch[1]);
+	edma_free_channel(prtd->asp_master_lch);
+	edma_free_slot(prtd->ram_link_lch);
+	if (prtd->ram_link_lch2 != -1) {
+		edma_free_slot(prtd->ram_link_lch2);
+		prtd->ram_link_lch2 = -1;
+	}
+	edma_free_channel(prtd->ram_master_lch);
 	kfree(prtd);
-
 	return 0;
 }
 
@@ -317,7 +572,9 @@ static struct snd_pcm_ops davinci_pcm_ops = {
 	.hw_params = 	davinci_pcm_hw_params,
 	.hw_free = 	davinci_pcm_hw_free,
 	.prepare = 	davinci_pcm_prepare,
-	.trigger = 	davinci_pcm_trigger,
+#ifdef BROKEN_MASTER
+	.trigger =	davinci_pcm_trigger,
+#endif
 	.pointer = 	davinci_pcm_pointer,
 	.mmap = 	davinci_pcm_mmap,
 };
@@ -327,21 +584,44 @@ static int davinci_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 	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;
+	struct snd_dma_buffer *iram_dma = NULL;
+	dma_addr_t iram_phys = 0;
+	void *iram_virt = NULL;
+	unsigned int iram_size = davinci_pcm_hardware.period_bytes_max;
 
 	buf->dev.type = SNDRV_DMA_TYPE_DEV;
 	buf->dev.dev = pcm->card->dev;
 	buf->private_data = NULL;
+	buf->bytes = size;
 	buf->area = dma_alloc_writecombine(pcm->card->dev, size,
 					   &buf->addr, GFP_KERNEL);
 
 	pr_debug("davinci_pcm: preallocate_dma_buffer: area=%p, addr=%p, "
 		"size=%d\n", (void *) buf->area, (void *) buf->addr, size);
 
-	if (!buf->area)
-		return -ENOMEM;
 
-	buf->bytes = size;
+	if (!buf->area)
+		goto exit1;
+	iram_virt = sram_alloc(iram_size, &iram_phys);
+	if (!iram_virt)
+		goto exit2;
+	iram_dma = kzalloc(sizeof(*iram_dma), GFP_KERNEL);
+	if (!iram_dma)
+		goto exit3;
+	iram_dma->area = iram_virt;
+	iram_dma->addr = iram_phys;
+	memset(iram_dma->area, 0, iram_size);
+	buf->private_data = iram_dma ;
 	return 0;
+	kfree(iram_dma);
+exit3:
+	if (iram_virt)
+		sram_free(iram_virt, iram_size);
+exit2:
+	dma_free_writecombine(pcm->card->dev, size, buf->area, buf->addr);
+	buf->area = NULL;
+exit1:
+	return -ENOMEM;
 }
 
 static void davinci_pcm_free(struct snd_pcm *pcm)
@@ -351,6 +631,8 @@ static void davinci_pcm_free(struct snd_pcm *pcm)
 	int stream;
 
 	for (stream = 0; stream < 2; stream++) {
+		unsigned int iram_size = davinci_pcm_hardware.period_bytes_max;
+		struct snd_dma_buffer *iram_dma;
 		substream = pcm->streams[stream].substream;
 		if (!substream)
 			continue;
@@ -362,6 +644,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_size);
+			kfree(iram_dma);
+		}
 	}
 }
 
@@ -379,18 +666,17 @@ 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);
 		if (ret)
 			return ret;
 	}
 
 	if (dai->capture.channels_min) {
 		ret = davinci_pcm_preallocate_dma_buffer(pcm,
-			SNDRV_PCM_STREAM_CAPTURE);
+				SNDRV_PCM_STREAM_CAPTURE);
 		if (ret)
 			return ret;
 	}
-
 	return 0;
 }
 
-- 
1.5.6.3

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

[Mark Brown]

On Sat, Jul 04, 2009 at 07:30:01PM -0700, 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.

Overall this looks good though clearly there are some cross tree merge
issues which will need to be resolved.  I do have some queries and
concerns below, though.

One more general question is what sort of testing you've done with user
space applications - what sort of things have you tried the new DMA code
with?  I'd be especially interested to see this tested with PulseAudio
simply because that is so good at turning up problems with DMA.

> +#define DAVINCI_PCM_DEBUG 0
> +#if DAVINCI_PCM_DEBUG
> +#define DPRINTK(format, arg...) printk(KERN_DEBUG format, ## arg)

I'd be inclined to use the standard DEBUG and VERBOSE_DEBUG defines
here together with their display macros here.

> +/*
> + * 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
> + */

Indentation.

> -	/* Request parameter RAM reload slot */
> -	ret = edma_alloc_slot(EDMA_SLOT_ANY);
> -	if (ret < 0) {
> -		edma_free_channel(prtd->master_lch);
> -		return ret;
> +	/* Request ram link channel */
> +	ret = prtd->ram_link_lch = edma_alloc_slot(
> +			EDMA_CTLR(prtd->ram_master_lch), EDMA_SLOT_ANY);

The signature for edma_alloc_slot() appears to have changed - that looks
awfully like the SRAM patch or whatever else introduced this change
ought to be including an update for this code to keep it buildable?

> +/* I2S master (codec/cpudai) should start/stop dma request,
> + *  we shouldn't ignore them
> + *  codec AIC33 needs fixed
> + */
> +#define BROKEN_MASTER 1
> +#ifdef BROKEN_MASTER

Could you expand on what's going on here?  The idea that if the CODEC is
master it should be starting DMA sounds wrong - obviously it should know
nothing about the DMA controller.  If you mean that it should start and
stop the clocks that causes issues in situations like TDM since there
can be transfers going on independantly of the CPU which may need the
clocks running.  Not everything will be able to gate the clocks, too.

In any case, this looks like a separate patch that should be split out
of this one.

> +	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);
> +		count_asp = asp_src - prtd->asp_params.src;
> +		count_ram = ram_src - prtd->ram_params.src;
> +		mod_ram = count_ram % period_size;
> +		mod_ram -= count_asp;

Is it perhaps saner to just look at the current position as being the
position of the transfer to SRAM?  This does mean more variation from
the point at which data is audibly playing but on the other hand as far
as the CPU is concerned once the audio gets to SRAM it can't work with
it any longer so it's potentially misleading to report the SRAM
position.  Not all hardware will be able to report the position at all
so userspace ought to be able to cope.

> +	iram_virt = sram_alloc(iram_size, &iram_phys);
> +	if (!iram_virt)
> +		goto exit2;

Should this perhaps be configured via platform data?  You've currently
picked 7 * 512 bytes but there appears to be no urgent reason for that
particular number and presumably SRAM is a limited resource which people
may want to prioritise differently.  That may mean having bigger buffers
for audio as well as less - things like MP3 players can get better
battery life by setting up very big DMAs.  On a similar vein is it worth
considering making this feature entirely optional and/or falling back to
non-SRAM if SRAM allocation fails?

I think we should be able to go with the approach you've got initially
but I suspect that as the SRAM gets more widely used it'll become more
of an issue.

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

[Troy Kisky]

Mark Brown wrote:
> On Sat, Jul 04, 2009 at 07:30:01PM -0700, Troy Kisky wrote:
> If you mean that it should start and
> stop the clocks
Yes.

> that causes issues in situations like TDM since there
> can be transfers going on independantly of the CPU which may need the
> clocks running.  Not everything will be able to gate the clocks, too.

What is TDM? time division multiplexing? You mean the frame carries
more than just audio data? I think I'm misunderstanding something.
Can you elaborate please.


> 
> In any case, this looks like a separate patch that should be split out
> of this one.
> 
>> +	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);
>> +		count_asp = asp_src - prtd->asp_params.src;
>> +		count_ram = ram_src - prtd->ram_params.src;
>> +		mod_ram = count_ram % period_size;
>> +		mod_ram -= count_asp;
> 
> Is it perhaps saner to just look at the current position as being the
> position of the transfer to SRAM?  This does mean more variation from
> the point at which data is audibly playing but on the other hand as far
> as the CPU is concerned once the audio gets to SRAM it can't work with
> it any longer so it's potentially misleading to report the SRAM
> position.  Not all hardware will be able to report the position at all
> so userspace ought to be able to cope.

Hmm. Good point. I just wanted as accurate lip-sync as possible.

> 
>> +	iram_virt = sram_alloc(iram_size, &iram_phys);
>> +	if (!iram_virt)
>> +		goto exit2;
> 
> Should this perhaps be configured via platform data?  You've currently
> picked 7 * 512 bytes but there appears to be no urgent reason for that
> particular number and presumably SRAM is a limited resource which people
> may want to prioritise differently.  That may mean having bigger buffers
> for audio as well as less - things like MP3 players can get better
> battery life by setting up very big DMAs.  On a similar vein is it worth
> considering making this feature entirely optional and/or falling back to
> non-SRAM if SRAM allocation fails?

Yes Chris Ring also asked for that, and it is fairly easy to allocate another
SDRAM buffer to use for the SRAM. But I'd rather another patch after this
if I need to use plaform data. Maybe audio_sram_buffer_size = 0 in platform
data to mean use SDRAM?

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

[Mark Brown]

On Mon, Jul 06, 2009 at 06:10:25PM -0700, Troy Kisky wrote:
> Mark Brown wrote:

> > that causes issues in situations like TDM since there
> > can be transfers going on independantly of the CPU which may need the
> > clocks running.  Not everything will be able to gate the clocks, too.

> What is TDM? time division multiplexing? You mean the frame carries
> more than just audio data? I think I'm misunderstanding something.
> Can you elaborate please.

Yes, time division multiplexing.  The extra signals could be non-audio
or they could be audio.  For example, you could have the CPU, CODEC and
a bluetooth chip connected to a single bus.  In such a scenario you
could have audio going directly between bluetooth and the CODEC with no
CPU involvement (allowing the CPU to suspend itself during a call).

That's not the only case - sometimes people do other things that need
the clocks running even when audio is off like sharing them with some
unrelated functionality.

> > Is it perhaps saner to just look at the current position as being the
> > position of the transfer to SRAM?  This does mean more variation from
> > the point at which data is audibly playing but on the other hand as far

> Hmm. Good point. I just wanted as accurate lip-sync as possible.

Yeah, it's tricky to know what's best.

> > battery life by setting up very big DMAs.  On a similar vein is it worth
> > considering making this feature entirely optional and/or falling back to
> > non-SRAM if SRAM allocation fails?

> Yes Chris Ring also asked for that, and it is fairly easy to allocate another
> SDRAM buffer to use for the SRAM. But I'd rather another patch after this

Incremental patches sounds fine to me.

> if I need to use plaform data. Maybe audio_sram_buffer_size = 0 in platform
> data to mean use SDRAM?

The only potential problem I see there is that that'd make disabling the
SDRAM the default.  I guess if people have been living without it so far
that's not a big problem, though, and it'll stop people getting surprised
by audio grabbing SDRAM when they weren't expecting it.

I guess if you're using SDRAM there's no need for the double buffering
(though that could be a third patch)?

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

[Troy Kisky]

Mark Brown wrote:
>>> battery life by setting up very big DMAs.  On a similar vein is it worth
>>> considering making this feature entirely optional and/or falling back to
>>> non-SRAM if SRAM allocation fails?
> 
>> Yes Chris Ring also asked for that, and it is fairly easy to allocate another
>> SDRAM buffer to use for the SRAM. But I'd rather another patch after this
> 
> Incremental patches sounds fine to me.
> 
>> if I need to use plaform data. Maybe audio_sram_buffer_size = 0 in platform
>> data to mean use SDRAM?
> 
> The only potential problem I see there is that that'd make disabling the
> SDRAM the default.  I guess if people have been living without it so far
> that's not a big problem, though, and it'll stop people getting surprised
> by audio grabbing SDRAM when they weren't expecting it.
> 
> I guess if you're using SDRAM there's no need for the double buffering
> (though that could be a third patch)?

Even when using SDRAM, there is still another advantage to double buffering.
That is tolerating very long interrupt latency on dma completion. The current
code sets up the next periods dma transfer on dma completion. This could easily
lead to underruns. So, even in this case, I think double buffering is worth
the extra dma engine copy.

Troy

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

[Troy Kisky]

Troy Kisky wrote:
> Mark Brown wrote:
> 
> Even when using SDRAM, there is still another advantage to double buffering.
> That is tolerating very long interrupt latency on dma completion. The current
> code sets up the next periods dma transfer on dma completion. This could easily
> lead to underruns. So, even in this case, I think double buffering is worth
> the extra dma engine copy.
> 
I overstated my case, it still would not "easily" happen.

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

[Troy Kisky]

Troy Kisky wrote:
> Troy Kisky wrote:
>> Mark Brown wrote:
>>
>> Even when using SDRAM, there is still another advantage to double buffering.
>> That is tolerating very long interrupt latency on dma completion. The current
>> code sets up the next periods dma transfer on dma completion. This could easily
>> lead to underruns. So, even in this case, I think double buffering is worth
>> the extra dma engine copy.
>>
> I overstated my case, it still would not "easily" happen.
In fact, I now agree with you :^)

Thanks
Troy

Re: [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates

[Mark Brown]

On Sat, Jul 04, 2009 at 07:30:00PM -0700, Troy Kisky wrote:
> If the codec is master, we support anything
> that the codec supports.

Hrm, tricky - the rate configuration doesn't depend on what is master so
this could cause confusion if the codec is slave.

> -#define DAVINCI_I2S_RATES	SNDRV_PCM_RATE_8000_96000
> +#define DAVINCI_I2S_RATES	(SNDRV_PCM_RATE_8000_96000 |\
> +	SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS)

Note that the ASoC core doesn't support _KNOT or _CONTINUOUS (at least
not properly) so the only thing you should get from this is 5512.  Is
that really worth worrying about the master/slave problem?

Re: [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates

[Troy Kisky]

Mark Brown wrote:
> On Sat, Jul 04, 2009 at 07:30:00PM -0700, Troy Kisky wrote:
>> If the codec is master, we support anything
>> that the codec supports.
> 
> Hrm, tricky - the rate configuration doesn't depend on what is master so
> this could cause confusion if the codec is slave.
> 
>> -#define DAVINCI_I2S_RATES	SNDRV_PCM_RATE_8000_96000
>> +#define DAVINCI_I2S_RATES	(SNDRV_PCM_RATE_8000_96000 |\
>> +	SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS)
> 
> Note that the ASoC core doesn't support _KNOT or _CONTINUOUS (at least
> not properly) so the only thing you should get from this is 5512.  Is
> that really worth worrying about the master/slave problem?
> 
Ok. I'll drop this. I just needed it when I was testing all rates my codec supports.
Now that I've tested it, I don't think I'll ever use the other rates.

But even if the cpu is the clock/frame master, the sample rate generator has an 8 bit
divider field, which seems to be always 0 in the current code. And I don't see any reference
to params_rate in the davinci-i2s.c file. Has anyone tried the cpu as master???


Troy

Re: [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates

[Mark Brown]

On Mon, Jul 06, 2009 at 03:01:54PM -0700, Troy Kisky wrote:

> But even if the cpu is the clock/frame master, the sample rate generator has an 8 bit
> divider field, which seems to be always 0 in the current code. And I don't see any reference
> to params_rate in the davinci-i2s.c file. Has anyone tried the cpu as master???

It does appear that way, doesn't it?  But looking at the code
davinci-sffsdr runs with the CPU as frame master so I'd have expected
that it would have shown any problems here.

Re: [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates

[Troy Kisky]

Mark Brown wrote:
> On Mon, Jul 06, 2009 at 03:01:54PM -0700, Troy Kisky wrote:
> 
>> But even if the cpu is the clock/frame master, the sample rate generator has an 8 bit
>> divider field, which seems to be always 0 in the current code. And I don't see any reference
>> to params_rate in the davinci-i2s.c file. Has anyone tried the cpu as master???
> 
> It does appear that way, doesn't it?  But looking at the code
> davinci-sffsdr runs with the CPU as frame master so I'd have expected
> that it would have shown any problems here.
> 
But in this case the clock to divide would come from the CLKR pin, so a divide
by (0+1) would be correct.

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Mark Brown]

On Sat, Jul 04, 2009 at 07:29:59PM -0700, Troy Kisky wrote:
> If the codec is master then prepare should call
> mcbsp_start, not trigger.

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

This and 1-7 all look good, I've applied them.  For this one (and some
of the others) it might've been nice to include a few more words about
the why.

> +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;

No need to bounce through rtd here or in prepare.

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Ambrose, Martin]

On Sat, Jul 04, 2009 at 21:29:59, Troy Kisky wrote:

> If the codec is master then prepare should call
> mcbsp_start, not trigger.

For me this change induces audio artifacts when starting or stopping a gstreamer pipeline.
Take, e.g., the pipeline
	gst-launch -v filesrc location=file.mp3 ! mad ! alsasink

In the setup, before the file is opened, gstreamer places the pipeline into the PAUSE state.
	... posting state-changed READY to PAUSED

This results in a call to the davinci_i2s_prepare function which, when the codec is master,
stops then starts the mcbsp driver. However at this point in the pipeline no valid data has
been provided to the DMA layer (davinci-pcm). Consequently there is random noise of somewhat
random length played out the DAC at the start of file playback. Moreover when the stream changes
from the PAUSE to the PLAYING state another mcbsp_stop/mcbsp_start sequence is performed which
seems inefficient. Similar behavior, and noise, occurs when the pipeline is shutdown.

The aplay utility has a simpler startup/shutdown procedure which doesn't produce any noise.

There doesn't seem to be any state context, e.g. pause vs start, provided to the prepare
function so I can't see how to handle such cases in the current driver while preserving
the use of this patch. If I revert this patch then I don't get any noise but I don't 
know what the consequences are. 

Comments/suggestions welcome.

Regards,
Martin

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Troy Kisky]

On 9/24/2010 9:48 AM, Ambrose, Martin wrote:
> On Sat, Jul 04, 2009 at 21:29:59, Troy Kisky wrote:
> 
>> If the codec is master then prepare should call
>> mcbsp_start, not trigger.
> 
> For me this change induces audio artifacts when starting or stopping a gstreamer pipeline.
> Take, e.g., the pipeline
> 	gst-launch -v filesrc location=file.mp3 ! mad ! alsasink
> 
> In the setup, before the file is opened, gstreamer places the pipeline into the PAUSE state.
> 	... posting state-changed READY to PAUSED
> 
> This results in a call to the davinci_i2s_prepare function which, when the codec is master,
> stops then starts the mcbsp driver. However at this point in the pipeline no valid data has
> been provided to the DMA layer (davinci-pcm). Consequently there is random noise of somewhat
> random length played out the DAC at the start of file playback. Moreover when the stream changes
> from the PAUSE to the PLAYING state another mcbsp_stop/mcbsp_start sequence is performed which
> seems inefficient. Similar behavior, and noise, occurs when the pipeline is shutdown.
> 
> The aplay utility has a simpler startup/shutdown procedure which doesn't produce any noise.
> 
> There doesn't seem to be any state context, e.g. pause vs start, provided to the prepare
> function so I can't see how to handle such cases in the current driver while preserving
> the use of this patch. If I revert this patch then I don't get any noise but I don't 
> know what the consequences are. 
> 
> Comments/suggestions welcome.
> 

I don't see the problem because my codec waits until trigger to activate its interface.

So the question is, who should be responsible for the final turn on?

My thought was that the device (master) which starts the external wires to wiggling should be last.

If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop
or noise.


It would be nice if such rules could be documented, are they???

Thanks
Troy

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Ambrose, Martin]

On Fri, Sep 24, 2010 at 14:14:32, Troy Kisky wrote:

> On 9/24/2010 9:48 AM, Ambrose, Martin wrote:
> > On Sat, Jul 04, 2009 at 21:29:59, Troy Kisky wrote:
> > 
> >> If the codec is master then prepare should call
> >> mcbsp_start, not trigger.
> > 
> > For me this change induces audio artifacts when starting or stopping a gstreamer pipeline.
> > Take, e.g., the pipeline
> > 	gst-launch -v filesrc location=file.mp3 ! mad ! alsasink
> > 
> > In the setup, before the file is opened, gstreamer places the pipeline into the PAUSE state.
> > 	... posting state-changed READY to PAUSED
> > 
> > This results in a call to the davinci_i2s_prepare function which, when the codec is master,
> > stops then starts the mcbsp driver. However at this point in the pipeline no valid data has
> > been provided to the DMA layer (davinci-pcm). Consequently there is random noise of somewhat
> > random length played out the DAC at the start of file playback. Moreover when the stream changes
> > from the PAUSE to the PLAYING state another mcbsp_stop/mcbsp_start sequence is performed which
> > seems inefficient. Similar behavior, and noise, occurs when the pipeline is shutdown.
> > 
> > The aplay utility has a simpler startup/shutdown procedure which doesn't produce any noise.
> > 
> > There doesn't seem to be any state context, e.g. pause vs start, provided to the prepare
> > function so I can't see how to handle such cases in the current driver while preserving
> > the use of this patch. If I revert this patch then I don't get any noise but I don't 
> > know what the consequences are. 
> > 
> > Comments/suggestions welcome.
> > 
> 
> I don't see the problem because my codec waits until trigger to activate its interface.

Thanks for the feedback. I'm using the tlv320aic3x codec driver. Which codec are you using?

> So the question is, who should be responsible for the final turn on?
> 
> My thought was that the device (master) which starts the external wires to wiggling should be last.

Fair enough. I suppose that is the reasoning for the addition of the prepare function.
The assumption being that a cpu_dai callback to the prepare function will always proceed
a call to the codec_dai trigger function. In this way the serial port can be configured and enabled.
Then the codec can turn on the bit/frame clocks which will start the flow of data. 
Although it would seem the codec is unmuted a bit prematurely in this scenario since it happens
before the clocks are enabled in the core prepare function -- at least I think this is the case.

However there would still seem to be spurious, or at least superfluous, calls
to mcbsp_stop/start when just requesting the device to enter the pause state.

I guess the call tree is then different in the case where the cpu, or machine, is the 
clock master. This has pros/cons obviously.

> If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop
> or noise.

I still a newbie when it comes to the ALSA architecture.
Is the proper signal indicating everything is ready, including valid data, a trigger call with cmd=START?

> It would be nice if such rules could be documented, are they???

Not that I know of. 

Regards,
Martin

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Troy Kisky]

>> I don't see the problem because my codec waits until trigger to activate its interface.
> 
> Thanks for the feedback. I'm using the tlv320aic3x codec driver. Which codec are you using?

A modified tlv320aic23. I wrote it before it was available in the kernel, and never finished
a merge.

> 
>> So the question is, who should be responsible for the final turn on?
>>
>> My thought was that the device (master) which starts the external wires to wiggling should be last.
> 
> Fair enough. I suppose that is the reasoning for the addition of the prepare function.
> The assumption being that a cpu_dai callback to the prepare function will always proceed
> a call to the codec_dai trigger function. In this way the serial port can be configured and enabled.
> Then the codec can turn on the bit/frame clocks which will start the flow of data. 
> Although it would seem the codec is unmuted a bit prematurely in this scenario since it happens
> before the clocks are enabled in the core prepare function -- at least I think this is the case.
> 
> However there would still seem to be spurious, or at least superfluous, calls
> to mcbsp_stop/start when just requesting the device to enter the pause state.
> 
> I guess the call tree is then different in the case where the cpu, or machine, is the 
> clock master. This has pros/cons obviously.
> 
>> If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop
>> or noise.
> 
> I still a newbie when it comes to the ALSA architecture.
> Is the proper signal indicating everything is ready, including valid data, a trigger call with cmd=START?
> 

That is my understanding.

I think the main problem with using trigger to start the codec wiggling wires
is the need to use schedule_work to do i2c writes. That's probably why the mainline
aic23 driver doesn't use trigger. Sample below.


static void codec_trigger_deferred(struct work_struct *work)
{
       struct aic23 *aic23 = container_of(work, struct aic23,
                       deferred_trigger_work);
       struct snd_soc_codec *codec = &aic23->codec;
       int playback = aic23->active_mask & ACTIVE_PLAYBACK;
       u16 dia = (aic23->active_mask) ? 1 : 0;
       if (playback) {
               tlv320aic23_set(codec, TLV320AIC23_ACTIVE, 1);
               tlv320aic23_modify(codec, TLV320AIC23_ANLG, 0, TLV320AIC23_DAC_SELECTED);
               tlv320aic23_mute_volume(codec, 0);
       } else {
               tlv320aic23_mute_volume(codec, 1);
               if (dia)
                       tlv320aic23_set(codec, TLV320AIC23_ACTIVE, dia);
               tlv320aic23_modify(codec, TLV320AIC23_ANLG, TLV320AIC23_DAC_SELECTED, 0);
               if (!dia)
                       tlv320aic23_set(codec, TLV320AIC23_ACTIVE, dia);
       }
}

static int tlv320aic23_trigger(struct snd_pcm_substream *substream, int cmd,
                  struct snd_soc_dai *dai)
{
       struct snd_soc_pcm_runtime *rtd = substream->private_data;
       struct snd_soc_device *socdev = rtd->socdev;
       struct snd_soc_codec *codec = socdev->card->codec;
       struct aic23 *aic23 = container_of(codec, struct aic23, codec);
       int ret = 0;
       int playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
       unsigned char mask = (playback)? ACTIVE_PLAYBACK : ACTIVE_CAPTURE;

       switch (cmd) {
       case SNDRV_PCM_TRIGGER_START:
       case SNDRV_PCM_TRIGGER_RESUME:
       case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
               aic23->active_mask |= mask;
               schedule_work(&aic23->deferred_trigger_work);
               break;
       case SNDRV_PCM_TRIGGER_STOP:
       case SNDRV_PCM_TRIGGER_SUSPEND:
       case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
               aic23->active_mask &= ~mask;
               schedule_work(&aic23->deferred_trigger_work);
               /* don't stop driving data lines
                * until digital_mute done
                */
               break;
       default:
       return ret;
}

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Mark Brown]

On Fri, Sep 24, 2010 at 02:43:30PM -0500, Ambrose, Martin wrote:
> On Fri, Sep 24, 2010 at 14:14:32, Troy Kisky wrote:

> > My thought was that the device (master) which starts the external
> > wires to wiggling should be last.

> Fair enough. I suppose that is the reasoning for the addition of the
> prepare function.  The assumption being that a cpu_dai callback to the
> prepare function will always proceed a call to the codec_dai trigger
> function. In this way the serial port can be configured and enabled.

These are all generic ALSA things, not ASoC specific - ASoC is just
passing them straight through.  The idea is that prepare() allows the
driver to finalise the configuration of the hardware and do any setup
that can't be done in an atomic context prior to the trigger, there's no
consideration for any aspect of the ASoC design in this arrangement.

> > If the codec is master, and starts the clocks before the mcbsp is
> > ready that could also cause a pop or noise.

> I still a newbie when it comes to the ALSA architecture.
> Is the proper signal indicating everything is ready, including valid
> data, a trigger call with cmd=START?

Yes, the trigger callback actually starts data flowing.  This is mostly
down to DMA, it's rare for anything except the CPU to be involved in the
trigger operation.

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Troy Kisky]

On 9/26/2010 5:56 PM, Mark Brown wrote:
> Yes, the trigger callback actually starts data flowing.  This is mostly
> down to DMA, it's rare for anything except the CPU to be involved in the
> trigger operation.
> _______________________________________________

So you'll revert the patch?

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Mark Brown]

On Mon, Sep 27, 2010 at 11:50:19AM -0700, Troy Kisky wrote:
> On 9/26/2010 5:56 PM, Mark Brown wrote:

> > Yes, the trigger callback actually starts data flowing.  This is mostly
> > down to DMA, it's rare for anything except the CPU to be involved in the
> > trigger operation.

> So you'll revert the patch?

I'll let you guys sort it out - it's not the end of the world to have it
as it is now, but it could lead to integration issues such as you've
been seeing so some change is probably needed here.

Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown

[Mark Brown]

On Fri, Sep 24, 2010 at 12:14:32PM -0700, Troy Kisky wrote:
> On 9/24/2010 9:48 AM, Ambrose, Martin wrote:

Guys, please fix the configurations of your mail clients to word wrap
within paragraphs at less than 80 columns - makes things much easier to
read.

> My thought was that the device (master) which starts the external
> wires to wiggling should be last.

> If the codec is master, and starts the clocks before the mcbsp is
> ready that could also cause a pop or noise.

That seems unlikely, especially since we keep the output muted until
very late on in the game.  Any noise should be consumed within the time
the mute is running.

There's more often a problem the other way around with things in the CPU
(or the software looking after them) getting upset when they're left
sitting without a clock for too long.

Re: [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono

[Mark Brown]

On Sat, Jul 04, 2009 at 07:29:58PM -0700, Troy Kisky wrote:
> This patch will reduce the number of underruns by
> shifting out 32 bit values instead of 16 bit. It also
> adds mono support.

> +	if (channels == 2) {
> +		/* Combining both channels into 1 element can allow x10 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.
> +		 * So, you may want to let the codec know to swap them back.

This seems dodgy.  The end resule sounds like a win but if it's going to
introduce a L/R swap then that's going to catch people out and not all
CODECs will have the ability to revert the swap.  On the other hand, the 
efficiency wins sound worthwhile from your description.  Is it possible
to make this behaviour optional?

I'm going to be doing some work on digital routing and mixing soon, it
may make sense to hold off on this change and see if it can be
integrated with those changes - it sounds like the sort of thing that we
ought to be able to represent better.  There's a bunch of other stuff
which could be done with better core support in that area.

> +		 *
> +		 * It may be x10 instead of x2 because the clock from the codec
> +		 * may run at mclk speed (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 mclk speed.

I'm also having a really hard time following this bit of the comment,
mostly I think because I'm not sure what clock you're talking about
here.

Re: [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono

[Steve Chen]

On Sat, 2009-07-04 at 19:29 -0700, Troy Kisky wrote:
> This patch will reduce the number of underruns by
> shifting out 32 bit values instead of 16 bit. It also
> adds mono support.

Doesn't ALSA already automatically handle mono-stero conversions?  I
don't think we need to provide the same functionality in the driver.

> 
> Signed-off-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> ---
>  sound/soc/davinci/davinci-i2s.c |  124 +++++++++++++++++++++++++++------------
>  sound/soc/davinci/davinci-pcm.c |   31 +++++++---
>  sound/soc/davinci/davinci-pcm.h |    3 +-
>  3 files changed, 110 insertions(+), 48 deletions(-)
> 
> diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
> index 6fa1b6a..a2ad53e 100644
> --- a/sound/soc/davinci/davinci-i2s.c
> +++ b/sound/soc/davinci/davinci-i2s.c
> @@ -356,26 +356,29 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
>  	struct davinci_mcbsp_dev *dev = rtd->dai->cpu_dai->private_data;
>  	struct snd_interval *i = NULL;
>  	int mcbsp_word_length;
> +	int bits_per_sample;
> +	int bits_per_frame;
>  	unsigned int rcr, xcr, srgr;
> +	int channels;
> +	int format;
> +	int element_cnt = 1;
>  	u32 spcr;
>  
> -	/* general line settings */
> -	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
> -	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
> -		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
> -		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
> -	} else {
> -		spcr |= DAVINCI_MCBSP_SPCR_XINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
> -		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
> -	}
> -
>  	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
> -	srgr = DAVINCI_MCBSP_SRGR_FSGM;
> -	srgr |= DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1);
> +	bits_per_sample = snd_interval_value(i);
> +	/* always 2 samples/frame, mono will convert to stereo */
> +	bits_per_frame = bits_per_sample << 1;
> +	srgr = DAVINCI_MCBSP_SRGR_FSGM |
> +		DAVINCI_MCBSP_SRGR_FPER(bits_per_frame - 1) |
> +		DAVINCI_MCBSP_SRGR_FWID(bits_per_sample - 1);
>  
> -	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
> -	srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
> -	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
> +	/* general line settings */
> +	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
> +	spcr |= DAVINCI_MCBSP_SPCR_FREE;
> +	spcr |= (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> +			DAVINCI_MCBSP_SPCR_XINTM(3) :
> +			DAVINCI_MCBSP_SPCR_RINTM(3);
> +	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
>  
>  	rcr = DAVINCI_MCBSP_RCR_RFIG;
>  	xcr = DAVINCI_MCBSP_XCR_XFIG;
> @@ -386,33 +389,80 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
>  		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
>  		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
>  	}
> +	channels = params_channels(params);
> +	format = params_format(params);
>  	/* 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:
> -		printk(KERN_WARNING "davinci-i2s: unsupported PCM format\n");
> -		return -EINVAL;
> +	if (channels == 2) {
> +		/* Combining both channels into 1 element can allow x10 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.
> +		 * So, you may want to let the codec know to swap them back.
> +		 *
> +		 * It may be x10 instead of x2 because the clock from the codec
> +		 * may run at mclk speed (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 mclk speed.
> +		 *
> +		 * In the now very 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.
> +		 */
> +		dma_params->convert_mono_stereo = 0;
> +		switch (format) {
> +		case SNDRV_PCM_FORMAT_S8:
> +			dma_params->data_type = 2;	/* 2 byte frame */
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_16;
> +			break;
> +		case SNDRV_PCM_FORMAT_S16_LE:
> +			dma_params->data_type = 4;	/* 4 byte frame */
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
> +			break;
> +		case SNDRV_PCM_FORMAT_S32_LE:
> +			element_cnt = 2;
> +			dma_params->data_type = 4;	/* 4 byte element */
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
> +			break;
> +		default:
> +			printk(KERN_WARNING
> +					"davinci-i2s: unsupported PCM format");
> +			return -EINVAL;
> +		}
> +	} else {
> +		dma_params->convert_mono_stereo = 1;
> +		/* 1 element in ram becomes 2 for stereo */
> +		element_cnt = 2;
> +		switch (format) {
> +		case SNDRV_PCM_FORMAT_S8:
> +			/* 1 byte frame in ram */
> +			dma_params->data_type = 1;
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_8;
> +			break;
> +		case SNDRV_PCM_FORMAT_S16_LE:
> +			/* 2 byte frame in ram */
> +			dma_params->data_type = 2;
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_16;
> +			break;
> +		case SNDRV_PCM_FORMAT_S32_LE:
> +			/* 4 byte element */
> +			dma_params->data_type = 4;
> +			mcbsp_word_length = DAVINCI_MCBSP_WORD_32;
> +			break;
> +		default:
> +			printk(KERN_WARNING
> +					"davinci-i2s: unsupported PCM format");
> +			return -EINVAL;
> +		}
>  	}
> -
> -	rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(1);
> -	xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(1);
> +	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);
>  	xcr |= DAVINCI_MCBSP_XCR_XWDLEN1(mcbsp_word_length) |
>  		DAVINCI_MCBSP_XCR_XWDLEN2(mcbsp_word_length);
> -
> +	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>  		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
>  	else
> @@ -542,12 +592,12 @@ struct snd_soc_dai davinci_i2s_dai = {
>  	.probe = davinci_i2s_probe,
>  	.remove = davinci_i2s_remove,
>  	.playback = {
> -		.channels_min = 2,
> +		.channels_min = 1,
>  		.channels_max = 2,
>  		.rates = DAVINCI_I2S_RATES,
>  		.formats = SNDRV_PCM_FMTBIT_S16_LE,},
>  	.capture = {
> -		.channels_min = 2,
> +		.channels_min = 1,
>  		.channels_max = 2,
>  		.rates = DAVINCI_I2S_RATES,
>  		.formats = SNDRV_PCM_FMTBIT_S16_LE,},
> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> index a059965..2002fdc 100644
> --- a/sound/soc/davinci/davinci-pcm.c
> +++ b/sound/soc/davinci/davinci-pcm.c
> @@ -65,9 +65,9 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  	unsigned int dma_offset;
>  	dma_addr_t dma_pos;
>  	dma_addr_t src, dst;
> -	unsigned short src_bidx, dst_bidx;
> -	unsigned int data_type;
>  	unsigned int count;
> +	unsigned int data_type = prtd->params->data_type;
> +	unsigned int convert_mono_stereo = prtd->params->convert_mono_stereo;
>  
>  	period_size = snd_pcm_lib_period_bytes(substream);
>  	dma_offset = prtd->period * period_size;
> @@ -76,26 +76,37 @@ static void davinci_pcm_enqueue_dma(struct snd_pcm_substream *substream)
>  	pr_debug("davinci_pcm: audio_set_dma_params_play channel = %d "
>  		"dma_ptr = %x period_size=%x\n", lch, dma_pos, period_size);
>  
> -	data_type = prtd->params->data_type;
>  	count = period_size / data_type;
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		src = dma_pos;
>  		dst = prtd->params->dma_addr;
> -		src_bidx = data_type;
> -		dst_bidx = 0;
> +		if (convert_mono_stereo)
> +			edma_set_src_index(lch, 0, data_type);
> +		else
> +			edma_set_src_index(lch, data_type, 0);
> +		edma_set_dest_index(lch, 0, 0);
>  	} else {
>  		src = prtd->params->dma_addr;
>  		dst = dma_pos;
> -		src_bidx = 0;
> -		dst_bidx = data_type;
> +		edma_set_src_index(lch, 0, 0);
> +		if (convert_mono_stereo)
> +			edma_set_dest_index(lch, 0, data_type);
> +		else
> +			edma_set_dest_index(lch, data_type, 0);
>  	}
>  
>  	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, data_type, count, 1, 0, ASYNC);
> +	if (convert_mono_stereo) {
> +		/*
> +		 * Each byte is sent twice, so
> +		 * A_CNT * B_CNT * C_CNT = 2 * period_size
> +		 */
> +		edma_set_transfer_params(lch, data_type, 2, count, 2, ASYNC);
> +	} else {
> +		edma_set_transfer_params(lch, data_type, count, 1, 0, ASYNC);
> +	}
>  
>  	prtd->period++;
>  	if (unlikely(prtd->period >= runtime->periods))
> diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
> index 62cb4eb..fc70161 100644
> --- a/sound/soc/davinci/davinci-pcm.h
> +++ b/sound/soc/davinci/davinci-pcm.h
> @@ -16,7 +16,8 @@ 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 int data_type;	/* xfer data type */
> +	unsigned char data_type;	/* xfer data type */
> +	unsigned char convert_mono_stereo;
>  };
>  
>  struct evm_snd_platform_data {

Re: [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono

[Mark Brown]

On Mon, Jul 06, 2009 at 06:09:16AM -0500, Steve Chen wrote:
> On Sat, 2009-07-04 at 19:29 -0700, Troy Kisky wrote:

> > This patch will reduce the number of underruns by
> > shifting out 32 bit values instead of 16 bit. It also
> > adds mono support.

> Doesn't ALSA already automatically handle mono-stero conversions?  I
> don't think we need to provide the same functionality in the driver.

If the hardware can natively play mono then that's less work for the CPU
to do which will improve efficiency.

Re: [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono

[Steve Chen]

On Mon, 2009-07-06 at 12:54 +0100, Mark Brown wrote:
> On Mon, Jul 06, 2009 at 06:09:16AM -0500, Steve Chen wrote:
> > On Sat, 2009-07-04 at 19:29 -0700, Troy Kisky wrote:
> 
> > > This patch will reduce the number of underruns by
> > > shifting out 32 bit values instead of 16 bit. It also
> > > adds mono support.
> 
> > Doesn't ALSA already automatically handle mono-stero conversions?  I
> > don't think we need to provide the same functionality in the driver.
> 
> If the hardware can natively play mono then that's less work for the CPU
> to do which will improve efficiency.

I see.  In this case, DaVinci hardware does not support mono, but the
patch does some DMA index magic to convert between mono and stereo.  It
works just as well then...

snd_pcm_format_set_silence parameter

[Guilherme Longo]

Mates,

What exactly does snd_pcm_format_set_silence is useful for???


That is the code:

if (count < chunk_size)    {
        snd_pcm_format_set_silence(hwparams.format, data + count * 
bits_per_frame / 8, (chunk_size - count) * hwparams.channels);
        count = chunk_size;
    }

Could I get some help?
Thanks

Re: snd_pcm_format_set_silence parameter

[Clemens Ladisch]

Guilherme Longo wrote:
> What exactly does snd_pcm_format_set_silence is useful for???

It allows you to write silence samples into a buffer.
There are sample formats where silent samples are not zero bytes.


HTH
Clemens

Re: davinci-i2c,pcm updates

[Mark Brown]

On Sat, Jul 04, 2009 at 07:29:50PM -0700, Troy Kisky wrote:
> This patch set is against git://git.alsa-project.org/alsa-kernel.git

Best to use

	git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6

or
	git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git

> but the last patch in series won't compile until the sram allocator patch
> is available.

I've not reviewed or checked the code yet but it's likely to collide
with the updates which Chaithrika U S did to add support for some newer
processors.  Those are still sitting unmerged on the davinci branch of
my git since I've not heard anything about the arch/arm changes for the
clocks that they depend on.  The last thing I saw was a query from Kevin
about the use of plain names.

Could you give a pointer to the SRAM allocator patch, please?

performance between access mothods!

[Guilherme Longo]

Hi all!

I came across a doubt regarding the the access mode between application 
and PCM device.
It is a hardware parameter related property and there are 5 different 
types available:

SND_PCM_ACCESS_MMAP_INTERLEAVED
SND_PCM_ACCESS_MMAP_NONINTERLEAVED
SND_PCM_ACCESS_MMAP_COMPLEX
SND_PCM_ACCESS_RW_INTERLEAVED
SND_PCM_ACCESS_RW_NONINTERLEAVED

taking in account that these first 3 parameters has direct communication 
with the memory, that means a higher speed operation? And the fact of 
the interleaved has all the samples mixed together , makes any 
difference in performance in comparison to the noninterleaved that has 
samples stored on different buffers?

Thanks in advanced

Re: performance between access mothods!

[Takashi Iwai]

At Sun, 05 Jul 2009 09:03:23 -0300,
Guilherme Longo wrote:
> 
> Hi all!
> 
> I came across a doubt regarding the the access mode between application 
> and PCM device.
> It is a hardware parameter related property and there are 5 different 
> types available:
> 
> SND_PCM_ACCESS_MMAP_INTERLEAVED
> SND_PCM_ACCESS_MMAP_NONINTERLEAVED
> SND_PCM_ACCESS_MMAP_COMPLEX
> SND_PCM_ACCESS_RW_INTERLEAVED
> SND_PCM_ACCESS_RW_NONINTERLEAVED
> 
> taking in account that these first 3 parameters has direct communication 
> with the memory, that means a higher speed operation?

Yes.  The others are using copy_from/to_user(), so there are slight
overheads by explicit copying.


> And the fact of 
> the interleaved has all the samples mixed together , makes any 
> difference in performance in comparison to the noninterleaved that has 
> samples stored on different buffers?

No.  It's just a difference of what format the hardware supports.


Takashi

Re: davinci-i2c,pcm updates

[Troy Kisky]

Mark Brown wrote:
> On Sat, Jul 04, 2009 at 07:29:50PM -0700, Troy Kisky wrote:
>> This patch set is against git://git.alsa-project.org/alsa-kernel.git
> 
> Best to use
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6
> 
> or
> 	git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git
> 
>> but the last patch in series won't compile until the sram allocator patch
>> is available.
> 
> I've not reviewed or checked the code yet but it's likely to collide
> with the updates which Chaithrika U S did to add support for some newer
> processors.  Those are still sitting unmerged on the davinci branch of
> my git since I've not heard anything about the arch/arm changes for the
> clocks that they depend on.  The last thing I saw was a query from Kevin
> about the use of plain names.
> 
> Could you give a pointer to the SRAM allocator patch, please?
> 
Your for-2.6.32 branch has the file arch/arm/mach-davinci/include/mach/sram.h
so it has the patch.

But I also rely on patch from davinci-git
commit 09f6541323bd216f3661385ce26819a4c65cbae6
Author: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Date:   Thu May 21 07:41:40 2009 -0400

ARM: DaVinci: Interface changes visible to EDMA clients



But the last patch in series needs rebased on to this. Unfortunately,
this patch is not yet in your for-2.6.32 branch. So, unless this patch
is going to merge first, I show remove the need for this patch???


Thanks
Troy

Re: davinci-i2c,pcm updates

[Mark Brown]

On Mon, Jul 06, 2009 at 02:30:21PM -0700, Troy Kisky wrote:

> ARM: DaVinci: Interface changes visible to EDMA clients

> But the last patch in series needs rebased on to this. Unfortunately,
> this patch is not yet in your for-2.6.32 branch. So, unless this patch
> is going to merge first, I show remove the need for this patch???

Unless you can make your changes such that the above patch will merge in
cleanly there's no point - we'll need to deal with the cross tree merge
issues somehow.  That'll require coordination with Kevin; probably the
best approach is to make a small branch which can be merged into both
trees but that may be too involved.

The other options are to punt on the patch until after the merges have
happened, cherry pick a small number of commits over in one direction or
another to preserve buildability or just accept that DaVinci ASoC will
need to be built from -next for this merge window.  Since pretty much
everyone working on DaVinci appears to be using the DaVinci tree rather
than mainline at the minute the latter option isn't as bad as it could
be.  I'm not sure what works best for you guys?

Re: davinci-i2c,pcm updates

[Kevin Hilman]

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> On Mon, Jul 06, 2009 at 02:30:21PM -0700, Troy Kisky wrote:
>
>> ARM: DaVinci: Interface changes visible to EDMA clients
>
>> But the last patch in series needs rebased on to this. Unfortunately,
>> this patch is not yet in your for-2.6.32 branch. So, unless this patch
>> is going to merge first, I show remove the need for this patch???
>
> Unless you can make your changes such that the above patch will merge in
> cleanly there's no point - we'll need to deal with the cross tree merge
> issues somehow.  That'll require coordination with Kevin; probably the
> best approach is to make a small branch which can be merged into both
> trees but that may be too involved.

For now, how about making anything that has arch/arm dependencies
apply to the 'davinci-next' branch of DaVinci git.

Kevin

Re: davinci-i2c,pcm updates

[Mark Brown]

On Mon, Jul 06, 2009 at 03:51:25PM -0700, Kevin Hilman wrote:

> For now, how about making anything that has arch/arm dependencies
> apply to the 'davinci-next' branch of DaVinci git.

Providing there aren't any dependencies on pending ASoC changes that
probably works.

Re: davinci-i2c,pcm updates

[Kevin Hilman]

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> Could you give a pointer to the SRAM allocator patch, please?

SRAM allocater changes are in mainline as of 2.6.31.  See
arch/arm/mach-davinci/sram.c.

Kevin