This is a multi-part message in MIME format.

--Multipart_Wed__3_Sep_2003_09:46:22_+0200_0822d400
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

I've been digging around the weird behaviour of the AC97 codec of the
ICH5 I have here.

First of all, kern/20368 is wrong, although the fix does the right thing
in the end. The main reason for that is that the codec listing is
erroneous: ADS 0x70 is ad1980, not ad1981. Thus the register names are
different. In the meantime, that codec should be probed as:

auich0: Analog Devices AD1980 codec

Numerically the patch that was applied is ok, but doesn't make any
sense. It's interesting to note that the two other OSes I've checked
(FreeBSD and Linux) don't bother with flag names, they just write the
numeric value.

Then I tried to understand why master volume is controlled by the
surround mixer entry (problem 1 in kern/22548).

It appears that some vendors (it is the case for the Dell I have here)
connect the line-out to the headphone pins, and vice-versa. That's the
reason why without the kern/20368 fix, things working OK as long as
headphones were plugged in the surround (line-out) jack.

The effect of kern/20368 is to have the headphone controlled by the
surround register, and the line-out controlled by the main mixer, thus
inverting the use of the two registers, creating the behaviour described
in kern/22548.

In the Linux code, the register inversion is not always done (there's a
quirk table listing some vendor/device entries in 2.5.68), I wonder if
there are ad1980 users with normal wiring out there.

The following patch corrects kern/20368, and add a quirk system allowing
the inversion of the two mixer controls. I think it is the best place to
do this (late in ac97_attach, but before the codec init function call),
so it is open to future quirks.

Quentin Garnier.

--Multipart_Wed__3_Sep_2003_09:46:22_+0200_0822d400
Content-Type: text/plain;
 name="ac97.c.patch"
Content-Disposition: attachment;
 filename="ac97.c.patch"
Content-Transfer-Encoding: quoted-printable

Index: ac97.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/ic/ac97.c,v
retrieving revision 1.43
diff -u -r1.43 ac97.c
--- ac97.c	2003/06/13 05:31:29	1.43
+++ ac97.c	2003/09/03 07:14:13
@@ -308,6 +308,7 @@
 	enum ac97_host_flags host_flags;
 	unsigned int ac97_clock; /* usually 48000 */
 #define AC97_STANDARD_CLOCK	48000U
+	u_int32_t id;
 	u_int16_t caps;		/* -> AC97_REG_RESET */
 	u_int16_t ext_id;	/* -> AC97_REG_EXT_AUDIO_ID */
 	u_int16_t shadow_reg[128];
@@ -324,7 +325,7 @@
 u_int16_t ac97_get_extcaps(struct ac97_codec_if *codec_if);
 int ac97_add_port(struct ac97_softc *as, const struct ac97_source_info *sr=
c);
=20
-static void ac97_ad1981_init(struct ac97_softc *);
+static void ac97_ad1980_init(struct ac97_softc *);
 static void ac97_alc650_init(struct ac97_softc *);
 static void ac97_vt1616_init(struct ac97_softc *);
=20
@@ -366,7 +367,7 @@
 	{ AC97_CODEC_ID('A', 'D', 'S', 0x63),
 	  0xffffffff,			"Analog Devices AD1886A" },
 	{ AC97_CODEC_ID('A', 'D', 'S', 0x70),
-	  0xffffffff,			"Analog Devices AD1981", ac97_ad1981_init },
+	  0xffffffff,			"Analog Devices AD1980", ac97_ad1980_init },
 	{ AC97_CODEC_ID('A', 'D', 'S', 0x72),
 	  0xffffffff,			"Analog Devices AD1981A" },
 	{ AC97_CODEC_ID('A', 'D', 'S', 0x74),
@@ -636,6 +637,21 @@
 	"20 bit ADC"
 };
=20
+static void ac97_do_quirks(struct ac97_softc *);
+static void ac97_quirk_invert_master_surround(struct ac97_softc *);
+
+#define AC97_QUIRK_INVERT_MASTER_SURROUND	1
+
+static const struct ac97_quirk {
+	u_int32_t	id;
+	u_int32_t	mask;
+	u_int32_t	quirk;
+} ac97_quirks[] =3D {
+	{ AC97_CODEC_ID('A', 'D', 'S', 0x70), 0xffffffff,
+		AC97_QUIRK_INVERT_MASTER_SURROUND },
+	{ 0, 0, 0 }
+};
+
=20
 int ac97_str_equal __P((const char *, const char *));
 int ac97_check_capability(struct ac97_softc *, int);
@@ -867,7 +883,6 @@
 	struct ac97_softc *as;
 	struct device *sc_dev =3D (struct device *)host_if->arg;
 	int error, i, j;
-	u_int32_t id;
 	u_int16_t id1, id2;
 	u_int16_t extstat, rate;
 	mixer_ctrl_t ctl;
@@ -901,7 +916,7 @@
 	ac97_read(as, AC97_REG_VENDOR_ID2, &id2);
 	ac97_read(as, AC97_REG_RESET, &as->caps);
=20
-	id =3D (id1 << 16) | id2;
+	as->id =3D (id1 << 16) | id2;
=20
 	aprint_normal("%s: ", sc_dev->dv_xname);
=20
@@ -909,20 +924,20 @@
 		if (ac97codecid[i].id =3D=3D 0) {
 			char pnp[4];
=20
-			AC97_GET_CODEC_ID(id, pnp);
+			AC97_GET_CODEC_ID(as->id, pnp);
 #define ISASCII(c) ((c) >=3D ' ' && (c) < 0x7f)
 			if (ISASCII(pnp[0]) && ISASCII(pnp[1]) &&
 			    ISASCII(pnp[2]))
 				aprint_normal("%c%c%c%d",
 				    pnp[0], pnp[1], pnp[2], pnp[3]);
 			else
-				aprint_normal("unknown (0x%08x)", id);
+				aprint_normal("unknown (0x%08x)", as->id);
 			break;
 		}
