* [PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop
@ 2020-09-06 8:25 Luke Jones
2020-09-07 7:03 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Luke Jones @ 2020-09-06 8:25 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: hui.wang, Luke D Jones, kailang
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),
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop
2020-09-06 8:25 [PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop Luke Jones
@ 2020-09-07 7:03 ` Takashi Iwai
2020-09-07 8:09 ` Luke Jones
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2020-09-07 7:03 UTC (permalink / raw)
To: Luke Jones; +Cc: hui.wang, alsa-devel, kailang
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop
2020-09-07 7:03 ` Takashi Iwai
@ 2020-09-07 8:09 ` Luke Jones
0 siblings, 0 replies; 3+ messages in thread
From: Luke Jones @ 2020-09-07 8:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: hui.wang, alsa-devel, kailang
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-07 8:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-06 8:25 [PATCH] ALSA: hda: fixup headset for ASUS GX502 laptop Luke Jones
2020-09-07 7:03 ` Takashi Iwai
2020-09-07 8:09 ` Luke Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).