diff mbox series

[RFC,12/15] ASoC: rt5677: Transfer one period at a time over SPI

Message ID 20190906194636.217881-13-cujomalainey@chromium.org (mailing list archive)
State New, archived
Headers show
Series Add Samus Hotwording for RT5677 | expand

Commit Message

Curtis Malainey Sept. 6, 2019, 7:46 p.m. UTC 
From: Ben Zhang <benzh@chromium.org>

Rewrite the ring buffer transfer functions to copy one period
at a time then call snd_pcm_period_elapsed(). This reduces the
latency of transferring the initial ~2sec of audio after hotword
detect since audio samples are available for userspace earlier.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677-spi.c | 91 +++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 31 deletions(-)

Comments

Mark Brown Sept. 11, 2019, 10:54 a.m. UTC | #1 
On Fri, Sep 06, 2019 at 12:46:34PM -0700, Curtis Malainey wrote:
> From: Ben Zhang <benzh@chromium.org>
> 
> Rewrite the ring buffer transfer functions to copy one period
> at a time then call snd_pcm_period_elapsed(). This reduces the
> latency of transferring the initial ~2sec of audio after hotword
> detect since audio samples are available for userspace earlier.

This is fixing an earlier patch in the series, why not squash it
in?
Curtis Malainey Sept. 11, 2019, 6:09 p.m. UTC | #2 
On Wed, Sep 11, 2019 at 3:54 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 06, 2019 at 12:46:34PM -0700, Curtis Malainey wrote:
> > From: Ben Zhang <benzh@chromium.org>
> >
> > Rewrite the ring buffer transfer functions to copy one period
> > at a time then call snd_pcm_period_elapsed(). This reduces the
> > latency of transferring the initial ~2sec of audio after hotword
> > detect since audio samples are available for userspace earlier.
>
> This is fixing an earlier patch in the series, why not squash it
> in?

Definitely can do that, I originally only squashed all the bug fixes
in. For v2 I can also squash any rewrites like this one.
diff mbox series

Patch

diff --git a/sound/soc/codecs/rt5677-spi.c b/sound/soc/codecs/rt5677-spi.c
index 25d75a803cb5..b9b0f646127d 100644
--- a/sound/soc/codecs/rt5677-spi.c
+++ b/sound/soc/codecs/rt5677-spi.c
@@ -51,7 +51,8 @@ 
 #define RT5677_BUF_BYTES_TOTAL		0x20000
 #define RT5677_MIC_BUF_ADDR		0x60030000
 #define RT5677_MODEL_ADDR		0x5FFC9800
-#define RT5677_MIC_BUF_BYTES		(RT5677_BUF_BYTES_TOTAL - sizeof(u32))
+#define RT5677_MIC_BUF_BYTES		((u32)(RT5677_BUF_BYTES_TOTAL - \
+					sizeof(u32)))
 #define RT5677_MIC_BUF_FIRST_READ_SIZE	0x10000
 
 static struct spi_device *g_spi;
@@ -205,15 +206,15 @@  static int rt5677_spi_mic_write_offset(u32 *mic_write_offset)
 }
 
 /*
- * Copy a block of audio samples from the DSP mic buffer to the dma_area of
- * the pcm runtime. The receiving buffer may wrap around.
+ * Copy one contiguous block of audio samples from the DSP mic buffer to the
+ * dma_area of the pcm runtime. The receiving buffer may wrap around.
  * @begin: start offset of the block to copy, in bytes.
  * @end:   offset of the first byte after the block to copy, must be greater
  *         than or equal to begin.
  *
  * Return: Zero if successful, or a negative error code on failure.
  */
