[PATCH] Fixes HP Spectre x360 15-eb1xxx speakers

[Takashi Iwai]

On Tue, 28 Sep 2021 12:36:05 +0200,
Takashi Iwai wrote:
> 
> On Sat, 25 Sep 2021 21:44:26 +0200,
> Davide Baldo wrote:
> > 
> > 
> > First kernel contribution, I only tested this patch myself on a 5.14.7.
> > There is probably some way to fix the remaining issues, but I need
> > some guidance on how I could test the different pinouts since
> > hdajackretask hangs the audio card and I can't test out combinations.
> > In the meantime this commit fix the most serius problem: silent
> > speakers.
> > 
> > In laptop 'HP Spectre x360 Convertible 15-eb1xxx/8811' both front and
> > rear speakers are silent, this patch fixes that by overriding the pin
> > layout and by initializing the amplifier which needs a GPIO pin to be
> > set to 1 then 0, similar to the existing HP Spectre x360 14 model.
> > 
> > This patch also correctly map the mute LED but since there is no
> > microphone on/off switch exposed by the alsa subsystem it never turns
> > on by itself.
> 
> Note that the recent kernel binds the mute and mic-mute LED with the
> leds subsystem, which can be controlled via sysfs, /sys/class/leds/*.
> 
> > There are still known audio issues in this laptop: headset microphone
> > doesn't work, the button to mute/unmute microphone is not yet mapped,
> > the LED of the mute/unmute speakers doesn't seems to be exposed via
> > GPIO and never turns on.
> > 
> > Signed-off-by: Davide Baldo <davide at baldo.me>
> 
> The changes look almost good, but could you fix the following and
> resubmit?
> 
> > ---
> >  sound/pci/hda/patch_realtek.c | 41 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 40 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> > index 70516527ebce..90f845976793 100644
> > --- a/sound/pci/hda/patch_realtek.c
> > +++ b/sound/pci/hda/patch_realtek.c
> > @@ -6414,6 +6414,38 @@ static void alc_fixup_no_int_mic(struct hda_codec *codec,
> >  	}
> >  }
> >  
> > +/* GPIO1 = amplifier on/off
> > + * GPIO3 = mic mute LED
> > + */
> > +static void alc285_fixup_hp_spectre_x360_eb1(struct hda_codec *codec,
> > +					  const struct hda_fixup *fix, int action)
> > +{
> > +	struct alc_spec *spec = codec->spec;
> > +	static const struct hda_pintbl pincfgs[] = {
> > +		{ 0x14, 0x90170110 },  /* front speakers */
> > +		{ }
> > +	};
> > +
> > +	//enable micmute led
> > +	alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
> > +	spec->micmute_led_polarity = 1;
> 
> This line should be moved under HDA_FIXUP_ACT_PRE_PROBE.  It needs to
> be set only once at the beginning.
> 
> > +
> > +	switch (action) {
> > +	case HDA_FIXUP_ACT_PRE_PROBE:
> > +		/* needed for amp of back speakers */
> > +		spec->gpio_mask |= 0x01;
> > +		spec->gpio_dir |= 0x01;
> > +		snd_hda_apply_pincfgs(codec, pincfgs);
> > +		break;
> > +	case HDA_FIXUP_ACT_INIT:
> > +		/* need to toggle GPIO to enable the amp of back speakers */
> > +		alc_update_gpio_data(codec, 0x01, true);
> > +		msleep(100);
> > +		alc_update_gpio_data(codec, 0x01, false);
> > +		break;
> > +	}
> > +}
> > +
> >  static void alc285_fixup_hp_spectre_x360(struct hda_codec *codec,
> >  					  const struct hda_fixup *fix, int action)
> >  {
> > @@ -6534,6 +6566,7 @@ enum {
> >  	ALC269_FIXUP_HP_DOCK_GPIO_MIC1_LED,
> >  	ALC280_FIXUP_HP_9480M,
> >  	ALC245_FIXUP_HP_X360_AMP,
> > +	ALC285_FIXUP_HP_SPECTRE_X360_EB1,
> >  	ALC288_FIXUP_DELL_HEADSET_MODE,
> >  	ALC288_FIXUP_DELL1_MIC_NO_PRESENCE,
> >  	ALC288_FIXUP_DELL_XPS_13,
> > @@ -6658,7 +6691,7 @@ enum {
> >  	ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP,
> >  	ALC623_FIXUP_LENOVO_THINKSTATION_P340,
> >  	ALC255_FIXUP_ACER_HEADPHONE_AND_MIC,
> > -	ALC236_FIXUP_HP_LIMIT_INT_MIC_BOOST,
> > +	ALC236_FIXUP_HP_LIMIT_INT_MIC_BOOST
> 
> This is unnecessary change.  It's fine to keep the comma of the last
> enum entry in Linux kernel coding style.
> 
> 
> >  };
> >  
> >  static const struct hda_fixup alc269_fixups[] = {
> > @@ -8222,6 +8255,10 @@ static const struct hda_fixup alc269_fixups[] = {
> >  		.type = HDA_FIXUP_FUNC,
> >  		.v.func = alc285_fixup_hp_spectre_x360,
> >  	},
> > +	[ALC285_FIXUP_HP_SPECTRE_X360_EB1] = {
> > +		.type = HDA_FIXUP_FUNC,
> > +		.v.func = alc285_fixup_hp_spectre_x360_eb1
> > +	},
> >  	[ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP] = {
> >  		.type = HDA_FIXUP_FUNC,
> >  		.v.func = alc285_fixup_ideapad_s740_coef,
> > @@ -8415,6 +8452,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
> >  	SND_PCI_QUIRK(0x103c, 0x84da, "HP OMEN dc0019-ur", ALC295_FIXUP_HP_OMEN),
> >  	SND_PCI_QUIRK(0x103c, 0x84e7, "HP Pavilion 15", ALC269_FIXUP_HP_MUTE_LED_MIC3),
> >  	SND_PCI_QUIRK(0x103c, 0x8519, "HP Spectre x360 15-df0xxx", ALC285_FIXUP_HP_SPECTRE_X360),
> > +	SND_PCI_QUIRK(0x103c, 0x8811, "HP Spectre x360 15-eb1xxx", ALC285_FIXUP_HP_SPECTRE_X360_EB1),
> >  	SND_PCI_QUIRK(0x103c, 0x861f, "HP Elite Dragonfly G1", ALC285_FIXUP_HP_GPIO_AMP_INIT),
> >  	SND_PCI_QUIRK(0x103c, 0x869d, "HP", ALC236_FIXUP_HP_MUTE_LED),
> >  	SND_PCI_QUIRK(0x103c, 0x86c7, "HP Envy AiO 32", ALC274_FIXUP_HP_ENVY_GPIO),
> 
> Please insert the entry in PCI SSID order.

Last but not least, at the next resubmission, please put maintainers
(e.g. me) to Cc.  This will avoid overlooking your post.


Takashi