[PATCH] ASoC: bdw-rt5677: channel constraint support

[PATCH] ASoC: bdw-rt5677: channel constraint support

[Brent Lu]

BDW boards using this machine driver supports only stereo capture and
playback. Implement a constraint to enforce it.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/intel/boards/bdw-rt5677.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
index 4a4d335..a312b55 100644
--- a/sound/soc/intel/boards/bdw-rt5677.c
+++ b/sound/soc/intel/boards/bdw-rt5677.c
@@ -22,6 +22,8 @@
 
 #include "../../codecs/rt5677.h"
 
+#define DUAL_CHANNEL 2
+
 struct bdw_rt5677_priv {
 	struct gpio_desc *gpio_hp_en;
 	struct snd_soc_component *component;
@@ -245,6 +247,36 @@ static int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
+static const unsigned int channels[] = {
+	DUAL_CHANNEL,
+};
+
+static const struct snd_pcm_hw_constraint_list constraints_channels = {
+	.count = ARRAY_SIZE(channels),
+	.list = channels,
+	.mask = 0,
+};
+
+static int bdw_fe_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	/*
+	 * On this platform for PCM device we support,
+	 * stereo
+	 */
+
+	runtime->hw.channels_max = DUAL_CHANNEL;
+	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+					   &constraints_channels);
+
+	return 0;
+}
+
+static const struct snd_soc_ops bdw_rt5677_fe_ops = {
+	.startup = bdw_fe_startup,
+};
+
 /* broadwell digital audio interface glue - connects codec <--> CPU */
 SND_SOC_DAILINK_DEF(dummy,
 	DAILINK_COMP_ARRAY(COMP_DUMMY()));
@@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
 		},
 		.dpcm_capture = 1,
 		.dpcm_playback = 1,
+		.ops = &bdw_rt5677_fe_ops,
 		SND_SOC_DAILINK_REG(fe, dummy, platform),
 	},
 
-- 
2.7.4

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Pierre-Louis Bossart]

On 9/5/19 8:24 PM, Brent Lu wrote:
> BDW boards using this machine driver supports only stereo capture and
> playback. Implement a constraint to enforce it.

Humm, can you clarify what problem/error this patch fixes?

There are already constraints on the hsw_dais[] where the channels are 
stereo only.

Thanks
-Pierre

> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>   sound/soc/intel/boards/bdw-rt5677.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
> index 4a4d335..a312b55 100644
> --- a/sound/soc/intel/boards/bdw-rt5677.c
> +++ b/sound/soc/intel/boards/bdw-rt5677.c
> @@ -22,6 +22,8 @@
>   
>   #include "../../codecs/rt5677.h"
>   
> +#define DUAL_CHANNEL 2
> +
>   struct bdw_rt5677_priv {
>   	struct gpio_desc *gpio_hp_en;
>   	struct snd_soc_component *component;
> @@ -245,6 +247,36 @@ static int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
>   	return 0;
>   }
>   
> +static const unsigned int channels[] = {
> +	DUAL_CHANNEL,
> +};
> +
> +static const struct snd_pcm_hw_constraint_list constraints_channels = {
> +	.count = ARRAY_SIZE(channels),
> +	.list = channels,
> +	.mask = 0,
> +};
> +
> +static int bdw_fe_startup(struct snd_pcm_substream *substream)
> +{
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	/*
> +	 * On this platform for PCM device we support,
> +	 * stereo
> +	 */
> +
> +	runtime->hw.channels_max = DUAL_CHANNEL;
> +	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
> +					   &constraints_channels);
> +
> +	return 0;
> +}
> +
> +static const struct snd_soc_ops bdw_rt5677_fe_ops = {
> +	.startup = bdw_fe_startup,
> +};
> +
>   /* broadwell digital audio interface glue - connects codec <--> CPU */
>   SND_SOC_DAILINK_DEF(dummy,
>   	DAILINK_COMP_ARRAY(COMP_DUMMY()));
> @@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
>   		},
>   		.dpcm_capture = 1,
>   		.dpcm_playback = 1,
> +		.ops = &bdw_rt5677_fe_ops,
>   		SND_SOC_DAILINK_REG(fe, dummy, platform),
>   	},
>   
> 

RE: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Lu, Brent]

Hi Pierre

We are working on a backport 3.14 branch for Chrome projects based on BDW platform. In the branch 4-channel capture is supported on some platforms but not all. So we need to add a constraint in the machine driver for machines don't support this feature.

I checked the for-next branch in the broonie repo. The channels_max of HSW_PCM_DAI_ID_SYSTEM is 4 for capture stream so I think it would have same issue like google's backport tree. I didn't find any constraint for this dai. Would you point out where it is?

		.capture = {
			.stream_name = "Analog Capture",
			.channels_min = 2,
			.channels_max = 4,
			.rates = SNDRV_PCM_RATE_48000,
			.formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE,
		},

Regards,
Brent Lu

-----Original Message-----
From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] 
Sent: Friday, September 6, 2019 10:21 PM
To: Lu, Brent <brent.lu@intel.com>; alsa-devel@alsa-project.org
Cc: Rojewski, Cezary <cezary.rojewski@intel.com>; liam.r.girdwood@linux.intel.com; yang.jie@linux.intel.com; broonie@kernel.org; perex@perex.cz; tiwai@suse.com; kuninori.morimoto.gx@renesas.com; tglx@linutronix.de; linux-kernel@vger.kernel.org
Subject: Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

On 9/5/19 8:24 PM, Brent Lu wrote:
> BDW boards using this machine driver supports only stereo capture and 
> playback. Implement a constraint to enforce it.

Humm, can you clarify what problem/error this patch fixes?

There are already constraints on the hsw_dais[] where the channels are stereo only.

Thanks
-Pierre

> 
> Signed-off-by: Brent Lu <brent.lu@intel.com>
> ---
>   sound/soc/intel/boards/bdw-rt5677.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/sound/soc/intel/boards/bdw-rt5677.c 
> b/sound/soc/intel/boards/bdw-rt5677.c
> index 4a4d335..a312b55 100644
> --- a/sound/soc/intel/boards/bdw-rt5677.c
> +++ b/sound/soc/intel/boards/bdw-rt5677.c
> @@ -22,6 +22,8 @@
>   
>   #include "../../codecs/rt5677.h"
>   
> +#define DUAL_CHANNEL 2
> +
>   struct bdw_rt5677_priv {
>   	struct gpio_desc *gpio_hp_en;
>   	struct snd_soc_component *component; @@ -245,6 +247,36 @@ static 
> int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
>   	return 0;
>   }
>   
> +static const unsigned int channels[] = {
> +	DUAL_CHANNEL,
> +};
> +
> +static const struct snd_pcm_hw_constraint_list constraints_channels = {
> +	.count = ARRAY_SIZE(channels),
> +	.list = channels,
> +	.mask = 0,
> +};
> +
> +static int bdw_fe_startup(struct snd_pcm_substream *substream) {
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	/*
> +	 * On this platform for PCM device we support,
> +	 * stereo
> +	 */
> +
> +	runtime->hw.channels_max = DUAL_CHANNEL;
> +	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
> +					   &constraints_channels);
> +
> +	return 0;
> +}
> +
> +static const struct snd_soc_ops bdw_rt5677_fe_ops = {
> +	.startup = bdw_fe_startup,
> +};
> +
>   /* broadwell digital audio interface glue - connects codec <--> CPU */
>   SND_SOC_DAILINK_DEF(dummy,
>   	DAILINK_COMP_ARRAY(COMP_DUMMY()));
> @@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
>   		},
>   		.dpcm_capture = 1,
>   		.dpcm_playback = 1,
> +		.ops = &bdw_rt5677_fe_ops,
>   		SND_SOC_DAILINK_REG(fe, dummy, platform),
>   	},
>   
> 

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Pierre-Louis Bossart]

Please don't top-post on public mailing lists.

> We are working on a backport 3.14 branch for Chrome projects based on BDW platform. In the branch 4-channel capture is supported on some platforms but not all. So we need to add a constraint in the machine driver for machines don't support this feature.
> 
> I checked the for-next branch in the broonie repo. The channels_max of HSW_PCM_DAI_ID_SYSTEM is 4 for capture stream so I think it would have same issue like google's backport tree. I didn't find any constraint for this dai. Would you point out where it is?
> 
> 		.capture = {
> 			.stream_name = "Analog Capture",
> 			.channels_min = 2,
> 			.channels_max = 4,
> 			.rates = SNDRV_PCM_RATE_48000,
> 			.formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE,
> 		},

