[alsa-devel] [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 at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel