lock sound device when adding/removing channels

implement setblocksize for vchans
don't panic when doing certain ioctls or aborting on a vchan
xmms now works with vchans

sys/dev/sound/pcm/channel.c

@@ -156,7 +156,7 @@ chn_wrupdate(struct pcm_channel *c)
 	CHN_LOCKASSERT(c);
 	KASSERT(c->direction == PCMDIR_PLAY, ("chn_wrupdate on bad channel"));
 
-	if ((c->flags & CHN_F_MAPPED) || !(c->flags & CHN_F_TRIGGERED))
+	if ((c->flags & (CHN_F_MAPPED | CHN_F_VIRTUAL)) || !(c->flags & CHN_F_TRIGGERED))
 		return;
 	chn_dmaupdate(c);
 	ret = chn_wrfeed(c);
@@ -533,7 +533,8 @@ chn_abort(struct pcm_channel *c)
 	/* kill the channel */
 	chn_trigger(c, PCMTRIG_ABORT);
 	sndbuf_setrun(b, 0);
-	chn_dmaupdate(c);
+	if (!(c->flags & CHN_F_VIRTUAL))
+		chn_dmaupdate(c);
     	missing = sndbuf_getready(bs) + sndbuf_getready(b);
 
 	c->flags &= ~CHN_F_ABORTING;
@@ -1056,10 +1057,19 @@ chn_notify(struct pcm_channel *c, u_int32_t flags)
 {
 	struct pcmchan_children *pce;
 	struct pcm_channel *child;
+	int run;
 
 	if (SLIST_EMPTY(&c->children))
 		return ENODEV;
 
+	run = (c->flags & CHN_F_TRIGGERED)? 1 : 0;
+	/*
+	 * if the hwchan is running, we can't change its rate, format or
+	 * blocksize
+	 */
+	if (run)
+		flags &= CHN_N_VOLUME | CHN_N_TRIGGER;
+
 	if (flags & CHN_N_RATE) {
 		/*
 		 * we could do something here, like scan children and decide on
@@ -1092,21 +1102,21 @@ chn_notify(struct pcm_channel *c, u_int32_t flags)
 		chn_setblocksize(c, 2, blksz);
 	}
 	if (flags & CHN_N_TRIGGER) {
-		int run;
+		int nrun;
 		/*
 		 * scan the children, and figure out if any are running
 		 * if so, we need to be running, otherwise we need to be stopped
 		 * if we aren't in our target sstate, move to it
 		 */
-		run = 0;
+		nrun = 0;
 		SLIST_FOREACH(pce, &c->children, link) {
 			child = pce->channel;
 			if (child->flags & CHN_F_TRIGGERED)
-				run = 1;
+				nrun = 1;
 		}
-		if (run && !(c->flags & CHN_F_TRIGGERED))
+		if (nrun && !run)
 			chn_start(c, 1);
-		if (!run && (c->flags & CHN_F_TRIGGERED))
+		if (!nrun && run)
 			chn_abort(c);
 	}
 	return 0;

sys/dev/sound/pcm/sound.c

@@ -174,28 +174,41 @@ pcm_chnalloc(struct snddev_info *d, int direction)
 	struct pcm_channel *c;
     	struct snddev_channel *sce;
 
+	snd_mtxlock(d->lock);
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
+		CHN_LOCK(c);
 		if ((c->direction == direction) && !(c->flags & CHN_F_BUSY)) {
 			c->flags |= CHN_F_BUSY;
+			CHN_UNLOCK(c);
+			snd_mtxunlock(d->lock);
 			return c;
 		}
+		CHN_UNLOCK(c);
 	}
+	snd_mtxunlock(d->lock);
 	return NULL;
 }
 
 int
 pcm_chnfree(struct pcm_channel *c)
 {
+	CHN_LOCK(c);
 	c->flags &= ~CHN_F_BUSY;
+	CHN_UNLOCK(c);
 	return 0;
 }
 
 int
 pcm_chnref(struct pcm_channel *c, int ref)
 {
+	int r;
+
+	CHN_LOCK(c);
 	c->refcount += ref;
-	return c->refcount;
+	r = c->refcount;
+	CHN_UNLOCK(c);
+	return r;
 }
 
 #ifdef USING_DEVFS
@@ -337,6 +350,7 @@ pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
 		return ENOMEM;
 	}
 
+	snd_mtxlock(d->lock);
 	sce->channel = ch;
 	SLIST_INSERT_HEAD(&d->channels, sce, link);
 
@@ -352,6 +366,7 @@ pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
     	if (d->chancount++ == 0)
 		pcm_makelinks(NULL);
 #endif
+	snd_mtxunlock(d->lock);
 
 	return 0;
 }
@@ -363,10 +378,12 @@ pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch)
     	int unit = device_get_unit(d->dev);
 	dev_t pdev;
 
+	snd_mtxlock(d->lock);
 	SLIST_FOREACH(sce, &d->channels, link) {
 		if (sce->channel == ch)
 			goto gotit;
 	}
+	snd_mtxunlock(d->lock);
 	return EINVAL;
 gotit:
 	d->chancount--;
@@ -383,6 +400,7 @@ pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch)
 	destroy_dev(pdev);
 	pdev = makedev(CDEV_MAJOR, PCMMKMINOR(unit, SND_DEV_AUDIO, d->chancount));
 	destroy_dev(pdev);
+	snd_mtxunlock(d->lock);
 
 	return 0;
 }
@@ -414,7 +432,9 @@ pcm_killchan(device_t dev)
     	struct snddev_info *d = device_get_softc(dev);
     	struct snddev_channel *sce;
 
+	snd_mtxlock(d->lock);
 	sce = SLIST_FIRST(&d->channels);
+	snd_mtxunlock(d->lock);
 
 	return pcm_chn_remove(d, sce->channel);
 }
@@ -423,7 +443,10 @@ int
 pcm_setstatus(device_t dev, char *str)
 {
     	struct snddev_info *d = device_get_softc(dev);
+
+	snd_mtxlock(d->lock);
 	strncpy(d->status, str, SND_STATUSLEN);
+	snd_mtxunlock(d->lock);
 	return 0;
 }
 
@@ -431,6 +454,7 @@ u_int32_t
 pcm_getflags(device_t dev)
 {
     	struct snddev_info *d = device_get_softc(dev);
+
 	return d->flags;
 }
 
@@ -438,6 +462,7 @@ void
 pcm_setflags(device_t dev, u_int32_t val)
 {
     	struct snddev_info *d = device_get_softc(dev);
+
 	d->flags = val;
 }
 
@@ -445,6 +470,7 @@ void *
 pcm_getdevinfo(device_t dev)
 {
     	struct snddev_info *d = device_get_softc(dev);
+
 	return d->devinfo;
 }
 
@@ -455,6 +481,8 @@ pcm_register(device_t dev, void *devinfo, int numplay, int numrec)
     	int sz, unit = device_get_unit(dev);
     	struct snddev_info *d = device_get_softc(dev);
 
+	d->lock = snd_mtxcreate(device_get_nameunit(dev));
+	snd_mtxlock(d->lock);
     	if (!pcm_devclass) {
     		pcm_devclass = device_get_devclass(dev);
 		status_dev = make_dev(&snd_cdevsw, PCMMKMINOR(0, SND_DEV_STATUS, 0),
@@ -492,13 +520,16 @@ pcm_register(device_t dev, void *devinfo, int numplay, int numrec)
 		goto no;
 	}
 #endif
-#if 0
+#if 1
 	vchan_initsys(d);
 #endif
+	snd_mtxunlock(d->lock);
     	return 0;
 no:
 	if (d->aplay) free(d->aplay, M_DEVBUF);
 	if (d->arec) free(d->arec, M_DEVBUF);
+	/* snd_mtxunlock(d->lock); */
+	snd_mtxfree(d->lock);
 	return ENXIO;
 }
 
@@ -510,14 +541,17 @@ pcm_unregister(device_t dev)
     	struct snddev_channel *sce;
 	dev_t pdev;
 
