Index | Thread | Search

From:
Alexandre Ratchov <alex@caoua.org>
Subject:
uaudio: Don't attempt to set rate on fixed-rate devices, please test
To:
tech@openbsd.org
Date:
Sat, 21 Dec 2024 19:25:20 +0100

Download raw body.

Thread
This diff makes uaudio(4) not attempt to set the rate of fixed-rate
audio interfaces. Certain audio interfaces, like the Dell Slimbar,
freeze if they receive the request to set the sample rate.

The diff is important to test on devices that work as well: hardware
bugs are common, so there might be devices on which the rate must be
set but that claim there's no rate control.

OK?

Index: uaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
diff -u -p -u -p -r1.176 uaudio.c
--- uaudio.c	1 Sep 2024 03:09:00 -0000	1.176
+++ uaudio.c	21 Dec 2024 18:17:54 -0000
@@ -87,12 +87,22 @@
 #define UAUDIO_AC_RATECONV		0xd
 
 /*
+ * AC class-specific CLKSRC controls
+ */
+#define UAUDIO_CLKSRC_FREQCTL		0x2
+
+/*
  * AS class-specific interface sub-types
  */
 #define UAUDIO_AS_GENERAL		0x1
 #define UAUDIO_AS_FORMAT		0x2
 
 /*
+ * AS class-specific endpoint sub-types
+ */
+#define UAUDIO_AS_EP_GENERAL		0x1
+
+/*
  * AS class-specific endpoint sub-type
  */
 #define UAUDIO_EP_GENERAL		0x1
