[alsa-devel] Via Codecs: functions mute_aa_path() and is_aa_path_mute() (file patch_via.c) seem to support different codecs.
These functions seem to support different codecs through different switch statements:
mute_aa_path:
... /* get nid of MW0 and start & end index */ switch (spec->codec_type) { case VT1708: nid_mixer = 0x17; start_idx = 2; end_idx = 4; break; case VT1709_10CH: case VT1709_6CH: nid_mixer = 0x18; start_idx = 2; end_idx = 4; break; case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: nid_mixer = 0x16; start_idx = 2; end_idx = 4; break; case VT1718S: nid_mixer = 0x21; start_idx = 1; end_idx = 3; break; default: return; }
is_aa_path_mute:
... /* get nid of MW0 and start & end index */ switch (spec->codec_type) { case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: nid_mixer = 0x16; start_idx = 2; end_idx = 4; break; case VT1702: nid_mixer = 0x1a; start_idx = 1; end_idx = 3; break; case VT1718S: nid_mixer = 0x21; start_idx = 1; end_idx = 3; break; case VT2002P: case VT1812: case VT1802: nid_mixer = 0x21; start_idx = 0; end_idx = 2; break; default: return 0; }
It would seem that AA-path can be muted but not inquired for mute state for codecs VT1708, VT1709_10CH and VT1709_6CH, vice-versa for codecs VT1702, VT2002P, VT1812, VT1802. If this is the real way such codecs work, I wonder if analog_input_switch_put() works as expected for codec VT1708, in which case, perhaps, it should be modified:
static int analog_input_switch_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { int change = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
set_widgets_power_state(codec); analog_low_current_mode(snd_kcontrol_chip(kcontrol), -1); if (snd_hda_get_bool_hint(codec, "analog_loopback_hp_detect") == 1) { if (is_aa_path_mute(codec)) vt1708_start_hp_work(codec->spec); else vt1708_stop_hp_work(codec->spec); } return change; }
Since is_aa_path_mute() always returns 0 for codec VT1708 (default case), and both vt1708_start_hp_work() and vt1708_stop_hp_work() only execute for such a codec (due to an if statement checking for codec type and returning from the function if that's not VT1708), vt1708_stop_hp_work() is the only one that can be executed, so perhaps the code could be clearer without the if statement checking is_aa_path_mute() result. Moreover:
static void vt1708_stop_hp_work(struct via_spec *spec) { if (spec->codec_type != VT1708 || spec->autocfg.hp_pins[0] == 0) return; if (snd_hda_get_bool_hint(spec->codec, "analog_loopback_hp_detect") == 1 && !is_aa_path_mute(spec->codec)) return; snd_hda_codec_write(spec->codec, 0x1, 0, 0xf81, !spec->vt1708_jack_detectect); cancel_delayed_work_sync(&spec->vt1708_hp_work); }
Since !is_aa_path_mute(spec->codec) is always true for VT1708, there is no real dependence on it in the if statement above, thus it should be removed. But this also means that, if snd_hda_get_bool_hint(spec->codec, "analog_loopback_hp_detect") == 1, then analog_input_switch_put() will call vt1708_stop_hp_work(), which in turn will return without performing any operation, so that it would seem useless, unless any reconfiguration might happen just in time between the first call to snd_hda_get_bool_hint in analog_input_switch_put() and the second call within vt1708_stop_hp_work().
Otherwise, if those switch statements should match against each other, they could be replaced with a call to a new inline function, so that a single switch could be moved within it and become easier to maintain. For instance:
static inline int get_supported_aa_mute_info( const struct via_spec * const spec, hda_nid_t *mixer, int *first_idx, int *last_idx ) { /* get correct nid of mixer and start & end index */ switch (spec->codec_type) { case VT1708: *mixer = 0x17; *first_idx = 2; *last_idx = 4; return 0; case VT1709_10CH: case VT1709_6CH: *mixer = 0x18; *first_idx = 2; *last_idx = 4; return 0; case VT1708B_8CH: case VT1708B_4CH: case VT1708S: case VT1716S: *mixer = 0x16; *first_idx = 2; *last_idx = 4; return 0; case VT1702: *mixer = 0x1a; *first_idx = 1; *last_idx = 3; return 0; case VT1718S: *mixer = 0x21; *first_idx = 1; *last_idx = 3; return 0; case VT2002P: case VT1812: case VT1802: *mixer = 0x21; *first_idx = 0; *last_idx = 2; return 0; default: return -EINVAL; } }
In mute_aa_path:
... if( get_supported_aa_mute_info( spec, &nid_mixer, &start_idx, &end_idx ) < 0 ) return;
In is_aa_path_mute:
... if( get_supported_aa_mute_info( spec, &nid_mixer, &start_idx, &end_idx ) < 0 ) return 0;
It happens that the list of codec types tested by is_aa_path_mute() is the same tested by function analog_low_current_mode(); however, there should be no dependence on that, because whatever is_aa_path_mute() returned for any other codec would be filtered out by the switch statement within analog_low_current_mode().
Regards.
participants (1)
-
alex dot baldacchino dot alsasub at gmail dot com