ok, I missed this capture case indeed, but when I look at the Chrome 
3.14 code I don't see this constraint being added, and we already have 
an ssp0_fixup function where 2 channels only are used.

	/* The ADSP will covert the FE rate to 48k, stereo */
	rate->min = rate->max = 48000;
	channels->min = channels->max = 2;

I also don't see any case where we support 4 channels in any broadwell 
machine driver?

So again can you point me to an issue or existing backport that requires 
the patch below. Not trying to be obtuse but we should only change older 
platforms when there is evidence that a change is needed.

-Pierre

> 
> Regards,
> Brent Lu
> 
> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> Sent: Friday, September 6, 2019 10:21 PM
> To: Lu, Brent <brent.lu@intel.com>; alsa-devel@alsa-project.org
> Cc: Rojewski, Cezary <cezary.rojewski@intel.com>; liam.r.girdwood@linux.intel.com; yang.jie@linux.intel.com; broonie@kernel.org; perex@perex.cz; tiwai@suse.com; kuninori.morimoto.gx@renesas.com; tglx@linutronix.de; linux-kernel@vger.kernel.org
> Subject: Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support
> 
> On 9/5/19 8:24 PM, Brent Lu wrote:
>> BDW boards using this machine driver supports only stereo capture and
>> playback. Implement a constraint to enforce it.
> 
> Humm, can you clarify what problem/error this patch fixes?
> 
> There are already constraints on the hsw_dais[] where the channels are stereo only.
> 
> Thanks
> -Pierre
> 
>>
>> Signed-off-by: Brent Lu <brent.lu@intel.com>
>> ---
>>    sound/soc/intel/boards/bdw-rt5677.c | 33 +++++++++++++++++++++++++++++++++
>>    1 file changed, 33 insertions(+)
>>
>> diff --git a/sound/soc/intel/boards/bdw-rt5677.c
>> b/sound/soc/intel/boards/bdw-rt5677.c
>> index 4a4d335..a312b55 100644
>> --- a/sound/soc/intel/boards/bdw-rt5677.c
>> +++ b/sound/soc/intel/boards/bdw-rt5677.c
>> @@ -22,6 +22,8 @@
>>    
>>    #include "../../codecs/rt5677.h"
>>    
>> +#define DUAL_CHANNEL 2
>> +
>>    struct bdw_rt5677_priv {
>>    	struct gpio_desc *gpio_hp_en;
>>    	struct snd_soc_component *component; @@ -245,6 +247,36 @@ static
>> int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
>>    	return 0;
>>    }
>>    
>> +static const unsigned int channels[] = {
>> +	DUAL_CHANNEL,
>> +};
>> +
>> +static const struct snd_pcm_hw_constraint_list constraints_channels = {
>> +	.count = ARRAY_SIZE(channels),
>> +	.list = channels,
>> +	.mask = 0,
>> +};
>> +
>> +static int bdw_fe_startup(struct snd_pcm_substream *substream) {
>> +	struct snd_pcm_runtime *runtime = substream->runtime;
>> +
>> +	/*
>> +	 * On this platform for PCM device we support,
>> +	 * stereo
>> +	 */
>> +
>> +	runtime->hw.channels_max = DUAL_CHANNEL;
>> +	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
>> +					   &constraints_channels);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct snd_soc_ops bdw_rt5677_fe_ops = {
>> +	.startup = bdw_fe_startup,
>> +};
>> +
>>    /* broadwell digital audio interface glue - connects codec <--> CPU */
>>    SND_SOC_DAILINK_DEF(dummy,
>>    	DAILINK_COMP_ARRAY(COMP_DUMMY()));
>> @@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
>>    		},
>>    		.dpcm_capture = 1,
>>    		.dpcm_playback = 1,
>> +		.ops = &bdw_rt5677_fe_ops,
>>    		SND_SOC_DAILINK_REG(fe, dummy, platform),
>>    	},
>>    
>>
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

RE: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Lu, Brent]

> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> Sent: Tuesday, September 10, 2019 1:53 AM
> To: Lu, Brent <brent.lu@intel.com>; alsa-devel@alsa-project.org
> Cc: Rojewski, Cezary <cezary.rojewski@intel.com>;
> kuninori.morimoto.gx@renesas.com; linux-kernel@vger.kernel.org;
> yang.jie@linux.intel.com; tiwai@suse.com; liam.r.girdwood@linux.intel.com;
> broonie@kernel.org; tglx@linutronix.de
> Subject: Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint
> support
> 
> Please don't top-post on public mailing lists.
> 
> > We are working on a backport 3.14 branch for Chrome projects based on
> BDW platform. In the branch 4-channel capture is supported on some
> platforms but not all. So we need to add a constraint in the machine driver
> for machines don't support this feature.
> >
> > I checked the for-next branch in the broonie repo. The channels_max of
> HSW_PCM_DAI_ID_SYSTEM is 4 for capture stream so I think it would have
> same issue like google's backport tree. I didn't find any constraint for this dai.
> Would you point out where it is?
> >
> > 		.capture = {
> > 			.stream_name = "Analog Capture",
> > 			.channels_min = 2,
> > 			.channels_max = 4,
> > 			.rates = SNDRV_PCM_RATE_48000,
> > 			.formats = SNDRV_PCM_FMTBIT_S24_LE |
> SNDRV_PCM_FMTBIT_S16_LE,
> > 		},
> 
> ok, I missed this capture case indeed, but when I look at the Chrome
> 3.14 code I don't see this constraint being added, and we already have an
> ssp0_fixup function where 2 channels only are used.
> 
> 	/* The ADSP will covert the FE rate to 48k, stereo */
> 	rate->min = rate->max = 48000;
> 	channels->min = channels->max = 2;
Yes I noticed it, but the channel max number returned by snd_pcm_hw_params_any() 
is still 4 for the port "Capture PCM" so I add a constraint to the FE dai.

> 
> I also don't see any case where we support 4 channels in any broadwell
> machine driver?
It's the bdw-rt5650.c which only exists in chrome's 3.14 branch supporting Buddy 
project. They submitted the machine driver but not yet merged.

https://patchwork.kernel.org/patch/11050985/

> 
> So again can you point me to an issue or existing backport that requires the
> patch below. Not trying to be obtuse but we should only change older
> platforms when there is evidence that a change is needed.
The story is Chrome has a tool called alsa_conformance_test which runs capture or 
playback against a PCM port with all possible configurations (channel, format, rate) 
then measure if the sample rate is correct. Since the channel max number reported 
is 4, it tests the 4-channel 48K capture and reports the actual sample rate is 24000 
instead of 48000. That's the reason we want to add a constraint in machine driver to 
avoid user space programs trying to do 4 channel recording since this machine does 
not support it in the beginning.


