Re: newbie problem with control settings

[Takashi Iwai <tiwai@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

At Wed, 23 Mar 2005 02:04:56 -0500 (EST),
D. Hugh Redelmeier wrote:
> 
> What confused me?  It turns out that my system's sound chip (C-Media
> Electronics Inc CM8738 built into my Asus A7V333 motherboard) has a
> couple of switches to change the line-in jack into an output.  They are
> called "Line-In As Rear" and "Line-In As Bass".  The way to control
> them in gnome-volume-control is to select or deselect "mute" for each
> of these controls.  I find this not at all obvious.
> 
> 	Line-In As Bass
> 	[ ] Mute
> 
> 	Line-In As Rear
> 	[ ] Mute
> 
> How would I improve it?  I would make the values of these switches be
> an enumerated type.  In UI terms: a set of radio buttons.
> 
>     Line-in jack is:
> 
>     [*] Line-in
>     [ ] bass output
>     [ ] rear Left and Right output

This sounds nice.  The (untested) patch below changes the handling of
these controls as you proposed.


> I must say that some of the controls have odd names.  One that really
> surprised me was "IEC958 Copyright" with a choice of mute or not.  (I
> do now have an idea what this is about.)

This is a problem of gnome-alsa-mixer.
ALSA provides only boolean "switches".  The mixer translates it as a
mute button wrongly.

> Recommendation: I think that there should be support for controls with
> enumerated type values.  Perhaps also boolean controls that could have
> a name other than "mute".

Again, from the driver viewpoint, it's nothing but an on/off switch.
Changing a simple boolean switch to enum is a bad idea in general,
IMO (except for the "modal" case like above).

> PS: here are the control names that I extracted from the driver
> source.  gnome-volume-control massaged them somewhat.  Some names are
> quite obscure.

I guess you mean "IEC958*" controls.  These can be seen in Windows
driver, too, but only in the advanced section.


Takashi

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

Index: alsa-kernel/pci/cmipci.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/pci/cmipci.c,v
retrieving revision 1.91
diff -u -r1.91 cmipci.c
--- alsa-kernel/pci/cmipci.c	22 Mar 2005 17:58:25 -0000	1.91
+++ alsa-kernel/pci/cmipci.c	23 Mar 2005 18:05:35 -0000
@@ -519,40 +519,50 @@
 }
 
 /* bit operations for dword register */
-static void snd_cmipci_set_bit(cmipci_t *cm, unsigned int cmd, unsigned int flag)
+static int snd_cmipci_set_bit(cmipci_t *cm, unsigned int cmd, unsigned int flag)
 {
-	unsigned int val;
-	val = inl(cm->iobase + cmd);
+	unsigned int val, oval;
+	val = oval = inl(cm->iobase + cmd);
 	val |= flag;
+	if (val == oval)
+		return 0;
 	outl(val, cm->iobase + cmd);
+	return 1;
 }
 
-static void snd_cmipci_clear_bit(cmipci_t *cm, unsigned int cmd, unsigned int flag)
+static int snd_cmipci_clear_bit(cmipci_t *cm, unsigned int cmd, unsigned int flag)
 {
-	unsigned int val;
-	val = inl(cm->iobase + cmd);
+	unsigned int val, oval;
+	val = oval = inl(cm->iobase + cmd);
 	val &= ~flag;
+	if (val == oval)
+		return 0;
 	outl(val, cm->iobase + cmd);
+	return 1;
 }
 
-#if 0 // not used
 /* bit operations for byte register */
-static void snd_cmipci_set_bit_b(cmipci_t *cm, unsigned int cmd, unsigned char flag)
+static int snd_cmipci_set_bit_b(cmipci_t *cm, unsigned int cmd, unsigned char flag)
 {
-	unsigned char val;
-	val = inb(cm->iobase + cmd);
+	unsigned char val, oval;
+	val = oval = inb(cm->iobase + cmd);
 	val |= flag;
+	if (val == oval)
+		return 0;
 	outb(val, cm->iobase + cmd);
+	return 1;
 }
 
