sound: Defer macio codec volume writes to a task

tumbler(4), snapper(4) and onyx(4) write the volume over I2C, and
iicbus_transfer() sleeps. This is why mixer_set() drops the mixer lock
around MIXER_SET() for non-MPSAFE drivers, relying on Giant to keep them
serialized.

Store the volume in the softc and let a task do the I2C write with no
lock held, so that the mixer method does not sleep at all. The lock
dropping for non-MPSAFE drivers will be removed in a follow-up patch.

Sponsored by:	The FreeBSD Foundation
MFC after:	1 month
Differential Revision:	https://reviews.freebsd.org/D59073

sys/dev/sound/macio/onyx.c

@@ -39,6 +39,7 @@
 #include <sys/malloc.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
+#include <sys/taskqueue.h>
 #include <machine/dbdma.h>
 #include <machine/intr_machdep.h>
 #include <machine/resource.h>
@@ -65,6 +66,10 @@ struct onyx_softc
 {
 	device_t sc_dev;
 	uint32_t sc_addr;
+	struct mtx sc_volume_mtx;
+	struct task sc_volume_task;
+	uint8_t sc_left;
+	uint8_t sc_right;
 };
 
 static int	onyx_probe(device_t);
@@ -185,6 +190,26 @@ onyx_write(struct onyx_softc *sc, uint8_t reg, const uint8_t value)
 	return (0);
 }
 
+/*
+ * onyx_write() sleeps in iicbus_transfer(), so onyx_set() cannot program the
+ * volume registers inline. Hand the new values to a task instead, which runs
+ * with no lock held.
+ */
+static void
+onyx_volume_task(void *arg, int pending __unused)
+{
+	struct onyx_softc *sc = arg;
+	uint8_t l, r;
+
+	mtx_lock(&sc->sc_volume_mtx);
+	l = sc->sc_left;
+	r = sc->sc_right;
+	mtx_unlock(&sc->sc_volume_mtx);
+
+	onyx_write(sc, PCM3052_REG_LEFT_ATTN, l);
+	onyx_write(sc, PCM3052_REG_RIGHT_ATTN, r);
+}
+
 static int
 onyx_probe(device_t dev)
 {
@@ -216,6 +241,9 @@ onyx_attach(device_t dev)
 	sc->sc_dev = dev;
 	sc->sc_addr = iicbus_get_addr(dev);
 
+	mtx_init(&sc->sc_volume_mtx, "onyx volume", NULL, MTX_DEF);
+	TASK_INIT(&sc->sc_volume_task, 0, onyx_volume_task, sc);
+
 	i2s_mixer_class = &onyx_mixer_class;
 	i2s_mixer = dev;
 
@@ -255,6 +283,12 @@ onyx_init(struct snd_mixer *m)
 static int
 onyx_uninit(struct snd_mixer *m)
 {
+	struct onyx_softc *sc;
+
+	sc = device_get_softc(mix_getdevinfo(m));
+
+	taskqueue_drain(taskqueue_thread, &sc->sc_volume_task);
+
 	return (0);
 }
 
@@ -280,8 +314,12 @@ onyx_set(struct snd_mixer *m, unsigned dev, unsigned left, unsigned right)
 		l = left + 128;
 		r = right + 128;
 
-		onyx_write(sc, PCM3052_REG_LEFT_ATTN, l);
-		onyx_write(sc, PCM3052_REG_RIGHT_ATTN, r);
+		mtx_lock(&sc->sc_volume_mtx);
+		sc->sc_left = l;
+		sc->sc_right = r;
+		mtx_unlock(&sc->sc_volume_mtx);
+
+		taskqueue_enqueue(taskqueue_thread, &sc->sc_volume_task);
 
 		return (left | (right << 8));
 	}

sys/dev/sound/macio/snapper.c

@@ -65,6 +65,7 @@
 #include <sys/malloc.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
+#include <sys/taskqueue.h>
 #include <machine/dbdma.h>
 #include <machine/intr_machdep.h>
 #include <machine/resource.h>
@@ -91,6 +92,9 @@ struct snapper_softc
 {
 	device_t sc_dev;
 	uint32_t sc_addr;
+	struct mtx sc_volume_mtx;
+	struct task sc_volume_task;
+	u_char sc_volume_reg[6];
 };
 
 static int	snapper_probe(device_t);
@@ -338,6 +342,24 @@ snapper_write(struct snapper_softc *sc, uint8_t reg, const void *data)
 	return (0);
 }
 
