[alsa-devel] [PATCH 0/5] Realtek auto-mic enhancements / cleanups
Hi,
this is a series of patches to enhance and clean up the automatic input selection via mic jack on HD-audio Realtek drivers.
[PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_* [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek [PATCH 3/5] ALSA: hda - Use standard sort function in [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP [PATCH 5/5] ALSA: hda - Refactor alc_kcontrol_new() usages
The first motivation was to generalize the code a bit better.
Then, I got a machine that should be able to switch front mic and the secondary headphone. The machine is suitable for the new auto-mic test, so I included this hack in 4th path as well.
Takashi
Put INPUT_PIN_ATTR_FRONT after INPUT_PIN_ATTR_REAR, and define INPUT_PIN_ATTR_LAST to point to the last element.
This is a preliminary work for cleaning up Realtek auto-mic parser.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_auto_parser.c | 2 +- sound/pci/hda/hda_auto_parser.h | 3 ++- sound/pci/hda/patch_via.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 4ec6dc8..90d5b39 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -375,7 +375,7 @@ static const char *hda_get_input_pin_label(struct hda_codec *codec, { unsigned int def_conf; static const char * const mic_names[] = { - "Internal Mic", "Dock Mic", "Mic", "Front Mic", "Rear Mic", + "Internal Mic", "Dock Mic", "Mic", "Rear Mic", "Front Mic" }; int attr;
diff --git a/sound/pci/hda/hda_auto_parser.h b/sound/pci/hda/hda_auto_parser.h index 632ad0a..b7d7103 100644 --- a/sound/pci/hda/hda_auto_parser.h +++ b/sound/pci/hda/hda_auto_parser.h @@ -51,8 +51,9 @@ enum { INPUT_PIN_ATTR_INT, /* internal mic/line-in */ INPUT_PIN_ATTR_DOCK, /* docking mic/line-in */ INPUT_PIN_ATTR_NORMAL, /* mic/line-in jack */ - INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */ INPUT_PIN_ATTR_REAR, /* mic/line-in jack in rear */ + INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */ + INPUT_PIN_ATTR_LAST = INPUT_PIN_ATTR_FRONT, };
int snd_hda_get_input_pin_attr(unsigned int def_conf); diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 274644f..9f04f33 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -1894,7 +1894,7 @@ static void mangle_smart51(struct hda_codec *codec) int i, j, nums, attr; int pins[AUTO_CFG_MAX_INS];
- for (attr = INPUT_PIN_ATTR_REAR; attr >= INPUT_PIN_ATTR_NORMAL; attr--) { + for (attr = INPUT_PIN_ATTR_LAST; attr >= INPUT_PIN_ATTR_NORMAL; attr--) { nums = 0; for (i = 0; i < cfg->num_inputs; i++) { unsigned int def;
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Put INPUT_PIN_ATTR_FRONT after INPUT_PIN_ATTR_REAR, and define INPUT_PIN_ATTR_LAST to point to the last element.
This is a preliminary work for cleaning up Realtek auto-mic parser.
What practical effect does switching "Rear Mic" and "Front Mic" actually have?
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/hda_auto_parser.c | 2 +- sound/pci/hda/hda_auto_parser.h | 3 ++- sound/pci/hda/patch_via.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 4ec6dc8..90d5b39 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -375,7 +375,7 @@ static const char *hda_get_input_pin_label(struct hda_codec *codec, { unsigned int def_conf; static const char * const mic_names[] = {
"Internal Mic", "Dock Mic", "Mic", "Front Mic", "Rear Mic",
"Internal Mic", "Dock Mic", "Mic", "Rear Mic", "Front Mic"diff --git a/sound/pci/hda/hda_auto_parser.h b/sound/pci/hda/hda_auto_parser.h index 632ad0a..b7d7103 100644 --- a/sound/pci/hda/hda_auto_parser.h +++ b/sound/pci/hda/hda_auto_parser.h @@ -51,8 +51,9 @@ enum { INPUT_PIN_ATTR_INT, /* internal mic/line-in */ INPUT_PIN_ATTR_DOCK, /* docking mic/line-in */ INPUT_PIN_ATTR_NORMAL, /* mic/line-in jack */
- INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */ INPUT_PIN_ATTR_REAR, /* mic/line-in jack in rear */
INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */
INPUT_PIN_ATTR_LAST = INPUT_PIN_ATTR_FRONT, };
int snd_hda_get_input_pin_attr(unsigned int def_conf);
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 274644f..9f04f33 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -1894,7 +1894,7 @@ static void mangle_smart51(struct hda_codec *codec) int i, j, nums, attr; int pins[AUTO_CFG_MAX_INS];
- for (attr = INPUT_PIN_ATTR_REAR; attr >= INPUT_PIN_ATTR_NORMAL; attr--) {
- for (attr = INPUT_PIN_ATTR_LAST; attr >= INPUT_PIN_ATTR_NORMAL; attr--) { nums = 0; for (i = 0; i < cfg->num_inputs; i++) { unsigned int def;
At Fri, 30 Nov 2012 00:54:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Put INPUT_PIN_ATTR_FRONT after INPUT_PIN_ATTR_REAR, and define INPUT_PIN_ATTR_LAST to point to the last element.
This is a preliminary work for cleaning up Realtek auto-mic parser.
What practical effect does switching "Rear Mic" and "Front Mic" actually have?
For making the front jack in a higher priority than the rear jack. This logic is used in the later patch in the series.
Takashi
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/hda_auto_parser.c | 2 +- sound/pci/hda/hda_auto_parser.h | 3 ++- sound/pci/hda/patch_via.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 4ec6dc8..90d5b39 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -375,7 +375,7 @@ static const char *hda_get_input_pin_label(struct hda_codec *codec, { unsigned int def_conf; static const char * const mic_names[] = {
"Internal Mic", "Dock Mic", "Mic", "Front Mic", "Rear Mic",
"Internal Mic", "Dock Mic", "Mic", "Rear Mic", "Front Mic"diff --git a/sound/pci/hda/hda_auto_parser.h b/sound/pci/hda/hda_auto_parser.h index 632ad0a..b7d7103 100644 --- a/sound/pci/hda/hda_auto_parser.h +++ b/sound/pci/hda/hda_auto_parser.h @@ -51,8 +51,9 @@ enum { INPUT_PIN_ATTR_INT, /* internal mic/line-in */ INPUT_PIN_ATTR_DOCK, /* docking mic/line-in */ INPUT_PIN_ATTR_NORMAL, /* mic/line-in jack */
- INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */ INPUT_PIN_ATTR_REAR, /* mic/line-in jack in rear */
INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */
INPUT_PIN_ATTR_LAST = INPUT_PIN_ATTR_FRONT, };
int snd_hda_get_input_pin_attr(unsigned int def_conf);
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 274644f..9f04f33 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -1894,7 +1894,7 @@ static void mangle_smart51(struct hda_codec *codec) int i, j, nums, attr; int pins[AUTO_CFG_MAX_INS];
- for (attr = INPUT_PIN_ATTR_REAR; attr >= INPUT_PIN_ATTR_NORMAL; attr--) {
- for (attr = INPUT_PIN_ATTR_LAST; attr >= INPUT_PIN_ATTR_NORMAL; attr--) { nums = 0; for (i = 0; i < cfg->num_inputs; i++) { unsigned int def;
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 11/30/2012 08:03 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 00:54:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Put INPUT_PIN_ATTR_FRONT after INPUT_PIN_ATTR_REAR, and define INPUT_PIN_ATTR_LAST to point to the last element.
This is a preliminary work for cleaning up Realtek auto-mic parser.
What practical effect does switching "Rear Mic" and "Front Mic" actually have?
For making the front jack in a higher priority than the rear jack. This logic is used in the later patch in the series.
Ok, then including that in the commit message, and maybe also something about that the order matters for priority as a code comment, would probably be a good improvement to this patch.
Takashi
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/hda_auto_parser.c | 2 +- sound/pci/hda/hda_auto_parser.h | 3 ++- sound/pci/hda/patch_via.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 4ec6dc8..90d5b39 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -375,7 +375,7 @@ static const char *hda_get_input_pin_label(struct hda_codec *codec, { unsigned int def_conf; static const char * const mic_names[] = {
"Internal Mic", "Dock Mic", "Mic", "Front Mic", "Rear Mic",
"Internal Mic", "Dock Mic", "Mic", "Rear Mic", "Front Mic"diff --git a/sound/pci/hda/hda_auto_parser.h b/sound/pci/hda/hda_auto_parser.h index 632ad0a..b7d7103 100644 --- a/sound/pci/hda/hda_auto_parser.h +++ b/sound/pci/hda/hda_auto_parser.h @@ -51,8 +51,9 @@ enum { INPUT_PIN_ATTR_INT, /* internal mic/line-in */ INPUT_PIN_ATTR_DOCK, /* docking mic/line-in */ INPUT_PIN_ATTR_NORMAL, /* mic/line-in jack */
- INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */ INPUT_PIN_ATTR_REAR, /* mic/line-in jack in rear */
INPUT_PIN_ATTR_FRONT, /* mic/line-in jack in front */
INPUT_PIN_ATTR_LAST = INPUT_PIN_ATTR_FRONT, };
int snd_hda_get_input_pin_attr(unsigned int def_conf);
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c index 274644f..9f04f33 100644 --- a/sound/pci/hda/patch_via.c +++ b/sound/pci/hda/patch_via.c @@ -1894,7 +1894,7 @@ static void mangle_smart51(struct hda_codec *codec) int i, j, nums, attr; int pins[AUTO_CFG_MAX_INS];
- for (attr = INPUT_PIN_ATTR_REAR; attr >= INPUT_PIN_ATTR_NORMAL; attr--) {
- for (attr = INPUT_PIN_ATTR_LAST; attr >= INPUT_PIN_ATTR_NORMAL; attr--) { nums = 0; for (i = 0; i < cfg->num_inputs; i++) { unsigned int def;
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
It's been a frequent demand from desktop machines, and we already do it for the headphone auto-mute, so no reason to avoid it now.
The logic is to check the attribute and location of input pins, and enable the automatic selection feature only if all such pins are in different locations (e.g. internal, front, rear, etc) and line-in or mic pins. That is, if multiple input pins are assigned to a single location, the feature isn't enabled because we don't know the priority.
(You may wonder why this restriction doesn't exist for the headphone automute. The reason is that the output case is different from the input: the input source is an exclusive selection while the output can be multiplexed.)
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 137 +++++++++++++++++++++++------------------- 1 file changed, 74 insertions(+), 63 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c7369a6..c5cc47b 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -28,6 +28,7 @@ #include <linux/slab.h> #include <linux/pci.h> #include <linux/module.h> +#include <linux/sort.h> #include <sound/core.h> #include <sound/jack.h> #include "hda_codec.h" @@ -98,6 +99,13 @@ enum { #define ALC_FIXUP_ACT_INIT HDA_FIXUP_ACT_INIT #define ALC_FIXUP_ACT_BUILD HDA_FIXUP_ACT_BUILD
+#define MAX_AUTO_MIC_PINS 3 + +struct alc_automic_entry { + hda_nid_t pin; /* pin */ + int idx; /* imux index, -1 = invalid */ + unsigned int attr; /* pin attribute (INPUT_PIN_ATTR_*) */ +};
struct alc_spec { struct hda_gen_spec gen; @@ -145,9 +153,6 @@ struct alc_spec { unsigned int num_mux_defs; const struct hda_input_mux *input_mux; unsigned int cur_mux[3]; - hda_nid_t ext_mic_pin; - hda_nid_t dock_mic_pin; - hda_nid_t int_mic_pin;
/* channel model */ const struct hda_channel_mode *channel_mode; @@ -169,9 +174,12 @@ struct alc_spec { hda_nid_t private_capsrc_nids[AUTO_CFG_MAX_OUTS]; hda_nid_t imux_pins[HDA_MAX_NUM_INPUTS]; unsigned int dyn_adc_idx[HDA_MAX_NUM_INPUTS]; - int int_mic_idx, ext_mic_idx, dock_mic_idx; /* for auto-mic */ hda_nid_t inv_dmic_pin;
+ /* auto-mic stuff */ + int am_num_entries; + struct alc_automic_entry am_entry[MAX_AUTO_MIC_PINS]; + /* hooks */ void (*init_hook)(struct hda_codec *codec); #ifdef CONFIG_PM @@ -632,22 +640,20 @@ static void alc_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jack static void alc_mic_automute(struct hda_codec *codec, struct hda_jack_tbl *jack) { struct alc_spec *spec = codec->spec; - hda_nid_t *pins = spec->imux_pins; + int i;
if (!spec->auto_mic || !spec->auto_mic_valid_imux) return; if (snd_BUG_ON(!spec->adc_nids)) return; - if (snd_BUG_ON(spec->int_mic_idx < 0 || spec->ext_mic_idx < 0)) - return;
- if (snd_hda_jack_detect(codec, pins[spec->ext_mic_idx])) - alc_mux_select(codec, 0, spec->ext_mic_idx, false); - else if (spec->dock_mic_idx >= 0 && - snd_hda_jack_detect(codec, pins[spec->dock_mic_idx])) - alc_mux_select(codec, 0, spec->dock_mic_idx, false); - else - alc_mux_select(codec, 0, spec->int_mic_idx, false); + for (i = spec->am_num_entries - 1; i > 0; i--) { + if (snd_hda_jack_detect(codec, spec->am_entry[i].pin)) { + alc_mux_select(codec, 0, spec->am_entry[i].idx, false); + return; + } + } + alc_mux_select(codec, 0, spec->am_entry[0].idx, false); }
/* update the master volume per volume-knob's unsol event */ @@ -1053,6 +1059,7 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; const struct hda_input_mux *imux; + int i;
if (!spec->auto_mic) return false; @@ -1066,21 +1073,20 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) }
imux = spec->input_mux; - spec->ext_mic_idx = find_idx_in_nid_list(spec->ext_mic_pin, - spec->imux_pins, imux->num_items); - spec->int_mic_idx = find_idx_in_nid_list(spec->int_mic_pin, - spec->imux_pins, imux->num_items); - spec->dock_mic_idx = find_idx_in_nid_list(spec->dock_mic_pin, - spec->imux_pins, imux->num_items); - if (spec->ext_mic_idx < 0 || spec->int_mic_idx < 0) { - spec->auto_mic = 0; - return false; /* no corresponding imux */ + for (i = 0; i < spec->am_num_entries; i++) { + spec->am_entry[i].idx = + find_idx_in_nid_list(spec->am_entry[i].pin, + spec->imux_pins, imux->num_items); + if (spec->am_entry[i].idx < 0) { + spec->auto_mic = 0; + return false; /* no corresponding imux */ + } }
- snd_hda_jack_detect_enable_callback(codec, spec->ext_mic_pin, - ALC_MIC_EVENT, alc_mic_automute); - if (spec->dock_mic_pin) - snd_hda_jack_detect_enable_callback(codec, spec->dock_mic_pin, + /* we don't need the jack detection for the first pin */ + for (i = 1; i < spec->am_num_entries; i++) + snd_hda_jack_detect_enable_callback(codec, + spec->am_entry[i].pin, ALC_MIC_EVENT, alc_mic_automute);
@@ -1089,6 +1095,13 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) return true; }
+static int compare_attr(const void *ap, const void *bp) +{ + const struct alc_automic_entry *a = ap; + const struct alc_automic_entry *b = bp; + return (int)(a->attr - b->attr); +} + /* * Check the availability of auto-mic switch; * Set up if really supported @@ -1097,67 +1110,63 @@ static void alc_init_auto_mic(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; struct auto_pin_cfg *cfg = &spec->autocfg; - hda_nid_t fixed, ext, dock; - int i; + unsigned int types; + int i, num_pins;
if (spec->shared_mic_hp) return; /* no auto-mic for the shared I/O */
- spec->ext_mic_idx = spec->int_mic_idx = spec->dock_mic_idx = -1; + types = 0; + num_pins = 0;
- fixed = ext = dock = 0; for (i = 0; i < cfg->num_inputs; i++) { hda_nid_t nid = cfg->inputs[i].pin; - unsigned int defcfg; - defcfg = snd_hda_codec_get_pincfg(codec, nid); - switch (snd_hda_get_input_pin_attr(defcfg)) { + unsigned int attr; + attr = snd_hda_codec_get_pincfg(codec, nid); + attr = snd_hda_get_input_pin_attr(attr); + if (types & (1 << attr)) + return; /* already occupied */ + switch (attr) { case INPUT_PIN_ATTR_INT: - if (fixed) - return; /* already occupied */ if (cfg->inputs[i].type != AUTO_PIN_MIC) return; /* invalid type */ - fixed = nid; break; case INPUT_PIN_ATTR_UNUSED: return; /* invalid entry */ - case INPUT_PIN_ATTR_DOCK: - if (dock) - return; /* already occupied */ - if (cfg->inputs[i].type > AUTO_PIN_LINE_IN) - return; /* invalid type */ - dock = nid; - break; default: - if (ext) - return; /* already occupied */ - if (cfg->inputs[i].type != AUTO_PIN_MIC) + if (cfg->inputs[i].type > AUTO_PIN_LINE_IN) return; /* invalid type */ - ext = nid; + if (!is_jack_detectable(codec, nid)) + return; /* no unsol support */ break; } + if (num_pins >= MAX_AUTO_MIC_PINS) + return; + types |= (1 << attr); + spec->am_entry[num_pins].pin = nid; + spec->am_entry[num_pins].attr = attr; + num_pins++; } - if (!ext && dock) { - ext = dock; - dock = 0; - } - if (!ext || !fixed) + + if (num_pins < 2) return; - if (!is_jack_detectable(codec, ext)) - return; /* no unsol support */ - if (dock && !is_jack_detectable(codec, dock)) - return; /* no unsol support */
- /* check imux indices */ - spec->ext_mic_pin = ext; - spec->int_mic_pin = fixed; - spec->dock_mic_pin = dock; + spec->am_num_entries = num_pins; + /* sort the am_entry in the order of attr so that the pin with a + * higher attr will be selected when the jack is plugged. + */ + sort(spec->am_entry, num_pins, sizeof(spec->am_entry[0]), + compare_attr, NULL);
+ /* check imux indices */ spec->auto_mic = 1; if (!alc_auto_mic_check_imux(codec)) return;
snd_printdd("realtek: Enable auto-mic switch on NID 0x%x/0x%x/0x%x\n", - ext, fixed, dock); + spec->am_entry[0].pin, + spec->am_entry[1].pin, + spec->am_entry[2].pin); }
/* check the availabilities of auto-mute and auto-mic switches */ @@ -6019,8 +6028,10 @@ static void alc271_hp_gate_mic_jack(struct hda_codec *codec, { struct alc_spec *spec = codec->spec;
+ if (snd_BUG_ON(!spec->am_entry[1].pin || !spec->autocfg.hp_pins[0])) + return; if (action == ALC_FIXUP_ACT_PROBE) - snd_hda_jack_set_gating_jack(codec, spec->ext_mic_pin, + snd_hda_jack_set_gating_jack(codec, spec->am_entry[1].pin, spec->autocfg.hp_pins[0]); }
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
It's been a frequent demand from desktop machines, and we already do it for the headphone auto-mute, so no reason to avoid it now.
The logic is to check the attribute and location of input pins, and enable the automatic selection feature only if all such pins are in different locations (e.g. internal, front, rear, etc) and line-in or mic pins. That is, if multiple input pins are assigned to a single location, the feature isn't enabled because we don't know the priority.
Hmm, I'm a bit worried about this, partially because it makes things inconsistent between codecs (and kernel versions, too).
In current PulseAudio, we disable the internal mic if any other mic is being detected as plugged in. But if there are other switches, say "front mic" and "rear mic", we don't do such disabling. Now you're disabling one of them automatically in the kernel, but for one codec only.
So my point is, when both "Front mic" and "Rear mic" are plugged in, we don't know whether to present to the user to choose between "Front Mic" and "Rear Mic", or to disable one of them.
One could possibly argue that the absence of a "Input Source" and "Capture Source" would indicate some form of auto-switch, but I wonder if this would really be a safe indicator...
(You may wonder why this restriction doesn't exist for the headphone automute. The reason is that the output case is different from the input: the input source is an exclusive selection while the output can be multiplexed.)
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_realtek.c | 137 +++++++++++++++++++++++------------------- 1 file changed, 74 insertions(+), 63 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c7369a6..c5cc47b 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -28,6 +28,7 @@ #include <linux/slab.h> #include <linux/pci.h> #include <linux/module.h> +#include <linux/sort.h> #include <sound/core.h> #include <sound/jack.h> #include "hda_codec.h" @@ -98,6 +99,13 @@ enum { #define ALC_FIXUP_ACT_INIT HDA_FIXUP_ACT_INIT #define ALC_FIXUP_ACT_BUILD HDA_FIXUP_ACT_BUILD
+#define MAX_AUTO_MIC_PINS 3
+struct alc_automic_entry {
- hda_nid_t pin; /* pin */
- int idx; /* imux index, -1 = invalid */
- unsigned int attr; /* pin attribute (INPUT_PIN_ATTR_*) */
+};
struct alc_spec { struct hda_gen_spec gen; @@ -145,9 +153,6 @@ struct alc_spec { unsigned int num_mux_defs; const struct hda_input_mux *input_mux; unsigned int cur_mux[3];
hda_nid_t ext_mic_pin;
hda_nid_t dock_mic_pin;
hda_nid_t int_mic_pin;
/* channel model */ const struct hda_channel_mode *channel_mode;
@@ -169,9 +174,12 @@ struct alc_spec { hda_nid_t private_capsrc_nids[AUTO_CFG_MAX_OUTS]; hda_nid_t imux_pins[HDA_MAX_NUM_INPUTS]; unsigned int dyn_adc_idx[HDA_MAX_NUM_INPUTS];
- int int_mic_idx, ext_mic_idx, dock_mic_idx; /* for auto-mic */ hda_nid_t inv_dmic_pin;
- /* auto-mic stuff */
- int am_num_entries;
- struct alc_automic_entry am_entry[MAX_AUTO_MIC_PINS];
- /* hooks */ void (*init_hook)(struct hda_codec *codec); #ifdef CONFIG_PM
@@ -632,22 +640,20 @@ static void alc_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jack static void alc_mic_automute(struct hda_codec *codec, struct hda_jack_tbl *jack) { struct alc_spec *spec = codec->spec;
- hda_nid_t *pins = spec->imux_pins;
int i;
if (!spec->auto_mic || !spec->auto_mic_valid_imux) return; if (snd_BUG_ON(!spec->adc_nids)) return;
if (snd_BUG_ON(spec->int_mic_idx < 0 || spec->ext_mic_idx < 0))
return;if (snd_hda_jack_detect(codec, pins[spec->ext_mic_idx]))
alc_mux_select(codec, 0, spec->ext_mic_idx, false);else if (spec->dock_mic_idx >= 0 &&
snd_hda_jack_detect(codec, pins[spec->dock_mic_idx]))alc_mux_select(codec, 0, spec->dock_mic_idx, false);else
alc_mux_select(codec, 0, spec->int_mic_idx, false);
for (i = spec->am_num_entries - 1; i > 0; i--) {
if (snd_hda_jack_detect(codec, spec->am_entry[i].pin)) {alc_mux_select(codec, 0, spec->am_entry[i].idx, false);return;}}
alc_mux_select(codec, 0, spec->am_entry[0].idx, false); }
/* update the master volume per volume-knob's unsol event */
@@ -1053,6 +1059,7 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; const struct hda_input_mux *imux;
int i;
if (!spec->auto_mic) return false;
@@ -1066,21 +1073,20 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) }
imux = spec->input_mux;
- spec->ext_mic_idx = find_idx_in_nid_list(spec->ext_mic_pin,
spec->imux_pins, imux->num_items);- spec->int_mic_idx = find_idx_in_nid_list(spec->int_mic_pin,
spec->imux_pins, imux->num_items);- spec->dock_mic_idx = find_idx_in_nid_list(spec->dock_mic_pin,
spec->imux_pins, imux->num_items);- if (spec->ext_mic_idx < 0 || spec->int_mic_idx < 0) {
spec->auto_mic = 0;return false; /* no corresponding imux */
- for (i = 0; i < spec->am_num_entries; i++) {
spec->am_entry[i].idx =find_idx_in_nid_list(spec->am_entry[i].pin,spec->imux_pins, imux->num_items);if (spec->am_entry[i].idx < 0) {spec->auto_mic = 0;return false; /* no corresponding imux */}
- snd_hda_jack_detect_enable_callback(codec, spec->ext_mic_pin,
ALC_MIC_EVENT, alc_mic_automute);- if (spec->dock_mic_pin)
snd_hda_jack_detect_enable_callback(codec, spec->dock_mic_pin,
- /* we don't need the jack detection for the first pin */
- for (i = 1; i < spec->am_num_entries; i++)
snd_hda_jack_detect_enable_callback(codec,spec->am_entry[i].pin, ALC_MIC_EVENT, alc_mic_automute);@@ -1089,6 +1095,13 @@ static bool alc_auto_mic_check_imux(struct hda_codec *codec) return true; }
+static int compare_attr(const void *ap, const void *bp) +{
- const struct alc_automic_entry *a = ap;
- const struct alc_automic_entry *b = bp;
- return (int)(a->attr - b->attr);
+}
- /*
- Check the availability of auto-mic switch;
- Set up if really supported
@@ -1097,67 +1110,63 @@ static void alc_init_auto_mic(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; struct auto_pin_cfg *cfg = &spec->autocfg;
- hda_nid_t fixed, ext, dock;
- int i;
unsigned int types;
int i, num_pins;
if (spec->shared_mic_hp) return; /* no auto-mic for the shared I/O */
- spec->ext_mic_idx = spec->int_mic_idx = spec->dock_mic_idx = -1;
- types = 0;
- num_pins = 0;
- fixed = ext = dock = 0; for (i = 0; i < cfg->num_inputs; i++) { hda_nid_t nid = cfg->inputs[i].pin;
unsigned int defcfg;defcfg = snd_hda_codec_get_pincfg(codec, nid);switch (snd_hda_get_input_pin_attr(defcfg)) {
unsigned int attr;attr = snd_hda_codec_get_pincfg(codec, nid);attr = snd_hda_get_input_pin_attr(attr);if (types & (1 << attr))return; /* already occupied */switch (attr) {
if (fixed)return; /* already occupied */ if (cfg->inputs[i].type != AUTO_PIN_MIC) return; /* invalid type */fixed = nid; break;case INPUT_PIN_ATTR_DOCK:if (dock)return; /* already occupied */if (cfg->inputs[i].type > AUTO_PIN_LINE_IN)return; /* invalid type */dock = nid;break;if (ext)return; /* already occupied */if (cfg->inputs[i].type != AUTO_PIN_MIC)
if (cfg->inputs[i].type > AUTO_PIN_LINE_IN) return; /* invalid type */
ext = nid;
if (!is_jack_detectable(codec, nid))return; /* no unsol support */ break;if (num_pins >= MAX_AUTO_MIC_PINS)return;types |= (1 << attr);spec->am_entry[num_pins].pin = nid;spec->am_entry[num_pins].attr = attr;num_pins++;
- if (!ext && dock) {
ext = dock;dock = 0;- }
- if (!ext || !fixed)
- if (num_pins < 2) return;
if (!is_jack_detectable(codec, ext))
return; /* no unsol support */if (dock && !is_jack_detectable(codec, dock))
return; /* no unsol support *//* check imux indices */
spec->ext_mic_pin = ext;
spec->int_mic_pin = fixed;
spec->dock_mic_pin = dock;
spec->am_num_entries = num_pins;
/* sort the am_entry in the order of attr so that the pin with a
* higher attr will be selected when the jack is plugged.*/sort(spec->am_entry, num_pins, sizeof(spec->am_entry[0]),
compare_attr, NULL);/* check imux indices */ spec->auto_mic = 1; if (!alc_auto_mic_check_imux(codec)) return;
snd_printdd("realtek: Enable auto-mic switch on NID 0x%x/0x%x/0x%x\n",
ext, fixed, dock);
spec->am_entry[0].pin,spec->am_entry[1].pin,spec->am_entry[2].pin);}
/* check the availabilities of auto-mute and auto-mic switches */
@@ -6019,8 +6028,10 @@ static void alc271_hp_gate_mic_jack(struct hda_codec *codec, { struct alc_spec *spec = codec->spec;
- if (snd_BUG_ON(!spec->am_entry[1].pin || !spec->autocfg.hp_pins[0]))
return;
snd_hda_jack_set_gating_jack(codec, spec->ext_mic_pin,
snd_hda_jack_set_gating_jack(codec, spec->am_entry[1].pin, spec->autocfg.hp_pins[0]);
At Fri, 30 Nov 2012 01:11:51 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
It's been a frequent demand from desktop machines, and we already do it for the headphone auto-mute, so no reason to avoid it now.
The logic is to check the attribute and location of input pins, and enable the automatic selection feature only if all such pins are in different locations (e.g. internal, front, rear, etc) and line-in or mic pins. That is, if multiple input pins are assigned to a single location, the feature isn't enabled because we don't know the priority.
Hmm, I'm a bit worried about this, partially because it makes things inconsistent between codecs (and kernel versions, too).
The behavior change is an open question, yes. If we'd like to be conservative, auto_mic can be set to 0 except for non-laptop cases.
In current PulseAudio, we disable the internal mic if any other mic is being detected as plugged in. But if there are other switches, say "front mic" and "rear mic", we don't do such disabling. Now you're disabling one of them automatically in the kernel, but for one codec only.
Porting this to other codecs shouldn't be too hard.
So my point is, when both "Front mic" and "Rear mic" are plugged in, we don't know whether to present to the user to choose between "Front Mic" and "Rear Mic", or to disable one of them.
The input is exclusive, so one of them is chosen. It's the front one. This is the same logic as we have for the headphone. Practically seen, the headphone auto-mute over line-out is working just because headphone jacks are implemented in only two ways: a headphone jack on a laptop or a headphone jack on a front panel. Thus, we do "choose the front" implicitly by the auto-mute feature.
And, the implementation for the generic auto-mic follows this logic. Prefer front to rear.
One could possibly argue that the absence of a "Input Source" and "Capture Source" would indicate some form of auto-switch, but I wonder if this would really be a safe indicator...
Heh, actually I expected this kind of response. A few more patches I'm going to post will add "Capture Source" back even for auto-mic mode, in addition to a new "Auto-Mic Mode" enum switch. In that way, you'll have almost full controls.
thanks,
Takashi
On 11/30/2012 08:17 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:11:51 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
It's been a frequent demand from desktop machines, and we already do it for the headphone auto-mute, so no reason to avoid it now.
The logic is to check the attribute and location of input pins, and enable the automatic selection feature only if all such pins are in different locations (e.g. internal, front, rear, etc) and line-in or mic pins. That is, if multiple input pins are assigned to a single location, the feature isn't enabled because we don't know the priority.
Hmm, I'm a bit worried about this, partially because it makes things inconsistent between codecs (and kernel versions, too).
The behavior change is an open question, yes. If we'd like to be conservative, auto_mic can be set to 0 except for non-laptop cases.
I'm not sure which one is better, really. If Windows/Mac does it in a specific way then maybe people are used to that and want it to work that way with Linux too. Maybe I should ask some design people about it...
In current PulseAudio, we disable the internal mic if any other mic is being detected as plugged in. But if there are other switches, say "front mic" and "rear mic", we don't do such disabling. Now you're disabling one of them automatically in the kernel, but for one codec only.
Porting this to other codecs shouldn't be too hard.
So my point is, when both "Front mic" and "Rear mic" are plugged in, we don't know whether to present to the user to choose between "Front Mic" and "Rear Mic", or to disable one of them.
The input is exclusive, so one of them is chosen. It's the front one.
Right, but this goes only for Realtek (given the current set of patches).
But thinking some more about it, I guess we could implement the same priority order in PulseAudio, so that the mic will autoswitch regardless of codec. That would ensure a consistent experience between codecs, and would be easier (and better) than trying to detect if the current codec is a Realtek or not.
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
the problem of addng line in jack to auto mic is line in does not has any boost (gain) but most mic have a boost control
the other problem is those acer aspire 8930g notebook has internal 5.1 speakers and three jacks which can support external 5.1 speakers
it seem the the channel mode control limit the max channels to 2 when there is internal 5.1 speaker
when channel mode is 6ch the mic jack is retasked as output and cannot be used
https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/943386
At Sun, 2 Dec 2012 09:55:56 +0800, Raymond Yau wrote:
Instead of limiting the automatic input-source selection only to the mics (internal, external and dock mics), allow it for generic inputs, e.g. switching between the rear line-in and the front mic.
the problem of addng line in jack to auto mic is line in does not has any boost (gain) but most mic have a boost control
I think it's pretty irrelevant. The auto-mic switches only the I/O direction.
the other problem is those acer aspire 8930g notebook has internal 5.1 speakers and three jacks which can support external 5.1 speakers
it seem the the channel mode control limit the max channels to 2 when there is internal 5.1 speaker
Again it's utterly irrelevant.
Takashi
when channel mode is 6ch the mic jack is retasked as output and cannot be used
https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/943386
At Mon, 03 Dec 2012 09:52:48 +0100, Takashi Iwai wrote:
the other problem is those acer aspire 8930g notebook has internal 5.1 speakers and three jacks which can support external 5.1 speakers
it seem the the channel mode control limit the max channels to 2 when there is internal 5.1 speaker
Again it's utterly irrelevant.
... and fixed by the patch below.
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda/realtek - Keep the channel count for multiple speakers
The current Realtek driver reconfigures the max PCM channels dynamically according to the value of Channel Mode enum if the multi-io retasking is available. It works fine for multi-io pins. But when multiple speaker pins are available, the channels of speakers also have to obey to the channel mode, which isn't nice. (That is, when you select "2ch" in Channel Mode so that the line-in and mic jack behave as input, you can't play surrounds properly from the built-in speaker.)
This patch fixes the problem by taking the channel number for multiple speakers into account in the channel-mode setup code. Also it fixes the wrongly set up max_channels value in the case of multi-io extension.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 5d8044d..7743775 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -153,8 +153,8 @@ struct alc_spec { const struct hda_channel_mode *channel_mode; int num_channel_mode; int need_dac_fix; - int const_channel_count; - int ext_channel_count; + int const_channel_count; /* min. channel count (for speakers) */ + int ext_channel_count; /* current channel count for multi-io */
/* PCM information */ struct hda_pcm pcm_rec[3]; /* used in alc_build_pcms() */ @@ -3961,8 +3961,9 @@ static int alc_auto_ch_mode_put(struct snd_kcontrol *kcontrol, spec->ext_channel_count = (ch + 1) * 2; for (i = 0; i < spec->multi_ios; i++) alc_set_multi_io(codec, i, i < ch); - spec->multiout.max_channels = spec->ext_channel_count; - if (spec->need_dac_fix && !spec->const_channel_count) + spec->multiout.max_channels = max(spec->ext_channel_count, + spec->const_channel_count); + if (spec->need_dac_fix) spec->multiout.num_dacs = spec->multiout.max_channels / 2; return 1; } @@ -4324,7 +4325,17 @@ static int alc_parse_auto_config(struct hda_codec *codec, if (err < 0) return err;
- spec->multiout.max_channels = spec->multiout.num_dacs * 2; + /* check the multiple speaker pins */ + if (cfg->line_out_type == AUTO_PIN_SPEAKER_OUT) + spec->const_channel_count = cfg->line_outs * 2; + else + spec->const_channel_count = cfg->speaker_outs * 2; + + if (spec->multi_ios > 0) + spec->multiout.max_channels = max(spec->ext_channel_count, + spec->const_channel_count); + else + spec->multiout.max_channels = spec->multiout.num_dacs * 2;
dig_only: alc_auto_parse_digital(codec);
Just refactoring, no functional changes.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_auto_parser.c | 106 ++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 59 deletions(-)
diff --git a/sound/pci/hda/hda_auto_parser.c b/sound/pci/hda/hda_auto_parser.c index 90d5b39..91232db 100644 --- a/sound/pci/hda/hda_auto_parser.c +++ b/sound/pci/hda/hda_auto_parser.c @@ -11,6 +11,7 @@
#include <linux/slab.h> #include <linux/export.h> +#include <linux/sort.h> #include <sound/core.h> #include "hda_codec.h" #include "hda_local.h" @@ -30,29 +31,30 @@ static int is_in_nid_list(hda_nid_t nid, const hda_nid_t *list) return 0; }
+/* a pair of input pin and its sequence */ +struct auto_out_pin { + hda_nid_t pin; + short seq; +}; + +static int compare_seq(const void *ap, const void *bp) +{ + const struct auto_out_pin *a = ap; + const struct auto_out_pin *b = bp; + return (int)(a->seq - b->seq); +}
/* * Sort an associated group of pins according to their sequence numbers. + * then store it to a pin array. */ -static void sort_pins_by_sequence(hda_nid_t *pins, short *sequences, +static void sort_pins_by_sequence(hda_nid_t *pins, struct auto_out_pin *list, int num_pins) { - int i, j; - short seq; - hda_nid_t nid; - - for (i = 0; i < num_pins; i++) { - for (j = i + 1; j < num_pins; j++) { - if (sequences[i] > sequences[j]) { - seq = sequences[i]; - sequences[i] = sequences[j]; - sequences[j] = seq; - nid = pins[i]; - pins[i] = pins[j]; - pins[j] = nid; - } - } - } + int i; + sort(list, num_pins, sizeof(list[0]), compare_seq, NULL); + for (i = 0; i < num_pins; i++) + pins[i] = list[i].pin; }
@@ -67,21 +69,11 @@ static void add_auto_cfg_input_pin(struct auto_pin_cfg *cfg, hda_nid_t nid, } }
-/* sort inputs in the order of AUTO_PIN_* type */ -static void sort_autocfg_input_pins(struct auto_pin_cfg *cfg) +static int compare_input_type(const void *ap, const void *bp) { - int i, j; - - for (i = 0; i < cfg->num_inputs; i++) { - for (j = i + 1; j < cfg->num_inputs; j++) { - if (cfg->inputs[i].type > cfg->inputs[j].type) { - struct auto_pin_cfg_item tmp; - tmp = cfg->inputs[i]; - cfg->inputs[i] = cfg->inputs[j]; - cfg->inputs[j] = tmp; - } - } - } + const struct auto_pin_cfg_item *a = ap; + const struct auto_pin_cfg_item *b = bp; + return (int)(a->type - b->type); }
/* Reorder the surround channels @@ -129,16 +121,16 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, { hda_nid_t nid, end_nid; short seq, assoc_line_out; - short sequences_line_out[ARRAY_SIZE(cfg->line_out_pins)]; - short sequences_speaker[ARRAY_SIZE(cfg->speaker_pins)]; - short sequences_hp[ARRAY_SIZE(cfg->hp_pins)]; + struct auto_out_pin line_out[ARRAY_SIZE(cfg->line_out_pins)]; + struct auto_out_pin speaker_out[ARRAY_SIZE(cfg->speaker_pins)]; + struct auto_out_pin hp_out[ARRAY_SIZE(cfg->hp_pins)]; int i;
memset(cfg, 0, sizeof(*cfg));
- memset(sequences_line_out, 0, sizeof(sequences_line_out)); - memset(sequences_speaker, 0, sizeof(sequences_speaker)); - memset(sequences_hp, 0, sizeof(sequences_hp)); + memset(line_out, 0, sizeof(line_out)); + memset(speaker_out, 0, sizeof(speaker_out)); + memset(hp_out, 0, sizeof(hp_out)); assoc_line_out = 0;
end_nid = codec->start_nid + codec->num_nodes; @@ -184,8 +176,8 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, continue; if (cfg->line_outs >= ARRAY_SIZE(cfg->line_out_pins)) continue; - cfg->line_out_pins[cfg->line_outs] = nid; - sequences_line_out[cfg->line_outs] = seq; + line_out[cfg->line_outs].pin = nid; + line_out[cfg->line_outs].seq = seq; cfg->line_outs++; break; case AC_JACK_SPEAKER: @@ -193,8 +185,8 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, assoc = get_defcfg_association(def_conf); if (cfg->speaker_outs >= ARRAY_SIZE(cfg->speaker_pins)) continue; - cfg->speaker_pins[cfg->speaker_outs] = nid; - sequences_speaker[cfg->speaker_outs] = (assoc << 4) | seq; + speaker_out[cfg->speaker_outs].pin = nid; + speaker_out[cfg->speaker_outs].seq = (assoc << 4) | seq; cfg->speaker_outs++; break; case AC_JACK_HP_OUT: @@ -202,8 +194,8 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, assoc = get_defcfg_association(def_conf); if (cfg->hp_outs >= ARRAY_SIZE(cfg->hp_pins)) continue; - cfg->hp_pins[cfg->hp_outs] = nid; - sequences_hp[cfg->hp_outs] = (assoc << 4) | seq; + hp_out[cfg->hp_outs].pin = nid; + hp_out[cfg->hp_outs].seq = (assoc << 4) | seq; cfg->hp_outs++; break; case AC_JACK_MIC_IN: @@ -248,34 +240,28 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, int i = 0; while (i < cfg->hp_outs) { /* The real HPs should have the sequence 0x0f */ - if ((sequences_hp[i] & 0x0f) == 0x0f) { + if ((hp_out[i].seq & 0x0f) == 0x0f) { i++; continue; } /* Move it to the line-out table */ - cfg->line_out_pins[cfg->line_outs] = cfg->hp_pins[i]; - sequences_line_out[cfg->line_outs] = sequences_hp[i]; - cfg->line_outs++; + line_out[cfg->line_outs++] = hp_out[i]; cfg->hp_outs--; - memmove(cfg->hp_pins + i, cfg->hp_pins + i + 1, - sizeof(cfg->hp_pins[0]) * (cfg->hp_outs - i)); - memmove(sequences_hp + i, sequences_hp + i + 1, - sizeof(sequences_hp[0]) * (cfg->hp_outs - i)); + memmove(hp_out + i, hp_out + i + 1, + sizeof(hp_out[0]) * (cfg->hp_outs - i)); } - memset(cfg->hp_pins + cfg->hp_outs, 0, - sizeof(hda_nid_t) * (AUTO_CFG_MAX_OUTS - cfg->hp_outs)); + memset(hp_out + cfg->hp_outs, 0, + sizeof(hp_out[0]) * (AUTO_CFG_MAX_OUTS - cfg->hp_outs)); if (!cfg->hp_outs) cfg->line_out_type = AUTO_PIN_HP_OUT;
}
/* sort by sequence */ - sort_pins_by_sequence(cfg->line_out_pins, sequences_line_out, - cfg->line_outs); - sort_pins_by_sequence(cfg->speaker_pins, sequences_speaker, + sort_pins_by_sequence(cfg->line_out_pins, line_out, cfg->line_outs); + sort_pins_by_sequence(cfg->speaker_pins, speaker_out, cfg->speaker_outs); - sort_pins_by_sequence(cfg->hp_pins, sequences_hp, - cfg->hp_outs); + sort_pins_by_sequence(cfg->hp_pins, hp_out, cfg->hp_outs);
/* * FIX-UP: if no line-outs are detected, try to use speaker or HP pin @@ -304,7 +290,9 @@ int snd_hda_parse_pin_defcfg(struct hda_codec *codec, reorder_outputs(cfg->hp_outs, cfg->hp_pins); reorder_outputs(cfg->speaker_outs, cfg->speaker_pins);
- sort_autocfg_input_pins(cfg); + /* sort inputs in the order of AUTO_PIN_* type */ + sort(cfg->inputs, cfg->num_inputs, sizeof(cfg->inputs[0]), + compare_input_type, NULL);
/* * debug prints of the parsed results
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 147 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c5cc47b..95c5bc7 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -213,6 +213,10 @@ struct alc_spec { unsigned int inv_dmic_muted:1; /* R-ch of inv d-mic is muted? */ unsigned int no_primary_hp:1; /* Don't prefer HP pins to speaker pins */
+ /* front-mic / HP sharing for HP Z220 */ + unsigned int shared_fmic_hp_mode:1; + hda_nid_t shared_fmic_hp_nid; + /* auto-mute control */ int automute_mode; hda_nid_t automute_mixer_nid[AUTO_CFG_MAX_OUTS]; @@ -5959,6 +5963,143 @@ static void alc269_fixup_pcm_44k(struct hda_codec *codec, spec->stream_analog_capture = &alc269_44k_pcm_analog_capture; }
+/* allow switching HP/front-mic on HP Z220 */ +static int alc221_shared_fmic_hp_mode_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char * const texts[] = { + "Mic", "Headphone" + }; + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + uinfo->count = 1; + uinfo->value.enumerated.items = 2; + if (uinfo->value.enumerated.item >= 2) + uinfo->value.enumerated.item = 1; + strcpy(uinfo->value.enumerated.name, + texts[uinfo->value.enumerated.item]); + return 0; +} + +static int alc221_shared_fmic_hp_mode_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct alc_spec *spec = codec->spec; + ucontrol->value.enumerated.item[0] = spec->shared_fmic_hp_mode; + return 0; +} + +static void alc221_shared_fmic_hp_mode_update(struct hda_codec *codec) +{ + struct alc_spec *spec = codec->spec; + struct hda_jack_tbl *jack; + hda_nid_t fmic_nid = spec->shared_fmic_hp_nid; + unsigned int pinctl, mute, idx; + hda_jack_callback callback; + + if (!fmic_nid) + return; + + /* utterly hackish: replace the secondary hp pin, enable/disable the + * automic on the fly + */ + if (spec->shared_fmic_hp_mode) { + spec->autocfg.hp_outs = 2; + spec->autocfg.hp_pins[1] = fmic_nid; + callback = alc_hp_automute; + pinctl = PIN_HP; + mute = AMP_OUT_UNMUTE; + spec->auto_mic = 0; + alc_mux_select(codec, 0, spec->am_entry[0].idx, false); + /* choose the same DAC as the primary HP output */ + idx = snd_hda_codec_read(codec, spec->autocfg.hp_pins[0], 0, + AC_VERB_GET_CONNECT_SEL, 0); + snd_hda_codec_write(codec, fmic_nid, 0, + AC_VERB_SET_CONNECT_SEL, idx); + } else { + spec->autocfg.hp_outs = 1; + spec->autocfg.hp_pins[1] = 0; + callback = alc_mic_automute; + pinctl = PIN_VREF80; + mute = AMP_OUT_MUTE; + spec->auto_mic = 1; + } + + snd_hda_codec_write(codec, fmic_nid, 0, + AC_VERB_SET_PIN_WIDGET_CONTROL, pinctl); + snd_hda_codec_write(codec, fmic_nid, 0, + AC_VERB_SET_AMP_GAIN_MUTE, mute); + + jack = snd_hda_jack_tbl_get(codec, fmic_nid); + if (jack) + jack->callback = callback; + else + snd_hda_jack_detect_enable_callback(codec, fmic_nid, + ALC_HP_EVENT, callback); + alc_hp_automute(codec, NULL); + alc_mic_automute(codec, NULL); +} + +static int alc221_shared_fmic_hp_mode_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct alc_spec *spec = codec->spec; + + if (spec->shared_fmic_hp_mode == ucontrol->value.enumerated.item[0]) + return 0; + spec->shared_fmic_hp_mode = ucontrol->value.enumerated.item[0]; + alc221_shared_fmic_hp_mode_update(codec); + return 1; +} + +static const struct snd_kcontrol_new alc221_shared_fmic_hp_mode_enum = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Front Mic Jack Mode", + .info = alc221_shared_fmic_hp_mode_info, + .get = alc221_shared_fmic_hp_mode_get, + .put = alc221_shared_fmic_hp_mode_put, +}; + +static int alc221_add_shared_fmic_hp_mode(struct hda_codec *codec) +{ + struct alc_spec *spec = codec->spec; + struct snd_kcontrol_new *knew; + char name[22]; + + if (spec->autocfg.hp_outs != 1 || spec->am_num_entries != 2) + return -EINVAL; + + snd_hda_get_pin_label(codec, spec->am_entry[1].pin, &spec->autocfg, + name, sizeof(name), NULL); + strlcat(name, " Jack Mode", sizeof(name)); + + knew = alc_kcontrol_new(spec); + if (!knew) + return -ENOMEM; + *knew = alc221_shared_fmic_hp_mode_enum; + knew->name = kstrdup(name, GFP_KERNEL); + if (!knew->name) + return -ENOMEM; + spec->shared_fmic_hp_nid = spec->am_entry[1].pin; + spec->shared_fmic_hp_mode = 0; + return 0; +} + +static void alc221_fixup_shared_fmic_hp(struct hda_codec *codec, + const struct alc_fixup *fix, int action) +{ + switch (action) { + case ALC_FIXUP_ACT_PROBE: + if (alc221_add_shared_fmic_hp_mode(codec) < 0) + return; + break; + case ALC_FIXUP_ACT_INIT: + alc221_shared_fmic_hp_mode_update(codec); + break; + } +} + static void alc269_fixup_stereo_dmic(struct hda_codec *codec, const struct alc_fixup *fix, int action) { @@ -6055,6 +6196,7 @@ enum { ALC269_FIXUP_MIC2_MUTE_LED, ALC269_FIXUP_INV_DMIC, ALC269_FIXUP_LENOVO_DOCK, + ALC221_FIXUP_SHARED_FMIC_HP, ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT, ALC271_FIXUP_AMIC_MIC2, ALC271_FIXUP_HP_GATE_MIC_JACK, @@ -6198,6 +6340,10 @@ static const struct alc_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT }, + [ALC221_FIXUP_SHARED_FMIC_HP] = { + .type = ALC_FIXUP_FUNC, + .v.func = alc221_fixup_shared_fmic_hp, + }, [ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT] = { .type = ALC_FIXUP_FUNC, .v.func = alc269_fixup_pincfg_no_hp_to_lineout, @@ -6224,6 +6370,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1025, 0x029b, "Acer 1810TZ", ALC269_FIXUP_INV_DMIC), SND_PCI_QUIRK(0x1025, 0x0349, "Acer AOD260", ALC269_FIXUP_INV_DMIC), SND_PCI_QUIRK(0x103c, 0x1586, "HP", ALC269_FIXUP_MIC2_MUTE_LED), + SND_PCI_QUIRK(0x103c, 0x1791, "HP Z220", ALC221_FIXUP_SHARED_FMIC_HP), SND_PCI_QUIRK(0x1043, 0x1427, "Asus Zenbook UX31E", ALC269VB_FIXUP_DMIC), SND_PCI_QUIRK(0x1043, 0x1517, "Asus Zenbook UX31A", ALC269VB_FIXUP_DMIC), SND_PCI_QUIRK(0x1043, 0x1a13, "Asus G73Jw", ALC269_FIXUP_ASUS_G73JW),
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
Hmm, in principle I don't like this hack. In many machines, both front jacks could probably work as anything (line in, line out, headphones, mic, etc), and that goes for many pins on many machines, and we would end up with a ton of ALSA kcontrols and an incredibly complex parsing machine.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
If you insist though, a slightly more consistent mechanism would be to use the same approach as we did with the single 3-pin jacks (on some Asus machines, IIRC), where selecting the Capture/Input Source would retask the pin, instead of adding a separate "Jack Mode" switch.
Also, the jack detection won't work well with PulseAudio, if I read the code correctly PulseAudio will think a front mic has been inserted regardless of the mode. You have been warned :-)
Now; if you have a very strong reason why this machine would need this special retask mechanism and all other machines don't, I'm willing to listen...
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_realtek.c | 147 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index c5cc47b..95c5bc7 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -213,6 +213,10 @@ struct alc_spec { unsigned int inv_dmic_muted:1; /* R-ch of inv d-mic is muted? */ unsigned int no_primary_hp:1; /* Don't prefer HP pins to speaker pins */
- /* front-mic / HP sharing for HP Z220 */
- unsigned int shared_fmic_hp_mode:1;
- hda_nid_t shared_fmic_hp_nid;
- /* auto-mute control */ int automute_mode; hda_nid_t automute_mixer_nid[AUTO_CFG_MAX_OUTS];
@@ -5959,6 +5963,143 @@ static void alc269_fixup_pcm_44k(struct hda_codec *codec, spec->stream_analog_capture = &alc269_44k_pcm_analog_capture; }
+/* allow switching HP/front-mic on HP Z220 */ +static int alc221_shared_fmic_hp_mode_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
- static const char * const texts[] = {
"Mic", "Headphone"- };
- uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
- uinfo->count = 1;
- uinfo->value.enumerated.items = 2;
- if (uinfo->value.enumerated.item >= 2)
uinfo->value.enumerated.item = 1;- strcpy(uinfo->value.enumerated.name,
texts[uinfo->value.enumerated.item]);- return 0;
+}
+static int alc221_shared_fmic_hp_mode_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
- struct alc_spec *spec = codec->spec;
- ucontrol->value.enumerated.item[0] = spec->shared_fmic_hp_mode;
- return 0;
+}
+static void alc221_shared_fmic_hp_mode_update(struct hda_codec *codec) +{
- struct alc_spec *spec = codec->spec;
- struct hda_jack_tbl *jack;
- hda_nid_t fmic_nid = spec->shared_fmic_hp_nid;
- unsigned int pinctl, mute, idx;
- hda_jack_callback callback;
- if (!fmic_nid)
return;- /* utterly hackish: replace the secondary hp pin, enable/disable the
* automic on the fly*/- if (spec->shared_fmic_hp_mode) {
spec->autocfg.hp_outs = 2;spec->autocfg.hp_pins[1] = fmic_nid;callback = alc_hp_automute;pinctl = PIN_HP;mute = AMP_OUT_UNMUTE;spec->auto_mic = 0;alc_mux_select(codec, 0, spec->am_entry[0].idx, false);/* choose the same DAC as the primary HP output */idx = snd_hda_codec_read(codec, spec->autocfg.hp_pins[0], 0,AC_VERB_GET_CONNECT_SEL, 0);snd_hda_codec_write(codec, fmic_nid, 0,AC_VERB_SET_CONNECT_SEL, idx);- } else {
spec->autocfg.hp_outs = 1;spec->autocfg.hp_pins[1] = 0;callback = alc_mic_automute;pinctl = PIN_VREF80;mute = AMP_OUT_MUTE;spec->auto_mic = 1;- }
- snd_hda_codec_write(codec, fmic_nid, 0,
AC_VERB_SET_PIN_WIDGET_CONTROL, pinctl);- snd_hda_codec_write(codec, fmic_nid, 0,
AC_VERB_SET_AMP_GAIN_MUTE, mute);- jack = snd_hda_jack_tbl_get(codec, fmic_nid);
- if (jack)
jack->callback = callback;- else
snd_hda_jack_detect_enable_callback(codec, fmic_nid,ALC_HP_EVENT, callback);- alc_hp_automute(codec, NULL);
- alc_mic_automute(codec, NULL);
+}
+static int alc221_shared_fmic_hp_mode_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)+{
- struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
- struct alc_spec *spec = codec->spec;
- if (spec->shared_fmic_hp_mode == ucontrol->value.enumerated.item[0])
return 0;- spec->shared_fmic_hp_mode = ucontrol->value.enumerated.item[0];
- alc221_shared_fmic_hp_mode_update(codec);
- return 1;
+}
+static const struct snd_kcontrol_new alc221_shared_fmic_hp_mode_enum = {
- .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
- .name = "Front Mic Jack Mode",
- .info = alc221_shared_fmic_hp_mode_info,
- .get = alc221_shared_fmic_hp_mode_get,
- .put = alc221_shared_fmic_hp_mode_put,
+};
+static int alc221_add_shared_fmic_hp_mode(struct hda_codec *codec) +{
- struct alc_spec *spec = codec->spec;
- struct snd_kcontrol_new *knew;
- char name[22];
- if (spec->autocfg.hp_outs != 1 || spec->am_num_entries != 2)
return -EINVAL;- snd_hda_get_pin_label(codec, spec->am_entry[1].pin, &spec->autocfg,
name, sizeof(name), NULL);- strlcat(name, " Jack Mode", sizeof(name));
- knew = alc_kcontrol_new(spec);
- if (!knew)
return -ENOMEM;- *knew = alc221_shared_fmic_hp_mode_enum;
- knew->name = kstrdup(name, GFP_KERNEL);
- if (!knew->name)
return -ENOMEM;- spec->shared_fmic_hp_nid = spec->am_entry[1].pin;
- spec->shared_fmic_hp_mode = 0;
- return 0;
+}
+static void alc221_fixup_shared_fmic_hp(struct hda_codec *codec,
const struct alc_fixup *fix, int action)+{
- switch (action) {
- case ALC_FIXUP_ACT_PROBE:
if (alc221_add_shared_fmic_hp_mode(codec) < 0)return;break;- case ALC_FIXUP_ACT_INIT:
alc221_shared_fmic_hp_mode_update(codec);break;- }
+}
- static void alc269_fixup_stereo_dmic(struct hda_codec *codec, const struct alc_fixup *fix, int action) {
@@ -6055,6 +6196,7 @@ enum { ALC269_FIXUP_MIC2_MUTE_LED, ALC269_FIXUP_INV_DMIC, ALC269_FIXUP_LENOVO_DOCK,
- ALC221_FIXUP_SHARED_FMIC_HP, ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT, ALC271_FIXUP_AMIC_MIC2, ALC271_FIXUP_HP_GATE_MIC_JACK,
@@ -6198,6 +6340,10 @@ static const struct alc_fixup alc269_fixups[] = { .chained = true, .chain_id = ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT },
- [ALC221_FIXUP_SHARED_FMIC_HP] = {
.type = ALC_FIXUP_FUNC,.v.func = alc221_fixup_shared_fmic_hp,- }, [ALC269_FIXUP_PINCFG_NO_HP_TO_LINEOUT] = { .type = ALC_FIXUP_FUNC, .v.func = alc269_fixup_pincfg_no_hp_to_lineout,
@@ -6224,6 +6370,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1025, 0x029b, "Acer 1810TZ", ALC269_FIXUP_INV_DMIC), SND_PCI_QUIRK(0x1025, 0x0349, "Acer AOD260", ALC269_FIXUP_INV_DMIC), SND_PCI_QUIRK(0x103c, 0x1586, "HP", ALC269_FIXUP_MIC2_MUTE_LED),
- SND_PCI_QUIRK(0x103c, 0x1791, "HP Z220", ALC221_FIXUP_SHARED_FMIC_HP), SND_PCI_QUIRK(0x1043, 0x1427, "Asus Zenbook UX31E", ALC269VB_FIXUP_DMIC), SND_PCI_QUIRK(0x1043, 0x1517, "Asus Zenbook UX31A", ALC269VB_FIXUP_DMIC), SND_PCI_QUIRK(0x1043, 0x1a13, "Asus G73Jw", ALC269_FIXUP_ASUS_G73JW),
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
Hmm, in principle I don't like this hack.
I don't like either, but there was some demand for such feature...
In many machines, both front jacks could probably work as anything (line in, line out, headphones, mic, etc), and that goes for many pins on many machines, and we would end up with a ton of ALSA kcontrols and an incredibly complex parsing machine.
Actually we can (or should) add "xxx Jack Mode" enum to every jack in future. See below.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use. Also, a big missing piece is the automatic retasking via jack detection as a headphone.
If you insist though, a slightly more consistent mechanism would be to use the same approach as we did with the single 3-pin jacks (on some Asus machines, IIRC), where selecting the Capture/Input Source would retask the pin, instead of adding a separate "Jack Mode" switch.
I thought of that, too. OTOH, "xx Jack Mode" enum is already present for IDT codecs over years. This isn't used for configuring the I/O direction of jacks but choosing bias levels. Why not applying the same logic for I/O direction switch?
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
About your alternative proposal: the problem by extending the input source selection is that this jack is supposed to be a front mic jack as default. So, this is always listed in the capture source imux.
Also, the jack detection won't work well with PulseAudio, if I read the code correctly PulseAudio will think a front mic has been inserted regardless of the mode. You have been warned :-)
Yeah, it's a concern. Currently it's no issue because no capture source enum is exposed, so PA can't do anything wrong. But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
Now; if you have a very strong reason why this machine would need this special retask mechanism and all other machines don't, I'm willing to listen...
A strong reason for adding the feature to this machine is that this has a print on the top of the front mic jack indicating it's a shared front-mic / headphone jack. Thus it must behave as advertised. This is user's expectation.
If other machines have such a print, the similar method should be implemented, too.
Takashi
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
Hmm, in principle I don't like this hack.
I don't like either, but there was some demand for such feature...
Heh, that was not too surprising :-)
In many machines, both front jacks could probably work as anything (line in, line out, headphones, mic, etc), and that goes for many pins on many machines, and we would end up with a ton of ALSA kcontrols and an incredibly complex parsing machine.
Actually we can (or should) add "xxx Jack Mode" enum to every jack in future. See below.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
If you insist though, a slightly more consistent mechanism would be to use the same approach as we did with the single 3-pin jacks (on some Asus machines, IIRC), where selecting the Capture/Input Source would retask the pin, instead of adding a separate "Jack Mode" switch.
I thought of that, too. OTOH, "xx Jack Mode" enum is already present for IDT codecs over years. This isn't used for configuring the I/O direction of jacks but choosing bias levels. Why not applying the same logic for I/O direction switch?
This might work for this particular case, but sounds like it could become a confusing set of alsa-mixer controls (and hairy kernel code to control them all!) - e g, what if two jacks that currently are not headphones both want to be headphones, and they have independent volume controls? Which one will be controlled by "Headphone Volume"?
As an example of current code that I have trouble understanding and fixing bugs in, is the badness calculation feature. This sounds like it could be even worse.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Yes, some kind of "advanced" tab in the sound settings where things can be reconfigured would be nice.
About your alternative proposal: the problem by extending the input source selection is that this jack is supposed to be a front mic jack as default. So, this is always listed in the capture source imux.
Well, if the "Capture Source" switches to "Rear Line In", then it does not hurt if we retask the front to headphone automatically. But if you're going to implement auto-switching based on impedance sensing, this alternative proposal might fail anyway?
Also, the jack detection won't work well with PulseAudio, if I read the code correctly PulseAudio will think a front mic has been inserted regardless of the mode. You have been warned :-)
Yeah, it's a concern. Currently it's no issue because no capture source enum is exposed, so PA can't do anything wrong.
If PA detects the "Front Mic Jack" kcontrol to be plugged in, and I implement the priority order as I suggested in the other comment, we have now disabled the rear line in because the user plugged in a headphone in the front mic jack.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Now; if you have a very strong reason why this machine would need this special retask mechanism and all other machines don't, I'm willing to listen...
A strong reason for adding the feature to this machine is that this has a print on the top of the front mic jack indicating it's a shared front-mic / headphone jack. Thus it must behave as advertised. This is user's expectation.
If other machines have such a print, the similar method should be implemented, too.
Yeah, that's a reasonable point.
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
Hmm, in principle I don't like this hack.
I don't like either, but there was some demand for such feature...
Heh, that was not too surprising :-)
In many machines, both front jacks could probably work as anything (line in, line out, headphones, mic, etc), and that goes for many pins on many machines, and we would end up with a ton of ALSA kcontrols and an incredibly complex parsing machine.
Actually we can (or should) add "xxx Jack Mode" enum to every jack in future. See below.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
If you insist though, a slightly more consistent mechanism would be to use the same approach as we did with the single 3-pin jacks (on some Asus machines, IIRC), where selecting the Capture/Input Source would retask the pin, instead of adding a separate "Jack Mode" switch.
I thought of that, too. OTOH, "xx Jack Mode" enum is already present for IDT codecs over years. This isn't used for configuring the I/O direction of jacks but choosing bias levels. Why not applying the same logic for I/O direction switch?
This might work for this particular case, but sounds like it could become a confusing set of alsa-mixer controls (and hairy kernel code to control them all!) - e g, what if two jacks that currently are not headphones both want to be headphones, and they have independent volume controls? Which one will be controlled by "Headphone Volume"?
As an example of current code that I have trouble understanding and fixing bugs in, is the badness calculation feature. This sounds like it could be even worse.
I think the only confusions will be the conflict between the multi-io channel mode and individual retasking, and the conflict between the capture source and individual retasking.
The basic topology is determined statically at the probe time based on the initial configuration. If a jack is already listed in multi-io mode, the output can be toggled by the channel mode, so we either don't add the output to the jack mode enum or do change the enum value forcibly when the channel mode is changed.
If a jack isn't in multi-io but is still capable to be an output, we need to check which DAC it can connect without disturbing the existing routing. Only if it can be connected to either a line-out DAC a headphone DAC, we can add a hook -- i.e. an additional enum item of this jack mode.
When a mic jack is retasked as headphone and it's the current capture source, what should we do? In my previous implementation, it was easy, because it's just one of two, so forced to switch to another. But in a general implementation, there can be more.
OTOH, setting the mic as the capture source itself doesn't "break". You'll get no input, but it's what user chooses now. So, maybe the intelligent selection of other source could be better done in the same place where the jack-detection-and-do-select is done (e.g. PA).
A remaining problem in the scenario above is possible conflicts of the new DAC hooks. The routes to a DAC might be exclusive with each other from both front and rear mics. Though, I don't think this would happen on most of codecs.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Yes, some kind of "advanced" tab in the sound settings where things can be reconfigured would be nice.
About your alternative proposal: the problem by extending the input source selection is that this jack is supposed to be a front mic jack as default. So, this is always listed in the capture source imux.
Well, if the "Capture Source" switches to "Rear Line In", then it does not hurt if we retask the front to headphone automatically. But if you're going to implement auto-switching based on impedance sensing, this alternative proposal might fail anyway?
It'd be a different logic, yes.
Also, the jack detection won't work well with PulseAudio, if I read the code correctly PulseAudio will think a front mic has been inserted regardless of the mode. You have been warned :-)
Yeah, it's a concern. Currently it's no issue because no capture source enum is exposed, so PA can't do anything wrong.
If PA detects the "Front Mic Jack" kcontrol to be plugged in, and I implement the priority order as I suggested in the other comment, we have now disabled the rear line in because the user plugged in a headphone in the front mic jack.
Yes, the policy conflicts.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
Now; if you have a very strong reason why this machine would need this special retask mechanism and all other machines don't, I'm willing to listen...
A strong reason for adding the feature to this machine is that this has a print on the top of the front mic jack indicating it's a shared front-mic / headphone jack. Thus it must behave as advertised. This is user's expectation.
If other machines have such a print, the similar method should be implemented, too.
Yeah, that's a reasonable point.
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
Once when it gets done, the rest is the automatic capture source selection. This would need more discussion, I suppose.
Takashi
At Fri, 30 Nov 2012 10:39:14 +0100, Takashi Iwai wrote:
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
On 11/29/2012 04:21 PM, Takashi Iwai wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
Hmm, in principle I don't like this hack.
I don't like either, but there was some demand for such feature...
Heh, that was not too surprising :-)
In many machines, both front jacks could probably work as anything (line in, line out, headphones, mic, etc), and that goes for many pins on many machines, and we would end up with a ton of ALSA kcontrols and an incredibly complex parsing machine.
Actually we can (or should) add "xxx Jack Mode" enum to every jack in future. See below.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
If you insist though, a slightly more consistent mechanism would be to use the same approach as we did with the single 3-pin jacks (on some Asus machines, IIRC), where selecting the Capture/Input Source would retask the pin, instead of adding a separate "Jack Mode" switch.
I thought of that, too. OTOH, "xx Jack Mode" enum is already present for IDT codecs over years. This isn't used for configuring the I/O direction of jacks but choosing bias levels. Why not applying the same logic for I/O direction switch?
This might work for this particular case, but sounds like it could become a confusing set of alsa-mixer controls (and hairy kernel code to control them all!) - e g, what if two jacks that currently are not headphones both want to be headphones, and they have independent volume controls? Which one will be controlled by "Headphone Volume"?
As an example of current code that I have trouble understanding and fixing bugs in, is the badness calculation feature. This sounds like it could be even worse.
I think the only confusions will be the conflict between the multi-io channel mode and individual retasking, and the conflict between the capture source and individual retasking.
The basic topology is determined statically at the probe time based on the initial configuration. If a jack is already listed in multi-io mode, the output can be toggled by the channel mode, so we either don't add the output to the jack mode enum or do change the enum value forcibly when the channel mode is changed.
If a jack isn't in multi-io but is still capable to be an output, we need to check which DAC it can connect without disturbing the existing routing. Only if it can be connected to either a line-out DAC a headphone DAC, we can add a hook -- i.e. an additional enum item of this jack mode.
When a mic jack is retasked as headphone and it's the current capture source, what should we do? In my previous implementation, it was easy, because it's just one of two, so forced to switch to another. But in a general implementation, there can be more.
OTOH, setting the mic as the capture source itself doesn't "break". You'll get no input, but it's what user chooses now. So, maybe the intelligent selection of other source could be better done in the same place where the jack-detection-and-do-select is done (e.g. PA).
A remaining problem in the scenario above is possible conflicts of the new DAC hooks. The routes to a DAC might be exclusive with each other from both front and rear mics. Though, I don't think this would happen on most of codecs.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Yes, some kind of "advanced" tab in the sound settings where things can be reconfigured would be nice.
About your alternative proposal: the problem by extending the input source selection is that this jack is supposed to be a front mic jack as default. So, this is always listed in the capture source imux.
Well, if the "Capture Source" switches to "Rear Line In", then it does not hurt if we retask the front to headphone automatically. But if you're going to implement auto-switching based on impedance sensing, this alternative proposal might fail anyway?
It'd be a different logic, yes.
Also, the jack detection won't work well with PulseAudio, if I read the code correctly PulseAudio will think a front mic has been inserted regardless of the mode. You have been warned :-)
Yeah, it's a concern. Currently it's no issue because no capture source enum is exposed, so PA can't do anything wrong.
If PA detects the "Front Mic Jack" kcontrol to be plugged in, and I implement the priority order as I suggested in the other comment, we have now disabled the rear line in because the user plugged in a headphone in the front mic jack.
Yes, the policy conflicts.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
Now; if you have a very strong reason why this machine would need this special retask mechanism and all other machines don't, I'm willing to listen...
A strong reason for adding the feature to this machine is that this has a print on the top of the front mic jack indicating it's a shared front-mic / headphone jack. Thus it must behave as advertised. This is user's expectation.
If other machines have such a print, the similar method should be implemented, too.
Yeah, that's a reasonable point.
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
Once when it gets done, the rest is the automatic capture source selection. This would need more discussion, I suppose.
FYI, I applied a few obvious cleanup patches in the previous series to for-next branch, and rebased the whole series. It's put in sound-unstable tree test/realtek-automic branch and pushed out now. Just for testing, not meant to be merged to the upstream.
Takashi
On 11/30/2012 10:39 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Trying to summarise this long thread, we seem to advocate these two options as I see it:
1) Use the hwdep interface:
Pro: Simplest kernel code
Pro: Can support more options, higher degree of freedom for user
Con: Any reconfiguration is a bit disturbing and can e g break currently running audio streams.
Con: (Currently) requires root privileges to change, since it works through the hwdep interface.
2) Base reconfiguration on jack-mode controls
Pro: Simpler for user to reconfigure jacks
Con: More complex kernel code than first option
Con: It might be difficult to get the volume/switch control names right in all scenarios.
Con: Needs work in PulseAudios and GUIs to support jack retasking better, or it will be either unsupported (for the average user) or confusing.
Also; since the parsing code is still separate between codecs, we have to implement all of this complexity over and over again...
Probably we can sort out most stuff with option 2) too, I mean, with the capture source, automute and autoswitch, rerouting paths, repurposing DACs, and all that, but the question is really, should we? Are we willing to take the cost of the added complexity that comes with it? Are we be able to get it right, or do we get several broken kernel versions before we get something that works really well?
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
Ok, so what do you mean with "automatic"?
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Btw, how "disturbing" is it when you retask jacks on Windows? What happens e g if you try to retask a jack you're currently outputting audio to?
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
How important is it for you to actually get support for this stuff in PulseAudio? I mean, you're certainly more than capable of adding kcontrols, but I've never seen you - or anyone else from your team - submitting patches against PulseAudio to have support for these things all the way through to the user.
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
As an example. Say that we have three DACs with amp-outs. In standard configuration the first DAC's volume control is called "Speaker Volume" and the second DAC's volume is called "Headphone Volume". Suddenly you get a Jack Mode switch or multi-I/O switch, so that the three DACs now go to "Front", "C/LFE" and "Rear". The headphone must then be rerouted to the first DAC. But what happens to the volume controls now?
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
What if there suddenly is "demand for such feature" for something that *does* change the topology a lot?
At Mon, 03 Dec 2012 14:43:37 +0100, David Henningsson wrote:
On 11/30/2012 10:39 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Trying to summarise this long thread, we seem to advocate these two options as I see it:
- Use the hwdep interface:
Pro: Simplest kernel code
Pro: Can support more options, higher degree of freedom for user
Con: Any reconfiguration is a bit disturbing and can e g break currently running audio streams.
Con: (Currently) requires root privileges to change, since it works through the hwdep interface.
Con: how can you ensure the exclusive setup and the exclusive access? Imagine multiple sessions are using different configurations.
Con: how would you manage the mixer elements from hwdep at all? There are independent mixer applications.
- Base reconfiguration on jack-mode controls
Pro: Simpler for user to reconfigure jacks
Con: More complex kernel code than first option
Con: It might be difficult to get the volume/switch control names right in all scenarios.
Con: Needs work in PulseAudios and GUIs to support jack retasking better, or it will be either unsupported (for the average user) or confusing.
Hmm... I don't get it. How can hda-jack-retask or whatever be a better option than PA? The integration work for PA is more or less necessary. Launching another application may be more confusion for user, no?
Also; since the parsing code is still separate between codecs, we have to implement all of this complexity over and over again...
It's "still". They will be merged sooner or later, anyway.
Probably we can sort out most stuff with option 2) too, I mean, with the capture source, automute and autoswitch, rerouting paths, repurposing DACs, and all that, but the question is really, should we? Are we willing to take the cost of the added complexity that comes with it? Are we be able to get it right, or do we get several broken kernel versions before we get something that works really well?
I don't think it'll be too complex in the end. We'll be reducing the code while merging such a new feature, especially by unifying the codes from multiple drivers. Once after this gets done, the driver will cover all needed demands for IDT, Conexant, Cirrus and VIA, and most of AD and C-Media codecs. Let's see.
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
Ok, so what do you mean with "automatic"?
Sorry, it wasn't automatic "retasking" but muting.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Btw, how "disturbing" is it when you retask jacks on Windows? What happens e g if you try to retask a jack you're currently outputting audio to?
I don't know exactly, too.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
How important is it for you to actually get support for this stuff in PulseAudio? I mean, you're certainly more than capable of adding kcontrols, but I've never seen you - or anyone else from your team - submitting patches against PulseAudio to have support for these things all the way through to the user.
Honestly, I don't care at this moment at all. A part of the reason is that I don't use PulseAudio on my machines.
But for normal users, PA support would be a really "good to have".
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
As an example. Say that we have three DACs with amp-outs. In standard configuration the first DAC's volume control is called "Speaker Volume" and the second DAC's volume is called "Headphone Volume". Suddenly you get a Jack Mode switch or multi-I/O switch, so that the three DACs now go to "Front", "C/LFE" and "Rear". The headphone must then be rerouted to the first DAC. But what happens to the volume controls now?
No. This doesn't happen.
The additional output via jack mode enum is just a hook to the existing I/O topology. It can hook to "Headphone" volume as a secondary headphone, but nothing else. If it can't to hook the headphone volume, no option will be provided.
If more drastic change is needed, user has to reconfigure the whole topology by setting the initial pin configs.
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
What if there suddenly is "demand for such feature" for something that *does* change the topology a lot?
Again, it doesn't change the topology. If it needs to change the topology, it's not what we support in the mixer API. This needs reconfiguration, thus it essentially needs to restart the whole driver functionality.
Takashi
On 12/03/2012 03:07 PM, Takashi Iwai wrote:
At Mon, 03 Dec 2012 14:43:37 +0100, David Henningsson wrote:
On 11/30/2012 10:39 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote:
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Trying to summarise this long thread, we seem to advocate these two options as I see it:
- Use the hwdep interface:
Pro: Simplest kernel code
Pro: Can support more options, higher degree of freedom for user
Con: Any reconfiguration is a bit disturbing and can e g break currently running audio streams.
Con: (Currently) requires root privileges to change, since it works through the hwdep interface.
Con: how can you ensure the exclusive setup and the exclusive access? Imagine multiple sessions are using different configurations.
Con: how would you manage the mixer elements from hwdep at all? There are independent mixer applications.
- Base reconfiguration on jack-mode controls
Pro: Simpler for user to reconfigure jacks
Con: More complex kernel code than first option
Con: It might be difficult to get the volume/switch control names right in all scenarios.
Con: Needs work in PulseAudios and GUIs to support jack retasking better, or it will be either unsupported (for the average user) or confusing.
Hmm... I don't get it. How can hda-jack-retask or whatever be a better option than PA? The integration work for PA is more or less necessary. Launching another application may be more confusion for user, no?
The integration work for PA in hda-jack-retask is such that hda-jack-retask kills PA before making a change (to avoid EBUSY and to make PA adjust to the new config).
But anyway. I agree that if we get a good mixer/kcontrol interface, the second solution has the advantage of an existing infrastructure for saving and restoring volumes between sessions, and support for different mixer application will already be in place.
Also; since the parsing code is still separate between codecs, we have to implement all of this complexity over and over again...
It's "still". They will be merged sooner or later, anyway.
I believe that when I see it - surprise me :-)
Probably we can sort out most stuff with option 2) too, I mean, with the capture source, automute and autoswitch, rerouting paths, repurposing DACs, and all that, but the question is really, should we? Are we willing to take the cost of the added complexity that comes with it? Are we be able to get it right, or do we get several broken kernel versions before we get something that works really well?
I don't think it'll be too complex in the end. We'll be reducing the code while merging such a new feature, especially by unifying the codes from multiple drivers. Once after this gets done, the driver will cover all needed demands for IDT, Conexant, Cirrus and VIA, and most of AD and C-Media codecs. Let's see.
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
Ok, so what do you mean with "automatic"?
Sorry, it wasn't automatic "retasking" but muting.
Ok, that makes sense.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Btw, how "disturbing" is it when you retask jacks on Windows? What happens e g if you try to retask a jack you're currently outputting audio to?
I don't know exactly, too.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
How important is it for you to actually get support for this stuff in PulseAudio? I mean, you're certainly more than capable of adding kcontrols, but I've never seen you - or anyone else from your team - submitting patches against PulseAudio to have support for these things all the way through to the user.
Honestly, I don't care at this moment at all. A part of the reason is that I don't use PulseAudio on my machines.
But for normal users, PA support would be a really "good to have".
Right, but you begin this thread by saying you didn't like the patch, but there was "demand for such feature". Doesn't the person/boss/customer/etc who demanded that feature care about it being accessible to the "normal users"?
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
As an example. Say that we have three DACs with amp-outs. In standard configuration the first DAC's volume control is called "Speaker Volume" and the second DAC's volume is called "Headphone Volume". Suddenly you get a Jack Mode switch or multi-I/O switch, so that the three DACs now go to "Front", "C/LFE" and "Rear". The headphone must then be rerouted to the first DAC. But what happens to the volume controls now?
No. This doesn't happen.
The additional output via jack mode enum is just a hook to the existing I/O topology. It can hook to "Headphone" volume as a secondary headphone, but nothing else. If it can't to hook the headphone volume, no option will be provided.
If more drastic change is needed, user has to reconfigure the whole topology by setting the initial pin configs.
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
What if there suddenly is "demand for such feature" for something that *does* change the topology a lot?
Again, it doesn't change the topology. If it needs to change the topology, it's not what we support in the mixer API. This needs reconfiguration, thus it essentially needs to restart the whole driver functionality.
Yes, this time around. But what about the next time there is a "demand for such feature" on a codec which has a different graph? Then you might not be so lucky that you can handle the same retasking without changing topology. That's the scenario I was wondering about.
At Mon, 03 Dec 2012 15:47:20 +0100, David Henningsson wrote:
On 12/03/2012 03:07 PM, Takashi Iwai wrote:
At Mon, 03 Dec 2012 14:43:37 +0100, David Henningsson wrote:
On 11/30/2012 10:39 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 09:46:29 +0100, David Henningsson wrote:
On 11/30/2012 08:57 AM, Takashi Iwai wrote:
At Fri, 30 Nov 2012 01:33:27 +0100, David Henningsson wrote: > I would recommend a user to use hda-jack-retask in these cases, instead > of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
I don't think any tool accessing hwdep to fiddle with the codec verb isn't suitable for normal usage. For any normal usage, the only access is via control API.
Trying to summarise this long thread, we seem to advocate these two options as I see it:
- Use the hwdep interface:
Pro: Simplest kernel code
Pro: Can support more options, higher degree of freedom for user
Con: Any reconfiguration is a bit disturbing and can e g break currently running audio streams.
Con: (Currently) requires root privileges to change, since it works through the hwdep interface.
Con: how can you ensure the exclusive setup and the exclusive access? Imagine multiple sessions are using different configurations.
Con: how would you manage the mixer elements from hwdep at all? There are independent mixer applications.
- Base reconfiguration on jack-mode controls
Pro: Simpler for user to reconfigure jacks
Con: More complex kernel code than first option
Con: It might be difficult to get the volume/switch control names right in all scenarios.
Con: Needs work in PulseAudios and GUIs to support jack retasking better, or it will be either unsupported (for the average user) or confusing.
Hmm... I don't get it. How can hda-jack-retask or whatever be a better option than PA? The integration work for PA is more or less necessary. Launching another application may be more confusion for user, no?
The integration work for PA in hda-jack-retask is such that hda-jack-retask kills PA before making a change (to avoid EBUSY and to make PA adjust to the new config).
But anyway. I agree that if we get a good mixer/kcontrol interface, the second solution has the advantage of an existing infrastructure for saving and restoring volumes between sessions, and support for different mixer application will already be in place.
Also; since the parsing code is still separate between codecs, we have to implement all of this complexity over and over again...
It's "still". They will be merged sooner or later, anyway.
I believe that when I see it - surprise me :-)
Hehe. It'll be likely earlier than proving Riemann hypothesis.
Probably we can sort out most stuff with option 2) too, I mean, with the capture source, automute and autoswitch, rerouting paths, repurposing DACs, and all that, but the question is really, should we? Are we willing to take the cost of the added complexity that comes with it? Are we be able to get it right, or do we get several broken kernel versions before we get something that works really well?
I don't think it'll be too complex in the end. We'll be reducing the code while merging such a new feature, especially by unifying the codes from multiple drivers. Once after this gets done, the driver will cover all needed demands for IDT, Conexant, Cirrus and VIA, and most of AD and C-Media codecs. Let's see.
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
Through impedance sensing? Does the ALC221 support that?
No. Otherwise it'd be easier :)
Ok, so what do you mean with "automatic"?
Sorry, it wasn't automatic "retasking" but muting.
Ok, that makes sense.
Windows have capabilities to tune each jack as well. This is a long-standing TODO, and I think "* Jack Mode" enum is the natural way to implement it. (How to let user setting it is another question. I can imagine that PA could ask user once when a jack is detected.)
Btw, how "disturbing" is it when you retask jacks on Windows? What happens e g if you try to retask a jack you're currently outputting audio to?
I don't know exactly, too.
But if we expose the capture source, it may conflict. One way to avoid it is to make the capture source inactive on the fly in auto mic mode. The question is how robust PA is, whether it's screwed up by such a change...
I don't know either...I mean, there is no dynamic reconfiguration, but whether it will crash or just simply ignore that it can't set the capture source I don't know. Try and see :-)
Btw, I was trying to figure out what PulseAudio version you're actually using for your enablement. I guess it's 0.9.23? [1]
Yeah, the primary requirement is that one, but I've been checking also with 2.0 and 2.1.
How important is it for you to actually get support for this stuff in PulseAudio? I mean, you're certainly more than capable of adding kcontrols, but I've never seen you - or anyone else from your team - submitting patches against PulseAudio to have support for these things all the way through to the user.
Honestly, I don't care at this moment at all. A part of the reason is that I don't use PulseAudio on my machines.
But for normal users, PA support would be a really "good to have".
Right, but you begin this thread by saying you didn't like the patch, but there was "demand for such feature". Doesn't the person/boss/customer/etc who demanded that feature care about it being accessible to the "normal users"?
Yes. It's not about the advanced user toying with the configuration. As I stated previously, the primary goal is to provide the headphone access from the front mic-in for normal users. Then I started from the generic auto-mic code improvement, and so on.
I think the auto-mic stuff isn't too bad, in general. But some particular changes, especially about the way of hooking the shared hp path, are pretty hackish. Maybe, the overall implementation could be better done by jack mode enums.
But still I think the method of creating more and more Jack Mode controls will lead to more long term maintenance problems (as the kernel code complexity increases), compared to implementing a more lightweight reconfiguration mechanism.
I don't think jack mode control itself would be too messy. From the top view, it provides either bias level and possibly I/O switch that makes this jack hooking to the existing output route. Thus, this doesn't disturb others.
Of course, your concern is about the combination of auto-mic and jack mode enums. That can be more different.
As an example. Say that we have three DACs with amp-outs. In standard configuration the first DAC's volume control is called "Speaker Volume" and the second DAC's volume is called "Headphone Volume". Suddenly you get a Jack Mode switch or multi-I/O switch, so that the three DACs now go to "Front", "C/LFE" and "Rear". The headphone must then be rerouted to the first DAC. But what happens to the volume controls now?
No. This doesn't happen.
The additional output via jack mode enum is just a hook to the existing I/O topology. It can hook to "Headphone" volume as a secondary headphone, but nothing else. If it can't to hook the headphone volume, no option will be provided.
If more drastic change is needed, user has to reconfigure the whole topology by setting the initial pin configs.
Instead of my previous patches, we can start of implementing jack mode enums at first. This allows the implementation of a lightweight jack retasking in user-space. Again, these enum controls won't provide the all possible retasking. They provide only cases where no drastic topology change is needed. This will serve most of user's demands, I guess. If not, user needs to create a better initial configuration for own demands.
What if there suddenly is "demand for such feature" for something that *does* change the topology a lot?
Again, it doesn't change the topology. If it needs to change the topology, it's not what we support in the mixer API. This needs reconfiguration, thus it essentially needs to restart the whole driver functionality.
Yes, this time around. But what about the next time there is a "demand for such feature" on a codec which has a different graph? Then you might not be so lucky that you can handle the same retasking without changing topology. That's the scenario I was wondering about.
In that case, yes, it won't be in the driver but will require a complete different approach. Typically, we provide a firmware "patch" to override the pin config or such for fulfilling the special demands. But if a dynamic reconfiguration is really mandatory, it'll be a tough issue, and I don't know what's the best way, too.
Takashi
Hi,
All things considered, if you believe you can make a Jack Mode implementation that is 1) Generic, i e, same code shared for all codecs 2) Free from regressions for the end user never touching ALSA and just running PulseAudio with GNOME/Unity UI. That includes all volume controls still working, non-existing ports should not show up, etc. 3) Bug free in other ways too ;-) 4) And do this within a kernel cycle, i e, without a few broken kernels (e g Realtek multi-IO implementation caused some regressions in 3.2 and 3.3)
...then I think you should go for it. But I would still see it as high risk. Maybe I should help testing somehow?
Then if PulseAudio doesn't support doing the retasking by default, then that's just something we have to work with in PulseAudio later on.
At Tue, 04 Dec 2012 14:11:15 +0100, David Henningsson wrote:
Hi,
All things considered, if you believe you can make a Jack Mode implementation that is
- Generic, i e, same code shared for all codecs
- Free from regressions for the end user never touching ALSA and just
running PulseAudio with GNOME/Unity UI. That includes all volume controls still working, non-existing ports should not show up, etc. 3) Bug free in other ways too ;-) 4) And do this within a kernel cycle, i e, without a few broken kernels (e g Realtek multi-IO implementation caused some regressions in 3.2 and 3.3)
Yes, these are all grand plan.
...then I think you should go for it. But I would still see it as high risk. Maybe I should help testing somehow?
Once when the code is ready, yes :)
Then if PulseAudio doesn't support doing the retasking by default, then that's just something we have to work with in PulseAudio later on.
Yes.
Takashi
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
I would recommend a user to use hda-jack-retask in these cases, instead of cluttering the kernel code.
Well, hda-jack-retask is a nice debugging tool, but it's not intended for the actual user interface for daily use.
Maybe we should consider making the reconfiguration more lightweight in
the kernel then, so that hda-jack-retask (or a more simplistic GUI) can be used for daily use?
only idt and realtek support dual headphones
the user are unlikely to use hda-jack-retask on the other hda codecs. via driver hardcode the headphone pin in widget set power state function
Also, a big missing piece is the automatic retasking via jack detection as a headphone.
This might work for this particular case, but sounds like it could become
a confusing set of alsa-mixer controls (and hairy kernel code to control them all!) - e g, what if two jacks that currently are not headphones both want to be headphones, and they have independent volume controls? Which one will be controlled by "Headphone Volume"?
http://www.alsa-project.org/db/?f=3cd5e75a351b4602609f7c478977cdb96d61c627
seem no headphone playback volume on z220 , line out and headphone share pcm playback volume and the mono speaker own speaker volume control
it is confusing when auto mic selection co exist with alt_capture device
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
I expect when any input jack(line in or mic) is retasked as output e.g. alc_set_multi_io() the corresponding jack detection kcontrol should change to false
At Fri, 30 Nov 2012 15:21:42 +0800, Raymond Yau wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
I expect when any input jack(line in or mic) is retasked as output e.g. alc_set_multi_io() the corresponding jack detection kcontrol should change to false
In the current implementation, this doesn't matter because the front panel has no capability of multi-io. The multi-io is enabled only when all jacks are located in the same place. On the front panel, there are only two.
But it's a thing to be considered in future if we'll need to extend such retasking functionalities more generically.
thanks,
Takashi
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
I expect when any input jack(line in or mic) is retasked as output e.g. alc_set_multi_io() the corresponding jack detection kcontrol should
change
to false
In the current implementation, this doesn't matter because the front panel has no capability of multi-io. The multi-io is enabled only when all jacks are located in the same place. On the front panel, there are only two.
But it's a thing to be considered in future if we'll need to extend such retasking functionalities more generically.
if mic_event change to hp_event in this case, does it mean that hp_event should change to mic_event in Q1-ultra
http://git.kernel.org/?p=linux/kernel/git/tiwai/sound.git;a=commitdiff;h=24d...
At Wed, 5 Dec 2012 16:20:29 +0800, Raymond Yau wrote:
HP Z220 with ALC221 codec has a front jack that should work as both the front mic and the secondary headphone. This patch adds a new mixer enum to change the jack behavior.
In the headphone mode, the auto-mic switching is disabled and the input is fixed to the rear line-in.
I expect when any input jack(line in or mic) is retasked as output e.g. alc_set_multi_io() the corresponding jack detection kcontrol should
change
to false
In the current implementation, this doesn't matter because the front panel has no capability of multi-io. The multi-io is enabled only when all jacks are located in the same place. On the front panel, there are only two.
But it's a thing to be considered in future if we'll need to extend such retasking functionalities more generically.
if mic_event change to hp_event in this case, does it mean that hp_event should change to mic_event in Q1-ultra
http://git.kernel.org/?p=linux/kernel/git/tiwai/sound.git;a=commitdiff;h=24d...
The code handling shared mic/HP of Q1 is totally different from the generic multi-io handling.
Takashi
Allocate the name string and assign the structure in alc_kcontrol_new() itself to reduce the code.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 54 +++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 35 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 95c5bc7..d3a4c2ca 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -913,22 +913,25 @@ static const struct snd_kcontrol_new alc_automute_mode_enum = { .put = alc_automute_mode_put, };
-static struct snd_kcontrol_new *alc_kcontrol_new(struct alc_spec *spec) +static struct snd_kcontrol_new * +alc_kcontrol_new(struct alc_spec *spec, const char *name, + const struct snd_kcontrol_new *temp) { - return snd_array_new(&spec->kctls); + struct snd_kcontrol_new *knew = snd_array_new(&spec->kctls); + if (!knew) + return NULL; + *knew = *temp; + knew->name = kstrdup(name, GFP_KERNEL); + if (!knew->name) + return NULL; + return knew; }
static int alc_add_automute_mode_enum(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; - struct snd_kcontrol_new *knew;
- knew = alc_kcontrol_new(spec); - if (!knew) - return -ENOMEM; - *knew = alc_automute_mode_enum; - knew->name = kstrdup("Auto-Mute Mode", GFP_KERNEL); - if (!knew->name) + if (!alc_kcontrol_new(spec, "Auto-Mute Mode", &alc_automute_mode_enum)) return -ENOMEM; return 0; } @@ -1769,12 +1772,9 @@ static const struct snd_kcontrol_new alc_inv_dmic_sw = { static int alc_add_inv_dmic_mixer(struct hda_codec *codec, hda_nid_t nid) { struct alc_spec *spec = codec->spec; - struct snd_kcontrol_new *knew = alc_kcontrol_new(spec); - if (!knew) - return -ENOMEM; - *knew = alc_inv_dmic_sw; - knew->name = kstrdup("Inverted Internal Mic Capture Switch", GFP_KERNEL); - if (!knew->name) + + if (!alc_kcontrol_new(spec, "Inverted Internal Mic Capture Switch", + &alc_inv_dmic_sw)) return -ENOMEM; spec->inv_dmic_fixup = 1; spec->inv_dmic_muted = 0; @@ -2550,13 +2550,9 @@ static int add_control(struct alc_spec *spec, int type, const char *name, { struct snd_kcontrol_new *knew;
- knew = alc_kcontrol_new(spec); + knew = alc_kcontrol_new(spec, name, &alc_control_templates[type]); if (!knew) return -ENOMEM; - *knew = alc_control_templates[type]; - knew->name = kstrdup(name, GFP_KERNEL); - if (!knew->name) - return -ENOMEM; knew->index = cidx; if (get_amp_nid_(val)) knew->subdevice = HDA_SUBDEV_AMP_FLAG; @@ -3999,14 +3995,8 @@ static int alc_auto_add_multi_channel_mode(struct hda_codec *codec) struct alc_spec *spec = codec->spec;
if (spec->multi_ios > 0) { - struct snd_kcontrol_new *knew; - - knew = alc_kcontrol_new(spec); - if (!knew) - return -ENOMEM; - *knew = alc_auto_channel_mode_enum; - knew->name = kstrdup("Channel Mode", GFP_KERNEL); - if (!knew->name) + if (!alc_kcontrol_new(spec, "Channel Mode", + &alc_auto_channel_mode_enum)) return -ENOMEM; } return 0; @@ -6064,7 +6054,6 @@ static const struct snd_kcontrol_new alc221_shared_fmic_hp_mode_enum = { static int alc221_add_shared_fmic_hp_mode(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; - struct snd_kcontrol_new *knew; char name[22];
if (spec->autocfg.hp_outs != 1 || spec->am_num_entries != 2) @@ -6074,12 +6063,7 @@ static int alc221_add_shared_fmic_hp_mode(struct hda_codec *codec) name, sizeof(name), NULL); strlcat(name, " Jack Mode", sizeof(name));
- knew = alc_kcontrol_new(spec); - if (!knew) - return -ENOMEM; - *knew = alc221_shared_fmic_hp_mode_enum; - knew->name = kstrdup(name, GFP_KERNEL); - if (!knew->name) + if (!alc_kcontrol_new(spec, name, &alc221_shared_fmic_hp_mode_enum)) return -ENOMEM; spec->shared_fmic_hp_nid = spec->am_entry[1].pin; spec->shared_fmic_hp_mode = 0;
This is yet more patche series I mentioned in the previous post. These should be applied on the top of previous patch set.
Takashi
Create "Capture Source" control even when the driver is running in the auto-mic mode. This allows user to switch to the non-standard input source.
When the input source is changed automatically via jack detection, the value change is notified to this control as well.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index d3a4c2ca..909a623 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -349,7 +349,7 @@ static void update_shared_mic_hp(struct hda_codec *codec, bool set_as_mic)
/* select the given imux item; either unmute exclusively or select the route */ static int alc_mux_select(struct hda_codec *codec, unsigned int adc_idx, - unsigned int idx, bool force) + unsigned int idx, bool force, bool notify) { struct alc_spec *spec = codec->spec; const struct hda_input_mux *imux; @@ -404,6 +404,16 @@ static int alc_mux_select(struct hda_codec *codec, unsigned int adc_idx, imux->items[idx].index); } alc_inv_dmic_sync(codec, true); + + if (notify) { + struct snd_ctl_elem_id id; + memset(&id, 0, sizeof(id)); + id.iface = SNDRV_CTL_ELEM_IFACE_MIXER; + strcpy(id.name, "Capture Source"); + snd_ctl_notify(codec->bus->card, SNDRV_CTL_EVENT_MASK_VALUE, + &id); + } + return 1; }
@@ -413,7 +423,7 @@ static int alc_mux_enum_put(struct snd_kcontrol *kcontrol, struct hda_codec *codec = snd_kcontrol_chip(kcontrol); unsigned int adc_idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); return alc_mux_select(codec, adc_idx, - ucontrol->value.enumerated.item[0], false); + ucontrol->value.enumerated.item[0], false, false); }
/* @@ -653,11 +663,12 @@ static void alc_mic_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
for (i = spec->am_num_entries - 1; i > 0; i--) { if (snd_hda_jack_detect(codec, spec->am_entry[i].pin)) { - alc_mux_select(codec, 0, spec->am_entry[i].idx, false); + alc_mux_select(codec, 0, spec->am_entry[i].idx, false, + true); return; } } - alc_mux_select(codec, 0, spec->am_entry[0].idx, false); + alc_mux_select(codec, 0, spec->am_entry[0].idx, false, true); }
/* update the master volume per volume-knob's unsol event */ @@ -1655,7 +1666,7 @@ static int alc_cap_sw_put(struct snd_kcontrol *kcontrol, { \ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \ /* .name = "Capture Source", */ \ - .name = "Input Source", \ + .name = num == 1 ? "Capture Source" : "Input Source", \ .count = num, \ .info = alc_mux_enum_info, \ .get = alc_mux_enum_get, \ @@ -4103,7 +4114,7 @@ static void alc_auto_init_input_src(struct hda_codec *codec) else nums = spec->num_adc_nids; for (c = 0; c < nums; c++) - alc_mux_select(codec, c, spec->cur_mux[c], true); + alc_mux_select(codec, c, spec->cur_mux[c], true, false); }
/* add mic boosts if needed */ @@ -4217,11 +4228,10 @@ static void set_capture_mixer(struct hda_codec *codec)
if (spec->input_mux && spec->input_mux->num_items > 1) mux = 1; - if (spec->auto_mic) { - num_adcs = 1; - mux = 0; - } else if (spec->dyn_adc_switch) + if (spec->auto_mic || spec->dyn_adc_switch) { num_adcs = 1; + mux = 1; + } if (!num_adcs) { if (spec->num_adc_nids > 3) spec->num_adc_nids = 3; @@ -6000,7 +6010,7 @@ static void alc221_shared_fmic_hp_mode_update(struct hda_codec *codec) pinctl = PIN_HP; mute = AMP_OUT_UNMUTE; spec->auto_mic = 0; - alc_mux_select(codec, 0, spec->am_entry[0].idx, false); + alc_mux_select(codec, 0, spec->am_entry[0].idx, false, true); /* choose the same DAC as the primary HP output */ idx = snd_hda_codec_read(codec, spec->autocfg.hp_pins[0], 0, AC_VERB_GET_CONNECT_SEL, 0);
Add another enum control to allow user explicitly enabling/disabling the automatic capture source selection via jack detection. This is a pretty similar like Auto-Mute Mode, but for inputs.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 53 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 909a623..aa32dd9 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -924,6 +924,57 @@ static const struct snd_kcontrol_new alc_automute_mode_enum = { .put = alc_automute_mode_put, };
+/* + * Auto-Mic mode mixer enum support + */ +static int alc_automic_mode_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char * const texts[] = { + "Disabled", "Enabled" + }; + + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + uinfo->count = 1; + uinfo->value.enumerated.items = 2; + if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items) + uinfo->value.enumerated.item = uinfo->value.enumerated.items - 1; + strcpy(uinfo->value.enumerated.name, + texts[uinfo->value.enumerated.item]); + return 0; +} + +static int alc_automic_mode_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct alc_spec *spec = codec->spec; + ucontrol->value.enumerated.item[0] = spec->auto_mic; + return 0; +} + +static int alc_automic_mode_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct alc_spec *spec = codec->spec; + unsigned int val = !!ucontrol->value.enumerated.item[0]; + + if (spec->auto_mic == val) + return 0; + spec->auto_mic = val; + alc_mic_automute(codec, NULL); + return 1; +} + +static const struct snd_kcontrol_new alc_automic_mode_enum = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Auto-Mic Mode", + .info = alc_automic_mode_info, + .get = alc_automic_mode_get, + .put = alc_automic_mode_put, +}; + static struct snd_kcontrol_new * alc_kcontrol_new(struct alc_spec *spec, const char *name, const struct snd_kcontrol_new *temp) @@ -1185,6 +1236,8 @@ static void alc_init_auto_mic(struct hda_codec *codec) spec->am_entry[0].pin, spec->am_entry[1].pin, spec->am_entry[2].pin); + + alc_kcontrol_new(spec, "Auto-Mic Mode", &alc_automic_mode_enum); }
/* check the availabilities of auto-mute and auto-mic switches */
To be conservative, turn off the auto-mic as default for non-laptop machines in order to be compatible with the older kernel.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index aa32dd9..fc70c0c 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -1238,6 +1238,10 @@ static void alc_init_auto_mic(struct hda_codec *codec) spec->am_entry[2].pin);
alc_kcontrol_new(spec, "Auto-Mic Mode", &alc_automic_mode_enum); + + /* enable auto mic only on machines with an internal mic as default */ + if (spec->am_entry[0].attr != INPUT_PIN_ATTR_INT) + spec->auto_mic = 0; }
/* check the availabilities of auto-mute and auto-mic switches */ @@ -2401,7 +2405,7 @@ static int alc_build_pcms(struct hda_codec *codec) * model, configure a second analog capture-only PCM. */ have_multi_adcs = (spec->num_adc_nids > 1) && - !spec->dyn_adc_switch && !spec->auto_mic && + !spec->dyn_adc_switch && !spec->auto_mic_valid_imux && (!spec->input_mux || spec->input_mux->num_items > 1); /* Additional Analaog capture for index #2 */ if (spec->alt_dac_nid || have_multi_adcs) { @@ -4114,6 +4118,7 @@ static void alc_remove_invalid_adc_nids(struct hda_codec *codec) codec->chip_name, spec->private_adc_nids[0]); spec->num_adc_nids = 1; spec->auto_mic = 0; + spec->auto_mic_valid_imux = 0; return; } } else if (nums != spec->num_adc_nids) { @@ -4281,7 +4286,7 @@ static void set_capture_mixer(struct hda_codec *codec)
if (spec->input_mux && spec->input_mux->num_items > 1) mux = 1; - if (spec->auto_mic || spec->dyn_adc_switch) { + if (spec->auto_mic_valid_imux || spec->dyn_adc_switch) { num_adcs = 1; mux = 1; }
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 55 +++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index fc70c0c..b3612a4 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -1002,12 +1002,12 @@ static int alc_add_automute_mode_enum(struct hda_codec *codec) * Check the availability of HP/line-out auto-mute; * Set up appropriately if really supported */ -static void alc_init_automute(struct hda_codec *codec) +static int alc_init_automute(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; struct auto_pin_cfg *cfg = &spec->autocfg; int present = 0; - int i; + int i, err;
if (cfg->hp_pins[0]) present++; @@ -1016,7 +1016,7 @@ static void alc_init_automute(struct hda_codec *codec) if (cfg->speaker_pins[0]) present++; if (present < 2) /* need two different output types */ - return; + return 0;
if (!cfg->speaker_pins[0] && cfg->line_out_type == AUTO_PIN_SPEAKER_OUT) { @@ -1066,9 +1066,13 @@ static void alc_init_automute(struct hda_codec *codec) spec->automute_lo = spec->automute_lo_possible; spec->automute_speaker = spec->automute_speaker_possible;
- if (spec->automute_speaker_possible || spec->automute_lo_possible) + if (spec->automute_speaker_possible || spec->automute_lo_possible) { /* create a control for automute mode */ - alc_add_automute_mode_enum(codec); + err = alc_add_automute_mode_enum(codec); + if (err < 0) + return err; + } + return 0; }
/* return the position of NID in the list, or -1 if not found */ @@ -1175,7 +1179,7 @@ static int compare_attr(const void *ap, const void *bp) * Check the availability of auto-mic switch; * Set up if really supported */ -static void alc_init_auto_mic(struct hda_codec *codec) +static int alc_init_auto_mic(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; struct auto_pin_cfg *cfg = &spec->autocfg; @@ -1183,7 +1187,7 @@ static void alc_init_auto_mic(struct hda_codec *codec) int i, num_pins;
if (spec->shared_mic_hp) - return; /* no auto-mic for the shared I/O */ + return 0; /* no auto-mic for the shared I/O */
types = 0; num_pins = 0; @@ -1194,23 +1198,23 @@ static void alc_init_auto_mic(struct hda_codec *codec) attr = snd_hda_codec_get_pincfg(codec, nid); attr = snd_hda_get_input_pin_attr(attr); if (types & (1 << attr)) - return; /* already occupied */ + return 0; /* already occupied */ switch (attr) { case INPUT_PIN_ATTR_INT: if (cfg->inputs[i].type != AUTO_PIN_MIC) - return; /* invalid type */ + return 0; /* invalid type */ break; case INPUT_PIN_ATTR_UNUSED: - return; /* invalid entry */ + return 0; /* invalid entry */ default: if (cfg->inputs[i].type > AUTO_PIN_LINE_IN) - return; /* invalid type */ + return 0; /* invalid type */ if (!is_jack_detectable(codec, nid)) - return; /* no unsol support */ + return 0; /* no unsol support */ break; } if (num_pins >= MAX_AUTO_MIC_PINS) - return; + return 0; types |= (1 << attr); spec->am_entry[num_pins].pin = nid; spec->am_entry[num_pins].attr = attr; @@ -1218,7 +1222,7 @@ static void alc_init_auto_mic(struct hda_codec *codec) }
if (num_pins < 2) - return; + return 0;
spec->am_num_entries = num_pins; /* sort the am_entry in the order of attr so that the pin with a @@ -1230,25 +1234,34 @@ static void alc_init_auto_mic(struct hda_codec *codec) /* check imux indices */ spec->auto_mic = 1; if (!alc_auto_mic_check_imux(codec)) - return; + return 0;
snd_printdd("realtek: Enable auto-mic switch on NID 0x%x/0x%x/0x%x\n", spec->am_entry[0].pin, spec->am_entry[1].pin, spec->am_entry[2].pin);
- alc_kcontrol_new(spec, "Auto-Mic Mode", &alc_automic_mode_enum); + if (!alc_kcontrol_new(spec, "Auto-Mic Mode", &alc_automic_mode_enum)) + return -ENOMEM;
/* enable auto mic only on machines with an internal mic as default */ if (spec->am_entry[0].attr != INPUT_PIN_ATTR_INT) spec->auto_mic = 0; + return 0; }
/* check the availabilities of auto-mute and auto-mic switches */ -static void alc_auto_check_switches(struct hda_codec *codec) +static int alc_auto_check_switches(struct hda_codec *codec) { - alc_init_automute(codec); - alc_init_auto_mic(codec); + int err; + + err = alc_init_automute(codec); + if (err < 0) + return err; + err = alc_init_auto_mic(codec); + if (err < 0) + return err; + return 0; }
/* @@ -4419,7 +4432,9 @@ static int alc_parse_auto_config(struct hda_codec *codec, alc_ssid_check(codec, ssid_nids);
if (!spec->no_analog) { - alc_auto_check_switches(codec); + err = alc_auto_check_switches(codec); + if (err < 0) + return err; err = alc_auto_add_mic_boost(codec); if (err < 0) return err;
When the auto-mic mode is enabled, changing the input source externally conflicts with the automatic selection mechanism. Make the control inactive when auto mic is set.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index b3612a4..3236645 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -347,6 +347,11 @@ static void update_shared_mic_hp(struct hda_codec *codec, bool set_as_mic) call_update_outputs(codec); }
+static struct snd_ctl_elem_id capture_source_id = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Capture Source" +}; + /* select the given imux item; either unmute exclusively or select the route */ static int alc_mux_select(struct hda_codec *codec, unsigned int adc_idx, unsigned int idx, bool force, bool notify) @@ -406,12 +411,8 @@ static int alc_mux_select(struct hda_codec *codec, unsigned int adc_idx, alc_inv_dmic_sync(codec, true);
if (notify) { - struct snd_ctl_elem_id id; - memset(&id, 0, sizeof(id)); - id.iface = SNDRV_CTL_ELEM_IFACE_MIXER; - strcpy(id.name, "Capture Source"); snd_ctl_notify(codec->bus->card, SNDRV_CTL_EVENT_MASK_VALUE, - &id); + &capture_source_id); }
return 1; @@ -703,9 +704,13 @@ static void alc880_unsol_event(struct hda_codec *codec, unsigned int res) /* call init functions of standard auto-mute helpers */ static void alc_inithook(struct hda_codec *codec) { + struct alc_spec *spec = codec->spec; alc_hp_automute(codec, NULL); alc_line_automute(codec, NULL); alc_mic_automute(codec, NULL); + if (spec->auto_mic_valid_imux) + snd_ctl_activate_id(codec->bus->card, &capture_source_id, + !spec->auto_mic); }
/* additional initialization for ALC888 variants */ @@ -964,6 +969,7 @@ static int alc_automic_mode_put(struct snd_kcontrol *kcontrol, return 0; spec->auto_mic = val; alc_mic_automute(codec, NULL); + snd_ctl_activate_id(codec->bus->card, &capture_source_id, !val); return 1; }
participants (3)
-
David Henningsson -
Raymond Yau -
Takashi Iwai