Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

So, a question of "Line Out" to be hyphened or not to be hyphened.

The problem is less serious than Hamlet, and I think we can get rid of
the redundant hyphen.  Not only because many women dislike hyphened
names nowadays (as you can find easily in Google), the naming rule
with a direction suffix looks more flexible, and I saw that both or
Mark and David don't matter much (so written in their posts).

The fix patch is below.  David, if you think it's too bad (that makes
your code too messy), please speak up.  Or, this is utterly broken,
also let me know.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda/realtek - Kill hyphnated names

Kill hyphens from "Line-Out" name strings, as suggested by Mark Brown.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_codec.c     |    4 ++--
 sound/pci/hda/patch_realtek.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 0ae6eb2..6843073 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -5118,7 +5118,7 @@ static int fill_audio_out_name(struct hda_codec *codec, hda_nid_t nid,
 	const char *pfx = "", *sfx = "";
 
 	/* handle as a speaker if it's a fixed line-out */
-	if (!strcmp(name, "Line-Out") && attr == INPUT_PIN_ATTR_INT)
+	if (!strcmp(name, "Line Out") && attr == INPUT_PIN_ATTR_INT)
 		name = "Speaker";
 	/* check the location */
 	switch (attr) {
@@ -5177,7 +5177,7 @@ int snd_hda_get_pin_label(struct hda_codec *codec, hda_nid_t nid,
 
 	switch (get_defcfg_device(def_conf)) {
 	case AC_JACK_LINE_OUT:
-		return fill_audio_out_name(codec, nid, cfg, "Line-Out",
+		return fill_audio_out_name(codec, nid, cfg, "Line Out",
 					   label, maxlen, indexp);
 	case AC_JACK_SPEAKER:
 		return fill_audio_out_name(codec, nid, cfg, "Speaker",
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 4fe2d59..a5be866 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1856,7 +1856,7 @@ static const char * const alc_slave_vols[] = {
 	"Headphone Playback Volume",
 	"Speaker Playback Volume",
 	"Mono Playback Volume",
-	"Line-Out Playback Volume",
+	"Line Out Playback Volume",
 	"CLFE Playback Volume",
 	"Bass Speaker Playback Volume",
 	"PCM Playback Volume",
@@ -1873,7 +1873,7 @@ static const char * const alc_slave_sws[] = {
 	"Speaker Playback Switch",
 	"Mono Playback Switch",
 	"IEC958 Playback Switch",
-	"Line-Out Playback Switch",
+	"Line Out Playback Switch",
 	"CLFE Playback Switch",
 	"Bass Speaker Playback Switch",
 	"PCM Playback Switch",
-- 
1.7.9.2

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 03/01/2012 06:48 PM, Takashi Iwai wrote:
> So, a question of "Line Out" to be hyphened or not to be hyphened.
>
> The problem is less serious than Hamlet, and I think we can get rid of
> the redundant hyphen.  Not only because many women dislike hyphened
> names nowadays (as you can find easily in Google),

Yes, it is also inspired by the global individualism movement, clearly 
'Line' and 'Out' do not want to appear to be dependent on each other.

> the naming rule
> with a direction suffix looks more flexible, and I saw that both or
> Mark and David don't matter much (so written in their posts).
>
> The fix patch is below.  David, if you think it's too bad (that makes
> your code too messy), please speak up.  Or, this is utterly broken,
> also let me know.

There is no problem with this change. But is it used for mixer controls 
anywhere, or only jacks? So far I haven't seen a mixer control having 
either 'Line-Out' nor 'Line Out' in it.

A followup, slightly more serious question is 'Line' vs 'Line In'. It 
seems PulseAudio only supports 'Line' today, but adding support is trivial.

>
>
> thanks,
>
> Takashi
>
> ---
> From: Takashi Iwai<tiwai@suse.de>
> Subject: [PATCH] ALSA: hda/realtek - Kill hyphnated names

Nitpick: Missing "e" in "hyphen".

>
> Kill hyphens from "Line-Out" name strings, as suggested by Mark Brown.
>
> Signed-off-by: Takashi Iwai<tiwai@suse.de>
> ---
>   sound/pci/hda/hda_codec.c     |    4 ++--
>   sound/pci/hda/patch_realtek.c |    4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)

I think you should change 'Line-Out' to 'Line Out' on a more global 
basis. A quick grep for 'Line-Out' shows hits (that are not comments) in 
patch_realtek.c, patch_cirrus.c and patch_conexant.c, as well as in 
ac97, aoa and parisc directories. At least the patch_cirrus one should 
be changed IMO.

Also, in the sound/soc there seem to be plenty of hits for 'Line Output' 
which could be changed to 'Line Out', mostly for consistency. Something 
for Mark?

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Fri, 02 Mar 2012 07:26:30 +0100,
David Henningsson wrote:
> 
> On 03/01/2012 06:48 PM, Takashi Iwai wrote:
> > So, a question of "Line Out" to be hyphened or not to be hyphened.
> >
> > The problem is less serious than Hamlet, and I think we can get rid of
> > the redundant hyphen.  Not only because many women dislike hyphened
> > names nowadays (as you can find easily in Google),
> 
> Yes, it is also inspired by the global individualism movement, clearly 
> 'Line' and 'Out' do not want to appear to be dependent on each other.

Yes, the modern time.  "Line In" has a right to keep its name even
after it being "Surround Out".

> > the naming rule
> > with a direction suffix looks more flexible, and I saw that both or
> > Mark and David don't matter much (so written in their posts).
> >
> > The fix patch is below.  David, if you think it's too bad (that makes
> > your code too messy), please speak up.  Or, this is utterly broken,
> > also let me know.
> 
> There is no problem with this change. But is it used for mixer controls 
> anywhere, or only jacks? So far I haven't seen a mixer control having 
> either 'Line-Out' nor 'Line Out' in it.

Good point.  I'll change it, too.

> A followup, slightly more serious question is 'Line' vs 'Line In'. It 
> seems PulseAudio only supports 'Line' today, but adding support is trivial.

It'd be safer if PA can support both.  But I guess mixer controls will
stay without "In" suffix.  That is, when the directional suffix is
omitted, it depends on the base type: the default for "Mic" is input,
"Headphone" is output.  And "Line" is input.


> > thanks,
> >
> > Takashi
> >
> > ---
> > From: Takashi Iwai<tiwai@suse.de>
> > Subject: [PATCH] ALSA: hda/realtek - Kill hyphnated names
> 
> Nitpick: Missing "e" in "hyphen".

Oops.

> > Kill hyphens from "Line-Out" name strings, as suggested by Mark Brown.
> >
> > Signed-off-by: Takashi Iwai<tiwai@suse.de>
> > ---
> >   sound/pci/hda/hda_codec.c     |    4 ++--
> >   sound/pci/hda/patch_realtek.c |    4 ++--
> >   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> I think you should change 'Line-Out' to 'Line Out' on a more global 
> basis. A quick grep for 'Line-Out' shows hits (that are not comments) in 
> patch_realtek.c, patch_cirrus.c and patch_conexant.c, as well as in 
> ac97, aoa and parisc directories. At least the patch_cirrus one should 
> be changed IMO.

That makes sense.

> Also, in the sound/soc there seem to be plenty of hits for 'Line Output' 
> which could be changed to 'Line Out', mostly for consistency. Something 
> for Mark?

I think we should standardize "Line Out" mixer control name at first,
e.g. updating the outdated ControlNames.txt document.
Anyway I'll leave ASoC part as is for now.


Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 818 bytes --]

On Fri, Mar 02, 2012 at 08:16:56AM +0100, Takashi Iwai wrote:
> David Henningsson wrote:

> > Also, in the sound/soc there seem to be plenty of hits for 'Line Output' 
> > which could be changed to 'Line Out', mostly for consistency. Something 
> > for Mark?

> I think we should standardize "Line Out" mixer control name at first,
> e.g. updating the outdated ControlNames.txt document.
> Anyway I'll leave ASoC part as is for now.

There's also a lot of LINEOUTn and so on.  I've no intention of worrying
about this stuff too much for ASoC, the name based routing is mostly
nonsensical there anyway and it's very common for the thing connected to
the line output to actually be another thing such as a speaker so acting
on the fact that it's a line output might be actively wrong at the user
level.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 1/2] ALSA: Use a define for the number of jack switch types

[Mark Brown]

This is intended to facilitate the merge of the two jack detection
mechanisms.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/jack.h |    3 +++
 sound/core/jack.c    |    4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 63c7907..5891657 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -53,6 +53,9 @@ enum snd_jack_types {
 	SND_JACK_BTN_5		= 0x0200,
 };
 
+/* Keep in sync with definitions above */
+#define SND_JACK_SWITCH_TYPES 6
+
 struct snd_jack {
 	struct input_dev *input_dev;
 	int registered;
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 26edf63..471e1e3 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -25,7 +25,7 @@
 #include <sound/jack.h>
 #include <sound/core.h>
 
-static int jack_switch_types[] = {
+static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
 	SW_HEADPHONE_INSERT,
 	SW_MICROPHONE_INSERT,
 	SW_LINEOUT_INSERT,
@@ -128,7 +128,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 
 	jack->type = type;
 
-	for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++)
+	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
 		if (type & (1 << i))
 			input_set_capability(jack->input_dev, EV_SW,
 					     jack_switch_types[i]);
-- 
1.7.9.rc1

[PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

Create controls for jack reporting when creating a standard jack, allowing
drivers to support both userspace interfaces via a single core interface.
There's a couple of improvements that could be made (like using the jack
name in the control name) but these are punted until after we remove direct
users of snd_kctl_jack_*().

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/jack.h |    2 +
 sound/core/Kconfig   |    1 +
 sound/core/jack.c    |   63 ++++++++++++++++++++++++++++++++++++++------------
 3 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 5891657..4e50ae5 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -58,10 +58,12 @@ enum snd_jack_types {
 
 struct snd_jack {
 	struct input_dev *input_dev;
+	struct snd_card *card;
 	int registered;
 	int type;
 	const char *id;
 	char name[100];
+	struct snd_kcontrol *ctl[SND_JACK_SWITCH_TYPES];
 	unsigned int key[6];   /* Keep in sync with definitions above */
 	void *private_data;
 	void (*private_free)(struct snd_jack *);
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index b413ed0..e99b0171 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -211,6 +211,7 @@ config SND_VMASTER
 
 config SND_KCTL_JACK
 	bool
+	default y if SND_JACK
 
 config SND_DMA_SGBUF
 	def_bool y
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 471e1e3..c0fe7b9 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -24,19 +24,25 @@
 #include <linux/module.h>
 #include <sound/jack.h>
 #include <sound/core.h>
-
-static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
-	SW_HEADPHONE_INSERT,
-	SW_MICROPHONE_INSERT,
-	SW_LINEOUT_INSERT,
-	SW_JACK_PHYSICAL_INSERT,
-	SW_VIDEOOUT_INSERT,
-	SW_LINEIN_INSERT,
+#include <sound/control.h>
+
+static const struct {
+	int input_type;
+	const char *name;
+} jack_switch_types[SND_JACK_SWITCH_TYPES] = {
+	{ SW_HEADPHONE_INSERT, "Headphone" },
+	{ SW_MICROPHONE_INSERT, "Microphone" },
+	{ SW_LINEOUT_INSERT, "Line Out" },
+	{ SW_JACK_PHYSICAL_INSERT, "Mechanical" },
+	{ SW_VIDEOOUT_INSERT, "Video Out" },
+	{ SW_LINEIN_INSERT, "Line In" },
 };
 
 static int snd_jack_dev_free(struct snd_device *device)
 {
+	struct snd_card *card = device->card;
 	struct snd_jack *jack = device->device_data;
+	int i;
 
 	if (jack->private_free)
 		jack->private_free(jack);
@@ -48,6 +54,10 @@ static int snd_jack_dev_free(struct snd_device *device)
 	else
 		input_free_device(jack->input_dev);
 
+	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
+		if (jack->ctl[i])
+			snd_ctl_remove(card, jack->ctl[i]);
+
 	kfree(jack->id);
 	kfree(jack);
 
@@ -105,8 +115,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 		 struct snd_jack **jjack)
 {
 	struct snd_jack *jack;
-	int err;
+	int err = 0;
 	int i;
+	struct snd_kcontrol *kcontrol;
 	static struct snd_device_ops ops = {
 		.dev_free = snd_jack_dev_free,
 		.dev_register = snd_jack_dev_register,
@@ -116,6 +127,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	if (jack == NULL)
 		return -ENOMEM;
 
+	jack->card = card;
 	jack->id = kstrdup(id, GFP_KERNEL);
 
 	jack->input_dev = input_allocate_device();
@@ -128,10 +140,28 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 
 	jack->type = type;
 
-	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
-		if (type & (1 << i))
-			input_set_capability(jack->input_dev, EV_SW,
-					     jack_switch_types[i]);
+	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++) {
+		if (!(type & (1 << i)))
+			continue;
+
+		input_set_capability(jack->input_dev, EV_SW,
+				     jack_switch_types[i].input_type);
+
+		kcontrol = snd_kctl_jack_new(jack_switch_types[i].name, 0,
+					     NULL);
+		if (kcontrol) {
+			err = snd_ctl_add(card, kcontrol);
+			if (!err)
+				jack->ctl[i] = kcontrol;
+			else
+				dev_warn(card->dev,
+					 "Failed to create %s control: %d\n",
+					 jack_switch_types[i].name, err);
+		} else {
+			dev_warn(card->dev, "Failed to create %s control\n",
+				 jack_switch_types[i].name);
+		}
+	}
 
 	err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
 	if (err < 0)
@@ -227,10 +257,13 @@ 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],
+					    jack_switch_types[i].input_type,
 					    status & testbit);
+			snd_kctl_jack_report(jack->card, jack->ctl[i],
+					     status & testbit);
+		}
 	}
 
 	input_sync(jack->input_dev);
-- 
1.7.9.rc1

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/07/2012 08:48 PM, Mark Brown wrote:
> Create controls for jack reporting when creating a standard jack, allowing
> drivers to support both userspace interfaces via a single core interface.
> There's a couple of improvements that could be made (like using the jack
> name in the control name) but these are punted until after we remove direct
> users of snd_kctl_jack_*().
>
> Signed-off-by: Mark Brown<broonie@opensource.wolfsonmicro.com>
> ---
>   include/sound/jack.h |    2 +
>   sound/core/Kconfig   |    1 +
>   sound/core/jack.c    |   63 ++++++++++++++++++++++++++++++++++++++------------
>   3 files changed, 51 insertions(+), 15 deletions(-)
>
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index 5891657..4e50ae5 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -58,10 +58,12 @@ enum snd_jack_types {
>
>   struct snd_jack {
>   	struct input_dev *input_dev;
> +	struct snd_card *card;
>   	int registered;
>   	int type;
>   	const char *id;
>   	char name[100];
> +	struct snd_kcontrol *ctl[SND_JACK_SWITCH_TYPES];
>   	unsigned int key[6];   /* Keep in sync with definitions above */
>   	void *private_data;
>   	void (*private_free)(struct snd_jack *);
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index b413ed0..e99b0171 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -211,6 +211,7 @@ config SND_VMASTER
>
>   config SND_KCTL_JACK
>   	bool
> +	default y if SND_JACK
>
>   config SND_DMA_SGBUF
>   	def_bool y
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index 471e1e3..c0fe7b9 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -24,19 +24,25 @@
>   #include<linux/module.h>
>   #include<sound/jack.h>
>   #include<sound/core.h>
> -
> -static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
> -	SW_HEADPHONE_INSERT,
> -	SW_MICROPHONE_INSERT,
> -	SW_LINEOUT_INSERT,
> -	SW_JACK_PHYSICAL_INSERT,
> -	SW_VIDEOOUT_INSERT,
> -	SW_LINEIN_INSERT,
> +#include<sound/control.h>
> +
> +static const struct {
> +	int input_type;
> +	const char *name;
> +} jack_switch_types[SND_JACK_SWITCH_TYPES] = {
> +	{ SW_HEADPHONE_INSERT, "Headphone" },
> +	{ SW_MICROPHONE_INSERT, "Microphone" },
> +	{ SW_LINEOUT_INSERT, "Line Out" },
> +	{ SW_JACK_PHYSICAL_INSERT, "Mechanical" },
> +	{ SW_VIDEOOUT_INSERT, "Video Out" },
> +	{ SW_LINEIN_INSERT, "Line In" },
>   };

I'd like these to match the names currently used in HDA, like this:

	{ SW_HEADPHONE_INSERT, "Headphone" },
	{ SW_MICROPHONE_INSERT, "Mic" },
	{ SW_LINEOUT_INSERT, "Line-Out" },
	{ SW_JACK_PHYSICAL_INSERT, "Mechanical" },
	{ SW_VIDEOOUT_INSERT, "Video-Out" },
	{ SW_LINEIN_INSERT, "Line" },

Actually, it matters less if we settle on the standard you set above, or 
what the HDA currently does, as long as the names are the same.

>
>   static int snd_jack_dev_free(struct snd_device *device)
>   {
> +	struct snd_card *card = device->card;
>   	struct snd_jack *jack = device->device_data;
> +	int i;
>
>   	if (jack->private_free)
>   		jack->private_free(jack);
> @@ -48,6 +54,10 @@ static int snd_jack_dev_free(struct snd_device *device)
>   	else
>   		input_free_device(jack->input_dev);
>
> +	for (i = 0; i<  SND_JACK_SWITCH_TYPES; i++)
> +		if (jack->ctl[i])
> +			snd_ctl_remove(card, jack->ctl[i]);
> +
>   	kfree(jack->id);
>   	kfree(jack);
>
> @@ -105,8 +115,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>   		 struct snd_jack **jjack)
>   {
>   	struct snd_jack *jack;
> -	int err;
> +	int err = 0;
>   	int i;
> +	struct snd_kcontrol *kcontrol;
>   	static struct snd_device_ops ops = {
>   		.dev_free = snd_jack_dev_free,
>   		.dev_register = snd_jack_dev_register,
> @@ -116,6 +127,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>   	if (jack == NULL)
>   		return -ENOMEM;
>
> +	jack->card = card;
>   	jack->id = kstrdup(id, GFP_KERNEL);
>
>   	jack->input_dev = input_allocate_device();
> @@ -128,10 +140,28 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>
>   	jack->type = type;
>
> -	for (i = 0; i<  SND_JACK_SWITCH_TYPES; i++)
> -		if (type&  (1<<  i))
> -			input_set_capability(jack->input_dev, EV_SW,
> -					     jack_switch_types[i]);
> +	for (i = 0; i<  SND_JACK_SWITCH_TYPES; i++) {
> +		if (!(type&  (1<<  i)))
> +			continue;
> +
> +		input_set_capability(jack->input_dev, EV_SW,
> +				     jack_switch_types[i].input_type);
> +
> +		kcontrol = snd_kctl_jack_new(jack_switch_types[i].name, 0,
> +					     NULL);
> +		if (kcontrol) {
> +			err = snd_ctl_add(card, kcontrol);
> +			if (!err)
> +				jack->ctl[i] = kcontrol;
> +			else
> +				dev_warn(card->dev,
> +					 "Failed to create %s control: %d\n",
> +					 jack_switch_types[i].name, err);
> +		} else {
> +			dev_warn(card->dev, "Failed to create %s control\n",
> +				 jack_switch_types[i].name);
> +		}
> +	}
>
>   	err = snd_device_new(card, SNDRV_DEV_JACK, jack,&ops);
>   	if (err<  0)
> @@ -227,10 +257,13 @@ 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],
> +					    jack_switch_types[i].input_type,
>   					    status&  testbit);
> +			snd_kctl_jack_report(jack->card, jack->ctl[i],
> +					     status&  testbit);
> +		}
>   	}
>
>   	input_sync(jack->input_dev);



-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 969 bytes --]

On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:
> On 02/07/2012 08:48 PM, Mark Brown wrote:

> I'd like these to match the names currently used in HDA, like this:

> 	{ SW_MICROPHONE_INSERT, "Mic" },

It seems odd to abbreviate this and nothing else but I'm not that
fussed either way.

> 	{ SW_LINEOUT_INSERT, "Line-Out" },
> 	{ SW_VIDEOOUT_INSERT, "Video-Out" },

For these it's really not normal English to hyphenate them, it looks
strange to do so.

> 	{ SW_LINEIN_INSERT, "Line" },

It seems odd and a bit undescriptive to not specify the description here
when it is specified for the outputs?

> Actually, it matters less if we settle on the standard you set
> above, or what the HDA currently does, as long as the names are the
> same.

Except for Mic where I don't mind either way I'd rather bring the HDA
names into (but note that the idea is to remove the HDA specific jack
controls - I think these names are used in other places though?).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/08/2012 12:46 PM, Mark Brown wrote:
> On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:
>> On 02/07/2012 08:48 PM, Mark Brown wrote:
>
>> I'd like these to match the names currently used in HDA, like this:
>
>> 	{ SW_MICROPHONE_INSERT, "Mic" },
>
> It seems odd to abbreviate this and nothing else but I'm not that
> fussed either way.
>
>> 	{ SW_LINEOUT_INSERT, "Line-Out" },
>> 	{ SW_VIDEOOUT_INSERT, "Video-Out" },
>
> For these it's really not normal English to hyphenate them, it looks
> strange to do so.

I guess it could be easier to parse if we avoid spaces in names, but 
right now there is no such parser AFAIK anyway, so it does not matter 
much. Video-Out is not present in HDA; I did that change to make it 
consistent with "Line-Out".

>> 	{ SW_LINEIN_INSERT, "Line" },
>
> It seems odd and a bit undescriptive to not specify the description here
> when it is specified for the outputs?
>
>> Actually, it matters less if we settle on the standard you set
>> above, or what the HDA currently does, as long as the names are the
>> same.
>
> Except for Mic where I don't mind either way I'd rather bring the HDA
> names into (but note that the idea is to remove the HDA specific jack
> controls - I think these names are used in other places though?).

I'll let Takashi have the final say about the names. But note that for 
HDA these names will not be enough, e g, we might have one "Front Mic" 
and one "Rear Mic" and need to know which one is which.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]

