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