-static void snd_cmipci_clear_bit_b(cmipci_t *cm, unsigned int cmd, unsigned char flag)
+static int snd_cmipci_clear_bit_b(cmipci_t *cm, unsigned int cmd, unsigned char flag)
 {
-	unsigned char val;
-	val = inb(cm->iobase + cmd);
+	unsigned char val, oval;
+	val = oval = inb(cm->iobase + cmd);
 	val &= ~flag;
+	if (val == oval)
+		return 0;
 	outb(val, cm->iobase + cmd);
+	return 1;
 }
-#endif
 
 
 /*
@@ -2250,8 +2260,8 @@
 DEFINE_SWITCH_ARG(exchange_dac, CM_REG_MISC_CTRL, CM_XCHGDAC, CM_XCHGDAC, 0, 0);
 #endif
 DEFINE_BIT_SWITCH_ARG(fourch, CM_REG_MISC_CTRL, CM_N4SPK3D, 0, 0);
-DEFINE_BIT_SWITCH_ARG(line_rear, CM_REG_MIXER1, CM_SPK4, 1, 0);
-DEFINE_BIT_SWITCH_ARG(line_bass, CM_REG_LEGACY_CTRL, CM_LINE_AS_BASS, 0, 0);
+// DEFINE_BIT_SWITCH_ARG(line_rear, CM_REG_MIXER1, CM_SPK4, 1, 0);
+// DEFINE_BIT_SWITCH_ARG(line_bass, CM_REG_LEGACY_CTRL, CM_LINE_AS_BASS, 0, 0);
 // DEFINE_BIT_SWITCH_ARG(joystick, CM_REG_FUNCTRL1, CM_JYSTK_EN, 0, 0); /* now module option */
 DEFINE_SWITCH_ARG(modem, CM_REG_MISC_CTRL, CM_FLINKON|CM_FLINKOFF, CM_FLINKON, 0, 0);
 
@@ -2300,10 +2310,114 @@
 }
 
 