On Wed, Feb 08, 2012 at 02:35:29PM +0100, David Henningsson wrote:
> On 02/08/2012 12:46 PM, Mark Brown wrote:

> >Except for Mic where I don't mind either way I'd rather bring the HDA
> >names into (but note that the idea is to remove the HDA specific jack
> >controls - I think these names are used in other places though?).

> I'll let Takashi have the final say about the names. But note that
> for HDA these names will not be enough, e g, we might have one
> "Front Mic" and one "Rear Mic" and need to know which one is which.

Remember that once these are merged properly we'll be adding the jack
name on the front so the bare names will only exist for one or two
commits - doing so sensibly would mean changing the kctl API and doing
a refactor on that when the immediate goal is to make it private seems
like more trouble than it's worth.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Wed, 08 Feb 2012 14:35:29 +0100,
David Henningsson wrote:
> 
> On 02/08/2012 12:46 PM, Mark Brown wrote:
> > On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:
> >> On 02/07/2012 08:48 PM, Mark Brown wrote:
> >
> >> I'd like these to match the names currently used in HDA, like this:
> >
> >> 	{ SW_MICROPHONE_INSERT, "Mic" },
> >
> > It seems odd to abbreviate this and nothing else but I'm not that
> > fussed either way.
> >
> >> 	{ SW_LINEOUT_INSERT, "Line-Out" },
> >> 	{ SW_VIDEOOUT_INSERT, "Video-Out" },
> >
> > For these it's really not normal English to hyphenate them, it looks
> > strange to do so.
> 
> I guess it could be easier to parse if we avoid spaces in names,

