[PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds 8 channel capability by allowing DSP modes. It
also increases the channels_max (in the bcm2835_i2s_dai structure)
to 8.

The driver guards against multitrack mode when the codec isn't
master. The driver guards against multitrack capability when in
I2S mode.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..022ca13 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -276,11 +276,16 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		/* otherwise calculate a fitting block ratio */
 		bclk_ratio = 2 * data_length;
 
-	/* Clock should only be set up here if CPU is clock master */
+	/* Clock should only be set up here if CPU is clock master.
+	   If codec isn't master guard against multichannel mode. */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 	case SND_SOC_DAIFMT_CBS_CFM:
 		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
+	case SND_SOC_DAIFMT_CBM_CFS:
+		/* Codec must be master in multichannel mode */
+		if (params_channels(params) > 2)
+			return -EINVAL;
 		break;
 	default:
 		break;
@@ -294,8 +299,14 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	format |= BCM2835_I2S_CHWID((data_length-8)&0xf);
 
+	/* Default data delay to 1 bit.
+	   In I2S mode, we must have 2 channels */
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		if (params_channels(params) != 2)
+			return -EINVAL;
+	case SND_SOC_DAIFMT_DSP_A:
+	case SND_SOC_DAIFMT_DSP_B:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +323,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -577,7 +589,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
@@ -585,7 +597,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 		},
 	.capture = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds 8 channel capability by allowing DSP modes. It
also increases the channels_max (in the bcm2835_i2s_dai structure)
to 8.

The driver guards against multitrack mode when the codec isn't
master. The driver guards against multitrack capability when in
I2S mode.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..022ca13 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -276,11 +276,16 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 		/* otherwise calculate a fitting block ratio */
 		bclk_ratio = 2 * data_length;
 
-	/* Clock should only be set up here if CPU is clock master */
+	/* Clock should only be set up here if CPU is clock master.
+	   If codec isn't master guard against multichannel mode. */
 	switch (dev->fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 	case SND_SOC_DAIFMT_CBS_CFM:
 		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
+	case SND_SOC_DAIFMT_CBM_CFS:
+		/* Codec must be master in multichannel mode */
+		if (params_channels(params) > 2)
+			return -EINVAL;
 		break;
 	default:
 		break;
@@ -294,8 +299,14 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	format |= BCM2835_I2S_CHWID((data_length-8)&0xf);
 
+	/* Default data delay to 1 bit.
+	   In I2S mode, we must have 2 channels */
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+		if (params_channels(params) != 2)
+			return -EINVAL;
+	case SND_SOC_DAIFMT_DSP_A:
+	case SND_SOC_DAIFMT_DSP_B:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +323,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -577,7 +589,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
@@ -585,7 +597,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 		},
 	.capture = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 876 bytes --]

On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:

>  	case SND_SOC_DAIFMT_CBS_CFM:
>  		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> +	case SND_SOC_DAIFMT_CBM_CFS:

Is this fall through deliberate?

> +	/* Default data delay to 1 bit.
> +	   In I2S mode, we must have 2 channels */
>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
> +		if (params_channels(params) != 2)
> +			return -EINVAL;
> +	case SND_SOC_DAIFMT_DSP_A:
> +	case SND_SOC_DAIFMT_DSP_B:
>  		data_delay = 1;
>  		break;
>  	default:

Same here.  This is also buggy in that it treats DSP A and DSP B
identically, they are different so the configuration must be incorrect
for one of them.  I suspect this is configuring for DSP A.

You should also really have a setup() function that imposes a channel
constraint when in I2S mode, wm8988 is one example here.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:

>  	case SND_SOC_DAIFMT_CBS_CFM:
>  		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> +	case SND_SOC_DAIFMT_CBM_CFS:

Is this fall through deliberate?

> +	/* Default data delay to 1 bit.
> +	   In I2S mode, we must have 2 channels */
>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
> +		if (params_channels(params) != 2)
> +			return -EINVAL;
> +	case SND_SOC_DAIFMT_DSP_A:
> +	case SND_SOC_DAIFMT_DSP_B:
>  		data_delay = 1;
>  		break;
>  	default:

Same here.  This is also buggy in that it treats DSP A and DSP B
identically, they are different so the configuration must be incorrect
for one of them.  I suspect this is configuring for DSP A.

You should also really have a setup() function that imposes a channel
constraint when in I2S mode, wm8988 is one example here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170208/3e1a2c01/attachment.sig>

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
> 
> >  	case SND_SOC_DAIFMT_CBS_CFM:
> >  		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> > +	case SND_SOC_DAIFMT_CBM_CFS:
> 
> Is this fall through deliberate?
> 
> > +	/* Default data delay to 1 bit.
> > +	   In I2S mode, we must have 2 channels */
> >  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >  	case SND_SOC_DAIFMT_I2S:
> > +		if (params_channels(params) != 2)
> > +			return -EINVAL;
> > +	case SND_SOC_DAIFMT_DSP_A:
> > +	case SND_SOC_DAIFMT_DSP_B:
> >  		data_delay = 1;
> >  		break;
> >  	default:

Matt, could you please include linux-rpi-kernel@lists.infradead.org
in your emails?

I fail to see the part where DSP modes are actually set up in
the hardware. bcm2835 still seems to be operating in 2-channel
stereo I2S mode, i.e. no real frame sync information at the
hardware level.

If all you do is adding code to pretend the bcm2835 could do
multichannel modes wouldn't it be easier to implement that as
a userspace alsa plugin?

> 
> Same here.  This is also buggy in that it treats DSP A and DSP B
> identically, they are different so the configuration must be incorrect
> for one of them.  I suspect this is configuring for DSP A.
> 
> You should also really have a setup() function that imposes a channel
> constraint when in I2S mode, wm8988 is one example here.



> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

[PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
> 
> >  	case SND_SOC_DAIFMT_CBS_CFM:
> >  		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> > +	case SND_SOC_DAIFMT_CBM_CFS:
> 
> Is this fall through deliberate?
> 
> > +	/* Default data delay to 1 bit.
> > +	   In I2S mode, we must have 2 channels */
> >  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >  	case SND_SOC_DAIFMT_I2S:
> > +		if (params_channels(params) != 2)
> > +			return -EINVAL;
> > +	case SND_SOC_DAIFMT_DSP_A:
> > +	case SND_SOC_DAIFMT_DSP_B:
> >  		data_delay = 1;
> >  		break;
> >  	default:

Matt, could you please include linux-rpi-kernel at lists.infradead.org
in your emails?

I fail to see the part where DSP modes are actually set up in
the hardware. bcm2835 still seems to be operating in 2-channel
stereo I2S mode, i.e. no real frame sync information at the
hardware level.

If all you do is adding code to pretend the bcm2835 could do
multichannel modes wouldn't it be easier to implement that as
a userspace alsa plugin?

> 
> Same here.  This is also buggy in that it treats DSP A and DSP B
> identically, they are different so the configuration must be incorrect
> for one of them.  I suspect this is configuring for DSP A.
> 
> You should also really have a setup() function that imposes a channel
> constraint when in I2S mode, wm8988 is one example here.



> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 05:54, Matthias Reichl wrote:
> On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>
>>>   	case SND_SOC_DAIFMT_CBS_CFM:
>>>   		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>> +	case SND_SOC_DAIFMT_CBM_CFS:
>> Is this fall through deliberate?
>>
>>> +	/* Default data delay to 1 bit.
>>> +	   In I2S mode, we must have 2 channels */
>>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>   	case SND_SOC_DAIFMT_I2S:
>>> +		if (params_channels(params) != 2)
>>> +			return -EINVAL;
>>> +	case SND_SOC_DAIFMT_DSP_A:
>>> +	case SND_SOC_DAIFMT_DSP_B:
>>>   		data_delay = 1;
>>>   		break;
>>>   	default:
> Matt, could you please include linux-rpi-kernel@lists.infradead.org
> in your emails?
I have joined that list now. It was included originally, but wasn't 
accepting my posts.
> I fail to see the part where DSP modes are actually set up in
> the hardware. bcm2835 still seems to be operating in 2-channel
> stereo I2S mode, i.e. no real frame sync information at the
> hardware level.
 From the SoC's perspective I agree with you. There is frame 
synchronisation at the hardware level, implemented in an master FPGA. 
This starts to hit at a lack of functionality in ALSA ... I will discuss 
more below.
> If all you do is adding code to pretend the bcm2835 could do
> multichannel modes wouldn't it be easier to implement that as
> a userspace alsa plugin?
>
>
I am not familiar with how to implement all of this with a plugin ? 
Could you give me a little hand in describing that further ? That would 
mean that an asoundrc needs to be used to defined to make the system 
usable ? Is it something which does the unpacking for us in user space ? 
If this happens in user space is there extra cost/latency ?

I would like to bring up another topic here.

In my opinion some of these changes we are making in this general thread 
are only really window dressing.

We have 4 ways of setting up master, however all of them assume that 
either the codec or the SoC is master. None of them allow for 
intermediate digital logic between the two.

In this case there is a FPGA which is matching the system differences 
between the Codec and the SoC. In actual fact, the FPGA needs to be 
master - a fifth mode.

A similar problem exists when you are using a sample rate converter 
chip. For example, the DAC and ADC are running at different sample 
rates. In this case ALSA can't represent both of the sample rates. For 
that reason, the ADCs and the DACs have to be hard coded - it is nasty.

The only solution for me is to use snd_soc_dai_set_fmt in the machine 
driver to instruct both to enter slave mode. For what it is worth, I can 
also

In my opinion there is nothing wrong with making hardware level 
introductions, such as an ASIC/FPGA to implement the hardware. I accept 
the inflexibility of ALSA w.r.t. this type of situation, however the 
real fix is to adjust the core of ALSA. Hardware ASICS and FPGAs which 
are intermediatries between codecs and SoCs exist and are used in industry.

This happens to be one of those cases.

thanks
Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 05:54, Matthias Reichl wrote:
> On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>
>>>   	case SND_SOC_DAIFMT_CBS_CFM:
>>>   		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>> +	case SND_SOC_DAIFMT_CBM_CFS:
>> Is this fall through deliberate?
>>
>>> +	/* Default data delay to 1 bit.
>>> +	   In I2S mode, we must have 2 channels */
>>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>   	case SND_SOC_DAIFMT_I2S:
>>> +		if (params_channels(params) != 2)
>>> +			return -EINVAL;
>>> +	case SND_SOC_DAIFMT_DSP_A:
>>> +	case SND_SOC_DAIFMT_DSP_B:
>>>   		data_delay = 1;
>>>   		break;
>>>   	default:
> Matt, could you please include linux-rpi-kernel at lists.infradead.org
> in your emails?
I have joined that list now. It was included originally, but wasn't 
accepting my posts.
> I fail to see the part where DSP modes are actually set up in
> the hardware. bcm2835 still seems to be operating in 2-channel
> stereo I2S mode, i.e. no real frame sync information at the
> hardware level.
 From the SoC's perspective I agree with you. There is frame 
synchronisation at the hardware level, implemented in an master FPGA. 
This starts to hit at a lack of functionality in ALSA ... I will discuss 
more below.
> If all you do is adding code to pretend the bcm2835 could do
> multichannel modes wouldn't it be easier to implement that as
> a userspace alsa plugin?
>
>
I am not familiar with how to implement all of this with a plugin ? 
Could you give me a little hand in describing that further ? That would 
mean that an asoundrc needs to be used to defined to make the system 
usable ? Is it something which does the unpacking for us in user space ? 
If this happens in user space is there extra cost/latency ?

I would like to bring up another topic here.

In my opinion some of these changes we are making in this general thread 
are only really window dressing.

We have 4 ways of setting up master, however all of them assume that 
either the codec or the SoC is master. None of them allow for 
intermediate digital logic between the two.

In this case there is a FPGA which is matching the system differences 
between the Codec and the SoC. In actual fact, the FPGA needs to be 
master - a fifth mode.

A similar problem exists when you are using a sample rate converter 
chip. For example, the DAC and ADC are running at different sample 
rates. In this case ALSA can't represent both of the sample rates. For 
that reason, the ADCs and the DACs have to be hard coded - it is nasty.

The only solution for me is to use snd_soc_dai_set_fmt in the machine 
driver to instruct both to enter slave mode. For what it is worth, I can 
also

In my opinion there is nothing wrong with making hardware level 
introductions, such as an ASIC/FPGA to implement the hardware. I accept 
the inflexibility of ALSA w.r.t. this type of situation, however the 
real fix is to adjust the core of ALSA. Hardware ASICS and FPGAs which 
are intermediatries between codecs and SoCs exist and are used in industry.

This happens to be one of those cases.

thanks
Matt

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 08:13, Matt Flax wrote:
>
>
> On 09/02/17 05:54, Matthias Reichl wrote:
>> On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
>>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>>
>>>>       case SND_SOC_DAIFMT_CBS_CFM:
>>>>           clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>>> +    case SND_SOC_DAIFMT_CBM_CFS:
>>> Is this fall through deliberate?
>>>
>>>> +    /* Default data delay to 1 bit.
>>>> +       In I2S mode, we must have 2 channels */
>>>>       switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>>       case SND_SOC_DAIFMT_I2S:
>>>> +        if (params_channels(params) != 2)
>>>> +            return -EINVAL;
>>>> +    case SND_SOC_DAIFMT_DSP_A:
>>>> +    case SND_SOC_DAIFMT_DSP_B:
>>>>           data_delay = 1;
>>>>           break;
>>>>       default:
>> Matt, could you please include linux-rpi-kernel@lists.infradead.org
>> in your emails?
> I have joined that list now. It was included originally, but wasn't 
> accepting my posts.
>> I fail to see the part where DSP modes are actually set up in
>> the hardware. bcm2835 still seems to be operating in 2-channel
>> stereo I2S mode, i.e. no real frame sync information at the
>> hardware level.
> From the SoC's perspective I agree with you. There is frame 
> synchronisation at the hardware level, implemented in an master FPGA. 
> This starts to hit at a lack of functionality in ALSA ... I will 
> discuss more below.
>> If all you do is adding code to pretend the bcm2835 could do
>> multichannel modes wouldn't it be easier to implement that as
>> a userspace alsa plugin?
>>
>>
> I am not familiar with how to implement all of this with a plugin ? 
> Could you give me a little hand in describing that further ? That 
> would mean that an asoundrc needs to be used to defined to make the 
> system usable ? Is it something which does the unpacking for us in 
> user space ? If this happens in user space is there extra cost/latency ?
>

You know, I am genuinely interested in your concept and still invite an 
example of your creativity, however ...
The more I think about this approach, the concept of pushing the support 
of hardware into user space the more I disagree with it. My 
understanding is that the Linux Kernel is there to support hardware. The 
concept of pushing hardware support into user space doesn't seem right.

As I have pointed out below, there are missing things in ALSA and as 
Mark previously pointed out "this is a thing". What I understand is that 
this hardware is a thing and has been thought of before - this happens 
to be a hardware implementation of this "thing" which ALSA doesn't 
currently have the capacity to support (e.g. an ASIC/FPGA which is 
mater, not the SoC nor the Codec).

I remember back in the '90s when ALSA was started - I witnessed its 
birth. ALSA was started because of inadequacies of OSS. I truly don't 
believe that we need a new sound system for Linux as of yet. I also 
don't believe that because ALSA has these inadequacies (which I mention 
below) that we need to start afresh. I would personally put effort into 
this part of ALSA if I had the money to support myself whilst I did it - 
but I don't. So for now, I am trying to make do with ALSA as best I can. 
I am trying to put the necessary support for existing hardware into ALSA 
in its current state and form - in the best possible manner. So please 
lets continue with support for this hardware in the kernel.

