Re: [PATCH 1/2] ALSA: hda/realtek: Fix headset mic on Acer Nitro 5

[Takashi Iwai <tiwai@xxxxxxx>]

On Thu, 14 Nov 2024 17:41:19 +0100,
Hridesh MG wrote:
> 
> The headset jack on the realtek ALC287 codec for the Acer Nitro 5
> AN515-58 laptop requires a fixup to detect microphone input.
> 
> The required pin config and verbs were obtained by reverse engineering
> the windows driver using RtHD dump.
> 
> Signed-off-by: Hridesh MG <hridesh699@xxxxxxxxx>
> ---
>  sound/pci/hda/patch_realtek.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 571fa8a6c9e1204892a93ffb24acce65400ce4b2..0204ed80791db87153dad80d6a25466985ce5784 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -7780,6 +7780,8 @@ enum {
>  	ALC287_FIXUP_LENOVO_SSID_17AA3820,
>  	ALC245_FIXUP_CLEVO_NOISY_MIC,
>  	ALC269_FIXUP_VAIO_VJFH52_MIC_NO_PRESENCE,
> +	ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_PIN,
> +	ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_VERBS,
>  };
>  
>  /* A special fixup for Lenovo C940 and Yoga Duet 7;
> @@ -10154,6 +10156,23 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC269_FIXUP_LIMIT_INT_MIC_BOOST
>  	},
> +	[ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_PIN] = {
> +		.type = HDA_FIXUP_PINS,
> +		.chained = true,
> +		.chain_id = ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_VERBS,
> +		.v.pins = (const struct hda_pintbl[]) {
> +			{ 0x19, 0x03a19020 }, /* missing pin for headphone jack mic */

This looks fine, but...

> +			{ }
> +		}
> +	},
> +	[ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_VERBS] = {
> +		.type = HDA_FIXUP_VERBS,
> +		.v.verbs = (const struct hda_verb[]) {
> +			{ 0x20, AC_VERB_SET_COEF_INDEX, 0x45 },
> +			{ 0x20, AC_VERB_SET_PROC_COEF, 0xD689 },

This is a generic setup for the headset input in CTIA mode.
Do you really need this explicity?  Usually it's set up at
alc_headset_mode_ctia() when the iPhone type is connected.

>  static const struct snd_pci_quirk alc269_fixup_tbl[] = {
> @@ -10201,6 +10220,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x1025, 0x1466, "Acer Aspire A515-56", ALC255_FIXUP_ACER_HEADPHONE_AND_MIC),
>  	SND_PCI_QUIRK(0x1025, 0x1534, "Acer Predator PH315-54", ALC255_FIXUP_ACER_MIC_NO_PRESENCE),
>  	SND_PCI_QUIRK(0x1025, 0x169a, "Acer Swift SFG16", ALC256_FIXUP_ACER_SFG16_MICMUTE_LED),
> +	SND_PCI_QUIRK(0x1025, 0x159c, "Acer Nitro 5 AN515-58", ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_PIN),

The table is sorted in PCI SSID order.  Please put at the right
place.

> +	{.id = ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_PIN, .name = "alc287-fixup-acer-nitro-headset-mic"},

Nowadays the less need for a new model name string.  You can specify
the model with an alias of SSID if any other device needs to apply the
same quirk, too.  So, unless it's mandatory, better to drop.


thanks,

Takashi