[PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop
From: Luke D Jones luke@ljones.dev
The GX502 requires a few steps to enable the headset i/o: pincfg, verbs to enable and unmute the amp used for headpone out, and a jacksense callback to toggle output via internal or jack using a verb.
Signed-off-by: Luke Jones luke@ljones.dev --- sound/pci/hda/patch_realtek.c | 70 ++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c521a1f17096..d2052a6e3b13 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5424,7 +5424,7 @@ static void alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec, struct alc_spec *spec = codec->spec; spec->parse_flags |= HDA_PINCFG_HEADSET_MIC; alc255_set_default_jack_type(codec); - } + } else alc_fixup_headset_mode(codec, fix, action); } @@ -5993,6 +5993,41 @@ static void alc_fixup_disable_mic_vref(struct hda_codec *codec, snd_hda_codec_set_pin_target(codec, 0x19, PIN_VREFHIZ); }
+ +static void alc294_gx502_toggle_output(struct hda_codec *codec, + struct hda_jack_callback *cb) +{ + /* The Windows driver sets the codec up in a very different way where + * it appears to leave 0x10 = 0x8a20 set. For Linux we need to toggle it + */ + if (snd_hda_jack_detect_state(codec, 0x21) == HDA_JACK_PRESENT) { + alc_write_coef_idx(codec, 0x10, 0x8a20); + } else { + alc_write_coef_idx(codec, 0x10, 0x0a20); + } +} + +static void alc294_fixup_gx502_hp(struct hda_codec *codec, + const struct hda_fixup *fix, int action) +{ + /* Pin 0x21: headphones/headset mic */ + if (!is_jack_detectable(codec, 0x21)) + return; + + switch (action) { + case HDA_FIXUP_ACT_PRE_PROBE: + snd_hda_jack_detect_enable_callback(codec, 0x21, + alc294_gx502_toggle_output); + break; + case HDA_FIXUP_ACT_INIT: + /* Make sure to start in a correct state, i.e. if + * headphones have been plugged in before powering up the system + */ + alc294_gx502_toggle_output(codec, NULL); + break; + } +} + static void alc285_fixup_hp_gpio_amp_init(struct hda_codec *codec, const struct hda_fixup *fix, int action) { @@ -6172,6 +6207,9 @@ enum { ALC285_FIXUP_THINKPAD_X1_GEN7, ALC285_FIXUP_THINKPAD_HEADSET_JACK, ALC294_FIXUP_ASUS_HPE, + ALC294_FIXUP_ASUS_GX502_HP, + ALC294_FIXUP_ASUS_GX502_PINS, + ALC294_FIXUP_ASUS_GX502_VERBS, ALC294_FIXUP_ASUS_COEF_1B, ALC285_FIXUP_HP_GPIO_LED, ALC285_FIXUP_HP_MUTE_LED, @@ -7338,6 +7376,35 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC294_FIXUP_ASUS_HEADSET_MIC }, + [ALC294_FIXUP_ASUS_GX502_PINS] = { + .type = HDA_FIXUP_PINS, + .v.pins = (const struct hda_pintbl[]) { + { 0x19, 0x03a11050 }, /* front HP mic */ + { 0x1a, 0x01a11830 }, //0x00a11030 }, /* rear external mic */ + { 0x21, 0x03211020 }, /* HP out */ + { } + }, + .chained = true, + .chain_id = ALC294_FIXUP_ASUS_GX502_VERBS + }, + [ALC294_FIXUP_ASUS_GX502_VERBS] = { + .type = HDA_FIXUP_VERBS, + .v.verbs = (const struct hda_verb[]) { + /* set 0x15 to HP-OUT ctrl */ + { 0x15, AC_VERB_SET_PIN_WIDGET_CONTROL, 0xc0 }, + /* unmute the 0x15 amp */ + { 0x15, AC_VERB_SET_AMP_GAIN_MUTE, 0xb000 }, + /* set 0x0a input converter to digital */ + { 0x0a, 0x70d, 0x01 }, + { } + }, + .chained = true, + .chain_id = ALC294_FIXUP_ASUS_GX502_HP + }, + [ALC294_FIXUP_ASUS_GX502_HP] = { + .type = HDA_FIXUP_FUNC, + .v.func = alc294_fixup_gx502_hp, + }, [ALC294_FIXUP_ASUS_COEF_1B] = { .type = HDA_FIXUP_VERBS, .v.verbs = (const struct hda_verb[]) { @@ -7711,6 +7778,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1043, 0x1ccd, "ASUS X555UB", ALC256_FIXUP_ASUS_MIC), SND_PCI_QUIRK(0x1043, 0x1e11, "ASUS Zephyrus G15", ALC289_FIXUP_ASUS_GA502), SND_PCI_QUIRK(0x1043, 0x1f11, "ASUS Zephyrus G14", ALC289_FIXUP_ASUS_GA401), + SND_PCI_QUIRK(0x1043, 0x1881, "ASUS Zephyrus S/M", ALC294_FIXUP_ASUS_GX502_PINS), SND_PCI_QUIRK(0x1043, 0x3030, "ASUS ZN270IE", ALC256_FIXUP_ASUS_AIO_GPIO2), SND_PCI_QUIRK(0x1043, 0x831a, "ASUS P901", ALC269_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x1043, 0x834a, "ASUS S101", ALC269_FIXUP_STEREO_DMIC),
On Sun, 06 Sep 2020 10:25:07 +0200, Luke Jones wrote:
From: Luke D Jones luke@ljones.dev
The GX502 requires a few steps to enable the headset i/o: pincfg, verbs to enable and unmute the amp used for headpone out, and a jacksense callback to toggle output via internal or jack using a verb.
Thanks for the patch. The code changes look mostly good, but a few minor coding problems (mostly coding style) are seen. Could you resubmit with fixes?
Signed-off-by: Luke Jones luke@ljones.dev
The SOB doesn't match with From line. Is it intentional?
....
--- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5424,7 +5424,7 @@ static void alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec, struct alc_spec *spec = codec->spec; spec->parse_flags |= HDA_PINCFG_HEADSET_MIC; alc255_set_default_jack_type(codec);
- }
- } else alc_fixup_headset_mode(codec, fix, action);
}
Please drop the unrelated change.
....
+static void alc294_gx502_toggle_output(struct hda_codec *codec,
struct hda_jack_callback *cb)
+{
- /* The Windows driver sets the codec up in a very different way where
* it appears to leave 0x10 = 0x8a20 set. For Linux we need to toggle it
*/
- if (snd_hda_jack_detect_state(codec, 0x21) == HDA_JACK_PRESENT) {
alc_write_coef_idx(codec, 0x10, 0x8a20);
- } else {
alc_write_coef_idx(codec, 0x10, 0x0a20);
- }
Remove braces for a single if line. Checkpatch would suggest it, too.
....
@@ -7338,6 +7376,35 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC294_FIXUP_ASUS_HEADSET_MIC },
- [ALC294_FIXUP_ASUS_GX502_PINS] = {
.type = HDA_FIXUP_PINS,
.v.pins = (const struct hda_pintbl[]) {
{ 0x19, 0x03a11050 }, /* front HP mic */
{ 0x1a, 0x01a11830 }, //0x00a11030 }, /* rear external mic */
The doubly comments look awkward. Please reformat it.
....
- [ALC294_FIXUP_ASUS_GX502_VERBS] = {
.type = HDA_FIXUP_VERBS,
.v.verbs = (const struct hda_verb[]) {
/* set 0x15 to HP-OUT ctrl */
Please align the comment at the right tab.
{ 0x15, AC_VERB_SET_PIN_WIDGET_CONTROL, 0xc0 },
/* unmute the 0x15 amp */
{ 0x15, AC_VERB_SET_AMP_GAIN_MUTE, 0xb000 },
/* set 0x0a input converter to digital */
{ 0x0a, 0x70d, 0x01 },
Use AC_VERB_SET_DIGI_CONVERT_1. And, how is this related with the headset fix? Is this really mandatory? If this widget is really wired, usually the digital I/O is controlled via IEC9158 status mixer.
thanks,
Takashi
On Mon, Sep 7, 2020 at 09:03, Takashi Iwai tiwai@suse.de wrote:
Signed-off-by: Luke Jones luke@ljones.dev
The SOB doesn't match with From line. Is it intentional?
I missed this sorry. Have corrected.
- }
- } else alc_fixup_headset_mode(codec, fix, action);
}
Please drop the unrelated change.
Looks like my vim settings removed an ending white-space. I'll remove the change.
....
+static void alc294_gx502_toggle_output(struct hda_codec *codec,
struct hda_jack_callback *cb)
+{
- /* The Windows driver sets the codec up in a very different way
where
* it appears to leave 0x10 = 0x8a20 set. For Linux we need to
toggle it
*/
- if (snd_hda_jack_detect_state(codec, 0x21) == HDA_JACK_PRESENT) {
alc_write_coef_idx(codec, 0x10, 0x8a20);
- } else {
alc_write_coef_idx(codec, 0x10, 0x0a20);
- }
Remove braces for a single if line. Checkpatch would suggest it, too.
Done
....
@@ -7338,6 +7376,35 @@ static const struct hda_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC294_FIXUP_ASUS_HEADSET_MIC },
- [ALC294_FIXUP_ASUS_GX502_PINS] = {
.type = HDA_FIXUP_PINS,
.v.pins = (const struct hda_pintbl[]) {
{ 0x19, 0x03a11050 }, /* front HP mic */
{ 0x1a, 0x01a11830 }, //0x00a11030 }, /* rear external mic */
The doubly comments look awkward. Please reformat it.
Missed this also, sorry :(
....
- [ALC294_FIXUP_ASUS_GX502_VERBS] = {
.type = HDA_FIXUP_VERBS,
.v.verbs = (const struct hda_verb[]) {
/* set 0x15 to HP-OUT ctrl */
Please align the comment at the right tab.
Done
{ 0x15, AC_VERB_SET_PIN_WIDGET_CONTROL, 0xc0 },
/* unmute the 0x15 amp */
{ 0x15, AC_VERB_SET_AMP_GAIN_MUTE, 0xb000 },
/* set 0x0a input converter to digital */
{ 0x0a, 0x70d, 0x01 },
Use AC_VERB_SET_DIGI_CONVERT_1. And, how is this related with the headset fix? Is this really mandatory? If this widget is really wired, usually the digital I/O is controlled via IEC9158 status mixer.
This I'm not actually sure about, I'm sort of fumbling around trying to get the rear mic jack working. I have now removed the comment and 0x0a line so the patch is focused solely on the headset.
Many thanks for the code review. I will submit the fixed version now.
Kind regards, Luke.
participants (2)
-
Luke Jones
-
Takashi Iwai