>Number:         16817
>Category:       kern
>Synopsis:       SPDIF fixes for cmpci
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Tue May 14 16:27:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Stephen Ma
>Release:        NetBSD 1.5ZC 2002-05-08
>Organization:
	People's Front for the correct spelling of the word "Organisation"
>Environment:
System: NetBSD khemlani 1.5ZC NetBSD 1.5ZC (KHEMLANI) #7: Wed May  8 09:35:49 PDT 2002     stephenm@khemlani:/usr/src/sys/arch/i386/compile/KHEMLANI i386
Architecture: i386
Machine: i386
>Description:
The following patch fixes two bugs in the SPDIF support of the cmpci
audio driver. The bugs were reported by Christian Biere (one in
kern/16047, and the other in private email), and the patch has been
tested by him.

One bug (kern/16047) relates to the 48kHz playback rate not being set
correctly. The other problem is the SPDIF voltage level setting in
mixerctl has inverted sense.
>How-To-Repeat:
See kern/16047 for the 48kHz rate problem. The voltage level can be
observed by a multimeter (or oscilloscope :-) connected to the coax
SPDIF output.
>Fix:
--- /v1/netbsd/src/sys/dev/pci/cmpcireg.h	Sun Nov  4 04:49:44 2001
+++ ./cmpcireg.h	Wed Mar 27 10:30:31 2002
@@ -122,6 +122,7 @@
 #  define CMPCI_REG_SPDIF_48K		0x00008000
 #  define CMPCI_REG_FM_ENABLE		0x00080000
 #  define CMPCI_REG_SPDFLOOPI		0x00100000
+#  define CMPCI_REG_SPDIF_48K_B		0x01000000
 #  define CMPCI_REG_5V			0x02000000
 #  define CMPCI_REG_N4SPK3D		0x04000000
 
--- /v1/netbsd/src/sys/dev/pci/cmpci.c	Sun Feb  3 04:08:23 2002
+++ ./cmpci.c	Wed May  8 09:34:51 2002
@@ -98,6 +98,10 @@ static __inline void cmpci_reg_set_4 __P
 					  int, uint32_t));
 static __inline void cmpci_reg_clear_4 __P((struct cmpci_softc *,
 					    int, uint32_t));
+static __inline void cmpci_reg_set_reg_misc __P((struct cmpci_softc *,
+						 uint32_t));
+static __inline void cmpci_reg_clear_reg_misc __P((struct cmpci_softc *,
+						   uint32_t));
 static int cmpci_rate_to_index __P((int));
 static __inline int cmpci_index_to_rate __P((int));
 static __inline int cmpci_index_to_divider __P((int));
@@ -293,7 +297,46 @@ cmpci_reg_clear_4(sc, no, mask)
 	delay(10);
 }
 
+/*
+ * The CMPCI_REG_MISC register needs special handling, since one of
+ * its bits has different read/write values.
+ */
+static __inline void
+cmpci_reg_set_reg_misc(sc, mask)
+	struct cmpci_softc *sc;
+	uint32_t mask;
+{
+	uint32_t regval;
+	
+	if (mask == CMPCI_REG_SPDIF_48K_B)
+		cmpci_reg_set_4(sc, CMPCI_REG_MISC, mask);
+	else {
+		regval = bus_space_read_4(sc->sc_iot, sc->sc_ioh, CMPCI_REG_MISC);
+		bus_space_write_4(sc->sc_iot, sc->sc_ioh, CMPCI_REG_MISC,
+		    (regval & ~CMPCI_REG_SPDIF_48K_B) | mask |
+		    (sc->sc_play.md_divide==CMPCI_REG_RATE_48000 ? CMPCI_REG_SPDIF_48K_B : 0));
+		delay(10);
+	}
+}
 