@@ -229,6 +239,7 @@ struct uaudio_softc {
 
 		/* sample rates, if this is a clock source */
 		struct uaudio_ranges rates;
+		int cap_freqctl;
 
 		/* mixer(4) bits */
 #define UAUDIO_CLASS_OUT	0
@@ -284,6 +295,7 @@ struct uaudio_softc {
 		int fps;		/* USB (micro-)frames per second */
 		int bps, bits, nch;	/* audio encoding */
 		int v1_rates;		/* if UAC 1.0, bitmap of rates */
+		int v1_cap_freqctl;	/* don't try to set rate */
 	} *alts;
 
 	/*
@@ -1032,10 +1044,10 @@ uaudio_alt_getrates(struct uaudio_softc 
 }
 
 /*
- * return the clock unit of the given terminal unit (v2 only)
+ * return the clock source unit
  */
-int
-uaudio_clock_id(struct uaudio_softc *sc)
+struct uaudio_unit *
+uaudio_clock(struct uaudio_softc *sc)
 {
 	struct uaudio_unit *u;
 
@@ -1043,11 +1055,11 @@ uaudio_clock_id(struct uaudio_softc *sc)
 	while (1) {
 		if (u == NULL) {
 			DPRINTF("%s: NULL clock pointer\n", __func__);
-			return -1;
+			return NULL;
 		}
 		switch (u->type) {
 		case UAUDIO_AC_CLKSRC:
-			return u->id;
+			return u;
 		case UAUDIO_AC_CLKSEL:
 			u = u->clock;
 			break;
@@ -1057,7 +1069,7 @@ uaudio_clock_id(struct uaudio_softc *sc)
 			break;
 		default:
 			DPRINTF("%s: no clock\n", __func__);
-			return -1;
+			return NULL;
 		}
 	}
 }
@@ -1494,6 +1506,9 @@ uaudio_process_unit(struct uaudio_softc 
 			return 0;
 		DPRINTF("%02u: clock source, attr = 0x%x, ctl = 0x%x\n",
 		    u->id, u->term, ctl);
+		u->cap_freqctl = !!(ctl & UAUDIO_CLKSRC_FREQCTL);
+		if (!u->cap_freqctl)
+			DPRINTF("%02u: clock with fixed rate\n", u->id);
 		break;
 	case UAUDIO_AC_CLKSEL:
 		DPRINTF("%02u: clock sel\n", u->id);
@@ -2394,6 +2409,31 @@ uaudio_process_as_ep(struct uaudio_softc
 }
 
 /*
+ * Parse AS class-specifig endpoint descriptor
+ */
+int
+uaudio_process_as_cs_ep(struct uaudio_softc *sc,
+	struct uaudio_blob *p, struct uaudio_alt *a, int nep)
+{
+	unsigned int subtype, attr;
+
+	if (!uaudio_getnum(p, 1, &subtype))
+		return 0;
+	if (subtype != UAUDIO_AS_EP_GENERAL) {
+		DPRINTF("%s: %d: bad cs ep subtype\n", __func__, subtype);
+		return 0;
+	}
+	if (!uaudio_getnum(p, 1, &attr))
+		return 0;
+	if (sc->version == UAUDIO_V1) {
+		a->v1_cap_freqctl = !!(attr & UAUDIO_EP_FREQCTL);
+		if (!a->v1_cap_freqctl)
+			DPRINTF("%s: alt %d fixed rate\n", DEVNAME(sc), a->altnum);
+	}
+	return 1;
+}
+
+/*
  * Parse AS general descriptor. Non-PCM interfaces are skipped. UAC
  * v2.0 report the number of channels. For UAC v1.0 we set the number
  * of channels to zero, it will be determined later from the format
@@ -2590,14 +2630,16 @@ uaudio_process_as(struct uaudio_softc *s
 			goto failed;
 		if (!uaudio_getnum(&dp, 1, &type))
 			goto failed;
-		if (type == UDESC_CS_ENDPOINT)
-			continue;
-		if (type != UDESC_ENDPOINT) {
+		if (type == UDESC_CS_ENDPOINT) {
+			if (!uaudio_process_as_cs_ep(sc, &dp, a, nep))
+				goto failed;
+		} else if (type == UDESC_ENDPOINT) {
+			if (!uaudio_process_as_ep(sc, &dp, a, nep))
+				goto failed;
+		} else {
 			p->rptr = savep;
 			break;
 		}
-		if (!uaudio_process_as_ep(sc, &dp, a, nep))
-			goto failed;
 	}
 
 	if (a->mode == 0) {
@@ -2901,10 +2943,11 @@ uaudio_stream_open(struct uaudio_softc *
 {
 	struct uaudio_stream *s;
 	struct uaudio_alt *a;
+	struct uaudio_unit *clock;
 	struct usbd_interface *iface;
 	unsigned char req_buf[4];
 	unsigned int bpa, spf_max, min_blksz;
-	int err, clock_id, i;
+	int err, i;
 
 	if (dir == AUMODE_PLAY) {
 		s = &sc->pstream;
@@ -3057,13 +3100,17 @@ uaudio_stream_open(struct uaudio_softc *
 	 */
 	switch (sc->version) {
 	case UAUDIO_V1:
+		if (!a->v1_cap_freqctl) {
+			DPRINTF("%s: not setting endpoint rate\n", __func__);
+			break;
+		}
 		req_buf[0] = sc->rate;
 		req_buf[1] = sc->rate >> 8;
 		req_buf[2] = sc->rate >> 16;
 		if (!uaudio_req(sc, UT_WRITE_CLASS_ENDPOINT,
 			UAUDIO_V1_REQ_SET_CUR, UAUDIO_REQSEL_RATE, 0,
 			a->data_addr, 0, req_buf, 3)) {
-			DPRINTF("%s: not setting endpoint rate\n", __func__);
+			printf("%s: failed to set endpoint rate\n", DEVNAME(sc));
 		}
 		break;
 	case UAUDIO_V2:
@@ -3071,15 +3118,19 @@ uaudio_stream_open(struct uaudio_softc *
 		req_buf[1] = sc->rate >> 8;
 		req_buf[2] = sc->rate >> 16;
 		req_buf[3] = sc->rate >> 24;
-		clock_id = uaudio_clock_id(sc);
-		if (clock_id < 0) {
-			printf("%s: can't get clock id\n", DEVNAME(sc));
+		clock = uaudio_clock(sc);
+		if (clock == NULL) {
+			printf("%s: can't get clock\n", DEVNAME(sc));
 			goto failed;
 		}
+		if (!clock->cap_freqctl) {
+			DPRINTF("%s: not setting clock rate\n", __func__);
+			break;
+		}
 		if (!uaudio_req(sc, UT_WRITE_CLASS_INTERFACE,
 			UAUDIO_V2_REQ_CUR, UAUDIO_REQSEL_RATE, 0,
-			sc->ctl_ifnum, clock_id, req_buf, 4)) {
-			DPRINTF("%s: not setting clock rate\n", __func__);
+			sc->ctl_ifnum, clock->id, req_buf, 4)) {
+			printf("%s: failed to set clock rate\n", DEVNAME(sc));
 		}
 		break;
 	}