From 6e0acd7cd4f836465164d2172434e6a6c7e279a2 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 8 Aug 2014 11:02:45 -0600 Subject: [PATCH] WM8904: Don't use MSEC2TICK in timeout calculation --- drivers/audio/wm8904.c | 36 ++++++++++++++++++++---------------- include/nuttx/clock.h | 22 +++++++++++----------- libc/audio/lib_buffer.c | 2 +- 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/drivers/audio/wm8904.c b/drivers/audio/wm8904.c index 707f5fd289..4103c447b5 100644 --- a/drivers/audio/wm8904.c +++ b/drivers/audio/wm8904.c @@ -1426,6 +1426,7 @@ static int wm8904_sendbuffer(FAR struct wm8904_dev_s *priv) FAR struct ap_buffer_s *apb; irqstate_t flags; uint32_t timeout; + int shift; int ret = OK; /* Loop while there are audio buffers to be sent and we have few than @@ -1461,27 +1462,30 @@ static int wm8904_sendbuffer(FAR struct wm8904_dev_s *priv) /* Send the entire audio buffer via I2S. What is a reasonable timeout * to use? This would depend on the bit rate and size of the buffer. * - * Samples in the buffer: - * = buffer_size * 8 / bpsamp samples - * Expected transfer time: - * = samples / samprate seconds - * = (samples * 1000) / (samprate * msec_per_tick) ticks - * = (buffer_size * 8000) /(samprate * bpsamp * msec_per_tick) + * Samples in the buffer (samples): + * = buffer_size * 8 / bpsamp samples + * Sample rate (samples/second): + * = samplerate * nchannels + * Expected transfer time (seconds): + * = (buffer_size * 8) / bpsamp / samplerate / nchannels * - * We will set the timeout about twice that. Here is a reasonable - * approximation that saves a multiply: - * = (buffer_size * 16384) /(samprate * bpsamp * msec_per_tick) + * We will set the timeout about twice that. * - * REVISIT: Does this take into account the number channels? Perhaps - * saving an reusing the bitrate would give a better and simpler - * calculation. + * NOTES: + * - The multiplier of 8 becomes 16000 for 2x and units of + * milliseconds. + * - 16000 is a approximately 16384 (1 << 14), bpsamp is either + * (1 << 3) or (1 << 4), and nchannels is either (1 << 0) or + * (1 << 1). So this can be simplifies to (milliseconds): * - * REVISIT: Should not use MSEC_PER_TICK. It can be inaccurate with - * microsecond resolution timer. + * = (buffer_size << shift) / samplerate */ - timeout = (((uint32_t)(apb->nbytes - apb->curbyte) << 14) / - ((uint32_t)priv->samprate * MSEC_PER_TICK * priv->bpsamp)); + shift = (priv->bpsamp == 8) ? 14 - 3 : 14 - 4; + shift -= (priv->nchannels > 1) ? 1 : 0; + + timeout = MSEC2TICK(((uint32_t)(apb->nbytes - apb->curbyte) << shift) / + (uint32_t)priv->samprate); ret = I2S_SEND(priv->i2s, apb, wm8904_senddone, priv, timeout); if (ret < 0) diff --git a/include/nuttx/clock.h b/include/nuttx/clock.h index b0f4c15056..bc7fd96e4e 100644 --- a/include/nuttx/clock.h +++ b/include/nuttx/clock.h @@ -55,7 +55,7 @@ * The code in this execution context can access the kernel global data * directly if: (1) we are not running tick-less (in which case there is * no global timer data), (2) this is an un-protected, non-kernel build, or - * (2) this is a protectd build, but this code is being built for execution + * (2) this is a protected build, but this code is being built for execution * within the kernel space. */ @@ -89,16 +89,16 @@ #define NSEC_PER_USEC 1000 /* If CONFIG_SCHED_TICKLESS is not defined, then the interrupt interval of - * the system timer is given by MSEC_PER_TICK. This is the expected number - * of milliseconds between calls from the processor-specific logic to - * sched_process_timer(). The default value of MSEC_PER_TICK is 10 - * milliseconds (100KHz). However, this default setting can be overridden + * the system timer is given by USEC_PER_TICK. This is the expected number + * of microseconds between calls from the processor-specific logic to + * sched_process_timer(). The default value of USEC_PER_TICK is 10000 + * microseconds (100KHz). However, this default setting can be overridden * by defining the interval in microseconds as CONFIG_USEC_PER_TICK in the * NuttX configuration file. * * The following calculations are only accurate when (1) there is no * truncation involved and (2) the underlying system timer is an even - * multiple of milliseconds. If (2) is not true, you will probably want + * multiple of microseconds. If (2) is not true, you will probably want * to redefine all of the following. */ @@ -114,7 +114,7 @@ */ #define TICK_PER_DSEC (USEC_PER_DSEC / USEC_PER_TICK) /* Truncates! */ -#define TICK_PER_SEC (USEC_PER_SEC / USEC_PER_TICK) /* Truncates! */ +#define TICK_PER_SEC (USEC_PER_SEC / USEC_PER_TICK) /* Truncates! */ #define TICK_PER_MSEC (USEC_PER_MSEC / USEC_PER_TICK) /* Truncates! */ #define MSEC_PER_TICK (USEC_PER_TICK / USEC_PER_MSEC) /* Truncates! */ #define NSEC_PER_TICK (USEC_PER_TICK * NSEC_PER_USEC) /* Exact */ @@ -128,11 +128,11 @@ # define MSEC2TICK(msec) USEC2TICK(msec * 1000) /* Rounds */ #endif -#define DSEC2TICK(dsec) MSEC2TICK((dsec)*MSEC_PER_DSEC) /* Exact */ -#define SEC2TICK(sec) MSEC2TICK((sec)*MSEC_PER_SEC) /* Exact */ +#define DSEC2TICK(dsec) MSEC2TICK((dsec) * MSEC_PER_DSEC) /* Rounds */ +#define SEC2TICK(sec) MSEC2TICK((sec) * MSEC_PER_SEC) /* Rounds */ -#define TICK2NSEC(tick) ((tick)*NSEC_PER_TICK) /* Exact */ -#define TICK2USEC(tick) ((tick)*USEC_PER_TICK) /* Exact */ +#define TICK2NSEC(tick) ((tick) * NSEC_PER_TICK) /* Exact */ +#define TICK2USEC(tick) ((tick) * USEC_PER_TICK) /* Exact */ #if (MSEC_PER_TICK * USEC_PER_MSEC) == USEC_PER_TICK # define TICK2MSEC(tick) ((tick)*MSEC_PER_TICK) /* Exact */ diff --git a/libc/audio/lib_buffer.c b/libc/audio/lib_buffer.c index 50c504b46a..3fff9f3e5a 100644 --- a/libc/audio/lib_buffer.c +++ b/libc/audio/lib_buffer.c @@ -174,7 +174,7 @@ void apb_free(FAR struct ap_buffer_s *apb) if (refcount <= 1) { - auddbg("Freeing %p\n", apb); + audvdbg("Freeing %p\n", apb); lib_ufree(apb); } }