Exactly, it was the reason.

> but 
> right now there is no such parser AFAIK anyway, so it does not matter 
> much. Video-Out is not present in HDA; I did that change to make it 
> consistent with "Line-Out".
> 
> >> 	{ SW_LINEIN_INSERT, "Line" },
> >
> > It seems odd and a bit undescriptive to not specify the description here
> > when it is specified for the outputs?

Historically, "Line" represents an line input in ALSA control names.
But it wouldn't be bad to an explicit directional notation, too.

> >> Actually, it matters less if we settle on the standard you set
> >> above, or what the HDA currently does, as long as the names are the
> >> same.
> >
> > Except for Mic where I don't mind either way I'd rather bring the HDA
> > names into (but note that the idea is to remove the HDA specific jack
> > controls - I think these names are used in other places though?).
> 
> I'll let Takashi have the final say about the names. But note that for 
> HDA these names will not be enough, e g, we might have one "Front Mic" 
> and one "Rear Mic" and need to know which one is which.

What I have in my mind is a form like "[location] base [channel]".
The location prefix (e.g. Front, Rear) is optional, and also the
channel suffix, too.

I have no strong opinion Whether to allow a space in the base name.
In my patch, I chose hyphens just to make parsing easier.

OTOH, we can take an optional directional suffix, i.e.
"[location] base [direction] [channel]", too.  For example, base can
be "Video" or "Line", and direction can be "Out" or "In".

I'd like to hear rather comments from others.


thanks,

Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 693 bytes --]

On Fri, Feb 10, 2012 at 11:55:10AM +0100, Takashi Iwai wrote:

> OTOH, we can take an optional directional suffix, i.e.
> "[location] base [direction] [channel]", too.  For example, base can
> be "Video" or "Line", and direction can be "Out" or "In".

These names are going to get pretty verbose very easily.  Might it be
worth putting some of this (and also other information like colour) into
TLV information rather than part of the base jack name?  So long as we
can extract a name for the jack itself (which is probably the location
in your example above) I think we're fine.

I think the main thing is to clean up this mess for 3.4 so we don't get
too many confused userspaces out there.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Fri, 10 Feb 2012 11:36:06 +0000,
Mark Brown wrote:
> 
> On Fri, Feb 10, 2012 at 11:55:10AM +0100, Takashi Iwai wrote:
> 
> > OTOH, we can take an optional directional suffix, i.e.
> > "[location] base [direction] [channel]", too.  For example, base can
> > be "Video" or "Line", and direction can be "Out" or "In".
> 
> These names are going to get pretty verbose very easily.  Might it be
> worth putting some of this (and also other information like colour) into
> TLV information rather than part of the base jack name?

Yeah, we should avoid to put too much there.  There is a limitation
in the name string size, too.

>  So long as we
> can extract a name for the jack itself (which is probably the location
> in your example above) I think we're fine.
> 
> I think the main thing is to clean up this mess for 3.4 so we don't get
> too many confused userspaces out there.

Agreed.


Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/10/2012 11:55 AM, Takashi Iwai wrote:
> At Wed, 08 Feb 2012 14:35:29 +0100,
> David Henningsson wrote:
>>
>> On 02/08/2012 12:46 PM, Mark Brown wrote:
>>> On Wed, Feb 08, 2012 at 09:36:21AM +0100, David Henningsson wrote:
>>>> On 02/07/2012 08:48 PM, Mark Brown wrote:
>>>
>>>> I'd like these to match the names currently used in HDA, like this:
>>>
>>>> 	{ SW_MICROPHONE_INSERT, "Mic" },
>>>
>>> It seems odd to abbreviate this and nothing else but I'm not that
>>> fussed either way.
>>>
>>>> 	{ SW_LINEOUT_INSERT, "Line-Out" },
>>>> 	{ SW_VIDEOOUT_INSERT, "Video-Out" },
>>>
>>> For these it's really not normal English to hyphenate them, it looks
>>> strange to do so.
>>
>> I guess it could be easier to parse if we avoid spaces in names,
>
> Exactly, it was the reason.

Given that the normal desktop user does not usually look at these values 
these days, I'd say being parser friendly weighs heavier than being 
"normal English".

>> but
>> right now there is no such parser AFAIK anyway, so it does not matter
>> much. Video-Out is not present in HDA; I did that change to make it
>> consistent with "Line-Out".
>>
>>>> 	{ SW_LINEIN_INSERT, "Line" },
>>>
>>> It seems odd and a bit undescriptive to not specify the description here
>>> when it is specified for the outputs?
>
> Historically, "Line" represents an line input in ALSA control names.
> But it wouldn't be bad to an explicit directional notation, too.
>
>>>> Actually, it matters less if we settle on the standard you set
>>>> above, or what the HDA currently does, as long as the names are the
>>>> same.
>>>
>>> Except for Mic where I don't mind either way I'd rather bring the HDA
>>> names into (but note that the idea is to remove the HDA specific jack
>>> controls - I think these names are used in other places though?).
>>
>> I'll let Takashi have the final say about the names. But note that for
>> HDA these names will not be enough, e g, we might have one "Front Mic"
>> and one "Rear Mic" and need to know which one is which.
>
> What I have in my mind is a form like "[location] base [channel]".
> The location prefix (e.g. Front, Rear) is optional, and also the
> channel suffix, too.
>
> I have no strong opinion Whether to allow a space in the base name.
> In my patch, I chose hyphens just to make parsing easier.
>
> OTOH, we can take an optional directional suffix, i.e.
> "[location] base [direction] [channel]", too.  For example, base can
> be "Video" or "Line", and direction can be "Out" or "In".
>
> I'd like to hear rather comments from others.

I think both naming schemes are good, and I'm not too worried about them 
being too verbose. If we run into names being longer than the string 
length we could back off and drop the location, I guess.

If we extend the thought about applying this scheme for mixer control 
names as well, we might run into controls that apply to two outputs but 
not the third, and then the complexity will increase, like "Front 
HP,Line-Out Playback Switch". (And what if the Headphone is located on 
the Front side and the Line-Out is on the back, etc) But then we run 
into the entire story of exposing a graph instead, I guess. :-/

Also don't forget about HDA HDMI, where we have four jacks but only one 
or two being physically connected, and we need to map these against a 
PCM device.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

On Fri, Feb 10, 2012 at 02:08:52PM +0100, David Henningsson wrote:
> On 02/10/2012 11:55 AM, Takashi Iwai wrote:
> > David Henningsson wrote:

> >> I guess it could be easier to parse if we avoid spaces in names,

> > Exactly, it was the reason.

> Given that the normal desktop user does not usually look at these values 
> these days, I'd say being parser friendly weighs heavier than being 
> "normal English".

Not all the world is desktop.

> > OTOH, we can take an optional directional suffix, i.e.
> > "[location] base [direction] [channel]", too.  For example, base can
> > be "Video" or "Line", and direction can be "Out" or "In".

> > I'd like to hear rather comments from others.

> I think both naming schemes are good, and I'm not too worried about them 
> being too verbose. If we run into names being longer than the string 
> length we could back off and drop the location, I guess.

It's a complete pain for actually working with them and doing
development - it renders badly in UIs (think about alsamixer for
example, or people looking at things on 80 column terminals) and isn't
friendly to people typing things in.

