[RFC] virtual master control for HD-audio

[RFC] virtual master control for HD-audio

[Takashi Iwai]

Hi,

another thing I'd like to push into the next kernel is the virtual
master volume control.  As I posted in earlier posts, it adds virtual
controls that have several slave controls with the same types,
e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
simultaneously via the master control.

It'd be appreciated if some one can test the patches with HD-audio h/w
that has no master control yet (e.g. some STAC codecs).

Note that this won't add the master volumes if there are no real
volume controls.  Some codecs have really no volume control, and this
won't help for such devices.

Two (and one for driver) patches will follow after this.


Takashi

[RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

This patch adds the virtual master control to sound core.


Takashi

diff -r 1207d46e60b3 core/Kconfig
--- a/core/Kconfig	Mon Dec 03 15:15:59 2007 +0100
+++ b/core/Kconfig	Mon Dec 03 15:46:17 2007 +0100
@@ -15,6 +15,9 @@ config SND_RAWMIDI
 config SND_RAWMIDI
 	tristate
 	depends on SND
+
+config SND_VMASTER
+	bool
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff -r 1207d46e60b3 core/Makefile
--- a/core/Makefile	Mon Dec 03 15:15:59 2007 +0100
+++ b/core/Makefile	Mon Dec 03 15:46:17 2007 +0100
@@ -6,6 +6,7 @@ snd-y     := sound.o init.o memory.o inf
 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-pcm-objs := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
 		pcm_memory.o
diff -r 1207d46e60b3 core/vmaster.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/core/vmaster.c	Mon Dec 03 15:46:17 2007 +0100
@@ -0,0 +1,379 @@
+/*
+ * Virtual master and slave controls
+ *
+ *  Copyright (c) by 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.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+#include <sound/driver.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/control.h>
+
+/*
+ * a subset of information returned via ctl info callback
+ */
+struct link_ctl_info {
+	int type;		/* value type */
+	int count;		/* item count */
+	int min_val, max_val;	/* min, max values */
+};
+
+/*
+ * link master - this contains a list of slave controls that are
+ * identical types, i.e. info returns the same value type and value
+ * ranges, but may have different number of counts.
+ *
+ * The master control is so far only mono volume/switch for simplicity.
+ * The same value will be applied to all slaves.
+ */
+struct link_master {
+	struct list_head slaves;
+	struct link_ctl_info info;
+	int val;		/* the master value */
+};
+
+/*
+ * link slave - this contains a slave control element
+ *
+ * It fakes the control callbacsk with additional attenuation by the
+ * master control.  A slave may have either one or two channels.
+ */
+
+struct link_slave {
+	struct list_head list;
+	struct link_master *master;
+	struct link_ctl_info info;
+	int vals[2];		/* current values */
+	struct snd_kcontrol slave; /* the copy of original control entry */
+};
+
+/* get the slave ctl info and save the initial values */
+static int slave_init(struct link_slave *slave)
+{
+	struct snd_ctl_elem_info *uinfo;
+	struct snd_ctl_elem_value *uctl;
+	int err, ch;
+
+	if (slave->info.count)
+		return 0; /* already initialized */
+
+	uinfo = kmalloc(sizeof(*uinfo), GFP_KERNEL);
+	if (!uinfo)
+		return -ENOMEM;
+	uinfo->id = slave->slave.id;
+	err = slave->slave.info(&slave->slave, uinfo);
+	if (err < 0) {
+		kfree(uinfo);
+		return err;
+	}
+	slave->info.type = uinfo->type;
+	slave->info.count = uinfo->count;
+	if (slave->info.count > 2  ||
+	    (slave->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER &&
+	     slave->info.type != SNDRV_CTL_ELEM_TYPE_BOOLEAN)) {
+		snd_printk(KERN_ERR "invalid slave element\n");
+		kfree(uinfo);
+		return -EINVAL;
+	}
+	slave->info.min_val = uinfo->value.integer.min;
+	slave->info.max_val = uinfo->value.integer.max;
+	kfree(uinfo);
+
+	uctl = kmalloc(sizeof(*uctl), GFP_KERNEL);
+	if (!uctl)
+		return -ENOMEM;
+	uctl->id = slave->slave.id;
+	err = slave->slave.get(&slave->slave, uctl);
+	for (ch = 0; ch < slave->info.count; ch++)
+		slave->vals[ch] = uctl->value.integer.value[ch];
+	kfree(uctl);
+	return 0;
+}
+
+/* initialize master volume */
+static int master_init(struct link_master *master)
+{
+	struct link_slave *slave;
+
+	if (master->info.count)
+		return 0; /* already initialized */
+
+	list_for_each_entry(slave, &master->slaves, list) {
+		int err = slave_init(slave);
+		if (err < 0)
+			return err;
+		master->info = slave->info;
+		master->info.count = 1; /* always mono */
+		/* set full volume as default (= no attenuation) */
+		master->val = master->info.max_val;
+		return 0;
+	}
+	return -ENOENT;
+}
+
+static int slave_get_val(struct link_slave *slave,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	int err, ch;
+
+	err = slave_init(slave);
+	if (err < 0)
+		return err;
+	for (ch = 0; ch < slave->info.count; ch++)
+		ucontrol->value.integer.value[ch] = slave->vals[ch];
+	return 0;
+}
+
+static int slave_put_val(struct link_slave *slave,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	int err, ch, vol;
+
+	err = master_init(slave->master);
+	if (err < 0)
+		return err;
+
+	switch (slave->info.type) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+		for (ch = 0; ch < slave->info.count; ch++)
+			ucontrol->value.integer.value[ch] &=
+				!!slave->master->val;
+		break;
+	case SNDRV_CTL_ELEM_TYPE_INTEGER:
+		for (ch = 0; ch < slave->info.count; ch++) {
+			/* max master volume is supposed to be 0 dB */
+			vol = ucontrol->value.integer.value[ch];
+			vol += slave->master->val - slave->master->info.max_val;
+			if (vol < slave->info.min_val)
+				vol = slave->info.min_val;
+			else if (vol > slave->info.max_val)
+				vol = slave->info.max_val;
+			ucontrol->value.integer.value[ch] = vol;
+		}
+		break;
+	}
+	return slave->slave.put(&slave->slave, ucontrol);
+}
+
+/*
+ * ctl callbacks for slaves
+ */
+static int slave_info(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_info *uinfo)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	return slave->slave.info(&slave->slave, uinfo);
+}
+
+static int slave_get(struct snd_kcontrol *kcontrol,
+		     struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	return slave_get_val(slave, ucontrol);
+}
+
+static int slave_put(struct snd_kcontrol *kcontrol,
+		     struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	int err, ch, changed = 0;
+
+	err = slave_init(slave);
+	if (err < 0)
+		return err;
+	for (ch = 0; ch < slave->info.count; ch++) {
+		if (slave->vals[ch] != ucontrol->value.integer.value[ch]) {
+			changed = 1;
+			slave->vals[ch] = ucontrol->value.integer.value[ch];
+		}
+	}
+	if (!changed)
+		return 0;
+	return slave_put_val(slave, ucontrol);
+}
+
+static int slave_tlv_cmd(struct snd_kcontrol *kcontrol,
+			 int op_flag, unsigned int size,
+			 unsigned int __user *tlv)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	/* FIXME: this assumes that the max volume is 0 dB */
+	return slave->slave.tlv.c(&slave->slave, op_flag, size, tlv);
+}
+
+static void slave_free(struct snd_kcontrol *kcontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	if (slave->slave.private_free)
+		slave->slave.private_free(&slave->slave);
+	if (slave->master)
+		list_del(&slave->list);
+	kfree(slave);
+}
+
+/*
+ * Add a slave control to the group with the given master control
+ *
+ * All slaves must be the same type (returning the same information
+ * via info callback).  The fucntion doesn't check it, so it's your
+ * responsibility.
+ *
+ * Also, some additional limitations:
+ * - at most two channels
+ * - logarithmic volume control (dB level), no linear volume
+ * - master can only attenuate the volume, no gain
+ */
+int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave)
+{
+	struct link_master *master_link = snd_kcontrol_chip(master);
+	struct link_slave *srec;
+
+	srec = kzalloc(sizeof(*srec) +
+		       slave->count * sizeof(*slave->vd), GFP_KERNEL);
+	if (!srec)
+		return -ENOMEM;
+	srec->slave = *slave;
+	memcpy(srec->slave.vd, slave->vd, slave->count * sizeof(*slave->vd));
+	srec->master = master_link;
+
+	/* override callbacks */
+	slave->info = slave_info;
+	slave->get = slave_get;
+	slave->put = slave_put;
+	if (slave->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK)
+		slave->tlv.c = slave_tlv_cmd;
+	slave->private_data = srec;
+	slave->private_free = slave_free;
+
+	list_add_tail(&srec->list, &master_link->slaves);
+	return 0;
+}
+
+EXPORT_SYMBOL(snd_ctl_add_slave);
+
+/*
+ * ctl callbacks for master controls
+ */
+static int master_info(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_info *uinfo)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	int ret;
+
+	ret = master_init(master);
+	if (ret < 0)
+		return ret;
+	uinfo->type = master->info.type;
+	uinfo->count = master->info.count;
+	uinfo->value.integer.min = master->info.min_val;
+	uinfo->value.integer.max = master->info.max_val;
+	return 0;
+}
+
+static int master_get(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	int err = master_init(master);
+	if (err < 0)
+		return err;
+	ucontrol->value.integer.value[0] = master->val;
+	return 0;
+}
+
+static int master_put(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	struct link_slave *slave;
+	struct snd_ctl_elem_value *uval;
+	int err, old_val;
+
+	err = master_init(master);
+	if (err < 0)
+		return err;
+	old_val = master->val;
+	if (ucontrol->value.integer.value[0] == old_val)
+		return 0;
+
+	uval = kmalloc(sizeof(*uval), GFP_KERNEL);
+	if (!uval)
+		return -ENOMEM;
+	list_for_each_entry(slave, &master->slaves, list) {
+		master->val = old_val;
+		uval->id = slave->slave.id;
+		slave_get_val(slave, uval);
+		master->val = ucontrol->value.integer.value[0];
+		slave_put_val(slave, uval);
+	}
+	kfree(uval);
+	return 1;
+}
+
+static void master_free(struct snd_kcontrol *kcontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	struct link_slave *slave;
+
+	list_for_each_entry(slave, &master->slaves, list)
+		slave->master = NULL;
+	kfree(master);
+}
+
+
+/*
+ * Create a virtual master control with the given name
+ */
+struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
+						 const unsigned int *tlv)
+{
+	struct link_master *master;
+	struct snd_kcontrol *kctl;
+	struct snd_kcontrol_new knew;
+
+	memset(&knew, 0, sizeof(knew));
+	knew.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	knew.name = name;
+	knew.info = master_info;
+
+	master = kzalloc(sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return NULL;
+	INIT_LIST_HEAD(&master->slaves);
+
+	kctl = snd_ctl_new1(&knew, master);
+	if (!kctl) {
+		kfree(master);
+		return NULL;
+	}
+	/* override some callbacks */
+	kctl->info = master_info;
+	kctl->get = master_get;
+	kctl->put = master_put;
+	kctl->private_free = master_free;
+
+	/* additional (constant) TLV read */
+	if (tlv) {
+		/* FIXME: this assumes that the max volume is 0 dB */
+		kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ;
+		kctl->tlv.p = tlv;
+	}
+	return kctl;
+}
+
+EXPORT_SYMBOL(snd_ctl_make_virtual_master);
diff -r 1207d46e60b3 include/control.h
--- a/include/control.h	Mon Dec 03 15:15:59 2007 +0100
+++ b/include/control.h	Mon Dec 03 15:46:17 2007 +0100
@@ -169,4 +169,12 @@ int snd_ctl_boolean_stereo_info(struct s
 int snd_ctl_boolean_stereo_info(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_info *uinfo);
 
+/*
+ * virtual master control
+ */
+struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
+						 const unsigned int *tlv);
+int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave);
+		      
+
 #endif	/* __SOUND_CONTROL_H */

[RFC] [Patch 2/2] Add virtual master to HD-audio

[Takashi Iwai]

This patch adds the virtual master controls to some codecs.
The virtual master controls are added only when no real "Master"
control is present.


Takashi

diff -r 861c951ac951 pci/Kconfig
--- a/pci/Kconfig	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/Kconfig	Tue Sep 18 15:48:29 2007 +0200
@@ -512,6 +512,7 @@ config SND_HDA_CODEC_REALTEK
 config SND_HDA_CODEC_REALTEK
 	bool "Build Realtek HD-audio codec support"
 	depends on SND_HDA_INTEL
+	select SND_VMASTER
 	default y
 	help
 	  Say Y here to include Realtek HD-audio codec support in
@@ -520,6 +521,7 @@ config SND_HDA_CODEC_ANALOG
 config SND_HDA_CODEC_ANALOG
 	bool "Build Analog Device HD-audio codec support"
 	depends on SND_HDA_INTEL
+	select SND_VMASTER
 	default y
 	help
 	  Say Y here to include Analog Device HD-audio codec support in
@@ -528,6 +530,7 @@ config SND_HDA_CODEC_SIGMATEL
 config SND_HDA_CODEC_SIGMATEL
 	bool "Build IDT/Sigmatel HD-audio codec support"
 	depends on SND_HDA_INTEL
+	select SND_VMASTER
 	default y
 	help
 	  Say Y here to include IDT (Sigmatel) HD-audio codec support in
diff -r 861c951ac951 pci/hda/hda_codec.c
--- a/pci/hda/hda_codec.c	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/hda/hda_codec.c	Tue Sep 18 15:48:29 2007 +0200
@@ -1016,6 +1016,68 @@ int snd_hda_mixer_amp_tlv(struct snd_kco
 		return -EFAULT;
 	return 0;
 }