-static int rt5677_spi_append_data(struct rt5677_dsp *rt5677_dsp,
+static int rt5677_spi_copy_block(struct rt5677_dsp *rt5677_dsp,
 		u32 begin, u32 end)
 {
 	struct snd_pcm_runtime *runtime = rt5677_dsp->substream->runtime;
@@ -269,6 +270,38 @@  static int rt5677_spi_append_data(struct rt5677_dsp *rt5677_dsp,
 	return ret;
 }
 
+/*
+ * Copy a given amount of audio samples from the DSP mic buffer starting at
+ * mic_read_offset, to the dma_area of the pcm runtime. The source buffer may
+ * wrap around. mic_read_offset is updated after successful copy.
+ * @amount: amount of samples to copy, in bytes.
+ *
+ * Return: Zero if successful, or a negative error code on failure.
+ */
+static int rt5677_spi_copy(struct rt5677_dsp *rt5677_dsp, u32 amount)
+{
+	int ret = 0;
+	u32 target;
+
+	if (amount == 0)
+		return ret;
+
+	target = rt5677_dsp->mic_read_offset + amount;
+	/* Copy the first chunk in DSP's mic buffer */
+	ret |= rt5677_spi_copy_block(rt5677_dsp, rt5677_dsp->mic_read_offset,
+			min(target, RT5677_MIC_BUF_BYTES));
+
+	if (target >= RT5677_MIC_BUF_BYTES) {
+		/* Wrap around, copy the second chunk */
+		target -= RT5677_MIC_BUF_BYTES;
+		ret |= rt5677_spi_copy_block(rt5677_dsp, 0, target);
+	}
+
+	if (!ret)
+		rt5677_dsp->mic_read_offset = target;
+	return ret;
+}
+
 /*
  * A delayed work that streams audio samples from the DSP mic buffer to the
  * dma_area of the pcm runtime via SPI.
@@ -279,7 +312,7 @@  static void rt5677_spi_copy_work(struct work_struct *work)
 		container_of(work, struct rt5677_dsp, copy_work.work);
 	struct snd_pcm_runtime *runtime;
 	u32 mic_write_offset;
-	size_t bytes_copied, period_bytes;
+	size_t new_bytes, copy_bytes, period_bytes;
 	int ret = 0;
 
 	/* Ensure runtime->dma_area buffer does not go away while copying. */
@@ -312,35 +345,31 @@  static void rt5677_spi_copy_work(struct work_struct *work)
 					RT5677_MIC_BUF_FIRST_READ_SIZE;
 	}
 
-	/* Copy all new samples from DSP's mic buffer to dma_area */
-	bytes_copied = 0;
-	if (rt5677_dsp->mic_read_offset < mic_write_offset) {
-		/* One chunk in DSP's mic buffer */
-		ret |= rt5677_spi_append_data(rt5677_dsp,
-				rt5677_dsp->mic_read_offset, mic_write_offset);
-		bytes_copied = mic_write_offset - rt5677_dsp->mic_read_offset;
-	} else if (rt5677_dsp->mic_read_offset > mic_write_offset) {
-		/* Wrap around, two chunks in DSP's mic buffer */
-		ret |= rt5677_spi_append_data(rt5677_dsp,
-				rt5677_dsp->mic_read_offset,
-				RT5677_MIC_BUF_BYTES);
-		ret |= rt5677_spi_append_data(rt5677_dsp, 0, mic_write_offset);
-		bytes_copied = RT5677_MIC_BUF_BYTES -
-				rt5677_dsp->mic_read_offset + mic_write_offset;
-	}
-	if (ret) {
-		dev_err(rt5677_dsp->dev, "Copy failed %d\n", ret);
-		goto done;
-	}
+	/* Calculate the amount of new samples in bytes */
+	if (rt5677_dsp->mic_read_offset <= mic_write_offset)
+		new_bytes = mic_write_offset - rt5677_dsp->mic_read_offset;
+	else
+		new_bytes = RT5677_MIC_BUF_BYTES + mic_write_offset
+				- rt5677_dsp->mic_read_offset;
 
-	rt5677_dsp->mic_read_offset = mic_write_offset;
-	rt5677_dsp->avail_bytes += bytes_copied;
+	/* Copy all new samples from DSP mic buffer, one period at a time */
 	period_bytes = snd_pcm_lib_period_bytes(rt5677_dsp->substream);
-
-	if (rt5677_dsp->avail_bytes >= period_bytes) {
-		snd_pcm_period_elapsed(rt5677_dsp->substream);
-		rt5677_dsp->avail_bytes = 0;
+	while (new_bytes) {
+		copy_bytes = min(new_bytes, period_bytes
+				- rt5677_dsp->avail_bytes);
+		ret = rt5677_spi_copy(rt5677_dsp, copy_bytes);
+		if (ret) {
+			dev_err(rt5677_dsp->dev, "Copy failed %d\n", ret);
+			goto done;
+		}
+		rt5677_dsp->avail_bytes += copy_bytes;
+		if (rt5677_dsp->avail_bytes >= period_bytes) {
+			snd_pcm_period_elapsed(rt5677_dsp->substream);
+			rt5677_dsp->avail_bytes = 0;
+		}
+		new_bytes -= copy_bytes;
 	}
+
 	/* TODO benzh: use better delay time based on period_bytes */
 	schedule_delayed_work(&rt5677_dsp->copy_work, msecs_to_jiffies(5));
 done: