diff mbox series

[1/2] ASoC: Intel: kbl: Add MST route change to kbl machine drivers

Message ID 20210324175200.44922-2-vamshi.krishna.gopal@intel.com (mailing list archive)
State Superseded
Headers show
Series kbl_da7219_max9357a machine changes for wov and MST | expand

Commit Message

Gopal, Vamshi Krishna March 24, 2021, 5:51 p.m. UTC 
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(-)

Comments

Pierre-Louis Bossart March 24, 2021, 6:34 p.m. UTC | #1 
> 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" },
>
Gopal, Vamshi Krishna March 25, 2021, 6:07 p.m. UTC | #2 
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" },
> >
Pierre-Louis Bossart March 25, 2021, 10:05 p.m. UTC | #3 
>>>
>>> -	{ "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!
Gopal, Vamshi Krishna March 31, 2021, 6:49 p.m. UTC | #4 
> >>>
> >>> -	{ "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!
diff mbox series

Patch

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" },