Subject: kern/29178: confusing behaviour because mixer classes in eap driver are misnamed
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Joachim Kuebart <kuebart@mathematik.uni-ulm.de>
List: netbsd-bugs
Date: 01/31/2005 23:00:01
>Number:         29178
>Category:       kern
>Synopsis:       confusing behaviour because mixer classes in eap driver are misnamed
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jan 31 23:00:01 +0000 2005
>Originator:     Joachim Kuebart
>Release:        NetBSD 2.0B
>Organization:
>Environment:
System: NetBSD jaja 2.0B NetBSD 2.0B (ALPHA-$Revision: 1.191 $) #3: Mon Jan 31 23:19:23 CET 2005 joki@jaja:/usr/obj/usr/src/sys/arch/alpha/compile/JAJA alpha
Architecture: alpha
Machine: alpha
>Description:
	The class assignments for the mixer controls in the eap driver are
	misleading. The current mixer looks like this:

		outputs.master=200,200 volume
		inputs.dac=200,200 volume
		inputs.fmsynth=200,200 volume
		inputs.cd=200,200 volume
		inputs.line=200,200 volume
		inputs.aux=200,200 volume
		inputs.mic=0 volume
		inputs.mic.preamp=off  [ off on ]
		record.source=mic  { mic cd line fmsynth aux dac }
		outputs.select=mic,cd,line,fmsynth,aux,dac  { mic cd line fmsynth aux dac }

	There are two problems with this: firstly, a study of
	<ftp://ftp.alsa-project.org/pub/manuals/asahi_kasei/4531.pdf> reveals
	that the mixers addressed by the inputs.* volume settings actually
	affect both the play-through as well as the recording volume. This
	means that they should be in the record.* class rather than the
	inputs.* class according to my understanding of the following section
	of audio(9):

		Each of the named sources for which the recording level can
		be set should have a control in the AudioCrecord class of
		type AUDIO_MIXER_VALUE...

		Controls for various sources that affect only the playback
		output, as opposed to recording, should be in the
		AudioCinputs class.

	Note that the AK4531 does not have any volume controls that affect
	only playback (except for outputs.master, obviously).

	Secondly, the outputs.select control is apparently wrong as it is
	supposed to refer to output destinations, cf. again audio(9):

		If the play mixer is capable of output to more than one
		destination, it should define AudioNselect in class
		AudioCoutputs. This mixer control should be of type
		AUDIO_MIXER_ENUM or AUDIO_MIXER_SET and enumerate the
		possible destinations.

	Instead, the channel names in the above example as well as the
	circuitry inside the AK4531 show that these controls actually control
	which inputs are switched through to the output mixer. The sentence

		...various sources in class AudioCrecord will be combined and
		presented to the single recording output, in the same fashion
		that the sources of class AudioCinputs are combined and
		presented to the playback output(s).

	in audio(9) suggests that this is what the inputs.select control is
	supposed to do.

	Note again that the AK4531 has only one output and thus does not
	offer the functionality described for outputs.select.

>How-To-Repeat:

	The described problem appears to be one of nomenclature at first
	sight, but it actually has a greater effect on how the "port" value
	in AUDIO_SET and AUDIO_GET ioctls is treated. With the current
	implementation, one has the problem that an audio program that
	selects "speaker" output (as, for example, libmikmod) will instead
	deactivate output alltogether because the outputs.select control gets
	set to zero for want of a mixer labelled "speaker" on the AK4531.
	This is clearly not the intended behaviour.

>Fix:

	The fix I suggest is to rename several mixer controls for the eap
	driver as follows:

		outputs.master=200,200 volume
		record.dac=200,200 volume
		record.fmsynth=200,200 volume
		record.cd=200,200 volume
		record.line=200,200 volume
		record.aux=200,200 volume
		record.mic=0 volume
		record.mic.preamp=off  [ off on ]
		record.source=mic  { mic cd line fmsynth aux dac }
		inputs.source=mic,cd,line,fmsynth,aux,dac  { mic cd line fmsynth aux dac }

	This is achieved by the following patch against -current sources:

Index: eap.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/eap.c,v
retrieving revision 1.76
diff -u -p -r1.76 eap.c
--- eap.c	15 Jan 2005 15:19:52 -0000	1.76
+++ eap.c	31 Jan 2005 22:53:52 -0000
@@ -164,7 +164,7 @@ struct eap_softc {
 
 	u_short	sc_port[AK_NPORTS];	/* mirror of the hardware setting */
 	u_int	sc_record_source;	/* recording source mask */
-	u_int	sc_output_source;	/* output source mask */
+	u_int	sc_input_source;	/* input source mask */
 	u_int	sc_mic_preamp;
 	char    sc_1371;		/* Using ES1371/AC97 codec */
 
@@ -703,7 +703,7 @@ eap_attach(struct device *parent, struct
 		eap_hw_if = &eap1370_hw_if;
 
 		/* Enable all relevant mixer switches. */
-		ctl.dev = EAP_OUTPUT_SELECT;
+		ctl.dev = EAP_INPUT_SOURCE;
 		ctl.type = AUDIO_MIXER_SET;
 		ctl.un.mask = 1 << EAP_VOICE_VOL | 1 << EAP_FM_VOL |
 			1 << EAP_CD_VOL | 1 << EAP_LINE_VOL | 1 << EAP_AUX_VOL |
@@ -1478,10 +1478,10 @@ eap1370_mixer_set_port(void *addr, mixer
 		eap1370_set_mixer(sc, AK_IN_MIXER2_R, r2);
 		return 0;
 	}
-	if (cp->dev == EAP_OUTPUT_SELECT) {
+	if (cp->dev == EAP_INPUT_SOURCE) {
 		if (cp->type != AUDIO_MIXER_SET)
 			return EINVAL;
-		m = sc->sc_output_source = cp->un.mask;
+		m = sc->sc_input_source = cp->un.mask;
 		o1 = o2 = 0;
 		if (m & (1 << EAP_VOICE_VOL))
 			o2 |= AK_M_VOICE_L | AK_M_VOICE_R;
@@ -1578,10 +1578,10 @@ eap1370_mixer_get_port(void *addr, mixer
 			return EINVAL;
 		cp->un.mask = sc->sc_record_source;
 		return 0;
-	case EAP_OUTPUT_SELECT:
+	case EAP_INPUT_SOURCE:
 		if (cp->type != AUDIO_MIXER_SET)
 			return EINVAL;
-		cp->un.mask = sc->sc_output_source;
+		cp->un.mask = sc->sc_input_source;
 		return 0;
 	case EAP_MIC_PREAMP:
 		if (cp->type != AUDIO_MIXER_ENUM)
@@ -1648,7 +1648,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_VOICE_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNdac);
@@ -1657,7 +1657,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_FM_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNfmsynth);
@@ -1666,7 +1666,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_CD_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNcd);
@@ -1675,7 +1675,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_LINE_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNline);
@@ -1684,7 +1684,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_AUX_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNaux);
@@ -1693,7 +1693,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_MIC_VOL:
 		dip->type = AUDIO_MIXER_VALUE;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = AUDIO_MIXER_LAST;
 		dip->next = EAP_MIC_PREAMP;
 		strcpy(dip->label.name, AudioNmicrophone);
@@ -1719,10 +1719,10 @@ eap1370_query_devinfo(void *addr, mixer_
 		strcpy(dip->un.s.member[5].label.name, AudioNdac);
 		dip->un.s.member[5].mask = 1 << EAP_VOICE_VOL;
 		return 0;
-	case EAP_OUTPUT_SELECT:
-		dip->mixer_class = EAP_OUTPUT_CLASS;
+	case EAP_INPUT_SOURCE:
+		dip->mixer_class = EAP_INPUT_CLASS;
 		dip->prev = dip->next = AUDIO_MIXER_LAST;
-		strcpy(dip->label.name, AudioNselect);
+		strcpy(dip->label.name, AudioNsource);
 		dip->type = AUDIO_MIXER_SET;
 		dip->un.s.num_mem = 6;
 		strcpy(dip->un.s.member[0].label.name, AudioNmicrophone);
@@ -1740,7 +1740,7 @@ eap1370_query_devinfo(void *addr, mixer_
 		return 0;
 	case EAP_MIC_PREAMP:
 		dip->type = AUDIO_MIXER_ENUM;
-		dip->mixer_class = EAP_INPUT_CLASS;
+		dip->mixer_class = EAP_RECORD_CLASS;
 		dip->prev = EAP_MIC_VOL;
 		dip->next = AUDIO_MIXER_LAST;
 		strcpy(dip->label.name, AudioNpreamp);
Index: eapreg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/eapreg.h,v
retrieving revision 1.9
diff -u -p -r1.9 eapreg.h
--- eapreg.h	15 Jan 2005 15:19:52 -0000	1.9
+++ eapreg.h	31 Jan 2005 22:53:52 -0000
@@ -269,7 +269,7 @@
 #define EAP_AUX_VOL		5
 #define EAP_MIC_VOL		6
 #define	EAP_RECORD_SOURCE	7
-#define EAP_OUTPUT_SELECT	8
+#define EAP_INPUT_SOURCE	8
 #define	EAP_MIC_PREAMP		9
 #define EAP_OUTPUT_CLASS	10
 #define EAP_RECORD_CLASS	11