> 
> -Pierre
> 
> >
> > Regards,
> > Brent Lu
> >
> > -----Original Message-----
> > From: Pierre-Louis Bossart
> > [mailto:pierre-louis.bossart@linux.intel.com]
> > Sent: Friday, September 6, 2019 10:21 PM
> > To: Lu, Brent <brent.lu@intel.com>; alsa-devel@alsa-project.org
> > Cc: Rojewski, Cezary <cezary.rojewski@intel.com>;
> > liam.r.girdwood@linux.intel.com; yang.jie@linux.intel.com;
> > broonie@kernel.org; perex@perex.cz; tiwai@suse.com;
> > kuninori.morimoto.gx@renesas.com; tglx@linutronix.de;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint
> > support
> >
> > On 9/5/19 8:24 PM, Brent Lu wrote:
> >> BDW boards using this machine driver supports only stereo capture and
> >> playback. Implement a constraint to enforce it.
> >
> > Humm, can you clarify what problem/error this patch fixes?
> >
> > There are already constraints on the hsw_dais[] where the channels are
> stereo only.
> >
> > Thanks
> > -Pierre
> >
> >>
> >> Signed-off-by: Brent Lu <brent.lu@intel.com>
> >> ---
> >>    sound/soc/intel/boards/bdw-rt5677.c | 33
> +++++++++++++++++++++++++++++++++
> >>    1 file changed, 33 insertions(+)
> >>
> >> diff --git a/sound/soc/intel/boards/bdw-rt5677.c
> >> b/sound/soc/intel/boards/bdw-rt5677.c
> >> index 4a4d335..a312b55 100644
> >> --- a/sound/soc/intel/boards/bdw-rt5677.c
> >> +++ b/sound/soc/intel/boards/bdw-rt5677.c
> >> @@ -22,6 +22,8 @@
> >>
> >>    #include "../../codecs/rt5677.h"
> >>
> >> +#define DUAL_CHANNEL 2
> >> +
> >>    struct bdw_rt5677_priv {
> >>    	struct gpio_desc *gpio_hp_en;
> >>    	struct snd_soc_component *component; @@ -245,6 +247,36 @@
> static
> >> int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
> >>    	return 0;
> >>    }
> >>
> >> +static const unsigned int channels[] = {
> >> +	DUAL_CHANNEL,
> >> +};
> >> +
> >> +static const struct snd_pcm_hw_constraint_list constraints_channels = {
> >> +	.count = ARRAY_SIZE(channels),
> >> +	.list = channels,
> >> +	.mask = 0,
> >> +};
> >> +
> >> +static int bdw_fe_startup(struct snd_pcm_substream *substream) {
> >> +	struct snd_pcm_runtime *runtime = substream->runtime;
> >> +
> >> +	/*
> >> +	 * On this platform for PCM device we support,
> >> +	 * stereo
> >> +	 */
> >> +
> >> +	runtime->hw.channels_max = DUAL_CHANNEL;
> >> +	snd_pcm_hw_constraint_list(runtime, 0,
> SNDRV_PCM_HW_PARAM_CHANNELS,
> >> +					   &constraints_channels);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct snd_soc_ops bdw_rt5677_fe_ops = {
> >> +	.startup = bdw_fe_startup,
> >> +};
> >> +
> >>    /* broadwell digital audio interface glue - connects codec <--> CPU */
> >>    SND_SOC_DAILINK_DEF(dummy,
> >>    	DAILINK_COMP_ARRAY(COMP_DUMMY()));
> >> @@ -273,6 +305,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] =
> {
> >>    		},
> >>    		.dpcm_capture = 1,
> >>    		.dpcm_playback = 1,
> >> +		.ops = &bdw_rt5677_fe_ops,
> >>    		SND_SOC_DAILINK_REG(fe, dummy, platform),
> >>    	},
> >>
> >>
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Pierre-Louis Bossart]

>> I also don't see any case where we support 4 channels in any broadwell
>> machine driver?
> It's the bdw-rt5650.c which only exists in chrome's 3.14 branch supporting Buddy
> project. They submitted the machine driver but not yet merged.
> 
> https://patchwork.kernel.org/patch/11050985/
> 
>>
>> So again can you point me to an issue or existing backport that requires the
>> patch below. Not trying to be obtuse but we should only change older
>> platforms when there is evidence that a change is needed.
> The story is Chrome has a tool called alsa_conformance_test which runs capture or
> playback against a PCM port with all possible configurations (channel, format, rate)
> then measure if the sample rate is correct. Since the channel max number reported
> is 4, it tests the 4-channel 48K capture and reports the actual sample rate is 24000
> instead of 48000. That's the reason we want to add a constraint in machine driver to
> avoid user space programs trying to do 4 channel recording since this machine does
> not support it in the beginning.

ok, that helps get context, thanks for the details.

I would have expected some error to be returned if there's a front-end 
opened with 4 channels and the back-end only supports two. Adding the 
constraint seems like a work-around to avoid dealing with the mismatch 
between FE and BE. I don't understand DPCM enough to suggest an 
alternative though. Ranjani, can you help on this one?

And even if we agree with this solution, it'd be nice to apply it for 
the Broadwell machine driver for consistency.

RE: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Lu, Brent]

> >
> > The story is Chrome has a tool called alsa_conformance_test which runs
> > capture or playback against a PCM port with all possible
> > configurations (channel, format, rate) then measure if the sample rate
> > is correct. Since the channel max number reported is 4, it tests the
> > 4-channel 48K capture and reports the actual sample rate is 24000
> > instead of 48000. That's the reason we want to add a constraint in
> > machine driver to avoid user space programs trying to do 4 channel
> recording since this machine does not support it in the beginning.
> 
> ok, that helps get context, thanks for the details.
> 
> I would have expected some error to be returned if there's a front-end
> opened with 4 channels and the back-end only supports two. Adding the
> constraint seems like a work-around to avoid dealing with the mismatch
> between FE and BE. I don't understand DPCM enough to suggest an
> alternative though. Ranjani, can you help on this one?
> 
> And even if we agree with this solution, it'd be nice to apply it for the
> Broadwell machine driver for consistency.

