git: 9548bfa342a7 - main - sound: Retire mixer_hwvol locked variants
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 18 Sep 2026 18:18:42 UTC
The branch main has been updated by christos:
URL: https://cgit.FreeBSD.org/src/commit/?id=9548bfa342a71f1ca0ad953bf323086e84a395c3
commit 9548bfa342a71f1ca0ad953bf323086e84a395c3
Author: Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2026-09-18 18:16:03 +0000
Commit: Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2026-09-18 18:16:03 +0000
sound: Retire mixer_hwvol locked variants
Prior to 9a00e0b8ca56 ("snd_uaudio: Do not use snd_mixer->lock as
mixer_lock"), there was a need for mixer_hwvol_mute_locked() and
mixer_hwvol_step_locked(), because the unlocked variants would acquire
the lock, but uaudio_hid_rx_callback() would also hold the lock, so this
was a measure to avoid recursion on snd_mixer->lock. Now that
snd_uaudio(4) has a private mixer lock, the locked variants are not only
unnecessary, but wrong, because we now lock the private lock and not the
snd_mixer one, which is what mixer_hwvol_mute_locked() and
mixer_hwvol_step_locked() expect. Retire the locked variants and call
the regular functions instead.
The unlocked variants take the mixer lock, which is now the PCM lock,
and reach uaudio_mixer_ctl_set(), which takes mixer_lock. Calling them
straight from uaudio_hid_rx_callback() would therefore take mixer_lock
and the PCM lock in the opposite order to the mixer ioctl path, so
record what the HID report asked for and perform the volume change at
the end of the callback, with mixer_lock dropped. The USB stack allows a
callback to drop its transfer mutex (see usbdi.9).
Sponsored by: The FreeBSD Foundation
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D59075
---
sys/dev/sound/pcm/mixer.c | 24 +++++-------------------
sys/dev/sound/pcm/mixer.h | 2 --
sys/dev/sound/usb/uaudio.c | 31 ++++++++++++++++++++++++++++---
3 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index 1c76ace34042..bad464538609 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -743,12 +743,6 @@ mixer_hwvol_init(device_t dev)
return 0;
}
-void
-mixer_hwvol_mute_locked(struct snd_mixer *m)
-{
- mix_setmutedevs(m, m->mutedevs ^ (1 << m->hwvol_mixer));
-}
-
void
mixer_hwvol_mute(device_t dev)
{
@@ -756,17 +750,19 @@ mixer_hwvol_mute(device_t dev)
m = mixer_get_devt(dev);
mtx_lock(m->lock);
- mixer_hwvol_mute_locked(m);
+ mix_setmutedevs(m, m->mutedevs ^ (1 << m->hwvol_mixer));
mtx_unlock(m->lock);
}
void
-mixer_hwvol_step_locked(struct snd_mixer *m, int left_step, int right_step)
+mixer_hwvol_step(device_t dev, int left_step, int right_step)
{
+ struct snd_mixer *m;
int level, left, right;
+ m = mixer_get_devt(dev);
+ mtx_lock(m->lock);
level = mixer_get(m, m->hwvol_mixer);
-
if (level != -1) {
left = level & 0xff;
right = (level >> 8) & 0xff;
@@ -783,16 +779,6 @@ mixer_hwvol_step_locked(struct snd_mixer *m, int left_step, int right_step)
mixer_set(m, m->hwvol_mixer, m->mutedevs, left | right << 8);
}
-}
-
-void
-mixer_hwvol_step(device_t dev, int left_step, int right_step)
-{
- struct snd_mixer *m;
-
- m = mixer_get_devt(dev);
- mtx_lock(m->lock);
- mixer_hwvol_step_locked(m, left_step, right_step);
mtx_unlock(m->lock);
}
diff --git a/sys/dev/sound/pcm/mixer.h b/sys/dev/sound/pcm/mixer.h
index d26895a41ba0..7c9d120463ca 100644
--- a/sys/dev/sound/pcm/mixer.h
+++ b/sys/dev/sound/pcm/mixer.h
@@ -72,9 +72,7 @@ int mixer_ioctl_cmd(struct cdev *i_dev, unsigned long cmd, caddr_t arg,
int mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi);
int mixer_hwvol_init(device_t dev);
-void mixer_hwvol_mute_locked(struct snd_mixer *m);
void mixer_hwvol_mute(device_t dev);
-void mixer_hwvol_step_locked(struct snd_mixer *m, int l_step, int r_step);
void mixer_hwvol_step(device_t dev, int left_step, int right_step);
int mix_set(struct snd_mixer *m, unsigned int dev, unsigned int left, unsigned int right);
diff --git a/sys/dev/sound/usb/uaudio.c b/sys/dev/sound/usb/uaudio.c
index 4e1d7c4c89d5..80c35aca89bc 100644
--- a/sys/dev/sound/usb/uaudio.c
+++ b/sys/dev/sound/usb/uaudio.c
@@ -6245,9 +6245,13 @@ uaudio_hid_rx_callback(struct usb_xfer *xfer, usb_error_t error)
struct snd_mixer *m;
uint8_t id;
int actlen;
+ bool mute, volume_up, volume_down;
usbd_xfer_status(xfer, &actlen, NULL, NULL, NULL);
+ m = NULL;
+ mute = volume_up = volume_down = false;
+
switch (USB_GET_STATE(xfer)) {
case USB_ST_TRANSFERRED:
DPRINTF("actlen=%d\n", actlen);
@@ -6269,7 +6273,7 @@ uaudio_hid_rx_callback(struct usb_xfer *xfer, usb_error_t error)
&sc->sc_hid.mute_loc)) {
DPRINTF("Mute toggle\n");
- mixer_hwvol_mute_locked(m);
+ mute = true;
}
if ((sc->sc_hid.flags & UAUDIO_HID_HAS_VOLUME_UP) &&
@@ -6278,7 +6282,7 @@ uaudio_hid_rx_callback(struct usb_xfer *xfer, usb_error_t error)
&sc->sc_hid.volume_up_loc)) {
DPRINTF("Volume Up\n");
- mixer_hwvol_step_locked(m, 1, 1);
+ volume_up = true;
}
if ((sc->sc_hid.flags & UAUDIO_HID_HAS_VOLUME_DOWN) &&
@@ -6287,7 +6291,7 @@ uaudio_hid_rx_callback(struct usb_xfer *xfer, usb_error_t error)
&sc->sc_hid.volume_down_loc)) {
DPRINTF("Volume Down\n");
- mixer_hwvol_step_locked(m, -1, -1);
+ volume_down = true;
}
case USB_ST_SETUP:
@@ -6308,6 +6312,27 @@ tr_setup:
}
break;
}
+
+ if (!mute && !volume_up && !volume_down)
+ return;
+
+ /*
+ * The mixer_hwvol_*() functions take the mixer lock, which is the PCM
+ * lock, and end up in uaudio_mixer_ctl_set(), which takes the
+ * mixer_lock this callback is entered with. Acquiring the two in that
+ * order here would reverse the order taken by the mixer ioctl path
+ * (PCM lock first, then mixer_lock), so drop mixer_lock for the
+ * duration. The USB stack explicitly allows a callback to drop its
+ * transfer mutex, and usbd_transfer_drain() accounts for it.
+ */
+ mtx_unlock(&sc->sc_child[0].mixer_lock);
+ if (mute)
+ mixer_hwvol_mute(m->dev);
+ if (volume_up)
+ mixer_hwvol_step(m->dev, 1, 1);
+ if (volume_down)
+ mixer_hwvol_step(m->dev, -1, -1);
+ mtx_lock(&sc->sc_child[0].mixer_lock);
}
static int