[PATCH v2] ASoC: bdw-rt5650: remove 3-channel capture support
Implement a constrain to exclude 3-channel capture since only 2 and 4 channel capture are supported on the platform.
Signed-off-by: Brent Lu brent.lu@intel.com --- sound/soc/intel/boards/bdw-rt5650.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index af2f502..eedbdad 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -83,6 +83,36 @@ static struct snd_soc_jack_pin mic_jack_pin = { .mask = SND_JACK_MICROPHONE, };
+static const unsigned int channels[] = { + 2, 4, +}; + +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, + * 2 or 4 channel capture + */ + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) + snd_pcm_hw_constraint_list(runtime, 0, + SNDRV_PCM_HW_PARAM_CHANNELS, + &constraints_channels); + + return 0; +} + +static const struct snd_soc_ops bdw_rt5650_fe_ops = { + .startup = bdw_fe_startup, +}; + static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) { @@ -234,6 +264,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .name = "System PCM", .stream_name = "System Playback", .dynamic = 1, + .ops = &bdw_rt5650_fe_ops, #if !IS_ENABLED(CONFIG_SND_SOC_SOF_BROADWELL) .init = bdw_rt5650_rtd_init, #endif
On 4/13/20 3:28 AM, Brent Lu wrote:
Implement a constrain to exclude 3-channel capture since only 2 and 4 channel capture are supported on the platform.
That looks like an error caught by the ALSA conformance tool?
What are the odds that we have a similar issue with the other broadwell drivers which don't have a constraint on the number of channels either on their 'System PCM' dailink?
Thanks -Pierre
Signed-off-by: Brent Lu brent.lu@intel.com
sound/soc/intel/boards/bdw-rt5650.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/sound/soc/intel/boards/bdw-rt5650.c b/sound/soc/intel/boards/bdw-rt5650.c index af2f502..eedbdad 100644 --- a/sound/soc/intel/boards/bdw-rt5650.c +++ b/sound/soc/intel/boards/bdw-rt5650.c @@ -83,6 +83,36 @@ static struct snd_soc_jack_pin mic_jack_pin = { .mask = SND_JACK_MICROPHONE, };
+static const unsigned int channels[] = {
- 2, 4,
+};
+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,* 2 or 4 channel capture*/- if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
snd_pcm_hw_constraint_list(runtime, 0,SNDRV_PCM_HW_PARAM_CHANNELS,&constraints_channels);- return 0;
+}
+static const struct snd_soc_ops bdw_rt5650_fe_ops = {
- .startup = bdw_fe_startup,
+};
- static int broadwell_ssp0_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params) {
@@ -234,6 +264,7 @@ static struct snd_soc_dai_link bdw_rt5650_dais[] = { .name = "System PCM", .stream_name = "System Playback", .dynamic = 1,
.ops = &bdw_rt5650_fe_ops,
That looks like an error caught by the ALSA conformance tool?
What are the odds that we have a similar issue with the other broadwell drivers which don't have a constraint on the number of channels either on their 'System PCM' dailink?
Thanks -Pierre
Yes. That's why I am sending patch for this old BDW platform...
So far as I know only 'buddy' supports 2/4-channel recording while other BDW Chrome products should support stereo recording only. Therefore, this defect should only be triggered by the ALSA conformance tool.
I am think about implementing the constraint in FE DAI's startup() callback instead of DAI Link's callback. Since the channels_max is 4 for the capture stream, ALSA conformance tool will always test 3-channel recording on any platforms using this driver. Does it make sense to you?
sound/soc/intel/haswell/sst-haswell-pcm.c: static struct snd_soc_dai_driver hsw_dais[] = { { .name = "System Pin", .id = HSW_PCM_DAI_ID_SYSTEM, .playback = { .stream_name = "System Playback", .channels_min = 2, .channels_max = 2, .rates = SNDRV_PCM_RATE_48000, .formats = SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE, }, .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
On 4/13/20 9:29 AM, Lu, Brent wrote:
That looks like an error caught by the ALSA conformance tool?
What are the odds that we have a similar issue with the other broadwell drivers which don't have a constraint on the number of channels either on their 'System PCM' dailink?
Thanks -Pierre
Yes. That's why I am sending patch for this old BDW platform...
So far as I know only 'buddy' supports 2/4-channel recording while other BDW Chrome products should support stereo recording only. Therefore, this defect should only be triggered by the ALSA conformance tool.
I am think about implementing the constraint in FE DAI's startup() callback instead of DAI Link's callback. Since the channels_max is 4 for the capture stream, ALSA conformance tool will always test 3-channel recording on any platforms using this driver. Does it make sense to you?
Looking back at previous threads, you indicated that the number of channels supported in propagated from BE to FE, so a similar patch to add 2ch constraints for bwd-rt5677 was dropped ("ASoC: bdw-rt5677: channel constraint support")
Actually I am not sure it was dropped since later you submitted another patch ("ASoC: bdw-rt5677: enable runtime channel merge"), and my feedback was that it seemed simpler to add constraints on all machine drivers.
And now this patch only addresses bdw-rt5650.c but with the initial solution suggested for bdw-rt5677.c
It seems like a generic problem on all Broadwell devices so let's solve with one a single patchset.
Shouldn't we just add the 2ch constraints for broadwell.c and bdw-rt5677.c, and the 2 or 4ch constraint for bdw-rt5650.c? Would this work for you?
Thanks -Pierre
It seems like a generic problem on all Broadwell devices so let's solve with one a single patchset.
Shouldn't we just add the 2ch constraints for broadwell.c and bdw-rt5677.c, and the 2 or 4ch constraint for bdw-rt5650.c? Would this work for you?
Thanks -Pierre
Hi Pierre,
Are you saying submitting a new patch to add constraints to all three broadwell.c, bdw-rt5650.c, and bdw-rt5677.c?
Regards, Brent
On 2020-04-17 03:32, Lu, Brent wrote:
It seems like a generic problem on all Broadwell devices so let's solve with one a single patchset.
Shouldn't we just add the 2ch constraints for broadwell.c and bdw-rt5677.c, and the 2 or 4ch constraint for bdw-rt5650.c? Would this work for you?
Thanks -Pierre
Hi Pierre,
Are you saying submitting a new patch to add constraints to all three broadwell.c, bdw-rt5650.c, and bdw-rt5677.c?
Regards, Brent
What Pierre suggested is that you submit a series of patches instead - one for each of BDW machine boards. If the same problem exists on all of them, there is no reason to left other boards unattended.
Czarek
On 4/17/20 2:59 AM, Cezary Rojewski wrote:
On 2020-04-17 03:32, Lu, Brent wrote:
It seems like a generic problem on all Broadwell devices so let's solve with one a single patchset.
Shouldn't we just add the 2ch constraints for broadwell.c and bdw-rt5677.c, and the 2 or 4ch constraint for bdw-rt5650.c? Would this work for you?
Thanks -Pierre
Hi Pierre,
Are you saying submitting a new patch to add constraints to all three broadwell.c, bdw-rt5650.c, and bdw-rt5677.c?
Regards, Brent
What Pierre suggested is that you submit a series of patches instead - one for each of BDW machine boards. If the same problem exists on all of them, there is no reason to left other boards unattended.
Yes, a series of 3 patches with the same code pattern, the only variation being 4 ch allowed for bdw-rt5650.c in addition to the default 2ch for all 3 boards.
Thanks -Pierre
participants (4)
-
Brent Lu -
Cezary Rojewski -
Lu, Brent
-
Pierre-Louis Bossart