Hi, Bossart,

Thanks for the quick review.

-----Original Message----- From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] Sent: Wednesday, August 1, 2018 3:52 PM To: Zhi, Yong yong.zhi@intel.com; broonie@kernel.org Cc: alsa-devel@alsa-project.org; N, Harshapriya harshapriya.n@intel.com; M, Naveen naveen.m@intel.com; Tc, Jenny jenny.tc@intel.com; Vinod Koul vkoul@kernel.org Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: bxt: Use refcap device for mono recording

On 8/1/18 10:03 AM, Yong Zhi wrote:

...

The refcap capture device supports mono recording only, this patch adds the channel constraints, also change stream_name to "Wake on

Voice".

the part on the channel constraints is legit, but I wonder if the renaming to "Wake-on-voice" is required or meaningful. IIRC the reference signal is just a downmixed/downsampled version whatever is being played to be used for echo cancellation, I don't see a link with Wake-on-voice?

The cras audio server defined HOTWORD_DEV as "Wake on Voice", but we can also use UCM file to tell the server that the Refcap is the HOTWORD_DEV, so the renaming is not straightly required, I can remove it in v2.

https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/...

...

Signed-off-by: Yong Zhi yong.zhi@intel.com

sound/soc/intel/boards/bxt_da7219_max98357a.c | 16

+++++++++++++++-

...

1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c index be6e4b4..097a089 100644 --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c @@ -319,6 +319,15 @@ static const unsigned int rates_16000[] = { 16000, };

+static const unsigned int ch_mono[] = {

• 1,

+};

+static const struct snd_pcm_hw_constraint_list constraints_refcap = {

• .count = ARRAY_SIZE(ch_mono),
• .list = ch_mono,

+};

• static const struct snd_pcm_hw_constraint_list constraints_16000 = { .count = ARRAY_SIZE(rates_16000), .list = rates_16000,

@@ -326,6 +335,11 @@ static const struct snd_pcm_hw_constraint_list constraints_16000 = {

static int broxton_refcap_startup(struct snd_pcm_substream

*substream)

...

{

• substream->runtime->hw.channels_max = 1;
• snd_pcm_hw_constraint_list(substream->runtime, 0,

• 		   SNDRV_PCM_HW_PARAM_CHANNELS,
  

• 		   &constraints_refcap);
  

• return snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_16000);

@@ -372,7 +386,7 @@ static struct snd_soc_dai_link broxton_dais[] = { [BXT_DPCM_AUDIO_REF_CP] = { .name = "Bxt Audio Reference cap",

• .stream_name = "Refcap",
  

• .stream_name = "Wake on Voice",
  

  .cpu_dai_name = "Reference Pin", .codec_name = "snd-soc-dummy", .codec_dai_name = "snd-soc-dummy-dai",

On 8/2/18 4:21 PM, Yong Zhi wrote:

The refcap capture device supports mono recording only, this patch adds the channel constraints.

Signed-off-by: Yong Zhi yong.zhi@intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

---

Changes since v1:

• Remove renaming of "Refcap".
• Add Reviewed-by tag.

LGTM, thanks.

sound/soc/intel/boards/bxt_da7219_max98357a.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)

diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c index be6e4b40bf03..6f052fc8d1e2 100644 --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c @@ -324,8 +324,22 @@ static const struct snd_pcm_hw_constraint_list constraints_16000 = { .list = rates_16000, };

+static const unsigned int ch_mono[] = {

• 1,

+};

+static const struct snd_pcm_hw_constraint_list constraints_refcap = {

• .count = ARRAY_SIZE(ch_mono),
• .list = ch_mono,

+};

• static int broxton_refcap_startup(struct snd_pcm_substream *substream) {
• substream->runtime->hw.channels_max = 1;
• snd_pcm_hw_constraint_list(substream->runtime, 0,

• 		   SNDRV_PCM_HW_PARAM_CHANNELS,
  

• 		   &constraints_refcap);
  

• return snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &constraints_16000);