+
+#ifdef CONFIG_SND_VMASTER
+/*
+ * set (static) TLV for virtual master volume; recalculated as max 0dB
+ */
+void snd_hda_set_vmaster_tlv(struct hda_codec *codec, hda_nid_t nid, int dir,
+			     unsigned int *tlv)
+{
+	u32 caps;
+	int nums, step;
+
+	caps = query_amp_caps(codec, nid, dir);
+	nums = (caps & AC_AMPCAP_NUM_STEPS) >> AC_AMPCAP_NUM_STEPS_SHIFT;
+	step = (caps & AC_AMPCAP_STEP_SIZE) >> AC_AMPCAP_STEP_SIZE_SHIFT;
+	step = (step + 1) * 25;
+	tlv[0] = SNDRV_CTL_TLVT_DB_SCALE;
+	tlv[1] = 2 * sizeof(unsigned int);
+	tlv[2] = -nums * step;
+	tlv[3] = step;
+}
+
+/* find a mixer control element with the given name */
+struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec,
+					    const char *name)
+{
+	struct snd_ctl_elem_id id;
+	memset(&id, 0, sizeof(id));
+	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	strcpy(id.name, name);
+	return snd_ctl_find_id(codec->bus->card, &id);
+}
+
+/* create a virtual master control and add slaves */
+int snd_hda_add_vmaster(struct hda_codec *codec, char *name,
+			unsigned int *tlv, const char **slaves)
+{
+	struct snd_kcontrol *kctl;
+	const char **s;
+	int err;
+
+	kctl = snd_ctl_make_virtual_master(name, tlv);
+	if (!kctl)
+		return -ENOMEM;
+	err = snd_ctl_add(codec->bus->card, kctl);
+	if (err < 0)
+		return err;
+	
+	for (s = slaves; *s; s++) {
+		struct snd_kcontrol *sctl;
+
+		sctl = snd_hda_find_mixer_ctl(codec, *s);
+		if (!sctl) {
+			snd_printdd("Cannot find slave %s, skipped\n", *s);
+			continue;
+		}
+		err = snd_ctl_add_slave(kctl, sctl);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+#endif /* CONFIG_SND_VMASTER */
 
 /* switch */
 int snd_hda_mixer_amp_switch_info(struct snd_kcontrol *kcontrol,
diff -r 861c951ac951 pci/hda/hda_local.h
--- a/pci/hda/hda_local.h	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/hda/hda_local.h	Tue Sep 18 15:48:29 2007 +0200
@@ -90,6 +90,13 @@ void snd_hda_codec_resume_amp(struct hda
 void snd_hda_codec_resume_amp(struct hda_codec *codec);
 #endif
 
+void snd_hda_set_vmaster_tlv(struct hda_codec *codec, hda_nid_t nid, int dir,
+			     unsigned int *tlv);
+struct snd_kcontrol *snd_hda_find_mixer_ctl(struct hda_codec *codec,
+					    const char *name);
+int snd_hda_add_vmaster(struct hda_codec *codec, char *name,
+			unsigned int *tlv, const char **slaves);
+
 /* amp value bits */
 #define HDA_AMP_MUTE	0x80
 #define HDA_AMP_UNMUTE	0x00
diff -r 861c951ac951 pci/hda/patch_analog.c
--- a/pci/hda/patch_analog.c	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/hda/patch_analog.c	Tue Sep 18 15:48:29 2007 +0200
@@ -79,6 +79,11 @@ struct ad198x_spec {
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	struct hda_loopback_check loopback;
 #endif
+	/* for virtual master */
+	hda_nid_t vmaster_nid;
+	u32 vmaster_tlv[4];
+	const char **slave_vols;
+	const char **slave_sws;
 };
 
 /*
@@ -125,6 +130,28 @@ static int ad198x_init(struct hda_codec 
 		snd_hda_sequence_write(codec, spec->init_verbs[i]);
 	return 0;
 }
+
+static const char *ad_slave_vols[] = {
+	"Front Playback Volume",
+	"Surround Playback Volume",
+	"Center Playback Volume",
+	"LFE Playback Volume",
+	"Side Playback Volume",
+	"Headphone Playback Volume",
+	"Mono Playback Volume",
+	NULL
+};
+
+static const char *ad_slave_sws[] = {
+	"Front Playback Switch",
+	"Surround Playback Switch",
+	"Center Playback Switch",
+	"LFE Playback Switch",
+	"Side Playback Switch",
+	"Headphone Playback Switch",
+	"Mono Playback Switch",
+	NULL
+};
 
 static int ad198x_build_controls(struct hda_codec *codec)
 {
@@ -147,6 +174,27 @@ static int ad198x_build_controls(struct 
 		if (err < 0)
 			return err;
 	}
+
+	/* if we have no master control, let's create it */
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Volume")) {
+		snd_hda_set_vmaster_tlv(codec, spec->vmaster_nid,
+					HDA_OUTPUT, spec->vmaster_tlv);
+		err = snd_hda_add_vmaster(codec, "Master Playback Volume",
+					  spec->vmaster_tlv,
+					  (spec->slave_vols ?
+					   spec->slave_vols : ad_slave_vols));
+		if (err < 0)
+			return err;
+	}
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) {
+		err = snd_hda_add_vmaster(codec, "Master Playback Switch",
+					  NULL,
+					  (spec->slave_sws ?
+					   spec->slave_sws : ad_slave_sws));
+		if (err < 0)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -897,6 +945,7 @@ static int patch_ad1986a(struct hda_code
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1986a_loopbacks;
 #endif
+	spec->vmaster_nid = 0x1b;
 
 	codec->patch_ops = ad198x_patch_ops;
 
@@ -1129,6 +1178,7 @@ static int patch_ad1983(struct hda_codec
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1983_loopbacks;
 #endif
+	spec->vmaster_nid = 0x05;
 
 	codec->patch_ops = ad198x_patch_ops;
 
@@ -1525,6 +1575,7 @@ static int patch_ad1981(struct hda_codec
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1981_loopbacks;
 #endif
+	spec->vmaster_nid = 0x05;
 
 	codec->patch_ops = ad198x_patch_ops;
 
@@ -2834,6 +2885,7 @@ static int patch_ad1988(struct hda_codec
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1988_loopbacks;
 #endif
+	spec->vmaster_nid = 0x04;
 
 	return 0;
 }
@@ -3000,6 +3052,19 @@ static struct hda_amp_list ad1884_loopba
 };
 #endif
 
+static const char *ad1884_slave_vols[] = {
+	"PCM Playback Volume",
+	"Mic Playback Volume",
+	"Mono Playback Volume",
+	"Front Mic Playback Volume",
+	"Mic Playback Volume",
+	"CD Playback Volume",
+	"Internal Mic Playback Volume",
+	"Docking Mic Playback Volume"
+	"Beep Playback Volume",
+	NULL
+};
+
 static int patch_ad1884(struct hda_codec *codec)
 {
 	struct ad198x_spec *spec;
@@ -3027,6 +3092,9 @@ static int patch_ad1884(struct hda_codec
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1884_loopbacks;
 #endif
+	spec->vmaster_nid = 0x04;
+	/* we need to cover all playback volumes */
+	spec->slave_vols = ad1884_slave_vols;
 
 	codec->patch_ops = ad198x_patch_ops;
 
@@ -3459,6 +3527,7 @@ static int patch_ad1882(struct hda_codec
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	spec->loopback.amplist = ad1882_loopbacks;
 #endif
+	spec->vmaster_nid = 0x04;
 
 	codec->patch_ops = ad198x_patch_ops;
 
diff -r 861c951ac951 pci/hda/patch_realtek.c
--- a/pci/hda/patch_realtek.c	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/hda/patch_realtek.c	Tue Sep 18 15:48:29 2007 +0200
@@ -247,6 +247,9 @@ struct alc_spec {
 	unsigned int sense_updated: 1;
 	unsigned int jack_present: 1;
 
+	/* for virtual master */
+	hda_nid_t vmaster_nid;
+	u32 vmaster_tlv[4];
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	struct hda_loopback_check loopback;
 #endif
@@ -1098,8 +1101,8 @@ static struct snd_kcontrol_new alc880_f1
 static struct snd_kcontrol_new alc880_f1734_mixer[] = {
 	HDA_CODEC_VOLUME("Headphone Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
 	HDA_BIND_MUTE("Headphone Playback Switch", 0x0c, 2, HDA_INPUT),
-	HDA_CODEC_VOLUME("Internal Speaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("Internal Speaker Playback Switch", 0x0d, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Speaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Speaker Playback Switch", 0x0d, 2, HDA_INPUT),
 	HDA_CODEC_VOLUME("CD Playback Volume", 0x0b, 0x04, HDA_INPUT),
 	HDA_CODEC_MUTE("CD Playback Switch", 0x0b, 0x04, HDA_INPUT),
 	HDA_CODEC_VOLUME("Mic Playback Volume", 0x0b, 0x0, HDA_INPUT),
@@ -1197,10 +1200,10 @@ static struct snd_kcontrol_new alc880_tc
 
 /* Uniwill */
 static struct snd_kcontrol_new alc880_uniwill_mixer[] = {
-	HDA_CODEC_VOLUME("HPhone Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("HPhone Playback Switch", 0x0c, 2, HDA_INPUT),
-	HDA_CODEC_VOLUME("iSpeaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("iSpeaker Playback Switch", 0x0d, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Headphone Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Headphone Playback Switch", 0x0c, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Speaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Speaker Playback Switch", 0x0d, 2, HDA_INPUT),
 	HDA_CODEC_VOLUME_MONO("Center Playback Volume", 0x0e, 1, 0x0, HDA_OUTPUT),
 	HDA_CODEC_VOLUME_MONO("LFE Playback Volume", 0x0e, 2, 0x0, HDA_OUTPUT),
 	HDA_BIND_MUTE_MONO("Center Playback Switch", 0x0e, 1, 2, HDA_INPUT),
@@ -1240,13 +1243,47 @@ static struct snd_kcontrol_new alc880_fu
 };
 
 static struct snd_kcontrol_new alc880_uniwill_p53_mixer[] = {
-	HDA_CODEC_VOLUME("HPhone Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("HPhone Playback Switch", 0x0c, 2, HDA_INPUT),
-	HDA_CODEC_VOLUME("iSpeaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("iSpeaker Playback Switch", 0x0d, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Headphone Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Headphone Playback Switch", 0x0c, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Speaker Playback Volume", 0x0d, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Speaker Playback Switch", 0x0d, 2, HDA_INPUT),
 	HDA_CODEC_VOLUME("Mic Playback Volume", 0x0b, 0x0, HDA_INPUT),
 	HDA_CODEC_MUTE("Mic Playback Switch", 0x0b, 0x0, HDA_INPUT),
 	{ } /* end */
+};
+
+/*
+ * virtual master controls
+ */
+
+/*
+ * slave controls for virtual master
+ */
+static const char *alc_slave_vols[] = {
+	"Front Playback Volume",
+	"Surround Playback Volume",
+	"Center Playback Volume",
+	"LFE Playback Volume",
+	"Side Playback Volume",
+	"Headphone Playback Volume",
+	"Speaker Playback Volume",
+	"Mono Playback Volume",
+	"iSpeaker Playback Volume",
+	"Line-Out Playback Volume",
+	NULL,
+};
+
+static const char *alc_slave_sws[] = {
+	"Front Playback Switch",
+	"Surround Playback Switch",
+	"Center Playback Switch",
+	"LFE Playback Switch",
+	"Side Playback Switch",
+	"Headphone Playback Switch",
+	"Speaker Playback Switch",
+	"Mono Playback Switch",
+	"iSpeaker Playback Switch",
+	NULL,
 };
 
 /*
@@ -1275,6 +1312,23 @@ static int alc_build_controls(struct hda
 		if (err < 0)
 			return err;
 	}
+
+	/* if we have no master control, let's create it */
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Volume")) {
+		snd_hda_set_vmaster_tlv(codec, spec->vmaster_nid,
+					HDA_OUTPUT, spec->vmaster_tlv);
+		err = snd_hda_add_vmaster(codec, "Master Playback Volume",
+					  spec->vmaster_tlv, alc_slave_vols);
+		if (err < 0)
+			return err;
+	}
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) {
+		err = snd_hda_add_vmaster(codec, "Master Playback Switch",
+					  NULL, alc_slave_sws);
+		if (err < 0)
+			return err;
+	}
+
 	return 0;
 }
 
@@ -1823,8 +1877,8 @@ static struct hda_channel_mode alc880_lg
 
 static struct snd_kcontrol_new alc880_lg_mixer[] = {
 	/* FIXME: it's not really "master" but front channels */
-	HDA_CODEC_VOLUME("Master Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
-	HDA_BIND_MUTE("Master Playback Switch", 0x0f, 2, HDA_INPUT),
+	HDA_CODEC_VOLUME("Front Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
+	HDA_BIND_MUTE("Front Playback Switch", 0x0f, 2, HDA_INPUT),
 	HDA_CODEC_VOLUME("Surround Playback Volume", 0x0c, 0x0, HDA_OUTPUT),
 	HDA_BIND_MUTE("Surround Playback Switch", 0x0c, 2, HDA_INPUT),
 	HDA_CODEC_VOLUME_MONO("Center Playback Volume", 0x0d, 1, 0x0, HDA_OUTPUT),
@@ -3390,6 +3444,8 @@ static int patch_alc880(struct hda_codec
 			spec->num_mixers++;
 		}
 	}
+
+	spec->vmaster_nid = 0x0c;
 
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC880_AUTO)
@@ -4762,6 +4818,8 @@ static int patch_alc260(struct hda_codec
 	spec->stream_digital_playback = &alc260_pcm_digital_playback;
 	spec->stream_digital_capture = &alc260_pcm_digital_capture;
 
+	spec->vmaster_nid = 0x08;
+
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC260_AUTO)
 		spec->init_hook = alc260_auto_init;
@@ -4962,15 +5020,15 @@ static struct snd_kcontrol_new alc882_ba
 };
 
 static struct snd_kcontrol_new alc885_mbp3_mixer[] = {
-	HDA_CODEC_VOLUME("Master Volume", 0x0c, 0x00, HDA_OUTPUT),
-	HDA_BIND_MUTE   ("Master Switch", 0x0c, 0x02, HDA_INPUT),
-	HDA_CODEC_MUTE  ("Speaker Switch", 0x14, 0x00, HDA_OUTPUT),
-	HDA_CODEC_VOLUME("Line Out Volume", 0x0d,0x00, HDA_OUTPUT),
-	HDA_CODEC_VOLUME("Line In Playback Volume", 0x0b, 0x02, HDA_INPUT),
-	HDA_CODEC_MUTE  ("Line In Playback Switch", 0x0b, 0x02, HDA_INPUT),
+	HDA_CODEC_VOLUME("Front Playback Volume", 0x0c, 0x00, HDA_OUTPUT),
+	HDA_BIND_MUTE   ("Front Playback Switch", 0x0c, 0x02, HDA_INPUT),
+	HDA_CODEC_MUTE  ("Speaker Playback Switch", 0x14, 0x00, HDA_OUTPUT),
+	HDA_CODEC_VOLUME("Line-Out Playback Volume", 0x0d, 0x00, HDA_OUTPUT),
+	HDA_CODEC_VOLUME("Line Playback Volume", 0x0b, 0x02, HDA_INPUT),
+	HDA_CODEC_MUTE  ("Line Playback Switch", 0x0b, 0x02, HDA_INPUT),
 	HDA_CODEC_VOLUME("Mic Playback Volume", 0x0b, 0x00, HDA_INPUT),
 	HDA_CODEC_MUTE  ("Mic Playback Switch", 0x0b, 0x00, HDA_INPUT),
-	HDA_CODEC_VOLUME("Line In Boost", 0x1a, 0x00, HDA_INPUT),
+	HDA_CODEC_VOLUME("Line Boost", 0x1a, 0x00, HDA_INPUT),
 	HDA_CODEC_VOLUME("Mic Boost", 0x18, 0x00, HDA_INPUT),
 	{ } /* end */
 };
@@ -5971,6 +6029,8 @@ static int patch_alc882(struct hda_codec
 			spec->num_mixers++;
 		}
 	}
+
+	spec->vmaster_nid = 0x0c;
 
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC882_AUTO)
@@ -7446,6 +7506,8 @@ static int patch_alc883(struct hda_codec
 		spec->adc_nids = alc883_adc_nids;
 		spec->num_adc_nids = ARRAY_SIZE(alc883_adc_nids);
 	}
+
+	spec->vmaster_nid = 0x0c;
 
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC883_AUTO)
@@ -8566,6 +8628,8 @@ static int patch_alc262(struct hda_codec
 		}
 	}
 
+	spec->vmaster_nid = 0x0c;
+
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC262_AUTO)
 		spec->init_hook = alc262_auto_init;
@@ -9240,6 +9304,9 @@ static int patch_alc268(struct hda_codec
 			}
 		}
 	}
+
+	spec->vmaster_nid = 0x02;
+
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC268_AUTO)
 		spec->init_hook = alc268_auto_init;
@@ -10394,6 +10461,8 @@ static int patch_alc861(struct hda_codec
 	spec->stream_digital_playback = &alc861_pcm_digital_playback;
 	spec->stream_digital_capture = &alc861_pcm_digital_capture;
 
+	spec->vmaster_nid = 0x03;
+
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC861_AUTO)
 		spec->init_hook = alc861_auto_init;
@@ -11370,6 +11439,8 @@ static int patch_alc861vd(struct hda_cod
 
 	spec->mixers[spec->num_mixers] = alc861vd_capture_mixer;
 	spec->num_mixers++;
+
+	spec->vmaster_nid = 0x02;
 
 	codec->patch_ops = alc_patch_ops;
 
@@ -12222,6 +12293,8 @@ static int patch_alc662(struct hda_codec
 		spec->num_adc_nids = ARRAY_SIZE(alc662_adc_nids);
 	}
 
+	spec->vmaster_nid = 0x02;
+
 	codec->patch_ops = alc_patch_ops;
 	if (board_config == ALC662_AUTO)
 		spec->init_hook = alc662_auto_init;
diff -r 861c951ac951 pci/hda/patch_sigmatel.c
--- a/pci/hda/patch_sigmatel.c	Tue Sep 18 15:48:16 2007 +0200
+++ b/pci/hda/patch_sigmatel.c	Tue Sep 18 15:48:29 2007 +0200
@@ -156,6 +156,9 @@ struct sigmatel_spec {
 	struct snd_kcontrol_new *kctl_alloc;
 	struct hda_input_mux private_dimux;
 	struct hda_input_mux private_imux;
+
+	/* virtual master */
+	unsigned int vmaster_tlv[4];
 };
 
 static hda_nid_t stac9200_adc_nids[1] = {
@@ -521,6 +524,34 @@ static struct snd_kcontrol_new stac927x_
 	{ } /* end */
 };
 
+static const char *slave_vols[] = {
+	"Front Playback Volume",
+	"Surround Playback Volume",
+	"Center Playback Volume",
+	"LFE Playback Volume",
+	"Side Playback Volume",
+	"Headphone Playback Volume",
+	"Headphone Playback Volume",
+	"Speaker Playback Volume",
+	"External Speaker Playback Volume",
+	"Speaker2 Playback Volume",
+	NULL
+};
+
+static const char *slave_sws[] = {
+	"Front Playback Switch",
+	"Surround Playback Switch",
+	"Center Playback Switch",
+	"LFE Playback Switch",
+	"Side Playback Switch",
+	"Headphone Playback Switch",
+	"Headphone Playback Switch",
+	"Speaker Playback Switch",
+	"External Speaker Playback Switch",
+	"Speaker2 Playback Switch",
+	NULL
+};
+
 static int stac92xx_build_controls(struct hda_codec *codec)
 {
 	struct sigmatel_spec *spec = codec->spec;
@@ -547,6 +578,23 @@ static int stac92xx_build_controls(struc
 		if (err < 0)
 			return err;
 	}
+
+	/* if we have no master control, let's create it */
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Volume")) {
+		snd_hda_set_vmaster_tlv(codec, spec->multiout.dac_nids[0],
+					HDA_OUTPUT, spec->vmaster_tlv);
+		err = snd_hda_add_vmaster(codec, "Master Playback Volume",
+					  spec->vmaster_tlv, slave_vols);
+		if (err < 0)
+			return err;
+	}
+	if (!snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) {
+		err = snd_hda_add_vmaster(codec, "Master Playback Switch",
+					  NULL, slave_sws);
+		if (err < 0)
+			return err;
+	}
+
 	return 0;	
 }

[RFC] [Patch] Add vmaster to alsa-driver

[Takashi Iwai]

This is the patch to alsa-driver tree.
Simply adds a stub to build vmaster object.

Rebuild from scratch (e.g. ./hgcompile) after vmaster patches because
you'll need to re-create configure script.


Takashi

diff -r 06162eef8936 acore/vmaster.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/acore/vmaster.c	Mon Oct 08 18:26:21 2007 +0200
@@ -0,0 +1,2 @@
+#define __NO_VERSION__
+#include "../alsa-kernel/core/vmaster.c"

[RFC] virtual master control for HD-audio

[Takashi Iwai]

[Resent as I don't see the message back; if you already received it,
 please disregard this]

Hi,

another thing I'd like to push into the next kernel is the virtual
master volume control.  As I posted in earlier posts, it adds virtual
controls that have several slave controls with the same types,
e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
simultaneously via the master control.

It'd be appreciated if some one can test the patches with HD-audio h/w
that has no master control yet (e.g. some STAC codecs).

Note that this won't add the master volumes if there are no real
volume controls.  Some codecs have really no volume control, and this
won't help for such devices.

Two (and one for driver) patches will follow after this.


Takashi

Re: [RFC] virtual master control for HD-audio

[Jaroslav Kysela]

On Thu, 20 Dec 2007, Takashi Iwai wrote:

> Hi,
> 
> another thing I'd like to push into the next kernel is the virtual
> master volume control.  As I posted in earlier posts, it adds virtual
> controls that have several slave controls with the same types,
> e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> simultaneously via the master control.
> 
> It'd be appreciated if some one can test the patches with HD-audio h/w
> that has no master control yet (e.g. some STAC codecs).
> 
> Note that this won't add the master volumes if there are no real
> volume controls.  Some codecs have really no volume control, and this
> won't help for such devices.
> 
> Two (and one for driver) patches will follow after this.

NAK from my side. I am convinced that this code can be implemented in the 
user space even without any daemon just in the mixer abstract layer using 
standard control elements and using eventually new user controls to store 
data for virtual mixer controls.

					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project

Re: [RFC] virtual master control for HD-audio

[Alexander E. Patrakov]

Jaroslav Kysela wrote:
> On Thu, 20 Dec 2007, Takashi Iwai wrote:
> 
>> Hi,
>>
>> another thing I'd like to push into the next kernel is the virtual
>> master volume control.  As I posted in earlier posts, it adds virtual
>> controls that have several slave controls with the same types,
>> e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
>> simultaneously via the master control.
>>
>> It'd be appreciated if some one can test the patches with HD-audio h/w
>> that has no master control yet (e.g. some STAC codecs).
>>
>> Note that this won't add the master volumes if there are no real
>> volume controls.  Some codecs have really no volume control, and this
>> won't help for such devices.
>>
>> Two (and one for driver) patches will follow after this.
> 
> NAK from my side. I am convinced that this code can be implemented in the 
> user space even without any daemon just in the mixer abstract layer using 
> standard control elements and using eventually new user controls to store 
> data for virtual mixer controls.

Aren't the patches supposed to be useful also for OSS-only apps, where userspace 
solutions can't work without LD_PRELOAD hacks? Currently, without the patch and 
the "aoss" wrapper, OSS apps sound too loud. I have not tested the patch, but I 
am too lazy to change each .desktop file for each OSS-only application provided 
by the distribution.

-- 
Alexander E. Patrakov

Re: [RFC] virtual master control for HD-audio

[Takashi Iwai]

At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 20 Dec 2007, Takashi Iwai wrote:
> 
> > Hi,
> > 
> > another thing I'd like to push into the next kernel is the virtual
> > master volume control.  As I posted in earlier posts, it adds virtual
> > controls that have several slave controls with the same types,
> > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > simultaneously via the master control.
> > 
> > It'd be appreciated if some one can test the patches with HD-audio h/w
> > that has no master control yet (e.g. some STAC codecs).
> > 
> > Note that this won't add the master volumes if there are no real
> > volume controls.  Some codecs have really no volume control, and this
> > won't help for such devices.
> > 
> > Two (and one for driver) patches will follow after this.
> 
> NAK from my side. I am convinced that this code can be implemented in the 
> user space even without any daemon just in the mixer abstract layer using 
> standard control elements and using eventually new user controls to store 
> data for virtual mixer controls.

A user-space implementation of virtual mixer elements would be far
more complicated than the simplistic kernel-space patch.  I've
considered it many times, even tried to implement it, but got that
conclusion.  You'll see obviously the following difficulties:

1. Many user-space virtual elements

Each slave control element needs a virtual element (eventually a
user-space one) because we need to keep both raw and virtual values to
handle saturation.  That is, the same number of additional controls
would be added.  Significant for 7.1 outputs.

2. Easy incosistency between virtual and raw elements

Even if the mixer abstraction hides the virtual elements, both
virtual and raw elements are exposed on control API.  This can cause
the value-inconsistency between them quite easily, because many apps
access directly via control API (even some mixer apps), and they
likely change only raw values.  The similar situation is for kernel
OSS emulation.

3. Complicated configuration

The requirement of virtual master controls is very much dependent on
the hardware.  In the case of HD-audio, it depends on codec chip
types, and even on the preset model chosen via PCI SSID or a module
option.  Implementing such a complex conditional in alsa-lib
configuration is a clear overhead.

4. More resource requirement

Clearly a user-space solution requires more layers in alsa-lib, which
is invoked by each process, and even more memory footprint than the
kernel solution, not only the additional complexity.


... and think again what is the benifit of the user-space solution in
this case, in comparison with the demerits above.

If you could implement the same feature on user-space with less amount
of codes, I'd happily take it.  But, I'm sure it's very hard.


Takashi

Re: [RFC] virtual master control for HD-audio

[Lee Revell]

On Dec 22, 2007 2:39 AM, Alexander E. Patrakov <patrakov@gmail.com> wrote:
> Aren't the patches supposed to be useful also for OSS-only apps, where userspace
> solutions can't work without LD_PRELOAD hacks? Currently, without the patch and
> the "aoss" wrapper, OSS apps sound too loud. I have not tested the patch, but I
> am too lazy to change each .desktop file for each OSS-only application provided
> by the distribution.

Please file a bug report with your distro.  It should be considered a
bug for a modern distro to ship OSS only apps withut using the aoss
wrapper.  It causes all sorts of problems, like blocking the soundcard
for ALSA apps.

Lee

Re: [RFC] virtual master control for HD-audio

[Takashi Iwai]

At Sat, 22 Dec 2007 15:55:19 +0100,
I wrote:
> 
> At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
> Jaroslav Kysela wrote:
> > 
> > On Thu, 20 Dec 2007, Takashi Iwai wrote:
> > 
> > > Hi,
> > > 
> > > another thing I'd like to push into the next kernel is the virtual
> > > master volume control.  As I posted in earlier posts, it adds virtual
> > > controls that have several slave controls with the same types,
> > > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > > simultaneously via the master control.
> > > 
> > > It'd be appreciated if some one can test the patches with HD-audio h/w
> > > that has no master control yet (e.g. some STAC codecs).
> > > 
> > > Note that this won't add the master volumes if there are no real
> > > volume controls.  Some codecs have really no volume control, and this
> > > won't help for such devices.
> > > 
> > > Two (and one for driver) patches will follow after this.
> > 
> > NAK from my side. I am convinced that this code can be implemented in the 
> > user space even without any daemon just in the mixer abstract layer using 
> > standard control elements and using eventually new user controls to store 
> > data for virtual mixer controls.
> 
> A user-space implementation of virtual mixer elements would be far
> more complicated than the simplistic kernel-space patch.  I've
> considered it many times, even tried to implement it, but got that
> conclusion.  You'll see obviously the following difficulties:
> 
> 1. Many user-space virtual elements
> 
> Each slave control element needs a virtual element (eventually a
> user-space one) because we need to keep both raw and virtual values to
> handle saturation.  That is, the same number of additional controls
> would be added.  Significant for 7.1 outputs.
> 
> 2. Easy incosistency between virtual and raw elements
> 
> Even if the mixer abstraction hides the virtual elements, both
> virtual and raw elements are exposed on control API.  This can cause
> the value-inconsistency between them quite easily, because many apps
> access directly via control API (even some mixer apps), and they
> likely change only raw values.  The similar situation is for kernel
> OSS emulation.
> 
> 3. Complicated configuration
> 
> The requirement of virtual master controls is very much dependent on
> the hardware.  In the case of HD-audio, it depends on codec chip
> types, and even on the preset model chosen via PCI SSID or a module
> option.  Implementing such a complex conditional in alsa-lib
> configuration is a clear overhead.
> 
> 4. More resource requirement
> 
> Clearly a user-space solution requires more layers in alsa-lib, which
> is invoked by each process, and even more memory footprint than the
> kernel solution, not only the additional complexity.
> 
> 
> ... and think again what is the benifit of the user-space solution in
> this case, in comparison with the demerits above.
> 
> If you could implement the same feature on user-space with less amount
> of codes, I'd happily take it.  But, I'm sure it's very hard.

The master control is really important feature.  Unless you have
really a better patch, I'd like to commit the vmaster patch soon, so
that more people can test.


thanks,

Takashi

Re: [RFC] virtual master control for HD-audio

[Jaroslav Kysela]

On Sat, 22 Dec 2007, Takashi Iwai wrote:

> At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
> Jaroslav Kysela wrote:
> > 
> > On Thu, 20 Dec 2007, Takashi Iwai wrote:
> > 
> > > Hi,
> > > 
> > > another thing I'd like to push into the next kernel is the virtual
> > > master volume control.  As I posted in earlier posts, it adds virtual
> > > controls that have several slave controls with the same types,
> > > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > > simultaneously via the master control.
> > > 
> > > It'd be appreciated if some one can test the patches with HD-audio h/w
> > > that has no master control yet (e.g. some STAC codecs).
> > > 
> > > Note that this won't add the master volumes if there are no real
> > > volume controls.  Some codecs have really no volume control, and this
> > > won't help for such devices.
> > > 
> > > Two (and one for driver) patches will follow after this.
> > 
> > NAK from my side. I am convinced that this code can be implemented in the 
> > user space even without any daemon just in the mixer abstract layer using 
> > standard control elements and using eventually new user controls to store 
> > data for virtual mixer controls.
> 
> A user-space implementation of virtual mixer elements would be far
> more complicated than the simplistic kernel-space patch.  I've
> considered it many times, even tried to implement it, but got that
> conclusion.  You'll see obviously the following difficulties:
> 
> 1. Many user-space virtual elements
> 
> Each slave control element needs a virtual element (eventually a
> user-space one) because we need to keep both raw and virtual values to
> handle saturation.  That is, the same number of additional controls
> would be added.  Significant for 7.1 outputs.

Note that these "extra" values can be handled together using only one 
user-space control element without "Volume/Switch" in name.

> 2. Easy incosistency between virtual and raw elements
> 
> Even if the mixer abstraction hides the virtual elements, both
> virtual and raw elements are exposed on control API.  This can cause
> the value-inconsistency between them quite easily, because many apps
> access directly via control API (even some mixer apps), and they
> likely change only raw values.  The similar situation is for kernel
> OSS emulation.

I have basically two ideas to handle this.

1) create a plugin system directly in the snd_ctl_* interface

   - basically code might be very same code as you proposed in kernel

2) create virtual things only in smixer

   - it's something I strongly prefer
   - if mixers uses CTL interface, it's not our bussiness because
     we haven't claimed to create virtual layers in CTL

> 3. Complicated configuration
> 
> The requirement of virtual master controls is very much dependent on
> the hardware.  In the case of HD-audio, it depends on codec chip
> types, and even on the preset model chosen via PCI SSID or a module
> option.  Implementing such a complex conditional in alsa-lib
> configuration is a clear overhead.

I don't agree much here. Of course, we might need some hints from the 
driver (using information about components in sndrv_ctl_card_info) or we 
might analyze available controls using their names if Master control is 
present or we can eventually create a configuration tool saving hints to 
alsa-lib's configuration files which controls should be added for given 
hardware (this configuration tool might create a list of useable PCM 
devices handled in the driver, too). Just idea.

> 4. More resource requirement
> 
> Clearly a user-space solution requires more layers in alsa-lib, which
> is invoked by each process, and even more memory footprint than the
> kernel solution, not only the additional complexity.
> 
> 
> ... and think again what is the benifit of the user-space solution in
> this case, in comparison with the demerits above.

The benefit is flexibility for future extensions.

> If you could implement the same feature on user-space with less amount
> of codes, I'd happily take it.  But, I'm sure it's very hard.

The question is also how to behave in future and if we should allow to 
break our basic rule in the current ALSA design - everything that can be
moved to user space will be moved to user space.

						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [RFC] virtual master control for HD-audio

[Takashi Iwai]

At Thu, 10 Jan 2008 11:29:39 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Sat, 22 Dec 2007, Takashi Iwai wrote:
> 
> > At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
> > Jaroslav Kysela wrote:
> > > 
> > > On Thu, 20 Dec 2007, Takashi Iwai wrote:
> > > 
> > > > Hi,
> > > > 
> > > > another thing I'd like to push into the next kernel is the virtual
> > > > master volume control.  As I posted in earlier posts, it adds virtual
> > > > controls that have several slave controls with the same types,
> > > > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > > > simultaneously via the master control.
> > > > 
> > > > It'd be appreciated if some one can test the patches with HD-audio h/w
> > > > that has no master control yet (e.g. some STAC codecs).
> > > > 
> > > > Note that this won't add the master volumes if there are no real
> > > > volume controls.  Some codecs have really no volume control, and this
> > > > won't help for such devices.
> > > > 
> > > > Two (and one for driver) patches will follow after this.
> > > 
> > > NAK from my side. I am convinced that this code can be implemented in the 
> > > user space even without any daemon just in the mixer abstract layer using 
> > > standard control elements and using eventually new user controls to store 
> > > data for virtual mixer controls.
> > 
> > A user-space implementation of virtual mixer elements would be far
> > more complicated than the simplistic kernel-space patch.  I've
> > considered it many times, even tried to implement it, but got that
> > conclusion.  You'll see obviously the following difficulties:
> > 
> > 1. Many user-space virtual elements
> > 
> > Each slave control element needs a virtual element (eventually a
> > user-space one) because we need to keep both raw and virtual values to
> > handle saturation.  That is, the same number of additional controls
> > would be added.  Significant for 7.1 outputs.
> 
> Note that these "extra" values can be handled together using only one 
> user-space control element without "Volume/Switch" in name.

This method is very likely fragile.  Think what happens if the number
of elements changes, or if the element numid changes.  Then the packed
values will be screwed up.


> > 2. Easy incosistency between virtual and raw elements
> > 
> > Even if the mixer abstraction hides the virtual elements, both
> > virtual and raw elements are exposed on control API.  This can cause
> > the value-inconsistency between them quite easily, because many apps
> > access directly via control API (even some mixer apps), and they
> > likely change only raw values.  The similar situation is for kernel
> > OSS emulation.
> 
> I have basically two ideas to handle this.
> 
> 1) create a plugin system directly in the snd_ctl_* interface
> 
>    - basically code might be very same code as you proposed in kernel

