Re: [PATCH] Fixes HP Spectre x360 15-eb1xxx speakers

[Takashi Iwai <tiwai@suse.de>]

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
>