Date: Fri, 22 Apr 2005 15:46:54 -0700 From: Luigi Rizzo <rizzo@icir.org> To: multimedia@freebsd.org Subject: dev/sound patches to reduce latency Message-ID: <20050422154654.A72976@xorpc.icir.org>
next in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050422154654.A72976>