[alsa-devel] [PATCH] ALSA: hda/jack - Also add jack kctls for Conexant codecs
Signed-off-by: David Henningsson david.henningsson@canonical.com ---
Hi Takashi,
A patch for your jack branch - it seems you forgot to add kctls for conexant? Also it would be nice to see this merged, soon it might be too late for 3.3?
sound/pci/hda/patch_conexant.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index ae9c028..99942e6 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -4325,6 +4325,7 @@ static int cx_auto_build_input_controls(struct hda_codec *codec)
static int cx_auto_build_controls(struct hda_codec *codec) { + struct conexant_spec *spec = codec->spec; int err;
err = cx_auto_build_output_controls(codec); @@ -4333,6 +4334,9 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = cx_auto_build_input_controls(codec); if (err < 0) return err; + err = snd_hda_jack_add_kctls(codec, &spec->autocfg); + if (err < 0) + return err; return conexant_build_controls(codec); }
At Tue, 20 Dec 2011 15:23:05 +0100, David Henningsson wrote:
Signed-off-by: David Henningsson david.henningsson@canonical.com
Hi Takashi,
A patch for your jack branch - it seems you forgot to add kctls for conexant?
Obviously yes. Applied now.
Also it would be nice to see this merged, soon it might be too late for 3.3?
OK, let's merge it. The things are still local to hda-intel, so we can fix pending issues and rewrite with a more generic framework later for unified jack-handling stuff.
thanks,
Takashi
sound/pci/hda/patch_conexant.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index ae9c028..99942e6 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -4325,6 +4325,7 @@ static int cx_auto_build_input_controls(struct hda_codec *codec)
static int cx_auto_build_controls(struct hda_codec *codec) {
struct conexant_spec *spec = codec->spec; int err;
err = cx_auto_build_output_controls(codec);
@@ -4333,6 +4334,9 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = cx_auto_build_input_controls(codec); if (err < 0) return err;
- err = snd_hda_jack_add_kctls(codec, &spec->autocfg);
- if (err < 0)
return err;}
-- 1.7.5.4
At Tue, 20 Dec 2011 15:32:21 +0100, Takashi Iwai wrote:
At Tue, 20 Dec 2011 15:23:05 +0100, David Henningsson wrote:
Signed-off-by: David Henningsson david.henningsson@canonical.com
Hi Takashi,
A patch for your jack branch - it seems you forgot to add kctls for conexant?
Obviously yes. Applied now.
Ah, I forgot that this was fixed in the branch in sound-unstable tree but I didn't merge back to sound tree. Sorry for confusion.
Now all merged and pushed out.
Takashi
Also it would be nice to see this merged, soon it might be too late for 3.3?
OK, let's merge it. The things are still local to hda-intel, so we can fix pending issues and rewrite with a more generic framework later for unified jack-handling stuff.
thanks,
Takashi
sound/pci/hda/patch_conexant.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index ae9c028..99942e6 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -4325,6 +4325,7 @@ static int cx_auto_build_input_controls(struct hda_codec *codec)
static int cx_auto_build_controls(struct hda_codec *codec) {
struct conexant_spec *spec = codec->spec; int err;
err = cx_auto_build_output_controls(codec);
@@ -4333,6 +4334,9 @@ static int cx_auto_build_controls(struct hda_codec *codec) err = cx_auto_build_input_controls(codec); if (err < 0) return err;
- err = snd_hda_jack_add_kctls(codec, &spec->autocfg);
- if (err < 0)
return err;}
-- 1.7.5.4
On Tue, Dec 20, 2011 at 03:32:21PM +0100, Takashi Iwai wrote:
David Henningsson wrote:
Also it would be nice to see this merged, soon it might be too late for 3.3?
OK, let's merge it. The things are still local to hda-intel, so we can fix pending issues and rewrite with a more generic framework later for unified jack-handling stuff.
I was rather expecting that the patches would be posted to the list for review at some point (though I may have missed them as not having generic support is really disappointing). We should at least verify that the userspace interface isn't going to cause problems for non-HDA systems.
At Tue, 20 Dec 2011 23:58:13 +0000, Mark Brown wrote:
On Tue, Dec 20, 2011 at 03:32:21PM +0100, Takashi Iwai wrote:
David Henningsson wrote:
Also it would be nice to see this merged, soon it might be too late for 3.3?
OK, let's merge it. The things are still local to hda-intel, so we can fix pending issues and rewrite with a more generic framework later for unified jack-handling stuff.
I was rather expecting that the patches would be posted to the list for review at some point (though I may have missed them as not having generic support is really disappointing).
Err, sorry, I thought I have posted some in RFC, but wasn't there. The essential patch for the common helper is attached below.
We should at least verify that the userspace interface isn't going to cause problems for non-HDA systems.
Well, the kctl-jack part itself can't break anything right now since it's used only in HD-audio. If others will use, this will be a pure addition, so it won't break except for possibly different ctl numids.
The question toward the integration with the input-jack stuff is still open. In the HD-audio case, input-jack functions are merged into the kctl-jack helpers. But for others, it doesn't have to be so.
One remaining obstacle for this integration is that the input-jack provides multiple key bits per jack entry while this doesn't exist in the kctl notification. With kctl, the notifier says just that the value was changed. Then the user-space can query the current value (not necessarily a boolean but can be an integer or else), but the value isn't passed in the notification itself. So, it's a different design.
thanks,
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: Introduce common helper functions for jack-detection control
Now move the helper function for creating and reporting the jack-detection to the common place. The driver that needs this functionality should select CONFIG_SND_KCTL_JACK kconfig.
Signed-off-by: Takashi Iwai tiwai@suse.de --- diff --git a/include/sound/control.h b/include/sound/control.h index 1a94a21..b2796e8 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -227,4 +227,12 @@ snd_ctl_add_slave_uncached(struct snd_kcontrol *master, return _snd_ctl_add_slave(master, slave, SND_CTL_SLAVE_NEED_UPDATE); }
+/* + * Helper functions for jack-detection controls + */ +struct snd_kcontrol * +snd_kctl_jack_new(const char *name, int idx, void *private_data); +void snd_kctl_jack_report(struct snd_card *card, + struct snd_kcontrol *kctl, bool status); + #endif /* __SOUND_CONTROL_H */ diff --git a/sound/core/Kconfig b/sound/core/Kconfig index 475455c..66f287f 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -207,6 +207,9 @@ config SND_PCM_XRUN_DEBUG config SND_VMASTER bool
+config SND_KCTL_JACK + bool + config SND_DMA_SGBUF def_bool y depends on X86 diff --git a/sound/core/Makefile b/sound/core/Makefile index 350a08d..b4637c3 100644 --- a/sound/core/Makefile +++ b/sound/core/Makefile @@ -7,6 +7,7 @@ snd-y := sound.o init.o memory.o info.o control.o misc.o device.o snd-$(CONFIG_ISA_DMA_API) += isadma.o snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o snd-$(CONFIG_SND_VMASTER) += vmaster.o +snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o snd-$(CONFIG_SND_JACK) += jack.o
snd-pcm-objs := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \ diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c new file mode 100644 index 0000000..af0e78a --- /dev/null +++ b/sound/core/ctljack.c @@ -0,0 +1,55 @@ +/* + * Helper functions for jack-detection kcontrols + * + * Copyright (c) 2011 Takashi Iwai tiwai@suse.de + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + */ + +#include <linux/kernel.h> +#include <sound/core.h> +#include <sound/control.h> + +#define jack_detect_kctl_info snd_ctl_boolean_mono_info + +static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + ucontrol->value.integer.value[0] = kcontrol->private_value; + return 0; +} + +static struct snd_kcontrol_new jack_detect_kctl = { + /* name is filled later */ + .iface = SNDRV_CTL_ELEM_IFACE_CARD, + .access = SNDRV_CTL_ELEM_ACCESS_READ, + .info = jack_detect_kctl_info, + .get = jack_detect_kctl_get, +}; + +struct snd_kcontrol * +snd_kctl_jack_new(const char *name, int idx, void *private_data) +{ + struct snd_kcontrol *kctl; + kctl = snd_ctl_new1(&jack_detect_kctl, private_data); + if (!kctl) + return NULL; + snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); + kctl->id.index = idx; + kctl->private_value = 0; + return kctl; +} +EXPORT_SYMBOL_GPL(snd_kctl_jack_new); + +void snd_kctl_jack_report(struct snd_card *card, + struct snd_kcontrol *kctl, bool status) +{ + if (kctl->private_value == status) + return; + kctl->private_value = status; + snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id); +} +EXPORT_SYMBOL_GPL(snd_kctl_jack_report);
On Wed, Dec 21, 2011 at 03:42:17PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
I was rather expecting that the patches would be posted to the list for review at some point (though I may have missed them as not having generic support is really disappointing).
Err, sorry, I thought I have posted some in RFC, but wasn't there. The essential patch for the common helper is attached below.
Thanks. It'll be tomorrow at the earliest before I can read it properly.
We should at least verify that the userspace interface isn't going to cause problems for non-HDA systems.
Well, the kctl-jack part itself can't break anything right now since it's used only in HD-audio. If others will use, this will be a pure addition, so it won't break except for possibly different ctl numids.
I'm more worried about the userspace interface than the in kernel one, we can rewrite the kernel if it sucks but we're stuck with userspace.
The question toward the integration with the input-jack stuff is still open. In the HD-audio case, input-jack functions are merged into the kctl-jack helpers. But for others, it doesn't have to be so.
I really don't see any reason why you'd want to have to worry about the specific mechanisms in drivers (and there's also the extcon/switch stuff that's looking likely to hit soon). Indeed the extcon stuff might make this needless anyway.
One remaining obstacle for this integration is that the input-jack provides multiple key bits per jack entry while this doesn't exist in the kctl notification. With kctl, the notifier says just that the value was changed. Then the user-space can query the current value (not necessarily a boolean but can be an integer or else), but the value isn't passed in the notification itself. So, it's a different design.
The detail in the notification seems completely trivial to deal with - the input layer also provides similar readback support, it just happens to have a more verbose report (and it needs the cache for readback anyway as it also uses it to eliminate noop reports).
On Wed, Dec 21, 2011 at 03:42:17PM +0100, Takashi Iwai wrote:
Well, the kctl-jack part itself can't break anything right now since it's used only in HD-audio. If others will use, this will be a pure addition, so it won't break except for possibly different ctl numids.
A pure addition can still cause problems as we roll out the same interface into other drivers, for example if there's assumptions that aren't generally true.
+void snd_kctl_jack_report(struct snd_card *card,
struct snd_kcontrol *kctl, bool status)+{
- if (kctl->private_value == status)
return;- kctl->private_value = status;
- snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
+} +EXPORT_SYMBOL_GPL(snd_kctl_jack_report);
So, this looks good in so far as it goes but obviously there's no semantics being defined here for the controls or their names. That's more the bit that worries me as we'll get applications starting to use the new interface and they should have something consistent to work with.
I'm guessing that as this is just a simple boolean each jack will have a series of controls like "Headset Jack Headphone" and "Headset Jack Microphone" or whatever and userspace should match them all together in the same way that it does for Volume and Switch controls? That sounds like it'll work well, we just need to define some strings for standard jacks and conections.
At Thu, 22 Dec 2011 12:21:55 +0000, Mark Brown wrote:
On Wed, Dec 21, 2011 at 03:42:17PM +0100, Takashi Iwai wrote:
Well, the kctl-jack part itself can't break anything right now since it's used only in HD-audio. If others will use, this will be a pure addition, so it won't break except for possibly different ctl numids.
A pure addition can still cause problems as we roll out the same interface into other drivers, for example if there's assumptions that aren't generally true.
In later future, yes. But I was talking about 3.3 kernel.
+void snd_kctl_jack_report(struct snd_card *card,
struct snd_kcontrol *kctl, bool status)+{
- if (kctl->private_value == status)
return;- kctl->private_value = status;
- snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
+} +EXPORT_SYMBOL_GPL(snd_kctl_jack_report);
So, this looks good in so far as it goes but obviously there's no semantics being defined here for the controls or their names. That's more the bit that worries me as we'll get applications starting to use the new interface and they should have something consistent to work with.
Right. We'll need a closer look back with the realistic use-cases.
I'm guessing that as this is just a simple boolean each jack will have a series of controls like "Headset Jack Headphone" and "Headset Jack Microphone" or whatever and userspace should match them all together in the same way that it does for Volume and Switch controls?
My current assumption is so, too. OTOH, if a single jack must provide several values inevitably, it may take an integer or an enum, in theory. But I think (and hope) this wouldn't happen.
That sounds like it'll work well, we just need to define some strings for standard jacks and conections.
Yeah. Unfortunately this standardization isn't always trivial, especially when multiple jacks are present with the same type...
My idea is to give some association between this jack kctl and another kctl, in a simple way like TLV. But it's likely a new year's dream.
thanks,
Takashi
On Thu, Dec 22, 2011 at 02:55:57PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
A pure addition can still cause problems as we roll out the same interface into other drivers, for example if there's assumptions that aren't generally true.
In later future, yes. But I was talking about 3.3 kernel.
Once we've got a userspace interface we're pretty much stuck with it...
I'm guessing that as this is just a simple boolean each jack will have a series of controls like "Headset Jack Headphone" and "Headset Jack Microphone" or whatever and userspace should match them all together in the same way that it does for Volume and Switch controls?
My current assumption is so, too. OTOH, if a single jack must provide several values inevitably, it may take an integer or an enum, in theory. But I think (and hope) this wouldn't happen.
Supporting multiple objects on a single jack is a very basic requirement in order to support headsets (which exist on PCs as well, the MacBooks for example) - we can usually distinguish between at least headset and headphone. I think if we're going to do this via a single multi-state control rather than a series of booleans it'd need to be an enum for usability ("an object of type 2!"), Android uses integers and it's pretty miserable for usability.
That sounds like it'll work well, we just need to define some strings for standard jacks and conections.
Yeah. Unfortunately this standardization isn't always trivial, especially when multiple jacks are present with the same type...
I was thinking more about the things you might detect rather than the jacks, that's a much more tractable problem.
My idea is to give some association between this jack kctl and another kctl, in a simple way like TLV. But it's likely a new year's dream.
Associating the jacks with the audio routes is definitely a separate problem.
At Thu, 22 Dec 2011 14:12:51 +0000, Mark Brown wrote:
On Thu, Dec 22, 2011 at 02:55:57PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
A pure addition can still cause problems as we roll out the same interface into other drivers, for example if there's assumptions that aren't generally true.
In later future, yes. But I was talking about 3.3 kernel.
Once we've got a userspace interface we're pretty much stuck with it...
Yes, but this is no new interface. It uses the existing control API.
I'm guessing that as this is just a simple boolean each jack will have a series of controls like "Headset Jack Headphone" and "Headset Jack Microphone" or whatever and userspace should match them all together in the same way that it does for Volume and Switch controls?
My current assumption is so, too. OTOH, if a single jack must provide several values inevitably, it may take an integer or an enum, in theory. But I think (and hope) this wouldn't happen.
Supporting multiple objects on a single jack is a very basic requirement in order to support headsets (which exist on PCs as well, the MacBooks for example) - we can usually distinguish between at least headset and headphone.
Interesting. This can be a single jack connected to multiple pins, so the codec sees two individual detection points. So, if we want to expose pins, it'd become different from the jack representation. Hmm...
I think if we're going to do this via a single multi-state control rather than a series of booleans it'd need to be an enum for usability ("an object of type 2!"), Android uses integers and it's pretty miserable for usability.
I see, good to know.
Takashi
That sounds like it'll work well, we just need to define some strings for standard jacks and conections.
Yeah. Unfortunately this standardization isn't always trivial, especially when multiple jacks are present with the same type...
I was thinking more about the things you might detect rather than the jacks, that's a much more tractable problem.
My idea is to give some association between this jack kctl and another kctl, in a simple way like TLV. But it's likely a new year's dream.
Associating the jacks with the audio routes is definitely a separate problem.
On Thu, Dec 22, 2011 at 03:19:40PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
Once we've got a userspace interface we're pretty much stuck with it...
Yes, but this is no new interface. It uses the existing control API.
In terms of system calls it's not new but once you start saying "jacks will appear as controls called X with type Y" or whatever that's also an interface built on top of the raw interface which applications can start relying on. I'd call the entire ALSA control naming standard an interface for example.
Supporting multiple objects on a single jack is a very basic requirement in order to support headsets (which exist on PCs as well, the MacBooks for example) - we can usually distinguish between at least headset and headphone.
Interesting. This can be a single jack connected to multiple pins, so the codec sees two individual detection points. So, if we want to expose pins, it'd become different from the jack representation. Hmm...
At the hardware level this is often a single detection point with multiple values - a common technique for distinguishing a headphone is that it's a microphone that's had the button shorted ever since it was plugged in. With all the different techniques for this stuff you end up with a N:M mapping between jacks and detection methods and the same again for things reported.
participants (3)
-
David Henningsson -
Mark Brown -
Takashi Iwai