It's not only the mismatch but also the design limitation. According to the 
information from google, the board (samus) only uses two microphone so 
3 or 4 channel recording are not supported. That's the reason we leverage 
the constraint from other machine driver (like kbl_da7219_max98357a.c) 
to remove the 3 and 4 channel recording option.

The difference after the constraint is implemented is that the 
snd_pcm_hw_params_set_channels() function will return error (Invalid 
argument) when channel number is 3 or 4 so the application knows the 
configuration is not supported.


Regards,
Brent

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Pierre-Louis Bossart]

On 9/12/19 1:00 AM, Lu, Brent wrote:
>>>
>>> The story is Chrome has a tool called alsa_conformance_test which runs
>>> capture or playback against a PCM port with all possible
>>> configurations (channel, format, rate) then measure if the sample rate
>>> is correct. Since the channel max number reported is 4, it tests the
>>> 4-channel 48K capture and reports the actual sample rate is 24000
>>> instead of 48000. That's the reason we want to add a constraint in
>>> machine driver to avoid user space programs trying to do 4 channel
>> recording since this machine does not support it in the beginning.
>>
>> ok, that helps get context, thanks for the details.
>>
>> I would have expected some error to be returned if there's a front-end
>> opened with 4 channels and the back-end only supports two. Adding the
>> constraint seems like a work-around to avoid dealing with the mismatch
>> between FE and BE. I don't understand DPCM enough to suggest an
>> alternative though. Ranjani, can you help on this one?
>>
>> And even if we agree with this solution, it'd be nice to apply it for the
>> Broadwell machine driver for consistency.
> 
> It's not only the mismatch but also the design limitation. According to the
> information from google, the board (samus) only uses two microphone so
> 3 or 4 channel recording are not supported. That's the reason we leverage
> the constraint from other machine driver (like kbl_da7219_max98357a.c)
> to remove the 3 and 4 channel recording option.

The design limitation is already handled by the fact that the SSP 
operates in 2ch mode, so it's a different case from KBL where indeed the 
DMIC-based back-end can support 4 channels.

> 
> The difference after the constraint is implemented is that the
> snd_pcm_hw_params_set_channels() function will return error (Invalid
> argument) when channel number is 3 or 4 so the application knows the
> configuration is not supported.

I get the error, I am just wondering if the fix is at the right 
location. I'll look into it, give me until tomorrow.

RE: [alsa-devel] [PATCH] ASoC: bdw-rt5677: channel constraint support

[Lu, Brent]

> >
> > It's not only the mismatch but also the design limitation. According
> > to the information from google, the board (samus) only uses two
> > microphone so
> > 3 or 4 channel recording are not supported. That's the reason we
> > leverage the constraint from other machine driver (like
> > kbl_da7219_max98357a.c) to remove the 3 and 4 channel recording option.
> 
> The design limitation is already handled by the fact that the SSP operates in
> 2ch mode, so it's a different case from KBL where indeed the DMIC-based
> back-end can support 4 channels.
> 
> >
> > The difference after the constraint is implemented is that the
> > snd_pcm_hw_params_set_channels() function will return error (Invalid
> > argument) when channel number is 3 or 4 so the application knows the
> > configuration is not supported.
> 
> I get the error, I am just wondering if the fix is at the right location. I'll look
> into it, give me until tomorrow.

I think I got your point. I cherry-pick these commits back to v3.14 branch to fix
the FE/BE mismatch without adding constraint in machine driver. Thanks.

b073ed4e ASoC: soc-pcm: DPCM cares BE format
f4c277b8 ASoC: soc-pcm: DPCM cares BE channel constraint
4f2bd18b ASoC: dpcm: extend channel merging to the backend cpu dai
435ffb76 ASoC: dpcm: rework runtime stream merge
baacd8d1 ASoC: dpcm: add rate merge to the BE stream merge