All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
@ 2021-09-25 19:44 Davide Baldo
  2021-09-28 10:36 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Davide Baldo @ 2021-09-25 19:44 UTC (permalink / raw)
  To: alsa-devel; +Cc: Davide Baldo


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.

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@baldo.me>
---
 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;
+
+	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
 };
 
 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),
@@ -8858,6 +8896,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC245_FIXUP_HP_X360_AMP, .name = "alc245-hp-x360-amp"},
 	{.id = ALC295_FIXUP_HP_OMEN, .name = "alc295-hp-omen"},
 	{.id = ALC285_FIXUP_HP_SPECTRE_X360, .name = "alc285-hp-spectre-x360"},
+	{.id = ALC285_FIXUP_HP_SPECTRE_X360_EB1, .name = "alc285-hp-spectre-x360-eb1"},
 	{.id = ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP, .name = "alc287-ideapad-bass-spk-amp"},
 	{.id = ALC623_FIXUP_LENOVO_THINKSTATION_P340, .name = "alc623-lenovo-thinkstation-p340"},
 	{.id = ALC255_FIXUP_ACER_HEADPHONE_AND_MIC, .name = "alc255-acer-headphone-and-mic"},
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
  2021-09-25 19:44 [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers Davide Baldo
@ 2021-09-28 10:36 ` Takashi Iwai
  2021-09-28 10:37   ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2021-09-28 10:36 UTC (permalink / raw)
  To: Davide Baldo; +Cc: alsa-devel

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@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.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
  2021-09-28 10:36 ` Takashi Iwai
@ 2021-09-28 10:37   ` Takashi Iwai
  2021-09-29 20:42     ` Davide Baldo
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2021-09-28 10:37 UTC (permalink / raw)
  To: Davide Baldo; +Cc: alsa-devel

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@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
  2021-09-28 10:37   ` Takashi Iwai
@ 2021-09-29 20:42     ` Davide Baldo
  2021-09-29 20:50       ` Geraldo Nascimento
  2021-09-30 11:46       ` Takashi Iwai
  0 siblings, 2 replies; 6+ messages in thread
From: Davide Baldo @ 2021-09-29 20:42 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, Davide Baldo

Thank your for your review Takashi, the entry is now correctly
ordered.
I've received some feedback from a user from bugzilla
https://bugzilla.kernel.org/show_bug.cgi?id=213953
To address his issues I've added a variant of the laptop and forced
DAC1 for both 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.

In order to have volume control, both front and rear speakers were
forced to use the DAC1.

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.

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@baldo.me>
---
 sound/pci/hda/patch_realtek.c | 48 ++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 70516527ebce..240f47a61a5a 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6414,6 +6414,44 @@ 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)
+{
+	static const hda_nid_t conn[] = { 0x02 };
+
+	struct alc_spec *spec = codec->spec;
+	static const struct hda_pintbl pincfgs[] = {
+		{ 0x14, 0x90170110 },  /* front/high speakers */
+		{ 0x17, 0x90170130 },  /* back/bass speakers */
+		{ }
+	};
+
+	//enable micmute led
+	alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
+	spec->micmute_led_polarity = 1;
+
+	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);
+		/* share DAC to have unified volume control */
+		snd_hda_override_conn_list(codec, 0x14, ARRAY_SIZE(conn), conn);
+		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
+		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 +6572,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 +6697,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
 };
 
 static const struct hda_fixup alc269_fixups[] = {
@@ -8222,6 +8261,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,
@@ -8442,6 +8485,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x103c, 0x87f7, "HP Spectre x360 14", ALC245_FIXUP_HP_X360_AMP),
 	SND_PCI_QUIRK(0x103c, 0x8805, "HP ProBook 650 G8 Notebook PC", ALC236_FIXUP_HP_GPIO_LED),
 	SND_PCI_QUIRK(0x103c, 0x880d, "HP EliteBook 830 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
+	SND_PCI_QUIRK(0x103c, 0x8811, "HP Spectre x360 15-eb1xxx", ALC285_FIXUP_HP_SPECTRE_X360_EB1),
+	SND_PCI_QUIRK(0x103c, 0x8812, "HP Spectre x360 15-eb1xxx", ALC285_FIXUP_HP_SPECTRE_X360_EB1),
 	SND_PCI_QUIRK(0x103c, 0x8846, "HP EliteBook 850 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
 	SND_PCI_QUIRK(0x103c, 0x8847, "HP EliteBook x360 830 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
 	SND_PCI_QUIRK(0x103c, 0x884b, "HP EliteBook 840 Aero G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
@@ -8858,6 +8903,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC245_FIXUP_HP_X360_AMP, .name = "alc245-hp-x360-amp"},
 	{.id = ALC295_FIXUP_HP_OMEN, .name = "alc295-hp-omen"},
 	{.id = ALC285_FIXUP_HP_SPECTRE_X360, .name = "alc285-hp-spectre-x360"},
+	{.id = ALC285_FIXUP_HP_SPECTRE_X360_EB1, .name = "alc285-hp-spectre-x360-eb1"},
 	{.id = ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP, .name = "alc287-ideapad-bass-spk-amp"},
 	{.id = ALC623_FIXUP_LENOVO_THINKSTATION_P340, .name = "alc623-lenovo-thinkstation-p340"},
 	{.id = ALC255_FIXUP_ACER_HEADPHONE_AND_MIC, .name = "alc255-acer-headphone-and-mic"},
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
  2021-09-29 20:42     ` Davide Baldo
@ 2021-09-29 20:50       ` Geraldo Nascimento
  2021-09-30 11:46       ` Takashi Iwai
  1 sibling, 0 replies; 6+ messages in thread
From: Geraldo Nascimento @ 2021-09-29 20:50 UTC (permalink / raw)
  To: Davide Baldo; +Cc: Takashi Iwai, alsa-devel

On Wed, Sep 29, 2021 at 10:42:37PM +0200, Davide Baldo wrote:
> Thank your for your review Takashi, the entry is now correctly
> ordered.
> I've received some feedback from a user from bugzilla
> https://bugzilla.kernel.org/show_bug.cgi?id=213953
> To address his issues I've added a variant of the laptop and forced
> DAC1 for both 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.
> 
> In order to have volume control, both front and rear speakers were
> forced to use the DAC1.
> 
> 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.
> 
> 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@baldo.me>

Hi Davide,

I think you forgot to add a v2 to the [PATCH] headline as in [PATCH v2].

Thanks!
Geraldo Nascimento

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers
  2021-09-29 20:42     ` Davide Baldo
  2021-09-29 20:50       ` Geraldo Nascimento
@ 2021-09-30 11:46       ` Takashi Iwai
  1 sibling, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-09-30 11:46 UTC (permalink / raw)
  To: Davide Baldo; +Cc: alsa-devel

On Wed, 29 Sep 2021 22:42:37 +0200,
Davide Baldo wrote:
> 
> Thank your for your review Takashi, the entry is now correctly
> ordered.
> I've received some feedback from a user from bugzilla
> https://bugzilla.kernel.org/show_bug.cgi?id=213953
> To address his issues I've added a variant of the laptop and forced
> DAC1 for both speakers.

Thanks, but the other points don't seem corrected (the
spec->mic_mute_led_polarity setup and the unneeded comma removal)?

Also, please note that the description here would be taken as the
commit log as-is, so you'd better to consider what's written there;
e.g. you need no greeting or mentioning that it's your first patch :)
Just describe the problem and the fix.  And, the references can be put
around the signed-off-by line like:
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213953

Last but not least, as Geraldo suggested, it'd be helpful to put "v2"
suffix in the subject line indicating it's a revised patch.


thanks,

Takashi

> 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.
> 
> In order to have volume control, both front and rear speakers were
> forced to use the DAC1.
> 
> 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.
> 
> 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@baldo.me>
> ---
>  sound/pci/hda/patch_realtek.c | 48 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 70516527ebce..240f47a61a5a 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6414,6 +6414,44 @@ 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)
> +{
> +	static const hda_nid_t conn[] = { 0x02 };
> +
> +	struct alc_spec *spec = codec->spec;
> +	static const struct hda_pintbl pincfgs[] = {
> +		{ 0x14, 0x90170110 },  /* front/high speakers */
> +		{ 0x17, 0x90170130 },  /* back/bass speakers */
> +		{ }
> +	};
> +
> +	//enable micmute led
> +	alc_fixup_hp_gpio_led(codec, action, 0x00, 0x04);
> +	spec->micmute_led_polarity = 1;
> +
> +	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);
> +		/* share DAC to have unified volume control */
> +		snd_hda_override_conn_list(codec, 0x14, ARRAY_SIZE(conn), conn);
> +		snd_hda_override_conn_list(codec, 0x17, ARRAY_SIZE(conn), conn);
> +		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 +6572,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 +6697,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
>  };
>  
>  static const struct hda_fixup alc269_fixups[] = {
> @@ -8222,6 +8261,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,
> @@ -8442,6 +8485,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x103c, 0x87f7, "HP Spectre x360 14", ALC245_FIXUP_HP_X360_AMP),
>  	SND_PCI_QUIRK(0x103c, 0x8805, "HP ProBook 650 G8 Notebook PC", ALC236_FIXUP_HP_GPIO_LED),
>  	SND_PCI_QUIRK(0x103c, 0x880d, "HP EliteBook 830 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
> +	SND_PCI_QUIRK(0x103c, 0x8811, "HP Spectre x360 15-eb1xxx", ALC285_FIXUP_HP_SPECTRE_X360_EB1),
> +	SND_PCI_QUIRK(0x103c, 0x8812, "HP Spectre x360 15-eb1xxx", ALC285_FIXUP_HP_SPECTRE_X360_EB1),
>  	SND_PCI_QUIRK(0x103c, 0x8846, "HP EliteBook 850 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
>  	SND_PCI_QUIRK(0x103c, 0x8847, "HP EliteBook x360 830 G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
>  	SND_PCI_QUIRK(0x103c, 0x884b, "HP EliteBook 840 Aero G8 Notebook PC", ALC285_FIXUP_HP_GPIO_LED),
> @@ -8858,6 +8903,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
>  	{.id = ALC245_FIXUP_HP_X360_AMP, .name = "alc245-hp-x360-amp"},
>  	{.id = ALC295_FIXUP_HP_OMEN, .name = "alc295-hp-omen"},
>  	{.id = ALC285_FIXUP_HP_SPECTRE_X360, .name = "alc285-hp-spectre-x360"},
> +	{.id = ALC285_FIXUP_HP_SPECTRE_X360_EB1, .name = "alc285-hp-spectre-x360-eb1"},
>  	{.id = ALC287_FIXUP_IDEAPAD_BASS_SPK_AMP, .name = "alc287-ideapad-bass-spk-amp"},
>  	{.id = ALC623_FIXUP_LENOVO_THINKSTATION_P340, .name = "alc623-lenovo-thinkstation-p340"},
>  	{.id = ALC255_FIXUP_ACER_HEADPHONE_AND_MIC, .name = "alc255-acer-headphone-and-mic"},
> -- 
> 2.32.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-30 11:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-25 19:44 [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers Davide Baldo
2021-09-28 10:36 ` Takashi Iwai
2021-09-28 10:37   ` Takashi Iwai
2021-09-29 20:42     ` Davide Baldo
2021-09-29 20:50       ` Geraldo Nascimento
2021-09-30 11:46       ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.