> I would like to bring up another topic here.
>
> In my opinion some of these changes we are making in this general 
> thread are only really window dressing.
>
> We have 4 ways of setting up master, however all of them assume that 
> either the codec or the SoC is master. None of them allow for 
> intermediate digital logic between the two.
>
> In this case there is a FPGA which is matching the system differences 
> between the Codec and the SoC. In actual fact, the FPGA needs to be 
> master - a fifth mode.
>
> A similar problem exists when you are using a sample rate converter 
> chip. For example, the DAC and ADC are running at different sample 
> rates. In this case ALSA can't represent both of the sample rates. For 
> that reason, the ADCs and the DACs have to be hard coded - it is nasty.
>
> The only solution for me is to use snd_soc_dai_set_fmt in the machine 
> driver to instruct both to enter slave mode. For what it is worth, I 
> can also
>
> In my opinion there is nothing wrong with making hardware level 
> introductions, such as an ASIC/FPGA to implement the hardware. I 
> accept the inflexibility of ALSA w.r.t. this type of situation, 
> however the real fix is to adjust the core of ALSA. Hardware ASICS and 
> FPGAs which are intermediatries between codecs and SoCs exist and are 
> used in industry.
>
> This happens to be one of those cases.
>
> thanks
> Matt
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 08:13, Matt Flax wrote:
>
>
> On 09/02/17 05:54, Matthias Reichl wrote:
>> On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
>>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>>
>>>>       case SND_SOC_DAIFMT_CBS_CFM:
>>>>           clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>>> +    case SND_SOC_DAIFMT_CBM_CFS:
>>> Is this fall through deliberate?
>>>
>>>> +    /* Default data delay to 1 bit.
>>>> +       In I2S mode, we must have 2 channels */
>>>>       switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>>       case SND_SOC_DAIFMT_I2S:
>>>> +        if (params_channels(params) != 2)
>>>> +            return -EINVAL;
>>>> +    case SND_SOC_DAIFMT_DSP_A:
>>>> +    case SND_SOC_DAIFMT_DSP_B:
>>>>           data_delay = 1;
>>>>           break;
>>>>       default:
>> Matt, could you please include linux-rpi-kernel at lists.infradead.org
>> in your emails?
> I have joined that list now. It was included originally, but wasn't 
> accepting my posts.
>> I fail to see the part where DSP modes are actually set up in
>> the hardware. bcm2835 still seems to be operating in 2-channel
>> stereo I2S mode, i.e. no real frame sync information at the
>> hardware level.
> From the SoC's perspective I agree with you. There is frame 
> synchronisation at the hardware level, implemented in an master FPGA. 
> This starts to hit at a lack of functionality in ALSA ... I will 
> discuss more below.
>> If all you do is adding code to pretend the bcm2835 could do
>> multichannel modes wouldn't it be easier to implement that as
>> a userspace alsa plugin?
>>
>>
> I am not familiar with how to implement all of this with a plugin ? 
> Could you give me a little hand in describing that further ? That 
> would mean that an asoundrc needs to be used to defined to make the 
> system usable ? Is it something which does the unpacking for us in 
> user space ? If this happens in user space is there extra cost/latency ?
>

You know, I am genuinely interested in your concept and still invite an 
example of your creativity, however ...
The more I think about this approach, the concept of pushing the support 
of hardware into user space the more I disagree with it. My 
understanding is that the Linux Kernel is there to support hardware. The 
concept of pushing hardware support into user space doesn't seem right.

As I have pointed out below, there are missing things in ALSA and as 
Mark previously pointed out "this is a thing". What I understand is that 
this hardware is a thing and has been thought of before - this happens 
to be a hardware implementation of this "thing" which ALSA doesn't 
currently have the capacity to support (e.g. an ASIC/FPGA which is 
mater, not the SoC nor the Codec).

I remember back in the '90s when ALSA was started - I witnessed its 
birth. ALSA was started because of inadequacies of OSS. I truly don't 
believe that we need a new sound system for Linux as of yet. I also 
don't believe that because ALSA has these inadequacies (which I mention 
below) that we need to start afresh. I would personally put effort into 
this part of ALSA if I had the money to support myself whilst I did it - 
but I don't. So for now, I am trying to make do with ALSA as best I can. 
I am trying to put the necessary support for existing hardware into ALSA 
in its current state and form - in the best possible manner. So please 
lets continue with support for this hardware in the kernel.

> I would like to bring up another topic here.
>
> In my opinion some of these changes we are making in this general 
> thread are only really window dressing.
>
> We have 4 ways of setting up master, however all of them assume that 
> either the codec or the SoC is master. None of them allow for 
> intermediate digital logic between the two.
>
> In this case there is a FPGA which is matching the system differences 
> between the Codec and the SoC. In actual fact, the FPGA needs to be 
> master - a fifth mode.
>
> A similar problem exists when you are using a sample rate converter 
> chip. For example, the DAC and ADC are running at different sample 
> rates. In this case ALSA can't represent both of the sample rates. For 
> that reason, the ADCs and the DACs have to be hard coded - it is nasty.
>
> The only solution for me is to use snd_soc_dai_set_fmt in the machine 
> driver to instruct both to enter slave mode. For what it is worth, I 
> can also
>
> In my opinion there is nothing wrong with making hardware level 
> introductions, such as an ASIC/FPGA to implement the hardware. I 
> accept the inflexibility of ALSA w.r.t. this type of situation, 
> however the real fix is to adjust the core of ALSA. Hardware ASICS and 
> FPGAs which are intermediatries between codecs and SoCs exist and are 
> used in industry.
>
> This happens to be one of those cases.
>
> thanks
> Matt
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Thu, Feb 09, 2017 at 09:48:04AM +1100, Matt Flax wrote:
> 
> On 09/02/17 08:13, Matt Flax wrote:
> >
> >
> >On 09/02/17 05:54, Matthias Reichl wrote:
> >>On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
> >>>On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
> >>>
> >>>>      case SND_SOC_DAIFMT_CBS_CFM:
> >>>>          clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> >>>>+    case SND_SOC_DAIFMT_CBM_CFS:
> >>>Is this fall through deliberate?
> >>>
> >>>>+    /* Default data delay to 1 bit.
> >>>>+       In I2S mode, we must have 2 channels */
> >>>>      switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >>>>      case SND_SOC_DAIFMT_I2S:
> >>>>+        if (params_channels(params) != 2)
> >>>>+            return -EINVAL;
> >>>>+    case SND_SOC_DAIFMT_DSP_A:
> >>>>+    case SND_SOC_DAIFMT_DSP_B:
> >>>>          data_delay = 1;
> >>>>          break;
> >>>>      default:
> >>Matt, could you please include linux-rpi-kernel@lists.infradead.org
> >>in your emails?
> >I have joined that list now. It was included originally, but wasn't
> >accepting my posts.
> >>I fail to see the part where DSP modes are actually set up in
> >>the hardware. bcm2835 still seems to be operating in 2-channel
> >>stereo I2S mode, i.e. no real frame sync information at the
> >>hardware level.
> >From the SoC's perspective I agree with you. There is frame
> >synchronisation at the hardware level, implemented in an master FPGA. This
> >starts to hit at a lack of functionality in ALSA ... I will discuss more
> >below.
> >>If all you do is adding code to pretend the bcm2835 could do
> >>multichannel modes wouldn't it be easier to implement that as
> >>a userspace alsa plugin?
> >>
> >>
> >I am not familiar with how to implement all of this with a plugin ? Could
> >you give me a little hand in describing that further ? That would mean
> >that an asoundrc needs to be used to defined to make the system usable ?
> >Is it something which does the unpacking for us in user space ? If this
> >happens in user space is there extra cost/latency ?

I haven't written a plugin myself, but there are plenty of examples out
there. Maybe there's already one that does what you need.

Have a look at the code in alsa-lib
http://git.alsa-project.org/?p=alsa-lib.git;a=tree;f=src/pcm

Also have a look at how iec958 is handled in most setups (pcm_iec958
plugin, integration via /usr/share/alsa/cards/*.conf

Plugins don't necessarily add a latency overhead, I've got the impression
all you need to do is to hook into hw_params and setup the slave hw
to 2 channels and 4 times the requested samplerate.

eg when 8-channel 48kHz is requested configure the hardware to
2-channel 192kHz.

You can pass additional information to the machine driver via controls,
eg to signal that you are transmitting 8-channel PCM encapsulated in
a 2-channel stream. Again a good example is iec958, the AES channel
status bits are transferred to the card via the IEC958 Playback Default
control in the card .conf via a hook plug.

> You know, I am genuinely interested in your concept and still invite an
> example of your creativity, however ...
> The more I think about this approach, the concept of pushing the support of
> hardware into user space the more I disagree with it. My understanding is
> that the Linux Kernel is there to support hardware. The concept of pushing
> hardware support into user space doesn't seem right.
> 
> As I have pointed out below, there are missing things in ALSA and as Mark
> previously pointed out "this is a thing". What I understand is that this
> hardware is a thing and has been thought of before - this happens to be a
> hardware implementation of this "thing" which ALSA doesn't currently have
> the capacity to support (e.g. an ASIC/FPGA which is mater, not the SoC nor
> the Codec).

>From what you describe it looks to me the FPGA is just acting as a codec,
converting between 2-channel I2S and 8-channel (DSP?) at 1/4 of the I2S rate.

So you have bcm2835 I2S <-> FPGA <-> codec - IOW a standard codec<->codec
link.

What you seem to be missing is just a method to transfer your 8-channel
data via a 2-channel link - userspace want's to see an 8-channel PCM,
but the hardware link (bcm2835-i2s) is only 2-channel.

And that's where IMO as userspace plugin looks like a very good solution.
It's basically the counterpart of your FPGA and contains the code that's
neccessary to encapsulate/pack/whatever the 8-channel data into a 2-channel
stream so it can then be unpacked to 8-channel by the FPGA.

If you go this route your hardware and machine driver will work with
other I2S codecs as well, and IMO that's a far better solution than
adding very ugly hacks to a single I2S driver.

so long,

Hias

> 
> I remember back in the '90s when ALSA was started - I witnessed its birth.
> ALSA was started because of inadequacies of OSS. I truly don't believe that
> we need a new sound system for Linux as of yet. I also don't believe that
> because ALSA has these inadequacies (which I mention below) that we need to
> start afresh. I would personally put effort into this part of ALSA if I had
> the money to support myself whilst I did it - but I don't. So for now, I am
> trying to make do with ALSA as best I can. I am trying to put the necessary
> support for existing hardware into ALSA in its current state and form - in
> the best possible manner. So please lets continue with support for this
> hardware in the kernel.
> 
> >I would like to bring up another topic here.
> >
> >In my opinion some of these changes we are making in this general thread
> >are only really window dressing.
> >
> >We have 4 ways of setting up master, however all of them assume that
> >either the codec or the SoC is master. None of them allow for intermediate
> >digital logic between the two.
> >
> >In this case there is a FPGA which is matching the system differences
> >between the Codec and the SoC. In actual fact, the FPGA needs to be master
> >- a fifth mode.
> >
> >A similar problem exists when you are using a sample rate converter chip.
> >For example, the DAC and ADC are running at different sample rates. In
> >this case ALSA can't represent both of the sample rates. For that reason,
> >the ADCs and the DACs have to be hard coded - it is nasty.
> >
> >The only solution for me is to use snd_soc_dai_set_fmt in the machine
> >driver to instruct both to enter slave mode. For what it is worth, I can
> >also
> >
> >In my opinion there is nothing wrong with making hardware level
> >introductions, such as an ASIC/FPGA to implement the hardware. I accept
> >the inflexibility of ALSA w.r.t. this type of situation, however the real
> >fix is to adjust the core of ALSA. Hardware ASICS and FPGAs which are
> >intermediatries between codecs and SoCs exist and are used in industry.
> >
> >This happens to be one of those cases.
> >
> >thanks
> >Matt
> >
> >_______________________________________________
> >Alsa-devel mailing list
> >Alsa-devel@alsa-project.org
> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

[alsa-devel] [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Thu, Feb 09, 2017 at 09:48:04AM +1100, Matt Flax wrote:
> 
> On 09/02/17 08:13, Matt Flax wrote:
> >
> >
> >On 09/02/17 05:54, Matthias Reichl wrote:
> >>On Wed, Feb 08, 2017 at 06:28:35PM +0000, Mark Brown wrote:
> >>>On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
> >>>
> >>>>      case SND_SOC_DAIFMT_CBS_CFM:
> >>>>          clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
> >>>>+    case SND_SOC_DAIFMT_CBM_CFS:
> >>>Is this fall through deliberate?
> >>>
> >>>>+    /* Default data delay to 1 bit.
> >>>>+       In I2S mode, we must have 2 channels */
> >>>>      switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >>>>      case SND_SOC_DAIFMT_I2S:
> >>>>+        if (params_channels(params) != 2)
> >>>>+            return -EINVAL;
> >>>>+    case SND_SOC_DAIFMT_DSP_A:
> >>>>+    case SND_SOC_DAIFMT_DSP_B:
> >>>>          data_delay = 1;
> >>>>          break;
> >>>>      default:
> >>Matt, could you please include linux-rpi-kernel at lists.infradead.org
> >>in your emails?
> >I have joined that list now. It was included originally, but wasn't
> >accepting my posts.
> >>I fail to see the part where DSP modes are actually set up in
> >>the hardware. bcm2835 still seems to be operating in 2-channel
> >>stereo I2S mode, i.e. no real frame sync information at the
> >>hardware level.
> >From the SoC's perspective I agree with you. There is frame
> >synchronisation at the hardware level, implemented in an master FPGA. This
> >starts to hit at a lack of functionality in ALSA ... I will discuss more
> >below.
> >>If all you do is adding code to pretend the bcm2835 could do
> >>multichannel modes wouldn't it be easier to implement that as
> >>a userspace alsa plugin?
> >>
> >>
> >I am not familiar with how to implement all of this with a plugin ? Could
> >you give me a little hand in describing that further ? That would mean
> >that an asoundrc needs to be used to defined to make the system usable ?
> >Is it something which does the unpacking for us in user space ? If this
> >happens in user space is there extra cost/latency ?

I haven't written a plugin myself, but there are plenty of examples out
there. Maybe there's already one that does what you need.

Have a look at the code in alsa-lib
http://git.alsa-project.org/?p=alsa-lib.git;a=tree;f=src/pcm

Also have a look at how iec958 is handled in most setups (pcm_iec958
plugin, integration via /usr/share/alsa/cards/*.conf

Plugins don't necessarily add a latency overhead, I've got the impression
all you need to do is to hook into hw_params and setup the slave hw
to 2 channels and 4 times the requested samplerate.

eg when 8-channel 48kHz is requested configure the hardware to
2-channel 192kHz.

You can pass additional information to the machine driver via controls,
eg to signal that you are transmitting 8-channel PCM encapsulated in
a 2-channel stream. Again a good example is iec958, the AES channel
status bits are transferred to the card via the IEC958 Playback Default
control in the card .conf via a hook plug.

> You know, I am genuinely interested in your concept and still invite an
> example of your creativity, however ...
> The more I think about this approach, the concept of pushing the support of
> hardware into user space the more I disagree with it. My understanding is
> that the Linux Kernel is there to support hardware. The concept of pushing
> hardware support into user space doesn't seem right.
> 
> As I have pointed out below, there are missing things in ALSA and as Mark
> previously pointed out "this is a thing". What I understand is that this
> hardware is a thing and has been thought of before - this happens to be a
> hardware implementation of this "thing" which ALSA doesn't currently have
> the capacity to support (e.g. an ASIC/FPGA which is mater, not the SoC nor
> the Codec).

>From what you describe it looks to me the FPGA is just acting as a codec,
converting between 2-channel I2S and 8-channel (DSP?) at 1/4 of the I2S rate.

So you have bcm2835 I2S <-> FPGA <-> codec - IOW a standard codec<->codec
link.

What you seem to be missing is just a method to transfer your 8-channel
data via a 2-channel link - userspace want's to see an 8-channel PCM,
but the hardware link (bcm2835-i2s) is only 2-channel.

And that's where IMO as userspace plugin looks like a very good solution.
It's basically the counterpart of your FPGA and contains the code that's
neccessary to encapsulate/pack/whatever the 8-channel data into a 2-channel
stream so it can then be unpacked to 8-channel by the FPGA.

If you go this route your hardware and machine driver will work with
other I2S codecs as well, and IMO that's a far better solution than
adding very ugly hacks to a single I2S driver.

so long,

Hias

> 
> I remember back in the '90s when ALSA was started - I witnessed its birth.
> ALSA was started because of inadequacies of OSS. I truly don't believe that
> we need a new sound system for Linux as of yet. I also don't believe that
> because ALSA has these inadequacies (which I mention below) that we need to
> start afresh. I would personally put effort into this part of ALSA if I had
> the money to support myself whilst I did it - but I don't. So for now, I am
> trying to make do with ALSA as best I can. I am trying to put the necessary
> support for existing hardware into ALSA in its current state and form - in
> the best possible manner. So please lets continue with support for this
> hardware in the kernel.
> 
> >I would like to bring up another topic here.
> >
> >In my opinion some of these changes we are making in this general thread
> >are only really window dressing.
> >
> >We have 4 ways of setting up master, however all of them assume that
> >either the codec or the SoC is master. None of them allow for intermediate
> >digital logic between the two.
> >
> >In this case there is a FPGA which is matching the system differences
> >between the Codec and the SoC. In actual fact, the FPGA needs to be master
> >- a fifth mode.
> >
> >A similar problem exists when you are using a sample rate converter chip.
> >For example, the DAC and ADC are running at different sample rates. In
> >this case ALSA can't represent both of the sample rates. For that reason,
> >the ADCs and the DACs have to be hard coded - it is nasty.
> >
> >The only solution for me is to use snd_soc_dai_set_fmt in the machine
> >driver to instruct both to enter slave mode. For what it is worth, I can
> >also
> >
> >In my opinion there is nothing wrong with making hardware level
> >introductions, such as an ASIC/FPGA to implement the hardware. I accept
> >the inflexibility of ALSA w.r.t. this type of situation, however the real
> >fix is to adjust the core of ALSA. Hardware ASICS and FPGAs which are
> >intermediatries between codecs and SoCs exist and are used in industry.
> >
> >This happens to be one of those cases.
> >
> >thanks
> >Matt
> >
> >_______________________________________________
> >Alsa-devel mailing list
> >Alsa-devel at alsa-project.org
> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 05:28, Mark Brown wrote:
> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>
>>   	case SND_SOC_DAIFMT_CBS_CFM:
>>   		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>> +	case SND_SOC_DAIFMT_CBM_CFS:
> Is this fall through deliberate?
>
>> +	/* Default data delay to 1 bit.
>> +	   In I2S mode, we must have 2 channels */
>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>   	case SND_SOC_DAIFMT_I2S:
>> +		if (params_channels(params) != 2)
>> +			return -EINVAL;
>> +	case SND_SOC_DAIFMT_DSP_A:
>> +	case SND_SOC_DAIFMT_DSP_B:
>>   		data_delay = 1;
>>   		break;
>>   	default:
> Same here.  This is also buggy in that it treats DSP A and DSP B
> identically, they are different so the configuration must be incorrect
> for one of them.  I suspect this is configuring for DSP A.
I can remove DSP_B. In actual fact this isn't the real problem.
> You should also really have a setup() function that imposes a channel
> constraint when in I2S mode, wm8988 is one example here.
>
I had a look at wm8988.c and couldn't see this channel constraint 
linking to I2S. Can you point me to it ?

thanks
Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 05:28, Mark Brown wrote:
> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>
>>   	case SND_SOC_DAIFMT_CBS_CFM:
>>   		clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>> +	case SND_SOC_DAIFMT_CBM_CFS:
> Is this fall through deliberate?
>
>> +	/* Default data delay to 1 bit.
>> +	   In I2S mode, we must have 2 channels */
>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>   	case SND_SOC_DAIFMT_I2S:
>> +		if (params_channels(params) != 2)
>> +			return -EINVAL;
>> +	case SND_SOC_DAIFMT_DSP_A:
>> +	case SND_SOC_DAIFMT_DSP_B:
>>   		data_delay = 1;
>>   		break;
>>   	default:
> Same here.  This is also buggy in that it treats DSP A and DSP B
> identically, they are different so the configuration must be incorrect
> for one of them.  I suspect this is configuring for DSP A.
I can remove DSP_B. In actual fact this isn't the real problem.
> You should also really have a setup() function that imposes a channel
> constraint when in I2S mode, wm8988 is one example here.
>
I had a look at wm8988.c and couldn't see this channel constraint 
linking to I2S. Can you point me to it ?

thanks
Matt

Re: [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 07:41, Matt Flax wrote:
>
> On 09/02/17 05:28, Mark Brown wrote:
>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>
>>>       case SND_SOC_DAIFMT_CBS_CFM:
>>>           clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>> +    case SND_SOC_DAIFMT_CBM_CFS:
>> Is this fall through deliberate?
>>
>>> +    /* Default data delay to 1 bit.
>>> +       In I2S mode, we must have 2 channels */
>>>       switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>       case SND_SOC_DAIFMT_I2S:
>>> +        if (params_channels(params) != 2)
>>> +            return -EINVAL;
>>> +    case SND_SOC_DAIFMT_DSP_A:
>>> +    case SND_SOC_DAIFMT_DSP_B:
>>>           data_delay = 1;
>>>           break;
>>>       default:
>> Same here.  This is also buggy in that it treats DSP A and DSP B
>> identically, they are different so the configuration must be incorrect
>> for one of them.  I suspect this is configuring for DSP A.
> I can remove DSP_B. In actual fact this isn't the real problem.
>> You should also really have a setup() function that imposes a channel
>> constraint when in I2S mode, wm8988 is one example here.
>>
> I had a look at wm8988.c and couldn't see this channel constraint 
> linking to I2S. Can you point me to it ?
Hang on, I have got it. It is the patch I sent in previously for the 
wm8580.c codec !

Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 09/02/17 07:41, Matt Flax wrote:
>
> On 09/02/17 05:28, Mark Brown wrote:
>> On Tue, Feb 07, 2017 at 10:09:36AM +1100, Matt Flax wrote:
>>
>>>       case SND_SOC_DAIFMT_CBS_CFM:
>>>           clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
>>> +    case SND_SOC_DAIFMT_CBM_CFS:
>> Is this fall through deliberate?
>>
>>> +    /* Default data delay to 1 bit.
>>> +       In I2S mode, we must have 2 channels */
>>>       switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>>       case SND_SOC_DAIFMT_I2S:
>>> +        if (params_channels(params) != 2)
>>> +            return -EINVAL;
>>> +    case SND_SOC_DAIFMT_DSP_A:
>>> +    case SND_SOC_DAIFMT_DSP_B:
>>>           data_delay = 1;
>>>           break;
>>>       default:
>> Same here.  This is also buggy in that it treats DSP A and DSP B
>> identically, they are different so the configuration must be incorrect
>> for one of them.  I suspect this is configuring for DSP A.
> I can remove DSP_B. In actual fact this isn't the real problem.
>> You should also really have a setup() function that imposes a channel
>> constraint when in I2S mode, wm8988 is one example here.
>>
> I had a look at wm8988.c and couldn't see this channel constraint 
> linking to I2S. Can you point me to it ?
Hang on, I have got it. It is the patch I sent in previously for the 
wm8580.c codec !

Matt

[PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
Min channels is always set to 2. These settings are accomplished
using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown max channels is set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expaneded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..98a90b5 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);
+	else
+		return snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
Min channels is always set to 2. These settings are accomplished
using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown max channels is set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expaneded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..98a90b5 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);
+	else
+		return snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_minmax(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1027 bytes --]

Matt Flax <flatmax@flatmax.org> writes:

> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if

"expanded"

> supported by the sound card hardware.

This is looking a lot nicer!  I'm totally deferring to the audio guys
here for deciding if it's the right fix, but for what it's worth, from
the platform side:

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Eric Anholt]

Matt Flax <flatmax@flatmax.org> writes:

> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if

"expanded"

> supported by the sound card hardware.

This is looking a lot nicer!  I'm totally deferring to the audio guys
here for deciding if it's the right fix, but for what it's worth, from
the platform side:

Acked-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170213/f78424b1/attachment.sig>

Re: [PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Charles Keepax]

On Mon, Feb 13, 2017 at 10:27:15AM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
<snip>
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>  
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);