> If we extend the thought about applying this scheme for mixer control 
> names as well, we might run into controls that apply to two outputs but 
> not the third, and then the complexity will increase, like "Front 
> HP,Line-Out Playback Switch". (And what if the Headphone is located on 
> the Front side and the Line-Out is on the back, etc) But then we run 
> into the entire story of exposing a graph instead, I guess. :-/

There's no solution except to expose a graph - modern devices are going
to have sufficiently flexible routing that most of the controls have no
association with a particular output (or input for that matter).  

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/10/2012 04:50 PM, Mark Brown wrote:
> On Fri, Feb 10, 2012 at 02:08:52PM +0100, David Henningsson wrote:
>> On 02/10/2012 11:55 AM, Takashi Iwai wrote:
>>> David Henningsson wrote:
>
>>>> I guess it could be easier to parse if we avoid spaces in names,
>>> Exactly, it was the reason.
>> Given that the normal desktop user does not usually look at these values
>> these days, I'd say being parser friendly weighs heavier than being
>> "normal English".
> Not all the world is desktop.

The normal end user of the embedded system does not look at those values 
either.

>>> OTOH, we can take an optional directional suffix, i.e.
>>> "[location] base [direction] [channel]", too.  For example, base can
>>> be "Video" or "Line", and direction can be "Out" or "In".
>
>>> I'd like to hear rather comments from others.
>
>> I think both naming schemes are good, and I'm not too worried about them
>> being too verbose. If we run into names being longer than the string
>> length we could back off and drop the location, I guess.
>
> It's a complete pain for actually working with them and doing
> development - it renders badly in UIs (think about alsamixer for
> example, or people looking at things on 80 column terminals) and isn't
> friendly to people typing things in.

So your suggestion was, to avoid "Front Headphone" and "Rear Headphone" 
because the names were too long, and instead have "Headphone" and 
"Headphone,index=1" and have "Front" and "Rear" read out of a TLV?

That will be worse for everyone, both those doing work with them, 
looking in alsamixer, etc.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2095 bytes --]

On Fri, Feb 10, 2012 at 05:09:35PM +0100, David Henningsson wrote:
> On 02/10/2012 04:50 PM, Mark Brown wrote:

> >>Given that the normal desktop user does not usually look at these values
> >>these days, I'd say being parser friendly weighs heavier than being
> >>"normal English".

> >Not all the world is desktop.

> The normal end user of the embedded system does not look at those
> values either.

There's rather a lot of people who do embedded development and do need
to look at this stuff, though.  There's a real cost to raising the bar
for humans to understand things.

> >>I think both naming schemes are good, and I'm not too worried about them
> >>being too verbose. If we run into names being longer than the string
> >>length we could back off and drop the location, I guess.

> >It's a complete pain for actually working with them and doing
> >development - it renders badly in UIs (think about alsamixer for
> >example, or people looking at things on 80 column terminals) and isn't
> >friendly to people typing things in.

> So your suggestion was, to avoid "Front Headphone" and "Rear
> Headphone" because the names were too long, and instead have
> "Headphone" and "Headphone,index=1" and have "Front" and "Rear" read
> out of a TLV?

No, some of the information should go in the name but we keep on coming
up with more and more things to add and when we end up with "Left Silver
Headset Jack Headphone" or whatever (which would be a fairly complete
description of half the headset jack on one of my laptops) we probably
ought to be cutting some of that out.  

Though thinking about it duplicating all the individual bits of
information in TLVs would be helpful to automatic parsing, we could
possibly even do something clever like make minimal subset names that
contained only the bits that were unique on a given system if we were
bored enough.

Oh, by the way the extcon (Android switch class more or less) code looks
to be coming along quite well which is going to be a third way of
getting jack information out to userspace.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Fri, 10 Feb 2012 16:39:46 +0000,
Mark Brown wrote:
> 
> On Fri, Feb 10, 2012 at 05:09:35PM +0100, David Henningsson wrote:
> > On 02/10/2012 04:50 PM, Mark Brown wrote:
> 
> > >>Given that the normal desktop user does not usually look at these values
> > >>these days, I'd say being parser friendly weighs heavier than being
> > >>"normal English".
> 
> > >Not all the world is desktop.
> 
> > The normal end user of the embedded system does not look at those
> > values either.
> 
> There's rather a lot of people who do embedded development and do need
> to look at this stuff, though.  There's a real cost to raising the bar
> for humans to understand things.

Hm, I'm afraid that this is getting apart from the real worth
discussion.  I'm interested in whether "Line-Out" is acceptable
enough, or it's a matter of taste, just like Marmite is acceptable as
the normal English breakfasts.


> > >>I think both naming schemes are good, and I'm not too worried about them
> > >>being too verbose. If we run into names being longer than the string
> > >>length we could back off and drop the location, I guess.
> 
> > >It's a complete pain for actually working with them and doing
> > >development - it renders badly in UIs (think about alsamixer for
> > >example, or people looking at things on 80 column terminals) and isn't
> > >friendly to people typing things in.
> 
> > So your suggestion was, to avoid "Front Headphone" and "Rear
> > Headphone" because the names were too long, and instead have
> > "Headphone" and "Headphone,index=1" and have "Front" and "Rear" read
> > out of a TLV?
> 
> No, some of the information should go in the name but we keep on coming
> up with more and more things to add and when we end up with "Left Silver
> Headset Jack Headphone" or whatever (which would be a fairly complete
> description of half the headset jack on one of my laptops) we probably
> ought to be cutting some of that out.  

Actually, in my very first version, there was no location prefix or
channel suffix but it was only a simple name like "Headphone" with
index numbers.  Then, I faced a problem when trying to integration
with the input-jack layer.  The input-jack doesn't provide the index
number.  So, you can't register the same name correctly.  Thus I tried
to create a unique name string as input-jack id.

Now, looking back to your implementation, the kctl jack is created
with a name string based on the switch type, not from the name string
passed to snd_jack_new() itself.  Why not passing the string as is?
If multiple kctls are needed (e.g. for headset), you can either create
a new name or just create the multiple instances with the same name
but give extra TLVs to identify which type it is.  Or, you can create
an enum control so that both the input-jack and the kctl-jack
correspond as 1:1.

In other words, if 1:1 mapping isn't needed, we don't have to create a
different name string at all.  Just create "Jack Detect" controls with
different indices and with corresponding TLVs.  This would be enough
(could be even easier) for a machine parser.

And, it seems that there is no standard rule defined for the id string
of input-jack.  If 1:1 model is used, we'd need to define this rule.

> Though thinking about it duplicating all the individual bits of
> information in TLVs would be helpful to automatic parsing, we could
> possibly even do something clever like make minimal subset names that
> contained only the bits that were unique on a given system if we were
> bored enough.

BTW, I found another issue.  With this implementation, you can't build
kjack ctl without input-jack.  I can work around it for HD-audio by
adding more ifdef, but it'd be ugly...


> Oh, by the way the extcon (Android switch class more or less) code looks
> to be coming along quite well which is going to be a third way of
> getting jack information out to userspace.

Yeah, we enjoy freedom :)


thanks,

Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 4389 bytes --]

On Mon, Feb 13, 2012 at 02:56:55PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > There's rather a lot of people who do embedded development and do need
> > to look at this stuff, though.  There's a real cost to raising the bar
> > for humans to understand things.

> Hm, I'm afraid that this is getting apart from the real worth
> discussion.  I'm interested in whether "Line-Out" is acceptable
> enough, or it's a matter of taste, just like Marmite is acceptable as
> the normal English breakfasts.

Well, I dunno - you did suggest adding direction modifiers to the jack
names which would both be a more general solution to the problem and
clean up the naming issue.

> with the input-jack layer.  The input-jack doesn't provide the index
> number.  So, you can't register the same name correctly.  Thus I tried
> to create a unique name string as input-jack id.

It's really never been the intention to have an "input-jack" layer -
the API is there to hide all the input layer stuff it can from the
individual audio drivers.  The only thing that leaks out is the button
remapping which I couldn't think of anything more sensible to do with
them.

> Now, looking back to your implementation, the kctl jack is created
> with a name string based on the switch type, not from the name string
> passed to snd_jack_new() itself.  Why not passing the string as is?

We can't pass it as-is since we'll need multiple controls per jack -
literally all the systems I have available to test on would fail if they
used the same name as they all have headset jacks.  As I said further up
the thread my plan had been to refactor the name generation code once
the separate kctl code is gone and they can be merged into the same
source file which makes this a lot easier.

With these patches I'm mainly looking for a quick and simple refactoring
to get rid of the duplication - once we've got rid of the duplicate
driver APIs then we can refactor the internals of the implementation
however we like but there's a pretty pressing need to fix the driver
API and get all the drivers doing the same thing to userspace even if
it's not ideal.

> If multiple kctls are needed (e.g. for headset), you can either create
> a new name or just create the multiple instances with the same name
> but give extra TLVs to identify which type it is.  Or, you can create
> an enum control so that both the input-jack and the kctl-jack
> correspond as 1:1.

This stuff is why I've been asking you about the ABI and general
semantics for the new controls, I was never sure what the intention was
- when you said they were just plain booleans I just went ahead and
created multiple controls as that seemed to be what the current idiom
was.

I don't think an enumeration is going to scale so well given the number
of possible combinations you can get on more complex jacks.

> In other words, if 1:1 mapping isn't needed, we don't have to create a
> different name string at all.  Just create "Jack Detect" controls with
> different indices and with corresponding TLVs.  This would be enough
> (could be even easier) for a machine parser.

That'd work I think, though I would create a new control for each
physical jack at least just for clarity.

> And, it seems that there is no standard rule defined for the id string
> of input-jack.  If 1:1 model is used, we'd need to define this rule.

The input stack doesn't have the same concept of machine parsable names
for things that ALSA does (honestly that's pretty unique to ALSA within
the kernel) - the names it users are all free form text intended to be
meaningful to humans only.

> BTW, I found another issue.  With this implementation, you can't build
> kjack ctl without input-jack.  I can work around it for HD-audio by
> adding more ifdef, but it'd be ugly...

I really don't see this as a problem; it seems vanishingly unlikely that
any realistic system would want to disable the input API and need
accessory detection.  Obviously the end goal here is that you wouldn't
be able to have HDA specific hacks in the code as everything will be
handled by the ALSA core code...