No.  The big difference is that the raw values are still exposed via
control API (otherwise how do you read/write raw values?).  That is,
any user-space program may break consistency.  OTOH, the kernel
solution doesn't expose raw values.

> 2) create virtual things only in smixer
> 
>    - it's something I strongly prefer
>    - if mixers uses CTL interface, it's not our bussiness because
>      we haven't claimed to create virtual layers in CTL

Hm, I have to say it's a bad attitude.  It's our business to keep the
whole system consistent.  We shouldn't build such a weak and fragile
system any more.

And what about the kernel OSS mixer?  There are still many apps to
handle it.

> > 3. Complicated configuration
> > 
> > The requirement of virtual master controls is very much dependent on
> > the hardware.  In the case of HD-audio, it depends on codec chip
> > types, and even on the preset model chosen via PCI SSID or a module
> > option.  Implementing such a complex conditional in alsa-lib
> > configuration is a clear overhead.
> 
> I don't agree much here. Of course, we might need some hints from the 
> driver (using information about components in sndrv_ctl_card_info) or we 
> might analyze available controls using their names if Master control is 
> present or we can eventually create a configuration tool saving hints to 
> alsa-lib's configuration files which controls should be added for given 
> hardware (this configuration tool might create a list of useable PCM 
> devices handled in the driver, too). Just idea.

