git: b9761547077f - main - sound: Defer macio codec volume writes to a task
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 18 Sep 2026 18:18:40 UTC
The branch main has been updated by christos:
URL: https://cgit.FreeBSD.org/src/commit/?id=b9761547077fb94e12c0bfa81f3b8b2b2df3c0bb
commit b9761547077fb94e12c0bfa81f3b8b2b2df3c0bb
Author: Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2026-09-18 18:14:29 +0000
Commit: Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2026-09-18 18:14:29 +0000
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 | 42 ++++++++++++++++++++++++++++++++++++++++--
sys/dev/sound/macio/snapper.c | 37 ++++++++++++++++++++++++++++++++++++-
sys/dev/sound/macio/tumbler.c | 37 ++++++++++++++++++++++++++++++++++++-
3 files changed, 112 insertions(+), 4 deletions(-)
diff --git a/sys/dev/sound/macio/onyx.c b/sys/dev/sound/macio/onyx.c
index 745f861790db..8f1685e5f774 100644
--- a/sys/dev/sound/macio/onyx.c
+++ b/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));
}
diff --git a/sys/dev/sound/macio/snapper.c b/sys/dev/sound/macio/snapper.c
index c68ce20ee0b6..d2d139d7225d 100644
--- a/sys/dev/sound/macio/snapper.c
+++ b/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));
}
diff --git a/sys/dev/sound/macio/tumbler.c b/sys/dev/sound/macio/tumbler.c
index 0bb619f2fd90..edb420fc8099 100644
--- a/sys/dev/sound/macio/tumbler.c
+++ b/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));
}