To my mind we should just support everything and let userspace worry
about deciding what to use, this isn't the sort of thing which should
require a kernel rebuild to configure and it's not like there's any
fundamental problem or massive overhead caused by having the devices
show up.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Mon, 13 Feb 2012 07:44:59 -0800,
Mark Brown wrote:
> 
> On Mon, Feb 13, 2012 at 02:56:55PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > There's rather a lot of people who do embedded development and do need
> > > to look at this stuff, though.  There's a real cost to raising the bar
> > > for humans to understand things.
> 
> > Hm, I'm afraid that this is getting apart from the real worth
> > discussion.  I'm interested in whether "Line-Out" is acceptable
> > enough, or it's a matter of taste, just like Marmite is acceptable as
> > the normal English breakfasts.
> 
> Well, I dunno - you did suggest adding direction modifiers to the jack
> names which would both be a more general solution to the problem and
> clean up the naming issue.

OK, let's postpone about it.

> > with the input-jack layer.  The input-jack doesn't provide the index
> > number.  So, you can't register the same name correctly.  Thus I tried
> > to create a unique name string as input-jack id.
> 
> It's really never been the intention to have an "input-jack" layer -

I called it just to distinguish with kctl jack, not intended to stress
that it's an input layer.

> the API is there to hide all the input layer stuff it can from the
> individual audio drivers.  The only thing that leaks out is the button
> remapping which I couldn't think of anything more sensible to do with
> them.
> 
> > Now, looking back to your implementation, the kctl jack is created
> > with a name string based on the switch type, not from the name string
> > passed to snd_jack_new() itself.  Why not passing the string as is?
> 
> We can't pass it as-is since we'll need multiple controls per jack -
> literally all the systems I have available to test on would fail if they
> used the same name as they all have headset jacks.  As I said further up
> the thread my plan had been to refactor the name generation code once
> the separate kctl code is gone and they can be merged into the same
> source file which makes this a lot easier.

Makes sense.

> With these patches I'm mainly looking for a quick and simple refactoring
> to get rid of the duplication - once we've got rid of the duplicate
> driver APIs then we can refactor the internals of the implementation
> however we like but there's a pretty pressing need to fix the driver
> API and get all the drivers doing the same thing to userspace even if
> it's not ideal.

Hm, then I'd say let's hold on.  Merging this half-baked stuff is
rather confusing at this point since the use of kctl jack isn't
targeted yet for ASoC.

I believe what we need _now_ is to decide the policy (i.e. element
naming rule and TLV definitions) for kctl jacks quickly.

> > If multiple kctls are needed (e.g. for headset), you can either create
> > a new name or just create the multiple instances with the same name
> > but give extra TLVs to identify which type it is.  Or, you can create
> > an enum control so that both the input-jack and the kctl-jack
> > correspond as 1:1.
> 
> This stuff is why I've been asking you about the ABI and general
> semantics for the new controls, I was never sure what the intention was
> - when you said they were just plain booleans I just went ahead and
> created multiple controls as that seemed to be what the current idiom
> was.
> 
> I don't think an enumeration is going to scale so well given the number
> of possible combinations you can get on more complex jacks.

OK.

> > In other words, if 1:1 mapping isn't needed, we don't have to create a
> > different name string at all.  Just create "Jack Detect" controls with
> > different indices and with corresponding TLVs.  This would be enough
> > (could be even easier) for a machine parser.
> 
> That'd work I think, though I would create a new control for each
> physical jack at least just for clarity.

They are indeed individual ctl elements.  They just share the same
name and are distinguished by the index numbers.  For the parser
program, it would be even easier because it needs to look only for
elements containing the single string.  The rest is the parse of TLVs
of gathered elements, which would be needed no matter whether we have
multiple name strings or not.

> > And, it seems that there is no standard rule defined for the id string
> > of input-jack.  If 1:1 model is used, we'd need to define this rule.
> 
> The input stack doesn't have the same concept of machine parsable names
> for things that ALSA does (honestly that's pretty unique to ALSA within
> the kernel) - the names it users are all free form text intended to be
> meaningful to humans only.

Yeah, maybe.  But, if you have two headset jacks, how would you
identify with the current jack stuff...?

> > BTW, I found another issue.  With this implementation, you can't build
> > kjack ctl without input-jack.  I can work around it for HD-audio by
> > adding more ifdef, but it'd be ugly...
> 
> I really don't see this as a problem; it seems vanishingly unlikely that
> any realistic system would want to disable the input API and need
> accessory detection.  Obviously the end goal here is that you wouldn't
> be able to have HDA specific hacks in the code as everything will be
> handled by the ALSA core code...

Yeah, the more code may be moved toward the core once when we decide
the standards.

> To my mind we should just support everything and let userspace worry
> about deciding what to use, this isn't the sort of thing which should
> require a kernel rebuild to configure and it's not like there's any
> fundamental problem or massive overhead caused by having the devices
> show up.

That's one option, yes.  OTOH, people always want to diet the kernel
code.  It's a good dream that is hardly to achieve in reality (for
human beings, too), though.


So, at this point, I can list the following policies for kctl jacks:

A. all kctls have one name string with different indices.  Each jack
   is distinguished by the TLV provided to each kctl.
  
B. kctls are built with only a base name ("Headphone", "Mic") with
   different indices and TLVs.

C. kctls contain unique names ([location] base [direction] [channel])
   optionally with TLVs

It'd be clear when thinking some real cases.  Assume a headset jack
containing a headphone and a mic switch:
A: two kctls with TLV JACK_HEADSET but with TYPE_HEADPHONE and
   TYPE_MIC, respectively.
B1: two kctls with the same "Headset" name, with TLV TYPE_HEADPHONE
    and TYPE_MIC
B2: separate to a kctl with "Headphone" and a kctl with "Mic"
C1: same as B1
C2: same as B2

When an extra headphone jack is added to the above,
A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE
B1: A kctl "Headphone" with TLV TYPE_HEADPHONE
B2: A kctl "Headphone" with index 1
C1: same as B1
C2: A kctl "Rear Headphone" (and rename other "Front Headphone")

Yet more another headphone jack on left,
A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE, LOC_LEFT
B1: A kctl "Headphone" index 1 with TLV TYPE_HEADPHONE, LOC_LEFT
B2: A kctl "Headphone" index 2 with LOC_LEFT
C1: A kctl "Left Headphone" (rename other "Rear Headphone")
C2: A kctl "Left Headphone"

... or does it make things more complicated? :)


thanks,

Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 3157 bytes --]

On Mon, Feb 13, 2012 at 06:40:23PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > With these patches I'm mainly looking for a quick and simple refactoring
> > to get rid of the duplication - once we've got rid of the duplicate
> > driver APIs then we can refactor the internals of the implementation
> > however we like but there's a pretty pressing need to fix the driver
> > API and get all the drivers doing the same thing to userspace even if
> > it's not ideal.

> Hm, then I'd say let's hold on.  Merging this half-baked stuff is
> rather confusing at this point since the use of kctl jack isn't
> targeted yet for ASoC.

> I believe what we need _now_ is to decide the policy (i.e. element
> naming rule and TLV definitions) for kctl jacks quickly.

Now that the kctl jacks are there I'm getting people asking me about it
often enough so I'd like to see it merged.

> > > In other words, if 1:1 mapping isn't needed, we don't have to create a
> > > different name string at all.  Just create "Jack Detect" controls with
> > > different indices and with corresponding TLVs.  This would be enough
> > > (could be even easier) for a machine parser.

> > That'd work I think, though I would create a new control for each
> > physical jack at least just for clarity.

> They are indeed individual ctl elements.  They just share the same
> name and are distinguished by the index numbers.  For the parser
> program, it would be even easier because it needs to look only for
> elements containing the single string.  The rest is the parse of TLVs
> of gathered elements, which would be needed no matter whether we have
> multiple name strings or not.

I think we're mostly agreeing here - what I'd like is to be able to give
each jack a descriptive name which shows up in the control name
("Headset", "Docking Station") along with "Jack" or some other keyword.
The descriptive name could be optional, but it seems nice to put it in
the control name.

> > > And, it seems that there is no standard rule defined for the id string
> > > of input-jack.  If 1:1 model is used, we'd need to define this rule.

> > The input stack doesn't have the same concept of machine parsable names
> > for things that ALSA does (honestly that's pretty unique to ALSA within
> > the kernel) - the names it users are all free form text intended to be
> > meaningful to humans only.

> Yeah, maybe.  But, if you have two headset jacks, how would you
> identify with the current jack stuff...?

You've just got the descriptive name to go on, there's no way to
directly associate it back.

> So, at this point, I can list the following policies for kctl jacks:

> A. all kctls have one name string with different indices.  Each jack
>    is distinguished by the TLV provided to each kctl.

> B. kctls are built with only a base name ("Headphone", "Mic") with
>    different indices and TLVs.

> C. kctls contain unique names ([location] base [direction] [channel])
>    optionally with TLVs

I think I prefer B here with the base name being a descriptive name
(which could often be like the above) that should be unique within the
card, even if the driver just adds numbers.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/13/2012 06:40 PM, Takashi Iwai wrote:
> At Mon, 13 Feb 2012 07:44:59 -0800,
> Mark Brown wrote:
>>
>> On Mon, Feb 13, 2012 at 02:56:55PM +0100, Takashi Iwai wrote:
>>> Mark Brown wrote:
>>
>>>> There's rather a lot of people who do embedded development and do need
>>>> to look at this stuff, though.  There's a real cost to raising the bar
>>>> for humans to understand things.
>>
>>> Hm, I'm afraid that this is getting apart from the real worth
>>> discussion.  I'm interested in whether "Line-Out" is acceptable
>>> enough, or it's a matter of taste, just like Marmite is acceptable as
>>> the normal English breakfasts.
>>
>> Well, I dunno - you did suggest adding direction modifiers to the jack
>> names which would both be a more general solution to the problem and
>> clean up the naming issue.
>
> OK, let's postpone about it.

I've seen enough "Line-Out"s in my life that I'm pretty sure everyone 
will understand it, even though "Line Out" is more common.