But it *is* an overhead.

> > 4. More resource requirement
> > 
> > Clearly a user-space solution requires more layers in alsa-lib, which
> > is invoked by each process, and even more memory footprint than the
> > kernel solution, not only the additional complexity.
> > 
> > 
> > ... and think again what is the benifit of the user-space solution in
> > this case, in comparison with the demerits above.
> 
> The benefit is flexibility for future extensions.

The demerit is complexity even longing for future :)

And the worst thing is that it is not implemented yet *at all*.  How
many years have we told that this kind of thing must be implemented in
user-space?

Seriously, we need a solution RIGHT NOW.

> > If you could implement the same feature on user-space with less amount
> > of codes, I'd happily take it.  But, I'm sure it's very hard.
> 
> The question is also how to behave in future and if we should allow to 
> break our basic rule in the current ALSA design - everything that can be
> moved to user space will be moved to user space.

Hey, it's just the question of balance.  Insisting on a rule makes you
blind.  The rule exists not for following rules.  The rule exists for
creating a better system.

Note taht this isn't for solving every mixer problem.  It doesn't
solve the world hunger.  But, it solves nicely the really annoying
problem we face right now, without messy, fragile and overly complex
implementation.


Takashi

Re: [RFC] virtual master control for HD-audio

[Jaroslav Kysela]

On Thu, 10 Jan 2008, Takashi Iwai wrote:

> At Thu, 10 Jan 2008 11:29:39 +0100 (CET),
> Jaroslav Kysela wrote:
> > 
> > On Sat, 22 Dec 2007, Takashi Iwai wrote:
> > 
> > > At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
> > > Jaroslav Kysela wrote:
> > > > 
> > > > On Thu, 20 Dec 2007, Takashi Iwai wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > another thing I'd like to push into the next kernel is the virtual
> > > > > master volume control.  As I posted in earlier posts, it adds virtual
> > > > > controls that have several slave controls with the same types,
> > > > > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > > > > simultaneously via the master control.
> > > > > 
> > > > > It'd be appreciated if some one can test the patches with HD-audio h/w
> > > > > that has no master control yet (e.g. some STAC codecs).
> > > > > 
> > > > > Note that this won't add the master volumes if there are no real
> > > > > volume controls.  Some codecs have really no volume control, and this
> > > > > won't help for such devices.
> > > > > 
> > > > > Two (and one for driver) patches will follow after this.
> > > > 
> > > > NAK from my side. I am convinced that this code can be implemented in the 
> > > > user space even without any daemon just in the mixer abstract layer using 
> > > > standard control elements and using eventually new user controls to store 
> > > > data for virtual mixer controls.
> > > 
> > > A user-space implementation of virtual mixer elements would be far
> > > more complicated than the simplistic kernel-space patch.  I've
> > > considered it many times, even tried to implement it, but got that
> > > conclusion.  You'll see obviously the following difficulties:
> > > 
> > > 1. Many user-space virtual elements
> > > 
> > > Each slave control element needs a virtual element (eventually a
> > > user-space one) because we need to keep both raw and virtual values to
> > > handle saturation.  That is, the same number of additional controls
> > > would be added.  Significant for 7.1 outputs.
> > 
> > Note that these "extra" values can be handled together using only one 
> > user-space control element without "Volume/Switch" in name.
> 
> This method is very likely fragile.  Think what happens if the number
> of elements changes, or if the element numid changes.  Then the packed
> values will be screwed up.
> 
> 
> > > 2. Easy incosistency between virtual and raw elements
> > > 
> > > Even if the mixer abstraction hides the virtual elements, both
> > > virtual and raw elements are exposed on control API.  This can cause
> > > the value-inconsistency between them quite easily, because many apps
> > > access directly via control API (even some mixer apps), and they
> > > likely change only raw values.  The similar situation is for kernel
> > > OSS emulation.
> > 
> > I have basically two ideas to handle this.
> > 
> > 1) create a plugin system directly in the snd_ctl_* interface
> > 
> >    - basically code might be very same code as you proposed in kernel
> 
> No.  The big difference is that the raw values are still exposed via
> control API (otherwise how do you read/write raw values?).  That is,
> any user-space program may break consistency.  OTOH, the kernel
> solution doesn't expose raw values.

Alsa-lib can filter raw writes from applications, but it's really not 
something I prefer, of course.

> > 2) create virtual things only in smixer
> > 
> >    - it's something I strongly prefer
> >    - if mixers uses CTL interface, it's not our bussiness because
> >      we haven't claimed to create virtual layers in CTL
> 
> Hm, I have to say it's a bad attitude.  It's our business to keep the
> whole system consistent.  We shouldn't build such a weak and fragile
> system any more.

But the definition of consistency is relative in this case. If we have 
universal control API (which is not a mixer API, but just API to pass 
runtime configuration parameters to driver) and second one API (simple 
mixer - fully abstract) then everything is consistent.

> And what about the kernel OSS mixer?  There are still many apps to
> handle it.

Yes, OSS mixer support still requires a rework. My idea is to create a 
helper userspace application which will start automatically by udev when 
/dev/mixer devices are created passing values through smixer API in user 
space.

For OSS PCM emulation, similar thing can be done using snd-aloop module.

> > > 3. Complicated configuration
> > > 
> > > The requirement of virtual master controls is very much dependent on
> > > the hardware.  In the case of HD-audio, it depends on codec chip
> > > types, and even on the preset model chosen via PCI SSID or a module
> > > option.  Implementing such a complex conditional in alsa-lib
> > > configuration is a clear overhead.
> > 
> > I don't agree much here. Of course, we might need some hints from the 
> > driver (using information about components in sndrv_ctl_card_info) or we 
> > might analyze available controls using their names if Master control is 
> > present or we can eventually create a configuration tool saving hints to 
> > alsa-lib's configuration files which controls should be added for given 
> > hardware (this configuration tool might create a list of useable PCM 
> > devices handled in the driver, too). Just idea.
> 
> But it *is* an overhead.

Maybe for this simple case. I would be really happy to see a system which 
allows me to split for example six channel hardware to three 
stereo soundcards with full abstract and independent mixers.

