* [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls @ 2015-03-20 8:55 Jie Yang 2015-03-20 8:55 ` Jie Yang ` (2 more replies) 0 siblings, 3 replies; 40+ messages in thread From: Jie Yang @ 2015-03-20 8:55 UTC (permalink / raw) To: tiwai, perex, broonie; +Cc: alsa-devel, liam.r.girdwood 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(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls 2015-03-20 8:55 [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls Jie Yang @ 2015-03-20 8:55 ` Jie Yang 2015-03-20 8:55 ` [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device Jie Yang 2015-03-20 8:55 ` [PATCH v2 2/2] ALSA: hda - Remove jack kctls Jie Yang 2 siblings, 0 replies; 40+ messages in thread From: Jie Yang @ 2015-03-20 8:55 UTC (permalink / raw) To: tiwai, perex, broonie; +Cc: alsa-devel, liam.r.girdwood 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(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 8:55 [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls Jie Yang 2015-03-20 8:55 ` Jie Yang @ 2015-03-20 8:55 ` Jie Yang 2015-03-20 9:27 ` Takashi Iwai 2015-03-20 10:30 ` Raymond Yau 2015-03-20 8:55 ` [PATCH v2 2/2] ALSA: hda - Remove jack kctls Jie Yang 2 siblings, 2 replies; 40+ messages in thread From: Jie Yang @ 2015-03-20 8:55 UTC (permalink / raw) To: tiwai, perex, broonie; +Cc: Liam Girdwood, alsa-devel, liam.r.girdwood 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); -- 1.9.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 8:55 ` [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device Jie Yang @ 2015-03-20 9:27 ` Takashi Iwai 2015-03-20 9:29 ` Takashi Iwai 2015-03-20 12:22 ` Jie, Yang 2015-03-20 10:30 ` Raymond Yau 1 sibling, 2 replies; 40+ messages in thread From: Takashi Iwai @ 2015-03-20 9:27 UTC (permalink / raw) To: Jie Yang; +Cc: Liam Girdwood, alsa-devel, broonie, liam.r.girdwood 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 9:27 ` Takashi Iwai @ 2015-03-20 9:29 ` Takashi Iwai 2015-03-20 12:22 ` Jie, Yang 1 sibling, 0 replies; 40+ messages in thread From: Takashi Iwai @ 2015-03-20 9:29 UTC (permalink / raw) To: Jie Yang; +Cc: Liam Girdwood, alsa-devel, broonie, liam.r.girdwood 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 9:27 ` Takashi Iwai 2015-03-20 9:29 ` Takashi Iwai @ 2015-03-20 12:22 ` Jie, Yang 2015-03-20 12:26 ` Takashi Iwai 1 sibling, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-20 12:22 UTC (permalink / raw) To: Takashi Iwai; +Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R > -----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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 12:22 ` Jie, Yang @ 2015-03-20 12:26 ` Takashi Iwai 2015-03-20 12:50 ` Jie, Yang 0 siblings, 1 reply; 40+ messages in thread From: Takashi Iwai @ 2015-03-20 12:26 UTC (permalink / raw) To: Jie, Yang; +Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 12:26 ` Takashi Iwai @ 2015-03-20 12:50 ` Jie, Yang 2015-03-20 13:21 ` Takashi Iwai 2015-03-28 6:09 ` Raymond Yau 0 siblings, 2 replies; 40+ messages in thread From: Jie, Yang @ 2015-03-20 12:50 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, Tanu Kaskinen (tanu.kaskinen@linux.intel.com), Liam Girdwood, broonie, Girdwood, Liam R > -----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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 12:50 ` Jie, Yang @ 2015-03-20 13:21 ` Takashi Iwai 2015-03-20 13:49 ` Jie, Yang 2015-03-28 6:09 ` Raymond Yau 1 sibling, 1 reply; 40+ messages in thread From: Takashi Iwai @ 2015-03-20 13:21 UTC (permalink / raw) To: Jie, Yang Cc: alsa-devel, Tanu Kaskinen (tanu.kaskinen@linux.intel.com), Liam Girdwood, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 13:21 ` Takashi Iwai @ 2015-03-20 13:49 ` Jie, Yang 2015-03-20 14:18 ` Takashi Iwai 0 siblings, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-20 13:49 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, Tanu Kaskinen (tanu.kaskinen@linux.intel.com), Liam Girdwood, broonie, Girdwood, Liam R > -----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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 13:49 ` Jie, Yang @ 2015-03-20 14:18 ` Takashi Iwai 2015-03-23 10:56 ` Tanu Kaskinen 0 siblings, 1 reply; 40+ messages in thread From: Takashi Iwai @ 2015-03-20 14:18 UTC (permalink / raw) To: Jie, Yang Cc: alsa-devel, Tanu Kaskinen (tanu.kaskinen@linux.intel.com), Liam Girdwood, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 14:18 ` Takashi Iwai @ 2015-03-23 10:56 ` Tanu Kaskinen 2015-03-23 11:51 ` David Henningsson 2015-03-25 7:53 ` Jie, Yang 0 siblings, 2 replies; 40+ messages in thread From: Tanu Kaskinen @ 2015-03-23 10:56 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson 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=7369a53ab5f606e87a3cd1cd4eebd40226bab090 -- Tanu ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 10:56 ` Tanu Kaskinen @ 2015-03-23 11:51 ` David Henningsson 2015-03-23 11:59 ` Takashi Iwai ` (3 more replies) 2015-03-25 7:53 ` Jie, Yang 1 sibling, 4 replies; 40+ messages in thread From: David Henningsson @ 2015-03-23 11:51 UTC (permalink / raw) To: Tanu Kaskinen, Takashi Iwai Cc: alsa-devel, Liam Girdwood, broonie, Girdwood, Liam R 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-daemon/trunk/files/head:/plugins/media-keys/what-did-you-plug-in/ -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 11:51 ` David Henningsson @ 2015-03-23 11:59 ` Takashi Iwai 2015-03-23 16:41 ` Mark Brown ` (2 subsequent siblings) 3 siblings, 0 replies; 40+ messages in thread From: Takashi Iwai @ 2015-03-23 11:59 UTC (permalink / raw) To: David Henningsson Cc: Tanu Kaskinen, alsa-devel, Liam Girdwood, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 11:51 ` David Henningsson 2015-03-23 11:59 ` Takashi Iwai @ 2015-03-23 16:41 ` Mark Brown 2015-03-24 6:50 ` David Henningsson 2015-03-25 13:53 ` Jie, Yang 2015-03-26 2:34 ` Raymond Yau 3 siblings, 1 reply; 40+ messages in thread From: Mark Brown @ 2015-03-23 16:41 UTC (permalink / raw) To: David Henningsson Cc: Tanu Kaskinen, Takashi Iwai, alsa-devel, Liam Girdwood, Girdwood, Liam R [-- Attachment #1.1: Type: text/plain, Size: 645 bytes --] 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. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 16:41 ` Mark Brown @ 2015-03-24 6:50 ` David Henningsson 2015-03-24 17:57 ` Mark Brown 2015-03-25 2:14 ` Raymond Yau 0 siblings, 2 replies; 40+ messages in thread From: David Henningsson @ 2015-03-24 6:50 UTC (permalink / raw) To: Mark Brown Cc: Tanu Kaskinen, Takashi Iwai, alsa-devel, Liam Girdwood, Girdwood, Liam R 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. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-24 6:50 ` David Henningsson @ 2015-03-24 17:57 ` Mark Brown 2015-03-25 0:48 ` Raymond Yau 2015-03-25 2:14 ` Raymond Yau 1 sibling, 1 reply; 40+ messages in thread From: Mark Brown @ 2015-03-24 17:57 UTC (permalink / raw) To: David Henningsson Cc: Tanu Kaskinen, Takashi Iwai, alsa-devel, Liam Girdwood, Girdwood, Liam R [-- Attachment #1.1: Type: text/plain, Size: 1193 bytes --] 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. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-24 17:57 ` Mark Brown @ 2015-03-25 0:48 ` Raymond Yau 0 siblings, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-25 0:48 UTC (permalink / raw) To: Mark Brown Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, Girdwood, Liam R, David Henningsson > > > >>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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-24 6:50 ` David Henningsson 2015-03-24 17:57 ` Mark Brown @ 2015-03-25 2:14 ` Raymond Yau 1 sibling, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-25 2:14 UTC (permalink / raw) To: David Henningsson Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, Mark Brown, Girdwood, Liam R >> >> >>>> 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-17/manuals Alienware 17 Quick Start Guide ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 11:51 ` David Henningsson 2015-03-23 11:59 ` Takashi Iwai 2015-03-23 16:41 ` Mark Brown @ 2015-03-25 13:53 ` Jie, Yang 2015-03-25 16:13 ` Raymond Yau 2015-03-26 6:42 ` David Henningsson 2015-03-26 2:34 ` Raymond Yau 3 siblings, 2 replies; 40+ messages in thread From: Jie, Yang @ 2015-03-25 13:53 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-25 13:53 ` Jie, Yang @ 2015-03-25 16:13 ` Raymond Yau 2015-03-26 6:42 ` David Henningsson 1 sibling, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-25 16:13 UTC (permalink / raw) To: Jie, Yang Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson > > 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/pci/hda?id=5780b627e24113323427c102175296ae43dfb9d7 http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=7369a53ab5f606e87a3cd1cd4eebd40226bab090 http://www.asus.com/Notebooks_Ultrabooks/Eee_PC_1015PX/specifications/ ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-25 13:53 ` Jie, Yang 2015-03-25 16:13 ` Raymond Yau @ 2015-03-26 6:42 ` David Henningsson 2015-03-26 8:29 ` Jie, Yang 1 sibling, 1 reply; 40+ messages in thread From: David Henningsson @ 2015-03-26 6:42 UTC (permalink / raw) To: Jie, Yang, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 6:42 ` David Henningsson @ 2015-03-26 8:29 ` Jie, Yang 2015-03-26 9:05 ` David Henningsson 0 siblings, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-26 8:29 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 8:29 ` Jie, Yang @ 2015-03-26 9:05 ` David Henningsson 2015-03-26 12:39 ` Jie, Yang 2015-03-26 12:43 ` Jie, Yang 0 siblings, 2 replies; 40+ messages in thread From: David Henningsson @ 2015-03-26 9:05 UTC (permalink / raw) To: Jie, Yang, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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"). -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 9:05 ` David Henningsson @ 2015-03-26 12:39 ` Jie, Yang 2015-03-27 0:39 ` Raymond Yau 2015-03-26 12:43 ` Jie, Yang 1 sibling, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-26 12:39 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 > > 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"). > > -- > David Henningsson, Canonical Ltd. > https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 12:39 ` Jie, Yang @ 2015-03-27 0:39 ` Raymond Yau 2015-03-27 2:13 ` Jie, Yang 0 siblings, 1 reply; 40+ messages in thread From: Raymond Yau @ 2015-03-27 0:39 UTC (permalink / raw) To: Jie, Yang Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson 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/hda/hda_jack.c 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-27 0:39 ` Raymond Yau @ 2015-03-27 2:13 ` Jie, Yang 0 siblings, 0 replies; 40+ messages in thread From: Jie, Yang @ 2015-03-27 2:13 UTC (permalink / raw) To: Raymond Yau Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson 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/hda/hda_jack.c 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? _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 9:05 ` David Henningsson 2015-03-26 12:39 ` Jie, Yang @ 2015-03-26 12:43 ` Jie, Yang 2015-03-27 6:50 ` David Henningsson 1 sibling, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-26 12:43 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-26 12:43 ` Jie, Yang @ 2015-03-27 6:50 ` David Henningsson 2015-03-27 7:45 ` Jie, Yang 0 siblings, 1 reply; 40+ messages in thread From: David Henningsson @ 2015-03-27 6:50 UTC (permalink / raw) To: Jie, Yang, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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 > -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-27 6:50 ` David Henningsson @ 2015-03-27 7:45 ` Jie, Yang 2015-03-27 8:01 ` David Henningsson 0 siblings, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-27 7:45 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R > -----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 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 > > > > -- > David Henningsson, Canonical Ltd. > https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-27 7:45 ` Jie, Yang @ 2015-03-27 8:01 ` David Henningsson 2015-03-27 9:11 ` Jie, Yang 0 siblings, 1 reply; 40+ messages in thread From: David Henningsson @ 2015-03-27 8:01 UTC (permalink / raw) To: Jie, Yang, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-27 8:01 ` David Henningsson @ 2015-03-27 9:11 ` Jie, Yang 2015-03-30 7:27 ` David Henningsson 0 siblings, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-27 9:11 UTC (permalink / raw) To: David Henningsson, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R > -----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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-27 9:11 ` Jie, Yang @ 2015-03-30 7:27 ` David Henningsson 0 siblings, 0 replies; 40+ messages in thread From: David Henningsson @ 2015-03-30 7:27 UTC (permalink / raw) To: Jie, Yang, Tanu Kaskinen, Takashi Iwai Cc: Liam Girdwood, alsa-devel, broonie, Girdwood, Liam R 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. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 11:51 ` David Henningsson ` (2 preceding siblings ...) 2015-03-25 13:53 ` Jie, Yang @ 2015-03-26 2:34 ` Raymond Yau 3 siblings, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-26 2:34 UTC (permalink / raw) To: David Henningsson Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen, Liam Girdwood, broonie, Girdwood, Liam R >> >> 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/hda?qt=grep&q=dell-headset-multi 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-23 10:56 ` Tanu Kaskinen 2015-03-23 11:51 ` David Henningsson @ 2015-03-25 7:53 ` Jie, Yang 2015-03-25 9:27 ` Tanu Kaskinen 1 sibling, 1 reply; 40+ messages in thread From: Jie, Yang @ 2015-03-25 7:53 UTC (permalink / raw) To: Tanu Kaskinen, Takashi Iwai Cc: alsa-devel, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson > -----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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-25 7:53 ` Jie, Yang @ 2015-03-25 9:27 ` Tanu Kaskinen 2015-03-25 14:13 ` Jie, Yang 0 siblings, 1 reply; 40+ messages in thread From: Tanu Kaskinen @ 2015-03-25 9:27 UTC (permalink / raw) To: Jie, Yang Cc: alsa-devel, Takashi Iwai, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson 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, > 1. we need these naming, what you summarized below, that's quite helpful. David's summary is even more 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"? 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.) -- Tanu ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-25 9:27 ` Tanu Kaskinen @ 2015-03-25 14:13 ` Jie, Yang 0 siblings, 0 replies; 40+ messages in thread From: Jie, Yang @ 2015-03-25 14:13 UTC (permalink / raw) To: Tanu Kaskinen Cc: alsa-devel, Takashi Iwai, Liam Girdwood, broonie, Girdwood, Liam R, David Henningsson > -----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, > > 1. we need these naming, what you summarized below, that's quite helpful. > > David's summary is even more 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"? > > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 12:50 ` Jie, Yang 2015-03-20 13:21 ` Takashi Iwai @ 2015-03-28 6:09 ` Raymond Yau 1 sibling, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-28 6:09 UTC (permalink / raw) To: Jie, Yang Cc: ALSA Development Mailing List, Takashi Iwai, Tanu Kaskinen (tanu.kaskinen@linux.intel.com), Liam Girdwood, broonie, Girdwood, Liam R > > > > > > > > +} > > > > > + > > > > > +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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device 2015-03-20 8:55 ` [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device Jie Yang 2015-03-20 9:27 ` Takashi Iwai @ 2015-03-20 10:30 ` Raymond Yau 1 sibling, 0 replies; 40+ messages in thread From: Raymond Yau @ 2015-03-20 10:30 UTC (permalink / raw) To: Jie Yang; +Cc: alsa-devel, tiwai, Liam Girdwood, broonie, liam.r.girdwood > > 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 ? ^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v2 2/2] ALSA: hda - Remove jack kctls 2015-03-20 8:55 [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls Jie Yang 2015-03-20 8:55 ` Jie Yang 2015-03-20 8:55 ` [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device Jie Yang @ 2015-03-20 8:55 ` Jie Yang 2 siblings, 0 replies; 40+ messages in thread From: Jie Yang @ 2015-03-20 8:55 UTC (permalink / raw) To: tiwai, perex, broonie; +Cc: alsa-devel, liam.r.girdwood 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 * -- 1.9.1 ^ permalink raw reply related [flat|nested] 40+ messages in thread
end of thread, other threads:[~2015-03-30 7:27 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-20 8:55 [PATCH v2 0/2] ALSA: jack: Refactoring for jack kctls Jie Yang 2015-03-20 8:55 ` Jie Yang 2015-03-20 8:55 ` [PATCH v2 1/2] ALSA: jack: create jack kcontrols for every jack input device Jie Yang 2015-03-20 9:27 ` Takashi Iwai 2015-03-20 9:29 ` Takashi Iwai 2015-03-20 12:22 ` Jie, Yang 2015-03-20 12:26 ` Takashi Iwai 2015-03-20 12:50 ` Jie, Yang 2015-03-20 13:21 ` Takashi Iwai 2015-03-20 13:49 ` Jie, Yang 2015-03-20 14:18 ` Takashi Iwai 2015-03-23 10:56 ` Tanu Kaskinen 2015-03-23 11:51 ` David Henningsson 2015-03-23 11:59 ` Takashi Iwai 2015-03-23 16:41 ` Mark Brown 2015-03-24 6:50 ` David Henningsson 2015-03-24 17:57 ` Mark Brown 2015-03-25 0:48 ` Raymond Yau 2015-03-25 2:14 ` Raymond Yau 2015-03-25 13:53 ` Jie, Yang 2015-03-25 16:13 ` Raymond Yau 2015-03-26 6:42 ` David Henningsson 2015-03-26 8:29 ` Jie, Yang 2015-03-26 9:05 ` David Henningsson 2015-03-26 12:39 ` Jie, Yang 2015-03-27 0:39 ` Raymond Yau 2015-03-27 2:13 ` Jie, Yang 2015-03-26 12:43 ` Jie, Yang 2015-03-27 6:50 ` David Henningsson 2015-03-27 7:45 ` Jie, Yang 2015-03-27 8:01 ` David Henningsson 2015-03-27 9:11 ` Jie, Yang 2015-03-30 7:27 ` David Henningsson 2015-03-26 2:34 ` Raymond Yau 2015-03-25 7:53 ` Jie, Yang 2015-03-25 9:27 ` Tanu Kaskinen 2015-03-25 14:13 ` Jie, Yang 2015-03-28 6:09 ` Raymond Yau 2015-03-20 10:30 ` Raymond Yau 2015-03-20 8:55 ` [PATCH v2 2/2] ALSA: hda - Remove jack kctls Jie Yang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.