* [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_*
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
@ 2012-11-29 15:21 ` Takashi Iwai
2012-11-29 23:54 ` David Henningsson
2012-11-29 15:21 ` [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs Takashi Iwai
` (4 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-11-29 15:21 UTC (permalink / raw)
To: alsa-devel
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;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_*
2012-11-29 15:21 ` [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_* Takashi Iwai
@ 2012-11-29 23:54 ` David Henningsson
2012-11-30 7:03 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-11-29 23:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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"
> };
> 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;
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_*
2012-11-29 23:54 ` David Henningsson
@ 2012-11-30 7:03 ` Takashi Iwai
2012-11-30 7:31 ` David Henningsson
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 7:03 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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"
> > };
> > 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;
> >
>
>
>
> --
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_*
2012-11-30 7:03 ` Takashi Iwai
@ 2012-11-30 7:31 ` David Henningsson
0 siblings, 0 replies; 37+ messages in thread
From: David Henningsson @ 2012-11-30 7:31 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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"
>>> };
>>> 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;
>>>
>>
>>
>>
>> --
>> David Henningsson, Canonical Ltd.
>> https://launchpad.net/~diwic
>>
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
2012-11-29 15:21 ` [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_* Takashi Iwai
@ 2012-11-29 15:21 ` Takashi Iwai
2012-11-30 0:11 ` David Henningsson
2012-12-02 1:55 ` Raymond Yau
2012-11-29 15:21 ` [PATCH 3/5] ALSA: hda - Use standard sort function in hda_auto_parser.c Takashi Iwai
` (3 subsequent siblings)
5 siblings, 2 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-29 15:21 UTC (permalink / raw)
To: alsa-devel
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]);
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-11-29 15:21 ` [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs Takashi Iwai
@ 2012-11-30 0:11 ` David Henningsson
2012-11-30 7:17 ` Takashi Iwai
2012-12-02 1:55 ` Raymond Yau
1 sibling, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-11-30 0:11 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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) {
> 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]);
> }
>
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-11-30 0:11 ` David Henningsson
@ 2012-11-30 7:17 ` Takashi Iwai
2012-11-30 7:54 ` David Henningsson
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 7:17 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-11-30 7:17 ` Takashi Iwai
@ 2012-11-30 7:54 ` David Henningsson
0 siblings, 0 replies; 37+ messages in thread
From: David Henningsson @ 2012-11-30 7:54 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-11-29 15:21 ` [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs Takashi Iwai
2012-11-30 0:11 ` David Henningsson
@ 2012-12-02 1:55 ` Raymond Yau
2012-12-03 8:52 ` Takashi Iwai
1 sibling, 1 reply; 37+ messages in thread
From: Raymond Yau @ 2012-12-02 1:55 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
>
> 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-12-02 1:55 ` Raymond Yau
@ 2012-12-03 8:52 ` Takashi Iwai
2012-12-03 10:55 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-12-03 8:52 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs
2012-12-03 8:52 ` Takashi Iwai
@ 2012-12-03 10:55 ` Takashi Iwai
0 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-12-03 10:55 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
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);
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/5] ALSA: hda - Use standard sort function in hda_auto_parser.c
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
2012-11-29 15:21 ` [PATCH 1/5] ALSA: hda - Rearrange INPUT_PIN_ATTR_* Takashi Iwai
2012-11-29 15:21 ` [PATCH 2/5] ALSA: hda - More generic auto-mic switching for Realtek codecs Takashi Iwai
@ 2012-11-29 15:21 ` Takashi Iwai
2012-11-29 15:21 ` [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220 Takashi Iwai
` (2 subsequent siblings)
5 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-29 15:21 UTC (permalink / raw)
To: alsa-devel
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
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
` (2 preceding siblings ...)
2012-11-29 15:21 ` [PATCH 3/5] ALSA: hda - Use standard sort function in hda_auto_parser.c Takashi Iwai
@ 2012-11-29 15:21 ` Takashi Iwai
2012-11-30 0:33 ` David Henningsson
2012-11-30 7:21 ` Raymond Yau
2012-11-29 15:21 ` [PATCH 5/5] ALSA: hda - Refactor alc_kcontrol_new() usages Takashi Iwai
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
5 siblings, 2 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-29 15:21 UTC (permalink / raw)
To: alsa-devel
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),
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-29 15:21 ` [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220 Takashi Iwai
@ 2012-11-30 0:33 ` David Henningsson
2012-11-30 7:57 ` Takashi Iwai
2012-11-30 7:21 ` Raymond Yau
1 sibling, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-11-30 0:33 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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),
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 0:33 ` David Henningsson
@ 2012-11-30 7:57 ` Takashi Iwai
2012-11-30 8:46 ` David Henningsson
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 7:57 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 7:57 ` Takashi Iwai
@ 2012-11-30 8:46 ` David Henningsson
2012-11-30 9:39 ` Takashi Iwai
2012-12-04 2:48 ` Raymond Yau
0 siblings, 2 replies; 37+ messages in thread
From: David Henningsson @ 2012-11-30 8:46 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
[1]
https://www.suse.com/LinuxPackages/packageRouter.jsp?product=desktop&version=11&service_pack=sp2&architecture=i386&package_name=index_all
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 8:46 ` David Henningsson
@ 2012-11-30 9:39 ` Takashi Iwai
2012-11-30 10:12 ` Takashi Iwai
2012-12-03 13:43 ` David Henningsson
2012-12-04 2:48 ` Raymond Yau
1 sibling, 2 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 9:39 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 9:39 ` Takashi Iwai
@ 2012-11-30 10:12 ` Takashi Iwai
2012-12-03 13:43 ` David Henningsson
1 sibling, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 10:12 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 9:39 ` Takashi Iwai
2012-11-30 10:12 ` Takashi Iwai
@ 2012-12-03 13:43 ` David Henningsson
2012-12-03 14:07 ` Takashi Iwai
1 sibling, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-12-03 13:43 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-03 13:43 ` David Henningsson
@ 2012-12-03 14:07 ` Takashi Iwai
2012-12-03 14:47 ` David Henningsson
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-12-03 14:07 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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:
>
> 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.
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.
> 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.
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-03 14:07 ` Takashi Iwai
@ 2012-12-03 14:47 ` David Henningsson
2012-12-03 15:04 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-12-03 14:47 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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:
>>
>> 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.
>
> 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.
>
>> 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.
>
> 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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-03 14:47 ` David Henningsson
@ 2012-12-03 15:04 ` Takashi Iwai
2012-12-04 13:11 ` David Henningsson
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-12-03 15:04 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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:
> >>
> >> 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.
> >
> > 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.
> >
> >> 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.
> >
> > 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-03 15:04 ` Takashi Iwai
@ 2012-12-04 13:11 ` David Henningsson
2012-12-04 13:45 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: David Henningsson @ 2012-12-04 13:11 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-04 13:11 ` David Henningsson
@ 2012-12-04 13:45 ` Takashi Iwai
0 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-12-04 13:45 UTC (permalink / raw)
To: David Henningsson; +Cc: alsa-devel
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
> 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)
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 8:46 ` David Henningsson
2012-11-30 9:39 ` Takashi Iwai
@ 2012-12-04 2:48 ` Raymond Yau
1 sibling, 0 replies; 37+ messages in thread
From: Raymond Yau @ 2012-12-04 2:48 UTC (permalink / raw)
To: David Henningsson; +Cc: Takashi Iwai, alsa-devel
>
>>>> 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-29 15:21 ` [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220 Takashi Iwai
2012-11-30 0:33 ` David Henningsson
@ 2012-11-30 7:21 ` Raymond Yau
2012-11-30 7:59 ` Takashi Iwai
1 sibling, 1 reply; 37+ messages in thread
From: Raymond Yau @ 2012-11-30 7:21 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
>
> 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 7:21 ` Raymond Yau
@ 2012-11-30 7:59 ` Takashi Iwai
2012-12-05 8:20 ` Raymond Yau
0 siblings, 1 reply; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 7:59 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-11-30 7:59 ` Takashi Iwai
@ 2012-12-05 8:20 ` Raymond Yau
2012-12-05 8:27 ` Takashi Iwai
0 siblings, 1 reply; 37+ messages in thread
From: Raymond Yau @ 2012-12-05 8:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
> >
> > > 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=24de183ed0369d73af89e6752fcc1ecbde59053d;hp=82e14a4754599952f5504c1f696bbc77bdfd6009
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220
2012-12-05 8:20 ` Raymond Yau
@ 2012-12-05 8:27 ` Takashi Iwai
0 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-12-05 8:27 UTC (permalink / raw)
To: Raymond Yau; +Cc: alsa-devel
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=24de183ed0369d73af89e6752fcc1ecbde59053d;hp=82e14a4754599952f5504c1f696bbc77bdfd6009
The code handling shared mic/HP of Q1 is totally different from the
generic multi-io handling.
Takashi
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 5/5] ALSA: hda - Refactor alc_kcontrol_new() usages
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
` (3 preceding siblings ...)
2012-11-29 15:21 ` [PATCH 4/5] ALSA: hda - Implement shared front mic/hp jack for HP Z220 Takashi Iwai
@ 2012-11-29 15:21 ` Takashi Iwai
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
5 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-29 15:21 UTC (permalink / raw)
To: alsa-devel
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;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 0/5] More patches for Relatek auto-mic things
2012-11-29 15:21 [PATCH 0/5] Realtek auto-mic enhancements / cleanups Takashi Iwai
` (4 preceding siblings ...)
2012-11-29 15:21 ` [PATCH 5/5] ALSA: hda - Refactor alc_kcontrol_new() usages Takashi Iwai
@ 2012-11-30 8:15 ` Takashi Iwai
2012-11-30 8:15 ` [PATCH 1/5] ALSA: hda - Create Capture Source enum even for auto-mic mode Takashi Iwai
` (4 more replies)
5 siblings, 5 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:15 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
This is yet more patche series I mentioned in the previous post.
These should be applied on the top of previous patch set.
Takashi
^ permalink raw reply [flat|nested] 37+ messages in thread* [PATCH 1/5] ALSA: hda - Create Capture Source enum even for auto-mic mode
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
@ 2012-11-30 8:15 ` Takashi Iwai
2012-11-30 8:16 ` [PATCH 2/5] ALSA: hda - Add Auto-Mic Mode enum to Realtek codecs Takashi Iwai
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:15 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
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);
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 2/5] ALSA: hda - Add Auto-Mic Mode enum to Realtek codecs
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
2012-11-30 8:15 ` [PATCH 1/5] ALSA: hda - Create Capture Source enum even for auto-mic mode Takashi Iwai
@ 2012-11-30 8:16 ` Takashi Iwai
2012-11-30 8:16 ` [PATCH 3/5] ALSA: hda - Disable auto-mic as default for non-laptop machines Takashi Iwai
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:16 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
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 */
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 3/5] ALSA: hda - Disable auto-mic as default for non-laptop machines
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
2012-11-30 8:15 ` [PATCH 1/5] ALSA: hda - Create Capture Source enum even for auto-mic mode Takashi Iwai
2012-11-30 8:16 ` [PATCH 2/5] ALSA: hda - Add Auto-Mic Mode enum to Realtek codecs Takashi Iwai
@ 2012-11-30 8:16 ` Takashi Iwai
2012-11-30 8:16 ` [PATCH 4/5] ALSA: hda - Pass errors properly in alc_auto_check_switches() Takashi Iwai
2012-11-30 8:16 ` [PATCH 5/5] ALSA: hda - Activate Capture Source selection only when auto_mic=0 Takashi Iwai
4 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:16 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
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;
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 4/5] ALSA: hda - Pass errors properly in alc_auto_check_switches()
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
` (2 preceding siblings ...)
2012-11-30 8:16 ` [PATCH 3/5] ALSA: hda - Disable auto-mic as default for non-laptop machines Takashi Iwai
@ 2012-11-30 8:16 ` Takashi Iwai
2012-11-30 8:16 ` [PATCH 5/5] ALSA: hda - Activate Capture Source selection only when auto_mic=0 Takashi Iwai
4 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:16 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
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;
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH 5/5] ALSA: hda - Activate Capture Source selection only when auto_mic=0
2012-11-30 8:15 ` [PATCH 0/5] More patches for Relatek auto-mic things Takashi Iwai
` (3 preceding siblings ...)
2012-11-30 8:16 ` [PATCH 4/5] ALSA: hda - Pass errors properly in alc_auto_check_switches() Takashi Iwai
@ 2012-11-30 8:16 ` Takashi Iwai
4 siblings, 0 replies; 37+ messages in thread
From: Takashi Iwai @ 2012-11-30 8:16 UTC (permalink / raw)
To: alsa-devel; +Cc: David Henningsson
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;
}
--
1.8.0.1
^ permalink raw reply related [flat|nested] 37+ messages in thread