If we agree that it's the way to go, it does not make much sense to 
implement these things again in the kernel space, because virtual master
implemented there becomes unuseable.

> Note taht this isn't for solving every mixer problem.  It doesn't
> solve the world hunger.  But, it solves nicely the really annoying
> problem we face right now, without messy, fragile and overly complex
> implementation.

My point is that if we allow this now, we might have trouble to remove it 
in future when it becomes obsolete. It is much more difficult for the 
kernel code. We definitely need to settle a clear way about future mixer 
directions now and decide where the abstraction should be.

I'm sure I can hack smixer - simple_none.c - to add virtual master support 
in few days. If I fail for a reason or code is really ugly, I give you my 
ack for your implementation, ok?

						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [RFC] virtual master control for HD-audio

[Takashi Iwai]

At Thu, 10 Jan 2008 15:09:46 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 10 Jan 2008, Takashi Iwai wrote:
> 
> > At Thu, 10 Jan 2008 11:29:39 +0100 (CET),
> > Jaroslav Kysela wrote:
> > > 
> > > On Sat, 22 Dec 2007, Takashi Iwai wrote:
> > > 
> > > > At Fri, 21 Dec 2007 20:37:09 +0100 (CET),
> > > > Jaroslav Kysela wrote:
> > > > > 
> > > > > On Thu, 20 Dec 2007, Takashi Iwai wrote:
> > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > another thing I'd like to push into the next kernel is the virtual
> > > > > > master volume control.  As I posted in earlier posts, it adds virtual
> > > > > > controls that have several slave controls with the same types,
> > > > > > e.g. Front, Surround, Center, LFE, etc.  Then these are adjusted
> > > > > > simultaneously via the master control.
> > > > > > 
> > > > > > It'd be appreciated if some one can test the patches with HD-audio h/w
> > > > > > that has no master control yet (e.g. some STAC codecs).
> > > > > > 
> > > > > > Note that this won't add the master volumes if there are no real
> > > > > > volume controls.  Some codecs have really no volume control, and this
> > > > > > won't help for such devices.
> > > > > > 
> > > > > > Two (and one for driver) patches will follow after this.
> > > > > 
> > > > > NAK from my side. I am convinced that this code can be implemented in the 
> > > > > user space even without any daemon just in the mixer abstract layer using 
> > > > > standard control elements and using eventually new user controls to store 
> > > > > data for virtual mixer controls.
> > > > 
> > > > A user-space implementation of virtual mixer elements would be far
> > > > more complicated than the simplistic kernel-space patch.  I've
> > > > considered it many times, even tried to implement it, but got that
> > > > conclusion.  You'll see obviously the following difficulties:
> > > > 
> > > > 1. Many user-space virtual elements
> > > > 
> > > > Each slave control element needs a virtual element (eventually a
> > > > user-space one) because we need to keep both raw and virtual values to
> > > > handle saturation.  That is, the same number of additional controls
> > > > would be added.  Significant for 7.1 outputs.
> > > 
> > > Note that these "extra" values can be handled together using only one 
> > > user-space control element without "Volume/Switch" in name.
> > 
> > This method is very likely fragile.  Think what happens if the number
> > of elements changes, or if the element numid changes.  Then the packed
> > values will be screwed up.
> > 
> > 
> > > > 2. Easy incosistency between virtual and raw elements
> > > > 
> > > > Even if the mixer abstraction hides the virtual elements, both
> > > > virtual and raw elements are exposed on control API.  This can cause
> > > > the value-inconsistency between them quite easily, because many apps
> > > > access directly via control API (even some mixer apps), and they
> > > > likely change only raw values.  The similar situation is for kernel
> > > > OSS emulation.
> > > 
> > > I have basically two ideas to handle this.
> > > 
> > > 1) create a plugin system directly in the snd_ctl_* interface
> > > 
> > >    - basically code might be very same code as you proposed in kernel
> > 
> > No.  The big difference is that the raw values are still exposed via
> > control API (otherwise how do you read/write raw values?).  That is,
> > any user-space program may break consistency.  OTOH, the kernel
> > solution doesn't expose raw values.
> 
> Alsa-lib can filter raw writes from applications, but it's really not 
> something I prefer, of course.

Right.  It'd be really bad to do that.

> > > 2) create virtual things only in smixer
> > > 
> > >    - it's something I strongly prefer
> > >    - if mixers uses CTL interface, it's not our bussiness because
> > >      we haven't claimed to create virtual layers in CTL
> > 
> > Hm, I have to say it's a bad attitude.  It's our business to keep the
> > whole system consistent.  We shouldn't build such a weak and fragile
> > system any more.
> 
> But the definition of consistency is relative in this case. If we have 
> universal control API (which is not a mixer API, but just API to pass 
> runtime configuration parameters to driver) and second one API (simple 
> mixer - fully abstract) then everything is consistent.

What I meant is that this would create bogus values easily, that is,
screw up the volumes.  The kernel OSS can change the raw value while
mixer changes the value at the same time.

> > And what about the kernel OSS mixer?  There are still many apps to
> > handle it.
> 
> Yes, OSS mixer support still requires a rework. My idea is to create a 
> helper userspace application which will start automatically by udev when 
> /dev/mixer devices are created passing values through smixer API in user 
> space.
> 
> For OSS PCM emulation, similar thing can be done using snd-aloop module.

Yeah, that's good.  But this needs a lot of work and more resources.
And don't forget that many people hate daemons.

> > > > 3. Complicated configuration
> > > > 
> > > > The requirement of virtual master controls is very much dependent on
> > > > the hardware.  In the case of HD-audio, it depends on codec chip
> > > > types, and even on the preset model chosen via PCI SSID or a module
> > > > option.  Implementing such a complex conditional in alsa-lib
> > > > configuration is a clear overhead.
> > > 
> > > I don't agree much here. Of course, we might need some hints from the 
> > > driver (using information about components in sndrv_ctl_card_info) or we 
> > > might analyze available controls using their names if Master control is 
> > > present or we can eventually create a configuration tool saving hints to 
> > > alsa-lib's configuration files which controls should be added for given 
> > > hardware (this configuration tool might create a list of useable PCM 
> > > devices handled in the driver, too). Just idea.
> > 
> > But it *is* an overhead.
> 
> Maybe for this simple case.

Exactly, that's the point.  This case is DAMN SIMPLE.

The virtual master patch simply fills the hole in the driver.  It
should have been there, but I didn't put it.  That's my fault.

It has nothing to do with the virtual cards or complicated layering.
It just adds a control for binding several registers.  We don't need
too much work for this simple purpose.

> I would be really happy to see a system which 
> allows me to split for example six channel hardware to three 
> stereo soundcards with full abstract and independent mixers.

I'm afraid this would result in a similar situation like PCM plugins.
Without a proper setup utility, the whole thing could be too
complicated to use in practice...

> If we agree that it's the way to go, it does not make much sense to 
> implement these things again in the kernel space, because virtual master
> implemented there becomes unuseable.

The virtual master in the kernel doesn't conflict with the user-space
additions.  It's simply a master volume in the end, nothing
different from the real master volume on hardware.  So, it's fine to
have both.

> > Note taht this isn't for solving every mixer problem.  It doesn't
> > solve the world hunger.  But, it solves nicely the really annoying
> > problem we face right now, without messy, fragile and overly complex
> > implementation.
> 
> My point is that if we allow this now, we might have trouble to remove it 
> in future when it becomes obsolete. It is much more difficult for the 
> kernel code. We definitely need to settle a clear way about future mixer 
> directions now and decide where the abstraction should be.

It doesn't have to be obsoleted.  The user-space solution requires
more resources for the same purpose.  Then why do we need to switch?
And above all, both can coexist peacefully.

> I'm sure I can hack smixer - simple_none.c - to add virtual master support 
> in few days. If I fail for a reason or code is really ugly, I give you my 
> ack for your implementation, ok?

Well, as I wrote, smixer alone won't solve all the corner cases.
Imlementing smixer would be relatively easy.  But it can't be solid
like the kernel driver, and we have still problems with control API
and kernel mixer OSS.


Takashi

Re: [RFC] virtual master control for HD-audio

[Jaroslav Kysela]

On Thu, 10 Jan 2008, Takashi Iwai wrote:

> > I'm sure I can hack smixer - simple_none.c - to add virtual master support 
> > in few days. If I fail for a reason or code is really ugly, I give you my 
> > ack for your implementation, ok?
> 
> Well, as I wrote, smixer alone won't solve all the corner cases.
> Imlementing smixer would be relatively easy.  But it can't be solid
> like the kernel driver, and we have still problems with control API
> and kernel mixer OSS.

Ok, I switch to rather neutral mode in this issue. Although I don't think 
that it's the right solution. You can commit this code to repo. Time shows 
us if it was good or not. Back to coding.

					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [RFC] virtual master control for HD-audio

[Takashi Iwai]

At Thu, 10 Jan 2008 16:43:24 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 10 Jan 2008, Takashi Iwai wrote:
> 
> > > I'm sure I can hack smixer - simple_none.c - to add virtual master support 
> > > in few days. If I fail for a reason or code is really ugly, I give you my 
> > > ack for your implementation, ok?
> > 
> > Well, as I wrote, smixer alone won't solve all the corner cases.
> > Imlementing smixer would be relatively easy.  But it can't be solid
> > like the kernel driver, and we have still problems with control API
> > and kernel mixer OSS.
> 
> Ok, I switch to rather neutral mode in this issue. Although I don't think 
> that it's the right solution. You can commit this code to repo. Time shows 
> us if it was good or not. Back to coding.

Glad to hear that.

The user-space solution would be of course nice, but in this
particular case, it results in far more codes and may give even more
problems.  Once after we get a better infrastructure for mixer
abstraction, let's clean up the whole stuff.


Takashi

Re: [RFC] [Patch 2/2] Add virtual master to HD-audio

[Daniel Jacobowitz]

