diff --git a/share/man/man4/snd_uaudio.4 b/share/man/man4/snd_uaudio.4 --- a/share/man/man4/snd_uaudio.4 +++ b/share/man/man4/snd_uaudio.4 @@ -86,16 +86,6 @@ by .An Hiten Pandya Aq Mt hmp@FreeBSD.org . .Sh BUGS -The -.Tn PCM -framework in -.Fx -only supports synchronous device detach. -That means all mixer and DSP character devices belonging to a given -USB audio device must be closed when receiving an error on a DSP read, -a DSP write or a DSP IOCTL request. -Else the USB audio driver will wait for this to happen, preventing -enumeration of new devices on the parenting USB controller. .Pp Some USB audio devices might refuse to work properly unless the sample rate is configured the same for both recording and playback, even if diff --git a/sys/dev/sound/clone.h b/sys/dev/sound/clone.h --- a/sys/dev/sound/clone.h +++ b/sys/dev/sound/clone.h @@ -102,6 +102,7 @@ int snd_clone_enable(struct snd_clone *); int snd_clone_disable(struct snd_clone *); int snd_clone_getsize(struct snd_clone *); +int snd_clone_getrefcount(struct snd_clone *); int snd_clone_getmaxunit(struct snd_clone *); int snd_clone_setmaxunit(struct snd_clone *, int); int snd_clone_getdeadline(struct snd_clone *); @@ -112,6 +113,8 @@ uint32_t snd_clone_setdevflags(struct cdev *, uint32_t); int snd_clone_gc(struct snd_clone *); void snd_clone_destroy(struct snd_clone *); +void snd_clone_sleep(struct snd_clone *, struct mtx *); +void snd_clone_wakeup(struct snd_clone *); int snd_clone_acquire(struct cdev *); int snd_clone_release(struct cdev *); int snd_clone_ref(struct cdev *); diff --git a/sys/dev/sound/clone.c b/sys/dev/sound/clone.c --- a/sys/dev/sound/clone.c +++ b/sys/dev/sound/clone.c @@ -37,10 +37,7 @@ #include "opt_snd.h" #endif -#if defined(SND_DIAGNOSTIC) || defined(SND_DEBUG) #include -#endif - #include /* @@ -80,6 +77,7 @@ struct snd_clone { TAILQ_HEAD(link_head, snd_clone_entry) head; struct timespec tsp; + struct cv cv; int refcount; int size; int typemask; @@ -115,6 +113,7 @@ ("invalid clone flags=0x%08x", flags)); c = malloc(sizeof(*c), M_DEVBUF, M_WAITOK | M_ZERO); + cv_init(&c->cv, "clone"); c->refcount = 0; c->size = 0; c->typemask = typemask; @@ -188,6 +187,14 @@ return (c->size); } +int +snd_clone_getrefcount(struct snd_clone *c) +{ + SND_CLONE_ASSERT(c != NULL, ("NULL snd_clone")); + + return (c->refcount); +} + int snd_clone_getmaxunit(struct snd_clone *c) { @@ -342,7 +349,7 @@ ce->pid = -1; } else { TAILQ_REMOVE(&c->head, ce, link); - destroy_dev(ce->devt); + destroy_dev_sched(ce->devt); free(ce, M_DEVBUF); c->size--; } @@ -370,9 +377,26 @@ ce = tmp; } + cv_destroy(&c->cv); free(c, M_DEVBUF); } +void +snd_clone_sleep(struct snd_clone *c, struct mtx *lock) +{ + SND_CLONE_ASSERT(c != NULL, ("NULL snd_clone")); + + cv_wait(&c->cv, lock); +} + +void +snd_clone_wakeup(struct snd_clone *c) +{ + SND_CLONE_ASSERT(c != NULL, ("NULL snd_clone")); + + cv_broadcastpri(&c->cv, PRIBIO); +} + /* * snd_clone_acquire() : The vital part of concurrency management. Must be * called somewhere at the beginning of open() handler. ENODEV is not really @@ -489,6 +513,9 @@ (c->refcount == 0 && (c->flags & SND_CLONE_GC_LASTREF)))) (void)snd_clone_gc(c); + if (c->refcount == 0) + snd_clone_wakeup(c); + return (c->refcount); } diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c --- a/sys/dev/sound/pcm/dsp.c +++ b/sys/dev/sound/pcm/dsp.c @@ -796,14 +796,7 @@ * cleanup process. */ (void)snd_clone_release(i_dev); - - /* - * destroy_dev() might sleep, so release pcm lock - * here and rely on pcm cv serialization. - */ - PCM_UNLOCK(d); (void)snd_clone_unref(i_dev); - PCM_LOCK(d); PCM_RELEASE(d); PCM_UNLOCK(d); diff --git a/sys/dev/sound/pcm/mixer.h b/sys/dev/sound/pcm/mixer.h --- a/sys/dev/sound/pcm/mixer.h +++ b/sys/dev/sound/pcm/mixer.h @@ -30,6 +30,7 @@ #ifndef _PCM_MIXER_H_ #define _PCM_MIXER_H_ +struct snd_mixer *mixer_get_sndmixer(device_t dev); struct snd_mixer *mixer_create(device_t dev, kobj_class_t cls, void *devinfo, const char *desc); int mixer_delete(struct snd_mixer *m); @@ -46,6 +47,8 @@ void mixer_hwvol_step(device_t dev, int left_step, int right_step); int mixer_busy(struct snd_mixer *m); +void mixer_sleep(struct snd_mixer *m, struct mtx *lock); +void mixer_wakeup(struct snd_mixer *m); int mix_get_locked(struct snd_mixer *m, u_int dev, int *pleft, int *pright); int mix_set_locked(struct snd_mixer *m, u_int dev, int left, int right); diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c --- a/sys/dev/sound/pcm/mixer.c +++ b/sys/dev/sound/pcm/mixer.c @@ -71,6 +71,7 @@ * mixer applications should update their displays. */ int modify_counter; + struct cv cv; }; static u_int16_t snd_mixerdefaults[SOUND_MIXER_NRDEVICES] = { @@ -122,6 +123,17 @@ return snddev->mixer_dev; } +struct snd_mixer * +mixer_get_sndmixer(device_t dev) +{ + struct snd_mixer *m; + + m = mixer_get_devt(dev)->si_drv1; + KASSERT(m != NULL, ("NULL snd_mixer")); + + return (m); +} + static int mixer_lookup(char *devname) { @@ -996,6 +1008,22 @@ return (m->busy); } +void +mixer_sleep(struct snd_mixer *m, struct mtx *lock) +{ + KASSERT(m != NULL, ("NULL snd_mixer")); + + cv_wait(&m->cv, lock); +} + +void +mixer_wakeup(struct snd_mixer *m) +{ + KASSERT(m != NULL, ("NULL snd_mixer")); + + cv_broadcastpri(&m->cv, PRIBIO); +} + int mix_set(struct snd_mixer *m, u_int dev, u_int left, u_int right) { @@ -1113,6 +1141,7 @@ snd_mtxlock(m->lock); ret = (m->busy == 0) ? EBADF : 0; m->busy = 0; + mixer_wakeup(m); snd_mtxunlock(m->lock); return (ret); diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h --- a/sys/dev/sound/pcm/sound.h +++ b/sys/dev/sound/pcm/sound.h @@ -399,6 +399,7 @@ #define PCM_MODE_PLAY 0x02 #define PCM_MODE_REC 0x04 +#define PCM_LOCKPTR(d) ((d)->lock) #define PCM_LOCKOWNED(d) mtx_owned((d)->lock) #define PCM_LOCK(d) mtx_lock((d)->lock) #define PCM_UNLOCK(d) mtx_unlock((d)->lock) diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c --- a/sys/dev/sound/pcm/sound.c +++ b/sys/dev/sound/pcm/sound.c @@ -1176,37 +1176,42 @@ CHN_FOREACH(ch, d, channels.pcm) { CHN_LOCK(ch); - if (ch->refcount > 0) { - device_printf(dev, - "unregister: channel %s busy (pid %d)\n", - ch->name, ch->pid); - CHN_UNLOCK(ch); - PCM_RELEASE_QUICK(d); - return (EBUSY); + if (ch->flags & CHN_F_SLEEPING) { + /* + * We are detaching, so do not wait for the timeout in + * chn_read()/chn_write(). Wake up the thread and kill + * the channel immediately. + */ + CHN_BROADCAST(&ch->intr_cv); + ch->flags |= CHN_F_DEAD; } CHN_UNLOCK(ch); } if (d->clones != NULL) { - if (snd_clone_busy(d->clones) != 0) { - device_printf(dev, "unregister: clone busy\n"); + while (snd_clone_getrefcount(d->clones) > 0) { + /* + * Wait for all sleeping threads to drain so that we + * can destroy the clones safely. + */ PCM_RELEASE_QUICK(d); - return (EBUSY); - } else { PCM_LOCK(d); - (void)snd_clone_disable(d->clones); + snd_clone_sleep(d->clones, PCM_LOCKPTR(d)); PCM_UNLOCK(d); + PCM_ACQUIRE_QUICK(d); } + PCM_LOCK(d); + (void)snd_clone_disable(d->clones); + PCM_UNLOCK(d); } - if (mixer_uninit(dev) == EBUSY) { - device_printf(dev, "unregister: mixer busy\n"); + while (mixer_uninit(dev) == EBUSY) { + /* Wait until the mixer is closed to destroy it safely. */ + PCM_RELEASE_QUICK(d); PCM_LOCK(d); - if (d->clones != NULL) - (void)snd_clone_enable(d->clones); - PCM_RELEASE(d); + mixer_sleep(mixer_get_sndmixer(dev), PCM_LOCKPTR(d)); PCM_UNLOCK(d); - return (EBUSY); + PCM_ACQUIRE_QUICK(d); } /* remove /dev/sndstat entry first */ diff --git a/sys/dev/sound/usb/uaudio.c b/sys/dev/sound/usb/uaudio.c --- a/sys/dev/sound/usb/uaudio.c +++ b/sys/dev/sound/usb/uaudio.c @@ -1250,20 +1250,12 @@ unsigned i = uaudio_get_child_index_by_dev(sc, dev); int error = 0; -repeat: - if (sc->sc_child[i].pcm_registered) { + if (sc->sc_child[i].pcm_registered) error = pcm_unregister(dev); - } else { - if (sc->sc_child[i].mixer_init) - error = mixer_uninit(dev); - } + else if (sc->sc_child[i].mixer_init) + error = mixer_uninit(dev); - if (error) { - device_printf(dev, "Waiting for sound application to exit!\n"); - usb_pause_mtx(NULL, 2 * hz); - goto repeat; /* try again */ - } - return (0); /* success */ + return (error); } static int