Re: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers by byte rate
- In reply to: Ronald Klop : "Re: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers by byte rate"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 26 Jul 2026 22:08:45 UTC
On Fri Jul 24, 2026 at 5:46 PM EEST, Ronald Klop wrote:
> Is this a good candidate for
> Relnotes: yes
> in the commit message?
I don't think so to be honest.
> Regards,
> Ronald.
>
>
>
> Van: Christos Margiolis <christos@FreeBSD.org>
> Datum: vrijdag, 24 juli 2026 15:59
> Aan: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
> CC: Kevin Bowling <kbowling@FreeBSD.org>
> Onderwerp: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers by byte rate
>>
>> 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;
>>
>>
>>
>>
>
>