+/*
+ * snapper_write() sleeps in iicbus_transfer(), so snapper_set() cannot
+ * program the volume registers inline. Hand the new values to a task
+ * instead, which runs with no lock held.
+ */
+static void
+snapper_volume_task(void *arg, int pending __unused)
+{
+	struct snapper_softc *sc = arg;
+	u_char reg[6];
+
+	mtx_lock(&sc->sc_volume_mtx);
+	memcpy(reg, sc->sc_volume_reg, sizeof(reg));
+	mtx_unlock(&sc->sc_volume_mtx);
+
+	snapper_write(sc, SNAPPER_VOLUME, reg);
+}
+
 static int
 snapper_probe(device_t dev)
 {
@@ -371,6 +393,9 @@ snapper_attach(device_t dev)
 	sc->sc_dev = dev;
 	sc->sc_addr = iicbus_get_addr(dev);
 
+	mtx_init(&sc->sc_volume_mtx, "snapper volume", NULL, MTX_DEF);
+	TASK_INIT(&sc->sc_volume_task, 0, snapper_volume_task, sc);
+
 	i2s_mixer_class = &snapper_mixer_class;
 	i2s_mixer = dev;
 
@@ -423,6 +448,12 @@ snapper_init(struct snd_mixer *m)
 static int
 snapper_uninit(struct snd_mixer *m)
 {
+	struct snapper_softc *sc;
+
+	sc = device_get_softc(mix_getdevinfo(m));
+
+	taskqueue_drain(taskqueue_thread, &sc->sc_volume_task);
+
 	return (0);
 }
 
@@ -456,7 +487,11 @@ snapper_set(struct snd_mixer *m, unsigned dev, unsigned left, unsigned right)
 		reg[4] = (r & 0x00ff00) >> 8;
 		reg[5] = r & 0x0000ff;
 
-		snapper_write(sc, SNAPPER_VOLUME, reg);
+		mtx_lock(&sc->sc_volume_mtx);
+		memcpy(sc->sc_volume_reg, reg, sizeof(reg));
+		mtx_unlock(&sc->sc_volume_mtx);
+
+		taskqueue_enqueue(taskqueue_thread, &sc->sc_volume_task);
 
 		return (left | (right << 8));
 	}

sys/dev/sound/macio/tumbler.c

@@ -65,6 +65,7 @@
 #include <sys/malloc.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
+#include <sys/taskqueue.h>
 #include <machine/dbdma.h>
 #include <machine/intr_machdep.h>
 #include <machine/resource.h>
@@ -91,6 +92,9 @@ struct tumbler_softc
 {
 	device_t sc_dev;
 	uint32_t sc_addr;
+	struct mtx sc_volume_mtx;
+	struct task sc_volume_task;
+	u_char sc_volume_reg[6];
 };
 
 static int	tumbler_probe(device_t);
@@ -299,6 +303,24 @@ tumbler_write(struct tumbler_softc *sc, uint8_t reg, const void *data)
 	return (0);
 }
 
+/*
+ * tumbler_write() sleeps in iicbus_transfer(), so tumbler_set() cannot
+ * program the volume registers inline. Hand the new values to a task
+ * instead, which runs with no lock held.
+ */
+static void
+tumbler_volume_task(void *arg, int pending __unused)
+{
+	struct tumbler_softc *sc = arg;
+	u_char reg[6];
+
+	mtx_lock(&sc->sc_volume_mtx);
+	memcpy(reg, sc->sc_volume_reg, sizeof(reg));
+	mtx_unlock(&sc->sc_volume_mtx);
+
+	tumbler_write(sc, TUMBLER_VOLUME, reg);
+}
+
 static int
 tumbler_probe(device_t dev)
 {
@@ -326,6 +348,9 @@ tumbler_attach(device_t dev)
 	sc->sc_dev = dev;
 	sc->sc_addr = iicbus_get_addr(dev);
 
+	mtx_init(&sc->sc_volume_mtx, "tumbler volume", NULL, MTX_DEF);
+	TASK_INIT(&sc->sc_volume_task, 0, tumbler_volume_task, sc);
+
 	i2s_mixer_class = &tumbler_mixer_class;
 	i2s_mixer = dev;
 
@@ -370,6 +395,12 @@ tumbler_init(struct snd_mixer *m)
 static int
 tumbler_uninit(struct snd_mixer *m)
 {
+	struct tumbler_softc *sc;
+
+	sc = device_get_softc(mix_getdevinfo(m));
+
+	taskqueue_drain(taskqueue_thread, &sc->sc_volume_task);
+
 	return (0);
 }
 
@@ -403,7 +434,11 @@ tumbler_set(struct snd_mixer *m, unsigned dev, unsigned left, unsigned right)
 		reg[4] = (r & 0x00ff00) >> 8;
 		reg[5] = r & 0x0000ff;
 
-		tumbler_write(sc, TUMBLER_VOLUME, reg);
+		mtx_lock(&sc->sc_volume_mtx);
+		memcpy(sc->sc_volume_reg, reg, sizeof(reg));
+		mtx_unlock(&sc->sc_volume_mtx);
+
+		taskqueue_enqueue(taskqueue_thread, &sc->sc_volume_task);
 
 		return (left | (right << 8));
 	}