[alsa-devel] [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and has been sponsored by Shotspotter Inc. It also fixes a FSYNC rate calculation bug when McBSP is FSYNC master.
Signed-off-by: Graeme Gregory gg@slimlogic.co.uk Signed-off-by: Liam Girdwood lrg@slimlogic.co.uk --- sound/soc/omap/omap-mcbsp.c | 59 ++++++++++++++++++++++++------------------- 1 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 3341f49..3ea4a00 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -49,6 +49,8 @@ struct omap_mcbsp_data { */ int active; int configured; + unsigned int in_freq; + int clk_div; };
#define to_mcbsp(priv) container_of((priv), struct omap_mcbsp_data, bus_id) @@ -257,7 +259,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, int dma, bus_id = mcbsp_data->bus_id, id = cpu_dai->id; int wlen, channels, wpf, sync_mode = OMAP_DMA_SYNC_ELEMENT; unsigned long port; - unsigned int format; + unsigned int format, div, framesize;
if (cpu_class_is_omap1()) { dma = omap1_dma_reqs[bus_id][substream->stream]; @@ -294,28 +296,19 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK; wpf = channels = params_channels(params); - switch (channels) { - case 2: - if (format == SND_SOC_DAIFMT_I2S) { - /* Use dual-phase frames */ - regs->rcr2 |= RPHASE; - regs->xcr2 |= XPHASE; - /* Set 1 word per (McBSP) frame for phase1 and phase2 */ - wpf--; - regs->rcr2 |= RFRLEN2(wpf - 1); - regs->xcr2 |= XFRLEN2(wpf - 1); - } - case 1: - case 4: - /* Set word per (McBSP) frame for phase1 */ - regs->rcr1 |= RFRLEN1(wpf - 1); - regs->xcr1 |= XFRLEN1(wpf - 1); - break; - default: - /* Unsupported number of channels */ - return -EINVAL; + if (channels == 2 && format == SND_SOC_DAIFMT_I2S) { + /* Use dual-phase frames */ + regs->rcr2 |= RPHASE; + regs->xcr2 |= XPHASE; + /* Set 1 word per (McBSP) frame for phase1 and phase2 */ + wpf--; + regs->rcr2 |= RFRLEN2(wpf - 1); + regs->xcr2 |= XFRLEN2(wpf - 1); }
+ regs->rcr1 |= RFRLEN1(wpf - 1); + regs->xcr1 |= XFRLEN1(wpf - 1); + switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: /* Set word lengths */ @@ -330,15 +323,26 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
+ /* In McBSP master modes, FRAME (i.e. sample rate) is generated + * by _counting_ BCLKs. Calculate frame size in BCLKs */ + div = mcbsp_data->clk_div ? mcbsp_data->clk_div : 1; + framesize = (mcbsp_data->in_freq / div) / params_rate(params); + + if (framesize < wlen * channels) { + printk(KERN_ERR "%s: not enough bandwidth for desired rate and channels\n", + __func__); + return -EINVAL; + } + /* Set FS period and length in terms of bit clock periods */ switch (format) { case SND_SOC_DAIFMT_I2S: - regs->srgr2 |= FPER(wlen * channels - 1); - regs->srgr1 |= FWID(wlen - 1); + regs->srgr2 |= FPER(framesize - 1); + regs->srgr1 |= FWID((framesize >> 1) - 1); break; case SND_SOC_DAIFMT_DSP_A: case SND_SOC_DAIFMT_DSP_B: - regs->srgr2 |= FPER(wlen * channels - 1); + regs->srgr2 |= FPER(framesize - 1); regs->srgr1 |= FWID(0); break; } @@ -454,6 +458,7 @@ static int omap_mcbsp_dai_set_clkdiv(struct snd_soc_dai *cpu_dai, if (div_id != OMAP_MCBSP_CLKGDV) return -ENODEV;
+ mcbsp_data->clk_div = div; regs->srgr1 |= CLKGDV(div - 1);
return 0; @@ -554,6 +559,8 @@ static int omap_mcbsp_dai_set_dai_sysclk(struct snd_soc_dai *cpu_dai, struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs; int err = 0;
+ mcbsp_data->in_freq = freq; + switch (clk_id) { case OMAP_MCBSP_SYSCLK_CLK: regs->srgr2 |= CLKSM; @@ -598,13 +605,13 @@ static struct snd_soc_dai_ops omap_mcbsp_dai_ops = { .id = (link_id), \ .playback = { \ .channels_min = 1, \ - .channels_max = 4, \ + .channels_max = 16, \ .rates = OMAP_MCBSP_RATES, \ .formats = SNDRV_PCM_FMTBIT_S16_LE, \ }, \ .capture = { \ .channels_min = 1, \ - .channels_max = 4, \ + .channels_max = 16, \ .rates = OMAP_MCBSP_RATES, \ .formats = SNDRV_PCM_FMTBIT_S16_LE, \ }, \
On Thu, 05 Nov 2009 21:00:08 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and has been sponsored by Shotspotter Inc. It also fixes a FSYNC rate calculation bug when McBSP is FSYNC master.
I would not call it as bug but more like an implementation restriction :-)
But as the patch now requires that machine driver using OMAP as a master must provide the sample rate generator input clock frequency, I would like to hear how it will work on Pandora.
Grazvydas: are you able to test is the Pandora still playing after applying patch [1] to omap-mcbsp.c and my patch below to omap3pandora.c?
I made an assumption that input clock frequency on Pandora is 8 * 32 * sample_rate.
On Fri, Nov 6, 2009 at 8:24 PM, Jarkko Nikula jhnikula@gmail.com wrote:
On Thu, 05 Nov 2009 21:00:08 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and has been sponsored by Shotspotter Inc. It also fixes a FSYNC rate calculation bug when McBSP is FSYNC master.
I would not call it as bug but more like an implementation restriction :-)
But as the patch now requires that machine driver using OMAP as a master must provide the sample rate generator input clock frequency, I would like to hear how it will work on Pandora.
Grazvydas: are you able to test is the Pandora still playing after applying patch [1] to omap-mcbsp.c and my patch below to omap3pandora.c?
I made an assumption that input clock frequency on Pandora is 8 * 32 * sample_rate.
Seems to be working fine, tried playing several different sample rates and also recording path. FYI pandora has TWL4030 256FS clock connected to OMAP's CLKS pin.
Tested-by: Grazvydas Ignotas notasas@gmail.com
-- Jarkko
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797.ht...
diff --git a/sound/soc/omap/omap3pandora.c b/sound/soc/omap/omap3pandora.c index ad219aa..d0949fe 100644 --- a/sound/soc/omap/omap3pandora.c +++ b/sound/soc/omap/omap3pandora.c @@ -40,9 +40,12 @@
#define PREFIX "ASoC omap3pandora: "
-static int omap3pandora_cmn_hw_params(struct snd_soc_dai *codec_dai,
- struct snd_soc_dai *cpu_dai, unsigned int fmt)
+static int omap3pandora_cmn_hw_params(struct snd_pcm_substream *substream,
- struct snd_pcm_hw_params *params, unsigned int fmt)
{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
- struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
int ret;
/* Set codec DAI configuration */ @@ -68,8 +71,9 @@ static int omap3pandora_cmn_hw_params(struct snd_soc_dai *codec_dai, }
/* Set McBSP clock to external */
- ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_SYSCLK_CLKS_EXT, 0,
- SND_SOC_CLOCK_IN);
- ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_SYSCLK_CLKS_EXT,
- 8 * 32 * params_rate(params),
- SND_SOC_CLOCK_IN);
if (ret < 0) { pr_err(PREFIX "can't set cpu system clock\n"); return ret; @@ -87,11 +91,7 @@ static int omap3pandora_cmn_hw_params(struct snd_soc_dai *codec_dai, static int omap3pandora_out_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) {
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
- struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
- return omap3pandora_cmn_hw_params(codec_dai, cpu_dai,
- return omap3pandora_cmn_hw_params(substream, params,
SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_IB_NF | SND_SOC_DAIFMT_CBS_CFS); @@ -100,11 +100,7 @@ static int omap3pandora_out_hw_params(struct snd_pcm_substream *substream, static int omap3pandora_in_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) {
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *codec_dai = rtd->dai->codec_dai;
- struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
- return omap3pandora_cmn_hw_params(codec_dai, cpu_dai,
- return omap3pandora_cmn_hw_params(substream, params,
SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS);
On Sat, 2009-11-07 at 22:11 +0200, Grazvydas Ignotas wrote:
On Fri, Nov 6, 2009 at 8:24 PM, Jarkko Nikula jhnikula@gmail.com wrote:
On Thu, 05 Nov 2009 21:00:08 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
From: Graeme Gregory gg@slimlogic.co.uk
This patch increases the number of supported audio channels from 4 to 16 and has been sponsored by Shotspotter Inc. It also fixes a FSYNC rate calculation bug when McBSP is FSYNC master.
I would not call it as bug but more like an implementation restriction :-)
But as the patch now requires that machine driver using OMAP as a master must provide the sample rate generator input clock frequency, I would like to hear how it will work on Pandora.
Grazvydas: are you able to test is the Pandora still playing after applying patch [1] to omap-mcbsp.c and my patch below to omap3pandora.c?
I made an assumption that input clock frequency on Pandora is 8 * 32 * sample_rate.
Seems to be working fine, tried playing several different sample rates and also recording path. FYI pandora has TWL4030 256FS clock connected to OMAP's CLKS pin.
Tested-by: Grazvydas Ignotas notasas@gmail.com
Jarkko, Peter,
Any chance we could have your Ack before applying.
Thanks
Liam
On Sun, 08 Nov 2009 20:27:01 +0000 Liam Girdwood lrg@slimlogic.co.uk wrote:
Seems to be working fine, tried playing several different sample rates and also recording path. FYI pandora has TWL4030 256FS clock connected to OMAP's CLKS pin.
Tested-by: Grazvydas Ignotas notasas@gmail.com
Jarkko, Peter,
Any chance we could have your Ack before applying.
Definitely. I just sent a patch which must be applied before yours to the Pandora. It is otherwise the same than I sent before but now with commit log and 8*32 replaced with 256.
Mark: The omap3pandora.c must be patches before Liam's patch.
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022921.ht... http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797.ht...
Acked-by: Jarkko Nikula jhnikula@gmail.com
On Monday 09 November 2009 08:57:22 ext Jarkko Nikula wrote:
On Sun, 08 Nov 2009 20:27:01 +0000
Liam Girdwood lrg@slimlogic.co.uk wrote:
Seems to be working fine, tried playing several different sample rates and also recording path. FYI pandora has TWL4030 256FS clock connected to OMAP's CLKS pin.
Tested-by: Grazvydas Ignotas notasas@gmail.com
Jarkko, Peter,
Any chance we could have your Ack before applying.
Definitely. I just sent a patch which must be applied before yours to the Pandora. It is otherwise the same than I sent before but now with commit log and 8*32 replaced with 256.
Mark: The omap3pandora.c must be patches before Liam's patch.
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022921.h tml http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797. html
Acked-by: Jarkko Nikula jhnikula@gmail.com
I would reconsider this ack, since the patch breaks the McBSP slave operation.. There is no meaning for the infrequency nor for the divider in slave mode, but we can invent something like OMAP_MCBSP_FAKECLK_MASTER to be 'handled' in omap_mcbsp_dai_set_dai_sysclk, when the OMAP_MCBSP_FAKECLK_MASTER is being configured, we just take the frequency, which ahs to be calculated in the machine drivers beforehand (sample rate * bits * channels?), than we need to update all the machine drivers, where McBSP is slave to pass this information in order to pass the test in omap_mcbsp_dai_hw_params.
On Sunday 08 November 2009 22:27:01 ext Liam Girdwood wrote:
Any chance we could have your Ack before applying.
Well, I have tried it on a system, where the McBSP is slave (codec provides the bit and FS clock) with the following result:
aplay -Dplughw:0 /usr/share/sounds/alsa/Front_Center.wav -q omap_mcbsp_dai_hw_params: not enough bandwidth for desired rate and channels asoc: interface omap-mcbsp-dai-0 hw params failed aplay: set_params:1022: Unable to install hw params:
So this patch definitely breaks McBSP slave mode. In case when McBSP is slave, the SRG is not in use, and there is no point of passing some bogus information in order to pass the test in omap_mcbsp_dai_hw_params introduced by this patch.
I think if I set the mcbsp_data->in_freq to be: params_rate(params) * sample_bits * channels, and configure the mcbsp_data-
clk_div to be 1, might work in this case.
But after this patch the beagle board and boards which has the McBSP as slave will stop working.
I'm afraid, I can not give my Ack for this :(
Thanks
Liam
Resending this, since it seams that something eaten up the previous mail, might be duplicate over time.
On Sunday 08 November 2009 22:27:01 ext Liam Girdwood wrote:
Any chance we could have your Ack before applying.
Well, I have tried it on a system, where the McBSP is slave (codec provides the bit and FS clock) with the following result:
aplay -Dplughw:0 /usr/share/sounds/alsa/Front_Center.wav -q omap_mcbsp_dai_hw_params: not enough bandwidth for desired rate and channels asoc: interface omap-mcbsp-dai-0 hw params failed aplay: set_params:1022: Unable to install hw params:
So this patch definitely breaks McBSP slave mode. In case when McBSP is slave, the SRG is not in use, and there is no point of passing some bogus information in order to pass the test in omap_mcbsp_dai_hw_params introduced by this patch.
I think if I set the mcbsp_data->in_freq to be: params_rate(params) * sample_bits * channels, and configure the mcbsp_data-
clk_div to be 1, might work in this case.
But after this patch the beagle board and boards which has the McBSP as slave will stop working.
I'm afraid, I can not give my Ack for this :(
Thanks
Liam
On Mon, 9 Nov 2009 09:29:01 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
So this patch definitely breaks McBSP slave mode. In case when McBSP is slave, the SRG is not in use, and there is no point of passing some bogus information in order to pass the test in omap_mcbsp_dai_hw_params introduced by this patch.
I think if I set the mcbsp_data->in_freq to be: params_rate(params) * sample_bits * channels, and configure the mcbsp_data-
clk_div to be 1, might work in this case.
But after this patch the beagle board and boards which has the McBSP as slave will stop working.
I'm afraid, I can not give my Ack for this :(
Big thanks Peter! Me either. How on earth I didn't notice or test this simple thing that framesize calculatation leads to zero for all other boards than Pandora... and working Beagle audio is *must* have feature for me...
On Mon, 9 Nov 2009 09:29:01 +0200 Peter Ujfalusi peter.ujfalusi@nokia.com wrote:
I think if I set the mcbsp_data->in_freq to be: params_rate(params) * sample_bits * channels, and configure the mcbsp_data-
clk_div to be 1, might work in this case.
I think it's cleanest to calculate frame size conditionally based on SND_SOC_DAIFMT_CBS_CFS or testing !in_freq and let the frame size to be wlen * channels in other cases.
If it's based on !in_freq, then the in_freq must be set in omap_mcbsp_dai_set_dai_sysclk only for OMAP_MCBSP_SYSCLK_xxx cases since this function is called also when setting CLKR and FSR sources (no need to pass frequency).
participants (4)
-
Grazvydas Ignotas -
Jarkko Nikula -
Liam Girdwood -
Peter Ujfalusi