The branch stable/14 has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=13ee4d7f332ee1948f2886aeec70e97a4cf5457e
commit 13ee4d7f332ee1948f2886aeec70e97a4cf5457e Author: Christos Margiolis <[email protected]> AuthorDate: 2024-05-06 18:26:37 +0000 Commit: Christos Margiolis <[email protected]> CommitDate: 2024-05-17 19:30:34 +0000 sound: Merge pcm_chn_create() and chn_init() Follow-up of b3ea087c05d8c75978a302cbb3fa92ce1afa3e49 ("sound: Merge pcm_chn_destroy() and chn_kill()") While here, add device_printf()'s to all failure points. Also fix an existing bug where we'd unlock an already unlocked channel, in case we went to "out" (now "out2") before locking the channel. Sponsored by: The FreeBSD Foundation MFC after: 1 week Reviewed by: dev_submerge.ch Differential Revision: https://reviews.freebsd.org/D44993 (cherry picked from commit 2e9962ef57044b191431fe9228841e1415574d82) --- sys/dev/sound/pcm/channel.c | 167 ++++++++++++++++++++++++++++++++++++++------ sys/dev/sound/pcm/channel.h | 3 +- sys/dev/sound/pcm/sound.c | 114 +----------------------------- sys/dev/sound/pcm/sound.h | 1 - sys/dev/sound/pcm/vchan.c | 2 +- 5 files changed, 151 insertions(+), 136 deletions(-) diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c index 859476f212ae..996cb0cb04f7 100644 --- a/sys/dev/sound/pcm/channel.c +++ b/sys/dev/sound/pcm/channel.c @@ -1157,14 +1157,112 @@ chn_reset(struct pcm_channel *c, uint32_t fmt, uint32_t spd) return r; } -int -chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction) +struct pcm_channel * +chn_init(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, + int dir, int num, void *devinfo) { + struct pcm_channel *c; struct feeder_class *fc; struct snd_dbuf *b, *bs; - int i, ret; + char *dirs, *devname, buf[CHN_NAMELEN]; + int i, ret, direction, rpnum, *pnum, max, type, unit; + PCM_BUSYASSERT(d); + PCM_LOCKASSERT(d); + KASSERT(num >= -1, ("invalid num=%d", num)); + + switch (dir) { + case PCMDIR_PLAY: + dirs = "play"; + direction = PCMDIR_PLAY; + pnum = &d->playcount; + type = SND_DEV_DSPHW_PLAY; + max = SND_MAXHWCHAN; + break; + case PCMDIR_PLAY_VIRTUAL: + dirs = "virtual_play"; + direction = PCMDIR_PLAY; + pnum = &d->pvchancount; + type = SND_DEV_DSPHW_VPLAY; + max = SND_MAXVCHANS; + break; + case PCMDIR_REC: + dirs = "record"; + direction = PCMDIR_REC; + pnum = &d->reccount; + type = SND_DEV_DSPHW_REC; + max = SND_MAXHWCHAN; + break; + case PCMDIR_REC_VIRTUAL: + dirs = "virtual_record"; + direction = PCMDIR_REC; + pnum = &d->rvchancount; + type = SND_DEV_DSPHW_VREC; + max = SND_MAXVCHANS; + break; + default: + device_printf(d->dev, + "%s(): invalid channel direction: %d\n", + __func__, dir); + goto out1; + } + + unit = (num == -1) ? 0 : num; + + if (*pnum >= max || unit >= max) { + device_printf(d->dev, "%s(): unit=%d or pnum=%d >= than " + "max=%d\n", __func__, unit, *pnum, max); + goto out1; + } + + rpnum = 0; + + CHN_FOREACH(c, d, channels.pcm) { + if (c->type != type) + continue; + if (unit == c->unit && num != -1) { + device_printf(d->dev, + "%s(): channel num=%d already allocated\n", + __func__, unit); + goto out1; + } + unit++; + if (unit >= max) { + device_printf(d->dev, + "%s(): chan=%d >= max=%d\n", __func__, unit, max); + goto out1; + } + rpnum++; + } + + if (*pnum != rpnum) { + device_printf(d->dev, + "%s(): pnum screwed: dirs=%s pnum=%d rpnum=%d\n", + __func__, dirs, *pnum, rpnum); + goto out1; + } + + PCM_UNLOCK(d); + c = malloc(sizeof(*c), M_DEVBUF, M_WAITOK | M_ZERO); + c->methods = kobj_create(cls, M_DEVBUF, M_WAITOK | M_ZERO); + c->type = type; + c->unit = unit; + c->pid = -1; + strlcpy(c->comm, CHN_COMM_UNUSED, sizeof(c->comm)); + c->parentsnddev = d; + c->parentchannel = parent; + c->dev = d->dev; + c->trigger = PCMTRIG_STOP; chn_lockinit(c, dir); + devname = dsp_unit2name(buf, sizeof(buf), c); + if (devname == NULL) { + ret = EINVAL; + device_printf(d->dev, "%s(): failed to create channel name", + __func__); + goto out2; + } + snprintf(c->name, sizeof(c->name), "%s:%s:%s", + device_get_nameunit(c->dev), dirs, devname); b = NULL; bs = NULL; @@ -1177,20 +1275,31 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction) ret = ENOMEM; b = sndbuf_create(c->dev, c->name, "primary", c); - if (b == NULL) - goto out; + if (b == NULL) { + device_printf(d->dev, "%s(): failed to create hardware buffer\n", + __func__); + goto out2; + } bs = sndbuf_create(c->dev, c->name, "secondary", c); - if (bs == NULL) - goto out; + if (bs == NULL) { + device_printf(d->dev, "%s(): failed to create software buffer\n", + __func__); + goto out2; + } CHN_LOCK(c); ret = EINVAL; fc = feeder_getclass(NULL); - if (fc == NULL) - goto out; - if (chn_addfeeder(c, fc, NULL)) - goto out; + if (fc == NULL) { + device_printf(d->dev, "%s(): failed to get feeder class\n", + __func__); + goto out2; + } + if (chn_addfeeder(c, fc, NULL)) { + device_printf(d->dev, "%s(): failed to add feeder\n", __func__); + goto out2; + } /* * XXX - sndbuf_setup() & sndbuf_resize() expect to be called @@ -1226,12 +1335,17 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction) CHN_UNLOCK(c); /* XXX - Unlock for CHANNEL_INIT() malloc() call */ c->devinfo = CHANNEL_INIT(c->methods, devinfo, b, c, direction); CHN_LOCK(c); - if (c->devinfo == NULL) - goto out; + if (c->devinfo == NULL) { + device_printf(d->dev, "%s(): NULL devinfo\n", __func__); + goto out2; + } ret = ENOMEM; - if ((sndbuf_getsize(b) == 0) && ((c->flags & CHN_F_VIRTUAL) == 0)) - goto out; + if ((sndbuf_getsize(b) == 0) && ((c->flags & CHN_F_VIRTUAL) == 0)) { + device_printf(d->dev, "%s(): hardware buffer's size is 0\n", + __func__); + goto out2; + } ret = 0; c->direction = direction; @@ -1251,12 +1365,14 @@ chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction) bs->shadbuf = malloc(bs->sl, M_DEVBUF, M_NOWAIT); if (bs->shadbuf == NULL) { ret = ENOMEM; - goto out; + device_printf(d->dev, "%s(): failed to create shadow " + "buffer\n", __func__); + goto out2; } } - -out: - CHN_UNLOCK(c); +out2: + if (CHN_LOCKOWNED(c)) + CHN_UNLOCK(c); if (ret) { if (c->devinfo) { if (CHANNEL_FREE(c->methods, c->devinfo)) @@ -1270,10 +1386,19 @@ out: c->flags |= CHN_F_DEAD; chn_lockdestroy(c); - return ret; + PCM_LOCK(d); + + kobj_delete(c->methods, M_DEVBUF); + free(c, M_DEVBUF); + + return (NULL); } - return 0; + PCM_LOCK(d); + + return (c); +out1: + return (NULL); } void diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h index 698a1186924f..6aca089a7c4d 100644 --- a/sys/dev/sound/pcm/channel.h +++ b/sys/dev/sound/pcm/channel.h @@ -259,7 +259,8 @@ int chn_sync(struct pcm_channel *c, int threshold); int chn_flush(struct pcm_channel *c); int chn_poll(struct pcm_channel *c, int ev, struct thread *td); -int chn_init(struct pcm_channel *c, void *devinfo, int dir, int direction); +struct pcm_channel *chn_init(struct snddev_info *d, struct pcm_channel *parent, + kobj_class_t cls, int dir, int num, void *devinfo); void chn_kill(struct pcm_channel *c); void chn_shutdown(struct pcm_channel *c); int chn_release(struct pcm_channel *c); diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c index 9d5eaf3f5ad7..cc19d119ac36 100644 --- a/sys/dev/sound/pcm/sound.c +++ b/sys/dev/sound/pcm/sound.c @@ -379,116 +379,6 @@ SYSCTL_PROC(_hw_snd, OID_AUTO, maxautovchans, sysctl_hw_snd_maxautovchans, "I", "maximum virtual channel"); -struct pcm_channel * -pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, int dir, int num, void *devinfo) -{ - struct pcm_channel *ch; - int direction, err, rpnum, *pnum, max; - int type, unit; - char *dirs, *devname, buf[CHN_NAMELEN]; - - PCM_BUSYASSERT(d); - PCM_LOCKASSERT(d); - KASSERT(num >= -1, ("invalid num=%d", num)); - - switch (dir) { - case PCMDIR_PLAY: - dirs = "play"; - direction = PCMDIR_PLAY; - pnum = &d->playcount; - type = SND_DEV_DSPHW_PLAY; - max = SND_MAXHWCHAN; - break; - case PCMDIR_PLAY_VIRTUAL: - dirs = "virtual_play"; - direction = PCMDIR_PLAY; - pnum = &d->pvchancount; - type = SND_DEV_DSPHW_VPLAY; - max = SND_MAXVCHANS; - break; - case PCMDIR_REC: - dirs = "record"; - direction = PCMDIR_REC; - pnum = &d->reccount; - type = SND_DEV_DSPHW_REC; - max = SND_MAXHWCHAN; - break; - case PCMDIR_REC_VIRTUAL: - dirs = "virtual_record"; - direction = PCMDIR_REC; - pnum = &d->rvchancount; - type = SND_DEV_DSPHW_VREC; - max = SND_MAXVCHANS; - break; - default: - return (NULL); - } - - unit = (num == -1) ? 0 : num; - - if (*pnum >= max || unit >= max) - return (NULL); - - rpnum = 0; - - CHN_FOREACH(ch, d, channels.pcm) { - if (ch->type != type) - continue; - if (unit == ch->unit && num != -1) { - device_printf(d->dev, - "channel num=%d allocated!\n", unit); - return (NULL); - } - unit++; - if (unit >= max) { - device_printf(d->dev, - "chan=%d > %d\n", unit, max); - return (NULL); - } - rpnum++; - } - - if (*pnum != rpnum) { - device_printf(d->dev, - "%s(): WARNING: pnum screwed : dirs=%s pnum=%d rpnum=%d\n", - __func__, dirs, *pnum, rpnum); - return (NULL); - } - - PCM_UNLOCK(d); - ch = malloc(sizeof(*ch), M_DEVBUF, M_WAITOK | M_ZERO); - ch->methods = kobj_create(cls, M_DEVBUF, M_WAITOK | M_ZERO); - ch->type = type; - ch->unit = unit; - ch->pid = -1; - strlcpy(ch->comm, CHN_COMM_UNUSED, sizeof(ch->comm)); - ch->parentsnddev = d; - ch->parentchannel = parent; - ch->dev = d->dev; - ch->trigger = PCMTRIG_STOP; - devname = dsp_unit2name(buf, sizeof(buf), ch); - if (devname == NULL) { - device_printf(d->dev, "Failed to query device name"); - kobj_delete(ch->methods, M_DEVBUF); - free(ch, M_DEVBUF); - return (NULL); - } - snprintf(ch->name, sizeof(ch->name), "%s:%s:%s", - device_get_nameunit(ch->dev), dirs, devname); - - err = chn_init(ch, devinfo, dir, direction); - PCM_LOCK(d); - if (err) { - device_printf(d->dev, "chn_init(%s) failed: err = %d\n", - ch->name, err); - kobj_delete(ch->methods, M_DEVBUF); - free(ch, M_DEVBUF); - return (NULL); - } - - return (ch); -} - int pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch) { @@ -569,9 +459,9 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo) PCM_BUSYASSERT(d); PCM_LOCK(d); - ch = pcm_chn_create(d, NULL, cls, dir, -1, devinfo); + ch = chn_init(d, NULL, cls, dir, -1, devinfo); if (!ch) { - device_printf(d->dev, "pcm_chn_create(%s, %d, %p) failed\n", + device_printf(d->dev, "chn_init(%s, %d, %p) failed\n", cls->name, dir, devinfo); PCM_UNLOCK(d); return (ENODEV); diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h index 08aa56cc96f7..09e739ea5c20 100644 --- a/sys/dev/sound/pcm/sound.h +++ b/sys/dev/sound/pcm/sound.h @@ -297,7 +297,6 @@ int pcm_setvchans(struct snddev_info *d, int direction, int newcnt, int num); int pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction, pid_t pid, char *comm); -struct pcm_channel *pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, int dir, int num, void *devinfo); int pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch); int pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch); diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c index c3bc36d924bd..ccac08891220 100644 --- a/sys/dev/sound/pcm/vchan.c +++ b/sys/dev/sound/pcm/vchan.c @@ -698,7 +698,7 @@ vchan_create(struct pcm_channel *parent, int num) } /* create a new playback channel */ - ch = pcm_chn_create(d, parent, &vchan_class, direction, num, parent); + ch = chn_init(d, parent, &vchan_class, direction, num, parent); if (ch == NULL) { PCM_UNLOCK(d); CHN_LOCK(parent);