>>> with the input-jack layer.  The input-jack doesn't provide the index
>>> number.  So, you can't register the same name correctly.  Thus I tried
>>> to create a unique name string as input-jack id.
>>
>> It's really never been the intention to have an "input-jack" layer -
>
> I called it just to distinguish with kctl jack, not intended to stress
> that it's an input layer.
>
>> the API is there to hide all the input layer stuff it can from the
>> individual audio drivers.  The only thing that leaks out is the button
>> remapping which I couldn't think of anything more sensible to do with
>> them.
>>
>>> Now, looking back to your implementation, the kctl jack is created
>>> with a name string based on the switch type, not from the name string
>>> passed to snd_jack_new() itself.  Why not passing the string as is?
>>
>> We can't pass it as-is since we'll need multiple controls per jack -
>> literally all the systems I have available to test on would fail if they
>> used the same name as they all have headset jacks.  As I said further up
>> the thread my plan had been to refactor the name generation code once
>> the separate kctl code is gone and they can be merged into the same
>> source file which makes this a lot easier.
>
> Makes sense.
>
>> With these patches I'm mainly looking for a quick and simple refactoring
>> to get rid of the duplication - once we've got rid of the duplicate
>> driver APIs then we can refactor the internals of the implementation
>> however we like but there's a pretty pressing need to fix the driver
>> API and get all the drivers doing the same thing to userspace even if
>> it's not ideal.
>
> Hm, then I'd say let's hold on.  Merging this half-baked stuff is
> rather confusing at this point since the use of kctl jack isn't
> targeted yet for ASoC.
>
> I believe what we need _now_ is to decide the policy (i.e. element
> naming rule and TLV definitions) for kctl jacks quickly.
>
>>> If multiple kctls are needed (e.g. for headset), you can either create
>>> a new name or just create the multiple instances with the same name
>>> but give extra TLVs to identify which type it is.  Or, you can create
>>> an enum control so that both the input-jack and the kctl-jack
>>> correspond as 1:1.
>>
>> This stuff is why I've been asking you about the ABI and general
>> semantics for the new controls, I was never sure what the intention was
>> - when you said they were just plain booleans I just went ahead and
>> created multiple controls as that seemed to be what the current idiom
>> was.
>>
>> I don't think an enumeration is going to scale so well given the number
>> of possible combinations you can get on more complex jacks.
>
> OK.
>
>>> In other words, if 1:1 mapping isn't needed, we don't have to create a
>>> different name string at all.  Just create "Jack Detect" controls with
>>> different indices and with corresponding TLVs.  This would be enough
>>> (could be even easier) for a machine parser.
>>
>> That'd work I think, though I would create a new control for each
>> physical jack at least just for clarity.
>
> They are indeed individual ctl elements.  They just share the same
> name and are distinguished by the index numbers.  For the parser
> program, it would be even easier because it needs to look only for
> elements containing the single string.  The rest is the parse of TLVs
> of gathered elements, which would be needed no matter whether we have
> multiple name strings or not.
>
>>> And, it seems that there is no standard rule defined for the id string
>>> of input-jack.  If 1:1 model is used, we'd need to define this rule.
>>
>> The input stack doesn't have the same concept of machine parsable names
>> for things that ALSA does (honestly that's pretty unique to ALSA within
>> the kernel) - the names it users are all free form text intended to be
>> meaningful to humans only.
>
> Yeah, maybe.  But, if you have two headset jacks, how would you
> identify with the current jack stuff...?
>
>>> BTW, I found another issue.  With this implementation, you can't build
>>> kjack ctl without input-jack.  I can work around it for HD-audio by
>>> adding more ifdef, but it'd be ugly...
>>
>> I really don't see this as a problem; it seems vanishingly unlikely that
>> any realistic system would want to disable the input API and need
>> accessory detection.  Obviously the end goal here is that you wouldn't
>> be able to have HDA specific hacks in the code as everything will be
>> handled by the ALSA core code...
>
> Yeah, the more code may be moved toward the core once when we decide
> the standards.
>
>> To my mind we should just support everything and let userspace worry
>> about deciding what to use, this isn't the sort of thing which should
>> require a kernel rebuild to configure and it's not like there's any
>> fundamental problem or massive overhead caused by having the devices
>> show up.
>
> That's one option, yes.  OTOH, people always want to diet the kernel
> code.  It's a good dream that is hardly to achieve in reality (for
> human beings, too), though.
>
>
> So, at this point, I can list the following policies for kctl jacks:
>
> A. all kctls have one name string with different indices.  Each jack
>     is distinguished by the TLV provided to each kctl.
>
> B. kctls are built with only a base name ("Headphone", "Mic") with
>     different indices and TLVs.
>
> C. kctls contain unique names ([location] base [direction] [channel])
>     optionally with TLVs

I would vote for "C2", although I would probably prefer "[location] base 
[channel]" over "[location] base [direction] [channel]", as direction is 
superfluous given "base". This is also what the current implementation 
offers, and what I've based my PulseAudio patches on.

A and B requires additional work as we would need an API in alsa-lib 
from where you can read and interpret the TLVs, and stuff like amixer 
needs to understand it too make it reasonable easy to work with and 
test. And C gives the option to add TLVs later whenever we feel the need 
for it, so it seems to be an all-win.

Also; as this graph exposing thing is unlikely to be implemented in all 
layers of the audio stack any time soon, maybe C2 is also the one that 
gives the most obvious matching between mixer kcontrols and jack 
kcontrols? I'd like to move in this direction; not only because we 
currently do not have the graph, but also because that even if we have 
it, userspace apps choosing not to implement it will have a good option.

> It'd be clear when thinking some real cases.  Assume a headset jack
> containing a headphone and a mic switch:
> A: two kctls with TLV JACK_HEADSET but with TYPE_HEADPHONE and
>     TYPE_MIC, respectively.
> B1: two kctls with the same "Headset" name, with TLV TYPE_HEADPHONE
>      and TYPE_MIC
> B2: separate to a kctl with "Headphone" and a kctl with "Mic"
> C1: same as B1
> C2: same as B2
>
> When an extra headphone jack is added to the above,
> A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE
> B1: A kctl "Headphone" with TLV TYPE_HEADPHONE
> B2: A kctl "Headphone" with index 1
> C1: same as B1
> C2: A kctl "Rear Headphone" (and rename other "Front Headphone")
>
> Yet more another headphone jack on left,
> A: another kctl with TLV_JACK_HEADPHONE, TYPE_HEADPHONE, LOC_LEFT
> B1: A kctl "Headphone" index 1 with TLV TYPE_HEADPHONE, LOC_LEFT
> B2: A kctl "Headphone" index 2 with LOC_LEFT
> C1: A kctl "Left Headphone" (rename other "Rear Headphone")
> C2: A kctl "Left Headphone"
>
> ... or does it make things more complicated? :)
>
>
> thanks,
>
> Takashi
>



-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1552 bytes --]

On Tue, Feb 14, 2012 at 08:20:50AM +0100, David Henningsson wrote:

Please delete irrelevant context from mails, it makes it much easier
to find what you've added,

> On 02/13/2012 06:40 PM, Takashi Iwai wrote:
> >Mark Brown wrote:

> >C. kctls contain unique names ([location] base [direction] [channel])
> >    optionally with TLVs

> I would vote for "C2", although I would probably prefer "[location]
> base [channel]" over "[location] base [direction] [channel]", as
> direction is superfluous given "base". This is also what the current
> implementation offers, and what I've based my PulseAudio patches on.

If we're going to do stuff like this Takashi's suggestion of splitting
direction from the channel seems like a really good idea, it both makes
things read naturallly and means applications are more likely to be able
to cope usefully with base types they've never heard of before.

> Also; as this graph exposing thing is unlikely to be implemented in
> all layers of the audio stack any time soon, maybe C2 is also the
> one that gives the most obvious matching between mixer kcontrols and
> jack kcontrols? I'd like to move in this direction; not only because
> we currently do not have the graph, but also because that even if we
> have it, userspace apps choosing not to implement it will have a
> good option.

The naming stuff gets really painful for anything that isn't a basic PC
audio card - the whole model used for the standard ALSA controls is very
much fixed to an extremely basic model of what the hardware might look
like.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Mon, 13 Feb 2012 11:23:10 -0800,
Mark Brown wrote:
> 
> On Mon, Feb 13, 2012 at 06:40:23PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > With these patches I'm mainly looking for a quick and simple refactoring
> > > to get rid of the duplication - once we've got rid of the duplicate
> > > driver APIs then we can refactor the internals of the implementation
> > > however we like but there's a pretty pressing need to fix the driver
> > > API and get all the drivers doing the same thing to userspace even if
> > > it's not ideal.
> 
> > Hm, then I'd say let's hold on.  Merging this half-baked stuff is
> > rather confusing at this point since the use of kctl jack isn't
> > targeted yet for ASoC.
> 
> > I believe what we need _now_ is to decide the policy (i.e. element
> > naming rule and TLV definitions) for kctl jacks quickly.
> 
> Now that the kctl jacks are there I'm getting people asking me about it
> often enough so I'd like to see it merged.

Yeah, if things were easy, I'd be happy to merge.
But, judging from the situation, I see no big reason to hurry too
much.

> > > > In other words, if 1:1 mapping isn't needed, we don't have to create a
> > > > different name string at all.  Just create "Jack Detect" controls with
> > > > different indices and with corresponding TLVs.  This would be enough
> > > > (could be even easier) for a machine parser.
> 
> > > That'd work I think, though I would create a new control for each
> > > physical jack at least just for clarity.
> 
> > They are indeed individual ctl elements.  They just share the same
> > name and are distinguished by the index numbers.  For the parser
> > program, it would be even easier because it needs to look only for
> > elements containing the single string.  The rest is the parse of TLVs
> > of gathered elements, which would be needed no matter whether we have
> > multiple name strings or not.
> 
> I think we're mostly agreeing here - what I'd like is to be able to give
> each jack a descriptive name which shows up in the control name
> ("Headset", "Docking Station") along with "Jack" or some other keyword.
> The descriptive name could be optional, but it seems nice to put it in
> the control name.

Yes.  My concern is then how to map the existing jack API to the new
name.  Thus I proposed some examples.

> > So, at this point, I can list the following policies for kctl jacks:
> 
> > A. all kctls have one name string with different indices.  Each jack
> >    is distinguished by the TLV provided to each kctl.
> 
> > B. kctls are built with only a base name ("Headphone", "Mic") with
> >    different indices and TLVs.
> 
> > C. kctls contain unique names ([location] base [direction] [channel])
> >    optionally with TLVs
> 
> I think I prefer B here with the base name being a descriptive name
> (which could often be like the above) that should be unique within the
> card, even if the driver just adds numbers.

... and David wrote:

At Tue, 14 Feb 2012 08:20:50 +0100,
David Henningsson wrote:
> I would vote for "C2", although I would probably prefer "[location] base 
> [channel]" over "[location] base [direction] [channel]", as direction is 
> superfluous given "base". This is also what the current implementation 
> offers, and what I've based my PulseAudio patches on.

Actually, C > B, thus we may take an approach of C optionally.
In both cases, we'd need an id string or such (e.g. "Jack" suffix) to
indicate that it's a jack kctl.  If this is satisfied, the rest prefix
and suffix can be fully optional.

That is, for those who don't need the additional information (yet),
just pass the base name.  For multiple items, use the multiple
indices.  This is equivalent with B.

For those who care about the multiple items (e.g. HD-audio), they can
pass the more verbose name.  It's C.

The rest is the job of the user-space side parser.  It'll gather all
items with "Jack" suffix, then trim location, direction or channel
prefix/suffix.  It shouldn't be too hard.

Yes, it's redundant.  One can represent it in both ways via TLV or
name string.  But it works even now (before the TLV definitions),
that's the point.


> A and B requires additional work as we would need an API in alsa-lib 
> from where you can read and interpret the TLVs, and stuff like amixer 
> needs to understand it too make it reasonable easy to work with and 
> test. And C gives the option to add TLVs later whenever we feel the need 
> for it, so it seems to be an all-win.

Honestly speaking, I think the plan A isn't that bad.  The suffix
look-up and filtering as mentioned above looks slightly messy.
It'd be much simpler in both kernel and user-space implementations.
The biggest drawback is that David would need to rewrite his patch to
match with this scheme :)  And, of course, it's not visible with the
current alsa-utils stuff.  The verbose name string would be visible by
alsactl or whatever even in the current version, yes.


> Also; as this graph exposing thing is unlikely to be implemented in all 
> layers of the audio stack any time soon, maybe C2 is also the one that 
> gives the most obvious matching between mixer kcontrols and jack 
> kcontrols? I'd like to move in this direction; not only because we 
> currently do not have the graph, but also because that even if we have 
> it, userspace apps choosing not to implement it will have a good option.

IMO, this graph things should be considered separately at this
moment.  The name string itself won't be always enough to identify the
connected kctls.  If any, we can add a new TLV to represent the
associated kctls, as the driver already knows which widget NID is
assigned to which control.  This would be simpler and more robust, I
guess.


thanks,

Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1151 bytes --]

On Wed, Feb 22, 2012 at 05:52:17PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > Now that the kctl jacks are there I'm getting people asking me about it
> > often enough so I'd like to see it merged.

> Yeah, if things were easy, I'd be happy to merge.
> But, judging from the situation, I see no big reason to hurry too
> much.

I'm not sure what problems you see here - all the issues that are being
discussed here are about the kctl interface to applications, there's no
issues I can see with the in-kernel interfaces.

> Yes.  My concern is then how to map the existing jack API to the new
> name.  Thus I proposed some examples.

But this is an issue solely concerned with the userspace API and is also
going to result in something different to what HDA is doing currently.
I don't see that we need to wait for everyone to make up your mind
what's going on there in order to deal with the custom userspace
interface that HDA has.

Once we've got everything unified then we can refactor in place as we
please and whatever is done will cover all drivers, including any
updates to the core interface required to advertise the new information.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Wed, 22 Feb 2012 17:18:44 +0000,
Mark Brown wrote:
> 
> On Wed, Feb 22, 2012 at 05:52:17PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > Now that the kctl jacks are there I'm getting people asking me about it
> > > often enough so I'd like to see it merged.
> 
> > Yeah, if things were easy, I'd be happy to merge.
> > But, judging from the situation, I see no big reason to hurry too
> > much.
> 
> I'm not sure what problems you see here - all the issues that are being
> discussed here are about the kctl interface to applications, there's no
> issues I can see with the in-kernel interfaces.

First of all, we don't agree yet which naming rule to be applied.
Since your patch assumes the case B, i.e. constant names corresponding
only to the key type (HEADPHONE, etc), it's incompatible with the
current implementation in HD-audio.

That is, once when the patch is merged, the kctl expression will be
forcibly to case B but without TLV yet, because kctls will be created
automatically when the jack instance is created.

In other words, if I merge your patch now, the only solution for
HD-audio side for the time being is to disable
CONFIG_SND_HDA_INPUT_JACK.  That's why I hesitate to merge it now.
And, it's why I prefer defining the naming rule at first, thus
refining the implementation not to conflict with the existing one.


Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1454 bytes --]

On Wed, Feb 22, 2012 at 06:34:49PM +0100, Takashi Iwai wrote:

> First of all, we don't agree yet which naming rule to be applied.
> Since your patch assumes the case B, i.e. constant names corresponding
> only to the key type (HEADPHONE, etc), it's incompatible with the
> current implementation in HD-audio.

As I've *repeatedly* said the idea was to prefix the name once we'd got
stuff merged into the same file (which means ripping out the HDA
specific code) but if we're going to have to completely rework the ABI
before you'll consider trying to fix things up in kernel then there's no
point bothering.

> In other words, if I merge your patch now, the only solution for
> HD-audio side for the time being is to disable
> CONFIG_SND_HDA_INPUT_JACK.  That's why I hesitate to merge it now.

Well, that whole config option just shouldn't be there in the first
place but that's another story...

> And, it's why I prefer defining the naming rule at first, thus
> refining the implementation not to conflict with the existing one.

It seems like it's a bit late to worry about the naming scheme to that
extent given that the existing ABI has already managed to get into one
kernel release and looks like it's also going to get into 3.4 as well.

Right now the only tractable fix I can see is to implement something
approximating the HDA ABI in ASoC but that just makes things even worse
from a maintability point of view.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Wed, 22 Feb 2012 18:54:38 +0000,
Mark Brown wrote:
> 
> On Wed, Feb 22, 2012 at 06:34:49PM +0100, Takashi Iwai wrote:
> 
> > First of all, we don't agree yet which naming rule to be applied.
> > Since your patch assumes the case B, i.e. constant names corresponding
> > only to the key type (HEADPHONE, etc), it's incompatible with the
> > current implementation in HD-audio.
> 
> As I've *repeatedly* said the idea was to prefix the name once we'd got
> stuff merged into the same file (which means ripping out the HDA
> specific code)

Even if so, it can't be merged until this prefix stuff comes in.
As mentioned, otherwise it conflicts.

Alternatively, deprecate CONFIG_SND_HDA_INPUT_JACK.  This is another
option.

> but if we're going to have to completely rework the ABI
> before you'll consider trying to fix things up in kernel then there's no
> point bothering.

Right.  If we change.

> > In other words, if I merge your patch now, the only solution for
> > HD-audio side for the time being is to disable
> > CONFIG_SND_HDA_INPUT_JACK.  That's why I hesitate to merge it now.
> 
> Well, that whole config option just shouldn't be there in the first
> place but that's another story...

Yes, but it's really too late to grumble.  This was merged years ago.

Meanwhile, we can mark it deprecated now as we provide an alternative
solution in 3.3 frame.  Then drop it in 3.4 or 3.5 where your patch
comes in.  THis sounds feasible.

> > And, it's why I prefer defining the naming rule at first, thus
> > refining the implementation not to conflict with the existing one.
> 
> It seems like it's a bit late to worry about the naming scheme to that
> extent given that the existing ABI has already managed to get into one
> kernel release and looks like it's also going to get into 3.4 as well.

Which ABI?  The kctl jack hasn't been in released kernels yet.  It was
first merged in 3.3.  So I really want to fix up the naming rule as
soon as possible before 3.3 release.  For example, with or with
hyphen.  It's an easy task to fix in the code once when decided.

> Right now the only tractable fix I can see is to implement something
> approximating the HDA ABI in ASoC but that just makes things even worse
> from a maintability point of view.

Well, as I already wrote, the prefix and suffix are supposed to be
optional.  So, the current HD-audio implementation doesn't block your
interpretation in jack.c by itself.  The only blockages are that the
jack.c implementation doesn't provide enough for HD-audio requirement
(with the first version), and the detailed naming rule like hyphening
isn't decided yet.


Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 3400 bytes --]

On Wed, Feb 22, 2012 at 09:35:08PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > As I've *repeatedly* said the idea was to prefix the name once we'd got
> > stuff merged into the same file (which means ripping out the HDA
> > specific code)

> Even if so, it can't be merged until this prefix stuff comes in.
> As mentioned, otherwise it conflicts.

Well, yeah.  But if there's an insistance that the new ABI is reworked
as well then it's not worthwhile.  It's a pretty simple change to make,
I just didn't want to faff around with the HDA code to discuss the core
interface - IIRC I mostly posted the patch to make it clear that both
APIs just use sets of booleans.

> Alternatively, deprecate CONFIG_SND_HDA_INPUT_JACK.  This is another
> option.

I think we should do that anyway, just unconditionally enable the
feature.

> Meanwhile, we can mark it deprecated now as we provide an alternative
> solution in 3.3 frame.  Then drop it in 3.4 or 3.5 where your patch
> comes in.  THis sounds feasible.

We're well past the point at which we can change 3.3 substantially.

> > It seems like it's a bit late to worry about the naming scheme to that
> > extent given that the existing ABI has already managed to get into one
> > kernel release and looks like it's also going to get into 3.4 as well.

> Which ABI?  The kctl jack hasn't been in released kernels yet.  It was
> first merged in 3.3.  So I really want to fix up the naming rule as
> soon as possible before 3.3 release.  For example, with or with

We're pretty committed to what's in 3.3 though.  If it's just the hyphen
that's a fairly meaningless difference in terms of the stuff I'm talking
about, it's trivial to change.  But if you guys decide to go with things
like adding TLV information or whatever then that's more involved and I
can't see that getting in at this point, we'd need to remove the kctl
jacks from the userspace interface for 3.3 or accept churn.

It did also make it out in the last alsa-driver release IIRC.

> hyphen.  It's an easy task to fix in the code once when decided.

I mostly agree, I'm pushing to merge now because it seems like we can
make the change before or after merging the APIs and I don't want to get
to the merge window without the two interfaces being brought into line
because there's still ongoing discussion of exactly what the userspace
interface for the control jacks should look like.

> > Right now the only tractable fix I can see is to implement something
> > approximating the HDA ABI in ASoC but that just makes things even worse
> > from a maintability point of view.

