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.
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;
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);
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;
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");
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); }
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; }
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; }
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 {
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,
If the codec is master, we support anything that the codec supports.
Signed-off-by: Troy Kisky troy.kisky@boundarydevices.com --- 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,
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; }
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.
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?
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)?
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
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.
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
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?
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
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.
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.
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.
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
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
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
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; }
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.
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?
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.
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.
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.
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@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_min = 2,
.channels_min = 1,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);elseedma_set_src_index(lch, data_type, 0);edma_set_dest_index(lch, 0, 0);
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);elseedma_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 {
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.
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...
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
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?
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
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
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
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?
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
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
-
Ambrose, Martin -
Clemens Ladisch -
Guilherme Longo -
Kevin Hilman -
Mark Brown -
Steve Chen -
Takashi Iwai -
Troy Kisky