+	snd_mtxlock(d->lock);
 	SLIST_FOREACH(sce, &d->channels, link) {
 		if (sce->channel->refcount > 0) {
 			device_printf(dev, "unregister: channel busy");
+			snd_mtxunlock(d->lock);
 			return EBUSY;
 		}
 	}
 	if (mixer_isbusy(d->mixer)) {
 		device_printf(dev, "unregister: mixer busy");
+		snd_mtxunlock(d->lock);
 		return EBUSY;
 	}
 
@@ -539,6 +573,8 @@ pcm_unregister(device_t dev)
 	chn_kill(d->fakechan);
 	fkchan_kill(d->fakechan);
 
+	/* snd_mtxunlock(d->lock); */
+	snd_mtxfree(d->lock);
 	return 0;
 }
 
@@ -766,7 +802,9 @@ status_init(struct sbuf *s)
     	struct snddev_info *d;
     	struct snddev_channel *sce;
 	struct pcm_channel *c;
+#ifdef SNDSTAT_VERBOSE
 	struct pcm_feeder *f;
+#endif
 
 	sbuf_printf(s, "FreeBSD Audio Driver (newpcm) %s %s\nInstalled devices:\n",
 		 	__DATE__, __TIME__);
@@ -775,6 +813,7 @@ status_init(struct sbuf *s)
 		d = devclass_get_softc(pcm_devclass, i);
 		if (!d)
 			continue;
+		snd_mtxlock(d->lock);
 		dev = devclass_get_device(pcm_devclass, i);
 		sbuf_printf(s, "pcm%d: <%s> %s", i, device_get_desc(dev), d->status);
 		if (d->chancount > 0) {
@@ -824,6 +863,7 @@ status_init(struct sbuf *s)
 #endif
 		} else
 			sbuf_printf(s, " (mixer only)\n");
+		snd_mtxunlock(d->lock);
     	}
 	sbuf_finish(s);
     	return sbuf_len(s);

sys/dev/sound/pcm/sound.h

@@ -116,7 +116,7 @@ struct snddev_info {
 	char status[SND_STATUSLEN];
 	struct sysctl_ctx_list sysctl_tree;
 	struct sysctl_oid *sysctl_tree_top;
-	struct mtx mutex;
+	void *lock;
 };
 
 #ifndef ISADMA_WRITE

sys/dev/sound/pcm/vchan.c

@@ -31,7 +31,7 @@
 #include "feeder_if.h"
 
 struct vchinfo {
-	u_int32_t spd, fmt, blksz, run;
+	u_int32_t spd, fmt, blksz, bps, run;
 	struct pcm_channel *channel, *parent;
 	struct pcmchan_caps caps;
 };
@@ -94,8 +94,10 @@ feed_vchan_s16(struct pcm_feeder *f, struct pcm_channel *c, u_int8_t *b, u_int32
 	bzero(tmp, count);
 	SLIST_FOREACH(cce, &c->children, link) {
 		ch = cce->channel;
-		cnt = FEEDER_FEED(ch->feeder, ch, (u_int8_t *)tmp, count, ch->bufsoft);
-		vchan_mix_s16(dst, tmp, cnt / 2);
+		if (ch->flags & CHN_F_TRIGGERED) {
+			cnt = FEEDER_FEED(ch->feeder, ch, (u_int8_t *)tmp, count, ch->bufsoft);
+			vchan_mix_s16(dst, tmp, cnt / 2);
+		}
 	}
 
 	return count;
@@ -146,6 +148,10 @@ vchan_setformat(kobj_t obj, void *data, u_int32_t format)
 	struct pcm_channel *parent = ch->parent;
 
 	ch->fmt = format;
+	ch->bps = 1;
+	ch->bps <<= (ch->fmt & AFMT_STEREO)? 1 : 0;
+	ch->bps <<= (ch->fmt & AFMT_16BIT)? 1 : 0;
+	ch->bps <<= (ch->fmt & AFMT_32BIT)? 2 : 0;
 	chn_notify(parent, CHN_N_FORMAT);
 	return 0;
 }
