[PATCH 0/2] kbl_da7219_max9357a machine changes for wov and MST
From: Vamshi Krishna Gopal vamshi.krishna.gopal@intel.com
Hello,
This patch series about creating dailink for Wake on voice functionality and also adding MST route changes
Mac Chiang (1): ASoc: Intel: board: add BE DAI link for WoV
Vamshi Krishna Gopal (1): ASoC: Intel: kbl: Add MST route change to kbl machine drivers
sound/soc/intel/boards/kbl_da7219_max98357a.c | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-)
From: Vamshi Krishna Gopal vamshi.krishna.gopal@intel.com
To support MST hdmi audio, modify the current routes to be based on port in da7219_max98357a machine.
Signed-off-by: Vamshi Krishna Gopal vamshi.krishna.gopal@intel.com --- sound/soc/intel/boards/kbl_da7219_max98357a.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index dc3d897ad280..1d6b2855874d 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -91,7 +91,9 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_SPK("Spk", NULL), SND_SOC_DAPM_MIC("SoC DMIC", NULL), SND_SOC_DAPM_SPK("DP", NULL), - SND_SOC_DAPM_SPK("HDMI", NULL), + SND_SOC_DAPM_SPK("HDMI1", NULL), + SND_SOC_DAPM_SPK("HDMI2", NULL), + SND_SOC_DAPM_SPK("HDMI3", NULL), SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, platform_clock_control, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD), @@ -108,9 +110,6 @@ static const struct snd_soc_dapm_route kabylake_map[] = { { "MIC", NULL, "Headset Mic" }, { "DMic", NULL, "SoC DMIC" },
- { "HDMI", NULL, "hif5 Output" }, - { "DP", NULL, "hif6 Output" }, - /* CODEC BE connections */ { "HiFi Playback", NULL, "ssp0 Tx" }, { "ssp0 Tx", NULL, "codec0_out" },
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index dc3d897ad280..1d6b2855874d 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -91,7 +91,9 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_SPK("Spk", NULL), SND_SOC_DAPM_MIC("SoC DMIC", NULL), SND_SOC_DAPM_SPK("DP", NULL),
- SND_SOC_DAPM_SPK("HDMI", NULL),
- SND_SOC_DAPM_SPK("HDMI1", NULL),
- SND_SOC_DAPM_SPK("HDMI2", NULL),
- SND_SOC_DAPM_SPK("HDMI3", NULL),
that seems consistent with other BXT/KBL machine drivers, but...
SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, platform_clock_control, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD), @@ -108,9 +110,6 @@ static const struct snd_soc_dapm_route kabylake_map[] = { { "MIC", NULL, "Headset Mic" }, { "DMic", NULL, "SoC DMIC" },
- { "HDMI", NULL, "hif5 Output" },
- { "DP", NULL, "hif6 Output" },
... this doesn't:
other machine drivers use this:
{"HDMI1", NULL, "hif5-0 Output"}, {"HDMI2", NULL, "hif6-0 Output"}, {"HDMI2", NULL, "hif7-0 Output"},
And if you start changing HDMI support, you should also fix the other machine drivers that used the same pattern, e.g.
kbl_da7219_max98927.c\0129: { "HDMI", NULL, "hif5 Output" }, kbl_rt5663_max98927.c\0214: { "HDMI", NULL, "hif5 Output" },
/* CODEC BE connections */ { "HiFi Playback", NULL, "ssp0 Tx" }, { "ssp0 Tx", NULL, "codec0_out" },
On 3/25/2021 11:32 PM, Vamshi Krishna wrote:
From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Thursday, March 25, 2021 12:04 AM To: Gopal, Vamshi Krishna vamshi.krishna.gopal@intel.com; alsa- devel@alsa-project.org Cc: N, Harshapriya harshapriya.n@intel.com; broonie@kernel.org; M R, Sathya Prakash sathya.prakash.m.r@intel.com; biernacki@google.com; Bossart, Pierre-louis pierre-louis.bossart@intel.com Subject: Re: [PATCH 1/2] ASoC: Intel: kbl: Add MST route change to kbl machine drivers
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index dc3d897ad280..1d6b2855874d 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -91,7 +91,9 @@ static const struct snd_soc_dapm_widget
kabylake_widgets[] = {
SND_SOC_DAPM_SPK("Spk", NULL), SND_SOC_DAPM_MIC("SoC DMIC", NULL), SND_SOC_DAPM_SPK("DP", NULL),
- SND_SOC_DAPM_SPK("HDMI", NULL),
- SND_SOC_DAPM_SPK("HDMI1", NULL),
- SND_SOC_DAPM_SPK("HDMI2", NULL),
- SND_SOC_DAPM_SPK("HDMI3", NULL),
that seems consistent with other BXT/KBL machine drivers, but...
SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, platform_clock_control, SND_SOC_DAPM_PRE_PMU
|
SND_SOC_DAPM_POST_PMD),@@ -108,9 +110,6 @@ static const struct snd_soc_dapm_route
kabylake_map[] = {
{ "MIC", NULL, "Headset Mic" }, { "DMic", NULL, "SoC DMIC" },
- { "HDMI", NULL, "hif5 Output" },
- { "DP", NULL, "hif6 Output" },
... this doesn't:
other machine drivers use this:
{"HDMI1", NULL, "hif5-0 Output"}, {"HDMI2", NULL, "hif6-0 Output"}, {"HDMI2", NULL, "hif7-0 Output"},
Hello Pierre, Thanks for reviewing the patch. I looked through the change you suggested in bxt_da7219_max98357a.c machine, but I noticed hif6-0 Output and hif7-0 Output are having same port HDMI2, This looks not correct.
And if you start changing HDMI support, you should also fix the other machine drivers that used the same pattern, e.g.
kbl_da7219_max98927.c\0129: { "HDMI", NULL, "hif5 Output" }, kbl_rt5663_max98927.c\0214: { "HDMI", NULL, "hif5 Output" },
Submitted a v2 patch which follows same pattern across KBL machine drivers.
/* CODEC BE connections */ { "HiFi Playback", NULL, "ssp0 Tx" }, { "ssp0 Tx", NULL, "codec0_out" },
- { "HDMI", NULL, "hif5 Output" },
- { "DP", NULL, "hif6 Output" },
... this doesn't:
other machine drivers use this:
{"HDMI1", NULL, "hif5-0 Output"}, {"HDMI2", NULL, "hif6-0 Output"}, {"HDMI2", NULL, "hif7-0 Output"},
Hello Pierre, Thanks for reviewing the patch. I looked through the change you suggested in bxt_da7219_max98357a.c machine, but I noticed hif6-0 Output and hif7-0 Output are having same port HDMI2, This looks not correct.
D'oh! You're right, this makes no sense to me either. I see 4 occurrences in the code.
bxt_da7219_max98357a.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_da7219_max98357a.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_da7219_max98357a.c: {"HDMI2", NULL, "hif7-0 Output"},
bxt_rt298.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif7-0 Output"},
bxt_rt298.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif7-0 Output"},
glk_rt5682_max98357a.c: { "HDMI1", NULL, "hif5-0 Output" }, glk_rt5682_max98357a.c: { "HDMI2", NULL, "hif6-0 Output" }, glk_rt5682_max98357a.c: { "HDMI2", NULL, "hif7-0 Output" },
Harsha and team, the HDMI2 duplicates seem like recurring copy/paste mistakes, can you double check what the intent was? If this is indeed unintentional, we probably need a patch per file with a Fixes tag to have this applied to the stable kernel.
Thanks!
- { "HDMI", NULL, "hif5 Output" },
- { "DP", NULL, "hif6 Output" },
... this doesn't:
other machine drivers use this:
{"HDMI1", NULL, "hif5-0 Output"}, {"HDMI2", NULL, "hif6-0 Output"}, {"HDMI2", NULL, "hif7-0 Output"},
Hello Pierre, Thanks for reviewing the patch. I looked through the change you suggested in bxt_da7219_max98357a.c
machine, but I noticed hif6-0 Output and hif7-0 Output are having same port HDMI2, This looks not correct.
D'oh! You're right, this makes no sense to me either. I see 4 occurrences in the code.
[Gopal, Vamshi Krishna] Hello Pierre, I will send the patches for bxt and GLK drivers separately after doing the validation. I have submitted the v2 patch with fix for KBL drivers, can we merge the KBL patches first ?
bxt_da7219_max98357a.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_da7219_max98357a.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_da7219_max98357a.c: {"HDMI2", NULL, "hif7-0 Output"},
bxt_rt298.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif7-0 Output"},
bxt_rt298.c: {"HDMI1", NULL, "hif5-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif6-0 Output"}, bxt_rt298.c: {"HDMI2", NULL, "hif7-0 Output"},
glk_rt5682_max98357a.c: { "HDMI1", NULL, "hif5-0 Output" }, glk_rt5682_max98357a.c: { "HDMI2", NULL, "hif6-0 Output" }, glk_rt5682_max98357a.c: { "HDMI2", NULL, "hif7-0 Output" },
Harsha and team, the HDMI2 duplicates seem like recurring copy/paste mistakes, can you double check what the intent was? If this is indeed unintentional, we probably need a patch per file with a Fixes tag to have this applied to the stable kernel.
Thanks!
From: Mac Chiang mac.chiang@intel.com
create dai link in kbl_da7219_max98357a driver for wake on voice functionality.
changes picked from broonie's tree commit 0c7941a63a0f ("ASoC: Intel: Skylake: Use refcap device for mono recording") commit 2154be362c90 ("ASoc: Intel: boards: Add WOV as sink for nau88l25_ssm4567 machine")
Signed-off-by: Mac Chiang mac.chiang@intel.com Signed-off-by: Vamshi Krishna Gopal vamshi.krishna.gopal@intel.com Tested-by: Kaiyen Chang kaiyen.chang@intel.corp-partner.google.com Tested-by: luke yang luke_yang@compal.corp-partner.google.com Tested-by: Grace Kao grace.kao@intel.com Tested-by: Kaiyen Chang kaiyen.chang@intel.com Reviewed-by: Cheng-Yi Chiang cychiang@chromium.org --- sound/soc/intel/boards/kbl_da7219_max98357a.c | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index 1d6b2855874d..c9d83eebf4a8 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -44,6 +44,7 @@ struct kbl_codec_private { enum { KBL_DPCM_AUDIO_PB = 0, KBL_DPCM_AUDIO_CP, + KBL_DPCM_AUDIO_REF_CP, KBL_DPCM_AUDIO_DMIC_CP, KBL_DPCM_AUDIO_HDMI1_PB, KBL_DPCM_AUDIO_HDMI2_PB, @@ -335,12 +336,36 @@ static struct snd_soc_ops kabylake_dmic_ops = { .startup = kabylake_dmic_startup, };
+static const struct snd_pcm_hw_constraint_list constraints_refcap = { + .count = ARRAY_SIZE(ch_mono), + .list = ch_mono, +}; + +static int kabylake_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); +} + +static struct snd_soc_ops skylaye_refcap_ops = { + .startup = kabylake_refcap_startup, +}; + SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
+SND_SOC_DAILINK_DEF(reference, + DAILINK_COMP_ARRAY(COMP_CPU("Reference Pin"))); + SND_SOC_DAILINK_DEF(dmic, DAILINK_COMP_ARRAY(COMP_CPU("DMIC Pin")));
@@ -415,6 +440,16 @@ static struct snd_soc_dai_link kabylake_dais[] = { .ops = &kabylake_da7219_fe_ops, SND_SOC_DAILINK_REG(system, dummy, platform), }, + [KBL_DPCM_AUDIO_REF_CP] = { + .name = "Kbl Audio Reference cap", + .stream_name = "Wake on Voice", + .init = NULL, + .dpcm_capture = 1, + .nonatomic = 1, + .dynamic = 1, + .ops = &skylaye_refcap_ops, + SND_SOC_DAILINK_REG(reference, dummy, platform), + }, [KBL_DPCM_AUDIO_DMIC_CP] = { .name = "Kbl Audio DMIC cap", .stream_name = "dmiccap",
On 3/24/21 12:52 PM, vamshi.krishna.gopal@intel.com wrote:
From: Mac Chiang mac.chiang@intel.com
create dai link in kbl_da7219_max98357a driver for wake on voice functionality.
changes picked from broonie's tree commit 0c7941a63a0f ("ASoC: Intel: Skylake: Use refcap device for mono recording") commit 2154be362c90 ("ASoc: Intel: boards: Add WOV as sink for nau88l25_ssm4567 machine")
Signed-off-by: Mac Chiang mac.chiang@intel.com Signed-off-by: Vamshi Krishna Gopal vamshi.krishna.gopal@intel.com Tested-by: Kaiyen Chang kaiyen.chang@intel.corp-partner.google.com Tested-by: luke yang luke_yang@compal.corp-partner.google.com Tested-by: Grace Kao grace.kao@intel.com Tested-by: Kaiyen Chang kaiyen.chang@intel.com Reviewed-by: Cheng-Yi Chiang cychiang@chromium.org
sound/soc/intel/boards/kbl_da7219_max98357a.c | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index 1d6b2855874d..c9d83eebf4a8 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -44,6 +44,7 @@ struct kbl_codec_private { enum { KBL_DPCM_AUDIO_PB = 0, KBL_DPCM_AUDIO_CP,
- KBL_DPCM_AUDIO_REF_CP, KBL_DPCM_AUDIO_DMIC_CP, KBL_DPCM_AUDIO_HDMI1_PB, KBL_DPCM_AUDIO_HDMI2_PB,
@@ -335,12 +336,36 @@ static struct snd_soc_ops kabylake_dmic_ops = { .startup = kabylake_dmic_startup, };
+static const struct snd_pcm_hw_constraint_list constraints_refcap = {
- .count = ARRAY_SIZE(ch_mono),
- .list = ch_mono,
+};
+static int kabylake_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);+}
+static struct snd_soc_ops skylaye_refcap_ops = {
- .startup = kabylake_refcap_startup,
+};
SND_SOC_DAILINK_DEF(dummy, DAILINK_COMP_ARRAY(COMP_DUMMY()));
SND_SOC_DAILINK_DEF(system, DAILINK_COMP_ARRAY(COMP_CPU("System Pin")));
+SND_SOC_DAILINK_DEF(reference,
- DAILINK_COMP_ARRAY(COMP_CPU("Reference Pin")));
- SND_SOC_DAILINK_DEF(dmic, DAILINK_COMP_ARRAY(COMP_CPU("DMIC Pin")));
@@ -415,6 +440,16 @@ static struct snd_soc_dai_link kabylake_dais[] = { .ops = &kabylake_da7219_fe_ops, SND_SOC_DAILINK_REG(system, dummy, platform), },
- [KBL_DPCM_AUDIO_REF_CP] = {
.name = "Kbl Audio Reference cap",.stream_name = "Wake on Voice",
Does anyone have a clear definition of what "REF_CP" and "reference" mean? it's not echo reference since there is a separate entry for this, so not sure what the term means.
Half of the SKL/KBL drivers seem to use this FE for "Wake on Voice" and the other half use it for 'Refcap', this doesn't look very consistent.
.init = NULL,.dpcm_capture = 1,.nonatomic = 1,.dynamic = 1,.ops = &skylaye_refcap_ops,SND_SOC_DAILINK_REG(reference, dummy, platform),- }, [KBL_DPCM_AUDIO_DMIC_CP] = { .name = "Kbl Audio DMIC cap", .stream_name = "dmiccap",
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next] [also build test ERROR on sound/for-next v5.12-rc4 next-20210324] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/vamshi-krishna-gopal-intel-com/kbl_... base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next config: x86_64-randconfig-a015-20210325 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project a4fb88669cd98db6fef7dcac88e3ec425d40c00d) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/53b070ce8badeefb7fde6432ed4a5078cefe... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review vamshi-krishna-gopal-intel-com/kbl_da7219_max9357a-machine-changes-for-wov-and-MST/20210325-015625 git checkout 53b070ce8badeefb7fde6432ed4a5078cefe28e3 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
sound/soc/intel/boards/kbl_da7219_max98357a.c:340:22: error: use of undeclared identifier 'ch_mono'
.count = ARRAY_SIZE(ch_mono), ^
sound/soc/intel/boards/kbl_da7219_max98357a.c:340:22: error: use of undeclared identifier 'ch_mono' sound/soc/intel/boards/kbl_da7219_max98357a.c:340:22: error: use of undeclared identifier 'ch_mono'
sound/soc/intel/boards/kbl_da7219_max98357a.c:341:11: error: use of undeclared identifier 'ch_mono' .list = ch_mono, ^
sound/soc/intel/boards/kbl_da7219_max98357a.c:353:7: error: use of undeclared identifier 'constraints_16000'; did you mean 'constraints_rates'?
&constraints_16000); ^~~~~~~~~~~~~~~~~ constraints_rates sound/soc/intel/boards/kbl_da7219_max98357a.c:251:48: note: 'constraints_rates' declared here static const struct snd_pcm_hw_constraint_list constraints_rates = { ^ 5 errors generated.
vim +/ch_mono +340 sound/soc/intel/boards/kbl_da7219_max98357a.c
338 339 static const struct snd_pcm_hw_constraint_list constraints_refcap = {
340 .count = ARRAY_SIZE(ch_mono),
341 .list = ch_mono, 342 }; 343 344 static int kabylake_refcap_startup(struct snd_pcm_substream *substream) 345 { 346 substream->runtime->hw.channels_max = 1; 347 snd_pcm_hw_constraint_list(substream->runtime, 0, 348 SNDRV_PCM_HW_PARAM_CHANNELS, 349 &constraints_refcap); 350 351 return snd_pcm_hw_constraint_list(substream->runtime, 0, 352 SNDRV_PCM_HW_PARAM_RATE,
353 &constraints_16000);
354 } 355
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
participants (4)
-
Gopal, Vamshi Krishna -
kernel test robot -
Pierre-Louis Bossart -
vamshi.krishna.gopal＠intel.com