Would it be worth using snd_pcm_hw_constraint_list here? We are
going to error out of hw_params if the user requests anything
other than 2 or 8 channels so we should probably just constrain
to those two.

> +	else
> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);

snd_pcm_hw_constraint_single, it's exactly the same thing but I
guess we should probably use the helper if it exists.

>  }
>  
>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
>  	 */
>  	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
>  }

ditto.

Thanks,
Charles

[alsa-devel] [PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Charles Keepax]

On Mon, Feb 13, 2017 at 10:27:15AM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
<snip>
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>  
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);

Would it be worth using snd_pcm_hw_constraint_list here? We are
going to error out of hw_params if the user requests anything
other than 2 or 8 channels so we should probably just constrain
to those two.

> +	else
> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);

snd_pcm_hw_constraint_single, it's exactly the same thing but I
guess we should probably use the helper if it exists.

>  }
>  
>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
>  	 */
>  	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_minmax(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
>  }

ditto.

Thanks,
Charles

Re: [PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 14/02/17 20:32, Charles Keepax wrote:
> On Mon, Feb 13, 2017 at 10:27:15AM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
>>
>> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
>> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
>> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
>> Min channels is always set to 2. These settings are accomplished
>> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>>
>> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>>
>> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
>> Before replicating the format variable (from ch2 to ch1) for
>> register loading, requested channels are checked to be either 2 or 8.
>> This can be expaneded later to accomodate other channel counts if
>> supported by the sound card hardware.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
> <snip>
>> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>>   
>> -	return 0;
>> +	/* Set the max channels to 8 if the codec is master and
>> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
>> +	 */
>> +	if (dev->fmt &
>> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
>> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);
> Would it be worth using snd_pcm_hw_constraint_list here? We are
> going to error out of hw_params if the user requests anything
> other than 2 or 8 channels so we should probably just constrain
> to those two.

Actually, I didn't think of this, but there is no reason to have 2 
channels available in DSP A mode with codec master. I will change this 
to constrain the channels to 8.
>
>> +	else
>> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
> snd_pcm_hw_constraint_single, it's exactly the same thing but I
> guess we should probably use the helper if it exists.

Will do.

>>   }
>>   
>>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>>   	 */
>>   	bcm2835_i2s_stop_clock(dev);
>> +
>> +	/* Default to 2 channels */
>> +	snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
>>   }
> ditto.

Will do.

>
> Thanks,
> Charles

[alsa-devel] [PATCH v2] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 14/02/17 20:32, Charles Keepax wrote:
> On Mon, Feb 13, 2017 at 10:27:15AM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
>>
>> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
>> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
>> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
>> Min channels is always set to 2. These settings are accomplished
>> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>>
>> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>>
>> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
>> Before replicating the format variable (from ch2 to ch1) for
>> register loading, requested channels are checked to be either 2 or 8.
>> This can be expaneded later to accomodate other channel counts if
>> supported by the sound card hardware.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
> <snip>
>> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>>   
>> -	return 0;
>> +	/* Set the max channels to 8 if the codec is master and
>> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
>> +	 */
>> +	if (dev->fmt &
>> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
>> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 8);
> Would it be worth using snd_pcm_hw_constraint_list here? We are
> going to error out of hw_params if the user requests anything
> other than 2 or 8 channels so we should probably just constrain
> to those two.

Actually, I didn't think of this, but there is no reason to have 2 
channels available in DSP A mode with codec master. I will change this 
to constrain the channels to 8.
>
>> +	else
>> +		return snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
> snd_pcm_hw_constraint_single, it's exactly the same thing but I
> guess we should probably use the helper if it exists.

Will do.

>>   }
>>   
>>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>>   	 */
>>   	bcm2835_i2s_stop_clock(dev);
>> +
>> +	/* Default to 2 channels */
>> +	snd_pcm_hw_constraint_minmax(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
>>   }
> ditto.

Will do.

>
> Thanks,
> Charles

[PATCH v3] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
Min channels is always set to 2. These settings are accomplished
using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown max channels is set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expaneded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v3] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
Min channels is always set to 2. These settings are accomplished
using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown max channels is set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expaneded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH v3] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 15/02/17 07:52, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if
> supported by the sound card hardware.

I notice my commit message didn't update, will resend with an updated 
commit message as v4.

> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>   sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..300516d 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>   	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>   		data_delay = 1;
>   		break;
>   	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (params_channels(params)) {
>   	case 2:
> +	case 8:
>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>   
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE

[alsa-devel] [PATCH v3] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 15/02/17 07:52, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_minmax is used to set
> max channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and
> SND_SOC_DAIFMT_DSP_A are set. Otherwise, max channels is set to 2.
> Min channels is always set to 2. These settings are accomplished
> using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown max channels is set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expaneded later to accomodate other channel counts if
> supported by the sound card hardware.

I notice my commit message didn't update, will resend with an updated 
commit message as v4.

> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>   sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..300516d 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>   	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>   		data_delay = 1;
>   		break;
>   	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (params_channels(params)) {
>   	case 2:
> +	case 8:
>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>   
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE

[PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Arnaud Mouiche]

Hi,

On 14/02/2017 22:04, Matt Flax wrote:
> [...]
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))

SND_SOC_DAIFMT_DSP_A is not a bit. It is an enum
	#define SND_SOC_DAIFMT_I2S		1 /* I2S mode */
	#define SND_SOC_DAIFMT_RIGHT_J		2 /* Right Justified mode */
	#define SND_SOC_DAIFMT_LEFT_J		3 /* Left Justified mode */
	#define SND_SOC_DAIFMT_DSP_A		4 /* L data MSB after FRM LRC */
	#define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
	#define SND_SOC_DAIFMT_AC97		6 /* AC97 */
	#define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */

So your test is wrong.

Other question that bother me (I'm not familiar with bcm2835, so may be I'm wrong):
why should we put channel number constraint in the SOC driver whereas ASOC already compute
the constraint by making the intersection of channels constraints coming from soc AND codecs.

If you connect one 8 channel capable soc driver (eg. bcm2835) with a 2 channel codec in DSP_A mode, ASOC already tell
the user space that the limit is 2 channels.
Shouldn't you just specify that bcm2835 is 8 channel capable (and may be more), whatever the format is, and let ASOC do the job.
Given a specific device tree connecting the bcm2835 with one or more codecs is sufficient to specify the limits.

And if in the future someone wants to connect a 4 channels capable codec, or a 2x 2channels codecs, must he also send a new patch with 4 channels support ?

Regards,
Arnaud

> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE

