| 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 |
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
> > + [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.
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
> 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.
> know. I'll make an attempt to understand the cases and process > coefficients before sending v2. I did a deep dive into the code and frankly I'm a bit stumped right now since it appears that the code for the headset type detection was written by Kailang Yang from Realtek and i could not understand it since I'm not sure where to look for documentation on realtek codec processing coefficients. To rephrase what I had meant earlier, type detection for the ALC287 is currently not supported. I made an educated guess and added the codec to an existing block of code, which detected it as CTIA. However, I have no way of verifying if this guess is correct. Do you have any advice on what my next steps should be?
On Sat, 30 Nov 2024 19:58:26 +0100, Hridesh MG wrote: > > > know. I'll make an attempt to understand the cases and process > > coefficients before sending v2. > I did a deep dive into the code and frankly I'm a bit stumped right > now since it appears that the code for the headset type detection was > written by Kailang Yang from Realtek and i could not understand it > since I'm not sure where to look for documentation on realtek codec > processing coefficients. > > To rephrase what I had meant earlier, type detection for the ALC287 is > currently not supported. I made an educated guess and added the codec > to an existing block of code, which detected it as CTIA. However, I > have no way of verifying if this guess is correct. Do you have any > advice on what my next steps should be? Kailang, could you check that? thanks, Takashi
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 \
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(+)