@@ -166,9 +172,17 @@ vchan_setblocksize(kobj_t obj, void *data, u_int32_t blocksize)
 {
 	struct vchinfo *ch = data;
 	struct pcm_channel *parent = ch->parent;
+	int prate, crate;
 
 	ch->blksz = blocksize;
 	chn_notify(parent, CHN_N_BLOCKSIZE);
+
+	crate = ch->spd * ch->bps;
+	prate = sndbuf_getspd(parent->bufhard) * sndbuf_getbps(parent->bufhard);
+	blocksize = sndbuf_getblksz(parent->bufhard);
+	blocksize *= prate;
+	blocksize /= crate;
+
 	return blocksize;
 }
 
@@ -222,17 +236,23 @@ vchan_create(struct pcm_channel *parent)
 	struct pcm_channel *child;
 	int err, first;
 
-	if (!(parent->flags & CHN_F_BUSY))
+	CHN_LOCK(parent);
+	if (!(parent->flags & CHN_F_BUSY)) {
+		CHN_UNLOCK(parent);
 		return EBUSY;
+	}
 
 	pce = malloc(sizeof(*pce), M_DEVBUF, M_WAITOK | M_ZERO);
-	if (!pce)
+	if (!pce) {
+		CHN_UNLOCK(parent);
 		return ENOMEM;
+	}
 
 	/* create a new playback channel */
 	child = pcm_chn_create(d, parent, &vchan_class, PCMDIR_VIRTUAL, parent);
 	if (!child) {
 		free(pce, M_DEVBUF);
+		CHN_UNLOCK(parent);
 		return ENODEV;
 	}
 
@@ -240,6 +260,7 @@ vchan_create(struct pcm_channel *parent)
 	/* add us to our parent channel's children */
 	pce->channel = child;
 	SLIST_INSERT_HEAD(&parent->children, pce, link);
+	CHN_UNLOCK(parent);
 
 	/* add us to our grandparent's channel list */
 	err = pcm_chn_add(d, child);
@@ -269,21 +290,27 @@ vchan_destroy(struct pcm_channel *c)
 	struct pcmchan_children *pce;
 	int err;
 
-	if (!(parent->flags & CHN_F_BUSY))
-		return EBUSY;
-	if (SLIST_EMPTY(&parent->children))
-		return EINVAL;
-
 	/* remove us from our grantparent's channel list */
 	err = pcm_chn_remove(d, c);
 	if (err)
 		return err;
 
+	CHN_LOCK(parent);
+	if (!(parent->flags & CHN_F_BUSY)) {
+		CHN_UNLOCK(parent);
+		return EBUSY;
+	}
+	if (SLIST_EMPTY(&parent->children)) {
+		CHN_UNLOCK(parent);
+		return EINVAL;
+	}
+
 	/* remove us from our parent's children list */
 	SLIST_FOREACH(pce, &parent->children, link) {
 		if (pce->channel == c)
 			goto gotch;
 	}
+	CHN_UNLOCK(parent);
 	return EINVAL;
 gotch:
 	SLIST_REMOVE(&parent->children, pce, pcmchan_children, link);
@@ -291,6 +318,7 @@ vchan_destroy(struct pcm_channel *c)
 
 	if (SLIST_EMPTY(&parent->children))
 		parent->flags &= ~CHN_F_BUSY;
+	CHN_UNLOCK(parent);
 	/* destroy ourselves */
 	err = pcm_chn_destroy(c);
 
@@ -308,6 +336,7 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 
 	d = oidp->oid_arg1;
 
+	snd_mtxlock(d->lock);
 	cnt = 0;
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
@@ -341,6 +370,7 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 				goto addok;
 addskip:
 			}
+			snd_mtxunlock(d->lock);
 			return EBUSY;
 addok:
 			c->flags |= CHN_F_BUSY;
@@ -356,6 +386,7 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 					if ((c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == CHN_F_VIRTUAL)
 						goto remok;
 				}
+				snd_mtxunlock(d->lock);
 				return EINVAL;
 remok:
 				err = vchan_destroy(c);
@@ -365,6 +396,7 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 		}
 	}
 
+	snd_mtxunlock(d->lock);
 	return err;
 }
 #endif