+static int snd_cmipci_line_in_mode_info(snd_kcontrol_t *kcontrol,
+					snd_ctl_elem_info_t *uinfo)
+{
+	cmipci_t *cm = snd_kcontrol_chip(kcontrol);
+	static char *texts[3] = { "Line-In", "Rear Output", "Bass Output" };
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
+	uinfo->count = 1;
+	uinfo->value.enumerated.items = cm->chip_version >= 39 ? 3 : 2;
+	if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items)
+		uinfo->value.enumerated.item = uinfo->value.enumerated.items - 1;
+	strcpy(uinfo->value.enumerated.name, texts[uinfo->value.enumerated.item]);
+	return 0;
+}
+
+static inline unsigned int get_line_in_mode(cmipci_t *cm)
+{
+	unsigned int val;
+	if (cm->chip_version >= 39) {
+		val = snd_cmipci_read(cm, CM_REG_LEGACY_CTRL);
+		if (val & CM_LINE_AS_BASS)
+			return 2;
+	}
+	val = snd_cmipci_read_b(cm, CM_REG_MIXER1);
+	if (val & CM_SPK4)
+		return 1;
+	return 0;
+}
+
+static int snd_cmipci_line_in_mode_get(snd_kcontrol_t *kcontrol,
+				       snd_ctl_elem_value_t *ucontrol)
+{
+	cmipci_t *cm = snd_kcontrol_chip(kcontrol);
+
+	spin_lock_irq(&cm->reg_lock);
+	ucontrol->value.enumerated.item[0] = get_line_in_mode(cm);
+	spin_unlock_irq(&cm->reg_lock);
+	return 0;
+}
+
+static int snd_cmipci_line_in_mode_put(snd_kcontrol_t *kcontrol,
+				       snd_ctl_elem_value_t *ucontrol)
+{
+	cmipci_t *cm = snd_kcontrol_chip(kcontrol);
+	int change;
+
+	spin_lock_irq(&cm->reg_lock);
+	if (ucontrol->value.enumerated.item[0] == 2)
+		change = snd_cmipci_set_bit(cm, CM_REG_LEGACY_CTRL, CM_LINE_AS_BASS);
+	else
+		change = snd_cmipci_clear_bit(cm, CM_REG_LEGACY_CTRL, CM_LINE_AS_BASS);
+	if (ucontrol->value.enumerated.item[0] == 1)
+		change |= snd_cmipci_set_bit_b(cm, CM_REG_MIXER1, CM_SPK4);
+	else
+		change |= snd_cmipci_clear_bit_b(cm, CM_REG_MIXER1, CM_SPK4);
+	spin_unlock_irq(&cm->reg_lock);
+	return change;
+}
+
+static int snd_cmipci_mic_in_mode_info(snd_kcontrol_t *kcontrol,
+				       snd_ctl_elem_info_t *uinfo)
+{
+	static char *texts[2] = { "Mic-In", "Center/LFE Output" };
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
+	uinfo->count = 1;
+	uinfo->value.enumerated.items = 2;
+	if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items)
+		uinfo->value.enumerated.item = uinfo->value.enumerated.items - 1;
+	strcpy(uinfo->value.enumerated.name, texts[uinfo->value.enumerated.item]);
+	return 0;
+}
+
+static int snd_cmipci_mic_in_mode_get(snd_kcontrol_t *kcontrol,
+				      snd_ctl_elem_value_t *ucontrol)
+{
+	cmipci_t *cm = snd_kcontrol_chip(kcontrol);
+	/* same bit as spdi_phase */
+	spin_lock_irq(&cm->reg_lock);
+	ucontrol->value.enumerated.item[0] = 
+		(snd_cmipci_read_b(cm, CM_REG_MISC) & CM_SPDIF_INVERSE) ? 1 : 0;
+	spin_unlock_irq(&cm->reg_lock);
+	return 0;
+}
+
+static int snd_cmipci_mic_in_mode_put(snd_kcontrol_t *kcontrol,
+				      snd_ctl_elem_value_t *ucontrol)
+{
+	cmipci_t *cm = snd_kcontrol_chip(kcontrol);
+	int change;
+
+	spin_lock_irq(&cm->reg_lock);
+	if (ucontrol->value.enumerated.item[0])
+		change = snd_cmipci_set_bit_b(cm, CM_REG_MISC, CM_SPDIF_INVERSE);
+	else
+		change = snd_cmipci_clear_bit_b(cm, CM_REG_MISC, CM_SPDIF_INVERSE);
+	spin_unlock_irq(&cm->reg_lock);
+	return change;
+}
+
 /* both for CM8338/8738 */
 static snd_kcontrol_new_t snd_cmipci_mixer_switches[] __devinitdata = {
 	DEFINE_MIXER_SWITCH("Four Channel Mode", fourch),
-	DEFINE_MIXER_SWITCH("Line-In As Rear", line_rear),
+	{
+		.name = "Line-In Mode",
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.info = snd_cmipci_line_in_mode_info,
+		.get = snd_cmipci_line_in_mode_get,
+		.put = snd_cmipci_line_in_mode_put,
+	},
 };
 
 /* for non-multichannel chips */
@@ -2341,10 +2455,15 @@
 
 /* only for model 039 or later */
 static snd_kcontrol_new_t snd_cmipci_extra_mixer_switches[] __devinitdata = {
-	DEFINE_MIXER_SWITCH("Line-In As Bass", line_bass),
 	DEFINE_MIXER_SWITCH("IEC958 In Select", spdif_in_sel2),
 	DEFINE_MIXER_SWITCH("IEC958 In Phase Inverse", spdi_phase2),
-	DEFINE_MIXER_SWITCH("Mic As Center/LFE", spdi_phase), /* same bit as spdi_phase */
+	{
+		.name = "Mic-In Mode",
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.info = snd_cmipci_mic_in_mode_info,
+		.get = snd_cmipci_mic_in_mode_get,
+		.put = snd_cmipci_mic_in_mode_put,
+	}
 };
 
 /* card control switches */