[PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Liam Girdwood]

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,		\
 	},							\
-- 
1.6.3.3

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Jarkko Nikula]

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.

-- 
Jarkko

1.
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797.html

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

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Grazvydas Ignotas]

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
>
> 1.
> http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797.html
>
> ---
> 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);
>

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Liam Girdwood]

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

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Jarkko Nikula]

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.html
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-November/022797.html

Acked-by: Jarkko Nikula <jhnikula@gmail.com>

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Peter Ujfalusi]

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

-- 
Péter

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Peter Ujfalusi]

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.

> 

-- 
Péter

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Peter Ujfalusi]

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

-- 
Péter

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Jarkko Nikula]

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...


-- 
Jarkko

Re: [PATCH V2] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Jarkko Nikula]

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).


-- 
Jarkko