On Thu, Dec 20, 2007 at 04:46:35PM +0100, Takashi Iwai wrote:
>  static struct snd_kcontrol_new alc880_lg_mixer[] = {
>  	/* FIXME: it's not really "master" but front channels */
> -	HDA_CODEC_VOLUME("Master Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
> -	HDA_BIND_MUTE("Master Playback Switch", 0x0f, 2, HDA_INPUT),
> +	HDA_CODEC_VOLUME("Front Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
> +	HDA_BIND_MUTE("Front Playback Switch", 0x0f, 2, HDA_INPUT),

You've just fixed that FIXME :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 2/2] Add virtual master to HD-audio

[Takashi Iwai]

At Sat, 12 Jan 2008 15:21:22 -0500,
Daniel Jacobowitz wrote:
> 
> On Thu, Dec 20, 2007 at 04:46:35PM +0100, Takashi Iwai wrote:
> >  static struct snd_kcontrol_new alc880_lg_mixer[] = {
> >  	/* FIXME: it's not really "master" but front channels */
> > -	HDA_CODEC_VOLUME("Master Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
> > -	HDA_BIND_MUTE("Master Playback Switch", 0x0f, 2, HDA_INPUT),
> > +	HDA_CODEC_VOLUME("Front Playback Volume", 0x0f, 0x0, HDA_OUTPUT),
> > +	HDA_BIND_MUTE("Front Playback Switch", 0x0f, 2, HDA_INPUT),
> 
> You've just fixed that FIXME :-)

Yeah, fixed FIXME now ;)


Takashi

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Thu, Dec 20, 2007 at 04:45:17PM +0100, Takashi Iwai wrote:
> This patch adds the virtual master control to sound core.
> 
> 
> Takashi
> 
> diff -r 1207d46e60b3 core/Kconfig
> --- a/core/Kconfig	Mon Dec 03 15:15:59 2007 +0100
> +++ b/core/Kconfig	Mon Dec 03 15:46:17 2007 +0100
> @@ -15,6 +15,9 @@ config SND_RAWMIDI
>  config SND_RAWMIDI
>  	tristate
>  	depends on SND
> +
> +config SND_VMASTER
> +	bool
>  
>  config SND_SEQUENCER
>  	tristate "Sequencer support"

When you committed this to hg, you put vmaster.c into hda instead of
core - could you explain why?  I'd like to use it with ice1724.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

At Wed, 30 Jan 2008 10:19:10 -0500,
Daniel Jacobowitz wrote:
> 
> On Thu, Dec 20, 2007 at 04:45:17PM +0100, Takashi Iwai wrote:
> > This patch adds the virtual master control to sound core.
> > 
> > 
> > Takashi
> > 
> > diff -r 1207d46e60b3 core/Kconfig
> > --- a/core/Kconfig	Mon Dec 03 15:15:59 2007 +0100
> > +++ b/core/Kconfig	Mon Dec 03 15:46:17 2007 +0100
> > @@ -15,6 +15,9 @@ config SND_RAWMIDI
> >  config SND_RAWMIDI
> >  	tristate
> >  	depends on SND
> > +
> > +config SND_VMASTER
> > +	bool
> >  
> >  config SND_SEQUENCER
> >  	tristate "Sequencer support"
> 
> When you committed this to hg, you put vmaster.c into hda instead of
> core - could you explain why?  I'd like to use it with ice1724.

Because HD-audio is the only user, so far.  I don't want to expand the
core stuff (and kconfig) unless really needed.


Takashi

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Wed, Jan 30, 2008 at 04:27:07PM +0100, Takashi Iwai wrote:
> Because HD-audio is the only user, so far.  I don't want to expand the
> core stuff (and kconfig) unless really needed.

I said I'd want this for ice1724 / vt1616, and I finally got time to
do it.  What do you think of this patch?  I'm happily using it.

From: Daniel Jacobowitz <dan@codesourcery.com>

Move VMASTER from HDA out to core, and enable it for VT1616.

Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>

diff --git a/include/sound/control.h b/include/sound/control.h
index e79baa6..3dc1291 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -169,4 +169,11 @@ int snd_ctl_boolean_mono_info(struct snd_kcontrol *kcontrol,
 int snd_ctl_boolean_stereo_info(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_info *uinfo);
 
+/*
+ * virtual master control
+ */
+struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
+						 const unsigned int *tlv);
+int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave);
+		      
 #endif	/* __SOUND_CONTROL_H */
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 829ca38..b0528c4 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -16,6 +16,9 @@ config SND_RAWMIDI
 	tristate
 	depends on SND
 
+config SND_VMASTER
+       bool
+
 config SND_SEQUENCER
 	tristate "Sequencer support"
 	depends on SND
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 267039a..da8e685 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -6,6 +6,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-pcm-objs := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
 		pcm_memory.o
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
new file mode 100644
index 0000000..7cfd8b8
--- /dev/null
+++ b/sound/core/vmaster.c
@@ -0,0 +1,368 @@
+/*
+ * Virtual master and slave controls
+ *
+ *  Copyright (c) 2008 by 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, version 2.
+ *
+ */
+
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/control.h>
+
+/*
+ * a subset of information returned via ctl info callback
+ */
+struct link_ctl_info {
+	int type;		/* value type */
+	int count;		/* item count */
+	int min_val, max_val;	/* min, max values */
+};
+
+/*
+ * link master - this contains a list of slave controls that are
+ * identical types, i.e. info returns the same value type and value
+ * ranges, but may have different number of counts.
+ *
+ * The master control is so far only mono volume/switch for simplicity.
+ * The same value will be applied to all slaves.
+ */
+struct link_master {
+	struct list_head slaves;
+	struct link_ctl_info info;
+	int val;		/* the master value */
+};
+
+/*
+ * link slave - this contains a slave control element
+ *
+ * It fakes the control callbacsk with additional attenuation by the
+ * master control.  A slave may have either one or two channels.
+ */
+
+struct link_slave {
+	struct list_head list;
+	struct link_master *master;
+	struct link_ctl_info info;
+	int vals[2];		/* current values */
+	struct snd_kcontrol slave; /* the copy of original control entry */
+};
+
+/* get the slave ctl info and save the initial values */
+static int slave_init(struct link_slave *slave)
+{
+	struct snd_ctl_elem_info *uinfo;
+	struct snd_ctl_elem_value *uctl;
+	int err, ch;
+
+	if (slave->info.count)
+		return 0; /* already initialized */
+
+	uinfo = kmalloc(sizeof(*uinfo), GFP_KERNEL);
+	if (!uinfo)
+		return -ENOMEM;
+	uinfo->id = slave->slave.id;
+	err = slave->slave.info(&slave->slave, uinfo);
+	if (err < 0) {
+		kfree(uinfo);
+		return err;
+	}
+	slave->info.type = uinfo->type;
+	slave->info.count = uinfo->count;
+	if (slave->info.count > 2  ||
+	    (slave->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER &&
+	     slave->info.type != SNDRV_CTL_ELEM_TYPE_BOOLEAN)) {
+		snd_printk(KERN_ERR "invalid slave element\n");
+		kfree(uinfo);
+		return -EINVAL;
+	}
+	slave->info.min_val = uinfo->value.integer.min;
+	slave->info.max_val = uinfo->value.integer.max;
+	kfree(uinfo);
+
+	uctl = kmalloc(sizeof(*uctl), GFP_KERNEL);
+	if (!uctl)
+		return -ENOMEM;
+	uctl->id = slave->slave.id;
+	err = slave->slave.get(&slave->slave, uctl);
+	for (ch = 0; ch < slave->info.count; ch++)
+		slave->vals[ch] = uctl->value.integer.value[ch];
+	kfree(uctl);
+	return 0;
+}
+
+/* initialize master volume */
+static int master_init(struct link_master *master)
+{
+	struct link_slave *slave;
+
+	if (master->info.count)
+		return 0; /* already initialized */
+
+	list_for_each_entry(slave, &master->slaves, list) {
+		int err = slave_init(slave);
+		if (err < 0)
+			return err;
+		master->info = slave->info;
+		master->info.count = 1; /* always mono */
+		/* set full volume as default (= no attenuation) */
+		master->val = master->info.max_val;
+		return 0;
+	}
+	return -ENOENT;
+}
+
+static int slave_get_val(struct link_slave *slave,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	int err, ch;
+
+	err = slave_init(slave);
+	if (err < 0)
+		return err;
+	for (ch = 0; ch < slave->info.count; ch++)
+		ucontrol->value.integer.value[ch] = slave->vals[ch];
+	return 0;
+}
+
+static int slave_put_val(struct link_slave *slave,
+			 struct snd_ctl_elem_value *ucontrol)
+{
+	int err, ch, vol;
+
+	err = master_init(slave->master);
+	if (err < 0)
+		return err;
+
+	switch (slave->info.type) {
+	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+		for (ch = 0; ch < slave->info.count; ch++)
+			ucontrol->value.integer.value[ch] &=
+				!!slave->master->val;
+		break;
+	case SNDRV_CTL_ELEM_TYPE_INTEGER:
+		for (ch = 0; ch < slave->info.count; ch++) {
+			/* max master volume is supposed to be 0 dB */
+			vol = ucontrol->value.integer.value[ch];
+			vol += slave->master->val - slave->master->info.max_val;
+			if (vol < slave->info.min_val)
+				vol = slave->info.min_val;
+			else if (vol > slave->info.max_val)
+				vol = slave->info.max_val;
+			ucontrol->value.integer.value[ch] = vol;
+		}
+		break;
+	}
+	return slave->slave.put(&slave->slave, ucontrol);
+}
+
+/*
+ * ctl callbacks for slaves
+ */
+static int slave_info(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_info *uinfo)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	return slave->slave.info(&slave->slave, uinfo);
+}
+
+static int slave_get(struct snd_kcontrol *kcontrol,
+		     struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	return slave_get_val(slave, ucontrol);
+}
+
+static int slave_put(struct snd_kcontrol *kcontrol,
+		     struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	int err, ch, changed = 0;
+
+	err = slave_init(slave);
+	if (err < 0)
+		return err;
+	for (ch = 0; ch < slave->info.count; ch++) {
+		if (slave->vals[ch] != ucontrol->value.integer.value[ch]) {
+			changed = 1;
+			slave->vals[ch] = ucontrol->value.integer.value[ch];
+		}
+	}
+	if (!changed)
+		return 0;
+	return slave_put_val(slave, ucontrol);
+}
+
+static int slave_tlv_cmd(struct snd_kcontrol *kcontrol,
+			 int op_flag, unsigned int size,
+			 unsigned int __user *tlv)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	/* FIXME: this assumes that the max volume is 0 dB */
+	return slave->slave.tlv.c(&slave->slave, op_flag, size, tlv);
+}
+
+static void slave_free(struct snd_kcontrol *kcontrol)
+{
+	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
+	if (slave->slave.private_free)
+		slave->slave.private_free(&slave->slave);
+	if (slave->master)
+		list_del(&slave->list);
+	kfree(slave);
+}
+
+/*
+ * Add a slave control to the group with the given master control
+ *
+ * All slaves must be the same type (returning the same information
+ * via info callback).  The fucntion doesn't check it, so it's your
+ * responsibility.
+ *
+ * Also, some additional limitations:
+ * - at most two channels
+ * - logarithmic volume control (dB level), no linear volume
+ * - master can only attenuate the volume, no gain
+ */
+int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave)
+{
+	struct link_master *master_link = snd_kcontrol_chip(master);
+	struct link_slave *srec;
+
+	srec = kzalloc(sizeof(*srec) +
+		       slave->count * sizeof(*slave->vd), GFP_KERNEL);
+	if (!srec)
+		return -ENOMEM;
+	srec->slave = *slave;
+	memcpy(srec->slave.vd, slave->vd, slave->count * sizeof(*slave->vd));
+	srec->master = master_link;
+
+	/* override callbacks */
+	slave->info = slave_info;
+	slave->get = slave_get;
+	slave->put = slave_put;
+	if (slave->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK)
+		slave->tlv.c = slave_tlv_cmd;
+	slave->private_data = srec;
+	slave->private_free = slave_free;
+
+	list_add_tail(&srec->list, &master_link->slaves);
+	return 0;
+}
+
+EXPORT_SYMBOL(snd_ctl_add_slave);
+
+/*
+ * ctl callbacks for master controls
+ */
+static int master_info(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_info *uinfo)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	int ret;
+
+	ret = master_init(master);
+	if (ret < 0)
+		return ret;
+	uinfo->type = master->info.type;
+	uinfo->count = master->info.count;
+	uinfo->value.integer.min = master->info.min_val;
+	uinfo->value.integer.max = master->info.max_val;
+	return 0;
+}
+
+static int master_get(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	int err = master_init(master);
+	if (err < 0)
+		return err;
+	ucontrol->value.integer.value[0] = master->val;
+	return 0;
+}
+
+static int master_put(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	struct link_slave *slave;
+	struct snd_ctl_elem_value *uval;
+	int err, old_val;
+
+	err = master_init(master);
+	if (err < 0)
+		return err;
+	old_val = master->val;
+	if (ucontrol->value.integer.value[0] == old_val)
+		return 0;
+
+	uval = kmalloc(sizeof(*uval), GFP_KERNEL);
+	if (!uval)
+		return -ENOMEM;
+	list_for_each_entry(slave, &master->slaves, list) {
+		master->val = old_val;
+		uval->id = slave->slave.id;
+		slave_get_val(slave, uval);
+		master->val = ucontrol->value.integer.value[0];
+		slave_put_val(slave, uval);
+	}
+	kfree(uval);
+	return 1;
+}
+
+static void master_free(struct snd_kcontrol *kcontrol)
+{
+	struct link_master *master = snd_kcontrol_chip(kcontrol);
+	struct link_slave *slave;
+
+	list_for_each_entry(slave, &master->slaves, list)
+		slave->master = NULL;
+	kfree(master);
+}
+
+
+/*
+ * Create a virtual master control with the given name
+ */
+struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
+						 const unsigned int *tlv)
+{
+	struct link_master *master;
+	struct snd_kcontrol *kctl;
+	struct snd_kcontrol_new knew;
+
+	memset(&knew, 0, sizeof(knew));
+	knew.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	knew.name = name;
+	knew.info = master_info;
+
+	master = kzalloc(sizeof(*master), GFP_KERNEL);
+	if (!master)
+		return NULL;
+	INIT_LIST_HEAD(&master->slaves);
+
+	kctl = snd_ctl_new1(&knew, master);
+	if (!kctl) {
+		kfree(master);
+		return NULL;
+	}
+	/* override some callbacks */
+	kctl->info = master_info;
+	kctl->get = master_get;
+	kctl->put = master_put;
+	kctl->private_free = master_free;
+
+	/* additional (constant) TLV read */
+	if (tlv) {
+		/* FIXME: this assumes that the max volume is 0 dB */
+		kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ;
+		kctl->tlv.p = tlv;
+	}
+	return kctl;
+}
+
+EXPORT_SYMBOL(snd_ctl_make_virtual_master);
diff --git a/sound/drivers/Kconfig b/sound/drivers/Kconfig
index 75d4fe0..154e0ae 100644
--- a/sound/drivers/Kconfig
+++ b/sound/drivers/Kconfig
@@ -27,6 +27,7 @@ config SND_AC97_CODEC
 	tristate
 	select SND_PCM
 	select AC97_BUS
+	select SND_VMASTER
 
 config SND_DUMMY
 	tristate "Dummy (/dev/null) soundcard"
diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 812085d..48296d9 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -517,6 +517,7 @@ config SND_HDA_INTEL
 	tristate "Intel HD Audio"
 	depends on SND
 	select SND_PCM
+	select SND_VMASTER
 	help
 	  Say Y here to include support for Intel "High Definition
 	  Audio" (Azalia) motherboard devices.
diff --git a/sound/pci/ac97/ac97_patch.c b/sound/pci/ac97/ac97_patch.c
index 50c637e..2019602 100644
--- a/sound/pci/ac97/ac97_patch.c
+++ b/sound/pci/ac97/ac97_patch.c
@@ -3312,8 +3312,66 @@ AC97_SINGLE("Downmix LFE and Center to Front", 0x5a, 12, 1, 0),
 AC97_SINGLE("Downmix Surround to Front", 0x5a, 11, 1, 0),
 };
 