-		if (ac97codecid[i].id =3D=3D (id & ac97codecid[i].mask)) {
+		if (ac97codecid[i].id =3D=3D (as->id & ac97codecid[i].mask)) {
 			aprint_normal("%s", ac97codecid[i].name);
 			if (ac97codecid[i].mask =3D=3D AC97_VENDOR_ID_MASK) {
-				aprint_normal(" (0x%08x)", id);
+				aprint_normal(" (0x%08x)", as->id);
 			}
 			initfunc =3D ac97codecid[i].init;
 			break;
@@ -1057,6 +1072,8 @@
 					   AudioNlfe, NULL);
 	ac97_mixer_set_port(&as->codec_if, &ctl);
=20
+	ac97_do_quirks(as);
+
 	if (initfunc !=3D NULL)
 		initfunc(as);
 	return (0);
@@ -1457,22 +1474,41 @@
  */
=20
 #define AC97_AD_REG_MISC	0x76
-#define	AC97_AD_MISC_MBG	0x0001  /* 0 */
-#define AC97_AD_MISC_VREFD	0x0002  /* 1 */
-#define AC97_AD_MISC_VREFH	0x0004  /* 2 */
-#define AC97_AD_MISC_MADST	0x0008  /* 3 */
-#define AC97_AD_MISC_MADPD	0x0020  /* 5 */
-#define AC97_AD_MISC_FMXE	0x0100  /* 8 */
-#define AC97_AD_MISC_DAM	0x0400  /*10 */
-#define AC97_AD_MISC_MSPLT	0x1000  /*12 */
-#define AC97_AD_MISC_DACZ	0x4000  /*14 */
+
+#define AC97_AD1980_MISC_MBG0	0x0001	/* 0 */
+#define AC97_AD1980_MISC_MBG1	0x0002	/* 1 */
+#define AC97_AD1980_MISC_VREFD	0x0004	/* 2 */
+#define AC97_AD1980_MISC_VREFH	0x0008	/* 3 */
+#define AC97_AD1980_MISC_SRU	0x0010	/* 4 */
+#define AC97_AD1980_MISC_LOSEL	0x0020	/* 5 */
+#define AC97_AD1980_MISC_2CMIC	0x0040	/* 6 */
+#define AC97_AD1980_MISC_SPRD	0x0080	/* 7 */
+#define AC97_AD1980_MISC_DMIX0	0x0100	/* 8 */
+#define AC97_AD1980_MISC_DMIX1	0x0200	/* 9 */
+#define AC97_AD1980_MISC_HPSEL	0x0400	/*10 */
+#define AC97_AD1980_MISC_CLDIS	0x0800	/*11 */
+#define AC97_AD1980_MISC_LODIS	0x1000	/*12 */
+#define AC97_AD1980_MISC_MSPLT	0x2000	/*13 */
+#define AC97_AD1980_MISC_AC97NC	0x4000	/*14 */
+#define AC97_AD1980_MISC_DACZ	0x8000	/*15 */
+
+#define	AC97_AD1981_MISC_MBG	0x0001  /* 0 */
+#define AC97_AD1981_MISC_VREFD	0x0002  /* 1 */
+#define AC97_AD1981_MISC_VREFH	0x0004  /* 2 */
+#define AC97_AD1981_MISC_MADST	0x0008  /* 3 */
+#define AC97_AD1981_MISC_MADPD	0x0020  /* 5 */
+#define AC97_AD1981_MISC_FMXE	0x0100  /* 8 */
+#define AC97_AD1981_MISC_DAM	0x0400  /*10 */
+#define AC97_AD1981_MISC_MSPLT	0x1000  /*12 */
+#define AC97_AD1981_MISC_DACZ	0x4000  /*14 */
+
 static void
-ac97_ad1981_init(struct ac97_softc *as)
+ac97_ad1980_init(struct ac97_softc *as)
 {
 	unsigned short misc;
=20
 	ac97_read(as, AC97_AD_REG_MISC, &misc);
-	ac97_write(as, AC97_AD_REG_MISC, misc|AC97_AD_MISC_DAM|AC97_AD_MISC_MADPD=
);
+	ac97_write(as, AC97_AD_REG_MISC, misc|AC97_AD1980_MISC_LOSEL|AC97_AD1980_=
MISC_HPSEL);
 }
=20
 #define ALC650_REG_MULTI_CHANNEL_CONTROL	0x6a
@@ -1558,3 +1594,40 @@
 	ac97_add_port(as, &sources[2]);
 }
=20
+static void
+ac97_do_quirks(struct ac97_softc *as)
+{
+	const struct ac97_quirk *q;
+
+	for (q =3D ac97_quirks; q->id !=3D 0; q++)
+		if (q->id =3D=3D (as->id & q->mask))
+			break;
+
+	if (!q->id)
+		return;
+
+	switch(q->quirk) {
+	case AC97_QUIRK_INVERT_MASTER_SURROUND:
+		ac97_quirk_invert_master_surround(as);
+		break;
+	default:
+		aprint_error("%s: invalid quirk %d\n", __func__, q->quirk);
+		break;
+	}
+}
+
+static void
+ac97_quirk_invert_master_surround(struct ac97_softc *as)
+{
+	int i;
+
+	for (i =3D 0; i < as->num_source_info; i++) {
+		if (as->source_info[i].type !=3D AUDIO_MIXER_VALUE)
+			continue;
+
+		if (as->source_info[i].reg =3D=3D AC97_REG_MASTER_VOLUME)
+			as->source_info[i].reg =3D AC97_REG_SURR_MASTER;
+		else if (as->source_info[i].reg =3D=3D AC97_REG_SURR_MASTER)
+			as->source_info[i].reg =3D AC97_REG_MASTER_VOLUME;
+	}
+}

--Multipart_Wed__3_Sep_2003_09:46:22_+0200_0822d400--