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
State New
Headers show
Series [1/2] ASoC: Intel: kbl: Add MST route change to kbl machine drivers | 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

Gopal, Vamshi Krishna March 25, 2021, 6:07 p.m. UTC | #1 
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 | #2 
>>>
>>> -	{ "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 | #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.

> 

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