2024-01-04 11:11:23

by bo liu

[permalink] [raw]
Subject: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

When OMTP headset plugin the headset jack of CX8070 and SN6160 sound cards,
the headset type detection circuit will recognize the headset type as CTIA.
At this point, plugout and plugin the headset will get the correct headset
type as OMTP.
The reason for the failure of headset type recognition is that the sound
card creation will enable the VREF voltage of the headset mic, which
interferes with the headset type automatic detection circuit. Plugout and
plugin the headset will restart the headset detection and get the correct
headset type.
The patch is disable the VREF voltage when the headset is not present, and
will enable the VREF voltage when the headset is present.

Signed-off-by: bo liu <[email protected]>
---
sound/pci/hda/patch_conexant.c | 108 ++++++++++++++++++++++++++++++++-
1 file changed, 106 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index a889cccdd607..29c62181dbc3 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -42,7 +42,8 @@ struct conexant_spec {
unsigned int gpio_led;
unsigned int gpio_mute_led_mask;
unsigned int gpio_mic_led_mask;
-
+ unsigned int headset_present_flag;
+ bool is_cx8070_sn6140;
};


@@ -164,6 +165,22 @@ static void cxt_init_gpio_led(struct hda_codec *codec)
}
}

+static void cx_fixup_headset_recog(struct hda_codec *codec)
+{
+ unsigned int mic_persent;
+
+ /* fix some headset type recognize fail issue, such as EDIFIER headset */
+ snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
+ snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
+ snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);
+ /* fix reboot headset type recognize fail issue */
+ mic_persent = snd_hda_codec_read(codec, 0x19, 0, AC_VERB_GET_PIN_SENSE, 0x0);
+ if (mic_persent&0x80000000)
+ snd_hda_codec_write(codec, 0x19, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x24);
+ else
+ snd_hda_codec_write(codec, 0x19, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x20);
+}
+
static int cx_auto_init(struct hda_codec *codec)
{
struct conexant_spec *spec = codec->spec;
@@ -174,6 +191,9 @@ static int cx_auto_init(struct hda_codec *codec)
cxt_init_gpio_led(codec);
snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);

+ if (spec->is_cx8070_sn6140)
+ cx_fixup_headset_recog(codec);
+
return 0;
}

@@ -192,6 +212,81 @@ static void cx_auto_free(struct hda_codec *codec)
snd_hda_gen_free(codec);
}

