| Message ID | 20210316115549.240014-1-hui.wang@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda/realtek: add more delay time before determine_headset_type | expand |
On Tue, 16 Mar 2021 12:55:49 +0100, Hui Wang wrote: > > We found a recording issue on the headset-mic recently, sometimes > users plug a headset and select headset-mic from UI, but can't record > any sound from headset-mic. The root cause is the > determine_headset_type() returns a wrong type, e.g. users plug a ctia > type headset, but that function returns omtp type. > > In the past, determine_headset_type() worked well because the internal > mic is connected to the codec, so the "Input Source" or > "Capture Source" is internal mic by default when users plug a headset, > the determine_headset_type() will not be called unless users select > headset-mic from UI, when users select headset-mic, the plugging > action already finished and the headset is completely plugged into the > jack, so determine_headset_type() could return a correct type. > > But more and more machines connect the internal mic to the PCH now, > and the "Input Source" is headset mic by default, when users plug a > headset, the determine_headset_type() will be called immediately, if > the headset is not completely plugged in, it will return a wrong type. > > Here add 2s delay before calling determine_headset_type(), and since > there is a pop-up dialogue when users do plugging action, to avoid > freezing the UI, use the deleyed_work to call that function. Hm, two seconds are quite long, IMHO. How is this delay determined? Takashi > > Signed-off-by: Hui Wang <hui.wang@canonical.com> > --- > sound/pci/hda/patch_realtek.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > index b47504fa8dfd..1f6fc8addf3e 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -127,6 +127,8 @@ struct alc_spec { > unsigned int coef0; > struct input_dev *kb_dev; > u8 alc_mute_keycode_map[1]; > + struct hda_codec *codec; > + struct delayed_work headset_set_work; > }; > > /* > @@ -1160,6 +1162,7 @@ static int alc_alloc_spec(struct hda_codec *codec, hda_nid_t mixer_nid) > kfree(spec); > return err; > } > + spec->codec = codec; > return 0; > } > > @@ -5363,6 +5366,21 @@ static void alc_determine_headset_type(struct hda_codec *codec) > spec->current_headset_type = is_ctia ? ALC_HEADSET_TYPE_CTIA : ALC_HEADSET_TYPE_OMTP; > } > > +static void alc_headset_check_and_set(struct work_struct *work) > +{ > + struct alc_spec *spec = container_of(work, struct alc_spec, > + headset_set_work.work); > + struct hda_codec *codec = spec->codec; > + > + if (spec->current_headset_type == ALC_HEADSET_TYPE_UNKNOWN) > + alc_determine_headset_type(codec); > + if (spec->current_headset_type == ALC_HEADSET_TYPE_CTIA) > + alc_headset_mode_ctia(codec); > + else if (spec->current_headset_type == ALC_HEADSET_TYPE_OMTP) > + alc_headset_mode_omtp(codec); > + spec->gen.hp_jack_present = true; > +} > + > static void alc_update_headset_mode(struct hda_codec *codec) > { > struct alc_spec *spec = codec->spec; > @@ -5386,6 +5404,7 @@ static void alc_update_headset_mode(struct hda_codec *codec) > return; > } > > + cancel_delayed_work_sync(&spec->headset_set_work); > switch (new_headset_mode) { > case ALC_HEADSET_MODE_UNPLUGGED: > alc_headset_mode_unplugged(codec); > @@ -5394,13 +5413,7 @@ static void alc_update_headset_mode(struct hda_codec *codec) > spec->gen.hp_jack_present = false; > break; > case ALC_HEADSET_MODE_HEADSET: > - if (spec->current_headset_type == ALC_HEADSET_TYPE_UNKNOWN) > - alc_determine_headset_type(codec); > - if (spec->current_headset_type == ALC_HEADSET_TYPE_CTIA) > - alc_headset_mode_ctia(codec); > - else if (spec->current_headset_type == ALC_HEADSET_TYPE_OMTP) > - alc_headset_mode_omtp(codec); > - spec->gen.hp_jack_present = true; > + schedule_delayed_work(&spec->headset_set_work, msecs_to_jiffies(2000)); > break; > case ALC_HEADSET_MODE_MIC: > alc_headset_mode_mic_in(codec, hp_pin, spec->headphone_mic_pin); > @@ -5466,6 +5479,7 @@ static void alc_fixup_headset_mode(struct hda_codec *codec, > spec->parse_flags |= HDA_PINCFG_HEADSET_MIC | HDA_PINCFG_HEADPHONE_MIC; > break; > case HDA_FIXUP_ACT_PROBE: > + INIT_DELAYED_WORK(&spec->headset_set_work, alc_headset_check_and_set); > alc_probe_headset_mode(codec); > break; > case HDA_FIXUP_ACT_INIT: > @@ -5475,6 +5489,9 @@ static void alc_fixup_headset_mode(struct hda_codec *codec, > } > alc_update_headset_mode(codec); > break; > + case HDA_FIXUP_ACT_FREE: > + cancel_delayed_work_sync(&spec->headset_set_work); > + break; > } > } > > -- > 2.25.1 >
On 3/16/21 10:17 PM, Takashi Iwai wrote: > On Tue, 16 Mar 2021 12:55:49 +0100, > Hui Wang wrote: >> We found a recording issue on the headset-mic recently, sometimes >> users plug a headset and select headset-mic from UI, but can't record >> any sound from headset-mic. The root cause is the >> determine_headset_type() returns a wrong type, e.g. users plug a ctia >> type headset, but that function returns omtp type. >> >> In the past, determine_headset_type() worked well because the internal >> mic is connected to the codec, so the "Input Source" or >> "Capture Source" is internal mic by default when users plug a headset, >> the determine_headset_type() will not be called unless users select >> headset-mic from UI, when users select headset-mic, the plugging >> action already finished and the headset is completely plugged into the >> jack, so determine_headset_type() could return a correct type. >> >> But more and more machines connect the internal mic to the PCH now, >> and the "Input Source" is headset mic by default, when users plug a >> headset, the determine_headset_type() will be called immediately, if >> the headset is not completely plugged in, it will return a wrong type. >> >> Here add 2s delay before calling determine_headset_type(), and since >> there is a pop-up dialogue when users do plugging action, to avoid >> freezing the UI, use the deleyed_work to call that function. > Hm, two seconds are quite long, IMHO. How is this delay determined? > > > Takashi Two seconds delay is for a latest Dell AIO machine, and this issue is exposed on that machine. The audio jack is designed to the left side of that AIO machine, users need to use left hand to plug a headset, and since it is an AIO, it is similar to plug sth to the left side on a monitor, users usually don't use the great force to plug otherwise they could introduce the movement of monitor; sometimes they use left hand to plug a headset, the headset is not 100% plugged in, meanwhile they need to put the right hand to the right side of the monitor to fix the monitor. All these actions make the plugging not finished as fast as on laptops or normal desktops. Our QA tested different delays, it has a pretty good chance to return the correct type after adding two seconds delay on that AIO machine. And I guess all Dell AIO machines will face the same issue since they all have multi-function audio jack. And I did a test on some Dell laptops, on which the internal mic connects to the PCH, if I plug the headset a bit slower than normal speed on purpose, the determine_headset_type() has some chance to return the wrong type and make the headset-mic can't record sound. Adding two seconds delay could make them work more stable. Thanks, Hui.
Will send the v2 patch, in the v2, only extend the delay time for a specific codec series instead of all codec series, and the delay time is not two seconds, it is only 850ms. And we found another issue about update_headset_mode() today, will send the fix in the v2 too. Thanks. Hui. On 3/17/21 10:15 AM, Hui Wang wrote: > > On 3/16/21 10:17 PM, Takashi Iwai wrote: >> On Tue, 16 Mar 2021 12:55:49 +0100, >> Hui Wang wrote: >>> We found a recording issue on the headset-mic recently, sometimes >>> users plug a headset and select headset-mic from UI, but can't record >>> any sound from headset-mic. The root cause is the >>> determine_headset_type() returns a wrong type, e.g. users plug a ctia >>> type headset, but that function returns omtp type. >>> >>> In the past, determine_headset_type() worked well because the internal >>> mic is connected to the codec, so the "Input Source" or >>> "Capture Source" is internal mic by default when users plug a headset, >>> the determine_headset_type() will not be called unless users select >>> headset-mic from UI, when users select headset-mic, the plugging >>> action already finished and the headset is completely plugged into the >>> jack, so determine_headset_type() could return a correct type. >>> >>> But more and more machines connect the internal mic to the PCH now, >>> and the "Input Source" is headset mic by default, when users plug a >>> headset, the determine_headset_type() will be called immediately, if >>> the headset is not completely plugged in, it will return a wrong type. >>> >>> Here add 2s delay before calling determine_headset_type(), and since >>> there is a pop-up dialogue when users do plugging action, to avoid >>> freezing the UI, use the deleyed_work to call that function. >> Hm, two seconds are quite long, IMHO. How is this delay determined? >> >> >> Takashi > > Two seconds delay is for a latest Dell AIO machine, and this issue is > exposed on that machine. The audio jack is designed to the left side > of that AIO machine, users need to use left hand to plug a headset, > and since it is an AIO, it is similar to plug sth to the left side on > a monitor, users usually don't use the great force to plug otherwise > they could introduce the movement of monitor; sometimes they use left > hand to plug a headset, the headset is not 100% plugged in, meanwhile > they need to put the right hand to the right side of the monitor to > fix the monitor. All these actions make the plugging not finished as > fast as on laptops or normal desktops. Our QA tested different delays, > it has a pretty good chance to return the correct type after adding > two seconds delay on that AIO machine. And I guess all Dell AIO > machines will face the same issue since they all have multi-function > audio jack. > > > And I did a test on some Dell laptops, on which the internal mic > connects to the PCH, if I plug the headset a bit slower than normal > speed on purpose, the determine_headset_type() has some chance to > return the wrong type and make the headset-mic can't record sound. > Adding two seconds delay could make them work more stable. > > > Thanks, > > Hui. > >
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index b47504fa8dfd..1f6fc8addf3e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -127,6 +127,8 @@ struct alc_spec { unsigned int coef0; struct input_dev *kb_dev; u8 alc_mute_keycode_map[1]; + struct hda_codec *codec; + struct delayed_work headset_set_work; }; /* @@ -1160,6 +1162,7 @@ static int alc_alloc_spec(struct hda_codec *codec, hda_nid_t mixer_nid) kfree(spec); return err; } + spec->codec = codec; return 0; } @@ -5363,6 +5366,21 @@ static void alc_determine_headset_type(struct hda_codec *codec) spec->current_headset_type = is_ctia ? ALC_HEADSET_TYPE_CTIA : ALC_HEADSET_TYPE_OMTP; } +static void alc_headset_check_and_set(struct work_struct *work) +{ + struct alc_spec *spec = container_of(work, struct alc_spec, + headset_set_work.work); + struct hda_codec *codec = spec->codec; + + if (spec->current_headset_type == ALC_HEADSET_TYPE_UNKNOWN) + alc_determine_headset_type(codec); + if (spec->current_headset_type == ALC_HEADSET_TYPE_CTIA) + alc_headset_mode_ctia(codec); + else if (spec->current_headset_type == ALC_HEADSET_TYPE_OMTP) + alc_headset_mode_omtp(codec); + spec->gen.hp_jack_present = true; +} + static void alc_update_headset_mode(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; @@ -5386,6 +5404,7 @@ static void alc_update_headset_mode(struct hda_codec *codec) return; } + cancel_delayed_work_sync(&spec->headset_set_work); switch (new_headset_mode) { case ALC_HEADSET_MODE_UNPLUGGED: alc_headset_mode_unplugged(codec); @@ -5394,13 +5413,7 @@ static void alc_update_headset_mode(struct hda_codec *codec) spec->gen.hp_jack_present = false; break; case ALC_HEADSET_MODE_HEADSET: - if (spec->current_headset_type == ALC_HEADSET_TYPE_UNKNOWN) - alc_determine_headset_type(codec); - if (spec->current_headset_type == ALC_HEADSET_TYPE_CTIA) - alc_headset_mode_ctia(codec); - else if (spec->current_headset_type == ALC_HEADSET_TYPE_OMTP) - alc_headset_mode_omtp(codec); - spec->gen.hp_jack_present = true; + schedule_delayed_work(&spec->headset_set_work, msecs_to_jiffies(2000)); break; case ALC_HEADSET_MODE_MIC: alc_headset_mode_mic_in(codec, hp_pin, spec->headphone_mic_pin); @@ -5466,6 +5479,7 @@ static void alc_fixup_headset_mode(struct hda_codec *codec, spec->parse_flags |= HDA_PINCFG_HEADSET_MIC | HDA_PINCFG_HEADPHONE_MIC; break; case HDA_FIXUP_ACT_PROBE: + INIT_DELAYED_WORK(&spec->headset_set_work, alc_headset_check_and_set); alc_probe_headset_mode(codec); break; case HDA_FIXUP_ACT_INIT: @@ -5475,6 +5489,9 @@ static void alc_fixup_headset_mode(struct hda_codec *codec, } alc_update_headset_mode(codec); break; + case HDA_FIXUP_ACT_FREE: + cancel_delayed_work_sync(&spec->headset_set_work); + break; } }
We found a recording issue on the headset-mic recently, sometimes users plug a headset and select headset-mic from UI, but can't record any sound from headset-mic. The root cause is the determine_headset_type() returns a wrong type, e.g. users plug a ctia type headset, but that function returns omtp type. In the past, determine_headset_type() worked well because the internal mic is connected to the codec, so the "Input Source" or "Capture Source" is internal mic by default when users plug a headset, the determine_headset_type() will not be called unless users select headset-mic from UI, when users select headset-mic, the plugging action already finished and the headset is completely plugged into the jack, so determine_headset_type() could return a correct type. But more and more machines connect the internal mic to the PCH now, and the "Input Source" is headset mic by default, when users plug a headset, the determine_headset_type() will be called immediately, if the headset is not completely plugged in, it will return a wrong type. Here add 2s delay before calling determine_headset_type(), and since there is a pop-up dialogue when users do plugging action, to avoid freezing the UI, use the deleyed_work to call that function. Signed-off-by: Hui Wang <hui.wang@canonical.com> --- sound/pci/hda/patch_realtek.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)