+static const char *slave_vols_vt1616[] = {
+	"Front Playback Volume",
+	"Surround Playback Volume",
+	"Center Playback Volume",
+	"LFE Playback Volume",
+	NULL
+};
+
+static const char *slave_sws_vt1616[] = {
+	"Front Playback Switch",
+	"Surround Playback Switch",
+	"Center Playback Switch",
+	"LFE Playback Switch",
+	NULL
+};
+
+/* find a mixer control element with the given name */
+static struct snd_kcontrol *snd_ac97_find_mixer_ctl(struct snd_ac97 *ac97,
+						    const char *name)
+{
+	struct snd_ctl_elem_id id;
+	memset(&id, 0, sizeof(id));
+	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	strcpy(id.name, name);
+	return snd_ctl_find_id(ac97->bus->card, &id);
+}
+
+/* create a virtual master control and add slaves */
+int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
+			 const unsigned int *tlv, const char **slaves)
+{
+	struct snd_kcontrol *kctl;
+	const char **s;
+	int err;
+
+	kctl = snd_ctl_make_virtual_master(name, tlv);
+	if (!kctl)
+		return -ENOMEM;
+	err = snd_ctl_add(ac97->bus->card, kctl);
+	if (err < 0)
+		return err;
+	
+	for (s = slaves; *s; s++) {
+		struct snd_kcontrol *sctl;
+
+		sctl = snd_ac97_find_mixer_ctl(ac97, *s);
+		if (!sctl) {
+			snd_printdd("Cannot find slave %s, skipped\n", *s);
+			continue;
+		}
+		err = snd_ctl_add_slave(kctl, sctl);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+
 static int patch_vt1616_specific(struct snd_ac97 * ac97)
 {
+	struct snd_kcontrol *kctl;
 	int err;
 
 	if (snd_ac97_try_bit(ac97, 0x5a, 9))
@@ -3321,6 +3379,24 @@ static int patch_vt1616_specific(struct snd_ac97 * ac97)
 			return err;
 	if ((err = patch_build_controls(ac97, &snd_ac97_controls_vt1616[1], ARRAY_SIZE(snd_ac97_controls_vt1616) - 1)) < 0)
 		return err;
+
+	/* There is already a misnamed master switch.  Rename it.  */
+	kctl = snd_ac97_find_mixer_ctl(ac97, "Master Playback Volume");
+	if (!kctl)
+		return -EINVAL;
+
+	snd_ac97_rename_vol_ctl(ac97, "Master Playback", "Front Playback");
+
+	err = snd_ac97_add_vmaster(ac97, "Master Playback Volume",
+				   kctl->tlv.p, slave_vols_vt1616);
+	if (err < 0)
+		return err;
+
+	err = snd_ac97_add_vmaster(ac97, "Master Playback Switch",
+				   NULL, slave_sws_vt1616);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index 9e0d8a1..ab0c726 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -2,7 +2,7 @@ snd-hda-intel-y := hda_intel.o
 # since snd-hda-intel is the only driver using hda-codec,
 # merge it into a single module although it was originally
 # designed to be individual modules
-snd-hda-intel-y += hda_codec.o vmaster.o
+snd-hda-intel-y += hda_codec.o
 snd-hda-intel-$(CONFIG_PROC_FS) += hda_proc.o
 snd-hda-intel-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o
 snd-hda-intel-$(CONFIG_SND_HDA_GENERIC) += hda_generic.o
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
index ad0014a..5a859d8 100644
--- a/sound/pci/hda/hda_local.h
+++ b/sound/pci/hda/hda_local.h
@@ -407,11 +407,4 @@ int snd_hda_check_amp_list_power(struct hda_codec *codec,
 				 hda_nid_t nid);
 #endif /* CONFIG_SND_HDA_POWER_SAVE */
 
-/*
- * virtual master control
- */
-struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
-						 const unsigned int *tlv);
-int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave);
-		      
 #endif /* __SOUND_HDA_LOCAL_H */
diff --git a/sound/pci/hda/vmaster.c b/sound/pci/hda/vmaster.c
deleted file mode 100644
index 2da49d2..0000000
--- a/sound/pci/hda/vmaster.c
+++ /dev/null
@@ -1,364 +0,0 @@
-/*
- * Virtual master and slave controls
- *
- *  Copyright (c) 2008 by 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, version 2.
- *
- */
-
-#include <linux/slab.h>
-#include <sound/core.h>
-#include <sound/control.h>
-
-/*
- * a subset of information returned via ctl info callback
- */
-struct link_ctl_info {
-	int type;		/* value type */
-	int count;		/* item count */
-	int min_val, max_val;	/* min, max values */
-};
-
-/*
- * link master - this contains a list of slave controls that are
- * identical types, i.e. info returns the same value type and value
- * ranges, but may have different number of counts.
- *
- * The master control is so far only mono volume/switch for simplicity.
- * The same value will be applied to all slaves.
- */
-struct link_master {
-	struct list_head slaves;
-	struct link_ctl_info info;
-	int val;		/* the master value */
-};
-
-/*
- * link slave - this contains a slave control element
- *
- * It fakes the control callbacsk with additional attenuation by the
- * master control.  A slave may have either one or two channels.
- */
-
-struct link_slave {
-	struct list_head list;
-	struct link_master *master;
-	struct link_ctl_info info;
-	int vals[2];		/* current values */
-	struct snd_kcontrol slave; /* the copy of original control entry */
-};
-
-/* get the slave ctl info and save the initial values */
-static int slave_init(struct link_slave *slave)
-{
-	struct snd_ctl_elem_info *uinfo;
-	struct snd_ctl_elem_value *uctl;
-	int err, ch;
-
-	if (slave->info.count)
-		return 0; /* already initialized */
-
-	uinfo = kmalloc(sizeof(*uinfo), GFP_KERNEL);
-	if (!uinfo)
-		return -ENOMEM;
-	uinfo->id = slave->slave.id;
-	err = slave->slave.info(&slave->slave, uinfo);
-	if (err < 0) {
-		kfree(uinfo);
-		return err;
-	}
-	slave->info.type = uinfo->type;
-	slave->info.count = uinfo->count;
-	if (slave->info.count > 2  ||
-	    (slave->info.type != SNDRV_CTL_ELEM_TYPE_INTEGER &&
-	     slave->info.type != SNDRV_CTL_ELEM_TYPE_BOOLEAN)) {
-		snd_printk(KERN_ERR "invalid slave element\n");
-		kfree(uinfo);
-		return -EINVAL;
-	}
-	slave->info.min_val = uinfo->value.integer.min;
-	slave->info.max_val = uinfo->value.integer.max;
-	kfree(uinfo);
-
-	uctl = kmalloc(sizeof(*uctl), GFP_KERNEL);
-	if (!uctl)
-		return -ENOMEM;
-	uctl->id = slave->slave.id;
-	err = slave->slave.get(&slave->slave, uctl);
-	for (ch = 0; ch < slave->info.count; ch++)
-		slave->vals[ch] = uctl->value.integer.value[ch];
-	kfree(uctl);
-	return 0;
-}
-
-/* initialize master volume */
-static int master_init(struct link_master *master)
-{
-	struct link_slave *slave;
-
-	if (master->info.count)
-		return 0; /* already initialized */
-
-	list_for_each_entry(slave, &master->slaves, list) {
-		int err = slave_init(slave);
-		if (err < 0)
-			return err;
-		master->info = slave->info;
-		master->info.count = 1; /* always mono */
-		/* set full volume as default (= no attenuation) */
-		master->val = master->info.max_val;
-		return 0;
-	}
-	return -ENOENT;
-}
-
-static int slave_get_val(struct link_slave *slave,
-			 struct snd_ctl_elem_value *ucontrol)
-{
-	int err, ch;
-
-	err = slave_init(slave);
-	if (err < 0)
-		return err;
-	for (ch = 0; ch < slave->info.count; ch++)
-		ucontrol->value.integer.value[ch] = slave->vals[ch];
-	return 0;
-}
-
-static int slave_put_val(struct link_slave *slave,
-			 struct snd_ctl_elem_value *ucontrol)
-{
-	int err, ch, vol;
-
-	err = master_init(slave->master);
-	if (err < 0)
-		return err;
-
-	switch (slave->info.type) {
-	case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
-		for (ch = 0; ch < slave->info.count; ch++)
-			ucontrol->value.integer.value[ch] &=
-				!!slave->master->val;
-		break;
-	case SNDRV_CTL_ELEM_TYPE_INTEGER:
-		for (ch = 0; ch < slave->info.count; ch++) {
-			/* max master volume is supposed to be 0 dB */
-			vol = ucontrol->value.integer.value[ch];
-			vol += slave->master->val - slave->master->info.max_val;
-			if (vol < slave->info.min_val)
-				vol = slave->info.min_val;
-			else if (vol > slave->info.max_val)
-				vol = slave->info.max_val;
-			ucontrol->value.integer.value[ch] = vol;
-		}
-		break;
-	}
-	return slave->slave.put(&slave->slave, ucontrol);
-}
-
-/*
- * ctl callbacks for slaves
- */
-static int slave_info(struct snd_kcontrol *kcontrol,
-		      struct snd_ctl_elem_info *uinfo)
-{
-	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
-	return slave->slave.info(&slave->slave, uinfo);
-}
-
-static int slave_get(struct snd_kcontrol *kcontrol,
-		     struct snd_ctl_elem_value *ucontrol)
-{
-	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
-	return slave_get_val(slave, ucontrol);
-}
-
-static int slave_put(struct snd_kcontrol *kcontrol,
-		     struct snd_ctl_elem_value *ucontrol)
-{
-	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
-	int err, ch, changed = 0;
-
-	err = slave_init(slave);
-	if (err < 0)
-		return err;
-	for (ch = 0; ch < slave->info.count; ch++) {
-		if (slave->vals[ch] != ucontrol->value.integer.value[ch]) {
-			changed = 1;
-			slave->vals[ch] = ucontrol->value.integer.value[ch];
-		}
-	}
-	if (!changed)
-		return 0;
-	return slave_put_val(slave, ucontrol);
-}
-
-static int slave_tlv_cmd(struct snd_kcontrol *kcontrol,
-			 int op_flag, unsigned int size,
-			 unsigned int __user *tlv)
-{
-	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
-	/* FIXME: this assumes that the max volume is 0 dB */
-	return slave->slave.tlv.c(&slave->slave, op_flag, size, tlv);
-}
-
-static void slave_free(struct snd_kcontrol *kcontrol)
-{
-	struct link_slave *slave = snd_kcontrol_chip(kcontrol);
-	if (slave->slave.private_free)
-		slave->slave.private_free(&slave->slave);
-	if (slave->master)
-		list_del(&slave->list);
-	kfree(slave);
-}
-
-/*
- * Add a slave control to the group with the given master control
- *
- * All slaves must be the same type (returning the same information
- * via info callback).  The fucntion doesn't check it, so it's your
- * responsibility.
- *
- * Also, some additional limitations:
- * - at most two channels
- * - logarithmic volume control (dB level), no linear volume
- * - master can only attenuate the volume, no gain
- */
-int snd_ctl_add_slave(struct snd_kcontrol *master, struct snd_kcontrol *slave)
-{
-	struct link_master *master_link = snd_kcontrol_chip(master);
-	struct link_slave *srec;
-
-	srec = kzalloc(sizeof(*srec) +
-		       slave->count * sizeof(*slave->vd), GFP_KERNEL);
-	if (!srec)
-		return -ENOMEM;
-	srec->slave = *slave;
-	memcpy(srec->slave.vd, slave->vd, slave->count * sizeof(*slave->vd));
-	srec->master = master_link;
-
-	/* override callbacks */
-	slave->info = slave_info;
-	slave->get = slave_get;
-	slave->put = slave_put;
-	if (slave->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK)
-		slave->tlv.c = slave_tlv_cmd;
-	slave->private_data = srec;
-	slave->private_free = slave_free;
-
-	list_add_tail(&srec->list, &master_link->slaves);
-	return 0;
-}
-
-/*
- * ctl callbacks for master controls
- */
-static int master_info(struct snd_kcontrol *kcontrol,
-		      struct snd_ctl_elem_info *uinfo)
-{
-	struct link_master *master = snd_kcontrol_chip(kcontrol);
-	int ret;
-
-	ret = master_init(master);
-	if (ret < 0)
-		return ret;
-	uinfo->type = master->info.type;
-	uinfo->count = master->info.count;
-	uinfo->value.integer.min = master->info.min_val;
-	uinfo->value.integer.max = master->info.max_val;
-	return 0;
-}
-
-static int master_get(struct snd_kcontrol *kcontrol,
-		      struct snd_ctl_elem_value *ucontrol)
-{
-	struct link_master *master = snd_kcontrol_chip(kcontrol);
-	int err = master_init(master);
-	if (err < 0)
-		return err;
-	ucontrol->value.integer.value[0] = master->val;
-	return 0;
-}
-
-static int master_put(struct snd_kcontrol *kcontrol,
-		      struct snd_ctl_elem_value *ucontrol)
-{
-	struct link_master *master = snd_kcontrol_chip(kcontrol);
-	struct link_slave *slave;
-	struct snd_ctl_elem_value *uval;
-	int err, old_val;
-
-	err = master_init(master);
-	if (err < 0)
-		return err;
-	old_val = master->val;
-	if (ucontrol->value.integer.value[0] == old_val)
-		return 0;
-
-	uval = kmalloc(sizeof(*uval), GFP_KERNEL);
-	if (!uval)
-		return -ENOMEM;
-	list_for_each_entry(slave, &master->slaves, list) {
-		master->val = old_val;
-		uval->id = slave->slave.id;
-		slave_get_val(slave, uval);
-		master->val = ucontrol->value.integer.value[0];
-		slave_put_val(slave, uval);
-	}
-	kfree(uval);
-	return 1;
-}
-
-static void master_free(struct snd_kcontrol *kcontrol)
-{
-	struct link_master *master = snd_kcontrol_chip(kcontrol);
-	struct link_slave *slave;
-
-	list_for_each_entry(slave, &master->slaves, list)
-		slave->master = NULL;
-	kfree(master);
-}
-
-
-/*
- * Create a virtual master control with the given name
- */
-struct snd_kcontrol *snd_ctl_make_virtual_master(char *name,
-						 const unsigned int *tlv)
-{
-	struct link_master *master;
-	struct snd_kcontrol *kctl;
-	struct snd_kcontrol_new knew;
-
-	memset(&knew, 0, sizeof(knew));
-	knew.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
-	knew.name = name;
-	knew.info = master_info;
-
-	master = kzalloc(sizeof(*master), GFP_KERNEL);
-	if (!master)
-		return NULL;
-	INIT_LIST_HEAD(&master->slaves);
-
-	kctl = snd_ctl_new1(&knew, master);
-	if (!kctl) {
-		kfree(master);
-		return NULL;
-	}
-	/* override some callbacks */
-	kctl->info = master_info;
-	kctl->get = master_get;
-	kctl->put = master_put;
-	kctl->private_free = master_free;
-
-	/* additional (constant) TLV read */
-	if (tlv) {
-		/* FIXME: this assumes that the max volume is 0 dB */
-		kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ;
-		kctl->tlv.p = tlv;
-	}
-	return kctl;
-}

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

At Fri, 21 Mar 2008 15:08:55 -0400,
Daniel Jacobowitz wrote:
> 
> On Wed, Jan 30, 2008 at 04:27:07PM +0100, Takashi Iwai wrote:
> > Because HD-audio is the only user, so far.  I don't want to expand the
> > core stuff (and kconfig) unless really needed.
> 
> I said I'd want this for ice1724 / vt1616, and I finally got time to
> do it.  What do you think of this patch?  I'm happily using it.

Well, don't forget to check HG version before you submit a patch :)


Takashi

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Sat, Mar 22, 2008 at 10:09:23AM +0100, Takashi Iwai wrote:
> At Fri, 21 Mar 2008 15:08:55 -0400,
> Daniel Jacobowitz wrote:
> > 
> > On Wed, Jan 30, 2008 at 04:27:07PM +0100, Takashi Iwai wrote:
> > > Because HD-audio is the only user, so far.  I don't want to expand the
> > > core stuff (and kconfig) unless really needed.
> > 
> > I said I'd want this for ice1724 / vt1616, and I finally got time to
> > do it.  What do you think of this patch?  I'm happily using it.
> 
> Well, don't forget to check HG version before you submit a patch :)

Whoops!  This version works with the hg tree.  The new
snd_ac97_add_vmaster is mostly similar to the hda version so it might
make sense to put that in vmaster.c instead.

From: Daniel Jacobowitz <dan@codesourcery.com>

Enable VMASTER for VT1616 / VT1617A.

Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>

diff -r 0d5f43585ca7 drivers/Kconfig
--- a/drivers/Kconfig	Sat Mar 22 10:26:05 2008 +0100
+++ b/drivers/Kconfig	Sat Mar 22 11:02:16 2008 -0400
@@ -44,6 +44,7 @@ config SND_AC97_CODEC
 	tristate
 	select SND_PCM
 	select AC97_BUS
+	select SND_VMASTER
 
 config SND_DUMMY
 	tristate "Dummy (/dev/null) soundcard"
diff -r 0d5f43585ca7 pci/ac97/ac97_patch.c
--- a/pci/ac97/ac97_patch.c	Sat Mar 22 10:26:05 2008 +0100
+++ b/pci/ac97/ac97_patch.c	Sat Mar 22 11:02:16 2008 -0400
@@ -3332,8 +3332,66 @@ AC97_SINGLE("Downmix Surround to Front",
 AC97_SINGLE("Downmix Surround to Front", 0x5a, 11, 1, 0),
 };
 
+static const char *slave_vols_vt1616[] = {
+	"Front Playback Volume",
+	"Surround Playback Volume",
+	"Center Playback Volume",
+	"LFE Playback Volume",
+	NULL
+};
+
+static const char *slave_sws_vt1616[] = {
+	"Front Playback Switch",
+	"Surround Playback Switch",
+	"Center Playback Switch",
+	"LFE Playback Switch",
+	NULL
+};
+
+/* find a mixer control element with the given name */
+static struct snd_kcontrol *snd_ac97_find_mixer_ctl(struct snd_ac97 *ac97,
+						    const char *name)
+{
+	struct snd_ctl_elem_id id;
+	memset(&id, 0, sizeof(id));
+	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	strcpy(id.name, name);
+	return snd_ctl_find_id(ac97->bus->card, &id);
+}
+
+/* create a virtual master control and add slaves */
+int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
+			 const unsigned int *tlv, const char **slaves)
+{
+	struct snd_kcontrol *kctl;
+	const char **s;
+	int err;
+
+	kctl = snd_ctl_make_virtual_master(name, tlv);
+	if (!kctl)
+		return -ENOMEM;
+	err = snd_ctl_add(ac97->bus->card, kctl);
+	if (err < 0)
+		return err;
+	
+	for (s = slaves; *s; s++) {
+		struct snd_kcontrol *sctl;
+
+		sctl = snd_ac97_find_mixer_ctl(ac97, *s);
+		if (!sctl) {
+			snd_printdd("Cannot find slave %s, skipped\n", *s);
+			continue;
+		}
+		err = snd_ctl_add_slave(kctl, sctl);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+
 static int patch_vt1616_specific(struct snd_ac97 * ac97)
 {
+	struct snd_kcontrol *kctl;
 	int err;
 
 	if (snd_ac97_try_bit(ac97, 0x5a, 9))
@@ -3341,6 +3399,24 @@ static int patch_vt1616_specific(struct 
 			return err;
 	if ((err = patch_build_controls(ac97, &snd_ac97_controls_vt1616[1], ARRAY_SIZE(snd_ac97_controls_vt1616) - 1)) < 0)
 		return err;
+
+	/* There is already a misnamed master switch.  Rename it.  */
+	kctl = snd_ac97_find_mixer_ctl(ac97, "Master Playback Volume");
+	if (!kctl)
+		return -EINVAL;
+
+	snd_ac97_rename_vol_ctl(ac97, "Master Playback", "Front Playback");
+
+	err = snd_ac97_add_vmaster(ac97, "Master Playback Volume",
+				   kctl->tlv.p, slave_vols_vt1616);
+	if (err < 0)
+		return err;
+
+	err = snd_ac97_add_vmaster(ac97, "Master Playback Switch",
+				   NULL, slave_sws_vt1616);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

At Sat, 22 Mar 2008 11:30:46 -0400,
Daniel Jacobowitz wrote:
> 
> On Sat, Mar 22, 2008 at 10:09:23AM +0100, Takashi Iwai wrote:
> > At Fri, 21 Mar 2008 15:08:55 -0400,
> > Daniel Jacobowitz wrote:
> > > 
> > > On Wed, Jan 30, 2008 at 04:27:07PM +0100, Takashi Iwai wrote:
> > > > Because HD-audio is the only user, so far.  I don't want to expand the
> > > > core stuff (and kconfig) unless really needed.
> > > 
> > > I said I'd want this for ice1724 / vt1616, and I finally got time to
> > > do it.  What do you think of this patch?  I'm happily using it.
> > 
> > Well, don't forget to check HG version before you submit a patch :)
> 
> Whoops!  This version works with the hg tree.  The new
> snd_ac97_add_vmaster is mostly similar to the hda version so it might
> make sense to put that in vmaster.c instead.
> 
> From: Daniel Jacobowitz <dan@codesourcery.com>
> 
> Enable VMASTER for VT1616 / VT1617A.
> 
> Signed-off-by: Daniel Jacobowitz <dan@codesourcery.com>

Thanks.  This kind of change is basically good to apply, I think, but
a couple of things related are in my mind:

- other AC97 codecs need similar hacks, so it could be done
  cleaner (in a more generic way) than in each patch_*()?
- headphone and line-out have the similar issue - it's handled as a
  bind-control right now, though
- what about if the device actually uses only two channels although
  codec supports more than two?  Then the additional master control
  makes little sense.  But, we have no way to know it from the codec
  itself.  Another quirk table?  That's to be avoided...

Since this is a non-urgent fix and I'll be on vacation for three weeks
from tomorrow, I'd like to keep this pending unless other developers
want to put this in.  Is it OK?


Takashi


> diff -r 0d5f43585ca7 drivers/Kconfig
> --- a/drivers/Kconfig	Sat Mar 22 10:26:05 2008 +0100
> +++ b/drivers/Kconfig	Sat Mar 22 11:02:16 2008 -0400
> @@ -44,6 +44,7 @@ config SND_AC97_CODEC
>  	tristate
>  	select SND_PCM
>  	select AC97_BUS
> +	select SND_VMASTER
>  
>  config SND_DUMMY
>  	tristate "Dummy (/dev/null) soundcard"
> diff -r 0d5f43585ca7 pci/ac97/ac97_patch.c
> --- a/pci/ac97/ac97_patch.c	Sat Mar 22 10:26:05 2008 +0100
> +++ b/pci/ac97/ac97_patch.c	Sat Mar 22 11:02:16 2008 -0400
> @@ -3332,8 +3332,66 @@ AC97_SINGLE("Downmix Surround to Front",
>  AC97_SINGLE("Downmix Surround to Front", 0x5a, 11, 1, 0),
>  };
>  
> +static const char *slave_vols_vt1616[] = {
> +	"Front Playback Volume",
> +	"Surround Playback Volume",
> +	"Center Playback Volume",
> +	"LFE Playback Volume",
> +	NULL
> +};
> +
> +static const char *slave_sws_vt1616[] = {
> +	"Front Playback Switch",
> +	"Surround Playback Switch",
> +	"Center Playback Switch",
> +	"LFE Playback Switch",
> +	NULL
> +};
> +
> +/* find a mixer control element with the given name */
> +static struct snd_kcontrol *snd_ac97_find_mixer_ctl(struct snd_ac97 *ac97,
> +						    const char *name)
> +{
> +	struct snd_ctl_elem_id id;
> +	memset(&id, 0, sizeof(id));
> +	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> +	strcpy(id.name, name);
> +	return snd_ctl_find_id(ac97->bus->card, &id);
> +}
> +
> +/* create a virtual master control and add slaves */
> +int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
> +			 const unsigned int *tlv, const char **slaves)
> +{
> +	struct snd_kcontrol *kctl;
> +	const char **s;
> +	int err;
> +
> +	kctl = snd_ctl_make_virtual_master(name, tlv);
> +	if (!kctl)
> +		return -ENOMEM;
> +	err = snd_ctl_add(ac97->bus->card, kctl);
> +	if (err < 0)
> +		return err;
> +	
> +	for (s = slaves; *s; s++) {
> +		struct snd_kcontrol *sctl;
> +
> +		sctl = snd_ac97_find_mixer_ctl(ac97, *s);
> +		if (!sctl) {
> +			snd_printdd("Cannot find slave %s, skipped\n", *s);
> +			continue;
> +		}
> +		err = snd_ctl_add_slave(kctl, sctl);
> +		if (err < 0)
> +			return err;
> +	}
> +	return 0;
> +}
> +
>  static int patch_vt1616_specific(struct snd_ac97 * ac97)
>  {
> +	struct snd_kcontrol *kctl;
>  	int err;
>  
>  	if (snd_ac97_try_bit(ac97, 0x5a, 9))
> @@ -3341,6 +3399,24 @@ static int patch_vt1616_specific(struct 
>  			return err;
>  	if ((err = patch_build_controls(ac97, &snd_ac97_controls_vt1616[1], ARRAY_SIZE(snd_ac97_controls_vt1616) - 1)) < 0)
>  		return err;
> +
> +	/* There is already a misnamed master switch.  Rename it.  */
> +	kctl = snd_ac97_find_mixer_ctl(ac97, "Master Playback Volume");
> +	if (!kctl)
> +		return -EINVAL;
> +
> +	snd_ac97_rename_vol_ctl(ac97, "Master Playback", "Front Playback");
> +
> +	err = snd_ac97_add_vmaster(ac97, "Master Playback Volume",
> +				   kctl->tlv.p, slave_vols_vt1616);
> +	if (err < 0)
> +		return err;
> +
> +	err = snd_ac97_add_vmaster(ac97, "Master Playback Switch",
> +				   NULL, slave_sws_vt1616);
> +	if (err < 0)
> +		return err;
> +
>  	return 0;
>  }
>  
> 
> -- 
> Daniel Jacobowitz
> CodeSourcery
> 

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Sat, Mar 22, 2008 at 05:31:21PM +0100, Takashi Iwai wrote:
> Thanks.  This kind of change is basically good to apply, I think, but
> a couple of things related are in my mind:
> 
> - other AC97 codecs need similar hacks, so it could be done
>   cleaner (in a more generic way) than in each patch_*()?
> - headphone and line-out have the similar issue - it's handled as a
>   bind-control right now, though
> - what about if the device actually uses only two channels although
>   codec supports more than two?  Then the additional master control
>   makes little sense.  But, we have no way to know it from the codec
>   itself.  Another quirk table?  That's to be avoided...
> 
> Since this is a non-urgent fix and I'll be on vacation for three weeks
> from tomorrow, I'd like to keep this pending unless other developers
> want to put this in.  Is it OK?

Fine with me.  I don't think I can make broader changes than this
kind, because I don't know much about AC97; maybe there can be a flag
on recognized codecs saying which channels the front volume really
controls?

I noticed that the PCM mixer also only affected my front speakers.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Sat, Mar 22, 2008 at 03:15:51PM -0400, Daniel Jacobowitz wrote:
> On Sat, Mar 22, 2008 at 05:31:21PM +0100, Takashi Iwai wrote:
> > Thanks.  This kind of change is basically good to apply, I think, but
> > a couple of things related are in my mind:
> > 
> > - other AC97 codecs need similar hacks, so it could be done
> >   cleaner (in a more generic way) than in each patch_*()?
> > - headphone and line-out have the similar issue - it's handled as a
> >   bind-control right now, though
> > - what about if the device actually uses only two channels although
> >   codec supports more than two?  Then the additional master control
> >   makes little sense.  But, we have no way to know it from the codec
> >   itself.  Another quirk table?  That's to be avoided...
> > 
> > Since this is a non-urgent fix and I'll be on vacation for three weeks
> > from tomorrow, I'd like to keep this pending unless other developers
> > want to put this in.  Is it OK?
> 
> Fine with me.  I don't think I can make broader changes than this
> kind, because I don't know much about AC97; maybe there can be a flag
> on recognized codecs saying which channels the front volume really
> controls?
> 
> I noticed that the PCM mixer also only affected my front speakers.

Hi Takashi,

Have you had another chance to look at this?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

At Fri, 2 May 2008 14:01:54 -0400,
Daniel Jacobowitz wrote:
> 
> On Sat, Mar 22, 2008 at 03:15:51PM -0400, Daniel Jacobowitz wrote:
> > On Sat, Mar 22, 2008 at 05:31:21PM +0100, Takashi Iwai wrote:
> > > Thanks.  This kind of change is basically good to apply, I think, but
> > > a couple of things related are in my mind:
> > > 
> > > - other AC97 codecs need similar hacks, so it could be done
> > >   cleaner (in a more generic way) than in each patch_*()?
> > > - headphone and line-out have the similar issue - it's handled as a
> > >   bind-control right now, though
> > > - what about if the device actually uses only two channels although
> > >   codec supports more than two?  Then the additional master control
> > >   makes little sense.  But, we have no way to know it from the codec
> > >   itself.  Another quirk table?  That's to be avoided...
> > > 
> > > Since this is a non-urgent fix and I'll be on vacation for three weeks
> > > from tomorrow, I'd like to keep this pending unless other developers
> > > want to put this in.  Is it OK?
> > 
> > Fine with me.  I don't think I can make broader changes than this
> > kind, because I don't know much about AC97; maybe there can be a flag
> > on recognized codecs saying which channels the front volume really
> > controls?
> > 
> > I noticed that the PCM mixer also only affected my front speakers.
> 
> Hi Takashi,
> 
> Have you had another chance to look at this?

Not quite, unfortunately.  But, in the end, I think it's relatively
harmless to take your patch as is, since it's pretty local to VT161x
codecs.

I'll put it on my local tree and make testing, then merge to the
public tree soon later.


thanks,

Takashi

Re: [RFC] [Patch 1/2] Add virtual master control

[Daniel Jacobowitz]

On Tue, May 06, 2008 at 05:17:31PM +0200, Takashi Iwai wrote:
> Not quite, unfortunately.  But, in the end, I think it's relatively
> harmless to take your patch as is, since it's pretty local to VT161x
> codecs.
> 
> I'll put it on my local tree and make testing, then merge to the
> public tree soon later.

Great, thanks a lot!  I agree it could be improved - but I'll leave
that for another future user :-)

-- 
Daniel Jacobowitz
CodeSourcery