+enum {
+ CX_HEADSET_NOPRESENT = 0,
+ CX_HEADSET_PARTPRESENT,
+ CX_HEADSET_ALLPRESENT,
+};
+
+static void cx_process_headset_plugin(struct hda_codec *codec)
+{
+ unsigned int val;
+ unsigned int count = 0;
+
+ /* Wait headset detect done. */
+ do {
+ val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);
+ if (val&0x080) {
+ codec_dbg(codec, "headset type detect done!\n");
+ break;
+ }
+ msleep(20);
+ count++;
+ } while (count < 3);
+ val = snd_hda_codec_read(codec, 0x1c, 0, 0xcb0, 0x0);
+ if (val&0x800) {
+ codec_dbg(codec, "headset plugin, type is CTIA\n");
+ snd_hda_codec_write(codec, 0x19, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x24);
+ } else if (val&0x400) {
+ codec_dbg(codec, "headset plugin, type is OMTP\n");
+ snd_hda_codec_write(codec, 0x19, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x24);
+ } else {
+ codec_dbg(codec, "headphone plugin\n");
+ }
+}
+
+static void cx_update_headset_mic_vref(struct hda_codec *codec, unsigned int res)
+{
+ unsigned int phone_present, mic_persent, phone_tag, mic_tag;
+ struct conexant_spec *spec = codec->spec;
+
+ /* In cx8070 and sn6140, headset is fixed to use node 16 and node 19.
+ * Check hp and mic tag to process headset pulgin and plugout.
+ */
+ phone_tag = snd_hda_codec_read(codec, 0x16, 0, AC_VERB_GET_UNSOLICITED_RESPONSE, 0x0);
+ mic_tag = snd_hda_codec_read(codec, 0x19, 0, AC_VERB_GET_UNSOLICITED_RESPONSE, 0x0);
+ if ((phone_tag&(res>>AC_UNSOL_RES_TAG_SHIFT)) || (mic_tag&(res>>AC_UNSOL_RES_TAG_SHIFT))) {
+ phone_present = snd_hda_codec_read(codec, 0x16, 0, AC_VERB_GET_PIN_SENSE, 0x0);
+ if (!(phone_present&AC_PINSENSE_PRESENCE)) {/* headphone plugout */
+ spec->headset_present_flag = CX_HEADSET_NOPRESENT;
+ snd_hda_codec_write(codec, 0x19, 0, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x20);
+ return;
+ }
+ if (spec->headset_present_flag == CX_HEADSET_NOPRESENT) {
+ spec->headset_present_flag = CX_HEADSET_PARTPRESENT;
+ } else if (spec->headset_present_flag == CX_HEADSET_PARTPRESENT) {
+ mic_persent = snd_hda_codec_read(codec, 0x19, 0,
+ AC_VERB_GET_PIN_SENSE, 0x0);
+ /* headset is present */
+ if ((phone_present&AC_PINSENSE_PRESENCE) &&
+ (mic_persent&AC_PINSENSE_PRESENCE)) {
+ cx_process_headset_plugin(codec);
+ spec->headset_present_flag = CX_HEADSET_ALLPRESENT;
+ }
+ }
+ }
+}
+
+static void cx_jack_unsol_event(struct hda_codec *codec, unsigned int res)
+{
+ struct conexant_spec *spec = codec->spec;
+
+ if (spec->is_cx8070_sn6140)
+ cx_update_headset_mic_vref(codec, res);
+
+ snd_hda_jack_unsol_event(codec, res);
+}
+
#ifdef CONFIG_PM
static int cx_auto_suspend(struct hda_codec *codec)
{
@@ -205,7 +300,7 @@ static const struct hda_codec_ops cx_auto_patch_ops = {
.build_pcms = snd_hda_gen_build_pcms,
.init = cx_auto_init,
.free = cx_auto_free,
- .unsol_event = snd_hda_jack_unsol_event,
+ .unsol_event = cx_jack_unsol_event,
#ifdef CONFIG_PM
.suspend = cx_auto_suspend,
.check_power_status = snd_hda_gen_check_power_status,
@@ -1042,6 +1137,15 @@ static int patch_conexant_auto(struct hda_codec *codec)
codec->spec = spec;
codec->patch_ops = cx_auto_patch_ops;

+ /* init cx8070/sn6140 flag and reset headset_present_flag */
+ switch (codec->core.vendor_id) {
+ case 0x14f11f86:
+ case 0x14f11f87:
+ spec->is_cx8070_sn6140 = true;
+ spec->headset_present_flag = CX_HEADSET_NOPRESENT;
+ break;
+ }
+
cx_auto_parse_eapd(codec);
spec->gen.own_eapd_ctl = 1;

--
2.34.1



2024-01-05 17:01:56

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

On Thu, 04 Jan 2024 12:10:44 +0100,
bo liu wrote:
>
> When OMTP headset plugin the headset jack of CX8070 and SN6160 sound cards,
> the headset type detection circuit will recognize the headset type as CTIA.
> At this point, plugout and plugin the headset will get the correct headset
> type as OMTP.
> The reason for the failure of headset type recognition is that the sound
> card creation will enable the VREF voltage of the headset mic, which
> interferes with the headset type automatic detection circuit. Plugout and
> plugin the headset will restart the headset detection and get the correct
> headset type.
> The patch is disable the VREF voltage when the headset is not present, and
> will enable the VREF voltage when the headset is present.
>
> Signed-off-by: bo liu <[email protected]>

Please put the revision number to the subject prefix, i.e.
"Subject: [PATCH v3] ALSA: hda/conexant: ...."

> +static void cx_fixup_headset_recog(struct hda_codec *codec)
> +{
> + unsigned int mic_persent;
> +
> + /* fix some headset type recognize fail issue, such as EDIFIER headset */
> + snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);

Please use the defined verbs in sound/hda_verbs.h.
The arguments (0x320, 0x010) are (AC_VERB_SET_AMP_GAIN_MUTE, 0x2010)
etc.

Also, it's still not clear what if other nodes are used for headphone
and mic pins -- or when either only headphone or only mic is present.
A rare case, but we need to cover.

> + /* fix reboot headset type recognize fail issue */
> + mic_persent = snd_hda_codec_read(codec, 0x19, 0, AC_VERB_GET_PIN_SENSE, 0x0);
> + if (mic_persent&0x80000000)

A coding style problem. Similar issues seen in a few other places,
too. Consult scripts/checkpatch.pl.

> +enum {
> + CX_HEADSET_NOPRESENT = 0,
> + CX_HEADSET_PARTPRESENT,
> + CX_HEADSET_ALLPRESENT,
> +};

This should be defined earlier. You can use the enum type for
spec->headset_present_flag, too.

> +static void cx_process_headset_plugin(struct hda_codec *codec)
> +{
> + unsigned int val;
> + unsigned int count = 0;
> +
> + /* Wait headset detect done. */
> + do {
> + val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);

Use the verb: AC_VERB_GET_PROC_COEF, 0xa000.
At best, define the COEF values 0xa000 and 0xb000, and the
corresponding value bits, too.

> +static void cx_update_headset_mic_vref(struct hda_codec *codec, unsigned int res)
> +{
> + unsigned int phone_present, mic_persent, phone_tag, mic_tag;
> + struct conexant_spec *spec = codec->spec;
> +
> + /* In cx8070 and sn6140, headset is fixed to use node 16 and node 19.

Is it really guaranteed? IMO, we should check the pin configs
beforehand at the parsing time.


thanks,

Takashi

2024-01-08 08:42:55

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

On Mon, 08 Jan 2024 04:29:51 +0100,
???? wrote:
>
> hi Takashi Iwai,
> Thank you very much for your patient guidance. Below is the reply to
> the question, please kindly correct it, thanks.
>
> > +static void cx_fixup_headset_recog(struct hda_codec *codec) {
> > + unsigned int mic_persent;
> > +
> > + /* fix some headset type recognize fail issue, such as EDIFIER
> headset */
> > + snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
> > + snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
> > + snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);
>
> Please use the defined verbs in sound/hda_verbs.h.
> The arguments (0x320, 0x010) are (AC_VERB_SET_AMP_GAIN_MUTE, 0x2010) etc.
>
> Re: (0x1c, 0x320) is not amp gain register, but vendor defined register as
> rx control register. Use AC_VERB_SET_AMP_GAIN_MUTE will confused. It's
> similar to 0x4f0 and 0xca0.

Ah interesting. But the verb is actually seen as
AC_VERB_SET_AMP_GAIN_MUTE -- although the resultant bits seem invalid.

HD-audio combines the verb and the value into 20 bits, e.g. (0x320,
0x10) is composed as 0x32010, and (0x3b0, 0xe10) is 0x3be10.
0x3xx is translated as SET_AMP_GAIN_MUTE, but in your case, 0x32010
leaves 0 to both the input/output bits (bits 14 and 15), which makes
it as invalid.

0x3be10 is another invalid verb, which sets SET_AMP_GAIN_MUTE with
OUTPUT, but it sets both LEFT and RIGHT, and passes a high index
(14).

And, what actually (0x4f0, 0x0eb) does? It's composed as 0x4f0eb, and
in this case, it's a valid verb (SET_PROC_COEF + 0xf0eb). But COEF is
vendor-specific, so it can be translated in everything the chip
wants.

So, if those verbs are vendor-specific ones, please define them and/or
give proper comments to explain what they do for each.


> Also, it's still not clear what if other nodes are used for headphone and
> mic pins -- or when either only headphone or only mic is present.
> A rare case, but we need to cover.
>
> Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as
> headset. Other nodes can be used separately as headphones or microphones,
> but not as headset,
> so their configuration will not interfere with the type detection of
> headset.

OK, then explain this in comments, too (that we blindly assume those
pins).


> > + /* fix reboot headset type recognize fail issue */
> > + mic_persent = snd_hda_codec_read(codec, 0x19, 0,
> AC_VERB_GET_PIN_SENSE, 0x0);
> > + if (mic_persent&0x80000000)
>
> A coding style problem. Similar issues seen in a few other places, too.
> Consult scripts/checkpatch.pl.
>
> Re: was & need space? I have checked with scripts/checkpatch.pl before
> submitting the patch and there are no warnings or errors.

Yes. Please put spaces around the operators.


> > +static void cx_process_headset_plugin(struct hda_codec *codec) {
> > + unsigned int val;
> > + unsigned int count = 0;
> > +
> > + /* Wait headset detect done. */
> > + do {
> > + val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);
>
> Use the verb: AC_VERB_GET_PROC_COEF, 0xa000.
> At best, define the COEF values 0xa000 and 0xb000, and the corresponding
> value bits, too.
>
> Re: (0x1c, 0xca0) is not COEF register, but vendor defined register as
> jacksense register.
>
> > +static void cx_update_headset_mic_vref(struct hda_codec *codec,
> > +unsigned int res) {
> > + unsigned int phone_present, mic_persent, phone_tag, mic_tag;
> > + struct conexant_spec *spec = codec->spec;
> > +
> > + /* In cx8070 and sn6140, headset is fixed to use node 16 and node
> 19.
>
> Is it really guaranteed? IMO, we should check the pin configs beforehand at
> the parsing time.
>
> Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as
> headset. The node 16 can only be config to headphone or disable,
> The node 19 can only be config to microphone or disable. Only node 16 and
> node 19 both enable, the patch will process.

Then we still might need a check for the condition?


thanks,

Takashi

2024-01-08 10:39:23

by bo liu

[permalink] [raw]
Subject: Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070 and SN6140

hi Takashi Iwai,
Thank you very much for your patient guidance. Below is the reply to
the question, please kindly correct it, thanks.

> +static void cx_fixup_headset_recog(struct hda_codec *codec) {
> + unsigned int mic_persent;
> +
> + /* fix some headset type recognize fail issue, such as EDIFIER
headset */
> + snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);

Please use the defined verbs in sound/hda_verbs.h.
The arguments (0x320, 0x010) are (AC_VERB_SET_AMP_GAIN_MUTE, 0x2010) etc.

Re: (0x1c, 0x320) is not amp gain register, but vendor defined register as
rx control register. Use AC_VERB_SET_AMP_GAIN_MUTE will confused. It's
similar to 0x4f0 and 0xca0.

Also, it's still not clear what if other nodes are used for headphone and
mic pins -- or when either only headphone or only mic is present.
A rare case, but we need to cover.

Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as
headset. Other nodes can be used separately as headphones or microphones,
but not as headset,
so their configuration will not interfere with the type detection of
headset.

> + /* fix reboot headset type recognize fail issue */
> + mic_persent = snd_hda_codec_read(codec, 0x19, 0,
AC_VERB_GET_PIN_SENSE, 0x0);
> + if (mic_persent&0x80000000)

A coding style problem. Similar issues seen in a few other places, too.
Consult scripts/checkpatch.pl.

Re: was & need space? I have checked with scripts/checkpatch.pl before
submitting the patch and there are no warnings or errors.

> +static void cx_process_headset_plugin(struct hda_codec *codec) {
> + unsigned int val;
> + unsigned int count = 0;
> +
> + /* Wait headset detect done. */
> + do {
> + val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);

Use the verb: AC_VERB_GET_PROC_COEF, 0xa000.
At best, define the COEF values 0xa000 and 0xb000, and the corresponding
value bits, too.

Re: (0x1c, 0xca0) is not COEF register, but vendor defined register as
jacksense register.

> +static void cx_update_headset_mic_vref(struct hda_codec *codec,
> +unsigned int res) {
> + unsigned int phone_present, mic_persent, phone_tag, mic_tag;
> + struct conexant_spec *spec = codec->spec;
> +
> + /* In cx8070 and sn6140, headset is fixed to use node 16 and node
19.

Is it really guaranteed? IMO, we should check the pin configs beforehand at
the parsing time.

Re: in cx8070 and sn6140, only 0x16 and 0x19 can be used together as
headset. The node 16 can only be config to headphone or disable,
The node 19 can only be config to microphone or disable. Only node 16 and
node 19 both enable, the patch will process.

Best Regards
Bo Liu
??
-----?ʼ?ԭ??-----
??????: Takashi Iwai <[email protected]>
????ʱ??: 2024??1??6?? 1:02
?ռ???: bo liu <[email protected]>
????: [email protected]; [email protected]; [email protected];
[email protected]
????: Re: [PATCH] ALSA: hda/conexant: Fix headset auto detect fail in cx8070
and SN6140

On Thu, 04 Jan 2024 12:10:44 +0100,
bo liu wrote:
>
> When OMTP headset plugin the headset jack of CX8070 and SN6160 sound
> cards, the headset type detection circuit will recognize the headset type
as CTIA.
> At this point, plugout and plugin the headset will get the correct
> headset type as OMTP.
> The reason for the failure of headset type recognition is that the
> sound card creation will enable the VREF voltage of the headset mic,
> which interferes with the headset type automatic detection circuit.
> Plugout and plugin the headset will restart the headset detection and
> get the correct headset type.
> The patch is disable the VREF voltage when the headset is not present,
> and will enable the VREF voltage when the headset is present.
>
> Signed-off-by: bo liu <[email protected]>

Please put the revision number to the subject prefix, i.e.
"Subject: [PATCH v3] ALSA: hda/conexant: ...."

> +static void cx_fixup_headset_recog(struct hda_codec *codec) {
> + unsigned int mic_persent;
> +
> + /* fix some headset type recognize fail issue, such as EDIFIER
headset */
> + snd_hda_codec_write(codec, 0x1c, 0, 0x320, 0x010);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x3b0, 0xe10);
> + snd_hda_codec_write(codec, 0x1c, 0, 0x4f0, 0x0eb);

Please use the defined verbs in sound/hda_verbs.h.
The arguments (0x320, 0x010) are (AC_VERB_SET_AMP_GAIN_MUTE, 0x2010) etc.

Also, it's still not clear what if other nodes are used for headphone and
mic pins -- or when either only headphone or only mic is present.
A rare case, but we need to cover.

> + /* fix reboot headset type recognize fail issue */
> + mic_persent = snd_hda_codec_read(codec, 0x19, 0,
AC_VERB_GET_PIN_SENSE, 0x0);
> + if (mic_persent&0x80000000)

A coding style problem. Similar issues seen in a few other places, too.
Consult scripts/checkpatch.pl.

> +enum {
> + CX_HEADSET_NOPRESENT = 0,
> + CX_HEADSET_PARTPRESENT,
> + CX_HEADSET_ALLPRESENT,
> +};

This should be defined earlier. You can use the enum type for
spec->headset_present_flag, too.

> +static void cx_process_headset_plugin(struct hda_codec *codec) {
> + unsigned int val;
> + unsigned int count = 0;
> +
> + /* Wait headset detect done. */
> + do {
> + val = snd_hda_codec_read(codec, 0x1c, 0, 0xca0, 0x0);

Use the verb: AC_VERB_GET_PROC_COEF, 0xa000.
At best, define the COEF values 0xa000 and 0xb000, and the corresponding
value bits, too.

> +static void cx_update_headset_mic_vref(struct hda_codec *codec,
> +unsigned int res) {
> + unsigned int phone_present, mic_persent, phone_tag, mic_tag;
> + struct conexant_spec *spec = codec->spec;
> +
> + /* In cx8070 and sn6140, headset is fixed to use node 16 and node
19.

Is it really guaranteed? IMO, we should check the pin configs beforehand at
the parsing time.


thanks,

Takashi