From nobody Sun Jul 26 22:08:45 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4h7bdX4By7z6m0Cv; Sun, 26 Jul 2026 22:15:28 +0000 (UTC) (envelope-from christos@FreeBSD.org) Received: from margiolis.net (mail.margiolis.net [95.179.159.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA512) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4h7bdW4GxMz3mv0; Sun, 26 Jul 2026 22:15:27 +0000 (UTC) (envelope-from christos@FreeBSD.org) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=margiolis.net header.s=default header.b=G2SupCYA; spf=softfail (mx1.freebsd.org: 95.179.159.8 is neither permitted nor denied by domain of christos@FreeBSD.org) smtp.mailfrom=christos@FreeBSD.org; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=freebsd.org (policy=none) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=default; bh=xszIqra5Yers i5B6OqLSjr8qqNH+BAXSoEsuHWh+mYI=; h=in-reply-to:references:to:from: subject:cc:date; d=margiolis.net; b=G2SupCYAtr+zI5lycVLBfC6H+rls9YSAwf 1E4MqLzqmSUNq++VHTo6uCdJHVQYtGZk8jHZEb35NCilAW/2XqPLsK/EyAPA0/OmimWCuf W5Chy/8c/lO3qE3tJ5tPAHvBQyzOh7sYmj/tAxGuvOD7NvLkVrcno6pPxNyc8DYiMpU= Received: from localhost (109-178-193-69.pat.ren.cosmote.net [109.178.193.69]) by margiolis.net (OpenSMTPD) with ESMTPSA id 7d921be0 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 26 Jul 2026 16:08:47 -0600 (MDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 27 Jul 2026 01:08:45 +0300 Message-Id: Cc: , , "Kevin Bowling" , Subject: Re: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers by byte rate From: "Christos Margiolis" To: "Ronald Klop" X-Mailer: aerc 0.21.0 References: <6a636fb1.26412.26a07c1d@gitrepo.freebsd.org> <20302724.1251.1784904383819@localhost> In-Reply-To: <20302724.1251.1784904383819@localhost> X-Spamd-Result: default: False [2.30 / 15.00]; SPAM_FLAG(5.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MV_CASE(0.50)[]; R_DKIM_ALLOW(-0.20)[margiolis.net:s=default]; DMARC_POLICY_SOFTFAIL(0.10)[freebsd.org : No valid SPF, DKIM not aligned (relaxed),none]; MIME_GOOD(-0.10)[text/plain]; GREYLIST(0.00)[pass,body]; RCVD_TLS_ALL(0.00)[]; RECEIVED_HELO_LOCALHOST(0.00)[]; MIME_TRACE(0.00)[0:+]; DKIM_TRACE(0.00)[margiolis.net:+]; ARC_NA(0.00)[]; FROM_HAS_DN(0.00)[]; FREEFALL_USER(0.00)[christos]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; R_SPF_SOFTFAIL(0.00)[~all:c]; FROM_EQ_ENVFROM(0.00)[]; RCVD_COUNT_ONE(0.00)[1]; MLMMJ_DEST(0.00)[dev-commits-src-all@FreeBSD.org,dev-commits-src-main@FreeBSD.org]; RCVD_VIA_SMTP_AUTH(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; ASN(0.00)[asn:20473, ipnet:95.179.144.0/20, country:US]; RCPT_COUNT_FIVE(0.00)[5] X-Rspamd-Queue-Id: 4h7bdW4GxMz3mv0 X-Spamd-Bar: ++ 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. > > > =20 > Van: Christos Margiolis > Datum: vrijdag, 24 juli 2026 15:59 > Aan: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-com= mits-src-main@FreeBSD.org > CC: Kevin Bowling > Onderwerp: git: 967e86d1ef2a - main - sound: Scale PCM secondary buffers = by byte rate >>=20 >> The branch main has been updated by christos: >>=20 >> URL: https://cgit.FreeBSD.org/src/commit/?id=3D967e86d1ef2ac8711c0ae7be3= 53a9c08186f4e6f >>=20 >> commit 967e86d1ef2ac8711c0ae7be353a9c08186f4e6f >> Author: Kevin Bowling >> AuthorDate: 2026-07-24 13:57:27 +0000 >> Commit: Christos Margiolis >> CommitDate: 2026-07-24 13:58:14 +0000 >>=20 >> sound: Scale PCM secondary buffers by byte rate >> =20 >> The fixed 128 KiB secondary buffer cap dates from stereo-sized strea= ms. >> High channel-count or high sample-width OSS streams can consume most= of >> that budget in one graph quantum, leaving too little room for captur= e >> catch-up or playback headroom. >> =20 >> Keep 128 KiB as the low-rate floor, but derive the effective soft-ri= ng >> cap from the channel byte rate, clamped to 4 MiB. Use that per-chann= el >> cap when resizing the soft buffer and when clamping >> SNDCTL_DSP_SETFRAGMENT requests. >> =20 >> Also clamp SNDCTL_DSP_LOW_WATER to the current soft-buffer size so a= n >> impossible readiness threshold cannot make poll/select wait forever. >> =20 >> 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(-) >>=20 >> 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; >> } >> =20 >> +u_int32_t >> +chn_2ndbufmaxsize(struct pcm_channel *c) >> +{ >> + struct snd_dbuf *bs; >> + uint64_t maxsize; >> + >> + CHN_LOCKASSERT(c); >> + >> + bs =3D c->bufsoft; >> + maxsize =3D (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; >> =20 >> if (round < 1) >> round =3D 1; >> =20 >> - ret =3D min(round_pow2(v), CHN_2NDBUFMAXSIZE >> 1); >> + ret =3D min(round_pow2(v), maxsize >> 1); >> =20 >> if (ret > v && (ret >> 1) > 0 && (ret >> 1) >=3D ((v * 3) >> 2)) >> ret >>=3D 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 =3D=3D PCMDIR_PLAY || dir =3D=3D PCMDIR_REC)) { >> + if (max < CHN_2NDBUFSIZE_MIN) >> + max =3D CHN_2NDBUFSIZE_MIN; >> if (rblksz !=3D NULL) >> - *rblksz =3D CHN_2NDBUFMAXSIZE >> 1; >> + *rblksz =3D max >> 1; >> if (rblkcnt !=3D NULL) >> *rblkcnt =3D 2; >> printf("%s(): FAILED dir=3D%d latency=3D%d bps=3D%d " >> "datarate=3D%u max=3D%u\n", >> __func__, dir, latency, bps, datarate, max); >> - return CHN_2NDBUFMAXSIZE; >> + return max; >> } >> =20 >> lprofile =3D chn_latency_profile; >> @@ -1804,7 +1821,7 @@ chn_calclatency(int dir, int latency, int bps, u_i= nt32_t datarate, >> datarate)); >> if (bufsz > max) >> bufsz =3D max; >> - blksz =3D round_blksz(bufsz >> blkcnt, bps); >> + blksz =3D round_blksz(bufsz >> blkcnt, bps, max); >> =20 >> if (rblksz !=3D NULL) >> *rblksz =3D 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 =3D 0, nsblksz, nsblkcnt= ; >> int ret; >> + u_int32_t maxsize; >> =20 >> CHN_LOCKASSERT(c); >> =20 >> @@ -1843,14 +1861,15 @@ chn_resizebuf(struct pcm_channel *c, int latency= , >> =20 >> bs =3D c->bufsoft; >> b =3D c->bufhard; >> + maxsize =3D chn_2ndbufmaxsize(c); >> =20 >> if (!(blksz =3D=3D 0 || blkcnt =3D=3D -1) && >> (blksz < 16 || blksz < bs->align || blkcnt < 2 || >> - (blksz * blkcnt) > CHN_2NDBUFMAXSIZE)) >> + (uint64_t)blksz * blkcnt > maxsize)) >> return EINVAL; >> =20 >> chn_calclatency(c->direction, latency, bs->align, >> - bs->align * bs->spd, CHN_2NDBUFMAXSIZE, >> + bs->align * bs->spd, maxsize, >> &sblksz, &sblkcnt); >> =20 >> if (blksz =3D=3D 0 || blkcnt =3D=3D -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 =3D round_blksz(blksz, bs->align); >> + sblksz =3D round_blksz(blksz, bs->align, maxsize); >> sblkcnt =3D round_pow2(blkcnt); >> } >> =20 >> @@ -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 =3D 2; >> if (c->flags & CHN_F_HAS_SIZE) { >> hblksz =3D round_blksz(sndbuf_xbytes(sblksz, bs, b), >> - b->align); >> + b->align, CHN_2NDBUFSIZE_MIN); >> hblkcnt =3D 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); >> =20 >> if ((hblksz << 1) > b->maxsize) >> - hblksz =3D round_blksz(b->maxsize >> 1, b->align); >> + hblksz =3D round_blksz(b->maxsize >> 1, b->align, >> + CHN_2NDBUFSIZE_MIN); >> =20 >> while ((hblksz * hblkcnt) > b->maxsize) { >> if (hblkcnt < 4) >> @@ -1922,7 +1951,8 @@ chn_resizebuf(struct pcm_channel *c, int latency, >> =20 >> if (!CHN_EMPTY(c, children)) { >> nsblksz =3D round_blksz( >> - sndbuf_xbytes(b->blksz, b, bs), bs->align); >> + sndbuf_xbytes(b->blksz, b, bs), bs->align, >> + maxsize); >> nsblkcnt =3D b->blkcnt; >> if (c->direction =3D=3D PCMDIR_PLAY) { >> do { >> @@ -1938,13 +1968,13 @@ chn_resizebuf(struct pcm_channel *c, int latency= , >> limit =3D sndbuf_xbytes(b->blksz, b, bs) * 2; >> } >> =20 >> - if (limit > CHN_2NDBUFMAXSIZE) >> - limit =3D CHN_2NDBUFMAXSIZE; >> + if ((u_int32_t)limit > maxsize) >> + limit =3D maxsize; >> =20 >> - while ((sblksz * sblkcnt) < limit) >> + while ((uint64_t)sblksz * sblkcnt < (uint64_t)limit) >> sblkcnt <<=3D 1; >> =20 >> - while ((sblksz * sblkcnt) > CHN_2NDBUFMAXSIZE) { >> + while ((uint64_t)sblksz * sblkcnt > maxsize) { >> if (sblkcnt < 4) >> sblksz >>=3D 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 >> =20 >> -/* >> - * 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 =3D 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, targetin= g >> + * 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 memo= ry >> + * use for low byte-rate streams; it holds ~680 ms at the once-typical >> + * 48kHz * 16bit * stereo rate (192000 bytes/sec), well above the targe= t >> + * (~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 rat= es >> + * 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 *); >> =20 >> #define CHANNEL_DECLARE(name) static DEFINE_CLASS(name, name ## _method= s, 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 >> =20 >> +static uint32_t >> +dsp_clamp_fragments(uint32_t maxfrags, uint32_t fragsz, uint32_t maxsiz= e) >> +{ >> + if (maxfrags =3D=3D 0) >> + maxfrags =3D maxsize / fragsz; >> + if (maxfrags < 2) >> + maxfrags =3D 2; >> + if ((uint64_t)maxfrags * fragsz > maxsize) >> + maxfrags =3D 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 =3D 1 << fragln; >> =20 >> - if (maxfrags =3D=3D 0) >> - maxfrags =3D CHN_2NDBUFMAXSIZE / fragsz; >> - if (maxfrags < 2) >> - maxfrags =3D 2; >> - if (maxfrags * fragsz > CHN_2NDBUFMAXSIZE) >> - maxfrags =3D CHN_2NDBUFMAXSIZE / fragsz; >> - >> DEB(printf("SNDCTL_DSP_SETFRAGMENT %d frags, %d sz\n", maxfr= ags, fragsz)); >> PCM_ACQUIRE_QUICK(d); >> if (rdch) { >> CHN_LOCK(rdch); >> - ret =3D chn_setblocksize(rdch, maxfrags, fragsz); >> + ret =3D chn_setblocksize(rdch, >> + dsp_clamp_fragments(maxfrags, fragsz, >> + chn_2ndbufmaxsize(rdch)), fragsz); >> r_maxfrags =3D rdch->bufsoft->blkcnt; >> r_fragsz =3D 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 =3D=3D 0) { >> CHN_LOCK(wrch); >> - ret =3D chn_setblocksize(wrch, maxfrags, fragsz); >> + ret =3D chn_setblocksize(wrch, >> + dsp_clamp_fragments(maxfrags, fragsz, >> + chn_2ndbufmaxsize(wrch)), fragsz); >> maxfrags =3D wrch->bufsoft->blkcnt; >> fragsz =3D 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 !=3D NULL) { >> CHN_LOCK(wrch); >> - wrch->lw =3D (*arg_i > 1) ? *arg_i : 1; >> + wrch->lw =3D dsp_low_water(wrch, *arg_i); >> CHN_UNLOCK(wrch); >> } >> if (rdch !=3D NULL) { >> CHN_LOCK(rdch); >> - rdch->lw =3D (*arg_i > 1) ? *arg_i : 1; >> + rdch->lw =3D dsp_low_water(rdch, *arg_i); >> CHN_UNLOCK(rdch); >> } >> break; >> =20 >>=20 >>=20 >>=20 > > =20