[alsa-devel] [PATCH 0/2] Conexant HD-audio fixes
Hi,
while debugging a problem reported by a Thinkpad user, we found that there are fundamental problems in the current Conexant auto-parser code.
1. EAPDs on older chips don't correspond to the pin as 1:1.
A single EAPD can influence on the whole outputs, so we can't turn it off simply even when the assigned pin isn't used.
2. There is only a single input-amp in audio-in widgets.
This is contradicting against what I was informed before. There is only one capture level in each audio-in widget no matter how many sources are connected to it. For example, if an ADC has the connection to an internal mic and an external mic, the volume in ADC is only one, no individual amp per connection, unlike other widgets.
For the first issue, a fix is just keep EAPD turned on for such chips. This simplifies the previous David's fix.
About the second part, I'm not 100% sure whether it's correct to all Conexant chips. But this sounds more natural, and I always wondered about the old implementation.
Can anyone confirm that the patches work properly? The initial test was done with a machine with cx20549, so tests with other codecs like cx20551, cx20561, and cx2058x would be appreciated.
After applying the patch, load snd-hda-intel with model=auto option. You'll have a single "Capture" volume (or two capture volumes like "Mic" and "Internal Mic", which correspond to different ADCs). Check the different recording source and whether the capture volume(s) really changes the recording level properly.
thanks,
Takashi
In the old Conexant chips (5045, 5047, 5051 and 5066), a single EAPD may handle both headphone and speaker outputs while it's assigned only to one of them. Turning off dynamically leads to the unexpected silent output in such a configuration with the auto-mute function.
Since it's difficult to know how the EAPD is handled in the actual h/w implementation, better to keep EAPD on while running for such codecs.
Cc: stable@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_conexant.c | 43 ++++++++++++++++++++------------------- 1 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index aa0e4b9..feef775 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -136,6 +136,7 @@ struct conexant_spec { unsigned int thinkpad:1; unsigned int hp_laptop:1; unsigned int asus:1; + unsigned int pin_eapd_ctrls:1;
unsigned int adc_switching:1;
@@ -3429,12 +3430,14 @@ static void cx_auto_turn_eapd(struct hda_codec *codec, int num_pins, static void do_automute(struct hda_codec *codec, int num_pins, hda_nid_t *pins, bool on) { + struct conexant_spec *spec = codec->spec; int i; for (i = 0; i < num_pins; i++) snd_hda_codec_write(codec, pins[i], 0, AC_VERB_SET_PIN_WIDGET_CONTROL, on ? PIN_OUT : 0); - cx_auto_turn_eapd(codec, num_pins, pins, on); + if (spec->pin_eapd_ctrls) + cx_auto_turn_eapd(codec, num_pins, pins, on); }
static int detect_jacks(struct hda_codec *codec, int num_pins, hda_nid_t *pins) @@ -3459,9 +3462,12 @@ static void cx_auto_update_speakers(struct hda_codec *codec) int on = 1;
/* turn on HP EAPD when HP jacks are present */ - if (spec->auto_mute) - on = spec->hp_present; - cx_auto_turn_eapd(codec, cfg->hp_outs, cfg->hp_pins, on); + if (spec->pin_eapd_ctrls) { + if (spec->auto_mute) + on = spec->hp_present; + cx_auto_turn_eapd(codec, cfg->hp_outs, cfg->hp_pins, on); + } + /* mute speakers in auto-mode if HP or LO jacks are plugged */ if (spec->auto_mute) on = !(spec->hp_present || @@ -3888,20 +3894,10 @@ static void cx_auto_parse_beep(struct hda_codec *codec) #define cx_auto_parse_beep(codec) #endif
-static bool found_in_nid_list(hda_nid_t nid, const hda_nid_t *list, int nums) -{ - int i; - for (i = 0; i < nums; i++) - if (list[i] == nid) - return true; - return false; -} - -/* parse extra-EAPD that aren't assigned to any pins */ +/* parse EAPDs */ static void cx_auto_parse_eapd(struct hda_codec *codec) { struct conexant_spec *spec = codec->spec; - struct auto_pin_cfg *cfg = &spec->autocfg; hda_nid_t nid, end_nid;
end_nid = codec->start_nid + codec->num_nodes; @@ -3910,14 +3906,18 @@ static void cx_auto_parse_eapd(struct hda_codec *codec) continue; if (!(snd_hda_query_pin_caps(codec, nid) & AC_PINCAP_EAPD)) continue; - if (found_in_nid_list(nid, cfg->line_out_pins, cfg->line_outs) || - found_in_nid_list(nid, cfg->hp_pins, cfg->hp_outs) || - found_in_nid_list(nid, cfg->speaker_pins, cfg->speaker_outs)) - continue; spec->eapds[spec->num_eapds++] = nid; if (spec->num_eapds >= ARRAY_SIZE(spec->eapds)) break; } + + /* NOTE: below is a wild guess; if we have more than two EAPDs, + * it's a new chip, where EAPDs are supposed to be associated to + * pins, and we can control EAPD per pin. + * OTOH, if only one or two EAPDs are found, it's an old chip, + * thus it might control over all pins. + */ + spec->pin_eapd_ctrls = spec->num_eapds > 2; }
static int cx_auto_parse_auto_config(struct hda_codec *codec) @@ -4023,8 +4023,9 @@ static void cx_auto_init_output(struct hda_codec *codec) } } cx_auto_update_speakers(codec); - /* turn on/off extra EAPDs, too */ - cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, true); + /* turn on all EAPDs if no individual EAPD control is available */ + if (!spec->pin_eapd_ctrls) + cx_auto_turn_eapd(codec, spec->num_eapds, spec->eapds, true); }
static void cx_auto_init_input(struct hda_codec *codec)
It seems that Conexant chips handle only a single input-amp even when the audio-input widget has multiple sources. This has been never clear, and I implemented in the current way based on the debug information I got at the early time -- the device reacts individual input-amp values for different sources. But it doesn't look correct now.
This patch changes the auto-parser code to handle a single input-amp per audio-in widget. After applying this, you'll see only a single "Capture" volume control instead of separate "Mic" or "Line" captures when the device is set up to use a single ADC.
Cc: stable@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_proc.c | 7 +++++-- sound/pci/hda/patch_conexant.c | 38 +++++++++++++++++++++++++++----------- 2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 2c981b5..9431369 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -646,12 +646,15 @@ static void print_codec_info(struct snd_info_entry *entry, HDA_MAX_CONNECTIONS);
if (wid_caps & AC_WCAP_IN_AMP) { + int nums = conn_len; snd_iprintf(buffer, " Amp-In caps: "); print_amp_caps(buffer, codec, nid, HDA_INPUT); snd_iprintf(buffer, " Amp-In vals: "); + if (wid_type == AC_WID_PIN || + (wid_type == AC_WID_AUD_IN && codec->pin_amp_workaround)) + nums = 1; print_amp_vals(buffer, codec, nid, HDA_INPUT, - wid_caps & AC_WCAP_STEREO, - wid_type == AC_WID_PIN ? 1 : conn_len); + wid_caps & AC_WCAP_STEREO, nums); } if (wid_caps & AC_WCAP_OUT_AMP) { snd_iprintf(buffer, " Amp-Out caps: "); diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index feef775..f2d4933 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -4251,16 +4251,20 @@ static int cx_auto_build_input_controls(struct hda_codec *codec) struct conexant_spec *spec = codec->spec; struct hda_input_mux *imux = &spec->private_imux; const char *prev_label; - int input_conn[HDA_MAX_NUM_INPUTS]; - int i, err, cidx; + int i, j, err, cidx; int multi_connection;
+ if (!imux->num_items) + return 0; + multi_connection = 0; for (i = 0; i < imux->num_items; i++) { cidx = get_input_connection(codec, spec->imux_info[i].adc, spec->imux_info[i].pin); - input_conn[i] = (spec->imux_info[i].adc << 8) | cidx; - if (i > 0 && input_conn[i] != input_conn[0]) + if (cidx < 0) + continue; + if (i > 0 && !multi_connection && + spec->imux_info[i].adc != spec->imux_info[0].adc) multi_connection = 1; }
@@ -4282,15 +4286,27 @@ static int cx_auto_build_input_controls(struct hda_codec *codec) if (err < 0) return err;
- if (!multi_connection) { - if (i > 0) + if (multi_connection) { + bool found = false; + for (j = 0; j < i; j++) { + if (spec->imux_info[j].adc == spec->imux_info[i].adc) { + found = true; + break; + } + } + if (found) continue; - err = cx_auto_add_capture_volume(codec, nid, - "Capture", "", cidx); - } else { - err = cx_auto_add_capture_volume(codec, nid, - label, " Capture", cidx); + err = cx_auto_add_capture_volume(codec, nid, label, + " Capture", cidx); + if (err < 0) + return err; } + } + + if (!multi_connection) { + err = cx_auto_add_volume_idx(codec, "Capture", "", 0, + spec->imux_info[0].adc, + HDA_INPUT, 0); if (err < 0) return err; }
participants (1)
-
Takashi Iwai