Date: Fri, 24 Jul 2026 13:59:13 +0000 From: Christos Margiolis <christos@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Kevin Bowling <kbowling@FreeBSD.org> Subject: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers by byte rate Message-ID: <6a636fb1.26412.26a07c1d@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=967e86d1ef2ac8711c0ae7be353a9c08186f4e6f commit 967e86d1ef2ac8711c0ae7be353a9c08186f4e6f Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-07-24 13:57:27 +0000 Commit: Christos Margiolis <christos@FreeBSD.org> CommitDate: 2026-07-24 13:58:14 +0000 sound: Scale PCM secondary buffers by byte rate The fixed 128 KiB secondary buffer cap dates from stereo-sized streams. High channel-count or high sample-width OSS streams can consume most of that budget in one graph quantum, leaving too little room for capture catch-up or playback headroom. Keep 128 KiB as the low-rate floor, but derive the effective soft-ring cap from the channel byte rate, clamped to 4 MiB. Use that per-channel cap when resizing the soft buffer and when clamping SNDCTL_DSP_SETFRAGMENT requests. Also clamp SNDCTL_DSP_LOW_WATER to the current soft-buffer size so an impossible readiness threshold cannot make poll/select wait forever. MFC after: 3 weeks Reviewed by: christos Differential Revision: https://reviews.freebsd.org/D58064 --- sys/dev/sound/pcm/channel.c | 62 +++++++++++++++++++++++++++++++++------------ sys/dev/sound/pcm/channel.h | 30 +++++++++++++++++----- sys/dev/sound/pcm/dsp.c | 38 +++++++++++++++++++-------- 3 files changed, 96 insertions(+), 34 deletions(-) diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c index ff7620c772c3..21c223c61205 100644 --- a/sys/dev/sound/pcm/channel.c +++ b/sys/dev/sound/pcm/channel.c @@ -1657,15 +1657,30 @@ round_pow2(u_int32_t v) return ret; } +u_int32_t +chn_2ndbufmaxsize(struct pcm_channel *c) +{ + struct snd_dbuf *bs; + uint64_t maxsize; + + CHN_LOCKASSERT(c); + + bs = c->bufsoft; + maxsize = (uint64_t)bs->align * bs->spd * CHN_2NDBUFTIME_MS / 1000; + RANGE(maxsize, CHN_2NDBUFSIZE_MIN, CHN_2NDBUFSIZE_MAX); + + return ((u_int32_t)maxsize); +} + static u_int32_t -round_blksz(u_int32_t v, int round) +round_blksz(u_int32_t v, int round, u_int32_t maxsize) { u_int32_t ret, tmp; if (round < 1) round = 1; - ret = min(round_pow2(v), CHN_2NDBUFMAXSIZE >> 1); + ret = min(round_pow2(v), maxsize >> 1); if (ret > v && (ret >> 1) > 0 && (ret >> 1) >= ((v * 3) >> 2)) ret >>= 1; @@ -1780,14 +1795,16 @@ chn_calclatency(int dir, int latency, int bps, u_int32_t datarate, if (latency < CHN_LATENCY_MIN || latency > CHN_LATENCY_MAX || bps < 1 || datarate < 1 || !(dir == PCMDIR_PLAY || dir == PCMDIR_REC)) { + if (max < CHN_2NDBUFSIZE_MIN) + max = CHN_2NDBUFSIZE_MIN; if (rblksz != NULL) - *rblksz = CHN_2NDBUFMAXSIZE >> 1; + *rblksz = max >> 1; if (rblkcnt != NULL) *rblkcnt = 2; printf("%s(): FAILED dir=%d latency=%d bps=%d " "datarate=%u max=%u\n", __func__, dir, latency, bps, datarate, max); - return CHN_2NDBUFMAXSIZE; + return max; } lprofile = chn_latency_profile; @@ -1804,7 +1821,7 @@ chn_calclatency(int dir, int latency, int bps, u_int32_t datarate, datarate)); if (bufsz > max) bufsz = max; - blksz = round_blksz(bufsz >> blkcnt, bps); + blksz = round_blksz(bufsz >> blkcnt, bps, max); if (rblksz != NULL) *rblksz = blksz; @@ -1821,6 +1838,7 @@ chn_resizebuf(struct pcm_channel *c, int latency, struct snd_dbuf *b, *bs, *pb; int sblksz, sblkcnt, hblksz, hblkcnt, limit = 0, nsblksz, nsblkcnt; int ret; + u_int32_t maxsize; CHN_LOCKASSERT(c); @@ -1843,14 +1861,15 @@ chn_resizebuf(struct pcm_channel *c, int latency, bs = c->bufsoft; b = c->bufhard; + maxsize = chn_2ndbufmaxsize(c); if (!(blksz == 0 || blkcnt == -1) && (blksz < 16 || blksz < bs->align || blkcnt < 2 || - (blksz * blkcnt) > CHN_2NDBUFMAXSIZE)) + (uint64_t)blksz * blkcnt > maxsize)) return EINVAL; chn_calclatency(c->direction, latency, bs->align, - bs->align * bs->spd, CHN_2NDBUFMAXSIZE, + bs->align * bs->spd, maxsize, &sblksz, &sblkcnt); if (blksz == 0 || blkcnt == -1) { @@ -1871,7 +1890,7 @@ chn_resizebuf(struct pcm_channel *c, int latency, * defeat the purpose of having custom control. The least * we can do is round it to the nearest ^2 and align it. */ - sblksz = round_blksz(blksz, bs->align); + sblksz = round_blksz(blksz, bs->align, maxsize); sblkcnt = round_pow2(blkcnt); } @@ -1890,18 +1909,28 @@ chn_resizebuf(struct pcm_channel *c, int latency, sndbuf_xbytes(pb->blksz, pb, bs) * 2 : 0; } } else { + /* + * The byte-rate-scaled cap applies to the secondary buffer + * only. It exists to absorb userland scheduling latency, + * which the secondary buffer alone must cover; hardware + * buffer geometry keeps the historical cap, since enlarging + * it would change fragment sizes and interrupt cadence + * visible to drivers, and remains bounded by b->maxsize + * below. + */ hblkcnt = 2; if (c->flags & CHN_F_HAS_SIZE) { hblksz = round_blksz(sndbuf_xbytes(sblksz, bs, b), - b->align); + b->align, CHN_2NDBUFSIZE_MIN); hblkcnt = round_pow2(bs->blkcnt); } else chn_calclatency(c->direction, latency, b->align, b->align * b->spd, - CHN_2NDBUFMAXSIZE, &hblksz, &hblkcnt); + CHN_2NDBUFSIZE_MIN, &hblksz, &hblkcnt); if ((hblksz << 1) > b->maxsize) - hblksz = round_blksz(b->maxsize >> 1, b->align); + hblksz = round_blksz(b->maxsize >> 1, b->align, + CHN_2NDBUFSIZE_MIN); while ((hblksz * hblkcnt) > b->maxsize) { if (hblkcnt < 4) @@ -1922,7 +1951,8 @@ chn_resizebuf(struct pcm_channel *c, int latency, if (!CHN_EMPTY(c, children)) { nsblksz = round_blksz( - sndbuf_xbytes(b->blksz, b, bs), bs->align); + sndbuf_xbytes(b->blksz, b, bs), bs->align, + maxsize); nsblkcnt = b->blkcnt; if (c->direction == PCMDIR_PLAY) { do { @@ -1938,13 +1968,13 @@ chn_resizebuf(struct pcm_channel *c, int latency, limit = sndbuf_xbytes(b->blksz, b, bs) * 2; } - if (limit > CHN_2NDBUFMAXSIZE) - limit = CHN_2NDBUFMAXSIZE; + if ((u_int32_t)limit > maxsize) + limit = maxsize; - while ((sblksz * sblkcnt) < limit) + while ((uint64_t)sblksz * sblkcnt < (uint64_t)limit) sblkcnt <<= 1; - while ((sblksz * sblkcnt) > CHN_2NDBUFMAXSIZE) { + while ((uint64_t)sblksz * sblkcnt > maxsize) { if (sblkcnt < 4) sblksz >>= 1; else diff --git a/sys/dev/sound/pcm/channel.h b/sys/dev/sound/pcm/channel.h index c7f5bf93b8e5..c78c25dac13b 100644 --- a/sys/dev/sound/pcm/channel.h +++ b/sys/dev/sound/pcm/channel.h @@ -430,15 +430,31 @@ enum { #define CHN_TIMEOUT_MIN 1 #define CHN_TIMEOUT_MAX 10 -/* - * This should be large enough to hold all pcm data between - * tsleeps in chn_{read,write} at the highest sample rate. - * (which is usually 48kHz * 16bit * stereo = 192000 bytes/sec) - */ +/* Default block size for the secondary buffer. */ #define CHN_2NDBUFBLKSIZE (2 * 1024) /* The total number of blocks per secondary bufhard. */ #define CHN_2NDBUFBLKNUM (32) -/* The size of a whole secondary bufhard. */ -#define CHN_2NDBUFMAXSIZE (131072) +/* + * The secondary buffer cap scales with the channel byte rate, targeting + * CHN_2NDBUFTIME_MS of stream so that all pcm data between tsleeps in + * chn_{read,write} fits, clamped to [CHN_2NDBUFSIZE_MIN, + * CHN_2NDBUFSIZE_MAX]. + * + * The floor is the historical secondary buffer size and preserves memory + * use for low byte-rate streams; it holds ~680 ms at the once-typical + * 48kHz * 16bit * stereo rate (192000 bytes/sec), well above the target + * (~38 KiB there). + * + * The ceiling bounds per-channel buffer memory. Buffers are allocated at + * the derived size, so only streams that actually run at high byte rates + * approach it. It holds the full target through MADI-class streams + * (64ch * 32-bit * 48kHz, ~12.3 MB/s); beyond that, coverage shrinks + * proportionally (e.g. ~85 ms at 64ch * 32-bit * 192kHz). + */ +#define CHN_2NDBUFSIZE_MIN (131072) +#define CHN_2NDBUFSIZE_MAX (4 * 1024 * 1024) +#define CHN_2NDBUFTIME_MS 200 + +u_int32_t chn_2ndbufmaxsize(struct pcm_channel *); #define CHANNEL_DECLARE(name) static DEFINE_CLASS(name, name ## _methods, sizeof(struct kobj)) diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c index 0a5063410d24..f49df6e8a8c6 100644 --- a/sys/dev/sound/pcm/dsp.c +++ b/sys/dev/sound/pcm/dsp.c @@ -109,6 +109,25 @@ static int dsp_oss_setsong(struct pcm_channel *wrch, struct pcm_channel *rdch, o static int dsp_oss_setname(struct pcm_channel *wrch, struct pcm_channel *rdch, oss_longname_t *name); #endif +static uint32_t +dsp_clamp_fragments(uint32_t maxfrags, uint32_t fragsz, uint32_t maxsize) +{ + if (maxfrags == 0) + maxfrags = maxsize / fragsz; + if (maxfrags < 2) + maxfrags = 2; + if ((uint64_t)maxfrags * fragsz > maxsize) + maxfrags = maxsize / fragsz; + return (maxfrags); +} + +static unsigned int +dsp_low_water(struct pcm_channel *ch, int lw) +{ + RANGE(lw, 1, ch->bufsoft->bufsize); + return ((unsigned int)lw); +} + int dsp_make_dev(device_t dev) { @@ -1244,18 +1263,13 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode, RANGE(fragln, 4, 16); fragsz = 1 << fragln; - if (maxfrags == 0) - maxfrags = CHN_2NDBUFMAXSIZE / fragsz; - if (maxfrags < 2) - maxfrags = 2; - if (maxfrags * fragsz > CHN_2NDBUFMAXSIZE) - maxfrags = CHN_2NDBUFMAXSIZE / fragsz; - DEB(printf("SNDCTL_DSP_SETFRAGMENT %d frags, %d sz\n", maxfrags, fragsz)); PCM_ACQUIRE_QUICK(d); if (rdch) { CHN_LOCK(rdch); - ret = chn_setblocksize(rdch, maxfrags, fragsz); + ret = chn_setblocksize(rdch, + dsp_clamp_fragments(maxfrags, fragsz, + chn_2ndbufmaxsize(rdch)), fragsz); r_maxfrags = rdch->bufsoft->blkcnt; r_fragsz = rdch->bufsoft->blksz; CHN_UNLOCK(rdch); @@ -1265,7 +1279,9 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode, } if (wrch && ret == 0) { CHN_LOCK(wrch); - ret = chn_setblocksize(wrch, maxfrags, fragsz); + ret = chn_setblocksize(wrch, + dsp_clamp_fragments(maxfrags, fragsz, + chn_2ndbufmaxsize(wrch)), fragsz); maxfrags = wrch->bufsoft->blkcnt; fragsz = wrch->bufsoft->blksz; CHN_UNLOCK(wrch); @@ -1653,12 +1669,12 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode, */ if (wrch != NULL) { CHN_LOCK(wrch); - wrch->lw = (*arg_i > 1) ? *arg_i : 1; + wrch->lw = dsp_low_water(wrch, *arg_i); CHN_UNLOCK(wrch); } if (rdch != NULL) { CHN_LOCK(rdch); - rdch->lw = (*arg_i > 1) ? *arg_i : 1; + rdch->lw = dsp_low_water(rdch, *arg_i); CHN_UNLOCK(rdch); } break;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a636fb1.26412.26a07c1d>