> Well, as I already wrote, the prefix and suffix are supposed to be
> optional.  So, the current HD-audio implementation doesn't block your
> interpretation in jack.c by itself.  The only blockages are that the
> jack.c implementation doesn't provide enough for HD-audio requirement

So when I submit a patch removing the kctl jack support from HDA in
favour of something along the lines of the current code you'll be OK
with that so long as there's also something there to use the card naming
(I'd expect I'd end up submitting that as a followup patch for the sake
of making the individual patches clearer)?

> (with the first version), and the detailed naming rule like hyphening
> isn't decided yet.

Right, but like I say I don't think this should block fixing the problem
with multiple APIs and ABIs.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Takashi Iwai]

At Wed, 22 Feb 2012 20:55:30 +0000,
Mark Brown wrote:
> 
> On Wed, Feb 22, 2012 at 09:35:08PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > As I've *repeatedly* said the idea was to prefix the name once we'd got
> > > stuff merged into the same file (which means ripping out the HDA
> > > specific code)
> 
> > Even if so, it can't be merged until this prefix stuff comes in.
> > As mentioned, otherwise it conflicts.
> 
> Well, yeah.  But if there's an insistance that the new ABI is reworked
> as well then it's not worthwhile.  It's a pretty simple change to make,
> I just didn't want to faff around with the HDA code to discuss the core
> interface - IIRC I mostly posted the patch to make it clear that both
> APIs just use sets of booleans.

OK.

> > Alternatively, deprecate CONFIG_SND_HDA_INPUT_JACK.  This is another
> > option.
> 
> I think we should do that anyway, just unconditionally enable the
> feature.

OK.

> > Meanwhile, we can mark it deprecated now as we provide an alternative
> > solution in 3.3 frame.  Then drop it in 3.4 or 3.5 where your patch
> > comes in.  THis sounds feasible.
> 
> We're well past the point at which we can change 3.3 substantially.

Yes, but it's not too late to put a DEPRECATED marker in Kconfig right
now, at least.  It's no removal yet, so no problem for 3.3.


> > > It seems like it's a bit late to worry about the naming scheme to that
> > > extent given that the existing ABI has already managed to get into one
> > > kernel release and looks like it's also going to get into 3.4 as well.
> 
> > Which ABI?  The kctl jack hasn't been in released kernels yet.  It was
> > first merged in 3.3.  So I really want to fix up the naming rule as
> > soon as possible before 3.3 release.  For example, with or with
> 
> We're pretty committed to what's in 3.3 though.  If it's just the hyphen
> that's a fairly meaningless difference in terms of the stuff I'm talking
> about, it's trivial to change.  But if you guys decide to go with things
> like adding TLV information or whatever then that's more involved and I
> can't see that getting in at this point, we'd need to remove the kctl
> jacks from the userspace interface for 3.3 or accept churn.

As I showed, the implementation case C, with the verbose name string
and _optional_ TLV, doesn't conflict with both the current HD-audio
code and your future patch.  That is, it can be well "Mic" if it's a
single object.  First when you need to distinguish multiple items or
enumerate with attributes, you can choose either the prefix/suffix in
the name string or the base name with a TLV info: "Rear Mic" is
equivalent with "Mic" with TLV_POS_REAR.

This looks like the most feasible option to me.

> It did also make it out in the last alsa-driver release IIRC.

Bah, the alsa-driver was released by some reason I don't know of.

I really think the release schedule must be defined in a more strict
but easier way.  For example, just release alsa-driver at each time
the kernel is released.  It allows user of the older kernel to run the
driver on the latest kernel (not the latest development version!).

Well, it's another topic, and should be discussed in a different
thread...

> > hyphen.  It's an easy task to fix in the code once when decided.
> 
> I mostly agree, I'm pushing to merge now because it seems like we can
> make the change before or after merging the APIs and I don't want to get
> to the merge window without the two interfaces being brought into line
> because there's still ongoing discussion of exactly what the userspace
> interface for the control jacks should look like.

Right.

> > > Right now the only tractable fix I can see is to implement something
> > > approximating the HDA ABI in ASoC but that just makes things even worse
> > > from a maintability point of view.
> 
> > Well, as I already wrote, the prefix and suffix are supposed to be
> > optional.  So, the current HD-audio implementation doesn't block your
> > interpretation in jack.c by itself.  The only blockages are that the
> > jack.c implementation doesn't provide enough for HD-audio requirement
> 
> So when I submit a patch removing the kctl jack support from HDA in
> favour of something along the lines of the current code you'll be OK
> with that so long as there's also something there to use the card naming
> (I'd expect I'd end up submitting that as a followup patch for the sake
> of making the individual patches clearer)?

If the patch will cover the missing pieces, i.e. filling the location,
the channel and the multiple indices, I'm fine to remove the current
hd-audio kctl jack code, yes.


thanks,

Takashi

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[David Henningsson]

On 02/22/2012 05:52 PM, Takashi Iwai wrote:
> At Tue, 14 Feb 2012 08:20:50 +0100,
> David Henningsson wrote:
>> I would vote for "C2", although I would probably prefer "[location] base
>> [channel]" over "[location] base [direction] [channel]", as direction is
>> superfluous given "base". This is also what the current implementation
>> offers, and what I've based my PulseAudio patches on.
>
> Actually, C>  B, thus we may take an approach of C optionally.
> In both cases, we'd need an id string or such (e.g. "Jack" suffix) to
> indicate that it's a jack kctl.  If this is satisfied, the rest prefix
> and suffix can be fully optional.
>
> That is, for those who don't need the additional information (yet),
> just pass the base name.  For multiple items, use the multiple
> indices.  This is equivalent with B.
>
> For those who care about the multiple items (e.g. HD-audio), they can
> pass the more verbose name.  It's C.
>
> The rest is the job of the user-space side parser.  It'll gather all
> items with "Jack" suffix, then trim location, direction or channel
> prefix/suffix.  It shouldn't be too hard.
>
> Yes, it's redundant.  One can represent it in both ways via TLV or
> name string.  But it works even now (before the TLV definitions),
> that's the point.

I agree with this reasoning.

>> A and B requires additional work as we would need an API in alsa-lib
>> from where you can read and interpret the TLVs, and stuff like amixer
>> needs to understand it too make it reasonable easy to work with and
>> test. And C gives the option to add TLVs later whenever we feel the need
>> for it, so it seems to be an all-win.
>
> Honestly speaking, I think the plan A isn't that bad.  The suffix
> look-up and filtering as mentioned above looks slightly messy.
> It'd be much simpler in both kernel and user-space implementations.
> The biggest drawback is that David would need to rewrite his patch to
> match with this scheme :)  And, of course, it's not visible with the
> current alsa-utils stuff.  The verbose name string would be visible by
> alsactl or whatever even in the current version, yes.

And it requires some specification on how this TLV will look like, my 
head isn't completely clear on that. But maybe you got that all figured out?

But let me stress that the matching is actually what's most important to 
PulseAudio at this point: assume we have one 'Front Mic Jack' and one 
'Rear Mic Jack' - when the rear mic is plugged in, and the user wants to 
change the mic gain, PulseAudio should make sure that 'Input Source' is 
set to 'Rear Mic' and that it changes 'Rear Mic Boost', 'Capture' and 
nothing else. (And, for HDMI, we also need to match against the correct 
PCM device.)

That's why I like C best so far, because I can match on the names. (And 
yes, that I don't have to rewrite my patch a second time is an added 
bonus :-) )

>> Also; as this graph exposing thing is unlikely to be implemented in all
>> layers of the audio stack any time soon, maybe C2 is also the one that
>> gives the most obvious matching between mixer kcontrols and jack
>> kcontrols? I'd like to move in this direction; not only because we
>> currently do not have the graph, but also because that even if we have
>> it, userspace apps choosing not to implement it will have a good option.
>
> IMO, this graph things should be considered separately at this
> moment.  The name string itself won't be always enough to identify the
> connected kctls.  If any, we can add a new TLV to represent the
> associated kctls, as the driver already knows which widget NID is
> assigned to which control.  This would be simpler and more robust, I
> guess.

Hmm, but for that to actually work out reliably, we're going to need
1) all inputs and outputs, not only those with jacks. So we can know if 
there is a 'speaker' or not, and
2) deal with multiple DACs and ADCs somehow. Or are you assuming we 
always use the first DAC/ADC here?
3) a reliable way to tell that we can use this method for mixer probing 
or if we should fall back to the current method.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Raymond Yau]

2012/2/10, Takashi Iwai <tiwai@suse.de>:

>
> I'd like to hear rather comments from others.
>

1) The problem of core jack api which is tailor made for embedded
device, it assume there is only one sound card

when the desktop computer have more than one sound cards which have jack detect
there is no mapping to which sound card has the events.

How to differentiate . noboard hdmi port and graphic card's hdmi ?

2) if the objective of adding jack detection is for trapping
plug/unplug event to trigger the reconfiguration of audio server

http://thread.gmane.org/gmane.linux.alsa.devel/89781/focus=89788

does the audio server require to know the initial state of the hdmi
or just need the trigger event ?

3) There are laptops (e.g. alienware) which have two HP jacks and Mic
jack which need jack retasking for support surround5.1 since the name
of volume controls cannot be rename , the only way is to perform a
hda-reconfig or early patching

can those input device be deleted during the hda-reconfig ?

Re: [PATCH 2/2] ALSA: Integrate control based jack reporting with core jack reporting

[Mark Brown]

On Tue, Feb 14, 2012 at 09:29:27AM +0800, Raymond Yau wrote:

I really shouldn't have ot remind you to not remove people from the CC
list, this is really basic stuff...

> 1) The problem of core jack api which is tailor made for embedded
> device, it assume there is only one sound card

This is not the case at all, there is no such assumption and indeed many
embedded devices have multiple sound cards.

> when the desktop computer have more than one sound cards which have jack detect
> there is no mapping to which sound card has the events.

The jack device is a child of the sound card, userspace can look at the
parent to see which sound card a given jack is associated with.

> 2) if the objective of adding jack detection is for trapping
> plug/unplug event to trigger the reconfiguration of audio server

> http://thread.gmane.org/gmane.linux.alsa.devel/89781/focus=89788

> does the audio server require to know the initial state of the hdmi
> or just need the trigger event ?

For all three of the accessory detection mechanisms the state can be
read at any time.