Re: [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 15/02/17 19:30, Arnaud Mouiche wrote:
> Hi,
>
> On 14/02/2017 22:04, Matt Flax wrote:
>> [...]
>> +    if (dev->fmt &
>> +        (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
>
> SND_SOC_DAIFMT_DSP_A is not a bit. It is an enum
>     #define SND_SOC_DAIFMT_I2S        1 /* I2S mode */
>     #define SND_SOC_DAIFMT_RIGHT_J        2 /* Right Justified mode */
>     #define SND_SOC_DAIFMT_LEFT_J        3 /* Left Justified mode */
>     #define SND_SOC_DAIFMT_DSP_A        4 /* L data MSB after FRM LRC */
>     #define SND_SOC_DAIFMT_DSP_B        5 /* L data MSB during FRM LRC */
>     #define SND_SOC_DAIFMT_AC97        6 /* AC97 */
>     #define SND_SOC_DAIFMT_PDM        7 /* Pulse density modulation */
>
> So your test is wrong.
>

Indeed I am going to fix this in patch v5.

> Other question that bother me (I'm not familiar with bcm2835, so may 
> be I'm wrong):
> why should we put channel number constraint in the SOC driver whereas 
> ASOC already compute
> the constraint by making the intersection of channels constraints 
> coming from soc AND codecs.
>
> If you connect one 8 channel capable soc driver (eg. bcm2835) with a 2 
> channel codec in DSP_A mode, ASOC already tell
> the user space that the limit is 2 channels.
> Shouldn't you just specify that bcm2835 is 8 channel capable (and may 
> be more), whatever the format is, and let ASOC do the job.
> Given a specific device tree connecting the bcm2835 with one or more 
> codecs is sufficient to specify the limits.
>
> And if in the future someone wants to connect a 4 channels capable 
> codec, or a 2x 2channels codecs, must he also send a new patch with 4 
> channels support ?
>

It does seem simpler using this approach you mention. From what I 
understand it is necessary to guard carefully in the SoC driver so that 
the codecs and machine drivers don't have to implicitly define these guards.

> Regards,
> Arnaud
>
>> +        return snd_pcm_hw_constraint_single(substream->runtime,
>> +            SNDRV_PCM_HW_PARAM_CHANNELS, 8);
>> +    else
>> +        return snd_pcm_hw_constraint_single(substream->runtime,
>> +            SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>     static void bcm2835_i2s_shutdown(struct snd_pcm_substream 
>> *substream,
>> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct 
>> snd_pcm_substream *substream,
>>        * not stop the clock when SND_SOC_DAIFMT_CONT
>>        */
>>       bcm2835_i2s_stop_clock(dev);
>> +
>> +    /* Default to 2 channels */
>> +    snd_pcm_hw_constraint_single(substream->runtime,
>> +            SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>     static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
>> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver 
>> bcm2835_i2s_dai = {
>>       .name    = "bcm2835-i2s",
>>       .probe    = bcm2835_i2s_dai_probe,
>>       .playback = {
>> -        .channels_min = 2,
>> -        .channels_max = 2,
>>           .rates =    SNDRV_PCM_RATE_8000_192000,
>>           .formats =    SNDRV_PCM_FMTBIT_S16_LE
>>                   | SNDRV_PCM_FMTBIT_S24_LE
>>                   | SNDRV_PCM_FMTBIT_S32_LE
>>           },
>>       .capture = {
>> -        .channels_min = 2,
>> -        .channels_max = 2,
>>           .rates =    SNDRV_PCM_RATE_8000_192000,
>>           .formats =    SNDRV_PCM_FMTBIT_S16_LE
>>                   | SNDRV_PCM_FMTBIT_S24_LE
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Charles Keepax]

On Wed, Feb 15, 2017 at 08:04:35AM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

[PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Charles Keepax]

On Wed, Feb 15, 2017 at 08:04:35AM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Florian Kauer]

On 02/14/2017 10:04 PM, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Acked-by: Florian Kauer <florian.kauer@koalo.de>

Looks good and is a much better approach than just adding the additional
case.

Greetings,
Florian

[PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Florian Kauer]

On 02/14/2017 10:04 PM, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Acked-by: Florian Kauer <florian.kauer@koalo.de>

Looks good and is a much better approach than just adding the additional
case.

Greetings,
Florian

Re: [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 862 bytes --]

Matt Flax <flatmax@flatmax.org> writes:

> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Eric Anholt]

Matt Flax <flatmax@flatmax.org> writes:

> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>

Acked-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170215/e70917d7/attachment.sig>

[PATCH v5] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v5] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..300516d 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt &
+		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH v5] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch doesn't include the update, I will resend as v6


On 22/02/17 16:18, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>   sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..300516d 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>   	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>   		data_delay = 1;
>   		break;
>   	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (params_channels(params)) {
>   	case 2:
> +	case 8:
>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>   
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE

[PATCH v5] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch doesn't include the update, I will resend as v6


On 22/02/17 16:18, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
>
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>   sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..300516d 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>   	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>   		data_delay = 1;
>   		break;
>   	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>   
>   	switch (params_channels(params)) {
>   	case 2:
> +	case 8:
>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>   
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE

[PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..48ac884 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
+		  == (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..48ac884 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,16 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
+		  == (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [alsa-devel] [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2202 bytes --]

Hi Matt,

[auto build test WARNING on v4.9-rc8]
[also build test WARNING on next-20170221]
[cannot apply to linux-rpi/for-rpi-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Flax/ASoC-bcm2835-Add-8-channel-multitrack-capability/20170222-134433
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   sound/soc/bcm/bcm2835-i2s.c: In function 'bcm2835_i2s_startup':
>> sound/soc/bcm/bcm2835-i2s.c:534:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
     if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
                  ^

vim +534 sound/soc/bcm/bcm2835-i2s.c

   518		bcm2835_i2s_stop_clock(dev);
   519	
   520		/* Enable PCM block */
   521		regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
   522				BCM2835_I2S_EN, BCM2835_I2S_EN);
   523	
   524		/*
   525		 * Disable STBY.
   526		 * Requires at least 4 PCM clock cycles to take effect.
   527		 */
   528		regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
   529				BCM2835_I2S_STBY, BCM2835_I2S_STBY);
   530	
   531		/* Set the max channels to 8 if the codec is master and
   532		 * we are in DSP A mode. Otherwise only allow 2 channels.
   533		 */
 > 534		if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
   535			  == (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
   536			return snd_pcm_hw_constraint_single(substream->runtime,
   537				SNDRV_PCM_HW_PARAM_CHANNELS, 8);
   538		else
   539			return snd_pcm_hw_constraint_single(substream->runtime,
   540				SNDRV_PCM_HW_PARAM_CHANNELS, 2);
   541	}
   542	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47879 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[alsa-devel] [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[kbuild test robot]

Hi Matt,

[auto build test WARNING on v4.9-rc8]
[also build test WARNING on next-20170221]
[cannot apply to linux-rpi/for-rpi-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matt-Flax/ASoC-bcm2835-Add-8-channel-multitrack-capability/20170222-134433
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   sound/soc/bcm/bcm2835-i2s.c: In function 'bcm2835_i2s_startup':
>> sound/soc/bcm/bcm2835-i2s.c:534:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
     if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
                  ^

vim +534 sound/soc/bcm/bcm2835-i2s.c

   518		bcm2835_i2s_stop_clock(dev);
   519	
   520		/* Enable PCM block */
   521		regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
   522				BCM2835_I2S_EN, BCM2835_I2S_EN);
   523	
   524		/*
   525		 * Disable STBY.
   526		 * Requires at least 4 PCM clock cycles to take effect.
   527		 */
   528		regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
   529				BCM2835_I2S_STBY, BCM2835_I2S_STBY);
   530	
   531		/* Set the max channels to 8 if the codec is master and
   532		 * we are in DSP A mode. Otherwise only allow 2 channels.
   533		 */
 > 534		if (dev->fmt & (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK)
   535			  == (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
   536			return snd_pcm_hw_constraint_single(substream->runtime,
   537				SNDRV_PCM_HW_PARAM_CHANNELS, 8);
   538		else
   539			return snd_pcm_hw_constraint_single(substream->runtime,
   540				SNDRV_PCM_HW_PARAM_CHANNELS, 2);
   541	}
   542	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 47879 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170222/de4fd1b4/attachment-0001.gz>

Re: [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 335 bytes --]

On Wed, Feb 22, 2017 at 04:22:29PM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.

As I'm pretty sure I've said before please don't send new patches in
reply to existing threads, it makes it much harder to follow what's
going on and what the current versions of things are.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

On Wed, Feb 22, 2017 at 04:22:29PM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.

As I'm pretty sure I've said before please don't send new patches in
reply to existing threads, it makes it much harder to follow what's
going on and what the current versions of things are.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170222/f700dc63/attachment.sig>

Re: [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 23/02/17 05:28, Mark Brown wrote:
> On Wed, Feb 22, 2017 at 04:22:29PM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
> As I'm pretty sure I've said before please don't send new patches in
> reply to existing threads, it makes it much harder to follow what's
> going on and what the current versions of things are.
>

Understood, would you like me to resend my patch on a new thread ?

thanks
Matt

[alsa-devel] [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 23/02/17 05:28, Mark Brown wrote:
> On Wed, Feb 22, 2017 at 04:22:29PM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
> As I'm pretty sure I've said before please don't send new patches in
> reply to existing threads, it makes it much harder to follow what's
> going on and what the current versions of things are.
>

Understood, would you like me to resend my patch on a new thread ?

thanks
Matt

Re: [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 203 bytes --]

On Thu, Feb 23, 2017 at 09:45:54AM +1100, Matt Flax wrote:

> Understood, would you like me to resend my patch on a new thread ?

It's fine, I've seen it this time.  Just please remember this in future.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH v6] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Mark Brown]

On Thu, Feb 23, 2017 at 09:45:54AM +1100, Matt Flax wrote:

> Understood, would you like me to resend my patch on a new thread ?

It's fine, I've seen it this time.  Just please remember this in future.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170223/8a4360ea/attachment.sig>

[PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..4b5f3f1 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if ((dev->fmt &
+		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
+			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

This patch adds multitrack capability if in DSP mode A and the
codec is master.

In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
are set. Otherwise, channels are set to 2. These settings are
accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.

In bcm2835_i2s_shutdown the channels are set to 2 by default.

In bcm2835_i2s_hw_params, DSP mode A format is now an option.
Before replicating the format variable (from ch2 to ch1) for
register loading, requested channels are checked to be either 2 or 8.
This can be expanded later to accomodate other channel counts if
supported by the sound card hardware.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..4b5f3f1 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		data_delay = 1;
 		break;
 	default:
@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
 	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
 			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
 
-	return 0;
+	/* Set the max channels to 8 if the codec is master and
+	 * we are in DSP A mode. Otherwise only allow 2 channels.
+	 */
+	if ((dev->fmt &
+		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
+			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
+	else
+		return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
@@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
 	 * not stop the clock when SND_SOC_DAIFMT_CONT
 	 */
 	bcm2835_i2s_stop_clock(dev);
+
+	/* Default to 2 channels */
+	snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
 }
 
 static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
@@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.name	= "bcm2835-i2s",
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
 				| SNDRV_PCM_FMTBIT_S32_LE
 		},
 	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

Hi Matt,

please include me in CC, your emails don't seem to get through
linux-rpi-kernel reliably.

I did a few tests with WM5102 in DSP A mode connected to RPi3
with downstream kernel 4.9.11 plus your patch. wm5102 was configured
as master, cpu<->codec dai_link.dai_fmt =
SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM

Comments are inline.

On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax at flatmax.org>
> ---
>  sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..4b5f3f1 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>  		data_delay = 1;

In DSP_A mode data_delay should be set to 0. With data_delay = 1
the MSB is transmitted 1 clock cycle too late and the LSB of the
previous sample is received as MSB (i.e. you get loud noise).

See these screenshots, I2S data was 0xF000 (S16_LE format)

data_delay = 1:
http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png

data_delay = 0:
http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png

>  		break;
>  	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>  
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if ((dev->fmt &
> +		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
> +			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);

I'm not sure why you are limiting to exactly 8 channels. 2 channels
worked fine here, too. Haven't tested with 4 or 6 channels yet but I
guess any even number of channels should work.

In 8-channel configuration I often got swapped/shifted channels.
Not 100% sure why, but probably because bcm2835 hadn't DMAed the
data in when the codec started the clocks - in that case bcm2835 seems
to repeat the last stereo frame data it had in it's buffer. I haven't
digged into that deeper though, could be something in my test setup as
well.

> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>  }
>  
>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
>  	 */
>  	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>  }
>  
>  static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>  	.name	= "bcm2835-i2s",
>  	.probe	= bcm2835_i2s_dai_probe,
>  	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>  		.rates =	SNDRV_PCM_RATE_8000_192000,
>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>  				| SNDRV_PCM_FMTBIT_S24_LE
>  				| SNDRV_PCM_FMTBIT_S32_LE
>  		},
>  	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>  		.rates =	SNDRV_PCM_RATE_8000_192000,
>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>  				| SNDRV_PCM_FMTBIT_S24_LE

With .channels_max removed I get no alsa PCMs with the downstream
card drivers - "aplay -L" only reported the "null" PCM.

Changing that to .channels_min = 2, .channels_max = 8 brought
back the PCMs.

so long,

Hias

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

Hi Matt,

please include me in CC, your emails don't seem to get through
linux-rpi-kernel reliably.

I did a few tests with WM5102 in DSP A mode connected to RPi3
with downstream kernel 4.9.11 plus your patch. wm5102 was configured
as master, cpu<->codec dai_link.dai_fmt =
SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM

Comments are inline.

On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
> This patch adds multitrack capability if in DSP mode A and the
> codec is master.
> 
> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> are set. Otherwise, channels are set to 2. These settings are
> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> 
> In bcm2835_i2s_shutdown the channels are set to 2 by default.
> 
> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> Before replicating the format variable (from ch2 to ch1) for
> register loading, requested channels are checked to be either 2 or 8.
> This can be expanded later to accomodate other channel counts if
> supported by the sound card hardware.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>  sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..4b5f3f1 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>  	case SND_SOC_DAIFMT_I2S:
> +	case SND_SOC_DAIFMT_DSP_A:
>  		data_delay = 1;

In DSP_A mode data_delay should be set to 0. With data_delay = 1
the MSB is transmitted 1 clock cycle too late and the LSB of the
previous sample is received as MSB (i.e. you get loud noise).

See these screenshots, I2S data was 0xF000 (S16_LE format)

data_delay = 1:
http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png

data_delay = 0:
http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png

>  		break;
>  	default:
> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> @@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>  
> -	return 0;
> +	/* Set the max channels to 8 if the codec is master and
> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
> +	 */
> +	if ((dev->fmt &
> +		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
> +			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);

I'm not sure why you are limiting to exactly 8 channels. 2 channels
worked fine here, too. Haven't tested with 4 or 6 channels yet but I
guess any even number of channels should work.

In 8-channel configuration I often got swapped/shifted channels.
Not 100% sure why, but probably because bcm2835 hadn't DMAed the
data in when the codec started the clocks - in that case bcm2835 seems
to repeat the last stereo frame data it had in it's buffer. I haven't
digged into that deeper though, could be something in my test setup as
well.

> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>  }
>  
>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
>  	 */
>  	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>  }
>  
>  static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>  	.name	= "bcm2835-i2s",
>  	.probe	= bcm2835_i2s_dai_probe,
>  	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>  		.rates =	SNDRV_PCM_RATE_8000_192000,
>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>  				| SNDRV_PCM_FMTBIT_S24_LE
>  				| SNDRV_PCM_FMTBIT_S32_LE
>  		},
>  	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>  		.rates =	SNDRV_PCM_RATE_8000_192000,
>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>  				| SNDRV_PCM_FMTBIT_S24_LE

With .channels_max removed I get no alsa PCMs with the downstream
card drivers - "aplay -L" only reported the "null" PCM.

Changing that to .channels_min = 2, .channels_max = 8 brought
back the PCMs.

so long,

Hias

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 24/02/17 23:18, Matthias Reichl wrote:
> Hi Matt,
>
> please include me in CC, your emails don't seem to get through
> linux-rpi-kernel reliably.
>
> I did a few tests with WM5102 in DSP A mode connected to RPi3
> with downstream kernel 4.9.11 plus your patch. wm5102 was configured
> as master, cpu<->codec dai_link.dai_fmt =
> SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM
>
> Comments are inline.

Sure will do, thanks for testing. My comments are also inline.

>
> On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
>>
>> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
>> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
>> are set. Otherwise, channels are set to 2. These settings are
>> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>>
>> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>>
>> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
>> Before replicating the format variable (from ch2 to ch1) for
>> register loading, requested channels are checked to be either 2 or 8.
>> This can be expanded later to accomodate other channel counts if
>> supported by the sound card hardware.
>>
>> Signed-off-by: Matt Flax <flatmax at flatmax.org>
>> ---
>>   sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
>>   1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
>> index 6ba2049..4b5f3f1 100644
>> --- a/sound/soc/bcm/bcm2835-i2s.c
>> +++ b/sound/soc/bcm/bcm2835-i2s.c
>> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>   	case SND_SOC_DAIFMT_I2S:
>> +	case SND_SOC_DAIFMT_DSP_A:
>>   		data_delay = 1;
> In DSP_A mode data_delay should be set to 0. With data_delay = 1
> the MSB is transmitted 1 clock cycle too late and the LSB of the
> previous sample is received as MSB (i.e. you get loud noise).
>
> See these screenshots, I2S data was 0xF000 (S16_LE format)
>
> data_delay = 1:
> http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png
>
> data_delay = 0:
> http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png

I can do this, however that would require DSP mode B to have an offset 
of -1, which can't be set in the BCM2835 position register.

I really like your approach, however it needs extra resources at the 
hardware level. Without those resources it will never be usable and 
stable. The reason is that you will never be able to get proper 
synchrony and your channels will be randomly shifted. It is the curious 
nature of the BCM2835 I2S silicon - I have used an FPGA to overcome this 
problem.

>>   		break;
>>   	default:
>> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> @@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>>   
>> -	return 0;
>> +	/* Set the max channels to 8 if the codec is master and
>> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
>> +	 */
>> +	if ((dev->fmt &
>> +		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
>> +			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
>> +		return snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> I'm not sure why you are limiting to exactly 8 channels. 2 channels
> worked fine here, too. Haven't tested with 4 or 6 channels yet but I
> guess any even number of channels should work.
>
> In 8-channel configuration I often got swapped/shifted channels.
> Not 100% sure why, but probably because bcm2835 hadn't DMAed the
> data in when the codec started the clocks - in that case bcm2835 seems
> to repeat the last stereo frame data it had in it's buffer. I haven't
> digged into that deeper though, could be something in my test setup as
> well.

Ah - there you go - I would imagine they are only shifted, not swapped !

I have a very robust stream, where I never get swapped channels. In 
other words, I find that the channels 1 to 8 are always locked to the 
correct pins.

Multichannel simply can't be done (on the BCM2835) without a suitable 
FPGA/ASIC/chip between the codec and the BCM2835.

>> +	else
>> +		return snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>   
>>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>> @@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>>   	 */
>>   	bcm2835_i2s_stop_clock(dev);
>> +
>> +	/* Default to 2 channels */
>> +	snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>   
>>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
>> @@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>>   	.name	= "bcm2835-i2s",
>>   	.probe	= bcm2835_i2s_dai_probe,
>>   	.playback = {
>> -		.channels_min = 2,
>> -		.channels_max = 2,
>>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>>   				| SNDRV_PCM_FMTBIT_S24_LE
>>   				| SNDRV_PCM_FMTBIT_S32_LE
>>   		},
>>   	.capture = {
>> -		.channels_min = 2,
>> -		.channels_max = 2,
>>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>>   				| SNDRV_PCM_FMTBIT_S24_LE
> With .channels_max removed I get no alsa PCMs with the downstream
> card drivers - "aplay -L" only reported the "null" PCM.
>
> Changing that to .channels_min = 2, .channels_max = 8 brought
> back the PCMs.
>
>
That is curious, channels_max and channels_min are set in _startup 
implicitly in the snd_pcm_hw_constraint_single call which is inlined to 
the snd_pcm_hw_constraint_minmax call.

I have tested with an Audio Injector stereo card for the Pi ... I can 
confirm this problem. I will look into it further.

Can anyone explain why this would happen ?

Matt

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 24/02/17 23:18, Matthias Reichl wrote:
> Hi Matt,
>
> please include me in CC, your emails don't seem to get through
> linux-rpi-kernel reliably.
>
> I did a few tests with WM5102 in DSP A mode connected to RPi3
> with downstream kernel 4.9.11 plus your patch. wm5102 was configured
> as master, cpu<->codec dai_link.dai_fmt =
> SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM
>
> Comments are inline.

Sure will do, thanks for testing. My comments are also inline.

>
> On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
>> This patch adds multitrack capability if in DSP mode A and the
>> codec is master.
>>
>> In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
>> channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
>> are set. Otherwise, channels are set to 2. These settings are
>> accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
>>
>> In bcm2835_i2s_shutdown the channels are set to 2 by default.
>>
>> In bcm2835_i2s_hw_params, DSP mode A format is now an option.
>> Before replicating the format variable (from ch2 to ch1) for
>> register loading, requested channels are checked to be either 2 or 8.
>> This can be expanded later to accomodate other channel counts if
>> supported by the sound card hardware.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
>>   sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
>>   1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
>> index 6ba2049..4b5f3f1 100644
>> --- a/sound/soc/bcm/bcm2835-i2s.c
>> +++ b/sound/soc/bcm/bcm2835-i2s.c
>> @@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>>   	case SND_SOC_DAIFMT_I2S:
>> +	case SND_SOC_DAIFMT_DSP_A:
>>   		data_delay = 1;
> In DSP_A mode data_delay should be set to 0. With data_delay = 1
> the MSB is transmitted 1 clock cycle too late and the LSB of the
> previous sample is received as MSB (i.e. you get loud noise).
>
> See these screenshots, I2S data was 0xF000 (S16_LE format)
>
> data_delay = 1:
> http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png
>
> data_delay = 0:
> http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png

I can do this, however that would require DSP mode B to have an offset 
of -1, which can't be set in the BCM2835 position register.

I really like your approach, however it needs extra resources at the 
hardware level. Without those resources it will never be usable and 
stable. The reason is that you will never be able to get proper 
synchrony and your channels will be randomly shifted. It is the curious 
nature of the BCM2835 I2S silicon - I have used an FPGA to overcome this 
problem.

>>   		break;
>>   	default:
>> @@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
>>   
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> @@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
>>   	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
>>   			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
>>   
>> -	return 0;
>> +	/* Set the max channels to 8 if the codec is master and
>> +	 * we are in DSP A mode. Otherwise only allow 2 channels.
>> +	 */
>> +	if ((dev->fmt &
>> +		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
>> +			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
>> +		return snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> I'm not sure why you are limiting to exactly 8 channels. 2 channels
> worked fine here, too. Haven't tested with 4 or 6 channels yet but I
> guess any even number of channels should work.
>
> In 8-channel configuration I often got swapped/shifted channels.
> Not 100% sure why, but probably because bcm2835 hadn't DMAed the
> data in when the codec started the clocks - in that case bcm2835 seems
> to repeat the last stereo frame data it had in it's buffer. I haven't
> digged into that deeper though, could be something in my test setup as
> well.

Ah - there you go - I would imagine they are only shifted, not swapped !

I have a very robust stream, where I never get swapped channels. In 
other words, I find that the channels 1 to 8 are always locked to the 
correct pins.

Multichannel simply can't be done (on the BCM2835) without a suitable 
FPGA/ASIC/chip between the codec and the BCM2835.

>> +	else
>> +		return snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>   
>>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>> @@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>>   	 */
>>   	bcm2835_i2s_stop_clock(dev);
>> +
>> +	/* Default to 2 channels */
>> +	snd_pcm_hw_constraint_single(substream->runtime,
>> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>>   }
>>   
>>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
>> @@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>>   	.name	= "bcm2835-i2s",
>>   	.probe	= bcm2835_i2s_dai_probe,
>>   	.playback = {
>> -		.channels_min = 2,
>> -		.channels_max = 2,
>>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>>   				| SNDRV_PCM_FMTBIT_S24_LE
>>   				| SNDRV_PCM_FMTBIT_S32_LE
>>   		},
>>   	.capture = {
>> -		.channels_min = 2,
>> -		.channels_max = 2,
>>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>>   				| SNDRV_PCM_FMTBIT_S24_LE
> With .channels_max removed I get no alsa PCMs with the downstream
> card drivers - "aplay -L" only reported the "null" PCM.
>
> Changing that to .channels_min = 2, .channels_max = 8 brought
> back the PCMs.
>
>
That is curious, channels_max and channels_min are set in _startup 
implicitly in the snd_pcm_hw_constraint_single call which is inlined to 
the snd_pcm_hw_constraint_minmax call.

I have tested with an Audio Injector stereo card for the Pi ... I can 
confirm this problem. I will look into it further.

Can anyone explain why this would happen ?

Matt

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Sat, Feb 25, 2017 at 12:50:47AM +1100, Matt Flax wrote:
> 
> 
> On 24/02/17 23:18, Matthias Reichl wrote:
> >Hi Matt,
> >
> >please include me in CC, your emails don't seem to get through
> >linux-rpi-kernel reliably.
> >
> >I did a few tests with WM5102 in DSP A mode connected to RPi3
> >with downstream kernel 4.9.11 plus your patch. wm5102 was configured
> >as master, cpu<->codec dai_link.dai_fmt =
> >SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM
> >
> >Comments are inline.
> 
> Sure will do, thanks for testing. My comments are also inline.
> 
> >
> >On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
> >>This patch adds multitrack capability if in DSP mode A and the
> >>codec is master.
> >>
> >>In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> >>channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> >>are set. Otherwise, channels are set to 2. These settings are
> >>accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> >>
> >>In bcm2835_i2s_shutdown the channels are set to 2 by default.
> >>
> >>In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> >>Before replicating the format variable (from ch2 to ch1) for
> >>register loading, requested channels are checked to be either 2 or 8.
> >>This can be expanded later to accomodate other channel counts if
> >>supported by the sound card hardware.
> >>
> >>Signed-off-by: Matt Flax <flatmax at flatmax.org>
> >>---
> >>  sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
> >>  1 file changed, 17 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> >>index 6ba2049..4b5f3f1 100644
> >>--- a/sound/soc/bcm/bcm2835-i2s.c
> >>+++ b/sound/soc/bcm/bcm2835-i2s.c
> >>@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
> >>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >>  	case SND_SOC_DAIFMT_I2S:
> >>+	case SND_SOC_DAIFMT_DSP_A:
> >>  		data_delay = 1;
> >In DSP_A mode data_delay should be set to 0. With data_delay = 1
> >the MSB is transmitted 1 clock cycle too late and the LSB of the
> >previous sample is received as MSB (i.e. you get loud noise).
> >
> >See these screenshots, I2S data was 0xF000 (S16_LE format)
> >
> >data_delay = 1:
> >http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png
> >
> >data_delay = 0:
> >http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png
> 
> I can do this, however that would require DSP mode B to have an offset of
> -1, which can't be set in the BCM2835 position register.

I don't see where you are using mode B - you only added support for
mode A. Not quite sure what you mean with that.

> I really like your approach, however it needs extra resources at the
> hardware level. Without those resources it will never be usable and stable.
> The reason is that you will never be able to get proper synchrony and your
> channels will be randomly shifted. It is the curious nature of the BCM2835
> I2S silicon - I have used an FPGA to overcome this problem.

I don't quite follow you here. DSP mode A timing usually means that
the MSB follows immediately after the 1-cycle LR pulse. Or, if the
pulse is longer than one cycle, MSB starts 1 clk after the leading
LR pulse edge.

With data delay=1 you are using I2S timing, not mode A timing.

> >>  		break;
> >>  	default:
> >>@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
> >>  	switch (params_channels(params)) {
> >>  	case 2:
> >>+	case 8:
> >>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
> >>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
> >>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> >>@@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
> >>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
> >>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
> >>-	return 0;
> >>+	/* Set the max channels to 8 if the codec is master and
> >>+	 * we are in DSP A mode. Otherwise only allow 2 channels.
> >>+	 */
> >>+	if ((dev->fmt &
> >>+		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
> >>+			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> >>+		return snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> >I'm not sure why you are limiting to exactly 8 channels. 2 channels
> >worked fine here, too. Haven't tested with 4 or 6 channels yet but I
> >guess any even number of channels should work.
> >
> >In 8-channel configuration I often got swapped/shifted channels.
> >Not 100% sure why, but probably because bcm2835 hadn't DMAed the
> >data in when the codec started the clocks - in that case bcm2835 seems
> >to repeat the last stereo frame data it had in it's buffer. I haven't
> >digged into that deeper though, could be something in my test setup as
> >well.
> 
> Ah - there you go - I would imagine they are only shifted, not swapped !
> 
> I have a very robust stream, where I never get swapped channels. In other
> words, I find that the channels 1 to 8 are always locked to the correct
> pins.
> 
> Multichannel simply can't be done (on the BCM2835) without a suitable
> FPGA/ASIC/chip between the codec and the BCM2835.

I'd put it in another way: it seems to me that the bcm2835 I2S
won't reliably sync in DSP mode A.

I did some more tests and noticed channel/sample shifts both in
2-channel and in 8-channel DSP mode A. When using 2-channel I2S
mode I never got any channel/sample shifts.

Here are screenshots of 8-channel S16_LE data, first transmitted
sample is 0xf000, then 0xf100, then 0xf200 ... up to 0xf700.

This is the first full frame with data from bcm2835. Before that
the clocks were active for about 1.2ms and bcm2835 transmitted
a low data signal:

http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-8ch-frame1.png

Note that immediately after LR pulse there's not a 0xf000 sample
but 0x0000. The transmitted data doesn't seem to be synced on
sample boundaries at all (i.e. the seem to be bit-shifted),
let alone frame-synced.

Here's the second full frame:

http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-8ch-frame2.png

Note that the first sample is rather odd, MSB isn't 1 but 0
and then a long run of 1 bits follow. About 8us after that
the data sees to be sample-synced.

But also note that at the 3rd frame with data starts with 0xf500,
not 0xf000 as expected. At least the samples are now synced
and MSB occurs on the correct position. Still, the rest of the
stream has the samples/channels shifted by 5.

> 
> >>+	else
> >>+		return snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> >>  }
> >>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> >>@@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> >>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
> >>  	 */
> >>  	bcm2835_i2s_stop_clock(dev);
> >>+
> >>+	/* Default to 2 channels */
> >>+	snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> >>  }
> >>  static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> >>@@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
> >>  	.name	= "bcm2835-i2s",
> >>  	.probe	= bcm2835_i2s_dai_probe,
> >>  	.playback = {
> >>-		.channels_min = 2,
> >>-		.channels_max = 2,
> >>  		.rates =	SNDRV_PCM_RATE_8000_192000,
> >>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
> >>  				| SNDRV_PCM_FMTBIT_S24_LE
> >>  				| SNDRV_PCM_FMTBIT_S32_LE
> >>  		},
> >>  	.capture = {
> >>-		.channels_min = 2,
> >>-		.channels_max = 2,
> >>  		.rates =	SNDRV_PCM_RATE_8000_192000,
> >>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
> >>  				| SNDRV_PCM_FMTBIT_S24_LE
> >With .channels_max removed I get no alsa PCMs with the downstream
> >card drivers - "aplay -L" only reported the "null" PCM.
> >
> >Changing that to .channels_min = 2, .channels_max = 8 brought
> >back the PCMs.
> >
> >
> That is curious, channels_max and channels_min are set in _startup
> implicitly in the snd_pcm_hw_constraint_single call which is inlined to the
> snd_pcm_hw_constraint_minmax call.
> 
> I have tested with an Audio Injector stereo card for the Pi ... I can
> confirm this problem. I will look into it further.
> 
> Can anyone explain why this would happen ?
> 
> Matt
> 

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Sat, Feb 25, 2017 at 12:50:47AM +1100, Matt Flax wrote:
> 
> 
> On 24/02/17 23:18, Matthias Reichl wrote:
> >Hi Matt,
> >
> >please include me in CC, your emails don't seem to get through
> >linux-rpi-kernel reliably.
> >
> >I did a few tests with WM5102 in DSP A mode connected to RPi3
> >with downstream kernel 4.9.11 plus your patch. wm5102 was configured
> >as master, cpu<->codec dai_link.dai_fmt =
> >SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM
> >
> >Comments are inline.
> 
> Sure will do, thanks for testing. My comments are also inline.
> 
> >
> >On Wed, Feb 22, 2017 at 05:56:40PM +1100, Matt Flax wrote:
> >>This patch adds multitrack capability if in DSP mode A and the
> >>codec is master.
> >>
> >>In bcm2835_i2s_startup, snd_pcm_hw_constraint_single is used to set
> >>channels to 8 if both SND_SOC_DAIFMT_CBM_CFM and SND_SOC_DAIFMT_DSP_A
> >>are set. Otherwise, channels are set to 2. These settings are
> >>accomplished using the SNDRV_PCM_HW_PARAM_CHANNELS variable.
> >>
> >>In bcm2835_i2s_shutdown the channels are set to 2 by default.
> >>
> >>In bcm2835_i2s_hw_params, DSP mode A format is now an option.
> >>Before replicating the format variable (from ch2 to ch1) for
> >>register loading, requested channels are checked to be either 2 or 8.
> >>This can be expanded later to accomodate other channel counts if
> >>supported by the sound card hardware.
> >>
> >>Signed-off-by: Matt Flax <flatmax@flatmax.org>
> >>---
> >>  sound/soc/bcm/bcm2835-i2s.c | 22 +++++++++++++++++-----
> >>  1 file changed, 17 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> >>index 6ba2049..4b5f3f1 100644
> >>--- a/sound/soc/bcm/bcm2835-i2s.c
> >>+++ b/sound/soc/bcm/bcm2835-i2s.c
> >>@@ -296,6 +296,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
> >>  	switch (dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >>  	case SND_SOC_DAIFMT_I2S:
> >>+	case SND_SOC_DAIFMT_DSP_A:
> >>  		data_delay = 1;
> >In DSP_A mode data_delay should be set to 0. With data_delay = 1
> >the MSB is transmitted 1 clock cycle too late and the LSB of the
> >previous sample is received as MSB (i.e. you get loud noise).
> >
> >See these screenshots, I2S data was 0xF000 (S16_LE format)
> >
> >data_delay = 1:
> >http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-1.png
> >
> >data_delay = 0:
> >http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-delay-0.png
> 
> I can do this, however that would require DSP mode B to have an offset of
> -1, which can't be set in the BCM2835 position register.

I don't see where you are using mode B - you only added support for
mode A. Not quite sure what you mean with that.

> I really like your approach, however it needs extra resources at the
> hardware level. Without those resources it will never be usable and stable.
> The reason is that you will never be able to get proper synchrony and your
> channels will be randomly shifted. It is the curious nature of the BCM2835
> I2S silicon - I have used an FPGA to overcome this problem.

I don't quite follow you here. DSP mode A timing usually means that
the MSB follows immediately after the 1-cycle LR pulse. Or, if the
pulse is longer than one cycle, MSB starts 1 clk after the leading
LR pulse edge.

With data delay=1 you are using I2S timing, not mode A timing.

> >>  		break;
> >>  	default:
> >>@@ -312,6 +313,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
> >>  	switch (params_channels(params)) {
> >>  	case 2:
> >>+	case 8:
> >>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
> >>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
> >>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> >>@@ -526,7 +528,17 @@ static int bcm2835_i2s_startup(struct snd_pcm_substream *substream,
> >>  	regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
> >>  			BCM2835_I2S_STBY, BCM2835_I2S_STBY);
> >>-	return 0;
> >>+	/* Set the max channels to 8 if the codec is master and
> >>+	 * we are in DSP A mode. Otherwise only allow 2 channels.
> >>+	 */
> >>+	if ((dev->fmt &
> >>+		(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK))
> >>+			== (SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))
> >>+		return snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> >I'm not sure why you are limiting to exactly 8 channels. 2 channels
> >worked fine here, too. Haven't tested with 4 or 6 channels yet but I
> >guess any even number of channels should work.
> >
> >In 8-channel configuration I often got swapped/shifted channels.
> >Not 100% sure why, but probably because bcm2835 hadn't DMAed the
> >data in when the codec started the clocks - in that case bcm2835 seems
> >to repeat the last stereo frame data it had in it's buffer. I haven't
> >digged into that deeper though, could be something in my test setup as
> >well.
> 
> Ah - there you go - I would imagine they are only shifted, not swapped !
> 
> I have a very robust stream, where I never get swapped channels. In other
> words, I find that the channels 1 to 8 are always locked to the correct
> pins.
> 
> Multichannel simply can't be done (on the BCM2835) without a suitable
> FPGA/ASIC/chip between the codec and the BCM2835.

I'd put it in another way: it seems to me that the bcm2835 I2S
won't reliably sync in DSP mode A.

I did some more tests and noticed channel/sample shifts both in
2-channel and in 8-channel DSP mode A. When using 2-channel I2S
mode I never got any channel/sample shifts.

Here are screenshots of 8-channel S16_LE data, first transmitted
sample is 0xf000, then 0xf100, then 0xf200 ... up to 0xf700.

This is the first full frame with data from bcm2835. Before that
the clocks were active for about 1.2ms and bcm2835 transmitted
a low data signal:

http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-8ch-frame1.png

Note that immediately after LR pulse there's not a 0xf000 sample
but 0x0000. The transmitted data doesn't seem to be synced on
sample boundaries at all (i.e. the seem to be bit-shifted),
let alone frame-synced.

Here's the second full frame:

http://www.horus.com/~hias/tmp/rpi/bcm2835-dsp-a-8ch-frame2.png

Note that the first sample is rather odd, MSB isn't 1 but 0
and then a long run of 1 bits follow. About 8us after that
the data sees to be sample-synced.

But also note that at the 3rd frame with data starts with 0xf500,
not 0xf000 as expected. At least the samples are now synced
and MSB occurs on the correct position. Still, the rest of the
stream has the samples/channels shifted by 5.

> 
> >>+	else
> >>+		return snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> >>  }
> >>  static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> >>@@ -549,6 +561,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> >>  	 * not stop the clock when SND_SOC_DAIFMT_CONT
> >>  	 */
> >>  	bcm2835_i2s_stop_clock(dev);
> >>+
> >>+	/* Default to 2 channels */
> >>+	snd_pcm_hw_constraint_single(substream->runtime,
> >>+			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> >>  }
> >>  static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> >>@@ -576,16 +592,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
> >>  	.name	= "bcm2835-i2s",
> >>  	.probe	= bcm2835_i2s_dai_probe,
> >>  	.playback = {
> >>-		.channels_min = 2,
> >>-		.channels_max = 2,
> >>  		.rates =	SNDRV_PCM_RATE_8000_192000,
> >>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
> >>  				| SNDRV_PCM_FMTBIT_S24_LE
> >>  				| SNDRV_PCM_FMTBIT_S32_LE
> >>  		},
> >>  	.capture = {
> >>-		.channels_min = 2,
> >>-		.channels_max = 2,
> >>  		.rates =	SNDRV_PCM_RATE_8000_192000,
> >>  		.formats =	SNDRV_PCM_FMTBIT_S16_LE
> >>  				| SNDRV_PCM_FMTBIT_S24_LE
> >With .channels_max removed I get no alsa PCMs with the downstream
> >card drivers - "aplay -L" only reported the "null" PCM.
> >
> >Changing that to .channels_min = 2, .channels_max = 8 brought
> >back the PCMs.
> >
> >
> That is curious, channels_max and channels_min are set in _startup
> implicitly in the snd_pcm_hw_constraint_single call which is inlined to the
> snd_pcm_hw_constraint_minmax call.
> 
> I have tested with an Audio Injector stereo card for the Pi ... I can
> confirm this problem. I will look into it further.
> 
> Can anyone explain why this would happen ?
> 
> Matt
> 

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 07:25, Matthias Reichl wrote:

> I'd put it in another way: it seems to me that the bcm2835 I2S
> won't reliably sync in DSP mode A.
>
>

What you are explaining here is the nature of the BCM2835 responding to 
a codec master. The hardware setup you are describing is elegantly 
simple, but TOO simple.

Measurements on a system which uses a codec master don't represent a 
robust implementation of multichannel with the BCM2835. My experiments 
(and now yours) have shown that the system has a bit shift and an 
uncontrollable channel drift.

Multichannel simply can't be done (on the BCM2835) without control at 
the hardware level - this requires an intermediate master control chip.

The Audio Injector Octo machine driver, sets both the codec and the 
BCM2835 as slaves - the FPGA is master because it has to control both 
the BCM2835 and the codec's systems to get reliability. You can see the 
machine driver on github :
https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c

It has taken month to implement a robust solution to this problem using 
an FPGA and the result is a fantastic (Audio Injector Octo) sound card. 
I am almost certain this hardware technique can be used on all stereo 
I2S SoC chips.


Matt

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 07:25, Matthias Reichl wrote:

> I'd put it in another way: it seems to me that the bcm2835 I2S
> won't reliably sync in DSP mode A.
>
>

What you are explaining here is the nature of the BCM2835 responding to 
a codec master. The hardware setup you are describing is elegantly 
simple, but TOO simple.

Measurements on a system which uses a codec master don't represent a 
robust implementation of multichannel with the BCM2835. My experiments 
(and now yours) have shown that the system has a bit shift and an 
uncontrollable channel drift.

Multichannel simply can't be done (on the BCM2835) without control at 
the hardware level - this requires an intermediate master control chip.

The Audio Injector Octo machine driver, sets both the codec and the 
BCM2835 as slaves - the FPGA is master because it has to control both 
the BCM2835 and the codec's systems to get reliability. You can see the 
machine driver on github :
https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c

It has taken month to implement a robust solution to this problem using 
an FPGA and the result is a fantastic (Audio Injector Octo) sound card. 
I am almost certain this hardware technique can be used on all stereo 
I2S SoC chips.


Matt

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
> 
> 
> On 25/02/17 07:25, Matthias Reichl wrote:
> 
> >I'd put it in another way: it seems to me that the bcm2835 I2S
> >won't reliably sync in DSP mode A.
> >
> >
> 
> What you are explaining here is the nature of the BCM2835 responding to a
> codec master. The hardware setup you are describing is elegantly simple, but
> TOO simple.

Your patch only changes the CPU DAI driver. At this point it doesn't
matter if the codec or an external source is driving the clocks.

If the clock timing adheres to DSP mode A timing and you add code
to the the CPU DAI driver so it can operate in DSP mode A then
that should also work. If not, it's broken.

The (rather brief) publically available bcm2835 datasheet only
mentions I2S mode and my tests lead me to the conclusion that the
bcm2835 is actually expecting I2S timing, not DSP A - at least with
the current code.

> Measurements on a system which uses a codec master don't represent a robust
> implementation of multichannel with the BCM2835. My experiments (and now
> yours) have shown that the system has a bit shift and an uncontrollable
> channel drift.
> 
> Multichannel simply can't be done (on the BCM2835) without control at the
> hardware level - this requires an intermediate master control chip.
>
> The Audio Injector Octo machine driver, sets both the codec and the BCM2835
> as slaves - the FPGA is master because it has to control both the BCM2835
> and the codec's systems to get reliability. You can see the machine driver
> on github :
> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c
> 
> It has taken month to implement a robust solution to this problem using an
> FPGA and the result is a fantastic (Audio Injector Octo) sound card. I am
> almost certain this hardware technique can be used on all stereo I2S SoC
> chips.

Can you please provide details about the timing / protocol you are
actually using?

so long,

Hias

> 
> 
> Matt
> 

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matthias Reichl]

On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
> 
> 
> On 25/02/17 07:25, Matthias Reichl wrote:
> 
> >I'd put it in another way: it seems to me that the bcm2835 I2S
> >won't reliably sync in DSP mode A.
> >
> >
> 
> What you are explaining here is the nature of the BCM2835 responding to a
> codec master. The hardware setup you are describing is elegantly simple, but
> TOO simple.

Your patch only changes the CPU DAI driver. At this point it doesn't
matter if the codec or an external source is driving the clocks.

If the clock timing adheres to DSP mode A timing and you add code
to the the CPU DAI driver so it can operate in DSP mode A then
that should also work. If not, it's broken.

The (rather brief) publically available bcm2835 datasheet only
mentions I2S mode and my tests lead me to the conclusion that the
bcm2835 is actually expecting I2S timing, not DSP A - at least with
the current code.

> Measurements on a system which uses a codec master don't represent a robust
> implementation of multichannel with the BCM2835. My experiments (and now
> yours) have shown that the system has a bit shift and an uncontrollable
> channel drift.
> 
> Multichannel simply can't be done (on the BCM2835) without control at the
> hardware level - this requires an intermediate master control chip.
>
> The Audio Injector Octo machine driver, sets both the codec and the BCM2835
> as slaves - the FPGA is master because it has to control both the BCM2835
> and the codec's systems to get reliability. You can see the machine driver
> on github :
> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c
> 
> It has taken month to implement a robust solution to this problem using an
> FPGA and the result is a fantastic (Audio Injector Octo) sound card. I am
> almost certain this hardware technique can be used on all stereo I2S SoC
> chips.

Can you please provide details about the timing / protocol you are
actually using?

so long,

Hias

> 
> 
> Matt
> 

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Emmanuel Fusté]

Le 25/02/2017 à 00:02, Matthias Reichl a écrit :
> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>
>> On 25/02/17 07:25, Matthias Reichl wrote:
>>
>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>> won't reliably sync in DSP mode A.
>>>
>>>
>> What you are explaining here is the nature of the BCM2835 responding to a
>> codec master. The hardware setup you are describing is elegantly simple, but
>> TOO simple.
> Your patch only changes the CPU DAI driver. At this point it doesn't
> matter if the codec or an external source is driving the clocks.
>
> If the clock timing adheres to DSP mode A timing and you add code
> to the the CPU DAI driver so it can operate in DSP mode A then
> that should also work. If not, it's broken.
>
> The (rather brief) publically available bcm2835 datasheet only
> mentions I2S mode and my tests lead me to the conclusion that the
> bcm2835 is actually expecting I2S timing, not DSP A - at least with
> the current code.
After reading this long thread and gluing all info together, my guess is 
that the "real" thing he's trying to do is to introduce a new DPS mode 
which is nothing more than a multichannel capable I2S mode or a "DSP I" 
mode : DSP A with data_delay 1.
As it is basically an I2S (2 channel) encapsulation of multichannel 
stream, the real question is alsa plugin or new bastardized DSP mode ?

>
>> Measurements on a system which uses a codec master don't represent a robust
>> implementation of multichannel with the BCM2835. My experiments (and now
>> yours) have shown that the system has a bit shift and an uncontrollable
>> channel drift.
>>
>> Multichannel simply can't be done (on the BCM2835) without control at the
>> hardware level - this requires an intermediate master control chip.
>>
>> The Audio Injector Octo machine driver, sets both the codec and the BCM2835
>> as slaves - the FPGA is master because it has to control both the BCM2835
>> and the codec's systems to get reliability. You can see the machine driver
>> on github :
>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c
>>
>> It has taken month to implement a robust solution to this problem using an
>> FPGA and the result is a fantastic (Audio Injector Octo) sound card. I am
>> almost certain this hardware technique can be used on all stereo I2S SoC
>> chips.
> Can you please provide details about the timing / protocol you are
> actually using?
I suspect that the FPGA act as an I2S <=> DSP A translator, taking care 
of data_delay difference translation relative to the falling edge of LRCLK.
It could translate the duration of the one BCLK duration of LRCLK DSP A 
pulse to standard I2S LRCLK even if most codecs works with I2S fs LRCLK 
timing as the sync point is the falling edge.
As the FPGA has to deals with the clocks of the two sides, it's more 
simple if it's the master.
If I'm right, the real used mode from the soc point of view is I2S and 
all of this is valid for all I2S IP, there is nothing BMC2835 specific 
here. It is not the right place to do the mod.

Emmanuel.

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Emmanuel Fusté]

Le 25/02/2017 ? 00:02, Matthias Reichl a ?crit :
> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>
>> On 25/02/17 07:25, Matthias Reichl wrote:
>>
>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>> won't reliably sync in DSP mode A.
>>>
>>>
>> What you are explaining here is the nature of the BCM2835 responding to a
>> codec master. The hardware setup you are describing is elegantly simple, but
>> TOO simple.
> Your patch only changes the CPU DAI driver. At this point it doesn't
> matter if the codec or an external source is driving the clocks.
>
> If the clock timing adheres to DSP mode A timing and you add code
> to the the CPU DAI driver so it can operate in DSP mode A then
> that should also work. If not, it's broken.
>
> The (rather brief) publically available bcm2835 datasheet only
> mentions I2S mode and my tests lead me to the conclusion that the
> bcm2835 is actually expecting I2S timing, not DSP A - at least with
> the current code.
After reading this long thread and gluing all info together, my guess is 
that the "real" thing he's trying to do is to introduce a new DPS mode 
which is nothing more than a multichannel capable I2S mode or a "DSP I" 
mode : DSP A with data_delay 1.
As it is basically an I2S (2 channel) encapsulation of multichannel 
stream, the real question is alsa plugin or new bastardized DSP mode ?

>
>> Measurements on a system which uses a codec master don't represent a robust
>> implementation of multichannel with the BCM2835. My experiments (and now
>> yours) have shown that the system has a bit shift and an uncontrollable
>> channel drift.
>>
>> Multichannel simply can't be done (on the BCM2835) without control at the
>> hardware level - this requires an intermediate master control chip.
>>
>> The Audio Injector Octo machine driver, sets both the codec and the BCM2835
>> as slaves - the FPGA is master because it has to control both the BCM2835
>> and the codec's systems to get reliability. You can see the machine driver
>> on github :
>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c
>>
>> It has taken month to implement a robust solution to this problem using an
>> FPGA and the result is a fantastic (Audio Injector Octo) sound card. I am
>> almost certain this hardware technique can be used on all stereo I2S SoC
>> chips.
> Can you please provide details about the timing / protocol you are
> actually using?
I suspect that the FPGA act as an I2S <=> DSP A translator, taking care 
of data_delay difference translation relative to the falling edge of LRCLK.
It could translate the duration of the one BCLK duration of LRCLK DSP A 
pulse to standard I2S LRCLK even if most codecs works with I2S fs LRCLK 
timing as the sync point is the falling edge.
As the FPGA has to deals with the clocks of the two sides, it's more 
simple if it's the master.
If I'm right, the real used mode from the soc point of view is I2S and 
all of this is valid for all I2S IP, there is nothing BMC2835 specific 
here. It is not the right place to do the mod.

Emmanuel.

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 11:08, Emmanuel Fusté wrote:
> Le 25/02/2017 à 00:02, Matthias Reichl a écrit :
>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>
>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>
>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>> won't reliably sync in DSP mode A.
>>>>
>>>>
>>> What you are explaining here is the nature of the BCM2835 responding 
>>> to a
>>> codec master. The hardware setup you are describing is elegantly 
>>> simple, but
>>> TOO simple.
>> Your patch only changes the CPU DAI driver. At this point it doesn't
>> matter if the codec or an external source is driving the clocks.
>>
>> If the clock timing adheres to DSP mode A timing and you add code
>> to the the CPU DAI driver so it can operate in DSP mode A then
>> that should also work. If not, it's broken.
>>
>> The (rather brief) publically available bcm2835 datasheet only
>> mentions I2S mode and my tests lead me to the conclusion that the
>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>> the current code.
> After reading this long thread and gluing all info together, my guess 
> is that the "real" thing he's trying to do is to introduce a new DPS 
> mode which is nothing more than a multichannel capable I2S mode or a 
> "DSP I" mode : DSP A with data_delay 1.
> As it is basically an I2S (2 channel) encapsulation of multichannel 
> stream, the real question is alsa plugin or new bastardized DSP mode ?
>

I am not sure if creating a new DSP mode is necesary ?

Having a conflict over DSP mode is a "red herring". Below we talk about 
clocking and timing mater, it is likely that that is where the real 
problem is.

>>
>>> Measurements on a system which uses a codec master don't represent a 
>>> robust
>>> implementation of multichannel with the BCM2835. My experiments (and 
>>> now
>>> yours) have shown that the system has a bit shift and an uncontrollable
>>> channel drift.
>>>
>>> Multichannel simply can't be done (on the BCM2835) without control 
>>> at the
>>> hardware level - this requires an intermediate master control chip.
>>>
>>> The Audio Injector Octo machine driver, sets both the codec and the 
>>> BCM2835
>>> as slaves - the FPGA is master because it has to control both the 
>>> BCM2835
>>> and the codec's systems to get reliability. You can see the machine 
>>> driver
>>> on github :
>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>
>>>
>>> It has taken month to implement a robust solution to this problem 
>>> using an
>>> FPGA and the result is a fantastic (Audio Injector Octo) sound card. 
>>> I am
>>> almost certain this hardware technique can be used on all stereo I2S 
>>> SoC
>>> chips.
>> Can you please provide details about the timing / protocol you are
>> actually using?
> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
> care of data_delay difference translation relative to the falling edge 
> of LRCLK.
> It could translate the duration of the one BCLK duration of LRCLK DSP 
> A pulse to standard I2S LRCLK even if most codecs works with I2S fs 
> LRCLK timing as the sync point is the falling edge.
> As the FPGA has to deals with the clocks of the two sides, it's more 
> simple if it's the master.
> If I'm right, the real used mode from the soc point of view is I2S and 
> all of this is valid for all I2S IP, there is nothing BMC2835 specific 
> here. It is not the right place to do the mod.
>


That is right, the functionality doesn't exist in ALSA to have neither 
the codec nor the SoC as master.
  As I have mentioned before, this is really window dressing for the 
lack of functionality currently in ALSA.

Consider that the same problem exists for sample rate converter chips, 
it isn't only the Audio Injector Octo's use case.

I would like to change focus to the missing piece of the puzzle where an 
FPGA or ASIC is bit and frame master. For example a new definition :
#define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
master */

Now with this new mode, it will force all codecs and SoC drivers to fail 
EINVAL unless they have been manually set up to handle an ASIC master.

This also solves the problem Mattias brought up where choosing DSP mode 
and codec master causes this terrible channel swapping and bit offset. 
Now we fail if in DSP mode with a codec master, or succeed in DSP mode 
with an ASIC master :

Example for the bcm2835_is2.c :

     /* In DSP mode, ensure that an ASIC is master */
     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
                                           == SND_SOC_DAIFMT_DSP_A)
         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
                                        != SND_SOC_DAIFMT_ABM_AFM )
             return -EINVAL;
         else
             return snd_pcm_hw_constraint_single(substream->runtime,
SNDRV_PCM_HW_PARAM_CHANNELS, 8);


Matt

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 11:08, Emmanuel Fust? wrote:
> Le 25/02/2017 ? 00:02, Matthias Reichl a ?crit :
>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>
>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>
>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>> won't reliably sync in DSP mode A.
>>>>
>>>>
>>> What you are explaining here is the nature of the BCM2835 responding 
>>> to a
>>> codec master. The hardware setup you are describing is elegantly 
>>> simple, but
>>> TOO simple.
>> Your patch only changes the CPU DAI driver. At this point it doesn't
>> matter if the codec or an external source is driving the clocks.
>>
>> If the clock timing adheres to DSP mode A timing and you add code
>> to the the CPU DAI driver so it can operate in DSP mode A then
>> that should also work. If not, it's broken.
>>
>> The (rather brief) publically available bcm2835 datasheet only
>> mentions I2S mode and my tests lead me to the conclusion that the
>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>> the current code.
> After reading this long thread and gluing all info together, my guess 
> is that the "real" thing he's trying to do is to introduce a new DPS 
> mode which is nothing more than a multichannel capable I2S mode or a 
> "DSP I" mode : DSP A with data_delay 1.
> As it is basically an I2S (2 channel) encapsulation of multichannel 
> stream, the real question is alsa plugin or new bastardized DSP mode ?
>

I am not sure if creating a new DSP mode is necesary ?

Having a conflict over DSP mode is a "red herring". Below we talk about 
clocking and timing mater, it is likely that that is where the real 
problem is.

>>
>>> Measurements on a system which uses a codec master don't represent a 
>>> robust
>>> implementation of multichannel with the BCM2835. My experiments (and 
>>> now
>>> yours) have shown that the system has a bit shift and an uncontrollable
>>> channel drift.
>>>
>>> Multichannel simply can't be done (on the BCM2835) without control 
>>> at the
>>> hardware level - this requires an intermediate master control chip.
>>>
>>> The Audio Injector Octo machine driver, sets both the codec and the 
>>> BCM2835
>>> as slaves - the FPGA is master because it has to control both the 
>>> BCM2835
>>> and the codec's systems to get reliability. You can see the machine 
>>> driver
>>> on github :
>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>
>>>
>>> It has taken month to implement a robust solution to this problem 
>>> using an
>>> FPGA and the result is a fantastic (Audio Injector Octo) sound card. 
>>> I am
>>> almost certain this hardware technique can be used on all stereo I2S 
>>> SoC
>>> chips.
>> Can you please provide details about the timing / protocol you are
>> actually using?
> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
> care of data_delay difference translation relative to the falling edge 
> of LRCLK.
> It could translate the duration of the one BCLK duration of LRCLK DSP 
> A pulse to standard I2S LRCLK even if most codecs works with I2S fs 
> LRCLK timing as the sync point is the falling edge.
> As the FPGA has to deals with the clocks of the two sides, it's more 
> simple if it's the master.
> If I'm right, the real used mode from the soc point of view is I2S and 
> all of this is valid for all I2S IP, there is nothing BMC2835 specific 
> here. It is not the right place to do the mod.
>


That is right, the functionality doesn't exist in ALSA to have neither 
the codec nor the SoC as master.
  As I have mentioned before, this is really window dressing for the 
lack of functionality currently in ALSA.

Consider that the same problem exists for sample rate converter chips, 
it isn't only the Audio Injector Octo's use case.

I would like to change focus to the missing piece of the puzzle where an 
FPGA or ASIC is bit and frame master. For example a new definition :
#define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
master */

Now with this new mode, it will force all codecs and SoC drivers to fail 
EINVAL unless they have been manually set up to handle an ASIC master.

This also solves the problem Mattias brought up where choosing DSP mode 
and codec master causes this terrible channel swapping and bit offset. 
Now we fail if in DSP mode with a codec master, or succeed in DSP mode 
with an ASIC master :

Example for the bcm2835_is2.c :

     /* In DSP mode, ensure that an ASIC is master */
     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
                                           == SND_SOC_DAIFMT_DSP_A)
         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
                                        != SND_SOC_DAIFMT_ABM_AFM )
             return -EINVAL;
         else
             return snd_pcm_hw_constraint_single(substream->runtime,
SNDRV_PCM_HW_PARAM_CHANNELS, 8);


Matt

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 13:38, Matt Flax wrote:
>
>
> On 25/02/17 11:08, Emmanuel Fusté wrote:
>> Le 25/02/2017 à 00:02, Matthias Reichl a écrit :
>>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>>
>>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>>
>>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>>> won't reliably sync in DSP mode A.
>>>>>
>>>>>
>>>> What you are explaining here is the nature of the BCM2835 
>>>> responding to a
>>>> codec master. The hardware setup you are describing is elegantly 
>>>> simple, but
>>>> TOO simple.
>>> Your patch only changes the CPU DAI driver. At this point it doesn't
>>> matter if the codec or an external source is driving the clocks.
>>>
>>> If the clock timing adheres to DSP mode A timing and you add code
>>> to the the CPU DAI driver so it can operate in DSP mode A then
>>> that should also work. If not, it's broken.
>>>
>>> The (rather brief) publically available bcm2835 datasheet only
>>> mentions I2S mode and my tests lead me to the conclusion that the
>>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>>> the current code.
>> After reading this long thread and gluing all info together, my guess 
>> is that the "real" thing he's trying to do is to introduce a new DPS 
>> mode which is nothing more than a multichannel capable I2S mode or a 
>> "DSP I" mode : DSP A with data_delay 1.
>> As it is basically an I2S (2 channel) encapsulation of multichannel 
>> stream, the real question is alsa plugin or new bastardized DSP mode ?
>>
>
> I am not sure if creating a new DSP mode is necesary ?
>
> Having a conflict over DSP mode is a "red herring". Below we talk 
> about clocking and timing mater, it is likely that that is where the 
> real problem is.
>
>>>
>>>> Measurements on a system which uses a codec master don't represent 
>>>> a robust
>>>> implementation of multichannel with the BCM2835. My experiments 
>>>> (and now
>>>> yours) have shown that the system has a bit shift and an 
>>>> uncontrollable
>>>> channel drift.
>>>>
>>>> Multichannel simply can't be done (on the BCM2835) without control 
>>>> at the
>>>> hardware level - this requires an intermediate master control chip.
>>>>
>>>> The Audio Injector Octo machine driver, sets both the codec and the 
>>>> BCM2835
>>>> as slaves - the FPGA is master because it has to control both the 
>>>> BCM2835
>>>> and the codec's systems to get reliability. You can see the machine 
>>>> driver
>>>> on github :
>>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>>
>>>>
>>>> It has taken month to implement a robust solution to this problem 
>>>> using an
>>>> FPGA and the result is a fantastic (Audio Injector Octo) sound 
>>>> card. I am
>>>> almost certain this hardware technique can be used on all stereo 
>>>> I2S SoC
>>>> chips.
>>> Can you please provide details about the timing / protocol you are
>>> actually using?
>> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
>> care of data_delay difference translation relative to the falling 
>> edge of LRCLK.
>> It could translate the duration of the one BCLK duration of LRCLK DSP 
>> A pulse to standard I2S LRCLK even if most codecs works with I2S fs 
>> LRCLK timing as the sync point is the falling edge.
>> As the FPGA has to deals with the clocks of the two sides, it's more 
>> simple if it's the master.
>> If I'm right, the real used mode from the soc point of view is I2S 
>> and all of this is valid for all I2S IP, there is nothing BMC2835 
>> specific here. It is not the right place to do the mod.
>>
>
>
> That is right, the functionality doesn't exist in ALSA to have neither 
> the codec nor the SoC as master.
>  As I have mentioned before, this is really window dressing for the 
> lack of functionality currently in ALSA.
>
> Consider that the same problem exists for sample rate converter chips, 
> it isn't only the Audio Injector Octo's use case.
>
> I would like to change focus to the missing piece of the puzzle where 
> an FPGA or ASIC is bit and frame master. For example a new definition :
> #define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
> master */
>
> Now with this new mode, it will force all codecs and SoC drivers to 
> fail EINVAL unless they have been manually set up to handle an ASIC 
> master.
>
> This also solves the problem Mattias brought up where choosing DSP 
> mode and codec master causes this terrible channel swapping and bit 
> offset. Now we fail if in DSP mode with a codec master, or succeed in 
> DSP mode with an ASIC master :
>
> Example for the bcm2835_is2.c :
>
>     /* In DSP mode, ensure that an ASIC is master */
>     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
>                                           == SND_SOC_DAIFMT_DSP_A)
>         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
>                                        != SND_SOC_DAIFMT_ABM_AFM )
>             return -EINVAL;
>         else
>             return snd_pcm_hw_constraint_single(substream->runtime,
> SNDRV_PCM_HW_PARAM_CHANNELS, 8);
>
I am sending a patch set which implements this concept to the list.

Matt

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 13:38, Matt Flax wrote:
>
>
> On 25/02/17 11:08, Emmanuel Fust? wrote:
>> Le 25/02/2017 ? 00:02, Matthias Reichl a ?crit :
>>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>>
>>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>>
>>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>>> won't reliably sync in DSP mode A.
>>>>>
>>>>>
>>>> What you are explaining here is the nature of the BCM2835 
>>>> responding to a
>>>> codec master. The hardware setup you are describing is elegantly 
>>>> simple, but
>>>> TOO simple.
>>> Your patch only changes the CPU DAI driver. At this point it doesn't
>>> matter if the codec or an external source is driving the clocks.
>>>
>>> If the clock timing adheres to DSP mode A timing and you add code
>>> to the the CPU DAI driver so it can operate in DSP mode A then
>>> that should also work. If not, it's broken.
>>>
>>> The (rather brief) publically available bcm2835 datasheet only
>>> mentions I2S mode and my tests lead me to the conclusion that the
>>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>>> the current code.
>> After reading this long thread and gluing all info together, my guess 
>> is that the "real" thing he's trying to do is to introduce a new DPS 
>> mode which is nothing more than a multichannel capable I2S mode or a 
>> "DSP I" mode : DSP A with data_delay 1.
>> As it is basically an I2S (2 channel) encapsulation of multichannel 
>> stream, the real question is alsa plugin or new bastardized DSP mode ?
>>
>
> I am not sure if creating a new DSP mode is necesary ?
>
> Having a conflict over DSP mode is a "red herring". Below we talk 
> about clocking and timing mater, it is likely that that is where the 
> real problem is.
>
>>>
>>>> Measurements on a system which uses a codec master don't represent 
>>>> a robust
>>>> implementation of multichannel with the BCM2835. My experiments 
>>>> (and now
>>>> yours) have shown that the system has a bit shift and an 
>>>> uncontrollable
>>>> channel drift.
>>>>
>>>> Multichannel simply can't be done (on the BCM2835) without control 
>>>> at the
>>>> hardware level - this requires an intermediate master control chip.
>>>>
>>>> The Audio Injector Octo machine driver, sets both the codec and the 
>>>> BCM2835
>>>> as slaves - the FPGA is master because it has to control both the 
>>>> BCM2835
>>>> and the codec's systems to get reliability. You can see the machine 
>>>> driver
>>>> on github :
>>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>>
>>>>
>>>> It has taken month to implement a robust solution to this problem 
>>>> using an
>>>> FPGA and the result is a fantastic (Audio Injector Octo) sound 
>>>> card. I am
>>>> almost certain this hardware technique can be used on all stereo 
>>>> I2S SoC
>>>> chips.
>>> Can you please provide details about the timing / protocol you are
>>> actually using?
>> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
>> care of data_delay difference translation relative to the falling 
>> edge of LRCLK.
>> It could translate the duration of the one BCLK duration of LRCLK DSP 
>> A pulse to standard I2S LRCLK even if most codecs works with I2S fs 
>> LRCLK timing as the sync point is the falling edge.
>> As the FPGA has to deals with the clocks of the two sides, it's more 
>> simple if it's the master.
>> If I'm right, the real used mode from the soc point of view is I2S 
>> and all of this is valid for all I2S IP, there is nothing BMC2835 
>> specific here. It is not the right place to do the mod.
>>
>
>
> That is right, the functionality doesn't exist in ALSA to have neither 
> the codec nor the SoC as master.
>  As I have mentioned before, this is really window dressing for the 
> lack of functionality currently in ALSA.
>
> Consider that the same problem exists for sample rate converter chips, 
> it isn't only the Audio Injector Octo's use case.
>
> I would like to change focus to the missing piece of the puzzle where 
> an FPGA or ASIC is bit and frame master. For example a new definition :
> #define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
> master */
>
> Now with this new mode, it will force all codecs and SoC drivers to 
> fail EINVAL unless they have been manually set up to handle an ASIC 
> master.
>
> This also solves the problem Mattias brought up where choosing DSP 
> mode and codec master causes this terrible channel swapping and bit 
> offset. Now we fail if in DSP mode with a codec master, or succeed in 
> DSP mode with an ASIC master :
>
> Example for the bcm2835_is2.c :
>
>     /* In DSP mode, ensure that an ASIC is master */
>     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
>                                           == SND_SOC_DAIFMT_DSP_A)
>         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
>                                        != SND_SOC_DAIFMT_ABM_AFM )
>             return -EINVAL;
>         else
>             return snd_pcm_hw_constraint_single(substream->runtime,
> SNDRV_PCM_HW_PARAM_CHANNELS, 8);
>
I am sending a patch set which implements this concept to the list.

Matt

Re: [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 16:04, Matt Flax wrote:
>
>
> On 25/02/17 13:38, Matt Flax wrote:
>>
>>
>> On 25/02/17 11:08, Emmanuel Fusté wrote:
>>> Le 25/02/2017 à 00:02, Matthias Reichl a écrit :
>>>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>>>
>>>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>>>
>>>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>>>> won't reliably sync in DSP mode A.
>>>>>>
>>>>>>
>>>>> What you are explaining here is the nature of the BCM2835 
>>>>> responding to a
>>>>> codec master. The hardware setup you are describing is elegantly 
>>>>> simple, but
>>>>> TOO simple.
>>>> Your patch only changes the CPU DAI driver. At this point it doesn't
>>>> matter if the codec or an external source is driving the clocks.
>>>>
>>>> If the clock timing adheres to DSP mode A timing and you add code
>>>> to the the CPU DAI driver so it can operate in DSP mode A then
>>>> that should also work. If not, it's broken.
>>>>
>>>> The (rather brief) publically available bcm2835 datasheet only
>>>> mentions I2S mode and my tests lead me to the conclusion that the
>>>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>>>> the current code.
>>> After reading this long thread and gluing all info together, my 
>>> guess is that the "real" thing he's trying to do is to introduce a 
>>> new DPS mode which is nothing more than a multichannel capable I2S 
>>> mode or a "DSP I" mode : DSP A with data_delay 1.
>>> As it is basically an I2S (2 channel) encapsulation of multichannel 
>>> stream, the real question is alsa plugin or new bastardized DSP mode ?
>>>
>>
>> I am not sure if creating a new DSP mode is necesary ?
>>
>> Having a conflict over DSP mode is a "red herring". Below we talk 
>> about clocking and timing mater, it is likely that that is where the 
>> real problem is.
>>
>>>>
>>>>> Measurements on a system which uses a codec master don't represent 
>>>>> a robust
>>>>> implementation of multichannel with the BCM2835. My experiments 
>>>>> (and now
>>>>> yours) have shown that the system has a bit shift and an 
>>>>> uncontrollable
>>>>> channel drift.
>>>>>
>>>>> Multichannel simply can't be done (on the BCM2835) without control 
>>>>> at the
>>>>> hardware level - this requires an intermediate master control chip.
>>>>>
>>>>> The Audio Injector Octo machine driver, sets both the codec and 
>>>>> the BCM2835
>>>>> as slaves - the FPGA is master because it has to control both the 
>>>>> BCM2835
>>>>> and the codec's systems to get reliability. You can see the 
>>>>> machine driver
>>>>> on github :
>>>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>>>
>>>>>
>>>>> It has taken month to implement a robust solution to this problem 
>>>>> using an
>>>>> FPGA and the result is a fantastic (Audio Injector Octo) sound 
>>>>> card. I am
>>>>> almost certain this hardware technique can be used on all stereo 
>>>>> I2S SoC
>>>>> chips.
>>>> Can you please provide details about the timing / protocol you are
>>>> actually using?
>>> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
>>> care of data_delay difference translation relative to the falling 
>>> edge of LRCLK.
>>> It could translate the duration of the one BCLK duration of LRCLK 
>>> DSP A pulse to standard I2S LRCLK even if most codecs works with I2S 
>>> fs LRCLK timing as the sync point is the falling edge.
>>> As the FPGA has to deals with the clocks of the two sides, it's more 
>>> simple if it's the master.
>>> If I'm right, the real used mode from the soc point of view is I2S 
>>> and all of this is valid for all I2S IP, there is nothing BMC2835 
>>> specific here. It is not the right place to do the mod.
>>>
>>
>>
>> That is right, the functionality doesn't exist in ALSA to have 
>> neither the codec nor the SoC as master.
>>  As I have mentioned before, this is really window dressing for the 
>> lack of functionality currently in ALSA.
>>
>> Consider that the same problem exists for sample rate converter 
>> chips, it isn't only the Audio Injector Octo's use case.
>>
>> I would like to change focus to the missing piece of the puzzle where 
>> an FPGA or ASIC is bit and frame master. For example a new definition :
>> #define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
>> master */
>>
>> Now with this new mode, it will force all codecs and SoC drivers to 
>> fail EINVAL unless they have been manually set up to handle an ASIC 
>> master.
>>
>> This also solves the problem Mattias brought up where choosing DSP 
>> mode and codec master causes this terrible channel swapping and bit 
>> offset. Now we fail if in DSP mode with a codec master, or succeed in 
>> DSP mode with an ASIC master :
>>
>> Example for the bcm2835_is2.c :
>>
>>     /* In DSP mode, ensure that an ASIC is master */
>>     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
>>                                           == SND_SOC_DAIFMT_DSP_A)
>>         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
>>                                        != SND_SOC_DAIFMT_ABM_AFM )
>>             return -EINVAL;
>>         else
>>             return snd_pcm_hw_constraint_single(substream->runtime,
>> SNDRV_PCM_HW_PARAM_CHANNELS, 8);
>>
> I am sending a patch set which implements this concept to the list.
>

Hey Matthias, I had an idea which may work with your setup, which 
doesn't include the FPGA.

Firstly, add a DSP mode B to the bcm2835_i2s.c driver. Set its delay to 
0. Now we have (in the bcm2835_i2s.c) DSP mode A as delay of 1 and DSP 
mode B as a delay of 0. Set your codec to DSP mode A, the CPU (bcm2835) 
to DSP mode B. This solves the bit shift issue you mentioned before.

Secondly, to try to get rid of your random channels swapping, can you 
please check and if necessary change the DMA setup in the dts (or 
elsewhere if necessary) so that it will DMA in multiples of 8 ? Don't 
know but a hunch suggests that this may fix things ... which would be 
amazing.

Matt

[alsa-devel] [PATCH v8] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Matt Flax]

On 25/02/17 16:04, Matt Flax wrote:
>
>
> On 25/02/17 13:38, Matt Flax wrote:
>>
>>
>> On 25/02/17 11:08, Emmanuel Fust? wrote:
>>> Le 25/02/2017 ? 00:02, Matthias Reichl a ?crit :
>>>> On Sat, Feb 25, 2017 at 08:30:03AM +1100, Matt Flax wrote:
>>>>>
>>>>> On 25/02/17 07:25, Matthias Reichl wrote:
>>>>>
>>>>>> I'd put it in another way: it seems to me that the bcm2835 I2S
>>>>>> won't reliably sync in DSP mode A.
>>>>>>
>>>>>>
>>>>> What you are explaining here is the nature of the BCM2835 
>>>>> responding to a
>>>>> codec master. The hardware setup you are describing is elegantly 
>>>>> simple, but
>>>>> TOO simple.
>>>> Your patch only changes the CPU DAI driver. At this point it doesn't
>>>> matter if the codec or an external source is driving the clocks.
>>>>
>>>> If the clock timing adheres to DSP mode A timing and you add code
>>>> to the the CPU DAI driver so it can operate in DSP mode A then
>>>> that should also work. If not, it's broken.
>>>>
>>>> The (rather brief) publically available bcm2835 datasheet only
>>>> mentions I2S mode and my tests lead me to the conclusion that the
>>>> bcm2835 is actually expecting I2S timing, not DSP A - at least with
>>>> the current code.
>>> After reading this long thread and gluing all info together, my 
>>> guess is that the "real" thing he's trying to do is to introduce a 
>>> new DPS mode which is nothing more than a multichannel capable I2S 
>>> mode or a "DSP I" mode : DSP A with data_delay 1.
>>> As it is basically an I2S (2 channel) encapsulation of multichannel 
>>> stream, the real question is alsa plugin or new bastardized DSP mode ?
>>>
>>
>> I am not sure if creating a new DSP mode is necesary ?
>>
>> Having a conflict over DSP mode is a "red herring". Below we talk 
>> about clocking and timing mater, it is likely that that is where the 
>> real problem is.
>>
>>>>
>>>>> Measurements on a system which uses a codec master don't represent 
>>>>> a robust
>>>>> implementation of multichannel with the BCM2835. My experiments 
>>>>> (and now
>>>>> yours) have shown that the system has a bit shift and an 
>>>>> uncontrollable
>>>>> channel drift.
>>>>>
>>>>> Multichannel simply can't be done (on the BCM2835) without control 
>>>>> at the
>>>>> hardware level - this requires an intermediate master control chip.
>>>>>
>>>>> The Audio Injector Octo machine driver, sets both the codec and 
>>>>> the BCM2835
>>>>> as slaves - the FPGA is master because it has to control both the 
>>>>> BCM2835
>>>>> and the codec's systems to get reliability. You can see the 
>>>>> machine driver
>>>>> on github :
>>>>> https://github.com/flatmax/linux/blob/rpi-4.4.y/sound/soc/bcm/audioinjector-octo-soundcard.c 
>>>>>
>>>>>
>>>>> It has taken month to implement a robust solution to this problem 
>>>>> using an
>>>>> FPGA and the result is a fantastic (Audio Injector Octo) sound 
>>>>> card. I am
>>>>> almost certain this hardware technique can be used on all stereo 
>>>>> I2S SoC
>>>>> chips.
>>>> Can you please provide details about the timing / protocol you are
>>>> actually using?
>>> I suspect that the FPGA act as an I2S <=> DSP A translator, taking 
>>> care of data_delay difference translation relative to the falling 
>>> edge of LRCLK.
>>> It could translate the duration of the one BCLK duration of LRCLK 
>>> DSP A pulse to standard I2S LRCLK even if most codecs works with I2S 
>>> fs LRCLK timing as the sync point is the falling edge.
>>> As the FPGA has to deals with the clocks of the two sides, it's more 
>>> simple if it's the master.
>>> If I'm right, the real used mode from the soc point of view is I2S 
>>> and all of this is valid for all I2S IP, there is nothing BMC2835 
>>> specific here. It is not the right place to do the mod.
>>>
>>
>>
>> That is right, the functionality doesn't exist in ALSA to have 
>> neither the codec nor the SoC as master.
>>  As I have mentioned before, this is really window dressing for the 
>> lack of functionality currently in ALSA.
>>
>> Consider that the same problem exists for sample rate converter 
>> chips, it isn't only the Audio Injector Octo's use case.
>>
>> I would like to change focus to the missing piece of the puzzle where 
>> an FPGA or ASIC is bit and frame master. For example a new definition :
>> #define SND_SOC_DAIFMT_ABM_AFM          (5 << 12) /* ASIC clk & FRM 
>> master */
>>
>> Now with this new mode, it will force all codecs and SoC drivers to 
>> fail EINVAL unless they have been manually set up to handle an ASIC 
>> master.
>>
>> This also solves the problem Mattias brought up where choosing DSP 
>> mode and codec master causes this terrible channel swapping and bit 
>> offset. Now we fail if in DSP mode with a codec master, or succeed in 
>> DSP mode with an ASIC master :
>>
>> Example for the bcm2835_is2.c :
>>
>>     /* In DSP mode, ensure that an ASIC is master */
>>     if ((dev->fmt & SND_SOC_DAIFMT_FORMAT_MASK)
>>                                           == SND_SOC_DAIFMT_DSP_A)
>>         if ((dev->fmt & SND_SOC_DAIFMT_MASTER_MASK)
>>                                        != SND_SOC_DAIFMT_ABM_AFM )
>>             return -EINVAL;
>>         else
>>             return snd_pcm_hw_constraint_single(substream->runtime,
>> SNDRV_PCM_HW_PARAM_CHANNELS, 8);
>>
> I am sending a patch set which implements this concept to the list.
>

Hey Matthias, I had an idea which may work with your setup, which 
doesn't include the FPGA.

Firstly, add a DSP mode B to the bcm2835_i2s.c driver. Set its delay to 
0. Now we have (in the bcm2835_i2s.c) DSP mode A as delay of 1 and DSP 
mode B as a delay of 0. Set your codec to DSP mode A, the CPU (bcm2835) 
to DSP mode B. This solves the bit shift issue you mentioned before.

Secondly, to try to get rid of your random channels swapping, can you 
please check and if necessary change the DMA setup in the dts (or 
elsewhere if necessary) so that it will DMA in multiples of 8 ? Don't 
know but a hunch suggests that this may fix things ... which would be 
amazing.

Matt