+static __inline void
+cmpci_reg_clear_reg_misc(sc, mask)
+	struct cmpci_softc *sc;
+	uint32_t mask;
+{
+	uint32_t regval;
+	
+	if (mask == CMPCI_REG_SPDIF_48K_B)
+		cmpci_reg_clear_4(sc, CMPCI_REG_MISC, mask);
+	else {
+		regval = bus_space_read_4(sc->sc_iot, sc->sc_ioh, CMPCI_REG_MISC);
+		bus_space_write_4(sc->sc_iot, sc->sc_ioh, CMPCI_REG_MISC,
+		    (regval & ~mask & ~CMPCI_REG_SPDIF_48K_B) |
+		    (sc->sc_play.md_divide==CMPCI_REG_RATE_48000 ? CMPCI_REG_SPDIF_48K_B : 0));
+		delay(10);
+	}
+}
+	    
 /* rate */
 static const struct {
 	int rate;
@@ -1341,11 +1384,9 @@ cmpci_set_mixer_gain(sc, port)
 		if (CMPCI_ISCAP(sc, SPDOUT_VOLTAGE)) {
 			if (sc->sc_gain[CMPCI_SPDIF_OUT_VOLTAGE][CMPCI_LR]
 			    == CMPCI_SPDIF_OUT_VOLTAGE_LOW)
-				cmpci_reg_clear_4(sc, CMPCI_REG_MISC,
-						  CMPCI_REG_5V);
+				cmpci_reg_set_reg_misc(sc, CMPCI_REG_5V);
 			else
-				cmpci_reg_set_4(sc, CMPCI_REG_MISC,
-						CMPCI_REG_5V);
+				cmpci_reg_clear_reg_misc(sc, CMPCI_REG_5V);
 		}
 		return;
 	case CMPCI_SURROUND:
@@ -1361,11 +1402,9 @@ cmpci_set_mixer_gain(sc, port)
 	case CMPCI_REAR:
 		if (CMPCI_ISCAP(sc, REAR)) {
 			if (sc->sc_gain[CMPCI_REAR][CMPCI_LR])
-				cmpci_reg_set_4(sc, CMPCI_REG_MISC,
-						CMPCI_REG_N4SPK3D);
+				cmpci_reg_set_reg_misc(sc, CMPCI_REG_N4SPK3D);
 			else
-				cmpci_reg_clear_4(sc, CMPCI_REG_MISC,
-						  CMPCI_REG_N4SPK3D);
+				cmpci_reg_clear_reg_misc(sc, CMPCI_REG_N4SPK3D);
 		}
 		return;
 	case CMPCI_INDIVIDUAL:
@@ -1431,13 +1470,13 @@ cmpci_set_out_ports(sc)
 	/* SPDIF in select */
 	v = sc->sc_gain[CMPCI_SPDIF_IN_SELECT][CMPCI_LR];
 	if (v & CMPCI_SPDIFIN_SPDIFIN2)
-		cmpci_reg_set_4(sc, CMPCI_REG_MISC, CMPCI_REG_2ND_SPDIFIN);
+		cmpci_reg_set_reg_misc(sc, CMPCI_REG_2ND_SPDIFIN);
 	else
-		cmpci_reg_clear_4(sc, CMPCI_REG_MISC, CMPCI_REG_2ND_SPDIFIN);
+		cmpci_reg_clear_reg_misc(sc, CMPCI_REG_2ND_SPDIFIN);
 	if (v & CMPCI_SPDIFIN_SPDIFOUT)
-		cmpci_reg_set_4(sc, CMPCI_REG_MISC, CMPCI_REG_SPDFLOOPI);
+		cmpci_reg_set_reg_misc(sc, CMPCI_REG_SPDFLOOPI);
 	else
-		cmpci_reg_clear_4(sc, CMPCI_REG_MISC, CMPCI_REG_SPDFLOOPI);
+		cmpci_reg_clear_reg_misc(sc, CMPCI_REG_SPDFLOOPI);
 
 	/* playback to ... */
 	if (CMPCI_ISCAP(sc, SPDOUT) &&
@@ -1449,19 +1488,21 @@ cmpci_set_out_ports(sc)
 		/* playback to SPDIF */
 		cmpci_reg_set_4(sc, CMPCI_REG_FUNC_1, CMPCI_REG_SPDIF0_ENABLE);
 		enspdout = 1;
+		/*
+		 * XXX the cmpci_reg_blah_reg_misc() functions also
+		 * set/clear the CMPCI_REG_SPDIF_48K_B bit in the
+		 * CMPCI_REG_MISC REGISTER
+		 */
 		if (sc->sc_play.md_divide==CMPCI_REG_RATE_48000)
-			cmpci_reg_set_4(sc, CMPCI_REG_MISC,
-					CMPCI_REG_SPDIF_48K);
+			cmpci_reg_set_reg_misc(sc, CMPCI_REG_SPDIF_48K);
 		else
-			cmpci_reg_clear_4(sc, CMPCI_REG_MISC,
-					CMPCI_REG_SPDIF_48K);
+			cmpci_reg_clear_reg_misc(sc, CMPCI_REG_SPDIF_48K);
 	} else {
 		/* playback to DAC */
 		cmpci_reg_clear_4(sc, CMPCI_REG_FUNC_1,
 				  CMPCI_REG_SPDIF0_ENABLE);
 		if (CMPCI_ISCAP(sc, SPDOUT_48K))
-			cmpci_reg_clear_4(sc, CMPCI_REG_MISC,
-					  CMPCI_REG_SPDIF_48K);
+			cmpci_reg_clear_reg_misc(sc, CMPCI_REG_SPDIF_48K);
 	}
 
 	/* legacy to SPDIF/out or not */


>Release-Note:
>Audit-Trail:
>Unformatted: