[alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[Curtis Malainey]

This patch series adds the hotwording implementation used in the
Pixelbook on the RT5677 driver.

Known Issues:
There is a known issue where the system will fail to detect a hotword if
suspended while the stream is open. This is due to the fact that the
haswell-dsp suspends its I2S MCLK before the RT5677 suspends which
causes the writes and reads to become corrupted as a result. Any
recommendations to correct this behaviour would be appreciated.

Ben Zhang (12):
  ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF
  ASoC: rt5677: Add a PCM device for streaming hotword via SPI
  ASoC: rt5677: Load firmware via SPI
  ASoC: rt5677: Auto enable/disable DSP for hotwording
  ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device
  ASoC: rt5677: Enable jack detect while DSP is running
  ASoC: rt5677: Use delayed work for DSP firmware load
  ASoC: rt5677: Add DAPM audio path for hotword stream
  ASoC: rt5677: Mark reg RT5677_PWR_ANLG2 as volatile
  ASoC: rt5677: Stop and restart DSP over suspend/resume
  ASoC: rt5677: Transfer one period at a time over SPI
  ASoC: rt5677: Disable irq at suspend

Curtis Malainey (3):
  ASoC: rt5677: Remove magic number register writes
  ASoC: rt5677: Allow VAD to be shut on/off at all times
  ASoC: rt5677: Turn on MCLK1 for DSP via DAPM

 sound/soc/codecs/rt5677-spi.c       | 401 ++++++++++++++++++++++
 sound/soc/codecs/rt5677-spi.h       |   1 +
 sound/soc/codecs/rt5677.c           | 496 +++++++++++++++++++++++-----
 sound/soc/codecs/rt5677.h           |   9 +-
 sound/soc/intel/boards/bdw-rt5677.c |  18 +
 5 files changed, 843 insertions(+), 82 deletions(-)

-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 01/15] ASoC: rt5677: Remove magic number register writes

[Curtis Malainey]

In order to simplify understanding what register values are being
written to the codec for debugging more advanced features (such as
hotwording) it is best to remove magic numbers

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index c779dc3474f9..5b6ca3ced13b 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -691,10 +691,12 @@ static void rt5677_set_dsp_mode(struct snd_soc_component *component, bool on)
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 
 	if (on) {
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x2);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
+			RT5677_PWR_DSP, RT5677_PWR_DSP);
 		rt5677->is_dsp_mode = true;
 	} else {
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x0);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
+			RT5677_PWR_DSP, 0x0);
 		rt5677->is_dsp_mode = false;
 	}
 }
@@ -4466,7 +4468,8 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 
 			regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,
 				RT5677_LDO1_SEL_MASK | RT5677_LDO2_SEL_MASK,
-				0x0055);
+				5 << RT5677_LDO1_SEL_SFT |
+				5 << RT5677_LDO2_SEL_SFT);
 			regmap_update_bits(rt5677->regmap,
 				RT5677_PR_BASE + RT5677_BIAS_CUR4,
 				0x0f00, 0x0f00);
@@ -4491,7 +4494,9 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x0);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG1, 0x0000);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG2, 0x0000);
-		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1, 0x0022);
+		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1,
+			2 << RT5677_LDO1_SEL_SFT |
+			2 << RT5677_LDO2_SEL_SFT);
 		regmap_write(rt5677->regmap, RT5677_PWR_ANLG2, 0x0000);
 		regmap_update_bits(rt5677->regmap,
 			RT5677_PR_BASE + RT5677_BIAS_CUR4, 0x0f00, 0x0000);
@@ -4719,7 +4724,8 @@ static int rt5677_probe(struct snd_soc_component *component)
 
 	regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC,
 			~RT5677_IRQ_DEBOUNCE_SEL_MASK, 0x0020);
-	regmap_write(rt5677->regmap, RT5677_PWR_DSP2, 0x0c00);
+	regmap_write(rt5677->regmap, RT5677_PWR_DSP2,
+			RT5677_PWR_SLIM_ISO | RT5677_PWR_CORE_ISO);
 
 	for (i = 0; i < RT5677_GPIO_NUM; i++)
 		rt5677_gpio_config(rt5677, i, rt5677->pdata.gpio_config[i]);
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 02/15] ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

Instead of clearing RT5677_PWR_ANLG2 (MX-64h) to 0 at SND_SOC_BIAS_OFF,
we only clear the RT5677_PWR_CORE bit which is set at SND_SOC_BIAS_PREPARE.
MICBIAS control bits are left unchanged.

This fixed the bug where if MICBIAS1 widget is forced on, MICBIAS
control bits will be cleared at suspend and never turned back on again,
since DAPM thinks the widget is always on.

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 5b6ca3ced13b..315a3d39bc09 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4493,11 +4493,11 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 	case SND_SOC_BIAS_OFF:
 		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x0);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG1, 0x0000);
-		regmap_write(rt5677->regmap, RT5677_PWR_DIG2, 0x0000);
 		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1,
 			2 << RT5677_LDO1_SEL_SFT |
 			2 << RT5677_LDO2_SEL_SFT);
-		regmap_write(rt5677->regmap, RT5677_PWR_ANLG2, 0x0000);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
+			RT5677_PWR_CORE, 0);
 		regmap_update_bits(rt5677->regmap,
 			RT5677_PR_BASE + RT5677_BIAS_CUR4, 0x0f00, 0x0000);
 
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 03/15] ASoC: rt5677: Add a PCM device for streaming hotword via SPI

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

This patch implements a PCM interface for streaming hotword
phrases over SPI. Userspace can open the PCM device at anytime.
The stream is blocked when no hotword is detected. The mic
audio buffer on the DSP is a ~128KByte ring buffer that holds
~4sec of audio samples recorded from the DMIC (S16_LE, mono,
16KHz). After a hotword is detected, previous 2 seconds of audio
(containing the detected hotword) is streamed first, then live
capture continues until userspace closes the PCM stream.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677-spi.c | 360 ++++++++++++++++++++++++++++++++++
 sound/soc/codecs/rt5677-spi.h |   1 +
 2 files changed, 361 insertions(+)

diff --git a/sound/soc/codecs/rt5677-spi.c b/sound/soc/codecs/rt5677-spi.c
index d681488f5312..62621fe4747c 100644
--- a/sound/soc/codecs/rt5677-spi.c
+++ b/sound/soc/codecs/rt5677-spi.c
@@ -24,6 +24,8 @@
 #include <linux/firmware.h>
 #include <linux/acpi.h>
 
+#include <sound/soc.h>
+
 #include "rt5677-spi.h"
 
 #define DRV_NAME "rt5677spi"
@@ -45,9 +47,330 @@
 #define RT5677_SPI_WRITE_16	0x1
 #define RT5677_SPI_READ_16	0x0
 
+#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_FIRST_READ_SIZE	0x10000
+
 static struct spi_device *g_spi;
 static DEFINE_MUTEX(spi_mutex);
 
+struct rt5677_dsp {
+	struct device *dev;
+	struct delayed_work copy_work;
+	struct mutex dma_lock;
+	struct snd_pcm_substream *substream;
+	size_t dma_offset;	/* zero-based offset into runtime->dma_area */
+	size_t avail_bytes;	/* number of new bytes since last period */
+	u32 mic_read_offset;	/* zero-based offset into DSP's mic buffer */
+	bool new_hotword;	/* a new hotword is fired */
+};
+
+static const struct snd_pcm_hardware rt5677_spi_pcm_hardware = {
+	.info			= SNDRV_PCM_INFO_MMAP |
+				  SNDRV_PCM_INFO_MMAP_VALID |
+				  SNDRV_PCM_INFO_INTERLEAVED,
+	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
+	.period_bytes_min	= PAGE_SIZE,
+	.period_bytes_max	= RT5677_BUF_BYTES_TOTAL / 8,
+	.periods_min		= 8,
+	.periods_max		= 8,
+	.channels_min		= 1,
+	.channels_max		= 1,
+	.buffer_bytes_max	= RT5677_BUF_BYTES_TOTAL,
+};
+
+static struct snd_soc_dai_driver rt5677_spi_dai = {
+	/* The DAI name "rt5677-dsp-cpu-dai" is not used. The actual DAI name
+	 * registered with ASoC is the name of the device "spi-RT5677AA:00",
+	 * because we only have one DAI. See snd_soc_register_dais().
+	 */
+	.name = "rt5677-dsp-cpu-dai",
+	.id = 0,
+	.capture = {
+		.stream_name = "DSP Capture",
+		.channels_min = 1,
+		.channels_max = 1,
+		.rates = SNDRV_PCM_RATE_16000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+	},
+};
+
+/* PCM for streaming audio from the DSP buffer */
+static int rt5677_spi_pcm_open(struct snd_pcm_substream *substream)
+{
+	snd_soc_set_runtime_hwparams(substream, &rt5677_spi_pcm_hardware);
+	return 0;
+}
+
+static int rt5677_spi_pcm_close(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct rt5677_dsp *rt5677_dsp =
+			snd_soc_component_get_drvdata(component);
+
+	cancel_delayed_work_sync(&rt5677_dsp->copy_work);
+	return 0;
+}
+
+static int rt5677_spi_hw_params(struct snd_pcm_substream *substream,
+			       struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct rt5677_dsp *rt5677_dsp =
+			snd_soc_component_get_drvdata(component);
+	int ret;
+
+	mutex_lock(&rt5677_dsp->dma_lock);
+	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
+			params_buffer_bytes(hw_params));
+	rt5677_dsp->substream = substream;
+	mutex_unlock(&rt5677_dsp->dma_lock);
+
+	return ret;
+}
+
+static int rt5677_spi_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct rt5677_dsp *rt5677_dsp =
+			snd_soc_component_get_drvdata(component);
+
+	mutex_lock(&rt5677_dsp->dma_lock);
+	rt5677_dsp->substream = 0;
+	mutex_unlock(&rt5677_dsp->dma_lock);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int rt5677_spi_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct rt5677_dsp *rt5677_dsp =
+			snd_soc_component_get_drvdata(component);
+
+	rt5677_dsp->dma_offset = 0;
+	rt5677_dsp->avail_bytes = 0;
+	return 0;
+}
+
+static snd_pcm_uframes_t rt5677_spi_pcm_pointer(
+		struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct rt5677_dsp *rt5677_dsp =
+			snd_soc_component_get_drvdata(component);
+
+	return bytes_to_frames(runtime, rt5677_dsp->dma_offset);
+}
+
+static int rt5677_spi_mic_write_offset(u32 *mic_write_offset)
+{
+	int ret;
+	/* Grab the first 4 bytes that hold the write pointer on the
+	 * dsp, and check to make sure that it points somewhere inside the
+	 * buffer.
+	 */
+	ret = rt5677_spi_read(RT5677_MIC_BUF_ADDR, mic_write_offset,
+			sizeof(u32));
+	if (ret)
+		return ret;
+	/* Adjust the offset so that it's zero-based */
+	*mic_write_offset = *mic_write_offset - sizeof(u32);
+	return *mic_write_offset < RT5677_MIC_BUF_BYTES ? 0 : -EFAULT;
+}
+
+/*
+ * 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.
+ * @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,
+		u32 begin, u32 end)
+{
+	struct snd_pcm_runtime *runtime = rt5677_dsp->substream->runtime;
+	size_t bytes_per_frame = frames_to_bytes(runtime, 1);
+	size_t first_chunk_len, second_chunk_len;
+	int ret;
+
+	if (begin > end || runtime->dma_bytes < 2 * bytes_per_frame) {
+		dev_err(rt5677_dsp->dev,
+			"Invalid copy from (%u, %u), dma_area size %zu\n",
+			begin, end, runtime->dma_bytes);
+		return -EINVAL;
+	}
+
+	/* The block to copy is empty */
+	if (begin == end)
+		return 0;
+
+	/* If the incoming chunk is too big for the receiving buffer, only the
+	 * last "receiving buffer size - one frame" bytes are copied.
+	 */
+	if (end - begin > runtime->dma_bytes - bytes_per_frame)
+		begin = end - (runtime->dma_bytes - bytes_per_frame);
+
+	/* May need to split to two chunks, calculate the size of each */
+	first_chunk_len = end - begin;
+	second_chunk_len = 0;
+	if (rt5677_dsp->dma_offset + first_chunk_len > runtime->dma_bytes) {
+		/* Receiving buffer wrapped around */
+		second_chunk_len = first_chunk_len;
+		first_chunk_len = runtime->dma_bytes - rt5677_dsp->dma_offset;
+		second_chunk_len -= first_chunk_len;
+	}
+
+	/* Copy first chunk */
+	ret = rt5677_spi_read(RT5677_MIC_BUF_ADDR + sizeof(u32) + begin,
+			runtime->dma_area + rt5677_dsp->dma_offset,
+			first_chunk_len);
+	if (ret)
+		return ret;
+	rt5677_dsp->dma_offset += first_chunk_len;
+	if (rt5677_dsp->dma_offset == runtime->dma_bytes)
+		rt5677_dsp->dma_offset = 0;
+
+	/* Copy second chunk */
+	if (second_chunk_len) {
+		ret = rt5677_spi_read(RT5677_MIC_BUF_ADDR + sizeof(u32) +
+				begin + first_chunk_len, runtime->dma_area,
+				second_chunk_len);
+		if (!ret)
+			rt5677_dsp->dma_offset = second_chunk_len;
+	}
+	return ret;
+}
+
+/*
+ * A delayed work that streams audio samples from the DSP mic buffer to the
+ * dma_area of the pcm runtime via SPI.
+ */
+static void rt5677_spi_copy_work(struct work_struct *work)
+{
+	struct rt5677_dsp *rt5677_dsp =
+		container_of(work, struct rt5677_dsp, copy_work.work);
+	struct snd_pcm_runtime *runtime;
+	u32 mic_write_offset;
+	size_t bytes_copied, period_bytes;
+	int ret = 0;
+
+	/* Ensure runtime->dma_area buffer does not go away while copying. */
+	mutex_lock(&rt5677_dsp->dma_lock);
+	if (!rt5677_dsp->substream) {
+		dev_err(rt5677_dsp->dev, "No pcm substream\n");
+		goto done;
+	}
+
+	runtime = rt5677_dsp->substream->runtime;
+
+	if (rt5677_spi_mic_write_offset(&mic_write_offset)) {
+		dev_err(rt5677_dsp->dev, "No mic_write_offset\n");
+		goto done;
+	}
+
+	/* If this is the first time that we've asked for streaming data after
+	 * a hotword is fired, we should start reading from the previous 2
+	 * seconds of audio from wherever the mic_write_offset is currently.
+	 */
+	if (rt5677_dsp->new_hotword) {
+		rt5677_dsp->new_hotword = false;
+		/* See if buffer wraparound happens */
+		if (mic_write_offset < RT5677_MIC_BUF_FIRST_READ_SIZE)
+			rt5677_dsp->mic_read_offset = RT5677_MIC_BUF_BYTES -
+					(RT5677_MIC_BUF_FIRST_READ_SIZE -
+					mic_write_offset);
+		else
+			rt5677_dsp->mic_read_offset = mic_write_offset -
+					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;
+	}
+
+	rt5677_dsp->mic_read_offset = mic_write_offset;
+	rt5677_dsp->avail_bytes += bytes_copied;
+	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;
+	}
+	/* TODO benzh: use better delay time based on period_bytes */
+	schedule_delayed_work(&rt5677_dsp->copy_work, msecs_to_jiffies(5));
+done:
+	mutex_unlock(&rt5677_dsp->dma_lock);
+}
+
+struct page *rt5677_spi_pcm_page(struct snd_pcm_substream *substream,
+		unsigned long offset)
+{
+	return snd_pcm_lib_get_vmalloc_page(substream, offset);
+}
+
+static struct snd_pcm_ops rt5677_spi_pcm_ops = {
+	.open		= rt5677_spi_pcm_open,
+	.close		= rt5677_spi_pcm_close,
+	.hw_params	= rt5677_spi_hw_params,
+	.hw_free	= rt5677_spi_hw_free,
+	.prepare	= rt5677_spi_prepare,
+	.pointer	= rt5677_spi_pcm_pointer,
+	.page		= rt5677_spi_pcm_page,
+};
+
+static int rt5677_spi_pcm_probe(struct snd_soc_component *component)
+{
+	struct rt5677_dsp *rt5677_dsp;
+
+	rt5677_dsp = devm_kzalloc(component->dev, sizeof(*rt5677_dsp),
+			GFP_KERNEL);
+	rt5677_dsp->dev = &g_spi->dev;
+	mutex_init(&rt5677_dsp->dma_lock);
+	INIT_DELAYED_WORK(&rt5677_dsp->copy_work, rt5677_spi_copy_work);
+
+	snd_soc_component_set_drvdata(component, rt5677_dsp);
+	return 0;
+}
+
+static const struct snd_soc_component_driver rt5677_spi_dai_component = {
+	.name		= DRV_NAME,
+	.probe		= rt5677_spi_pcm_probe,
+	.ops		= &rt5677_spi_pcm_ops,
+};
+
 /* Select a suitable transfer command for the next transfer to ensure
  * the transfer address is always naturally aligned while minimizing
  * the total number of transfers required.
@@ -218,9 +541,45 @@ int rt5677_spi_write_firmware(u32 addr, const struct firmware *fw)
 }
 EXPORT_SYMBOL_GPL(rt5677_spi_write_firmware);
 
+void rt5677_spi_hotword_detected(void)
+{
+	struct rt5677_dsp *rt5677_dsp;
+
+	if (!g_spi)
+		return;
+
+	rt5677_dsp = dev_get_drvdata(&g_spi->dev);
+	if (!rt5677_dsp) {
+		dev_err(&g_spi->dev, "Can't get rt5677_dsp\n");
+		return;
+	}
+
+	mutex_lock(&rt5677_dsp->dma_lock);
+	dev_info(rt5677_dsp->dev, "Hotword detected\n");
+	rt5677_dsp->new_hotword = true;
+	mutex_unlock(&rt5677_dsp->dma_lock);
+
+	schedule_delayed_work(&rt5677_dsp->copy_work, 0);
+}
+EXPORT_SYMBOL_GPL(rt5677_spi_hotword_detected);
+
 static int rt5677_spi_probe(struct spi_device *spi)
 {
+	int ret;
+
 	g_spi = spi;
+
+	ret = snd_soc_register_component(&spi->dev, &rt5677_spi_dai_component,
+					 &rt5677_spi_dai, 1);
+	if (ret < 0)
+		dev_err(&spi->dev, "Failed to register component.\n");
+
+	return ret;
+}
+
+static int rt5677_spi_remove(struct spi_device *spi)
+{
+	snd_soc_unregister_component(&spi->dev);
 	return 0;
 }
 
@@ -236,6 +595,7 @@ static struct spi_driver rt5677_spi_driver = {
 		.acpi_match_table = ACPI_PTR(rt5677_spi_acpi_id),
 	},
 	.probe = rt5677_spi_probe,
+	.remove = rt5677_spi_remove,
 };
 module_spi_driver(rt5677_spi_driver);
 
diff --git a/sound/soc/codecs/rt5677-spi.h b/sound/soc/codecs/rt5677-spi.h
index 6ba3369dc235..3af36ec928e9 100644
--- a/sound/soc/codecs/rt5677-spi.h
+++ b/sound/soc/codecs/rt5677-spi.h
@@ -12,5 +12,6 @@
 int rt5677_spi_read(u32 addr, void *rxbuf, size_t len);
 int rt5677_spi_write(u32 addr, const void *txbuf, size_t len);
 int rt5677_spi_write_firmware(u32 addr, const struct firmware *fw);
+void rt5677_spi_hotword_detected(void);
 
 #endif /* __RT5677_SPI_H__ */
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 04/15] ASoC: rt5677: Load firmware via SPI

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

The firmware rt5677_elf_vad is an ELF binary obtained from
request_firmware(). Sections of the ELF are loaded to
the DSP via SPI. A model (e.g. en_us.mmap) can optionally be
loaded to the DSP at RT5677_MODEL_ADDR to overwrite the
baked-in model in rt5677_elf_vad.

When 'DSP VAD Switch' is turned on, rt5677_set_vad_source()
enables the following digital path:

DMIC L1 ->
Mono DMIC L Mux ->
Mono ADC2 L Mux ->
Mono ADC MIXL ->
VAD ADC Mux ->
IB01 Mux

Then we switch to DSP mode, load firmware, and let DSP run.
When a hotword is detected, an interrupt is fired and
rt5677_irq() is called. When 'DSP VAD Switch' is turned off,
the codec is set back to normal mode.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 306 +++++++++++++++++++++++++++++++-------
 sound/soc/codecs/rt5677.h |   1 +
 2 files changed, 254 insertions(+), 53 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 315a3d39bc09..35d4ec1b7dfd 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -38,6 +38,10 @@
 
 #define RT5677_DEVICE_ID 0x6327
 
+/* Register controlling boot vector */
+#define RT5677_DSP_BOOT_VECTOR		0x1801f090
+#define RT5677_MODEL_ADDR		0x5FFC9800
+
 #define RT5677_PR_RANGE_BASE (0xff + 1)
 #define RT5677_PR_SPACING 0x100
 
@@ -701,6 +705,210 @@ static void rt5677_set_dsp_mode(struct snd_soc_component *component, bool on)
 	}
 }
 
+static unsigned int rt5677_set_vad_source(
+	struct snd_soc_component *component)
+{
+	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
+
+	/* Mono ADC Capture Switch = unmute (default) */
+	regmap_update_bits(rt5677->regmap, RT5677_MONO_ADC_DIG_VOL,
+			RT5677_L_MUTE, 0);
+
+	/* Mono ADC Boost Volume = 24dB */
+	regmap_update_bits(rt5677->regmap, RT5677_ADC_BST_CTRL2,
+		RT5677_MONO_ADC_L_BST_MASK | RT5677_MONO_ADC_R_BST_MASK,
+		(0x2 << RT5677_MONO_ADC_L_BST_SFT) |
+		(0x2 << RT5677_MONO_ADC_R_BST_SFT));
+
+	/* Mono ADC MIXL = Mono ADC2 L Mux (unmute)
+	 *                 Mono ADC1 L Mux (mute)
+	 * Mono ADC2 L Mux = Mono DMIC L Mux
+	 * Mono DMIC L Mux = DMIC1 (left)
+	 */
+	regmap_update_bits(rt5677->regmap, RT5677_MONO_ADC_MIXER,
+		RT5677_M_MONO_ADC_L2 | RT5677_M_MONO_ADC_L1 |
+		RT5677_SEL_MONO_ADC_L2_MASK | RT5677_SEL_MONO_DMIC_L_MASK,
+		RT5677_M_MONO_ADC_L1 | (1 << RT5677_SEL_MONO_ADC_L2_SFT) |
+		(0 << RT5677_SEL_MONO_DMIC_L_SFT));
+
+	/* DMIC1 power = enabled
+	 * DMIC CLK = 256 * fs / 12
+	 */
+	regmap_update_bits(rt5677->regmap, RT5677_DMIC_CTRL1,
+		RT5677_DMIC_1_EN_MASK | RT5677_DMIC_CLK_MASK,
+		RT5677_DMIC_1_EN | (5 << RT5677_DMIC_CLK_SFT));
+
+	/* I2S pre divide 2 = /6 (clk_sys2) */
+	regmap_update_bits(rt5677->regmap, RT5677_CLK_TREE_CTRL1,
+		RT5677_I2S_PD2_MASK, RT5677_I2S_PD2_6);
+
+	/* System Clock = MCLK1
+	 * Stereo ADC/DAC over sample rate = 128Fs (default)
+	 */
+	regmap_write(rt5677->regmap, RT5677_GLB_CLK1, 0x0000);
+
+	/* DSP Clock = MCLK1 (bypassed PLL2) */
+	regmap_write(rt5677->regmap, RT5677_GLB_CLK2,
+		RT5677_DSP_CLK_SRC_BYPASS);
+
+	/* Clock source for Mono L ADC = clk_sys2 */
+	regmap_update_bits(rt5677->regmap, RT5677_ASRC_6,
+		RT5677_AD_MONOL_CLK_SEL_MASK, 7 << RT5677_AD_MONOL_CLK_SEL_SFT);
+
+	/* SAD Threshold1 */
+	regmap_write(rt5677->regmap, RT5677_VAD_CTRL2, 0x013f);
+	/* SAD Threshold2 */
+	regmap_write(rt5677->regmap, RT5677_VAD_CTRL3, 0x0ae5);
+	/* SAD Sample Rate Converter = Up 6 (8K to 48K)
+	 * SAD Output Sample Rate = Same as I2S
+	 * VAD ADC Mux = MONO ADC MIX L
+	 * SAD Threshold3
+	 */
+	regmap_write(rt5677->regmap, RT5677_VAD_CTRL4,
+		0x01 << RT5677_VAD_SRC_SFT |
+		0x7f << RT5677_VAD_LV_DIFF_SFT);
+	/* Minimum frame level within a pre-determined duration = 32 frames
+	 * Bypass ADPCM Encoder/Decoder = Bypass ADPCM
+	 * Automatic Push Data to SAD Buffer Once SAD Flag is triggered = enable
+	 * SAD Buffer Over-Writing = enable
+	 * SAD Buffer Pop Mode Control = disable
+	 * SAD Buffer Push Mode Control = enable
+	 * SAD Detector Control = enable
+	 * SAD Function Control = enable
+	 * SAD Function Reset = normal
+	 */
+	regmap_write(rt5677->regmap, RT5677_VAD_CTRL1,
+		RT5677_VAD_FUNC_RESET | RT5677_VAD_FUNC_ENABLE |
+		RT5677_VAD_DET_ENABLE | RT5677_VAD_BUF_PUSH |
+		RT5677_VAD_BUF_OW | RT5677_VAD_FG2ENC |
+		RT5677_VAD_ADPCM_BYPASS | 1 << RT5677_VAD_MIN_DUR_SFT);
+
+	/* InBound0/1 Source = VAD ADC/DAC1 FS */
+	regmap_update_bits(rt5677->regmap, RT5677_DSP_INB_CTRL1,
+		RT5677_IB01_SRC_MASK, 4 << RT5677_IB01_SRC_SFT);
+
+	/* IRQ Source of VAD Jack Detection = enable */
+	regmap_write(rt5677->regmap, RT5677_IRQ_CTRL2, 0x4000);
+
+	/* Enable Gating Mode with MCLK = enable */
+	regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x1);
+
+	/* Private register, no doc */
+	regmap_update_bits(rt5677->regmap, RT5677_PR_BASE + RT5677_BIAS_CUR4,
+		0x0f00, 0x0100);
+
+	/* adc mono left filter = power on */
+	regmap_update_bits(rt5677->regmap, RT5677_PWR_DIG2,
+		RT5677_PWR_ADC_MF_L, RT5677_PWR_ADC_MF_L);
+
+	/* LDO2 output = 1.2V
+	 * LDO1 output = 1.2V (LDO_IN = 1.8V)
+	 */
+	regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,
+		RT5677_LDO1_SEL_MASK | RT5677_LDO2_SEL_MASK,
+		5 << RT5677_LDO1_SEL_SFT | 5 << RT5677_LDO2_SEL_SFT);
+
+	/* Codec core power =  power on
+	 * LDO1 power = power on
+	 */
+	regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
+		RT5677_PWR_CORE | RT5677_PWR_LDO1,
+		RT5677_PWR_CORE | RT5677_PWR_LDO1);
+
+	/* Isolation for DCVDD4 = normal (set during probe)
+	 * Isolation for DCVDD2 = normal (set during probe)
+	 * Isolation for DSP = normal
+	 * Isolation for Band 0~7 = disable
+	 * Isolation for InBound 4~10 and OutBound 4~10 = disable
+	 */
+	regmap_write(rt5677->regmap, RT5677_PWR_DSP2,
+		RT5677_PWR_CORE_ISO | RT5677_PWR_DSP_ISO |
+		RT5677_PWR_SR7_ISO | RT5677_PWR_SR6_ISO |
+		RT5677_PWR_SR5_ISO | RT5677_PWR_SR4_ISO |
+		RT5677_PWR_SR3_ISO | RT5677_PWR_SR2_ISO |
+		RT5677_PWR_SR1_ISO | RT5677_PWR_SR0_ISO |
+		RT5677_PWR_MLT_ISO);
+
+	/* System Band 0~7 = power on
+	 * InBound 4~10 and OutBound 4~10 = power on
+	 * DSP = power on
+	 * DSP CPU = stop (will be set to "run" after firmware loaded)
+	 */
+	regmap_write(rt5677->regmap, RT5677_PWR_DSP1,
+		RT5677_PWR_SR7 | RT5677_PWR_SR6 |
+		RT5677_PWR_SR5 | RT5677_PWR_SR4 |
+		RT5677_PWR_SR3 | RT5677_PWR_SR2 |
+		RT5677_PWR_SR1 | RT5677_PWR_SR0 |
+		RT5677_PWR_MLT | RT5677_PWR_DSP |
+		RT5677_PWR_DSP_CPU);
+
+	return 0;
+}
+
+static int rt5677_parse_and_load_dsp(struct rt5677_priv *rt5677, const u8 *buf,
+		unsigned int len)
+{
+	struct snd_soc_component *component = rt5677->component;
+	Elf32_Ehdr *elf_hdr;
+	Elf32_Phdr *pr_hdr;
+	Elf32_Half i;
+	int ret = 0;
+
+	if (!buf || (len < sizeof(Elf32_Ehdr)))
+		return -ENOMEM;
+
+	elf_hdr = (Elf32_Ehdr *)buf;
+#ifndef EM_XTENSA
+#define EM_XTENSA	94
+#endif
+	if (strncmp(elf_hdr->e_ident, ELFMAG, sizeof(ELFMAG) - 1))
+		dev_err(component->dev, "Wrong ELF header prefix\n");
+	if (elf_hdr->e_ehsize != sizeof(Elf32_Ehdr))
+		dev_err(component->dev, "Wrong Elf header size\n");
+	if (elf_hdr->e_machine != EM_XTENSA)
+		dev_err(component->dev, "Wrong DSP code file\n");
+
+	if (len < elf_hdr->e_phoff)
+		return -ENOMEM;
+	pr_hdr = (Elf32_Phdr *)(buf + elf_hdr->e_phoff);
+	for (i = 0; i < elf_hdr->e_phnum; i++) {
+		/* TODO: handle p_memsz != p_filesz */
+		if (pr_hdr->p_paddr && pr_hdr->p_filesz) {
+			dev_info(component->dev, "Load 0x%x bytes to 0x%x\n",
+					pr_hdr->p_filesz, pr_hdr->p_paddr);
+
+			ret = rt5677_spi_write(pr_hdr->p_paddr,
+					buf + pr_hdr->p_offset,
+					pr_hdr->p_filesz);
+			if (ret)
+				dev_err(component->dev, "Load firmware failed %d\n",
+						ret);
+		}
+		pr_hdr++;
+	}
+	return ret;
+}
+
+static int rt5677_load_dsp_from_file(struct snd_soc_component *component)
+{
+	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
+	const struct firmware *fwp;
+	int ret = 0;
+
+	/* Load dsp firmware from rt5677_elf_vad file */
+	ret = request_firmware(&fwp, "rt5677_elf_vad", component->dev);
+	if (ret) {
+		dev_err(component->dev, "Request rt5677_elf_vad failed %d\n",
+			ret);
+		return ret;
+	}
+	dev_info(component->dev, "Requested rt5677_elf_vad (%zu)\n", fwp->size);
+
+	ret = rt5677_parse_and_load_dsp(rt5677, fwp->data, fwp->size);
+	release_firmware(fwp);
+	return ret;
+}
+
 static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
@@ -710,74 +918,52 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 	if (!IS_ENABLED(CONFIG_SND_SOC_RT5677_SPI))
 		return -ENXIO;
 
+	dev_info(component->dev, "DSP VAD: on=%d, activity=%d\n", on, activity);
 	if (on && !activity) {
 		activity = true;
 
-		regcache_cache_only(rt5677->regmap, false);
-		regcache_cache_bypass(rt5677->regmap, true);
+		/* Set GPIO1 as an output pin driving a 0. Firmware will
+		 * raise GPIO1 upon hotword detect.
+		 */
+		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
+			RT5677_GPIO1_DIR_MASK |	RT5677_GPIO1_OUT_MASK |
+			RT5677_GPIO1_P_MASK, RT5677_GPIO1_DIR_OUT |
+			RT5677_GPIO1_OUT_LO | RT5677_GPIO1_P_NOR);
+		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL1,
+			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_GPIO1);
 
-		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x1);
-		regmap_update_bits(rt5677->regmap,
-			RT5677_PR_BASE + RT5677_BIAS_CUR4, 0x0f00, 0x0f00);
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,
-			RT5677_LDO1_SEL_MASK, 0x0);
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
-			RT5677_PWR_LDO1, RT5677_PWR_LDO1);
-		switch (rt5677->type) {
-		case RT5677:
-			regmap_update_bits(rt5677->regmap, RT5677_GLB_CLK1,
-				RT5677_MCLK_SRC_MASK, RT5677_MCLK2_SRC);
-			regmap_update_bits(rt5677->regmap, RT5677_GLB_CLK2,
-				RT5677_PLL2_PR_SRC_MASK |
-				RT5677_DSP_CLK_SRC_MASK,
-				RT5677_PLL2_PR_SRC_MCLK2 |
-				RT5677_DSP_CLK_SRC_BYPASS);
-			break;
-		case RT5676:
-			regmap_update_bits(rt5677->regmap, RT5677_GLB_CLK2,
-				RT5677_DSP_CLK_SRC_MASK,
-				RT5677_DSP_CLK_SRC_BYPASS);
-			break;
-		default:
-			break;
-		}
-		regmap_write(rt5677->regmap, RT5677_PWR_DSP2, 0x07ff);
-		regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x07fd);
+		rt5677_set_vad_source(component);
 		rt5677_set_dsp_mode(component, true);
 
-		ret = request_firmware(&rt5677->fw1, RT5677_FIRMWARE1,
-			component->dev);
-		if (ret == 0) {
-			rt5677_spi_write_firmware(0x50000000, rt5677->fw1);
-			release_firmware(rt5677->fw1);
-		}
+		/* Boot the firmware from IRAM instead of SRAM0. */
+		rt5677_dsp_mode_i2c_write_addr(rt5677, RT5677_DSP_BOOT_VECTOR,
+			0x0009, 0x0003);
+		rt5677_dsp_mode_i2c_write_addr(rt5677, RT5677_DSP_BOOT_VECTOR,
+			0x0019, 0x0003);
+		rt5677_dsp_mode_i2c_write_addr(rt5677, RT5677_DSP_BOOT_VECTOR,
+			0x0009, 0x0003);
 
-		ret = request_firmware(&rt5677->fw2, RT5677_FIRMWARE2,
-			component->dev);
-		if (ret == 0) {
-			rt5677_spi_write_firmware(0x60000000, rt5677->fw2);
-			release_firmware(rt5677->fw2);
-		}
+		ret = rt5677_load_dsp_from_file(component);
 
+		/* Set DSP CPU to Run */
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x1, 0x0);
-
-		regcache_cache_bypass(rt5677->regmap, false);
-		regcache_cache_only(rt5677->regmap, true);
 	} else if (!on && activity) {
 		activity = false;
 
-		regcache_cache_only(rt5677->regmap, false);
-		regcache_cache_bypass(rt5677->regmap, true);
-
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x1, 0x1);
+		/* Set DSP CPU to Stop */
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
+			RT5677_PWR_DSP_CPU, RT5677_PWR_DSP_CPU);
 		rt5677_set_dsp_mode(component, false);
-		regmap_write(rt5677->regmap, RT5677_PWR_DSP1, 0x0001);
 
-		regmap_write(rt5677->regmap, RT5677_RESET, 0x10ec);
+		/* Disable and clear VAD interrupt */
+		regmap_write(rt5677->regmap, RT5677_VAD_CTRL1, 0x2184);
+		regmap_update_bits(rt5677->regmap, RT5677_IRQ_CTRL2,
+			0xF000, 0x0000);
+
+		/* Set GPIO1 pin back to be IRQ output for jack detect */
+		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL1,
+			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_IRQ);
 
-		regcache_cache_bypass(rt5677->regmap, false);
-		regcache_mark_dirty(rt5677->regmap);
-		regcache_sync(rt5677->regmap);
 	}
 
 	return 0;
@@ -4938,6 +5124,17 @@ static struct snd_soc_dai_driver rt5677_dai[] = {
 		},
 		.ops = &rt5677_aif_dai_ops,
 	},
+	{
+		.name = "rt5677-dspbuffer",
+		.id = RT5677_DSPBUFF,
+		.capture = {
+			.stream_name = "DSP Buffer",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = SNDRV_PCM_RATE_16000,
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+	},
 };
 
 static const struct snd_soc_component_driver soc_component_dev_rt5677 = {
@@ -5081,6 +5278,9 @@ static irqreturn_t rt5677_irq(int unused, void *data)
 
 	mutex_lock(&rt5677->irq_lock);
 
+	if (rt5677->dsp_vad_en)
+		rt5677_spi_hotword_detected();
+
 	/*
 	 * Loop to handle interrupts until the last i2c read shows no pending
 	 * irqs. The interrupt line is shared by multiple interrupt sources.
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 213f4b8ca269..2bbd618b51ac 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1730,6 +1730,7 @@ enum {
 	RT5677_AIF4,
 	RT5677_AIF5,
 	RT5677_AIFS,
+	RT5677_DSPBUFF,
 };
 
 enum {
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

The kcontrol 'DSP VAD Switch' is automatically enabled/disabled
when the hotwording PCM stream is opened/closed.

A function pointer in struct rt5677_priv is used to avoid module
dependency cycle between snd_soc_rt5677 and snd_soc_rt5677_spi.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677-spi.c | 12 ++++++++++++
 sound/soc/codecs/rt5677.c     |  6 +++++-
 sound/soc/codecs/rt5677.h     |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5677-spi.c b/sound/soc/codecs/rt5677-spi.c
index 62621fe4747c..25d75a803cb5 100644
--- a/sound/soc/codecs/rt5677-spi.c
+++ b/sound/soc/codecs/rt5677-spi.c
@@ -26,6 +26,7 @@
 
 #include <sound/soc.h>
 
+#include "rt5677.h"
 #include "rt5677-spi.h"
 
 #define DRV_NAME "rt5677spi"
@@ -100,6 +101,12 @@ static struct snd_soc_dai_driver rt5677_spi_dai = {
 /* PCM for streaming audio from the DSP buffer */
 static int rt5677_spi_pcm_open(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_component *component =
+			snd_soc_rtdcom_lookup(rtd, "rt5677");
+	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
+
+	rt5677->set_dsp_vad(component, true);
 	snd_soc_set_runtime_hwparams(substream, &rt5677_spi_pcm_hardware);
 	return 0;
 }
@@ -109,10 +116,15 @@ static int rt5677_spi_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component =
 			snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct snd_soc_component *codec_component =
+			snd_soc_rtdcom_lookup(rtd, "rt5677");
+	struct rt5677_priv *rt5677 =
+			snd_soc_component_get_drvdata(codec_component);
 	struct rt5677_dsp *rt5677_dsp =
 			snd_soc_component_get_drvdata(component);
 
 	cancel_delayed_work_sync(&rt5677_dsp->copy_work);
+	rt5677->set_dsp_vad(codec_component, false);
 	return 0;
 }
 
diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 35d4ec1b7dfd..9cdfe7d488fe 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -918,7 +918,9 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 	if (!IS_ENABLED(CONFIG_SND_SOC_RT5677_SPI))
 		return -ENXIO;
 
+	rt5677->dsp_vad_en = on;
 	dev_info(component->dev, "DSP VAD: on=%d, activity=%d\n", on, activity);
+
 	if (on && !activity) {
 		activity = true;
 
@@ -1005,7 +1007,8 @@ static int rt5677_dsp_vad_put(struct snd_kcontrol *kcontrol,
 	rt5677->dsp_vad_en = !!ucontrol->value.integer.value[0];
 
 	if (snd_soc_component_get_bias_level(component) == SND_SOC_BIAS_OFF)
-		rt5677_set_dsp_vad(component, rt5677->dsp_vad_en);
+		rt5677_set_dsp_vad(component,
+				!!ucontrol->value.integer.value[0]);
 
 	return 0;
 }
@@ -5471,6 +5474,7 @@ static int rt5677_i2c_probe(struct i2c_client *i2c)
 		return -ENOMEM;
 
 	rt5677->dev = &i2c->dev;
+	rt5677->set_dsp_vad = rt5677_set_dsp_vad;
 	i2c_set_clientdata(i2c, rt5677);
 
 	if (i2c->dev.of_node) {
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 2bbd618b51ac..ec5be7e01fd1 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1854,6 +1854,8 @@ struct rt5677_priv {
 	struct irq_domain *domain;
 	struct mutex irq_lock;
 	unsigned int irq_en;
+
+	int (*set_dsp_vad)(struct snd_soc_component *component, bool on);
 };
 
 int rt5677_sel_asrc_clk_src(struct snd_soc_component *component,
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 06/15] ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

This link is needed for the RT5677 DSP to do hotwording

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/intel/boards/bdw-rt5677.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
index 4a4d3353e26d..a02622fae035 100644
--- a/sound/soc/intel/boards/bdw-rt5677.c
+++ b/sound/soc/intel/boards/bdw-rt5677.c
@@ -74,6 +74,7 @@ static const struct snd_soc_dapm_route bdw_rt5677_map[] = {
 	/* CODEC BE connections */
 	{"SSP0 CODEC IN", NULL, "AIF1 Capture"},
 	{"AIF1 Playback", NULL, "SSP0 CODEC OUT"},
+	{"DSP Capture", NULL, "DSP Buffer"},
 };
 
 static const struct snd_kcontrol_new bdw_rt5677_controls[] = {
@@ -258,6 +259,16 @@ SND_SOC_DAILINK_DEF(platform,
 SND_SOC_DAILINK_DEF(be,
 	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-aif1")));
 
+/* Wake on voice interface */
+SND_SOC_DAILINK_DEF(fe_dsp,
+	DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")));
+
+SND_SOC_DAILINK_DEF(platform_dsp,
+	DAILINK_COMP_ARRAY(COMP_PLATFORM("spi-RT5677AA:00")));
+
+SND_SOC_DAILINK_DEF(be_dsp,
+	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-dspbuffer")));
+
 static struct snd_soc_dai_link bdw_rt5677_dais[] = {
 	/* Front End DAI links */
 	{
@@ -276,6 +287,13 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
 		SND_SOC_DAILINK_REG(fe, dummy, platform),
 	},
 
+	/* Non-DPCM links */
+	{
+		.name = "Codec DSP",
+		.stream_name = "Wake on Voice",
+		SND_SOC_DAILINK_REG(fe_dsp, be_dsp, platform_dsp),
+	},
+
 	/* Back End DAI links */
 	{
 		/* SSP0 - Codec */
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 07/15] ASoC: rt5677: Enable jack detect while DSP is running

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

Before a hotword is detected, GPIO1 pin is configured as IRQ
output so that jack detect works. When a hotword is detected,
the DSP firmware configures the GPIO1 pin as GPIO1 and
drives a 1. rt5677_irq() is called after a rising edge on
the GPIO1 pin, due to either jack detect event or hotword
event, or both. All possible events are checked and handled
in rt5677_irq() where GPIO1 pin is configured back to IRQ
output if a hotword is detected.

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 9cdfe7d488fe..8f5e4882120c 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -312,6 +312,8 @@ static bool rt5677_volatile_register(struct device *dev, unsigned int reg)
 	case RT5677_IRQ_CTRL1:
 	case RT5677_IRQ_CTRL2:
 	case RT5677_GPIO_ST:
+	case RT5677_GPIO_CTRL1: /* Modified by DSP firmware */
+	case RT5677_GPIO_CTRL2: /* Modified by DSP firmware */
 	case RT5677_DSP_INB1_SRC_CTRL4:
 	case RT5677_DSP_INB2_SRC_CTRL4:
 	case RT5677_DSP_INB3_SRC_CTRL4:
@@ -787,8 +789,11 @@ static unsigned int rt5677_set_vad_source(
 	regmap_update_bits(rt5677->regmap, RT5677_DSP_INB_CTRL1,
 		RT5677_IB01_SRC_MASK, 4 << RT5677_IB01_SRC_SFT);
 
-	/* IRQ Source of VAD Jack Detection = enable */
-	regmap_write(rt5677->regmap, RT5677_IRQ_CTRL2, 0x4000);
+	/* VAD/SAD is not routed to the IRQ output (i.e. MX-BE[14] = 0), but it
+	 * is routed to DSP_IRQ_0, so DSP firmware may use it to sleep and save
+	 * power. See ALC5677 datasheet section 9.17 "GPIO, Interrupt and Jack
+	 * Detection" for more info.
+	 */
 
 	/* Enable Gating Mode with MCLK = enable */
 	regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x1);
@@ -924,15 +929,15 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 	if (on && !activity) {
 		activity = true;
 
-		/* Set GPIO1 as an output pin driving a 0. Firmware will
-		 * raise GPIO1 upon hotword detect.
+		/* Before a hotword is detected, GPIO1 pin is configured as IRQ
+		 * output so that jack detect works. When a hotword is detected,
+		 * the DSP firmware configures the GPIO1 pin as GPIO1 and
+		 * drives a 1. rt5677_irq() is called after a rising edge on
+		 * the GPIO1 pin, due to either jack detect event or hotword
+		 * event, or both. All possible events are checked and handled
+		 * in rt5677_irq() where GPIO1 pin is configured back to IRQ
+		 * output if a hotword is detected.
 		 */
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
-			RT5677_GPIO1_DIR_MASK |	RT5677_GPIO1_OUT_MASK |
-			RT5677_GPIO1_P_MASK, RT5677_GPIO1_DIR_OUT |
-			RT5677_GPIO1_OUT_LO | RT5677_GPIO1_P_NOR);
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL1,
-			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_GPIO1);
 
 		rt5677_set_vad_source(component);
 		rt5677_set_dsp_mode(component, true);
@@ -952,6 +957,8 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 	} else if (!on && activity) {
 		activity = false;
 
+		/* Don't turn off the DSP while handling irqs */
+		mutex_lock(&rt5677->irq_lock);
 		/* Set DSP CPU to Stop */
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
 			RT5677_PWR_DSP_CPU, RT5677_PWR_DSP_CPU);
@@ -959,13 +966,12 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 
 		/* Disable and clear VAD interrupt */
 		regmap_write(rt5677->regmap, RT5677_VAD_CTRL1, 0x2184);
-		regmap_update_bits(rt5677->regmap, RT5677_IRQ_CTRL2,
-			0xF000, 0x0000);
 
 		/* Set GPIO1 pin back to be IRQ output for jack detect */
 		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL1,
 			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_IRQ);
 
+		mutex_unlock(&rt5677->irq_lock);
 	}
 
 	return 0;
@@ -5273,6 +5279,28 @@ static const struct rt5677_irq_desc rt5677_irq_descs[] = {
 	},
 };
 
+bool rt5677_check_hotword(struct rt5677_priv *rt5677)
+{
+	int reg_gpio;
+
+	if (!rt5677->is_dsp_mode)
+		return false;
+
+	if (regmap_read(rt5677->regmap, RT5677_GPIO_CTRL1, &reg_gpio))
+		return false;
+
+	/* Firmware sets GPIO1 pin to be GPIO1 after hotword is detected */
+	if ((reg_gpio & RT5677_GPIO1_PIN_MASK) == RT5677_GPIO1_PIN_IRQ)
+		return false;
+
+	/* Set GPIO1 pin back to be IRQ output for jack detect */
+	regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL1,
+			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_IRQ);
+
+	rt5677_spi_hotword_detected();
+	return true;
+}
+
 static irqreturn_t rt5677_irq(int unused, void *data)
 {
 	struct rt5677_priv *rt5677 = data;
@@ -5281,9 +5309,6 @@ static irqreturn_t rt5677_irq(int unused, void *data)
 
 	mutex_lock(&rt5677->irq_lock);
 
-	if (rt5677->dsp_vad_en)
-		rt5677_spi_hotword_detected();
-
 	/*
 	 * Loop to handle interrupts until the last i2c read shows no pending
 	 * irqs. The interrupt line is shared by multiple interrupt sources.
@@ -5321,7 +5346,13 @@ static irqreturn_t rt5677_irq(int unused, void *data)
 				reg_irq ^= rt5677_irq_descs[i].polarity_mask;
 			}
 		}
-		if (!irq_fired)
+
+		/* Exit the loop only when we know for sure that GPIO1 pin
+		 * was low at some point since irq_lock was acquired. Any event
+		 * after that point creates a rising edge that triggers another
+		 * call to rt5677_irq().
+		 */
+		if (!irq_fired && !rt5677_check_hotword(rt5677))
 			goto exit;
 
 		ret = regmap_write(rt5677->regmap, RT5677_IRQ_CTRL1, reg_irq);
@@ -5332,6 +5363,7 @@ static irqreturn_t rt5677_irq(int unused, void *data)
 		}
 	}
 exit:
+	WARN_ON_ONCE(loop == 20);
 	mutex_unlock(&rt5677->irq_lock);
 	if (irq_fired)
 		return IRQ_HANDLED;
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 08/15] ASoC: rt5677: Use delayed work for DSP firmware load

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

Without this patch, the DSP firmware is loaded/unloaded synchronously
in snd_pcm_ops.open/close context. When a hotword stream is opened,
snd_pcm_ops.open/close is called multiple times. Loading firmware
in the open/close context takes too long and makes audio playback
choppy.

This patch moves the firmware load to a delayed work.
snd_pcm_ops.open/close sets a flag to request DSP VAD enable/disable,
and schedules the delayed work.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 57 ++++++++++++++++++++++-----------------
 sound/soc/codecs/rt5677.h |  3 ++-
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 8f5e4882120c..f01fc9d44774 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -692,10 +692,8 @@ static int rt5677_dsp_mode_i2c_read(
 	return ret;
 }
 
-static void rt5677_set_dsp_mode(struct snd_soc_component *component, bool on)
+static void rt5677_set_dsp_mode(struct rt5677_priv *rt5677, bool on)
 {
-	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
-
 	if (on) {
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
 			RT5677_PWR_DSP, RT5677_PWR_DSP);
@@ -707,11 +705,8 @@ static void rt5677_set_dsp_mode(struct snd_soc_component *component, bool on)
 	}
 }
 
-static unsigned int rt5677_set_vad_source(
-	struct snd_soc_component *component)
+static unsigned int rt5677_set_vad_source(struct rt5677_priv *rt5677)
 {
-	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
-
 	/* Mono ADC Capture Switch = unmute (default) */
 	regmap_update_bits(rt5677->regmap, RT5677_MONO_ADC_DIG_VOL,
 			RT5677_L_MUTE, 0);
@@ -894,20 +889,19 @@ static int rt5677_parse_and_load_dsp(struct rt5677_priv *rt5677, const u8 *buf,
 	return ret;
 }
 
-static int rt5677_load_dsp_from_file(struct snd_soc_component *component)
+static int rt5677_load_dsp_from_file(struct rt5677_priv *rt5677)
 {
-	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 	const struct firmware *fwp;
+	struct device *dev = rt5677->component->dev;
 	int ret = 0;
 
 	/* Load dsp firmware from rt5677_elf_vad file */
-	ret = request_firmware(&fwp, "rt5677_elf_vad", component->dev);
+	ret = request_firmware(&fwp, "rt5677_elf_vad", dev);
 	if (ret) {
-		dev_err(component->dev, "Request rt5677_elf_vad failed %d\n",
-			ret);
+		dev_err(dev, "Request rt5677_elf_vad failed %d\n", ret);
 		return ret;
 	}
-	dev_info(component->dev, "Requested rt5677_elf_vad (%zu)\n", fwp->size);
+	dev_info(dev, "Requested rt5677_elf_vad (%zu)\n", fwp->size);
 
 	ret = rt5677_parse_and_load_dsp(rt5677, fwp->data, fwp->size);
 	release_firmware(fwp);
@@ -917,16 +911,27 @@ static int rt5677_load_dsp_from_file(struct snd_soc_component *component)
 static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
-	static bool activity;
-	int ret;
+	rt5677->dsp_vad_en = on;
 
 	if (!IS_ENABLED(CONFIG_SND_SOC_RT5677_SPI))
 		return -ENXIO;
 
-	rt5677->dsp_vad_en = on;
-	dev_info(component->dev, "DSP VAD: on=%d, activity=%d\n", on, activity);
+	schedule_delayed_work(&rt5677->dsp_work, 0);
+	return 0;
+}
+
+static void rt5677_dsp_work(struct work_struct *work)
+{
+	struct rt5677_priv *rt5677 =
+		container_of(work, struct rt5677_priv, dsp_work.work);
+	static bool activity;
+	bool enable = rt5677->dsp_vad_en;
+
 
-	if (on && !activity) {
+	dev_info(rt5677->component->dev, "DSP VAD: enable=%d, activity=%d\n",
+			enable, activity);
+
+	if (enable && !activity) {
 		activity = true;
 
 		/* Before a hotword is detected, GPIO1 pin is configured as IRQ
@@ -939,8 +944,8 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 		 * output if a hotword is detected.
 		 */
 
-		rt5677_set_vad_source(component);
-		rt5677_set_dsp_mode(component, true);
+		rt5677_set_vad_source(rt5677);
+		rt5677_set_dsp_mode(rt5677, true);
 
 		/* Boot the firmware from IRAM instead of SRAM0. */
 		rt5677_dsp_mode_i2c_write_addr(rt5677, RT5677_DSP_BOOT_VECTOR,
@@ -950,11 +955,11 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 		rt5677_dsp_mode_i2c_write_addr(rt5677, RT5677_DSP_BOOT_VECTOR,
 			0x0009, 0x0003);
 
-		ret = rt5677_load_dsp_from_file(component);
+		rt5677_load_dsp_from_file(rt5677);
 
 		/* Set DSP CPU to Run */
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x1, 0x0);
-	} else if (!on && activity) {
+	} else if (!enable && activity) {
 		activity = false;
 
 		/* Don't turn off the DSP while handling irqs */
@@ -962,7 +967,8 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 		/* Set DSP CPU to Stop */
 		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
 			RT5677_PWR_DSP_CPU, RT5677_PWR_DSP_CPU);
-		rt5677_set_dsp_mode(component, false);
+
+		rt5677_set_dsp_mode(rt5677, false);
 
 		/* Disable and clear VAD interrupt */
 		regmap_write(rt5677->regmap, RT5677_VAD_CTRL1, 0x2184);
@@ -973,8 +979,6 @@ static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 
 		mutex_unlock(&rt5677->irq_lock);
 	}
-
-	return 0;
 }
 
 static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -6525, 75, 0);
@@ -4935,6 +4939,8 @@ static void rt5677_remove(struct snd_soc_component *component)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 
+	cancel_delayed_work_sync(&rt5677->dsp_work);
+
 	regmap_write(rt5677->regmap, RT5677_RESET, 0x10ec);
 	gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
 	gpiod_set_value_cansleep(rt5677->reset_pin, 1);
@@ -5507,6 +5513,7 @@ static int rt5677_i2c_probe(struct i2c_client *i2c)
 
 	rt5677->dev = &i2c->dev;
 	rt5677->set_dsp_vad = rt5677_set_dsp_vad;
+	INIT_DELAYED_WORK(&rt5677->dsp_work, rt5677_dsp_work);
 	i2c_set_clientdata(i2c, rt5677);
 
 	if (i2c->dev.of_node) {
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index ec5be7e01fd1..d18b41da1176 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1846,9 +1846,10 @@ struct rt5677_priv {
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gpio_chip;
 #endif
-	bool dsp_vad_en;
+	bool dsp_vad_en; /* DSP VAD enable/disable request */
 	bool is_dsp_mode;
 	bool is_vref_slow;
+	struct delayed_work dsp_work;
 
 	/* Interrupt handling */
 	struct irq_domain *domain;
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 09/15] ASoC: rt5677: Add DAPM audio path for hotword stream

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

Add a DAPM audio path from "DMIC L1" to "DSP Buffer" so that
when hotwording is enabled, DAPM does not power off the codec
with SND_SOC_BIAS_OFF.

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index f01fc9d44774..3db26cb242d2 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -707,6 +707,15 @@ static void rt5677_set_dsp_mode(struct rt5677_priv *rt5677, bool on)
 
 static unsigned int rt5677_set_vad_source(struct rt5677_priv *rt5677)
 {
+	struct snd_soc_dapm_context *dapm =
+			snd_soc_component_get_dapm(rt5677->component);
+	/* The hotword audio path is from "DMIC L1" to "DSP Buffer".
+	 * "DSP Buffer" is then connected to "DSP Capture" which is the
+	 * rt5677-dsp-cpu-dai with a PCM interface in rt5677-spi.
+	 */
+	snd_soc_dapm_enable_pin(dapm, "DMIC L1");
+	snd_soc_dapm_sync(dapm);
+
 	/* Mono ADC Capture Switch = unmute (default) */
 	regmap_update_bits(rt5677->regmap, RT5677_MONO_ADC_DIG_VOL,
 			RT5677_L_MUTE, 0);
@@ -3209,6 +3218,7 @@ static const struct snd_soc_dapm_widget rt5677_dapm_widgets[] = {
 	SND_SOC_DAPM_AIF_OUT("AIF4TX", "AIF4 Capture", 0, SND_SOC_NOPM, 0, 0),
 	SND_SOC_DAPM_AIF_IN("SLBRX", "SLIMBus Playback", 0, SND_SOC_NOPM, 0, 0),
 	SND_SOC_DAPM_AIF_OUT("SLBTX", "SLIMBus Capture", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("DSPTX", "DSP Buffer", 0, SND_SOC_NOPM, 0, 0),
 
 	/* Sidetone Mux */
 	SND_SOC_DAPM_MUX("Sidetone Mux", SND_SOC_NOPM, 0, 0,
@@ -3743,11 +3753,24 @@ static const struct snd_soc_dapm_route rt5677_dapm_routes[] = {
 	{ "SLBTX", NULL, "SLB ADC3 Mux" },
 	{ "SLBTX", NULL, "SLB ADC4 Mux" },
 
+	{ "DSPTX", NULL, "IB01 Bypass Mux" },
+
 	{ "IB01 Mux", "IF1 DAC 01", "IF1 DAC01" },
 	{ "IB01 Mux", "IF2 DAC 01", "IF2 DAC01" },
 	{ "IB01 Mux", "SLB DAC 01", "SLB DAC01" },
 	{ "IB01 Mux", "STO1 ADC MIX", "Stereo1 ADC MIX" },
-	{ "IB01 Mux", "VAD ADC/DAC1 FS", "DAC1 FS" },
+	/* The IB01 Mux controls the source for InBound0 and InBound1.
+	 * When the mux option "VAD ADC/DAC1 FS" is selected, "VAD ADC" goes to
+	 * InBound0 and "DAC1 FS" goes to InBound1. "VAD ADC" is used for
+	 * hotwording. "DAC1 FS" is not used currently.
+	 *
+	 * Creating a common widget node for "VAD ADC" + "DAC1 FS" and
+	 * connecting the common widget to IB01 Mux causes the issue where
+	 * there is an active path going from system playback -> "DAC1 FS" ->
+	 * IB01 Mux -> DSP Buffer -> hotword stream. This wrong path confuses
+	 * DAPM. Therefore "DAC1 FS" is ignored for now.
+	 */
+	{ "IB01 Mux", "VAD ADC/DAC1 FS", "VAD ADC Mux" },
 
 	{ "IB01 Bypass Mux", "Bypass", "IB01 Mux" },
 	{ "IB01 Bypass Mux", "Pass SRC", "IB01 Mux" },
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 10/15] ASoC: rt5677: Mark reg RT5677_PWR_ANLG2 as volatile

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

The codec dies when RT5677_PWR_ANLG2(MX-64h) is set to 0xACE1
while it's streaming audio over SPI. The DSP firmware turns
on PLL2 (MX-64 bit 8) when SPI streaming starts.  However regmap
does not believe that register can change by itself. When
BST1 (bit 15) is turned on with regmap_update_bits(), it doesn't
read the register first before write, so PLL2 power bit is
cleared by accident.

Marking MX-64h as volatile in regmap solved the issue.

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 3db26cb242d2..f07d10a8b045 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -302,6 +302,7 @@ static bool rt5677_volatile_register(struct device *dev, unsigned int reg)
 	case RT5677_I2C_MASTER_CTRL7:
 	case RT5677_I2C_MASTER_CTRL8:
 	case RT5677_HAP_GENE_CTRL2:
+	case RT5677_PWR_ANLG2: /* Modified by DSP firmware */
 	case RT5677_PWR_DSP_ST:
 	case RT5677_PRIV_DATA:
 	case RT5677_ASRC_22:
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 11/15] ASoC: rt5677: Stop and restart DSP over suspend/resume

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

MCLK1 gets disabled at suspend and re-enabled at resume. Before
MCLK1 is re-enabled, if the DSP is already on (either the DSP was
left on during suspend, or the DSP is turned on early at resume),
i2c register read returns garbage and corrupts the regmap cache.

This patch stops the DSP before suspend and restarts it after
resume with a dalay to ensure MCLK is on while loading firmware.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 24 +++++++++++++++++++++---
 sound/soc/codecs/rt5677.h |  3 ++-
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index f07d10a8b045..098dcbaa4539 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -921,6 +921,7 @@ static int rt5677_load_dsp_from_file(struct rt5677_priv *rt5677)
 static int rt5677_set_dsp_vad(struct snd_soc_component *component, bool on)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
+	rt5677->dsp_vad_en_request = on;
 	rt5677->dsp_vad_en = on;
 
 	if (!IS_ENABLED(CONFIG_SND_SOC_RT5677_SPI))
@@ -1013,7 +1014,7 @@ static int rt5677_dsp_vad_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 
-	ucontrol->value.integer.value[0] = rt5677->dsp_vad_en;
+	ucontrol->value.integer.value[0] = rt5677->dsp_vad_en_request;
 
 	return 0;
 }
@@ -4680,14 +4681,15 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 			enum snd_soc_bias_level level)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
+	enum snd_soc_bias_level prev_bias =
+		snd_soc_component_get_bias_level(component);
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
 		break;
 
 	case SND_SOC_BIAS_PREPARE:
-		if (snd_soc_component_get_bias_level(component) == SND_SOC_BIAS_STANDBY) {
-			rt5677_set_dsp_vad(component, false);
+		if (prev_bias == SND_SOC_BIAS_STANDBY) {
 
 			regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,
 				RT5677_LDO1_SEL_MASK | RT5677_LDO2_SEL_MASK,
@@ -4711,9 +4713,25 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 		break;
 
 	case SND_SOC_BIAS_STANDBY:
+		if (prev_bias == SND_SOC_BIAS_OFF &&
+				rt5677->dsp_vad_en_request) {
+			/* Re-enable the DSP if it was turned off at suspend */
+			rt5677->dsp_vad_en = true;
+			/* The delay is to wait for MCLK */
+			schedule_delayed_work(&rt5677->dsp_work,
+					msecs_to_jiffies(1000));
+		}
 		break;
 
 	case SND_SOC_BIAS_OFF:
+		flush_delayed_work(&rt5677->dsp_work);
+		if (rt5677->is_dsp_mode) {
+			/* Turn off the DSP before suspend */
+			rt5677->dsp_vad_en = false;
+			schedule_delayed_work(&rt5677->dsp_work, 0);
+			flush_delayed_work(&rt5677->dsp_work);
+		}
+
 		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x0);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG1, 0x0000);
 		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1,
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index d18b41da1176..046ed2ee8e31 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1846,7 +1846,8 @@ struct rt5677_priv {
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip gpio_chip;
 #endif
-	bool dsp_vad_en; /* DSP VAD enable/disable request */
+	bool dsp_vad_en_request; /* DSP VAD enable/disable request */
+	bool dsp_vad_en; /* dsp_work parameter */
 	bool is_dsp_mode;
 	bool is_vref_slow;
 	struct delayed_work dsp_work;
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

[Curtis Malainey]

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(-)

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:
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 13/15] ASoC: rt5677: Disable irq at suspend

[Curtis Malainey]

From: Ben Zhang <benzh@chromium.org>

The irq is disabled at suspend to avoid running the threaded irq
handler after the codec has been powered off. At resume, codec irq is
re-enabled and the interrupt status register is checked to see if
headphone has been pluggnd/unplugged while the device is suspended.

There is still a chance that the headphone gets enabled or disabled
after the codec is suspended. disable_irq syncs the threaded irq
handler, but soc-jack's threaded irq handler schedules a delayed
work to poll gpios (for debounce). This is still OK. The codec won't
be powered back on again because all audio paths have been suspended,
and there are no force enabled supply widgets (MICBIAS1 is disabled).
The gpio status read after codec power off could be wrong, so the
gpio values are checked again after resume.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 46 +++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/rt5677.h |  2 ++
 2 files changed, 48 insertions(+)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 098dcbaa4539..a262a3dfbe2b 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4993,6 +4993,11 @@ static int rt5677_suspend(struct snd_soc_component *component)
 {
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 
+	if (rt5677->irq) {
+		cancel_delayed_work_sync(&rt5677->resume_irq_check);
+		disable_irq(rt5677->irq);
+	}
+
 	if (!rt5677->dsp_vad_en) {
 		regcache_cache_only(rt5677->regmap, true);
 		regcache_mark_dirty(rt5677->regmap);
@@ -5021,6 +5026,11 @@ static int rt5677_resume(struct snd_soc_component *component)
 		regcache_sync(rt5677->regmap);
 	}
 
+	if (rt5677->irq) {
+		enable_irq(rt5677->irq);
+		schedule_delayed_work(&rt5677->resume_irq_check, 0);
+	}
+
 	return 0;
 }
 #else
@@ -5419,6 +5429,39 @@ static irqreturn_t rt5677_irq(int unused, void *data)
 		return IRQ_NONE;
 }
 
+static void rt5677_resume_irq_check(struct work_struct *work)
+{
+	int i, virq;
+	struct rt5677_priv *rt5677 =
+		container_of(work, struct rt5677_priv, resume_irq_check.work);
+
+	/* This is needed to check and clear the interrupt status register
+	 * at resume. If the headset is plugged/unplugged when the device is
+	 * fully suspended, there won't be a rising edge at resume to trigger
+	 * the interrupt. Without this, we miss the next unplug/plug event.
+	 */
+	rt5677_irq(0, rt5677);
+
+	/* Call all enabled jack detect irq handlers again. This is needed in
+	 * addition to the above check for a corner case caused by jack gpio
+	 * debounce. After codec irq is disabled at suspend, the delayed work
+	 * scheduled by soc-jack may run and read wrong jack gpio values, since
+	 * the regmap is in cache only mode. At resume, there is no irq because
+	 * rt5677_irq has already ran and cleared the irq status at suspend.
+	 * Without this explicit check, unplug the headset right after suspend
+	 * starts, then after resume the headset is still shown as plugged in.
+	 */
+	mutex_lock(&rt5677->irq_lock);
+	for (i = 0; i < RT5677_IRQ_NUM; i++) {
+		if (rt5677->irq_en & rt5677_irq_descs[i].enable_mask) {
+			virq = irq_find_mapping(rt5677->domain, i);
+			if (virq)
+				handle_nested_irq(virq);
+		}
+	}
+	mutex_unlock(&rt5677->irq_lock);
+}
+
 static void rt5677_irq_bus_lock(struct irq_data *data)
 {
 	struct rt5677_priv *rt5677 = irq_data_get_irq_chip_data(data);
@@ -5494,6 +5537,7 @@ static int rt5677_init_irq(struct i2c_client *i2c)
 	}
 
 	mutex_init(&rt5677->irq_lock);
+	INIT_DELAYED_WORK(&rt5677->resume_irq_check, rt5677_resume_irq_check);
 
 	/*
 	 * Select RC as the debounce clock so that GPIO works even when
@@ -5539,6 +5583,8 @@ static int rt5677_init_irq(struct i2c_client *i2c)
 	if (ret)
 		dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
 
+	rt5677->irq = i2c->irq;
+
 	return ret;
 }
 
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 046ed2ee8e31..f8ada967fdbc 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1856,6 +1856,8 @@ struct rt5677_priv {
 	struct irq_domain *domain;
 	struct mutex irq_lock;
 	unsigned int irq_en;
+	struct delayed_work resume_irq_check;
+	int irq;
 
 	int (*set_dsp_vad)(struct snd_soc_component *component, bool on);
 };
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 14/15] ASoC: rt5677: Allow VAD to be shut on/off at all times

[Curtis Malainey]

Due to limitations of the clocking configuration, we have no way of
scheduling our hibernation before the bdw dsp hibernates. This causes
issues when the system suspends with an open stream. We need userspace
to toggle the kcontrol before we are suspended so that any writes on
suspend are not lost and we don't corrupt the regmap.

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index a262a3dfbe2b..29233ec8906e 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -1023,13 +1023,8 @@ static int rt5677_dsp_vad_put(struct snd_kcontrol *kcontrol,
 		struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
-
-	rt5677->dsp_vad_en = !!ucontrol->value.integer.value[0];
 
-	if (snd_soc_component_get_bias_level(component) == SND_SOC_BIAS_OFF)
-		rt5677_set_dsp_vad(component,
-				!!ucontrol->value.integer.value[0]);
+	rt5677_set_dsp_vad(component, !!ucontrol->value.integer.value[0]);
 
 	return 0;
 }
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [RFC 15/15] ASoC: rt5677: Turn on MCLK1 for DSP via DAPM

[Curtis Malainey]

The RT5677 DSP needs the I2S MCLK1 to run its DSP. Add a dapm route to
SSP0 CODEC IN so the clock is turned on automatically when the DSP is
turned on.

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
---
 sound/soc/codecs/rt5677.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 29233ec8906e..2827a6d00ead 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4305,6 +4305,8 @@ static const struct snd_soc_dapm_route rt5677_dapm_routes[] = {
 	{ "PDM1R", NULL, "PDM1 R Mux" },
 	{ "PDM2L", NULL, "PDM2 L Mux" },
 	{ "PDM2R", NULL, "PDM2 R Mux" },
+	{ "DSP Buffer", NULL, "SSP0 CODEC IN" },
+	{ "SSP0 CODEC IN", NULL, "DSPTX" },
 };
 
 static const struct snd_soc_dapm_route rt5677_dmic2_clk_1[] = {
-- 
2.23.0.187.g17f5b7556c-goog

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[Pierre-Louis Bossart]

On 9/6/19 2:46 PM, Curtis Malainey wrote:
> This patch series adds the hotwording implementation used in the
> Pixelbook on the RT5677 driver.
> 
> Known Issues:
> There is a known issue where the system will fail to detect a hotword if
> suspended while the stream is open. This is due to the fact that the
> haswell-dsp suspends its I2S MCLK before the RT5677 suspends which
> causes the writes and reads to become corrupted as a result. Any
> recommendations to correct this behaviour would be appreciated.

I don't get what 'suspend' and 'stream' refer to. is this pm_runtime, 
s2idle, system capture, SPI capture?

Can you elaborate on the sequence?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[Curtis Malainey]

Curtis Malainey | Software Engineer | cujomalainey@google.com | 650-898-3849


On Fri, Sep 6, 2019 at 1:41 PM Pierre-Louis Bossart <
pierre-louis.bossart@linux.intel.com> wrote:
>
> On 9/6/19 2:46 PM, Curtis Malainey wrote:
> > This patch series adds the hotwording implementation used in the
> > Pixelbook on the RT5677 driver.
> >
> > Known Issues:
> > There is a known issue where the system will fail to detect a hotword if
> > suspended while the stream is open. This is due to the fact that the
> > haswell-dsp suspends its I2S MCLK before the RT5677 suspends which
> > causes the writes and reads to become corrupted as a result. Any
> > recommendations to correct this behaviour would be appreciated.
>
> I don't get what 'suspend' and 'stream' refer to. is this pm_runtime,
> s2idle, system capture, SPI capture?
>
> Can you elaborate on the sequence?
Definitely can,

   1. open hotwording pcm with arecord in non-blocking mode
      - Codec won't send any data over SPI until the hotword is detected
   2. put system into S3 (see order of callbacks as follows)
      1. HSW DSP suspended which suspends stops I2S MCLK
      2. RT5677 suspended, all pm writes are lost due to the fact that the
      codec is still in DSP mode but has no clock
   3. System resumes and fails to restore the RT5677 due to the fact that
   the regmap is now out of sync

The rt5677 needs to suspend before the haswell dsp but I am not sure how to
schedule that appropriately. The reason this worked in Samus is because it
launched with a 3.14 kernel which did not
have 0d2135ecadb0b2eec5338a7587ba29724ddf612b ("ASoC: Intel: Work around to
fix HW D3 potential crash issue") which powers down the MCLK when the
haswell DSP is not in use.

Hope that clears things up.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[Pierre-Louis Bossart]

On 9/6/19 4:09 PM, Curtis Malainey wrote:
> 
> 
> Curtis Malainey | Software Engineer | cujomalainey@google.com 
> <mailto:cujomalainey@google.com> | 650-898-3849
> 
> 
> On Fri, Sep 6, 2019 at 1:41 PM Pierre-Louis Bossart 
> <pierre-louis.bossart@linux.intel.com 
> <mailto:pierre-louis.bossart@linux.intel.com>> wrote:
>  >
>  > On 9/6/19 2:46 PM, Curtis Malainey wrote:
>  > > This patch series adds the hotwording implementation used in the
>  > > Pixelbook on the RT5677 driver.
>  > >
>  > > Known Issues:
>  > > There is a known issue where the system will fail to detect a 
> hotword if
>  > > suspended while the stream is open. This is due to the fact that the
>  > > haswell-dsp suspends its I2S MCLK before the RT5677 suspends which
>  > > causes the writes and reads to become corrupted as a result. Any
>  > > recommendations to correct this behaviour would be appreciated.
>  >
>  > I don't get what 'suspend' and 'stream' refer to. is this pm_runtime,
>  > s2idle, system capture, SPI capture?
>  >
>  > Can you elaborate on the sequence?
> Definitely can,
> 
>  1. open hotwording pcm with arecord in non-blocking mode
>       * Codec won't send any data over SPI until the hotword is detected
>  2. put system into S3 (see order of callbacks as follows)

Before we start digging into dependencies below, is it really possible 
to enter S3 with the hotwording open? I vaguely remember being told that 
such cases would be trapped by the Chrome userspace and the PCM would be 
closed. I don't think anyone on the SOF team testing this case for newer 
platform, so that case on an old platform makes me nervous.

>      1. HSW DSP suspended which suspends stops I2S MCLK
>      2. RT5677 suspended, all pm writes are lost due to the fact that
>         the codec is still in DSP mode but has no clock

there's no real dependency or parent-child relationship between the two 
drivers, is there? so I am wondering if this order is intentional or 
just accidental.
The only thing I can think of is that there are multiple steps during 
the system suspend and maybe we can play with .suspend_late instead of 
.suspend?

>  3. System resumes and fails to restore the RT5677 due to the fact that
>     the regmap is now out of sync
> 
> The rt5677 needs to suspend before the haswell dsp but I am not sure how 
> to schedule that appropriately. The reason this worked in Samus is 
> because it launched with a 3.14 kernel which did not 
> have 0d2135ecadb0b2eec5338a7587ba29724ddf612b ("ASoC: Intel: Work around 
> to fix HW D3 potential crash issue") which powers down the MCLK when the 
> haswell DSP is not in use.
> 
> Hope that clears things up.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 06/15] ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device

[Kuninori Morimoto]

Hi Curtis

> From: Ben Zhang <benzh@chromium.org>
> 
> This link is needed for the RT5677 DSP to do hotwording
> 
> Signed-off-by: Ben Zhang <benzh@chromium.org>
> Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
> ---
(snip)
> +/* Wake on voice interface */
> +SND_SOC_DAILINK_DEF(fe_dsp,
> +	DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")));
> +
> +SND_SOC_DAILINK_DEF(platform_dsp,
> +	DAILINK_COMP_ARRAY(COMP_PLATFORM("spi-RT5677AA:00")));
> +
> +SND_SOC_DAILINK_DEF(be_dsp,
> +	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-dspbuffer")));
> +
(snip)
> +	/* Non-DPCM links */
> +	{
> +		.name = "Codec DSP",
> +		.stream_name = "Wake on Voice",
> +		SND_SOC_DAILINK_REG(fe_dsp, be_dsp, platform_dsp),
> +	},

If you don't need to re-use CPU/Codec/Platform definition,
I guess you can use more short version?

SND_SOC_DAILINK_DEFS(dsp,
	DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")),
	DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-dspbuffer")),
	DAILINK_COMP_ARRAY(COMP_PLATFORM("spi-RT5677AA:00")));

 struct snd_soc_dai_link link = {
	...
	SND_SOC_DAILINK_REG(dsp),
 };

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 02/15] ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 441 bytes --]

On Fri, Sep 06, 2019 at 12:46:24PM -0700, Curtis Malainey wrote:
> From: Ben Zhang <benzh@chromium.org>
> 
> Instead of clearing RT5677_PWR_ANLG2 (MX-64h) to 0 at SND_SOC_BIAS_OFF,
> we only clear the RT5677_PWR_CORE bit which is set at SND_SOC_BIAS_PREPARE.
> MICBIAS control bits are left unchanged.

This is a bug fix so should have been at the start of the series
rather than depending on the naming changes you had as patch 1.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: rt5677: Remove magic number register writes" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5677: Remove magic number register writes

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 33b773dc9288eb15d3216628d1d2381103d854a9 Mon Sep 17 00:00:00 2001
From: Curtis Malainey <cujomalainey@chromium.org>
Date: Fri, 6 Sep 2019 12:46:23 -0700
Subject: [PATCH] ASoC: rt5677: Remove magic number register writes

In order to simplify understanding what register values are being
written to the codec for debugging more advanced features (such as
hotwording) it is best to remove magic numbers

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
Link: https://lore.kernel.org/r/20190906194636.217881-2-cujomalainey@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5677.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index c779dc3474f9..5b6ca3ced13b 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -691,10 +691,12 @@ static void rt5677_set_dsp_mode(struct snd_soc_component *component, bool on)
 	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
 
 	if (on) {
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x2);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
+			RT5677_PWR_DSP, RT5677_PWR_DSP);
 		rt5677->is_dsp_mode = true;
 	} else {
-		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1, 0x2, 0x0);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_DSP1,
+			RT5677_PWR_DSP, 0x0);
 		rt5677->is_dsp_mode = false;
 	}
 }
@@ -4466,7 +4468,8 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 
 			regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG1,
 				RT5677_LDO1_SEL_MASK | RT5677_LDO2_SEL_MASK,
-				0x0055);
+				5 << RT5677_LDO1_SEL_SFT |
+				5 << RT5677_LDO2_SEL_SFT);
 			regmap_update_bits(rt5677->regmap,
 				RT5677_PR_BASE + RT5677_BIAS_CUR4,
 				0x0f00, 0x0f00);
@@ -4491,7 +4494,9 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x0);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG1, 0x0000);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG2, 0x0000);
-		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1, 0x0022);
+		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1,
+			2 << RT5677_LDO1_SEL_SFT |
+			2 << RT5677_LDO2_SEL_SFT);
 		regmap_write(rt5677->regmap, RT5677_PWR_ANLG2, 0x0000);
 		regmap_update_bits(rt5677->regmap,
 			RT5677_PR_BASE + RT5677_BIAS_CUR4, 0x0f00, 0x0000);
@@ -4719,7 +4724,8 @@ static int rt5677_probe(struct snd_soc_component *component)
 
 	regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC,
 			~RT5677_IRQ_DEBOUNCE_SEL_MASK, 0x0020);
-	regmap_write(rt5677->regmap, RT5677_PWR_DSP2, 0x0c00);
+	regmap_write(rt5677->regmap, RT5677_PWR_DSP2,
+			RT5677_PWR_SLIM_ISO | RT5677_PWR_CORE_ISO);
 
 	for (i = 0; i < RT5677_GPIO_NUM; i++)
 		rt5677_gpio_config(rt5677, i, rt5677->pdata.gpio_config[i]);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From dfe58f2011595e7512bde9dffbd0abfc3a736ab7 Mon Sep 17 00:00:00 2001
From: Ben Zhang <benzh@chromium.org>
Date: Fri, 6 Sep 2019 12:46:24 -0700
Subject: [PATCH] ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF

Instead of clearing RT5677_PWR_ANLG2 (MX-64h) to 0 at SND_SOC_BIAS_OFF,
we only clear the RT5677_PWR_CORE bit which is set at SND_SOC_BIAS_PREPARE.
MICBIAS control bits are left unchanged.

This fixed the bug where if MICBIAS1 widget is forced on, MICBIAS
control bits will be cleared at suspend and never turned back on again,
since DAPM thinks the widget is always on.

Signed-off-by: Ben Zhang <benzh@chromium.org>
Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
Link: https://lore.kernel.org/r/20190906194636.217881-3-cujomalainey@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5677.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 5b6ca3ced13b..315a3d39bc09 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4493,11 +4493,11 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 	case SND_SOC_BIAS_OFF:
 		regmap_update_bits(rt5677->regmap, RT5677_DIG_MISC, 0x1, 0x0);
 		regmap_write(rt5677->regmap, RT5677_PWR_DIG1, 0x0000);
-		regmap_write(rt5677->regmap, RT5677_PWR_DIG2, 0x0000);
 		regmap_write(rt5677->regmap, RT5677_PWR_ANLG1,
 			2 << RT5677_LDO1_SEL_SFT |
 			2 << RT5677_LDO2_SEL_SFT);
-		regmap_write(rt5677->regmap, RT5677_PWR_ANLG2, 0x0000);
+		regmap_update_bits(rt5677->regmap, RT5677_PWR_ANLG2,
+			RT5677_PWR_CORE, 0);
 		regmap_update_bits(rt5677->regmap,
 			RT5677_PR_BASE + RT5677_BIAS_CUR4, 0x0f00, 0x0000);
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 02/15] ASoC: rt5677: keep analog power register at SND_SOC_BIAS_OFF

[Curtis Malainey]

On Mon, Sep 9, 2019 at 2:54 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 06, 2019 at 12:46:24PM -0700, Curtis Malainey wrote:
> > From: Ben Zhang <benzh@chromium.org>
> >
> > Instead of clearing RT5677_PWR_ANLG2 (MX-64h) to 0 at SND_SOC_BIAS_OFF,
> > we only clear the RT5677_PWR_CORE bit which is set at SND_SOC_BIAS_PREPARE.
> > MICBIAS control bits are left unchanged.
>
> This is a bug fix so should have been at the start of the series
> rather than depending on the naming changes you had as patch 1.

Got it, will send bug fixes to the bottom of future series.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 00/15] Add Samus Hotwording for RT5677

[Curtis Malainey]

On Fri, Sep 6, 2019 at 3:13 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> On 9/6/19 4:09 PM, Curtis Malainey wrote:
> >
> >
> > Curtis Malainey | Software Engineer | cujomalainey@google.com
> > <mailto:cujomalainey@google.com> | 650-898-3849
> >
> >
> > On Fri, Sep 6, 2019 at 1:41 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com
> > <mailto:pierre-louis.bossart@linux.intel.com>> wrote:
> >  >
> >  > On 9/6/19 2:46 PM, Curtis Malainey wrote:
> >  > > This patch series adds the hotwording implementation used in the
> >  > > Pixelbook on the RT5677 driver.
> >  > >
> >  > > Known Issues:
> >  > > There is a known issue where the system will fail to detect a
> > hotword if
> >  > > suspended while the stream is open. This is due to the fact that the
> >  > > haswell-dsp suspends its I2S MCLK before the RT5677 suspends which
> >  > > causes the writes and reads to become corrupted as a result. Any
> >  > > recommendations to correct this behaviour would be appreciated.
> >  >
> >  > I don't get what 'suspend' and 'stream' refer to. is this pm_runtime,
> >  > s2idle, system capture, SPI capture?
> >  >
> >  > Can you elaborate on the sequence?
> > Definitely can,
> >
> >  1. open hotwording pcm with arecord in non-blocking mode
> >       * Codec won't send any data over SPI until the hotword is detected
> >  2. put system into S3 (see order of callbacks as follows)
>
> Before we start digging into dependencies below, is it really possible
> to enter S3 with the hotwording open? I vaguely remember being told that
> such cases would be trapped by the Chrome userspace and the PCM would be
> closed. I don't think anyone on the SOF team testing this case for newer
> platform, so that case on an old platform makes me nervous.
>
I vaguely recall that as well now that you mention it. I will follow
up internally, if that is true then this will be a non-issue from our
point of view.
> >      1. HSW DSP suspended which suspends stops I2S MCLK
> >      2. RT5677 suspended, all pm writes are lost due to the fact that
> >         the codec is still in DSP mode but has no clock
>
> there's no real dependency or parent-child relationship between the two
> drivers, is there? so I am wondering if this order is intentional or
> just accidental.
> The only thing I can think of is that there are multiple steps during
> the system suspend and maybe we can play with .suspend_late instead of
> .suspend?
Not that I am aware of, when used as a standard codec there is no
clock dependency. I will try and see if I can set the pm accordingly.
>
> >  3. System resumes and fails to restore the RT5677 due to the fact that
> >     the regmap is now out of sync
> >
> > The rt5677 needs to suspend before the haswell dsp but I am not sure how
> > to schedule that appropriately. The reason this worked in Samus is
> > because it launched with a 3.14 kernel which did not
> > have 0d2135ecadb0b2eec5338a7587ba29724ddf612b ("ASoC: Intel: Work around
> > to fix HW D3 potential crash issue") which powers down the MCLK when the
> > haswell DSP is not in use.
> >
> > Hope that clears things up.
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 06/15] ASoC: bdw-rt5677: Add a DAI link for rt5677 SPI PCM device

[Curtis Malainey]

On Sun, Sep 8, 2019 at 5:18 PM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi Curtis
>
> > From: Ben Zhang <benzh@chromium.org>
> >
> > This link is needed for the RT5677 DSP to do hotwording
> >
> > Signed-off-by: Ben Zhang <benzh@chromium.org>
> > Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
> > ---
> (snip)
> > +/* Wake on voice interface */
> > +SND_SOC_DAILINK_DEF(fe_dsp,
> > +     DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")));
> > +
> > +SND_SOC_DAILINK_DEF(platform_dsp,
> > +     DAILINK_COMP_ARRAY(COMP_PLATFORM("spi-RT5677AA:00")));
> > +
> > +SND_SOC_DAILINK_DEF(be_dsp,
> > +     DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-dspbuffer")));
> > +
> (snip)
> > +     /* Non-DPCM links */
> > +     {
> > +             .name = "Codec DSP",
> > +             .stream_name = "Wake on Voice",
> > +             SND_SOC_DAILINK_REG(fe_dsp, be_dsp, platform_dsp),
> > +     },
>
> If you don't need to re-use CPU/Codec/Platform definition,
> I guess you can use more short version?
>
> SND_SOC_DAILINK_DEFS(dsp,
>         DAILINK_COMP_ARRAY(COMP_CPU("spi-RT5677AA:00")),
>         DAILINK_COMP_ARRAY(COMP_CODEC("i2c-RT5677CE:00", "rt5677-dspbuffer")),
>         DAILINK_COMP_ARRAY(COMP_PLATFORM("spi-RT5677AA:00")));
>
>  struct snd_soc_dai_link link = {
>         ...
>         SND_SOC_DAILINK_REG(dsp),
>  };
>
Updated, thanks!
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 04/15] ASoC: rt5677: Load firmware via SPI

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 660 bytes --]

On Fri, Sep 06, 2019 at 12:46:26PM -0700, Curtis Malainey wrote:

> When 'DSP VAD Switch' is turned on, rt5677_set_vad_source()
> enables the following digital path:

> DMIC L1 ->
> Mono DMIC L Mux ->
> Mono ADC2 L Mux ->
> Mono ADC MIXL ->
> VAD ADC Mux ->
> IB01 Mux

> Then we switch to DSP mode, load firmware, and let DSP run.
> When a hotword is detected, an interrupt is fired and
> rt5677_irq() is called. When 'DSP VAD Switch' is turned off,
> the codec is set back to normal mode.

Usually we would configure all this routing in userspace.  Why
are we hard coding the use case here?  What if for example the
user wants to use a different microphone?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 262 bytes --]

On Fri, Sep 06, 2019 at 12:46:27PM -0700, Curtis Malainey wrote:
> From: Ben Zhang <benzh@chromium.org>
> 
> The kcontrol 'DSP VAD Switch' is automatically enabled/disabled
> when the hotwording PCM stream is opened/closed.

So why do we have the switch?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 08/15] ASoC: rt5677: Use delayed work for DSP firmware load

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 407 bytes --]

On Fri, Sep 06, 2019 at 12:46:30PM -0700, Curtis Malainey wrote:
> From: Ben Zhang <benzh@chromium.org>
> 
> Without this patch, the DSP firmware is loaded/unloaded synchronously
> in snd_pcm_ops.open/close context. When a hotword stream is opened,
> snd_pcm_ops.open/close is called multiple times. Loading firmware

It would be better to just do this in the patch that adds the
firmware loading.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 12/15] ASoC: rt5677: Transfer one period at a time over SPI

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 449 bytes --]

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?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 12/15] ASoC: rt5677: Transfer one period at a time over SPI

[Curtis Malainey]

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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Curtis Malainey]

On Wed, Sep 11, 2019 at 3:25 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 06, 2019 at 12:46:27PM -0700, Curtis Malainey wrote:
> > From: Ben Zhang <benzh@chromium.org>
> >
> > The kcontrol 'DSP VAD Switch' is automatically enabled/disabled
> > when the hotwording PCM stream is opened/closed.
>
> So why do we have the switch?

The source of the switch is commit af48f1d08a547 ("ASoC: rt5677:
Support DSP function for VAD application") and does not explain the
original intent of the switch. I believe the original intent of this
commit is to keep the switch in sync with the VAD state. I do not
believe we use the switch ourselves.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 593 bytes --]

On Wed, Sep 11, 2019 at 01:22:20PM -0700, Curtis Malainey wrote:

> The source of the switch is commit af48f1d08a547 ("ASoC: rt5677:
> Support DSP function for VAD application") and does not explain the
> original intent of the switch. I believe the original intent of this
> commit is to keep the switch in sync with the VAD state. I do not
> believe we use the switch ourselves.

Well, I would assume that the control is used to allow users to
enable and disable the VAD functionality at runtime.  As with the
routing if it's been exposed to users we should continue to let
them control it.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Curtis Malainey]

On Thu, Sep 12, 2019 at 2:26 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Sep 11, 2019 at 01:22:20PM -0700, Curtis Malainey wrote:
>
> > The source of the switch is commit af48f1d08a547 ("ASoC: rt5677:
> > Support DSP function for VAD application") and does not explain the
> > original intent of the switch. I believe the original intent of this
> > commit is to keep the switch in sync with the VAD state. I do not
> > believe we use the switch ourselves.
>
> Well, I would assume that the control is used to allow users to
> enable and disable the VAD functionality at runtime.  As with the
> routing if it's been exposed to users we should continue to let
> them control it.

I will work to add variable inputs, in the samus use case it doesn't
make much sense to use the hotword without the pcm open since that
audio needs to be captured. How would userspace received the detection
without the pcm open?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [RFC 05/15] ASoC: rt5677: Auto enable/disable DSP for hotwording

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 470 bytes --]

On Mon, Sep 16, 2019 at 02:29:32PM -0700, Curtis Malainey wrote:

> I will work to add variable inputs, in the samus use case it doesn't
> make much sense to use the hotword without the pcm open since that
> audio needs to be captured. How would userspace received the detection
> without the pcm open?

They broke it, they get to keep all the pieces.  Equally, if they have a
functional use case that differs to your hard coded one they aren't
prevented from using it.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel