diff mbox series

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

Message ID 20241114-alc287-nitro5-v1-1-72e5bf2275c3@gmail.com (mailing list archive)
State New
Headers show
Series Fix headset mic on Acer Nitro 5 | expand

Commit Message

Hridesh MG Nov. 14, 2024, 4:41 p.m. UTC
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@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Takashi Iwai Nov. 17, 2024, 1:45 p.m. UTC | #1
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@gmail.com>
> ---
>  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
Hridesh MG Nov. 17, 2024, 8:24 p.m. UTC | #2
> > +     [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.
Yeah, the mic wont pick up input without it. I did have a feeling that
it's supposed to be set up automatically rather than being set as a
verb but I couldn't figure out which part of the code I'm supposed to
modify to make that happen. I've tried running it with dynamic
debugging turned on but I don't see the output of codec_dbg from
alc_headset_mode_ctia() :(

> >  static const struct snd2_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.
Will do, thanks.

> > +     {.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.
I added the model name string to make debugging easier for users. When
troubleshooting this issue, I followed many guides that recommended
the 'Codec-Specific Models' document, but no entry existed for the
ALC287. I thought that adding a model name would help users quickly
identify and apply the correct quirk without needing kernel expertise.
However, if this goes against the current standards, I’m happy to
adjust.
Takashi Iwai Nov. 18, 2024, 1:52 p.m. UTC | #3
On Sun, 17 Nov 2024 21:24:45 +0100,
Hridesh MG wrote:
> 
> > > +     [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.
> Yeah, the mic wont pick up input without it. I did have a feeling that
> it's supposed to be set up automatically rather than being set as a
> verb but I couldn't figure out which part of the code I'm supposed to
> modify to make that happen. I've tried running it with dynamic
> debugging turned on but I don't see the output of codec_dbg from
> alc_headset_mode_ctia() :(

Is alc_fixup_headset_mode() called by the quirk chain?
When this is set up via quirks, alc_update_headset_mode() gets called
at initialization and this should detect the headset type.  At that
point, alc_headset_mode_ctia() would be called if the proper type gets
detected.

> > >  static const struct snd2_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.
> Will do, thanks.
> 
> > > +     {.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.
> I added the model name string to make debugging easier for users. When
> troubleshooting this issue, I followed many guides that recommended
> the 'Codec-Specific Models' document, but no entry existed for the
> ALC287. I thought that adding a model name would help users quickly
> identify and apply the correct quirk without needing kernel expertise.
> However, if this goes against the current standards, I’m happy to
> adjust.

We used to add static model names in the past, but since some years ago
the driver can accept a style like model=1025:169a option, so there is
no longer need for the explicit new model name string.


thanks,

Takashi
Hridesh MG Nov. 20, 2024, 5:32 p.m. UTC | #4
> Is alc_fixup_headset_mode() called by the quirk chain?
> When this is set up via quirks, alc_update_headset_mode() gets called
> at initialization and this should detect the headset type.  At that
> point, alc_headset_mode_ctia() would be called if the proper type gets
> detected.

Thanks for pointing me in the right direction, I believe I've managed
to find the root problem. I've set it up via quirks to call
alc_update_headset_mode(), when I plug in any earphone,
alc_determine_headset_type() incorrectly determines that the type is
OMTP despite it being CTIA (I've confirmed this on a friend's laptop).
Adding my codec to a case in alc_determine_headset_type() for my codec
seems to fix this issue.
After the headset is detected as CTIA, I also had to add my codec to a
case in alc_headset_mode_ctia() to set the process coefficient to
0xD689 (as written in my original patch) to finally pick up the mic
input.

I hope this is a satisfactory approach, if it is not, please let me
know. I'll make an attempt to understand the cases and process
coefficients before sending v2.
diff mbox series

Patch

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 */
+			{ }
+		}
+	},
+	[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 },
+			{ }
+		}
+	}
 };
 
 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),
 	SND_PCI_QUIRK(0x1028, 0x0470, "Dell M101z", ALC269_FIXUP_DELL_M101Z),
 	SND_PCI_QUIRK(0x1028, 0x053c, "Dell Latitude E5430", ALC292_FIXUP_DELL_E7X),
 	SND_PCI_QUIRK(0x1028, 0x054b, "Dell XPS one 2710", ALC275_FIXUP_DELL_XPS),
@@ -11217,6 +11237,7 @@  static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC255_FIXUP_ACER_HEADPHONE_AND_MIC, .name = "alc255-acer-headphone-and-mic"},
 	{.id = ALC285_FIXUP_HP_GPIO_AMP_INIT, .name = "alc285-hp-amp-init"},
 	{.id = ALC236_FIXUP_LENOVO_INV_DMIC, .name = "alc236-fixup-lenovo-inv-mic"},
+	{.id = ALC_287_FIXUP_ACER_NITRO_HEADSET_MIC_PIN, .name = "alc287-fixup-acer-nitro-headset-mic"},
 	{}
 };
 #define ALC225_STANDARD_PINS \