From owner-freebsd-multimedia@FreeBSD.ORG Fri Apr 22 22:46:56 2005 Return-Path: Delivered-To: freebsd-multimedia@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 019C316A4CE for ; Fri, 22 Apr 2005 22:46:56 +0000 (GMT) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.FreeBSD.org (Postfix) with ESMTP id 90C7D43D60 for ; Fri, 22 Apr 2005 22:46:55 +0000 (GMT) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.11/8.12.8) with ESMTP id j3MMksuV073111; Fri, 22 Apr 2005 15:46:54 -0700 (PDT) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.11/8.12.3/Submit) id j3MMks9K073110; Fri, 22 Apr 2005 15:46:54 -0700 (PDT) (envelope-from rizzo) Date: Fri, 22 Apr 2005 15:46:54 -0700 From: Luigi Rizzo To: multimedia@freebsd.org Message-ID: <20050422154654.A72976@xorpc.icir.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="n8g4imXOkfNTN/H1" Content-Disposition: inline User-Agent: Mutt/1.2.5.1i Subject: dev/sound patches to reduce latency X-BeenThere: freebsd-multimedia@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Multimedia discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Apr 2005 22:46:56 -0000 --n8g4imXOkfNTN/H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline attached are some patches for the sound driver to reduce latency in the playback channel. Background - by design, the driver tries to keep the "hard" buffer (the one used by the hardware to play samples out) always full even if userland supplied a small amount of data. While there is an ioctl() to set the size of the buffer in terms of a blocksize and number of blocks, the existing code failed in some cases to push down the info to the hardware, and had the tendency to use max-sized buffers in many cases. At 8khz, the 16k default buffers could cause very large delays in the playback of audio. With this patches, we try to pass the user-specified blocksize down to the hardware, and furthermore, use a small number of blocks in the "hard" buffer to minimize latency. I'd appreciate if people could test this code and report any good or bad news. I think it significantly improves what we have now. It could still benefit from an additional improvement: - make the choice of the number of buffers adaptive on the irq rate. which is not a very complex thing to do, but should be done in all individual drivers, so it takes a bit more testing. ---- detailed description --- This patch touches the following files: pci/ich.c comments and fixes the block allocation algorithm pcm/channel.c comment in detail the buffer sizing algorithm, and implement it. The resulting code is, i believe, a lot simpler and more readable than the previous one. pcm/dsp.c mostly comments to the existing code. Also partly related: pcm/ac97.c enables the "igain" mixer to drive the 20dB boost for the mic. ----------------- cheers luigi --n8g4imXOkfNTN/H1 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="pcm.diff" Index: pci/ich.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pci/ich.c,v retrieving revision 1.3.2.13 diff -u -r1.3.2.13 ich.c --- pci/ich.c 18 Aug 2003 15:41:01 -0000 1.3.2.13 +++ pci/ich.c 22 Apr 2005 22:26:13 -0000 @@ -183,6 +183,12 @@ /* -------------------------------------------------------------------- */ /* common routines */ +/* + * The hardware supports up to 32 buffers. We set blkcnt to a power of 2 + * between 2 and 32 (but as small as possible, now either 2 or 4). + * Buffers are powers of 2, the table is completely filled by repeating + * the patterns. + */ static void ich_filldtbl(struct sc_chinfo *ch) { @@ -190,11 +196,12 @@ int i; base = vtophys(sndbuf_getbuf(ch->buffer)); - ch->blkcnt = sndbuf_getsize(ch->buffer) / ch->blksz; - if (ch->blkcnt != 2 && ch->blkcnt != 4 && ch->blkcnt != 8 && ch->blkcnt != 16 && ch->blkcnt != 32) { - ch->blkcnt = 2; - ch->blksz = sndbuf_getsize(ch->buffer) / ch->blkcnt; - } + i = sndbuf_getmaxsize(ch->buffer) / ch->blksz; + /* use either 2 or 4 buffers. */ + if (i > 4) /* XXX at large irq rates, we should use more */ + i = 4; + ch->blkcnt = i; + sndbuf_resize(ch->buffer, ch->blkcnt, ch->blksz); for (i = 0; i < ICH_DTBL_LENGTH; i++) { ch->dtbl[i].buffer = base + (ch->blksz * (i % ch->blkcnt)); Index: pcm/ac97.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/ac97.c,v retrieving revision 1.5.2.16 diff -u -r1.5.2.16 ac97.c --- pcm/ac97.c 20 Jan 2004 17:49:24 -0000 1.5.2.16 +++ pcm/ac97.c 14 Jun 2004 15:46:10 -0000 @@ -84,7 +84,7 @@ [SOUND_MIXER_LINE] = { AC97_MIX_LINE, 5, 0, 1, 1, 5, 0, 1 }, [SOUND_MIXER_PHONEIN] = { AC97_MIX_PHONE, 5, 0, 0, 1, 8, 0, 0 }, [SOUND_MIXER_MIC] = { AC97_MIX_MIC, 5, 0, 0, 1, 1, 1, 1 }, -#if 0 +#if 1 /* use igain for the mic 20dB boost */ [SOUND_MIXER_IGAIN] = { -AC97_MIX_MIC, 1, 6, 0, 0, 0, 1, 1 }, #endif Index: pcm/channel.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/channel.c,v retrieving revision 1.19.2.19 diff -u -r1.19.2.19 channel.c --- pcm/channel.c 11 Mar 2003 15:15:41 -0000 1.19.2.19 +++ pcm/channel.c 22 Apr 2005 22:27:02 -0000 @@ -903,12 +903,42 @@ return r; } +/* + * given a bufsz value, round it to a power of 2 in the min-max range + * XXX only works if min and max are powers of 2 + */ +static int +round_bufsz(int bufsz, int min, int max) +{ + int tmp = min*2; + while (tmp <= bufsz) + tmp <<= 1; + tmp >>= 1; + if (tmp > max) + tmp = max; + return tmp; +} + +/* + * blksz should be a power of 2 between 4 and 16. + * It is useful that it has the same size for both bufsoft and bufhard. + * blkcnt is set by the user, between 2 and 2^17/blksz for bufsoft, + * but should be a power of 2 for bufhard to simplify life to low + * level drivers. Note, for the rec channel a large blkcnt is ok, + * but for the play channel we want blksz as small as possible to keep + * the delay small, because routines in the write path always try to + * keep bufhard full. + * + * Unless we have good reason to, use the values suggested by the caller. + */ + int chn_setblocksize(struct pcm_channel *c, int blkcnt, int blksz) { struct snd_dbuf *b = c->bufhard; struct snd_dbuf *bs = c->bufsoft; - int bufsz, irqhz, tmp, ret; + int irqhz, ret; + int rc = blkcnt, rs = blksz; CHN_LOCKASSERT(c); if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED)) @@ -917,26 +947,29 @@ ret = 0; DEB(printf("%s(%d, %d)\n", __func__, blkcnt, blksz)); if (blksz == 0 || blksz == -1) { - if (blksz == -1) + if (blksz == -1) /* delete previous values */ c->flags &= ~CHN_F_HAS_SIZE; - if (!(c->flags & CHN_F_HAS_SIZE)) { - blksz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / chn_targetirqrate; - tmp = 32; - while (tmp <= blksz) - tmp <<= 1; - tmp >>= 1; - blksz = tmp; + if (!(c->flags & CHN_F_HAS_SIZE)) { /* recompute */ + /* + * compute a base blksz according to the target irq + * rate, then round to the larger power of 2 which + * is smaller than the computed value and in the + * range 16.. 2^17/2. Then compute a suitable blkcnt. + */ + blksz = round_bufsz( (sndbuf_getbps(bs) * + sndbuf_getspd(bs)) / chn_targetirqrate, + 16, CHN_2NDBUFMAXSIZE / 2); blkcnt = CHN_2NDBUFMAXSIZE / blksz; - - RANGE(blksz, 16, CHN_2NDBUFMAXSIZE / 2); - RANGE(blkcnt, 2, CHN_2NDBUFMAXSIZE / blksz); - DEB(printf("%s: defaulting to (%d, %d)\n", __func__, blkcnt, blksz)); - } else { + } else { /* use previously defined value */ blkcnt = sndbuf_getblkcnt(bs); blksz = sndbuf_getblksz(bs); - DEB(printf("%s: updating (%d, %d)\n", __func__, blkcnt, blksz)); } } else { + /* + * use supplied values if reasonable. Note that here we + * might have blksz which is not a power of 2 if the + * ioctl() to compute it allows such values. + */ ret = EINVAL; if ((blksz < 16) || (blkcnt < 2) || (blkcnt * blksz > CHN_2NDBUFMAXSIZE)) goto out; @@ -944,35 +977,30 @@ c->flags |= CHN_F_HAS_SIZE; } - bufsz = blkcnt * blksz; - ret = ENOMEM; if (sndbuf_remalloc(bs, blkcnt, blksz)) goto out; ret = 0; - /* adjust for different hw format/speed */ + /* + * Now compute the approx irq rate for the given (soft) blksz, + * reduce to the acceptable range and compute a corresponding blksz + * for the hard buffer. Then set the channel's blocksize and + * corresponding hardbuf value. The number of blocks used should + * be set by the device-specific routine. In fact, even the + * call to sndbuf_setblksz() should not be here! XXX + */ irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / sndbuf_getblksz(bs); - DEB(printf("%s: soft bps %d, spd %d, irqhz == %d\n", __func__, sndbuf_getbps(bs), sndbuf_getspd(bs), irqhz)); RANGE(irqhz, 16, 512); - sndbuf_setblksz(b, (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz); - - /* round down to 2^x */ - blksz = 32; - while (blksz <= sndbuf_getblksz(b)) - blksz <<= 1; - blksz >>= 1; - - /* round down to fit hw buffer size */ - RANGE(blksz, 16, sndbuf_getmaxsize(b) / 2); - DEB(printf("%s: hard blksz requested %d (maxsize %d), ", __func__, blksz, sndbuf_getmaxsize(b))); - + blksz = round_bufsz( (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz, + 16, sndbuf_getmaxsize(b) / 2); sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz)); - irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b); - DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz)); - + printf("%s: chan %p req %d/%db bs %d/%db b %d/%db\n", + __func__, c, rc, rs, + sndbuf_getblkcnt(bs), sndbuf_getblksz(bs), + sndbuf_getblkcnt(b), sndbuf_getblksz(b)); chn_resetbuf(c); out: return ret; Index: pcm/dsp.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/dsp.c,v retrieving revision 1.15.2.14 diff -u -r1.15.2.14 dsp.c --- pcm/dsp.c 20 Jul 2004 14:29:34 -0000 1.15.2.14 +++ pcm/dsp.c 22 Apr 2005 21:20:47 -0000 @@ -769,6 +769,7 @@ u_int32_t fragln = (*arg_i) & 0x0000ffff; u_int32_t maxfrags = ((*arg_i) & 0xffff0000) >> 16; u_int32_t fragsz; + u_int32_t r_maxfrags, r_fragsz; RANGE(fragln, 4, 16); fragsz = 1 << fragln; @@ -786,9 +787,12 @@ if (rdch) { CHN_LOCK(rdch); ret = chn_setblocksize(rdch, maxfrags, fragsz); - maxfrags = sndbuf_getblkcnt(rdch->bufsoft); - fragsz = sndbuf_getblksz(rdch->bufsoft); + r_maxfrags = sndbuf_getblkcnt(rdch->bufsoft); + r_fragsz = sndbuf_getblksz(rdch->bufsoft); CHN_UNLOCK(rdch); + } else { + r_maxfrags = maxfrags; + r_fragsz = fragsz; } if (wrch && ret == 0) { CHN_LOCK(wrch); @@ -796,6 +800,9 @@ maxfrags = sndbuf_getblkcnt(wrch->bufsoft); fragsz = sndbuf_getblksz(wrch->bufsoft); CHN_UNLOCK(wrch); + } else { /* use whatever came from the read channel */ + maxfrags = r_maxfrags; + fragsz = r_fragsz; } fragln = 0; --n8g4imXOkfNTN/H1--