[alsa-devel] [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls
Currently only hda will create kctls for hda jacks, for asoc, people may need create jack kctls in specific driver.
Here we are introducing kctls for each jack, by creating kctls and add them to jack controls list (considering exist of combo jack). At the same time, we will report events for each control in the list.
With this implement, we are no longer need hda jack specific kctls, so here also remove jack kctls for hda.
Any comments are welcome.
Jie Yang (2): ALSA: jack: create jack kcontrols for every jack input device ALSA: hda - Remove jack kctls
include/sound/jack.h | 8 ++++ sound/core/Kconfig | 3 -- sound/core/Makefile | 3 +- sound/core/jack.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++- sound/pci/hda/Kconfig | 10 +---- sound/pci/hda/hda_jack.c | 45 ++-------------------- sound/pci/hda/hda_jack.h | 3 -- 7 files changed, 110 insertions(+), 61 deletions(-)
Currently only hda will create kctls for hda jacks, for asoc, people may need create jack kctls in specific driver.
Here we are introducing kctls for each jack, by creating kctls and add them to jack controls list (considering exist of combo jack). At the same time, we will report events for each control in the list.
With this implement, we are no longer need hda jack kctls, so here also remove jack kctls for hda.
Any comments are welcome.
Jie Yang (2): ALSA: jack: create jack kcontrols for every jack input device ALSA: hda - Remove jack kctls
include/sound/jack.h | 8 ++++ sound/core/Kconfig | 3 -- sound/core/Makefile | 3 +- sound/core/jack.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++- sound/pci/hda/Kconfig | 10 +---- sound/pci/hda/hda_jack.c | 45 ++-------------------- sound/pci/hda/hda_jack.h | 3 -- 7 files changed, 110 insertions(+), 61 deletions(-)
Currently the ALSA jack core registers only input devices for each jack registered. These jack input devices are not readable by userspace devices that run as non root.
This patch adds support for additionally registering jack kcontrol devices for every input jack registered. This allows non root userspace to read jack status.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com Modified-by: Jie Yang yang.jie@intel.com Signed-off-by: Jie Yang yang.jie@intel.com Reveiwed-by: Mark Brown broonie@kernel.org --- include/sound/jack.h | 8 +++++ sound/core/jack.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 2182350..ef0f0ed 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -73,6 +73,8 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev; + struct list_head kctl_list; + struct snd_card *card; int registered; int type; const char *id; @@ -82,6 +84,12 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl { + struct snd_kcontrol *kctl; + struct list_head jack_list; /* list of controls belong to the same jack*/ + unsigned int jack_bit_idx; /* the corresponding jack type bit index */ +}; + #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type, diff --git a/sound/core/jack.c b/sound/core/jack.c index 8658578..741924f 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -54,7 +55,13 @@ static int snd_jack_dev_disconnect(struct snd_device *device) static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data; + struct snd_card *card = device->card; + struct snd_jack_kctl *jack_kctl, *tmp_jack_kctl;
+ list_for_each_entry_safe(jack_kctl, tmp_jack_kctl, &jack->kctl_list, jack_list) { + list_del(&jack_kctl->jack_list); + snd_ctl_remove(card, jack_kctl->kctl); + } if (jack->private_free) jack->private_free(jack);
@@ -100,6 +107,73 @@ static int snd_jack_dev_register(struct snd_device *device) return err; }
+ +/* get the first unused/available index number for the given kctl name */ +static int get_available_index(struct snd_card *card, const char *name) +{ + struct snd_kcontrol *kctl; + int idx = 0; + int len = strlen(name); + + down_write(&card->controls_rwsem); + next: + list_for_each_entry(kctl, &card->controls, list) { + if (!strncmp(name, kctl->id.name, len) && + !strcmp(" Jack", kctl->id.name + len) && + kctl->id.index == idx) { + idx++; + goto next; + } + } + up_write(&card->controls_rwsem); + return idx; +} + +static void kctl_private_free(struct snd_kcontrol *kctl) +{ + struct snd_jack_kctl *jack_kctl = kctl->private_data; + list_del(&jack_kctl->jack_list); +} + +static int snd_jack_new_kctl(struct snd_card *card, struct snd_jack *jack, int type) +{ + struct snd_kcontrol *kctl; + struct snd_jack_kctl *jack_kctl; + int i, err, index, state = 0 /* use 0 for default state ?? */; + + INIT_LIST_HEAD(&jack->kctl_list); + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { + int testbit = 1 << i; + if (type & testbit) { + index = get_available_index(card,jack->id); + kctl = snd_kctl_jack_new(jack->id, index, card); + if (!kctl) + return -ENOMEM; + + err = snd_ctl_add(card, kctl); + if (err < 0) + return err; + + jack_kctl = kzalloc(sizeof(*jack_kctl), GFP_KERNEL); + + jack_kctl->kctl = kctl; + + kctl->private_data = jack_kctl; + kctl->private_free = kctl_private_free; + + /* use jack_bit_idx for the kctl type bit */ + jack_kctl->jack_bit_idx = i; + + list_add_tail(&jack_kctl->jack_list, &jack->kctl_list); + + /* set initial jack state */ + snd_kctl_jack_report(card, kctl, state & testbit); + } + } + + return 0; +} + /** * snd_jack_new - Create a new jack * @card: the card instance @@ -138,7 +212,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, }
jack->input_dev->phys = "ALSA"; - + jack->card = card; jack->type = type;
for (i = 0; i < SND_JACK_SWITCH_TYPES; i++) @@ -150,10 +224,17 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, if (err < 0) goto fail_input;
+ /* register jack kcontrols */ + err = snd_jack_new_kctl(card, jack, type); + if (err < 0) + goto fail_kctl; + *jjack = jack;
return 0;
+fail_kctl: + snd_device_free(card, jack); fail_input: input_free_device(jack->input_dev); kfree(jack->id); @@ -230,6 +311,7 @@ EXPORT_SYMBOL(snd_jack_set_key); */ void snd_jack_report(struct snd_jack *jack, int status) { + struct snd_jack_kctl *jack_kctl; int i;
if (!jack) @@ -245,13 +327,26 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i; - if (jack->type & testbit) + if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit); + } }
input_sync(jack->input_dev); + + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { + int testbit = 1 << i; + if (jack->type & testbit) { + list_for_each_entry(jack_kctl, &jack->kctl_list, jack_list) { + if (jack_kctl->jack_bit_idx == i) { + snd_kctl_jack_report(jack->card, jack_kctl->kctl, + status & testbit); + } + } + } + } } EXPORT_SYMBOL(snd_jack_report);
At Fri, 20 Mar 2015 16:55:18 +0800, Jie Yang wrote:
Currently the ALSA jack core registers only input devices for each jack registered. These jack input devices are not readable by userspace devices that run as non root.
This patch adds support for additionally registering jack kcontrol devices for every input jack registered. This allows non root userspace to read jack status.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com Modified-by: Jie Yang yang.jie@intel.com Signed-off-by: Jie Yang yang.jie@intel.com Reveiwed-by: Mark Brown broonie@kernel.org
include/sound/jack.h | 8 +++++ sound/core/jack.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 2182350..ef0f0ed 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -73,6 +73,8 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
- struct list_head kctl_list;
- struct snd_card *card; int registered; int type; const char *id;
@@ -82,6 +84,12 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl {
- struct snd_kcontrol *kctl;
- struct list_head jack_list; /* list of controls belong to the same jack*/
- unsigned int jack_bit_idx; /* the corresponding jack type bit index */
You can omit jack_ prefix here.
+};
#ifdef CONFIG_SND_JACK
Remove CONFIG_SND_JACK, both from Kconfig and ifdefs, as now it's always enabled together with CONFIG_SND_JACK. (Or do it later in the patch series.)
int snd_jack_new(struct snd_card *card, const char *id, int type, diff --git a/sound/core/jack.c b/sound/core/jack.c index 8658578..741924f 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -54,7 +55,13 @@ static int snd_jack_dev_disconnect(struct snd_device *device) static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data;
struct snd_card *card = device->card;
struct snd_jack_kctl *jack_kctl, *tmp_jack_kctl;
list_for_each_entry_safe(jack_kctl, tmp_jack_kctl, &jack->kctl_list, jack_list) {
list_del(&jack_kctl->jack_list);
Use list_del_init(). Otherwise it'll strike back. (list_del() will be called again in private_free callback from snd_ctl_remove(), and this can be broken.)
snd_ctl_remove(card, jack_kctl->kctl);- } if (jack->private_free) jack->private_free(jack);
@@ -100,6 +107,73 @@ static int snd_jack_dev_register(struct snd_device *device) return err; }
+/* get the first unused/available index number for the given kctl name */ +static int get_available_index(struct snd_card *card, const char *name) +{
- struct snd_kcontrol *kctl;
- int idx = 0;
- int len = strlen(name);
- down_write(&card->controls_rwsem);
Use down_read(), as Liam already suggested.
- next:
- list_for_each_entry(kctl, &card->controls, list) {
if (!strncmp(name, kctl->id.name, len) &&!strcmp(" Jack", kctl->id.name + len) &&kctl->id.index == idx) {idx++;goto next;}- }
Better to split a small function, e.g.
while (!jack_ctl_name_found(card, kctl)) idx++;
than using ugly goto.
- up_write(&card->controls_rwsem);
- return idx;
+}
+static void kctl_private_free(struct snd_kcontrol *kctl) +{
- struct snd_jack_kctl *jack_kctl = kctl->private_data;
- list_del(&jack_kctl->jack_list);
kfree() is forgotten.
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct snd_jack *jack, int type) +{
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ?? */;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {
With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for headset?
In the case of input device, the situation is a bit different, so we shouldn't mix up.
index = get_available_index(card,jack->id);kctl = snd_kctl_jack_new(jack->id, index, card);if (!kctl)return -ENOMEM;err = snd_ctl_add(card, kctl);if (err < 0)return err;jack_kctl = kzalloc(sizeof(*jack_kctl), GFP_KERNEL);
Missing NULL check.
jack_kctl->kctl = kctl;kctl->private_data = jack_kctl;kctl->private_free = kctl_private_free;/* use jack_bit_idx for the kctl type bit */jack_kctl->jack_bit_idx = i;
This can be better to hold bit mask instead of bit index. Then in the below...
@@ -245,13 +327,26 @@ void snd_jack_report(struct snd_jack *jack, int status)
....
}
input_sync(jack->input_dev);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (jack->type & testbit) {list_for_each_entry(jack_kctl, &jack->kctl_list, jack_list) {if (jack_kctl->jack_bit_idx == i) {snd_kctl_jack_report(jack->card, jack_kctl->kctl,status & testbit);}
.... you can reduce to a single loop.
thanks,
Takashi
At Fri, 20 Mar 2015 10:27:37 +0100, Takashi Iwai wrote:
+};
#ifdef CONFIG_SND_JACK
Remove CONFIG_SND_JACK, both from Kconfig and ifdefs, as now it's always enabled together with CONFIG_SND_JACK. (Or do it later in the patch series.)
Disregard this comment, I was confused about CONFIG_SND_KCTL_JACK.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 5:28 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 16:55:18 +0800, Jie Yang wrote:
Currently the ALSA jack core registers only input devices for each jack registered. These jack input devices are not readable by userspace devices that run as non root.
This patch adds support for additionally registering jack kcontrol devices for every input jack registered. This allows non root userspace to read jack status.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com Modified-by: Jie Yang yang.jie@intel.com Signed-off-by: Jie Yang yang.jie@intel.com Reveiwed-by: Mark Brown broonie@kernel.org
include/sound/jack.h | 8 +++++ sound/core/jack.c | 99
++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 2182350..ef0f0ed 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -73,6 +73,8 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
- struct list_head kctl_list;
- struct snd_card *card; int registered; int type; const char *id;
@@ -82,6 +84,12 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl {
- struct snd_kcontrol *kctl;
- struct list_head jack_list; /* list of controls belong to
the same jack*/
- unsigned int jack_bit_idx; /* the corresponding jack type bit
index */
You can omit jack_ prefix here.
[Keyon] OK.
+};
#ifdef CONFIG_SND_JACK
Remove CONFIG_SND_JACK, both from Kconfig and ifdefs, as now it's always enabled together with CONFIG_SND_JACK. (Or do it later in the patch series.)
int snd_jack_new(struct snd_card *card, const char *id, int type, diff --git a/sound/core/jack.c b/sound/core/jack.c index 8658578..741924f 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -54,7 +55,13 @@ static int snd_jack_dev_disconnect(struct snd_device *device) static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data;
struct snd_card *card = device->card;
struct snd_jack_kctl *jack_kctl, *tmp_jack_kctl;
list_for_each_entry_safe(jack_kctl, tmp_jack_kctl, &jack->kctl_list,
jack_list) {
list_del(&jack_kctl->jack_list);Use list_del_init(). Otherwise it'll strike back. (list_del() will be called again in private_free callback from snd_ctl_remove(), and this can be broken.)
[Keyon] good! Thanks for pointing out this potential risk!
snd_ctl_remove(card, jack_kctl->kctl);- } if (jack->private_free) jack->private_free(jack);
@@ -100,6 +107,73 @@ static int snd_jack_dev_register(struct snd_device
*device)
return err; }
+/* get the first unused/available index number for the given kctl +name */ static int get_available_index(struct snd_card *card, const +char *name) {
- struct snd_kcontrol *kctl;
- int idx = 0;
- int len = strlen(name);
- down_write(&card->controls_rwsem);
Use down_read(), as Liam already suggested.
- next:
- list_for_each_entry(kctl, &card->controls, list) {
if (!strncmp(name, kctl->id.name, len) &&!strcmp(" Jack", kctl->id.name + len) &&kctl->id.index == idx) {idx++;goto next;}- }
Better to split a small function, e.g.
while (!jack_ctl_name_found(card, kctl)) idx++;
than using ugly goto.
[Keyon] OK, will do like that.
- up_write(&card->controls_rwsem);
- return idx;
+}
+static void kctl_private_free(struct snd_kcontrol *kctl) {
- struct snd_jack_kctl *jack_kctl = kctl->private_data;
- list_del(&jack_kctl->jack_list);
kfree() is forgotten.
[Keyon] right.
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct snd_jack +*jack, int type) {
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ?? */;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g. headset. Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
In the case of input device, the situation is a bit different, so we shouldn't mix up.
index = get_available_index(card,jack->id);kctl = snd_kctl_jack_new(jack->id, index, card);if (!kctl)return -ENOMEM;err = snd_ctl_add(card, kctl);if (err < 0)return err;jack_kctl = kzalloc(sizeof(*jack_kctl), GFP_KERNEL);Missing NULL check.
[Keyon] got it, add it soon.
jack_kctl->kctl = kctl;kctl->private_data = jack_kctl;kctl->private_free = kctl_private_free;/* use jack_bit_idx for the kctl type bit */jack_kctl->jack_bit_idx = i;This can be better to hold bit mask instead of bit index.
[Keyon] good idea, will change to bit mask.
Then in the below...
@@ -245,13 +327,26 @@ void snd_jack_report(struct snd_jack *jack, int status)
....
}
input_sync(jack->input_dev);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (jack->type & testbit) {list_for_each_entry(jack_kctl, &jack->kctl_list,jack_list) {
if (jack_kctl->jack_bit_idx == i) {snd_kctl_jack_report(jack->card,jack_kctl->kctl,
status &testbit);
}.... you can reduce to a single loop.
thanks,
Takashi
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct snd_jack +*jack, int type) {
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ?? */;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g. headset. Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct +snd_jack *jack, int type) {
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ?? */;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off speaker/headphone.
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack Speakers".
Takashi
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct +snd_jack *jack, int type) {
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ?? */;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of "Headset Speaker Jack" or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the compatibility. HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct +snd_jack *jack, int type) {
- struct snd_kcontrol *kctl;
- struct snd_jack_kctl *jack_kctl;
- int i, err, index, state = 0 /* use 0 for default state ??
+*/;
- INIT_LIST_HEAD(&jack->kctl_list);
- for (i = 0; i < fls(SND_JACK_BTN_0); i++) {
int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for
headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of "Headset Speaker Jack" or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the compatibility. HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
thanks,
Takashi
At Fri, 20 Mar 2015 13:49:24 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
> +} > + > +static int snd_jack_new_kctl(struct snd_card *card, struct > +snd_jack *jack, int type) { > + struct snd_kcontrol *kctl; > + struct snd_jack_kctl *jack_kctl; > + int i, err, index, state = 0 /* use 0 for default state ?? > +*/; > + > + INIT_LIST_HEAD(&jack->kctl_list); > + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { > + int testbit = 1 << i; > + if (type & testbit) {
With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for
headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of "Headset Speaker Jack" or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the compatibility. HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
Just prepare two exceptions for SND_JACK_HEADSET and SND_JACK_VIDEOUT. These are already defined as single types in sound/jack.h. The code can't be so tricky if you write properly :)
Takashi
On Fri, 2015-03-20 at 15:18 +0100, Takashi Iwai wrote:
At Fri, 20 Mar 2015 13:49:24 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote:
> > +} > > + > > +static int snd_jack_new_kctl(struct snd_card *card, struct > > +snd_jack *jack, int type) { > > + struct snd_kcontrol *kctl; > > + struct snd_jack_kctl *jack_kctl; > > + int i, err, index, state = 0 /* use 0 for default state ?? > > +*/; > > + > > + INIT_LIST_HEAD(&jack->kctl_list); > > + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { > > + int testbit = 1 << i; > > + if (type & testbit) { > > With this implementation, you'll get multiple boolean kctls for > a headset. I thought we agreed with creating a single boolean for
headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of "Headset Speaker Jack" or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the compatibility. HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
Just prepare two exceptions for SND_JACK_HEADSET and SND_JACK_VIDEOUT. These are already defined as single types in sound/jack.h. The code can't be so tricky if you write properly :)
Can someone clarify what is the current plan? Will there be changes to the control naming or behaviour for existing hardware?
PulseAudio's jack handling seems somewhat messy to me, so it may be difficult to understand what kind of changes will break things and what won't. I think these are the jack controls that PulseAudio currently recognizes that are most important in this discussion:
* Headset Mic Jack * Headphone Mic Jack * Mic Jack * Headphone Jack
"Headset Mic Jack" indicates the availability of a headset mic. In PulseAudio it doesn't imply anything about the headset speaker availability, even though logically, if there's a headset mic plugged in, surely the headset speakers are available too.
"Headphone Mic Jack" indicates the availability of either headphones or a microphone. There's no information about which of those is plugged in, just that one of those devices is plugged in. This control is *not* for headsets, according to the commit that added the support for that control to PulseAudio[1].
"Mic Jack" indicates the availability of a standalone microphone. I don't know if the kernel currently exposes both "Headset Mick Jack" and "Mic Jack" if there's one physical jack that is able to distinguish between the two device types, but I suppose it would be good to have both controls in that case, so that applications know what kind of device has been plugged in.
"Headphone Jack" indicates the availability of headphones. PulseAudio doesn't currently have support for any kind of "Headset Speaker Jack" control, so I guess the kernel currently uses "Headphone Jack" also with physical jacks that support headsets.
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in. I know David has made a UI for Ubuntu for selecting the device type once something has been plugged in to such jack, but I don't remember how the UI can know that the jack supports all of those three device types.
[1] http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=7369a53ab5f606e...
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
* "Headphone Jack" * "Headset Mic Jack"
For headphone or headset, one hw switch only:
* "Headphone Jack" * "Headset Mic Phantom Jack"
Headphone or mic, one hw switch:
* "Headphone Mic Jack"
Headphone, headset, or mic, one hw switch only:
* "Headphone Mic Jack" * "Headset Mic Phantom Jack"
Headphone, headset, or mic, one switch for hp/mic and the other for the headset mic:
* "Headphone Mic Jack" * "Headset Mic Jack"
The first one is the most common one, but all of the other ones exist, especially on recent Dell machines.
I know David has made a UI for Ubuntu for selecting the device type once something has been plugged in to such jack, but I don't remember how the UI can know that the jack supports all of those three device types.
For reference, the code is here: http://bazaar.launchpad.net/~unity-settings-daemon-team/unity-settings-daemo...
At Mon, 23 Mar 2015 12:51:07 +0100, David Henningsson wrote:
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
Headphone or mic, one hw switch:
- "Headphone Mic Jack"
Headphone, headset, or mic, one hw switch only:
- "Headphone Mic Jack"
- "Headset Mic Phantom Jack"
Headphone, headset, or mic, one switch for hp/mic and the other for the headset mic:
- "Headphone Mic Jack"
- "Headset Mic Jack"
The first one is the most common one, but all of the other ones exist, especially on recent Dell machines.
Are all these about a single headset/headphone jack? If so, yeah, it's really messy.
Takashi
On Mon, Mar 23, 2015 at 12:51:07PM +0100, David Henningsson wrote:
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
I'd have expected these to be the same thing, just with the second case always having the same state for both switches.
On 2015-03-23 17:41, Mark Brown wrote:
On Mon, Mar 23, 2015 at 12:51:07PM +0100, David Henningsson wrote:
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
I'd have expected these to be the same thing, just with the second case always having the same state for both switches.
We need to distinguish the use cases: in case of independent switches, we can make the assumption that if the user plugged in a headset, the user wants to use the headset mic instead of the internal mic, so we switch to it.
In the other case, where we don't know if the user plugged in a headphone or a headset, we need to ask the user what (s)he plugged in, so we can determine whether to select headphone+internal mic or headphone+headset mic.
On Tue, Mar 24, 2015 at 07:50:39AM +0100, David Henningsson wrote:
On 2015-03-23 17:41, Mark Brown wrote:
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
I'd have expected these to be the same thing, just with the second case always having the same state for both switches.
We need to distinguish the use cases: in case of independent switches, we can make the assumption that if the user plugged in a headset, the user wants to use the headset mic instead of the internal mic, so we switch to it.
In the other case, where we don't know if the user plugged in a headphone or a headset, we need to ask the user what (s)he plugged in, so we can determine whether to select headphone+internal mic or headphone+headset mic.
Hrm, I can see that. However it feels wrong to have no state change at all on the microphone part of the jack - I think I would have expected this case to be flagged as a separate mode, say "Tied", so that simpler applications have more chance to do the right thing. It's a bit of a separate issue though and it's a bit late now as we've already shipped the ABI for the kcontrols.
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
I'd have expected these to be the same thing, just with the second case always having the same state for both switches.
We need to distinguish the use cases: in case of independent switches,
we
can make the assumption that if the user plugged in a headset, the user wants to use the headset mic instead of the internal mic, so we switch
to
it.
In the other case, where we don't know if the user plugged in a
headphone or
a headset, we need to ask the user what (s)he plugged in, so we can determine whether to select headphone+internal mic or headphone+headset
mic.::
Does the user maunal of those notebook explicitly state that the combo jack support headset and headphone ( or mic) ?
if not, why do the driver need to support both headphone and headset ( or mic ) especially when the icon near the jack is headset
One thing that is unclear for me is that how are those jacks
represented
that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
I'd have expected these to be the same thing, just with the second case always having the same state for both switches.
We need to distinguish the use cases: in case of independent switches, we
can make the assumption that if the user plugged in a headset, the user wants to use the headset mic instead of the internal mic, so we switch to it.
What will happen for those Dell Alienware 14, 15 and 17 which have three jacks: headset, headphone and mic
http://www.dell.com/support/home/us/en/19/product-support/product/alienware-...
Alienware 17 Quick Start Guide
Thank you for comprehensive summarizing, David. Can you help give more description as below?
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, March 23, 2015 7:51 PM To: Tanu Kaskinen; Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
[Keyon] here for the most common headset jack (SND_JACK_HEADPHONE | SND_JACK_MICROPHONE), we should create two independent kctls/switches--"Headphone Jack" + "Headset Mic Jack", right?
so, is it possible that there is only "Mic Jack" in this case? I mean that only input(no output, no physical headphone/speaker jack) jack exist.
If yes, then we may need change "Headset Mic Jack" to "Mic Jack"?
For headphone or headset, one hw switch only:
[Keyon] I am sorry I am not familiar with the jack HW circuit. What "one hw switch only" here means? Does it means that -- for headset Jack, the status(connected/disconnected) of HP pin is always exactly same with that of Mic pin?
- "Headphone Jack"
- "Headset Mic Phantom Jack"
[Keyon] for Headset of this type, do we need create only "Headset Mic Phantom Jack" kctl, or "Headphone Jack" kctl is also needed?
Headphone or mic, one hw switch:
[Keyon]I am fresh about this kind of hw jack, it should use different segment of the plug, seems we don't need check the actual connected status at the jack creation stage, just creating "Headphone Mic Jack" should works, right?
- "Headphone Mic Jack"
Headphone, headset, or mic, one hw switch only:
[Keyon] how many kctls should we create for this?
- "Headphone Mic Jack"
- "Headset Mic Phantom Jack"
Headphone, headset, or mic, one switch for hp/mic and the other for the headset mic:
[Keyon] I can't imagine how this works, it's too messy for me. :(
- "Headphone Mic Jack"
- "Headset Mic Jack"
The first one is the most common one, but all of the other ones exist, especially on recent Dell machines.
I know David has made a UI for Ubuntu for selecting the device type once something has been plugged in to such jack, but I don't remember how the UI can know that the jack supports all of those three device types.
For reference, the code is here: http://bazaar.launchpad.net/~unity-settings-daemon-team/unity-settings- daemon/trunk/files/head:/plugins/media-keys/what-did-you-plug-in/
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
Headphone or mic, one hw switch:
[Keyon]I am fresh about this kind of hw jack, it should use different segment of the plug, seems we don't need check the actual connected status at the jack creation stage, just creating "Headphone Mic Jack" should works, right?
- "Headphone Mic Jack"
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/sound/pc...
http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=7369a53ab5f606e...
http://www.asus.com/Notebooks_Ultrabooks/Eee_PC_1015PX/specifications/
On 2015-03-25 14:53, Jie, Yang wrote:
Thank you for comprehensive summarizing, David. Can you help give more description as below?
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, March 23, 2015 7:51 PM To: Tanu Kaskinen; Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
[Keyon] here for the most common headset jack (SND_JACK_HEADPHONE | SND_JACK_MICROPHONE), we should create two independent kctls/switches--"Headphone Jack" + "Headset Mic Jack", right?
so, is it possible that there is only "Mic Jack" in this case? I mean that only input(no output, no physical headphone/speaker jack) jack exist.
If yes, then we may need change "Headset Mic Jack" to "Mic Jack"?
I'm not sure I understand your question - of course there are input only jacks, but they would then be SND_JACK_MICROPHONE only, and also labelled "Mic Jack". But that's not a headset jack, that's a microphone jack.
For headphone or headset, one hw switch only:
[Keyon] I am sorry I am not familiar with the jack HW circuit. What "one hw switch only" here means? Does it means that -- for headset Jack, the status(connected/disconnected) of HP pin is always exactly same with that of Mic pin?
There is only one jack detection input from HW. Regardless of whether you plug in headphone or a headset, the HW switch would detect "plugged in". When the jack is unplugged, the HW switch would detect "unplugged".
The HW cannot tell us whether you plugged in a headphone or headset.
- "Headphone Jack"
- "Headset Mic Phantom Jack"
[Keyon] for Headset of this type, do we need create only "Headset Mic Phantom Jack" kctl, or "Headphone Jack" kctl is also needed?
We need both kctls.
Headphone or mic, one hw switch:
[Keyon]I am fresh about this kind of hw jack, it should use different segment of the plug, seems we don't need check the actual connected status at the jack creation stage, just creating "Headphone Mic Jack" should works, right?
It is not very common. It was used on some Asus netbooks a while ago. But yes, we should just create a "Headphone Mic Jack".
- "Headphone Mic Jack"
Headphone, headset, or mic, one hw switch only:
[Keyon] how many kctls should we create for this?
- "Headphone Mic Jack"
- "Headset Mic Phantom Jack"
Same as today, two: "Headphone Mic Jack" and "Headset Mic Phantom Jack".
Headphone, headset, or mic, one switch for hp/mic and the other for the headset mic:
[Keyon] I can't imagine how this works, it's too messy for me. :(
- "Headphone Mic Jack"
- "Headset Mic Jack"
Nothing plugged in: "Headphone Mic Jack" = false, "Headset Mic Jack" = false Headphones plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = false Headset plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = true Mic plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = false
As you can see, "Headphones" and "Mic" results in the same output, hence userspace needs to ask the user if (s)he plugged in a headphone or mic.
Thank you so much, David!
So, I summarized as below table:
Jack Type kctls/switches name detection description
Headphone Headphone Jack HP plugged in/unplugged
Mic Mic Jack Mic plugged in/unplugged
Headset Headphone Jack Nothing plugged in: ""Headphone Jack"" = false, ""Headset Mic Jack"" = false Headset Mic Jack" Headphones plugged in: ""Headphone Jack"" = true, ""Headset Mic Jack"" = false Headset plugged in: ""Headphone Jack"" = true, ""Headset Mic Jack"" = true Mic plugged in: ""Headphone Jack"" = true ""Headset Mic Jack"" = false(should not plugged in Mic, detect wrongly)" independent switches
Headset Mic Headphone Jack Nothing plugged in: ""Headphone Jack"" = false, ""Headset Mic Phantom Jack"" = false Headset Mic Phantom Jack Headphones plugged in: ""Headphone Jack"" = true, ""Headset Mic Phantom Jack"" = false? Headset plugged in: ""Headphone Jack"" = false, ""Headset Mic Phantom Jack"" = true ? Mic plugged in: ""Headphone Jack"" = true, ""Headset Mic Phantom Jack"" = false(should not plugged in Mic, detect wrongly)" one hw switch only
Headphone Mic Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false Headphones plugged in: ""Headphone Mic Jack"" = true Mic plugged in: ""Headphone Mic Jack"" = true Headset plugged in: ""Headphone Mic Jack"" = true?" one hw switch
Headset Headphone Mic Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Phantom Jack"" = false Headset Mic Phantom Jack Headphones plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Phantom Jack"" = false Headset plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Phantom Jack"" = true ? Mic plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Phantom Jack"" = false" one hw switch only
Headphone Mic Headset Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Jack"" = false Headset Mic Jack Headphones plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = false Headset plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = true Mic plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = false one switch for hp/mic and the other for the headset mic
so, we may need extend snd_jack_types enum, by adding types: Headset Mic, Headphone Mic, Headset Headphone Mic, Headphone Mic Headset.
enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE, SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT | SND_JACK_VIDEOOUT, SND_JACK_LINEIN = 0x0020,
SND_JACK_HEADSET_MIC = 0x0040, /* one hw switch only */ SND_JACK_HEADPHONE_MIC = 0x0080, /* one hw switch only, headphone, or mic */ SND_JACK_HEADSET_ HEADPHONE_MIC = 0x0100, /* one hw switch only, headset, headphone, or mic*/ SND_JACK_ HEADPHONE_HEADSET_ MIC = 0x0200, /* headset, headphone, or mic; one switch for hp/mic and the other for the headset mic */
/* Kept separate from switches to facilitate implementation */ SND_JACK_BTN_0 = 0x8000, SND_JACK_BTN_1 = 0x4000, SND_JACK_BTN_2 = 0x2000, SND_JACK_BTN_3 = 0x1000, SND_JACK_BTN_4 = 0x0800, SND_JACK_BTN_5 = 0x0400, };
And then, we may create corresponding kctls during jack creating.
Any comment?
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Thursday, March 26, 2015 2:42 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-25 14:53, Jie, Yang wrote:
Thank you for comprehensive summarizing, David. Can you help give more description as below?
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Monday, March 23, 2015 7:51 PM To: Tanu Kaskinen; Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-23 11:56, Tanu Kaskinen wrote:
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones,
but
don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
[Keyon] here for the most common headset jack (SND_JACK_HEADPHONE
|
SND_JACK_MICROPHONE), we should create two independent kctls/switches--"Headphone Jack" + "Headset Mic Jack", right?
so, is it possible that there is only "Mic Jack" in this case? I mean that only input(no output, no physical headphone/speaker jack) jack exist.
If yes, then we may need change "Headset Mic Jack" to "Mic Jack"?
I'm not sure I understand your question - of course there are input only jacks, but they would then be SND_JACK_MICROPHONE only, and also labelled "Mic Jack". But that's not a headset jack, that's a microphone jack.
For headphone or headset, one hw switch only:
[Keyon] I am sorry I am not familiar with the jack HW circuit. What "one hw switch only" here means? Does it means that -- for headset Jack, the status(connected/disconnected) of HP pin is always exactly same with that of Mic pin?
There is only one jack detection input from HW. Regardless of whether you plug in headphone or a headset, the HW switch would detect "plugged in". When the jack is unplugged, the HW switch would detect "unplugged".
The HW cannot tell us whether you plugged in a headphone or headset.
- "Headphone Jack"
- "Headset Mic Phantom Jack"
[Keyon] for Headset of this type, do we need create only "Headset Mic Phantom Jack" kctl, or "Headphone Jack" kctl is also needed?
We need both kctls.
Headphone or mic, one hw switch:
[Keyon]I am fresh about this kind of hw jack, it should use different segment of the plug, seems we don't need check the actual connected status at the jack creation stage, just creating "Headphone Mic Jack" should works, right?
It is not very common. It was used on some Asus netbooks a while ago. But yes, we should just create a "Headphone Mic Jack".
- "Headphone Mic Jack"
Headphone, headset, or mic, one hw switch only:
[Keyon] how many kctls should we create for this?
- "Headphone Mic Jack"
- "Headset Mic Phantom Jack"
Same as today, two: "Headphone Mic Jack" and "Headset Mic Phantom Jack".
Headphone, headset, or mic, one switch for hp/mic and the other for the headset mic:
[Keyon] I can't imagine how this works, it's too messy for me. :(
- "Headphone Mic Jack"
- "Headset Mic Jack"
Nothing plugged in: "Headphone Mic Jack" = false, "Headset Mic Jack" = false Headphones plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = false Headset plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = true Mic plugged in: "Headphone Mic Jack" = true, "Headset Mic Jack" = false
As you can see, "Headphones" and "Mic" results in the same output, hence userspace needs to ask the user if (s)he plugged in a headphone or mic.
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 2015-03-26 09:29, Jie, Yang wrote:
Thank you so much, David!
So, I summarized as below table:
I've added how I see them with the new API, see below for further comments:
Jack Type kctls/switches name detection description
Headphone Headphone Jack HP plugged in/unplugged
SND_JACK_HEADPHONE
Mic Mic Jack Mic plugged in/unplugged
SND_JACK_MICROPHONE
Headset Headphone Jack Nothing plugged in: ""Headphone Jack"" = false, ""Headset Mic Jack"" = false Headset Mic Jack" Headphones plugged in: ""Headphone Jack"" = true, ""Headset Mic Jack"" = false Headset plugged in: ""Headphone Jack"" = true, ""Headset Mic Jack"" = true Mic plugged in: ""Headphone Jack"" = true ""Headset Mic Jack"" = false(should not plugged in Mic, detect wrongly)" independent switches
SND_JACK_HEADSET
"Mic plugged in" case is irrelevant - not supported by hw
Headset Mic Headphone Jack Nothing plugged in: ""Headphone Jack"" = false, ""Headset Mic Phantom Jack"" = false Headset Mic Phantom Jack Headphones plugged in: ""Headphone Jack"" = true, ""Headset Mic Phantom Jack"" = false? Headset plugged in: ""Headphone Jack"" = false, ""Headset Mic Phantom Jack"" = true ? Mic plugged in: ""Headphone Jack"" = true, ""Headset Mic Phantom Jack"" = false(should not plugged in Mic, detect wrongly)" one hw switch only
SND_JACK_HEADPHONE, the "Headset Mic Phantom Jack" is created manually
Headphone Mic Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false Headphones plugged in: ""Headphone Mic Jack"" = true Mic plugged in: ""Headphone Mic Jack"" = true Headset plugged in: ""Headphone Mic Jack"" = true?" one hw switch
SND_JACK_HEADPHONE (probably)
Headset Headphone Mic Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Phantom Jack"" = false Headset Mic Phantom Jack Headphones plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Phantom Jack"" = false Headset plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Phantom Jack"" = true ? Mic plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Phantom Jack"" = false" one hw switch only
SND_JACK_HEADPHONE (probably), the "Headset Mic Phantom Jack" is created manually
Headphone Mic Headset Headphone Mic Jack Nothing plugged in: ""Headphone Mic Jack"" = false, ""Headset Mic Jack"" = false Headset Mic Jack Headphones plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = false Headset plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = true Mic plugged in: ""Headphone Mic Jack"" = true, ""Headset Mic Jack"" = false one switch for hp/mic and the other for the headset mic
SND_JACK_HEADSET (probably)
so, we may need extend snd_jack_types enum, by adding types: Headset Mic, Headphone Mic, Headset Headphone Mic, Headphone Mic Headset.
enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE, SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT | SND_JACK_VIDEOOUT, SND_JACK_LINEIN = 0x0020,
SND_JACK_HEADSET_MIC = 0x0040, /* one hw switch only */ SND_JACK_HEADPHONE_MIC = 0x0080, /* one hw switch only, headphone, or mic */ SND_JACK_HEADSET_ HEADPHONE_MIC = 0x0100, /* one hw switch only, headset, headphone, or mic*/ SND_JACK_ HEADPHONE_HEADSET_ MIC = 0x0200, /* headset, headphone, or mic; one switch for hp/mic and the other for the headset mic */
This seems overly complex. I don't think we really need to add all of that.
First, for the phantom jacks. We'll need phantom jacks not only for "Headset Mic Phantom Jack" but also for "Internal Speaker Phantom Jack" and "Internal Mic Phantom Jack", and probably some others as well. So that means that the HDA driver needs to still create some kctls using the current kctl API instead of your new unified API.
A phantom jack is merely a marker to userspace indicating what hardware is present. It always returns "plugged in" when trying to read it.
With the phantom jacks dealt with separately, the question remains about the Headphone Mic stuff. If the name remains the way it is, maybe all we need is a SND_JACK_HEADPHONE (or SND_JACK_HEADSET) jack with the "Headphone Mic Jack" name and then userspace would know that this jack could potentially accomodate a microphone too. Otherwise we might need to add a SND_JACK_HEADPHONE_OR_MIC constant, but I prefer that the names are kept properly, because there might be other relevant info added to the name too (e g "Front Headphone Jack" instead of "Headphone Jack").
OK, then I am thinking we can add jack kctls creating code like below:
snd_jack_new(struct snd_card *card, const char *id, int type, struct snd_jack **jjack) { ... Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: create "Mic Jack" kctl; break; } ... }
Jack Type Jack name kctls/switches name SND_JACK_HEADPHONE Headphone Headphone Jack SND_JACK_MICROPHONE Mic Mic Jack SND_JACK_HEADSET Headset Headphone Jack Headset Mic Jack SND_JACK_HEADPHONE Headset Mic Headphone Jack Headset Mic Phantom Jack SND_JACK_HEADPHONE Headphone Mic Headphone Mic Jack SND_JACK_HEADPHONE Headset Headphone Mic Headphone Mic Jack Headset Mic Phantom Jack SND_JACK_HEADSET Headphone Mic Headset Headphone Mic Jack Headset Mic Jack
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Thursday, March 26, 2015 5:06 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-26 09:29, Jie, Yang wrote:
Thank you so much, David!
So, I summarized as below table:
I've added how I see them with the new API, see below for further comments:
Jack Type kctls/switches name
detection descriptionHeadphone Headphone Jack
HP plugged in/unpluggedSND_JACK_HEADPHONE
Mic Mic Jack
Mic plugged in/unpluggedSND_JACK_MICROPHONE
Headset Headphone Jack
Nothing plugged in: ""Headphone Jack"" =false, ""Headset Mic Jack"" = false
Headset Mic Jack"Headphones plugged in: ""HeadphoneJack"" = true, ""Headset Mic Jack"" = false
Headset plugged in: ""Headphone Jack"" =true, ""Headset Mic Jack"" = true
Mic plugged in: ""Headphone Jack"" = true""Headset Mic Jack"" = false(should not plugged in Mic, detect wrongly)" independent switches
SND_JACK_HEADSET
"Mic plugged in" case is irrelevant - not supported by hw
Headset Mic Headphone Jack
Nothing plugged in: ""Headphone Jack"" =false, ""Headset Mic Phantom Jack"" = false
Headset Mic Phantom JackHeadphones plugged in: ""HeadphoneJack"" = true, ""Headset Mic Phantom Jack"" = false?
Headset plugged in: ""Headphone Jack"" =false, ""Headset Mic Phantom Jack"" = true ?
Mic plugged in: ""Headphone Jack"" = true,""Headset Mic Phantom Jack"" = false(should not plugged in Mic, detect wrongly)" one hw switch only
SND_JACK_HEADPHONE, the "Headset Mic Phantom Jack" is created manually
Headphone Mic Headphone Mic Jack
Nothing plugged in: ""Headphone Mic Jack""= false
Headphones plugged in: ""Headphone MicJack"" = true
Mic plugged in: ""Headphone Mic Jack"" =true
Headset plugged in: ""Headphone MicJack"" = true?" one hw switch
SND_JACK_HEADPHONE (probably)
Headset Headphone Mic Headphone Mic Jack
Nothing plugged in: ""Headphone Mic Jack""= false, ""Headset Mic Phantom Jack"" = false
Headset Mic Phantom JackHeadphones plugged in: ""Headphone MicJack"" = true, ""Headset Mic Phantom Jack"" = false
Headset plugged in: ""Headphone MicJack"" = false, ""Headset Mic Phantom Jack"" = true ?
Mic plugged in: ""Headphone Mic Jack"" =true, ""Headset Mic Phantom Jack"" = false" one hw switch only
SND_JACK_HEADPHONE (probably), the "Headset Mic Phantom Jack" is created manually
Headphone Mic Headset Headphone Mic Jack
Nothing plugged in: ""Headphone Mic Jack""= false, ""Headset Mic Jack"" = false
Headset Mic JackHeadphones plugged in: ""Headphone MicJack"" = true, ""Headset Mic Jack"" = false
Headset plugged in: ""Headphone MicJack"" = true, ""Headset Mic Jack"" = true
Mic plugged in: ""Headphone Mic Jack"" =true, ""Headset Mic Jack"" = false
one switch for hp/micand the other for the
headset mic
SND_JACK_HEADSET (probably)
so, we may need extend snd_jack_types enum, by adding types: Headset
Mic, Headphone Mic, Headset Headphone Mic, Headphone Mic Headset.
enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE |
SND_JACK_MICROPHONE,
SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT |
SND_JACK_VIDEOOUT,
SND_JACK_LINEIN = 0x0020,
SND_JACK_HEADSET_MIC = 0x0040, /* one hw switch only */ SND_JACK_HEADPHONE_MIC = 0x0080, /* one hw switch only,
headphone, or mic */
SND_JACK_HEADSET_ HEADPHONE_MIC = 0x0100, /* one hw
switch only, headset, headphone, or mic*/
SND_JACK_ HEADPHONE_HEADSET_ MIC = 0x0200, /* headset,
headphone, or mic; one switch for hp/mic and the other for the headset mic */
This seems overly complex. I don't think we really need to add all of that.
First, for the phantom jacks. We'll need phantom jacks not only for "Headset Mic Phantom Jack" but also for "Internal Speaker Phantom Jack" and "Internal Mic Phantom Jack", and probably some others as well. So that means that the HDA driver needs to still create some kctls using the current kctl API instead of your new unified API.
A phantom jack is merely a marker to userspace indicating what hardware is present. It always returns "plugged in" when trying to read it.
With the phantom jacks dealt with separately, the question remains about the Headphone Mic stuff. If the name remains the way it is, maybe all we need is a SND_JACK_HEADPHONE (or SND_JACK_HEADSET) jack with the "Headphone Mic Jack" name and then userspace would know that this jack could potentially accomodate a microphone too. Otherwise we might need to add a SND_JACK_HEADPHONE_OR_MIC constant, but I prefer that the names are kept properly, because there might be other relevant info added to the name too (e g "Front Headphone Jack" instead of "Headphone Jack").
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
l
OK, then I am thinking we can add jack kctls creating code like below:
snd_jack_new(struct snd_card *card, const char *id, int type, struct
snd_jack **jjack) {
... Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: create "Mic Jack" kctl; break; } ... }
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/pci/...
If you look at snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, const char *name, int idx) which add kctl and call snd_jack_new if it is not phantom
It is unlikely snd_jack_new() create those kctl
For mobile phone or tablet , there is one and only one input : headset
for notebook and desktop, there are dual headphone jacks, four line out jacks for 7.1 surround, line in,
phantom jack for spdif, surround speakers , hp and mic for those desktop using ac97 front audio panel
From: Raymond Yau [mailto:superquad.vortex2@gmail.com] Sent: Friday, March 27, 2015 8:39 AM To: Jie, Yang Cc: ALSA Development Mailing List; Liam Girdwood; Takashi Iwai; David Henningsson; Girdwood, Liam R; Tanu Kaskinen; broonie@kernel.org Subject: Re: [alsa-devel] [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
l
OK, then I am thinking we can add jack kctls creating code like below:
snd_jack_new(struct snd_card *card, const char *id, int type, struct snd_jack **jjack) { ... Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: create "Mic Jack" kctl; break; } ... }
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/pci/...
If you look at snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, const char *name, int idx) which add kctl and call snd_jack_new if it is not phantom
[Keyon] it’s good that we don’t need handle phantom jack in snd_jack_new().
It is unlikely snd_jack_new() create those kctl
For mobile phone or tablet , there is one and only one input : headset
[Keyon] that’s OK.
for notebook and desktop, there are dual headphone jacks, four line out jacks for 7.1 surround, line in,
phantom jack for spdif, surround speakers , hp and mic for those desktop using ac97 front audio panel
[Keyon] we will create kctls for each headphone/mic/headset jacks, but not for line in/out and phantom
jacks, is this OK?
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
snd_jack_new(struct snd_card *card, const char *id, int type, struct snd_jack **jjack) { ... Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: break; } ... }
Jack Type Jack name kctls/switches name SND_JACK_HEADPHONE Headphone Headphone Jack SND_JACK_MICROPHONE Mic Mic Jack SND_JACK_HEADSET Headset Headphone Jack Headset Mic Jack SND_JACK_HEADPHONE Headset Mic Headphone Jack Headset Mic Phantom Jack SND_JACK_HEADPHONE Headphone Mic Headphone Mic Jack SND_JACK_HEADPHONE Headset Headphone Mic Headphone Mic Jack Headset Mic Phantom Jack SND_JACK_HEADSET Headphone Mic Headset Headphone Mic Jack Headset Mic Jack
~Keyon
On 2015-03-26 13:43, Jie, Yang wrote:
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
It looks like your code can be simplified as:
switch (type | SND_JACK_HEADSET) { case SND_JACK_HEADSET: create "Headset Mic Jack" kctl; /* fall through */ case SND_JACK_MICROPHONE: case SND_JACK_HEADPHONE: create format("%s Jack", id) kctl; }
...that way, prefixes such as "Front Headphone Jack" are preserved too, which is a good thing.
// David
snd_jack_new(struct snd_card *card, const char *id, int type, struct snd_jack **jjack) { ... Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: break; } ... }
Jack Type Jack name kctls/switches name SND_JACK_HEADPHONE Headphone Headphone Jack SND_JACK_MICROPHONE Mic Mic Jack SND_JACK_HEADSET Headset Headphone Jack Headset Mic Jack SND_JACK_HEADPHONE Headset Mic Headphone Jack Headset Mic Phantom Jack SND_JACK_HEADPHONE Headphone Mic Headphone Mic Jack SND_JACK_HEADPHONE Headset Headphone Mic Headphone Mic Jack Headset Mic Phantom Jack SND_JACK_HEADSET Headphone Mic Headset Headphone Mic Jack Headset Mic Jack
~Keyon
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 2:50 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-26 13:43, Jie, Yang wrote:
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
It looks like your code can be simplified as:
[Keyon] thanks, I will try simplify them.
switch (type | SND_JACK_HEADSET) { case SND_JACK_HEADSET: create "Headset Mic Jack" kctl; /* fall through */ case SND_JACK_MICROPHONE: case SND_JACK_HEADPHONE: create format("%s Jack", id) kctl;
[Keyon] I don't want to fill "id" to kctl name, because sometimes it can be string which can't be understood by PA, such as:
soc/intel/mfld_machine.c: ret_val = snd_soc_card_jack_new(runtime->card, soc/intel/mfld_machine.c- "Intel(R) MID Audio Jack", SND_JACK_HEADSET | soc/intel/mfld_machine.c- SND_JACK_BTN_0 | SND_JACK_BTN_1, &mfld_jack,
soc/omap/ams-delta.c: ret = snd_soc_card_jack_new(card, "hook_switch", SND_JACK_HEADSET, soc/omap/ams-delta.c- &ams_delta_hook_switch, NULL, 0);
}
...that way, prefixes such as "Front Headphone Jack" are preserved too,
[Keyon] if PA can recognize "Front Headphone Jack" and "Rear Headphone Jack", then I prefer to add a determine here:
if (!strcmp(id, "Front Headphone Jack") || !strcmp(id, "Rear Headphone Jack")) create format("%s Jack", id) kctl;
which is a good thing.
// David
snd_jack_new(struct snd_card *card, const char *id, int type, struct
snd_jack **jjack) { ...
Switch(type | SND_JACK_HEADSET) { case SND_JACK_MICROPHONE: create "Mic Jack" kctl; break; case SND_JACK_HEADSET: if (id == "Headphone Mic Headset") { create " Headphone Mic Jack " kctl; create " Headset Mic Jack " kctl; } else { create " Headphone Jack " kctl; create " Headset Mic Jack " kctl; } break; case SND_JACK_HEADPHONE: if (id == "Headset Mic") { create " Headphone Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else if (id == "Headphone Mic") { create " Headphone Mic Jack " kctl; } else if (id == "Headset Headphone Mic") { create " Headphone Mic Jack " kctl; // create " Headset Mic Phantom Jack " kctl; } else { create " Headphone Jack " kctl; } break; default: break; } ... }
Jack Type Jack name kctls/switches
name
SND_JACK_HEADPHONE Headphone Headphone
Jack
SND_JACK_MICROPHONE Mic Mic Jack SND_JACK_HEADSET Headset Headphone
Jack
Headset MicJack
SND_JACK_HEADPHONE Headset Mic Headphone
Jack
Headset MicPhantom Jack
SND_JACK_HEADPHONE Headphone Mic Headphone
Mic Jack
SND_JACK_HEADPHONE Headset Headphone Mic Headphone
Mic Jack
Headset MicPhantom Jack
SND_JACK_HEADSET Headphone Mic Headset Headphone
Mic Jack
Headset MicJack
~Keyon
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 2015-03-27 08:45, Jie, Yang wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 2:50 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-26 13:43, Jie, Yang wrote:
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
It looks like your code can be simplified as:
[Keyon] thanks, I will try simplify them.
switch (type | SND_JACK_HEADSET) { case SND_JACK_HEADSET: create "Headset Mic Jack" kctl; /* fall through */ case SND_JACK_MICROPHONE: case SND_JACK_HEADPHONE: create format("%s Jack", id) kctl;
[Keyon] I don't want to fill "id" to kctl name, because sometimes it can be string which can't be understood by PA, such as:
soc/intel/mfld_machine.c: ret_val = snd_soc_card_jack_new(runtime->card, soc/intel/mfld_machine.c- "Intel(R) MID Audio Jack", SND_JACK_HEADSET | soc/intel/mfld_machine.c- SND_JACK_BTN_0 | SND_JACK_BTN_1, &mfld_jack,
soc/omap/ams-delta.c: ret = snd_soc_card_jack_new(card, "hook_switch", SND_JACK_HEADSET, soc/omap/ams-delta.c- &ams_delta_hook_switch, NULL, 0);
Ok, good point. It's a matter of different conventions between ASoC and HDA. In ASoC, naming is wild west. In HDA, naming is (mostly) consistent and we depend on the exact naming for userspace to know exactly how the jack functions.
Another example is HDMI/DisplayPort, where the hw often supports several ports and we need to know which jack belongs to which PCM device, and we use the name of the jack to determine that.
I would very much prefer that all the ASoC jack names (and mixer control names!) would be fixed up to be consistent to the extent that makes sense, but the ASoC people haven't really agreed to do that, but instead prefer to use a large database of UCM files. :-( And seen from a UCM context, that the ASoC jacks are labelled terribly inconsistent, isn't that big of a deal given that UCM will specify the jack name anyhow.
}
...that way, prefixes such as "Front Headphone Jack" are preserved too,
[Keyon] if PA can recognize "Front Headphone Jack" and "Rear Headphone Jack", then I prefer to add a determine here:
if (!strcmp(id, "Front Headphone Jack") || !strcmp(id, "Rear Headphone Jack")) create format("%s Jack", id) kctl;
This will not scale, there are too many variants.
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 4:01 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-27 08:45, Jie, Yang wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 2:50 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-26 13:43, Jie, Yang wrote:
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
It looks like your code can be simplified as:
[Keyon] thanks, I will try simplify them.
switch (type | SND_JACK_HEADSET) { case SND_JACK_HEADSET: create "Headset Mic Jack" kctl; /* fall through */ case SND_JACK_MICROPHONE: case SND_JACK_HEADPHONE: create format("%s Jack", id) kctl;
[Keyon] I don't want to fill "id" to kctl name, because sometimes it can be string which can't be understood by PA, such as:
soc/intel/mfld_machine.c: ret_val = snd_soc_card_jack_new(runtime- card, soc/intel/mfld_machine.c- "Intel(R) MID Audio Jack",
SND_JACK_HEADSET |
soc/intel/mfld_machine.c- SND_JACK_BTN_0 |
SND_JACK_BTN_1, &mfld_jack,
soc/omap/ams-delta.c: ret = snd_soc_card_jack_new(card,
"hook_switch", SND_JACK_HEADSET,
soc/omap/ams-delta.c- &ams_delta_hook_switch, NULL, 0);
Ok, good point. It's a matter of different conventions between ASoC and HDA. In ASoC, naming is wild west. In HDA, naming is (mostly) consistent and we depend on the exact naming for userspace to know exactly how the jack functions.
Another example is HDMI/DisplayPort, where the hw often supports several ports and we need to know which jack belongs to which PCM device, and we use the name of the jack to determine that.
I would very much prefer that all the ASoC jack names (and mixer control names!) would be fixed up to be consistent to the extent that makes sense, but the ASoC people haven't really agreed to do that, but instead prefer to use a large database of UCM files. :-( And seen from a UCM context, that the ASoC jacks are labelled terribly inconsistent, isn't that big of a deal given that UCM will specify the jack name anyhow.
[Keyon] that's the bad status. :( Maybe we can just make HDA jack kctls works and leave not standard name soc jack kctls invisible for PA?
}
...that way, prefixes such as "Front Headphone Jack" are preserved too,
[Keyon] if PA can recognize "Front Headphone Jack" and "Rear Headphone
Jack", then I
prefer to add a determine here:
if (!strcmp(id, "Front Headphone Jack") || !strcmp(id, "Rear
Headphone Jack"))
create format("%s Jack", id) kctl;This will not scale, there are too many variants.
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
On 2015-03-27 10:11, Jie, Yang wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 4:01 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-27 08:45, Jie, Yang wrote:
-----Original Message----- From: David Henningsson [mailto:david.henningsson@canonical.com] Sent: Friday, March 27, 2015 2:50 PM To: Jie, Yang; Tanu Kaskinen; Takashi Iwai Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On 2015-03-26 13:43, Jie, Yang wrote:
Sorry, Correct a typo(removing line "create "Mic Jack" kctl;" in default case)
It looks like your code can be simplified as:
[Keyon] thanks, I will try simplify them.
switch (type | SND_JACK_HEADSET) { case SND_JACK_HEADSET: create "Headset Mic Jack" kctl; /* fall through */ case SND_JACK_MICROPHONE: case SND_JACK_HEADPHONE: create format("%s Jack", id) kctl;
[Keyon] I don't want to fill "id" to kctl name, because sometimes it can be string which can't be understood by PA, such as:
soc/intel/mfld_machine.c: ret_val = snd_soc_card_jack_new(runtime- card, soc/intel/mfld_machine.c- "Intel(R) MID Audio Jack",
SND_JACK_HEADSET |
soc/intel/mfld_machine.c- SND_JACK_BTN_0 |
SND_JACK_BTN_1, &mfld_jack,
soc/omap/ams-delta.c: ret = snd_soc_card_jack_new(card,
"hook_switch", SND_JACK_HEADSET,
soc/omap/ams-delta.c- &ams_delta_hook_switch, NULL, 0);
Ok, good point. It's a matter of different conventions between ASoC and HDA. In ASoC, naming is wild west. In HDA, naming is (mostly) consistent and we depend on the exact naming for userspace to know exactly how the jack functions.
Another example is HDMI/DisplayPort, where the hw often supports several ports and we need to know which jack belongs to which PCM device, and we use the name of the jack to determine that.
I would very much prefer that all the ASoC jack names (and mixer control names!) would be fixed up to be consistent to the extent that makes sense, but the ASoC people haven't really agreed to do that, but instead prefer to use a large database of UCM files. :-( And seen from a UCM context, that the ASoC jacks are labelled terribly inconsistent, isn't that big of a deal given that UCM will specify the jack name anyhow.
[Keyon] that's the bad status. :( Maybe we can just make HDA jack kctls works and leave not standard name soc jack kctls invisible for PA?
If with "invisible for PA" you mean that there are still kctls created, but their naming of the jacks are non-standard, yes, I think this is an okay outcome of your patch. I still think ASoC should change their jack naming to be more consistent, but that can be addressed in some future patch set.
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in.
For headphone or headset, independent switches:
- "Headphone Jack"
- "Headset Mic Jack"
For headphone or headset, one hw switch only:
- "Headphone Jack"
- "Headset Mic Phantom Jack"
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/log/sound/pci/h...
I don't understand how can Dell OptiPlex 3030 All-in-One and XPS 15 laoptop can use the same pin config "dell-headset-multi" ?
There is line out jack in aio spec : Universal Headset (Side)1 Line-out(Rear)
While xps 15 notebook has internal speaker and headset
-----Original Message----- From: Tanu Kaskinen [mailto:tanu.kaskinen@linux.intel.com] Sent: Monday, March 23, 2015 6:57 PM To: Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood; David Henningsson Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On Fri, 2015-03-20 at 15:18 +0100, Takashi Iwai wrote:
At Fri, 20 Mar 2015 13:49:24 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 8:27 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote: > > > > +} > > > + > > > +static int snd_jack_new_kctl(struct snd_card *card, > > > +struct snd_jack *jack, int type) { > > > + struct snd_kcontrol *kctl; > > > + struct snd_jack_kctl *jack_kctl; > > > + int i, err, index, state = 0 /* use 0 for default state ?? > > > +*/; > > > + > > > + INIT_LIST_HEAD(&jack->kctl_list); > > > + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { > > > + int testbit = 1 << i; > > > + if (type & testbit) { > > > > With this implementation, you'll get multiple boolean > > kctls for a headset. I thought we agreed with creating a > > single boolean for
headset?
> [Keyon] We agreed with creating multiple kctls for combo jack, e.g. headset. > Furthermore, e.g., imagine that type = SND_JACK_HEADSET | > SND_JACK_BTN_0, we will create 3 kctls for the jack, when > BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls for a single headset) and its behavior is incompatible with the current code (both the name change and the behavior change). That is, your patch will very likely break the existing applications.
[Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of
"Headset Speaker Jack"
or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off
speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the
compatibility.
HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
Just prepare two exceptions for SND_JACK_HEADSET and
SND_JACK_VIDEOUT.
These are already defined as single types in sound/jack.h. The code can't be so tricky if you write properly :)
Can someone clarify what is the current plan? Will there be changes to the control naming or behaviour for existing hardware?
[Keyon] Hi Tanu, currently we plan to create kctls/switches at jack creating stage, and attached them to the jack. So, we need to decide the naming of these kctls/switches and make sure they will be understood by PA correctly. At the same time, we create jacks with snd_jack_types as parameter passed in, so, we need decision about what kctls/switches do we need create, for each separate or combo(bits) type jack: enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE, SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT | SND_JACK_VIDEOOUT, SND_JACK_LINEIN = 0x0020,
/* Kept separate from switches to facilitate implementation */ SND_JACK_BTN_0 = 0x4000, SND_JACK_BTN_1 = 0x2000, SND_JACK_BTN_2 = 0x1000, SND_JACK_BTN_3 = 0x0800, SND_JACK_BTN_4 = 0x0400, SND_JACK_BTN_5 = 0x0200, };
So, 1. we need these naming, what you summarized below, that's quite helpful. 2. I am a little concern if the exist snd_jack_types enum can indicate all diverse existing hardware, e.g. " Headphone Mic Jack " as you mentioned below, can we use any bits combination for this kind of jack? SND_JACK_HEADPHONE | SND_JACK_MICROPHONE seems can't tell difference With "Headset Mic Jack" + "Headphone Jack"?
PulseAudio's jack handling seems somewhat messy to me, so it may be difficult to understand what kind of changes will break things and what won't. I think these are the jack controls that PulseAudio currently recognizes that are most important in this discussion:
- Headset Mic Jack
- Headphone Mic Jack
- Mic Jack
- Headphone Jack
"Headset Mic Jack" indicates the availability of a headset mic. In PulseAudio it doesn't imply anything about the headset speaker availability, even though logically, if there's a headset mic plugged in, surely the headset speakers are available too.
"Headphone Mic Jack" indicates the availability of either headphones or a microphone. There's no information about which of those is plugged in, just that one of those devices is plugged in. This control is *not* for headsets, according to the commit that added the support for that control to PulseAudio[1].
"Mic Jack" indicates the availability of a standalone microphone. I don't know if the kernel currently exposes both "Headset Mick Jack" and "Mic Jack" if there's one physical jack that is able to distinguish between the two device types, but I suppose it would be good to have both controls in that case, so that applications know what kind of device has been plugged in.
"Headphone Jack" indicates the availability of headphones. PulseAudio doesn't currently have support for any kind of "Headset Speaker Jack" control, so I guess the kernel currently uses "Headphone Jack" also with physical jacks that support headsets.
One thing that is unclear for me is that how are those jacks represented that support any of headsets/headphones/microphones, but don't provide information about which device type has been plugged in. I know David has made a UI for Ubuntu for selecting the device type once something has been plugged in to such jack, but I don't remember how the UI can know that the jack supports all of those three device types.
[1] http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=7369a53ab5 f606e87a3cd1cd4eebd40226bab090
-- Tanu
On Wed, 2015-03-25 at 07:53 +0000, Jie, Yang wrote:
-----Original Message----- From: Tanu Kaskinen [mailto:tanu.kaskinen@linux.intel.com] Sent: Monday, March 23, 2015 6:57 PM To: Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood; David Henningsson Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On Fri, 2015-03-20 at 15:18 +0100, Takashi Iwai wrote:
At Fri, 20 Mar 2015 13:49:24 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote:
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Friday, March 20, 2015 8:27 PM > To: Jie, Yang > Cc: perex@perex.cz; broonie@kernel.org; > alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood > Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols > for every jack input device > > At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote: > > > > > > +} > > > > + > > > > +static int snd_jack_new_kctl(struct snd_card *card, > > > > +struct snd_jack *jack, int type) { > > > > + struct snd_kcontrol *kctl; > > > > + struct snd_jack_kctl *jack_kctl; > > > > + int i, err, index, state = 0 /* use 0 for default state ?? > > > > +*/; > > > > + > > > > + INIT_LIST_HEAD(&jack->kctl_list); > > > > + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { > > > > + int testbit = 1 << i; > > > > + if (type & testbit) { > > > > > > With this implementation, you'll get multiple boolean > > > kctls for a headset. I thought we agreed with creating a > > > single boolean for
headset?
> > [Keyon] We agreed with creating multiple kctls for combo jack, e.g. > headset. > > Furthermore, e.g., imagine that type = SND_JACK_HEADSET | > > SND_JACK_BTN_0, we will create 3 kctls for the jack, when > > BTN_0 is pressed, we will report to the 3rd kctl. > > Hm, I don't remember that I agreed with multiple kctls... > > The multiple kctls have a significant drawback (multiple event > calls for a single > headset) and its behavior is incompatible with the current > code (both the name change and the behavior change). That is, > your patch will very likely break the existing applications. [Keyon] I am not very clear with the existing applications that using these kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch controls, via the name, then we can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of
"Headset Speaker Jack"
or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in general.
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do anything, e.g. decide to switch on/off
speaker/headphone.
Won't this break any user-space stuff?
BTW, I haven't implemented the generating of combo jack kctls' name yet, currently, they looked like below: numid=12,iface=CARD,name='Headset Jack' numid=13,iface=CARD,name='Headset Jack',index=1 numid=14,iface=CARD,name='Headset Jack',index=2
once we have come to agreement, we can modify it in snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset Jack
Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the
compatibility.
HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
Just prepare two exceptions for SND_JACK_HEADSET and
SND_JACK_VIDEOUT.
These are already defined as single types in sound/jack.h. The code can't be so tricky if you write properly :)
Can someone clarify what is the current plan? Will there be changes to the control naming or behaviour for existing hardware?
[Keyon] Hi Tanu, currently we plan to create kctls/switches at jack creating stage, and attached them to the jack. So, we need to decide the naming of these kctls/switches and make sure they will be understood by PA correctly.
So the plan is to avoid any changes to what kctls applications see? That's very good.
At the same time, we create jacks with snd_jack_types as parameter passed in, so, we need decision about what kctls/switches do we need create, for each separate or combo(bits) type jack: enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE, SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT | SND_JACK_VIDEOOUT, SND_JACK_LINEIN = 0x0020,
/* Kept separate from switches to facilitate implementation */ SND_JACK_BTN_0 = 0x4000, SND_JACK_BTN_1 = 0x2000, SND_JACK_BTN_2 = 0x1000, SND_JACK_BTN_3 = 0x0800, SND_JACK_BTN_4 = 0x0400, SND_JACK_BTN_5 = 0x0200,};
So,
- we need these naming, what you summarized below, that's quite helpful.
David's summary is even more helpful!
- I am a little concern if the exist snd_jack_types enum can indicate all diverse
existing hardware, e.g. " Headphone Mic Jack " as you mentioned below, can we use any bits combination for this kind of jack? SND_JACK_HEADPHONE | SND_JACK_MICROPHONE seems can't tell difference With "Headset Mic Jack" + "Headphone Jack"?
Yes, it seems the existing enumeration isn't able to do that distinction. Can the enumeration be extended?
Another thing that might be a problem is that this jack enumeration doesn't make distinction between what device types can be used with the jack, and what level of device type detection is available. That is, is a headset jack only able to tell that "something was plugged in", or is it also able to tell whether that something was headphones, a microphone or a headset. (I suppose this might essentially be the same topic as discussed in the "ALSA: jack: Refactoring for jack kctls" thread about phantom jacks.)
-----Original Message----- From: Tanu Kaskinen [mailto:tanu.kaskinen@linux.intel.com] Sent: Wednesday, March 25, 2015 5:28 PM To: Jie, Yang Cc: Takashi Iwai; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood; David Henningsson Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On Wed, 2015-03-25 at 07:53 +0000, Jie, Yang wrote:
-----Original Message----- From: Tanu Kaskinen [mailto:tanu.kaskinen@linux.intel.com] Sent: Monday, March 23, 2015 6:57 PM To: Takashi Iwai Cc: Jie, Yang; perex@perex.cz; broonie@kernel.org; alsa-devel@alsa- project.org; Girdwood, Liam R; Liam Girdwood; David Henningsson Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
On Fri, 2015-03-20 at 15:18 +0100, Takashi Iwai wrote:
At Fri, 20 Mar 2015 13:49:24 +0000, Jie, Yang wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, March 20, 2015 9:21 PM To: Jie, Yang Cc: perex@perex.cz; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R; Liam Girdwood; Tanu Kaskinen (tanu.kaskinen@linux.intel.com) Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device
At Fri, 20 Mar 2015 12:50:33 +0000, Jie, Yang wrote: > > > -----Original Message----- > > From: Takashi Iwai [mailto:tiwai@suse.de] > > Sent: Friday, March 20, 2015 8:27 PM > > To: Jie, Yang > > Cc: perex@perex.cz; broonie@kernel.org; > > alsa-devel@alsa-project.org; Girdwood, Liam R; Liam > > Girdwood > > Subject: Re: [PATCH v2 1/2] ALSA: jack: create jack > > kcontrols for every jack input device > > > > At Fri, 20 Mar 2015 12:22:10 +0000, Jie, Yang wrote: > > > > > > > > +} > > > > > + > > > > > +static int snd_jack_new_kctl(struct snd_card *card, > > > > > +struct snd_jack *jack, int type) { > > > > > + struct snd_kcontrol *kctl; > > > > > + struct snd_jack_kctl *jack_kctl; > > > > > + int i, err, index, state = 0 /* use 0 for default state ?? > > > > > +*/; > > > > > + > > > > > + INIT_LIST_HEAD(&jack->kctl_list); > > > > > + for (i = 0; i < fls(SND_JACK_BTN_0); i++) { > > > > > + int testbit = 1 << i; > > > > > + if (type & testbit) { > > > > > > > > With this implementation, you'll get multiple boolean > > > > kctls for a headset. I thought we agreed with > > > > creating a single boolean for headset? > > > [Keyon] We agreed with creating multiple kctls for combo jack,
e.g.
> > headset. > > > Furthermore, e.g., imagine that type = SND_JACK_HEADSET > > > | SND_JACK_BTN_0, we will create 3 kctls for the jack, > > > when > > > BTN_0 is pressed, we will report to the 3rd kctl. > > > > Hm, I don't remember that I agreed with multiple kctls... > > > > The multiple kctls have a significant drawback (multiple > > event calls for a single > > headset) and its behavior is incompatible with the current > > code (both the name change and the behavior change). That > > is, your patch will very likely break the existing applications. > [Keyon] I am not very clear with the existing applications > that using these kctl events(seems Android use input subsystem
event?
> Which we didn't Change here. If I understand correctly, > Pulseaudio uses jack switch controls, via the name, then we > can use different name for headphone and mic here.)
PA uses jack kctls.
If you rename, how would you guarantee that the existing application will work as expected? PA doesn't have the definition of
"Headset Speaker Jack"
or such.
And, no, we have no option "fix PA". Other way round: we are not allowed to break the current PA (or any user-space) behavior in
general.
> we will generate 2 event calls(one headphone, one mic) when > Headset plug in/out, applications will receive these 2 > events, and they can do anything, e.g. decide to switch > on/off
speaker/headphone.
Won't this break any user-space stuff?
> BTW, I haven't implemented the generating of combo jack kctls' > name yet, currently, they looked like below: > numid=12,iface=CARD,name='Headset Jack' > numid=13,iface=CARD,name='Headset Jack',index=1 > numid=14,iface=CARD,name='Headset Jack',index=2 > > once we have come to agreement, we can modify it in > snd_jack_new_kctl(), e.g., "Headset Jack Mic" and "Headset > Jack Speakers".
.... and how the existing user-space works without changing its code?
Keyon, the most important point at this moment is to keep the
compatibility.
HD-audio is no new driver. It's a driver that has been present over a decade with (literally) thousands of variants. Please keep this in mind, and reconsider whether your patch will retain the compatibility, especially with PulseAudio.
[Keyon] understood. Then we should follow the HD-audio style, So, what do you suggest here? Should we create only one single Boolean kctls for headset, and report true when status in headphone bit it true? Then we need a tricky exception mapping here?
Sorry, I am a little confusing here, because Mark suggest to create multiple controls for multiple bits jack, and you also agreed with that. :(
Just prepare two exceptions for SND_JACK_HEADSET and
SND_JACK_VIDEOUT.
These are already defined as single types in sound/jack.h. The code can't be so tricky if you write properly :)
Can someone clarify what is the current plan? Will there be changes to the control naming or behaviour for existing hardware?
[Keyon] Hi Tanu, currently we plan to create kctls/switches at jack creating stage, and attached them to the jack. So, we need to decide the naming of these kctls/switches and make sure they will be understood by PA correctly.
So the plan is to avoid any changes to what kctls applications see? That's very good.
At the same time, we create jacks with snd_jack_types as parameter passed in, so, we need decision about what kctls/switches do we need create, for each separate or combo(bits) type jack: enum snd_jack_types { SND_JACK_HEADPHONE = 0x0001, SND_JACK_MICROPHONE = 0x0002, SND_JACK_HEADSET = SND_JACK_HEADPHONE |
SND_JACK_MICROPHONE,
SND_JACK_LINEOUT = 0x0004, SND_JACK_MECHANICAL = 0x0008, /* If detected separately */ SND_JACK_VIDEOOUT = 0x0010, SND_JACK_AVOUT = SND_JACK_LINEOUT | SND_JACK_VIDEOOUT, SND_JACK_LINEIN = 0x0020, /* Kept separate from switches to facilitate implementation */ SND_JACK_BTN_0 = 0x4000, SND_JACK_BTN_1 = 0x2000, SND_JACK_BTN_2 = 0x1000, SND_JACK_BTN_3 = 0x0800, SND_JACK_BTN_4 = 0x0400, SND_JACK_BTN_5 = 0x0200,};
So,
- we need these naming, what you summarized below, that's quite helpful.
David's summary is even more helpful!
- I am a little concern if the exist snd_jack_types enum can indicate
all diverse existing hardware, e.g. " Headphone Mic Jack " as you mentioned below, can we use any bits combination for this kind of jack? SND_JACK_HEADPHONE | SND_JACK_MICROPHONE seems can't tell
difference
With "Headset Mic Jack" + "Headphone Jack"?
Yes, it seems the existing enumeration isn't able to do that distinction. Can the enumeration be extended?
[Keyon] I think we can extend it if needed.
Another thing that might be a problem is that this jack enumeration doesn't make distinction between what device types can be used with the jack, and what level of device type detection is available. That is, is a headset jack only
[Keyon] our goal for this patch set should be achieve this, IMO.
able to tell that "something was plugged in", or is it also able to tell whether that something was headphones, a microphone or a headset. (I suppose this might essentially be the same topic as discussed in the "ALSA: jack: Refactoring for jack kctls" thread about phantom jacks.)
[Keyon] yes, we creating kctls and adding them to the kctl list which belong to the jack, hopefully, it may help giving out these information.
-- Tanu
+}
+static int snd_jack_new_kctl(struct snd_card *card, struct +snd_jack *jack, int type) {
struct snd_kcontrol *kctl;struct snd_jack_kctl *jack_kctl;int i, err, index, state = 0 /* use 0 for default state
?? */;
INIT_LIST_HEAD(&jack->kctl_list);for (i = 0; i < fls(SND_JACK_BTN_0); i++) {int testbit = 1 << i;if (type & testbit) {With this implementation, you'll get multiple boolean kctls for a headset. I thought we agreed with creating a single boolean for
headset?
[Keyon] We agreed with creating multiple kctls for combo jack, e.g.
headset.
Furthermore, e.g., imagine that type = SND_JACK_HEADSET | SND_JACK_BTN_0, we will create 3 kctls for the jack, when BTN_0 is pressed, we will report to the 3rd kctl.
Hm, I don't remember that I agreed with multiple kctls...
The multiple kctls have a significant drawback (multiple event calls
for a single
headset) and its behavior is incompatible with the current code (both
the
name change and the behavior change). That is, your patch will very
likely
break the existing applications.
[Keyon] I am not very clear with the existing applications that using
these
kctl events(seems Android use input subsystem event? Which we didn't Change here. If I understand correctly, Pulseaudio uses jack switch
controls,
via the name, then we can use different name for headphone and mic here.)
we will generate 2 event calls(one headphone, one mic) when Headset plug in/out, applications will receive these 2 events, and they can do
anything,
e.g. decide to switch on/off speaker/headphone.
Why do you need to generate two event (hp and mic) if soc jack only support headset ?
For mobile phone and tablet which only support headset using TRRRS connector and most of them don't support headphone using TRRS connector.
When the headset jack kctl is on, this implies headphone and headset mic are used
Currently the ALSA jack core registers only input devices for each jack registered. These jack input devices are not readable by userspace devices that run as non root.
This patch adds support for additionally registering jack kcontrol devices for every input jack registered. This allows non root userspace to read jack status.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com Modified-by: Jie Yang yang.jie@intel.com Signed-off-by: Jie Yang yang.jie@intel.com Reveiwed-by: Mark Brown broonie@kernel.org
include/sound/jack.h | 8 +++++ sound/core/jack.c | 99
++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/include/sound/jack.h b/include/sound/jack.h index 2182350..ef0f0ed 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -73,6 +73,8 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
struct list_head kctl_list;struct snd_card *card; int registered; int type; const char *id;@@ -82,6 +84,12 @@ struct snd_jack { void (*private_free)(struct snd_jack *); };
+struct snd_jack_kctl {
struct snd_kcontrol *kctl;struct list_head jack_list; /* list of controls
belong to the same jack*/
Does it mean that those multi io jacks (line in and mic jacks of desktop) and those headphone/mic combo jack of netbook have all the input and output kctls in this list ?
We move kctls creating to core jack part, which means that the kctls belonging to the jack will be created at jack new stage. Then we can remove hda jack kctls here.
Signed-off-by: Jie Yang yang.jie@intel.com --- sound/core/Kconfig | 3 --- sound/core/Makefile | 3 +-- sound/pci/hda/Kconfig | 10 +--------- sound/pci/hda/hda_jack.c | 45 +++------------------------------------------ sound/pci/hda/hda_jack.h | 3 --- 5 files changed, 5 insertions(+), 59 deletions(-)
diff --git a/sound/core/Kconfig b/sound/core/Kconfig index 313f22e..63cc2e9 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -221,9 +221,6 @@ config SND_PCM_XRUN_DEBUG config SND_VMASTER bool
-config SND_KCTL_JACK - bool - config SND_DMA_SGBUF def_bool y depends on X86 diff --git a/sound/core/Makefile b/sound/core/Makefile index 4daf2f5..e041dc2 100644 --- a/sound/core/Makefile +++ b/sound/core/Makefile @@ -7,8 +7,7 @@ snd-y := sound.o init.o memory.o info.o control.o misc.o device.o snd-$(CONFIG_ISA_DMA_API) += isadma.o snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o snd-$(CONFIG_SND_VMASTER) += vmaster.o -snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o -snd-$(CONFIG_SND_JACK) += jack.o +snd-$(CONFIG_SND_JACK) += ctljack.o jack.o
snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \ pcm_memory.o memalloc.o diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 7f0f2c5..7fe4a30 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -4,7 +4,7 @@ config SND_HDA tristate select SND_PCM select SND_VMASTER - select SND_KCTL_JACK + select SND_JACK
config SND_HDA_INTEL tristate "HD Audio PCI" @@ -86,14 +86,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK - bool "Support jack plugging notification via input layer" - depends on INPUT=y || INPUT=SND - select SND_JACK - help - Say Y here to enable the jack plugging notification via - input layer. - config SND_HDA_PATCH_LOADER bool "Support initialization patch loading for HD-audio" select FW_LOADER diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index e664307..5cbd1a7 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -132,11 +132,9 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
for (i = 0; i < codec->jacktbl.used; i++, jack++) { struct hda_jack_callback *cb, *next; -#ifdef CONFIG_SND_HDA_INPUT_JACK /* free jack instances manually when clearing/reconfiguring */ if (!codec->bus->shutdown && jack->jack) snd_device_free(codec->bus->card, jack->jack); -#endif for (cb = jack->callback; cb; cb = next) { next = cb->next; kfree(cb); @@ -337,20 +335,16 @@ void snd_hda_jack_report_sync(struct hda_codec *codec) jack = codec->jacktbl.list; for (i = 0; i < codec->jacktbl.used; i++, jack++) if (jack->nid) { - if (!jack->kctl || jack->block_report) + if (!jack->jack || jack->block_report) continue; state = get_jack_plug_state(jack->pin_sense); - snd_kctl_jack_report(codec->bus->card, jack->kctl, state); -#ifdef CONFIG_SND_HDA_INPUT_JACK if (jack->jack) snd_jack_report(jack->jack, state ? jack->type : 0); -#endif } } EXPORT_SYMBOL_GPL(snd_hda_jack_report_sync);
-#ifdef CONFIG_SND_HDA_INPUT_JACK /* guess the jack type from the pin-config */ static int get_input_jack_type(struct hda_codec *codec, hda_nid_t nid) { @@ -377,7 +371,6 @@ static void hda_free_jack_priv(struct snd_jack *jack) jacks->nid = 0; jacks->jack = NULL; } -#endif
/** * snd_hda_jack_add_kctl - Add a kctl for the given pin @@ -394,26 +387,15 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, const char *name, int idx, bool phantom_jack) { struct hda_jack_tbl *jack; - struct snd_kcontrol *kctl; int err, state;
jack = snd_hda_jack_tbl_new(codec, nid); if (!jack) return 0; - if (jack->kctl) + if (jack->jack) return 0; /* already created */ - kctl = snd_kctl_jack_new(name, idx, codec); - if (!kctl) - return -ENOMEM; - err = snd_hda_ctl_add(codec, nid, kctl); - if (err < 0) - return err; - jack->kctl = kctl; jack->phantom_jack = !!phantom_jack;
- state = snd_hda_jack_detect(codec, nid); - snd_kctl_jack_report(codec->bus->card, kctl, state); -#ifdef CONFIG_SND_HDA_INPUT_JACK if (!phantom_jack) { jack->type = get_input_jack_type(codec, nid); err = snd_jack_new(codec->bus->card, name, jack->type, @@ -422,9 +404,9 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, return err; jack->jack->private_data = jack; jack->jack->private_free = hda_free_jack_priv; + state = snd_hda_jack_detect(codec, nid); snd_jack_report(jack->jack, state ? jack->type : 0); } -#endif return 0; }
@@ -444,26 +426,6 @@ int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid, } EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctl);
-/* get the unique index number for the given kctl name */ -static int get_unique_index(struct hda_codec *codec, const char *name, int idx) -{ - struct hda_jack_tbl *jack; - int i, len = strlen(name); - again: - jack = codec->jacktbl.list; - for (i = 0; i < codec->jacktbl.used; i++, jack++) { - /* jack->kctl.id contains "XXX Jack" name string with index */ - if (jack->kctl && - !strncmp(name, jack->kctl->id.name, len) && - !strcmp(" Jack", jack->kctl->id.name + len) && - jack->kctl->id.index == idx) { - idx++; - goto again; - } - } - return idx; -} - static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid, const struct auto_pin_cfg *cfg, const char *base_name) @@ -490,7 +452,6 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid, if (phantom_jack) /* Example final name: "Internal Mic Phantom Jack" */ strncat(name, " Phantom", sizeof(name) - strlen(name) - 1); - idx = get_unique_index(codec, name, idx); err = __snd_hda_jack_add_kctl(codec, nid, name, idx, phantom_jack); if (err < 0) return err; diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h index b279e32..6530faf 100644 --- a/sound/pci/hda/hda_jack.h +++ b/sound/pci/hda/hda_jack.h @@ -39,11 +39,8 @@ struct hda_jack_tbl { unsigned int block_report:1; /* in a transitional state - do not report to userspace */ hda_nid_t gating_jack; /* valid when gating jack plugged */ hda_nid_t gated_jack; /* gated is dependent on this jack */ - struct snd_kcontrol *kctl; /* assigned kctl for jack-detection */ -#ifdef CONFIG_SND_HDA_INPUT_JACK int type; struct snd_jack *jack; -#endif };
struct hda_jack_tbl *
participants (7)
-
David Henningsson -
Jie Yang -
Jie, Yang
-
Mark Brown -
Raymond Yau -
Takashi Iwai -
Tanu Kaskinen