[RFC PATCH 0/4] Add support for detection

[RFC PATCH 0/4] Add support for detection

[Amadeusz Sławiński]

There are some scenarios when using DSP where one may want to have
partially active stream and fully enable it after some event occurs.

Following patchset adds new "detect" state to ALSA state machine to
allow waiting for condition to occur before fully starting a stream. In
further patches the state is propagated through ASoC components to allow
them to handling the state as necessary.

Main goal of this patchset is to allow handling scenarios like keyphrase
detection - where DSP analyses incoming signal and wakes userspace to
consume stream only when keyphrase is detected.

I'm sending this as RFC so we can discuss if this is the way to go or if
there is perhaps another preferred way of adding such interface.
Userspace part of implementation is available at
https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect

Amadeusz Sławiński (4):
  ALSA: core: Add support for running detect on capture stream
  ALSA: core: Allow polling for detection
  ASoC: pcm: Add support for running detect on capture stream
  ASoC: Propagate DETECT trigger

 include/sound/pcm.h         |  6 +++
 include/sound/soc-dpcm.h    |  2 +
 include/uapi/sound/asound.h |  4 +-
 sound/core/pcm_native.c     | 89 ++++++++++++++++++++++++++++++++++++-
 sound/soc/soc-component.c   |  2 +
 sound/soc/soc-dai.c         |  2 +
 sound/soc/soc-link.c        |  2 +
 sound/soc/soc-pcm.c         | 56 +++++++++++++++++++++++
 8 files changed, 160 insertions(+), 3 deletions(-)

-- 
2.34.1

[RFC PATCH 1/4] ALSA: core: Add support for running detect on capture stream

[Amadeusz Sławiński]

In order to save power when running detection on capture streams, there
is need for additional state which allows stream to be in "half" running
state. Goal is to allow for DSP to perform detection and then tell
userspace that it detected something and to trigger transfer to running
mode where capture will be performed for further analysis.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 include/sound/pcm.h         |  2 +
 include/uapi/sound/asound.h |  4 +-
 sound/core/pcm_native.c     | 77 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 0bf7d25434d7f..f06ef7c718733 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -102,6 +102,8 @@ struct snd_pcm_ops {
 #define SNDRV_PCM_TRIGGER_SUSPEND	5
 #define SNDRV_PCM_TRIGGER_RESUME	6
 #define SNDRV_PCM_TRIGGER_DRAIN		7
+#define SNDRV_PCM_TRIGGER_DETECT_STOP	8
+#define SNDRV_PCM_TRIGGER_DETECT_START	9
 
 #define SNDRV_PCM_POS_XRUN		((snd_pcm_uframes_t)-1)
 
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 4cd513215bcd8..c9f53d79638be 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -313,7 +313,8 @@ typedef int __bitwise snd_pcm_state_t;
 #define	SNDRV_PCM_STATE_PAUSED		((__force snd_pcm_state_t) 6) /* stream is paused */
 #define	SNDRV_PCM_STATE_SUSPENDED	((__force snd_pcm_state_t) 7) /* hardware is suspended */
 #define	SNDRV_PCM_STATE_DISCONNECTED	((__force snd_pcm_state_t) 8) /* hardware is disconnected */
-#define	SNDRV_PCM_STATE_LAST		SNDRV_PCM_STATE_DISCONNECTED
+#define	SNDRV_PCM_STATE_DETECTING	((__force snd_pcm_state_t) 9) /* stream is detecting (e.g. voice detection) */
+#define	SNDRV_PCM_STATE_LAST		SNDRV_PCM_STATE_DETECTING
 
 enum {
 	SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
@@ -703,6 +704,7 @@ enum {
 #define SNDRV_PCM_IOCTL_READI_FRAMES	_IOR('A', 0x51, struct snd_xferi)
 #define SNDRV_PCM_IOCTL_WRITEN_FRAMES	_IOW('A', 0x52, struct snd_xfern)
 #define SNDRV_PCM_IOCTL_READN_FRAMES	_IOR('A', 0x53, struct snd_xfern)
+#define SNDRV_PCM_IOCTL_DETECT		_IO('A', 0x54)
 #define SNDRV_PCM_IOCTL_LINK		_IOW('A', 0x60, int)
 #define SNDRV_PCM_IOCTL_UNLINK		_IO('A', 0x61)
 
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index b465fb6e1f5f0..4fe1d2f41149c 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1410,6 +1410,72 @@ static int snd_pcm_action_nonatomic(const struct action_ops *ops,
 	return res;
 }
 
+/*
+ * detection callbacks
+ */
+
+static int snd_pcm_pre_detect(struct snd_pcm_substream *substream,
+			      snd_pcm_state_t state)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	if (runtime->status->state != SNDRV_PCM_STATE_PREPARED)
+		return -EBADFD;
+	/* Detection only makes sense on capture stream */
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		return -EINVAL;
+	runtime->trigger_tstamp_latched = false;
+	runtime->trigger_master = substream;
+	return 0;
+}
+
+static int snd_pcm_do_detect(struct snd_pcm_substream *substream,
+			     snd_pcm_state_t state)
+{
+	if (substream->runtime->trigger_master != substream)
+		return 0;
+	return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_DETECT_START);
+}
+
+static void snd_pcm_undo_detect(struct snd_pcm_substream *substream,
+				snd_pcm_state_t state)
+{
+	if (substream->runtime->trigger_master == substream)
+		substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_DETECT_STOP);
+}
+
+static void snd_pcm_post_detect(struct snd_pcm_substream *substream,
+				snd_pcm_state_t state)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	snd_pcm_trigger_tstamp(substream);
+	runtime->hw_ptr_jiffies = jiffies;
+	runtime->hw_ptr_buffer_jiffies = (runtime->buffer_size * HZ) / runtime->rate;
+	runtime->status->state = state;
+	snd_pcm_timer_notify(substream, SNDRV_TIMER_EVENT_MSTART);
+}
+
+static const struct action_ops snd_pcm_action_detect = {
+	.pre_action = snd_pcm_pre_detect,
+	.do_action = snd_pcm_do_detect,
+	.undo_action = snd_pcm_undo_detect,
+	.post_action = snd_pcm_post_detect,
+};
+
+/**
+ * snd_pcm_detect - start all detection streams
+ * @substream: the PCM substream instance
+ *
+ * Return: Zero if successful, or a negative error code.
+ * The stream lock must be acquired before calling this function.
+ */
+int snd_pcm_detect(struct snd_pcm_substream *substream)
+{
+	return snd_pcm_action_lock_irq(&snd_pcm_action_detect, substream,
+				       SNDRV_PCM_STATE_DETECTING);
+}
+
 /*
  * start callbacks
  */
@@ -1417,7 +1483,8 @@ static int snd_pcm_pre_start(struct snd_pcm_substream *substream,
 			     snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	if (runtime->state != SNDRV_PCM_STATE_PREPARED)
+	if (runtime->state != SNDRV_PCM_STATE_PREPARED &&
+	    runtime->state != SNDRV_PCM_STATE_DETECTING)
 		return -EBADFD;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    !snd_pcm_playback_data(substream))
@@ -2919,10 +2986,14 @@ static int snd_pcm_release(struct inode *inode, struct file *file)
 static int do_pcm_hwsync(struct snd_pcm_substream *substream)
 {
 	switch (substream->runtime->state) {
+	case SNDRV_PCM_STATE_DETECTING:
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			return -EBADFD;
+		return snd_pcm_update_hw_ptr(substream);
 	case SNDRV_PCM_STATE_DRAINING:
 		if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
 			return -EBADFD;
-		fallthrough;
+		return snd_pcm_update_hw_ptr(substream);
 	case SNDRV_PCM_STATE_RUNNING:
 		return snd_pcm_update_hw_ptr(substream);
 	case SNDRV_PCM_STATE_PREPARED:
@@ -3342,6 +3413,8 @@ static int snd_pcm_common_ioctl(struct file *file,
 		return snd_pcm_prepare(substream, file);
 	case SNDRV_PCM_IOCTL_RESET:
 		return snd_pcm_reset(substream);
+	case SNDRV_PCM_IOCTL_DETECT:
+		return snd_pcm_detect(substream);
 	case SNDRV_PCM_IOCTL_START:
 		return snd_pcm_start_lock_irq(substream);
 	case SNDRV_PCM_IOCTL_LINK:
-- 
2.34.1

[RFC PATCH 2/4] ALSA: core: Allow polling for detection

[Amadeusz Sławiński]

In order for userspace to wait for detection event to happen, there
needs to be possibility to wait in some way. Add API allowing to poll
for event when stream is in detection state.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 include/sound/pcm.h     |  4 ++++
 sound/core/pcm_native.c | 12 ++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index f06ef7c718733..28df5bc5c131c 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -448,6 +448,9 @@ struct snd_pcm_runtime {
 	struct snd_pcm_audio_tstamp_report audio_tstamp_report;
 	struct timespec64 driver_tstamp;
 
+	/* -- detection -- */
+	unsigned int detected:1;	/* detection was triggered */
+
 #if IS_ENABLED(CONFIG_SND_PCM_OSS)
 	/* -- OSS things -- */
 	struct snd_pcm_oss_runtime oss;
@@ -592,6 +595,7 @@ int snd_pcm_start(struct snd_pcm_substream *substream);
 int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
 int snd_pcm_drain_done(struct snd_pcm_substream *substream);
 int snd_pcm_stop_xrun(struct snd_pcm_substream *substream);
+void snd_pcm_detected(struct snd_pcm_substream *substream);
 #ifdef CONFIG_PM
 int snd_pcm_suspend_all(struct snd_pcm *pcm);
 #else
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 4fe1d2f41149c..c024eb0ed74ff 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3662,6 +3662,14 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from)
 	return result;
 }
 
+void snd_pcm_detected(struct snd_pcm_substream *substream)
+{
+	substream->runtime->detected = 1;
+
+	wake_up(&substream->runtime->sleep);
+}
+EXPORT_SYMBOL_GPL(snd_pcm_detected);
+
 static __poll_t snd_pcm_poll(struct file *file, poll_table *wait)
 {
 	struct snd_pcm_file *pcm_file;
@@ -3690,6 +3698,10 @@ static __poll_t snd_pcm_poll(struct file *file, poll_table *wait)
 	guard(pcm_stream_lock_irq)(substream);
 	avail = snd_pcm_avail(substream);
 	switch (runtime->state) {
+	case SNDRV_PCM_STATE_DETECTING:
+		if (runtime->detected)
+			mask = ok;
+		break;
 	case SNDRV_PCM_STATE_RUNNING:
 	case SNDRV_PCM_STATE_PREPARED:
 	case SNDRV_PCM_STATE_PAUSED:
-- 
2.34.1

[RFC PATCH 3/4] ASoC: pcm: Add support for running detect on capture stream

[Amadeusz Sławiński]

In order to be able to run detection on capture streams, DPCM state
machine needs to be expanded to recognize "detect" state.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 include/sound/soc-dpcm.h |  2 ++
 sound/core/pcm_native.c  |  2 +-
 sound/soc/soc-pcm.c      | 56 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h
index c6fb350b4b062..7062f77528edd 100644
--- a/include/sound/soc-dpcm.h
+++ b/include/sound/soc-dpcm.h
@@ -40,6 +40,7 @@ enum snd_soc_dpcm_state {
 	SND_SOC_DPCM_STATE_OPEN,
 	SND_SOC_DPCM_STATE_HW_PARAMS,
 	SND_SOC_DPCM_STATE_PREPARE,
+	SND_SOC_DPCM_STATE_DETECT,
 	SND_SOC_DPCM_STATE_START,
 	SND_SOC_DPCM_STATE_STOP,
 	SND_SOC_DPCM_STATE_PAUSED,
@@ -98,6 +99,7 @@ struct snd_soc_dpcm_runtime {
 
 	int trigger_pending; /* trigger cmd + 1 if pending, 0 if not */
 
+	int be_detect; /* detection runinng on BE */
 	int be_start; /* refcount protected by BE stream pcm lock */
 	int be_pause; /* refcount protected by BE stream pcm lock */
 	bool fe_pause; /* used to track STOP after PAUSE */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c024eb0ed74ff..5370797801132 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1470,7 +1470,7 @@ static const struct action_ops snd_pcm_action_detect = {
  * Return: Zero if successful, or a negative error code.
  * The stream lock must be acquired before calling this function.
  */
-int snd_pcm_detect(struct snd_pcm_substream *substream)
+static int snd_pcm_detect(struct snd_pcm_substream *substream)
 {
 	return snd_pcm_action_lock_irq(&snd_pcm_action_detect, substream,
 				       SNDRV_PCM_STATE_DETECTING);
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 7a59121fc323c..2d65008ec7a2b 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -110,6 +110,7 @@ static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
 		SND_SOC_DPCM_STATE_START,
 		SND_SOC_DPCM_STATE_PAUSED,
 		SND_SOC_DPCM_STATE_SUSPEND,
+		SND_SOC_DPCM_STATE_DETECT,
 	};
 
 	return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state));
@@ -127,6 +128,7 @@ static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
 		SND_SOC_DPCM_STATE_PAUSED,
 		SND_SOC_DPCM_STATE_SUSPEND,
 		SND_SOC_DPCM_STATE_PREPARE,
+		SND_SOC_DPCM_STATE_DETECT,
 	};
 
 	return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state));
@@ -171,6 +173,8 @@ static const char *dpcm_state_string(enum snd_soc_dpcm_state state)
 		return "hw_params";
 	case SND_SOC_DPCM_STATE_PREPARE:
 		return "prepare";
+	case SND_SOC_DPCM_STATE_DETECT:
+		return "detect";
 	case SND_SOC_DPCM_STATE_START:
 		return "start";
 	case SND_SOC_DPCM_STATE_STOP:
@@ -1250,6 +1254,7 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_DETECT_START:
 		for (i = 0; i < TRIGGER_MAX; i++) {
 			r = trigger[start][i](substream, cmd, 0);
 			if (r < 0)
@@ -1274,6 +1279,9 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 			cmd = SNDRV_PCM_TRIGGER_PAUSE_PUSH;
 			break;
+		case SNDRV_PCM_TRIGGER_DETECT_START:
+			cmd = SNDRV_PCM_TRIGGER_DETECT_STOP;
+			break;
 		}
 	}
 
@@ -1284,6 +1292,7 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_DETECT_STOP:
 		for (i = TRIGGER_MAX; i > 0; i--) {
 			r = trigger[stop][i - 1](substream, cmd, rollback);
 			if (r < 0)
@@ -2247,6 +2256,25 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 			be->dai_link->name, cmd);
 
 		switch (cmd) {
+		case SNDRV_PCM_TRIGGER_DETECT_START:
+			if (!be->dpcm[stream].be_start &&
+			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
+			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP))
+				goto next;
+
+			be->dpcm[stream].be_detect++;
+			if (be->dpcm[stream].be_detect != 1)
+				goto next;
+
+			ret = soc_pcm_trigger(be_substream,
+					      SNDRV_PCM_TRIGGER_DETECT_START);
+			if (ret) {
+				be->dpcm[stream].be_detect--;
+				goto next;
+			}
+
+			be->dpcm[stream].state = SND_SOC_DPCM_STATE_DETECT;
+			break;
 		case SNDRV_PCM_TRIGGER_START:
 			if (!be->dpcm[stream].be_start &&
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
@@ -2254,6 +2282,8 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
 				goto next;
 
+			if (be->dpcm[stream].be_detect > 0)
+				be->dpcm[stream].be_detect = 0;
 			be->dpcm[stream].be_start++;
 			if (be->dpcm[stream].be_start != 1)
 				goto next;
@@ -2308,6 +2338,23 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
+		case SNDRV_PCM_TRIGGER_DETECT_STOP:
+			if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_DETECT)
+				goto next;
+
+			be->dpcm[stream].be_detect--;
+
+			if (be->dpcm[stream].be_detect != 0)
+				goto next;
+
+			ret = soc_pcm_trigger(be_substream, SNDRV_PCM_TRIGGER_DETECT_STOP);
+			if (ret) {
+				be->dpcm[stream].be_detect++;
+				goto next;
+			}
+
+			be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
+			break;
 		case SNDRV_PCM_TRIGGER_STOP:
 			if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_START) &&
 			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
@@ -2440,11 +2487,13 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 		case SNDRV_PCM_TRIGGER_RESUME:
 		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		case SNDRV_PCM_TRIGGER_DRAIN:
+		case SNDRV_PCM_TRIGGER_DETECT_START:
 			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
 			break;
 		case SNDRV_PCM_TRIGGER_STOP:
 		case SNDRV_PCM_TRIGGER_SUSPEND:
 		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		case SNDRV_PCM_TRIGGER_DETECT_STOP:
 			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
 			break;
 		default:
@@ -2458,11 +2507,13 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 		case SNDRV_PCM_TRIGGER_RESUME:
 		case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		case SNDRV_PCM_TRIGGER_DRAIN:
+		case SNDRV_PCM_TRIGGER_DETECT_START:
 			ret = dpcm_dai_trigger_fe_be(substream, cmd, false);
 			break;
 		case SNDRV_PCM_TRIGGER_STOP:
 		case SNDRV_PCM_TRIGGER_SUSPEND:
 		case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		case SNDRV_PCM_TRIGGER_DETECT_STOP:
 			ret = dpcm_dai_trigger_fe_be(substream, cmd, true);
 			break;
 		default:
@@ -2484,6 +2535,10 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 	}
 
 	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_DETECT_START:
+		// return -EINVAL;? as FE is not running when detect happens?
+		fe->dpcm[stream].state = SND_SOC_DPCM_STATE_DETECT;
+		break;
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
@@ -2491,6 +2546,7 @@ static int dpcm_fe_dai_do_trigger(struct snd_pcm_substream *substream, int cmd)
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_DETECT_STOP:
 		fe->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
 		break;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-- 
2.34.1

[RFC PATCH 4/4] ASoC: Propagate DETECT trigger

[Amadeusz Sławiński]

Adds support for DETECT trigger in ASoC API, so it can be propagated to
ALSA core.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-component.c | 2 ++
 sound/soc/soc-dai.c       | 2 ++
 sound/soc/soc-link.c      | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index b67ef78f405c3..1ac8acb326623 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -1237,6 +1237,7 @@ int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_DETECT_START:
 		for_each_rtd_components(rtd, i, component) {
 			ret = soc_component_trigger(component, substream, cmd);
 			if (ret < 0)
@@ -1247,6 +1248,7 @@ int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_DETECT_STOP:
 		for_each_rtd_components(rtd, i, component) {
 			if (rollback && !soc_component_mark_match(component, substream, trigger))
 				continue;
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index 4a1c85ad5a8d6..41fc4ce682a63 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -616,6 +616,7 @@ int snd_soc_pcm_dai_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_DETECT_START:
 		for_each_rtd_dais(rtd, i, dai) {
 			ret = soc_dai_trigger(dai, substream, cmd);
 			if (ret < 0)
@@ -630,6 +631,7 @@ int snd_soc_pcm_dai_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_DETECT_STOP:
 		for_each_rtd_dais(rtd, i, dai) {
 			if (rollback && !soc_dai_mark_match(dai, substream, trigger))
 				continue;
diff --git a/sound/soc/soc-link.c b/sound/soc/soc-link.c
index 7f1f1bc717e2c..30d283062baee 100644
--- a/sound/soc/soc-link.c
+++ b/sound/soc/soc-link.c
@@ -163,6 +163,7 @@ int snd_soc_link_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+	case SNDRV_PCM_TRIGGER_DETECT_START:
 		ret = soc_link_trigger(substream, cmd);
 		if (ret < 0)
 			break;
@@ -171,6 +172,7 @@ int snd_soc_link_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+	case SNDRV_PCM_TRIGGER_DETECT_STOP:
 		if (rollback && !soc_link_mark_match(rtd, substream, trigger))
 			break;
 
-- 
2.34.1

Re: [RFC PATCH 0/4] Add support for detection

[Takashi Iwai]

On Wed, 16 Oct 2024 15:02:24 +0200,
Amadeusz Sławiński wrote:
> 
> There are some scenarios when using DSP where one may want to have
> partially active stream and fully enable it after some event occurs.
> 
> Following patchset adds new "detect" state to ALSA state machine to
> allow waiting for condition to occur before fully starting a stream. In
> further patches the state is propagated through ASoC components to allow
> them to handling the state as necessary.
> 
> Main goal of this patchset is to allow handling scenarios like keyphrase
> detection - where DSP analyses incoming signal and wakes userspace to
> consume stream only when keyphrase is detected.
> 
> I'm sending this as RFC so we can discuss if this is the way to go or if
> there is perhaps another preferred way of adding such interface.
> Userspace part of implementation is available at
> https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect
> 
> Amadeusz Sławiński (4):
>   ALSA: core: Add support for running detect on capture stream
>   ALSA: core: Allow polling for detection
>   ASoC: pcm: Add support for running detect on capture stream
>   ASoC: Propagate DETECT trigger

Generally speaking, the addition of a new PCM state should be avoided.
It'll influence too badly on all user-space programs.  e.g. if an old
user-space program receives such a new state, what should it do?
How can it know it's a fatal error or it can be ignored / skipped?

And, if it's about the synchronization of the DSP readiness, can't it
be rather synced in each PCM open or prepare instead?


Takashi

Re: [RFC PATCH 0/4] Add support for detection

[Amadeusz Sławiński]

On 10/16/2024 3:11 PM, Takashi Iwai wrote:
> On Wed, 16 Oct 2024 15:02:24 +0200,
> Amadeusz Sławiński wrote:
>>
>> There are some scenarios when using DSP where one may want to have
>> partially active stream and fully enable it after some event occurs.
>>
>> Following patchset adds new "detect" state to ALSA state machine to
>> allow waiting for condition to occur before fully starting a stream. In
>> further patches the state is propagated through ASoC components to allow
>> them to handling the state as necessary.
>>
>> Main goal of this patchset is to allow handling scenarios like keyphrase
>> detection - where DSP analyses incoming signal and wakes userspace to
>> consume stream only when keyphrase is detected.
>>
>> I'm sending this as RFC so we can discuss if this is the way to go or if
>> there is perhaps another preferred way of adding such interface.
>> Userspace part of implementation is available at
>> https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect
>>
>> Amadeusz Sławiński (4):
>>    ALSA: core: Add support for running detect on capture stream
>>    ALSA: core: Allow polling for detection
>>    ASoC: pcm: Add support for running detect on capture stream
>>    ASoC: Propagate DETECT trigger
> 
> Generally speaking, the addition of a new PCM state should be avoided.
> It'll influence too badly on all user-space programs.  e.g. if an old
> user-space program receives such a new state, what should it do?
> How can it know it's a fatal error or it can be ignored / skipped?

In this case it should not get into new state unless specifically 
requested from userspace, unless I'm missing something?

Goal is to allow something along the lines of following in arecord or 
similar:

         ret = snd_pcm_detect(handle); // here only parts of DSP FW 
needed for detection are running
         c = snd_pcm_poll_descriptors_count(handle);

         pfds = malloc(sizeof(struct pollfd) * c);

         snd_pcm_poll_descriptors(handle, pfds, c); // polling for 
detect event to happen

         while (!detected) {
                 ret = poll(pfds, c, -1);
                 snd_pcm_poll_descriptors_revents(handle, pfds, c, 
&revents);

                 if (revents == POLLERR) {
                         error(_("poll, revents == |POLLERR"));
                 }
                 if (revents == POLLIN) {
                         error(_("poll, revents == |POLLIN"));
                         detected = 1;
                 }
         }
         ret = snd_pcm_start(handle); // starts whatever else is needed 
for PCM to work

> 
> And, if it's about the synchronization of the DSP readiness, can't it
> be rather synced in each PCM open or prepare instead?
> 

It's too early. We need to do it after hw_params as it needs to create 
all paths needed for full stream. During detection it just activates 
ones needed for detection and only after receiving detection event we 
want to start ones needed for draining.

Re: [RFC PATCH 0/4] Add support for detection

[Takashi Iwai]

On Wed, 16 Oct 2024 15:29:35 +0200,
Amadeusz Sławiński wrote:
> 
> On 10/16/2024 3:11 PM, Takashi Iwai wrote:
> > On Wed, 16 Oct 2024 15:02:24 +0200,
> > Amadeusz Sławiński wrote:
> >> 
> >> There are some scenarios when using DSP where one may want to have
> >> partially active stream and fully enable it after some event occurs.
> >> 
> >> Following patchset adds new "detect" state to ALSA state machine to
> >> allow waiting for condition to occur before fully starting a stream. In
> >> further patches the state is propagated through ASoC components to allow
> >> them to handling the state as necessary.
> >> 
> >> Main goal of this patchset is to allow handling scenarios like keyphrase
> >> detection - where DSP analyses incoming signal and wakes userspace to
> >> consume stream only when keyphrase is detected.
> >> 
> >> I'm sending this as RFC so we can discuss if this is the way to go or if
> >> there is perhaps another preferred way of adding such interface.
> >> Userspace part of implementation is available at
> >> https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect
> >> 
> >> Amadeusz Sławiński (4):
> >>    ALSA: core: Add support for running detect on capture stream
> >>    ALSA: core: Allow polling for detection
> >>    ASoC: pcm: Add support for running detect on capture stream
> >>    ASoC: Propagate DETECT trigger
> > 
> > Generally speaking, the addition of a new PCM state should be avoided.
> > It'll influence too badly on all user-space programs.  e.g. if an old
> > user-space program receives such a new state, what should it do?
> > How can it know it's a fatal error or it can be ignored / skipped?
> 
> In this case it should not get into new state unless specifically
> requested from userspace, unless I'm missing something?

Hmm, if the state is exclusive only for the requested program and
influences only on that program itself, why does it have to be a
global PCM state that essentially every program code has to deal with?

> Goal is to allow something along the lines of following in arecord or
> similar:
> 
>         ret = snd_pcm_detect(handle); // here only parts of DSP FW
> needed for detection are running
>         c = snd_pcm_poll_descriptors_count(handle);
> 
>         pfds = malloc(sizeof(struct pollfd) * c);
> 
>         snd_pcm_poll_descriptors(handle, pfds, c); // polling for
> detect event to happen
> 
>         while (!detected) {
>                 ret = poll(pfds, c, -1);
>                 snd_pcm_poll_descriptors_revents(handle, pfds, c,
> &revents);
> 
>                 if (revents == POLLERR) {
>                         error(_("poll, revents == |POLLERR"));
>                 }
>                 if (revents == POLLIN) {
>                         error(_("poll, revents == |POLLIN"));
>                         detected = 1;
>                 }
>         }

It's too complex if it's needed for each program.
If any, it'd be easier to implement an ioctl() for triggering the
detect and the sync...

>         ret = snd_pcm_start(handle); // starts whatever else is needed
> for PCM to work
> 
> > 
> > And, if it's about the synchronization of the DSP readiness, can't it
> > be rather synced in each PCM open or prepare instead?
> > 
> 
> It's too early. We need to do it after hw_params as it needs to create
> all paths needed for full stream.

It can be done in hw_params, too.  I don't mind where to put, but my
point is that the sync can be implemented internally without changing
the external API to user-space.

> During detection it just activates
> ones needed for detection and only after receiving detection event we
> want to start ones needed for draining.

So if the driver needs the sync, it can be done in hw_params or
prepare, too.  Something like stop_sync stuff we introduced.
In the case of stop_sync, hw_params waits for the pending task by the
previous stop trigger.  In this case, the sync can be performed after
hw_params (if requested) instead.


thanks,

Takashi

Re: [RFC PATCH 0/4] Add support for detection

[Amadeusz Sławiński]

On 10/16/2024 3:47 PM, Takashi Iwai wrote:
 > On Wed, 16 Oct 2024 15:29:35 +0200,
 > Amadeusz Sławiński wrote:
 >>
 >> On 10/16/2024 3:11 PM, Takashi Iwai wrote:
 >>> On Wed, 16 Oct 2024 15:02:24 +0200,
 >>> Amadeusz Sławiński wrote:
 >>>>
 >>>> There are some scenarios when using DSP where one may want to have
 >>>> partially active stream and fully enable it after some event occurs.
 >>>>
 >>>> Following patchset adds new "detect" state to ALSA state machine to
 >>>> allow waiting for condition to occur before fully starting a 
stream. In
 >>>> further patches the state is propagated through ASoC components to 
allow
 >>>> them to handling the state as necessary.
 >>>>
 >>>> Main goal of this patchset is to allow handling scenarios like 
keyphrase
 >>>> detection - where DSP analyses incoming signal and wakes userspace to
 >>>> consume stream only when keyphrase is detected.
 >>>>
 >>>> I'm sending this as RFC so we can discuss if this is the way to go 
or if
 >>>> there is perhaps another preferred way of adding such interface.
 >>>> Userspace part of implementation is available at
 >>>> https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect
 >>>>
 >>>> Amadeusz Sławiński (4):
 >>>>     ALSA: core: Add support for running detect on capture stream
 >>>>     ALSA: core: Allow polling for detection
 >>>>     ASoC: pcm: Add support for running detect on capture stream
 >>>>     ASoC: Propagate DETECT trigger
 >>>
 >>> Generally speaking, the addition of a new PCM state should be avoided.
 >>> It'll influence too badly on all user-space programs.  e.g. if an old
 >>> user-space program receives such a new state, what should it do?
 >>> How can it know it's a fatal error or it can be ignored / skipped?
 >>
 >> In this case it should not get into new state unless specifically
 >> requested from userspace, unless I'm missing something?
 >
 > Hmm, if the state is exclusive only for the requested program and
 > influences only on that program itself, why does it have to be a
 > global PCM state that essentially every program code has to deal with?

It is stream state, so we can have stream in detect state and wait for 
some event on DSP, it can be for example like detecting someone saying 
something above background noise threshold and then we want to process 
it in userspace, only when it happens and when there is no more 
interesting data we want to be able to return back into detection state.

 >> Goal is to allow something along the lines of following in arecord or
 >> similar:
 >>
 >>          ret = snd_pcm_detect(handle); // here only parts of DSP FW
 >> needed for detection are running
 >>          c = snd_pcm_poll_descriptors_count(handle);
 >>
 >>          pfds = malloc(sizeof(struct pollfd) * c);
 >>
 >>          snd_pcm_poll_descriptors(handle, pfds, c); // polling for
 >> detect event to happen
 >>
 >>          while (!detected) {
 >>                  ret = poll(pfds, c, -1);
 >>                  snd_pcm_poll_descriptors_revents(handle, pfds, c,
 >> &revents);
 >>
 >>                  if (revents == POLLERR) {
 >>                          error(_("poll, revents == |POLLERR"));
 >>                  }
 >>                  if (revents == POLLIN) {
 >>                          error(_("poll, revents == |POLLIN"));
 >>                          detected = 1;
 >>                  }
 >>          }
 >
 > It's too complex if it's needed for each program.
 > If any, it'd be easier to implement an ioctl() for triggering the
 > detect and the sync...
 >

Well we can implement custom IOCTL in HWDEP, but I'd rather prefer to 
have this functionality within machine state, so there is no confusion 
between state machine and custom IOCTL.

 >>          ret = snd_pcm_start(handle); // starts whatever else is needed
 >> for PCM to work
 >>
 >>>
 >>> And, if it's about the synchronization of the DSP readiness, can't it
 >>> be rather synced in each PCM open or prepare instead?
 >>>
 >>
 >> It's too early. We need to do it after hw_params as it needs to create
 >> all paths needed for full stream.
 >
 > It can be done in hw_params, too.  I don't mind where to put, but my
 > point is that the sync can be implemented internally without changing
 > the external API to user-space.
 >

hw_params is too early, as not all paths may be ready, additionally we 
need to allow for sending additional configuration from userspace in 
some use cases.

 >> During detection it just activates
 >> ones needed for detection and only after receiving detection event we
 >> want to start ones needed for draining.
 >
 > So if the driver needs the sync, it can be done in hw_params or
 > prepare, too.  Something like stop_sync stuff we introduced.
 > In the case of stop_sync, hw_params waits for the pending task by the
 > previous stop trigger.  In this case, the sync can be performed after
 > hw_params (if requested) instead.

I'm not sure where sync comes in here, maybe we are talking about two 
different use cases?

Our use case is to:
1. Program DSP with all pipelines it needs to implement whole scenario 
(hw_params)
2. Perform additional configuration from usespace
3. Start detection pipelines
4. Wait for event to happen (it can take however long necessary, for 
example hours)
5. When event happens start drain pipelines and process data in 
userspace likes in standard capture scenario
6. When no more data is needed, either stop drain pipelines and go back 
to 3 or just close stream

Re: [RFC PATCH 0/4] Add support for detection

[Jaroslav Kysela]

On 18. 10. 24 16:16, Amadeusz Sławiński wrote:

> Our use case is to:

Inlined my proposal:

> 1. Program DSP with all pipelines it needs to implement whole scenario
> (hw_params)

SNDRV_PCM_IOCTL_HW_PARAMS

> 2. Perform additional configuration from usespace

Can be done using control API (/dev/snd/controlC*). E.g. We have already:

numid=19,iface=PCM,name='ELD',device=3
numid=32,iface=PCM,name='Playback Channel Map',device=3

Those controls can be R/W as you like.

> 3. Start detection pipelines

SNDRV_PCM_IOCTL_PREPARE

> 4. Wait for event to happen (it can take however long necessary, for
> example hours)

Can be done using control API (/dev/snd/controlC*). There is a mechanism for 
event handling already.

Something like (boolean type):

iface=PCM,device=2,name="Event X"   # first event type
iface=PCM,device=2,name="Event Y"   # second event type ...

The PCM stream may be active only when at least one event from the above set 
is active.

> 5. When event happens start drain pipelines and process data in
> userspace likes in standard capture scenario

SNDRV_PCM_IOCTL_START

The driver may set the correct timestamp when the capturing was started. The 
sample buffers are already allocated and ready for transfers, so we can assume 
that the time window between the event notification (event) and START ioctl is 
small, so no samples should be lost.

> 6. When no more data is needed, either stop drain pipelines and go back
> to 3 or just close stream

SNDRV_PCM_IOCTL_STOP / DRAIN, SNDRV_PCM_IOCTL_HW_PARAMS or close(fd)

Basically, I think that this extension can be implemented with minimal changes 
to the PCM API - it's just about to slightly extend existing APIs.

				Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [RFC PATCH 0/4] Add support for detection

[Amadeusz Sławiński]

On 10/18/2024 7:15 PM, Jaroslav Kysela wrote:
> On 18. 10. 24 16:16, Amadeusz Sławiński wrote:
> 
>> Our use case is to:
> 
> Inlined my proposal:
> 
>> 1. Program DSP with all pipelines it needs to implement whole scenario
>> (hw_params)
> 
> SNDRV_PCM_IOCTL_HW_PARAMS
> 
>> 2. Perform additional configuration from usespace
> 
> Can be done using control API (/dev/snd/controlC*). E.g. We have already:
> 
> numid=19,iface=PCM,name='ELD',device=3
> numid=32,iface=PCM,name='Playback Channel Map',device=3
> 
> Those controls can be R/W as you like.
> 
>> 3. Start detection pipelines
> 
> SNDRV_PCM_IOCTL_PREPARE
> 

Will check, but from what I recall streams should not be started in 
prepare due to it being used for xrun recovery, but maybe it doesn't 
matter for detection ones.

>> 4. Wait for event to happen (it can take however long necessary, for
>> example hours)
> 
> Can be done using control API (/dev/snd/controlC*). There is a mechanism 
> for event handling already.
> 
> Something like (boolean type):
> 
> iface=PCM,device=2,name="Event X"   # first event type
> iface=PCM,device=2,name="Event Y"   # second event type ...
> 
> The PCM stream may be active only when at least one event from the above 
> set is active.
> 
>> 5. When event happens start drain pipelines and process data in
>> userspace likes in standard capture scenario
> 
> SNDRV_PCM_IOCTL_START
> 
> The driver may set the correct timestamp when the capturing was started. 
> The sample buffers are already allocated and ready for transfers, so we 
> can assume that the time window between the event notification (event) 
> and START ioctl is small, so no samples should be lost.
> 
>> 6. When no more data is needed, either stop drain pipelines and go back
>> to 3 or just close stream
> 
> SNDRV_PCM_IOCTL_STOP / DRAIN, SNDRV_PCM_IOCTL_HW_PARAMS or close(fd)
> 
> Basically, I think that this extension can be implemented with minimal 
> changes to the PCM API - it's just about to slightly extend existing APIs.
> 
>                  Jaroslav
> 

Re: [RFC PATCH 0/4] Add support for detection

[Jaroslav Kysela]

On 16. 10. 24 15:02, Amadeusz Sławiński wrote:
> There are some scenarios when using DSP where one may want to have
> partially active stream and fully enable it after some event occurs.
> 
> Following patchset adds new "detect" state to ALSA state machine to
> allow waiting for condition to occur before fully starting a stream. In
> further patches the state is propagated through ASoC components to allow
> them to handling the state as necessary.
> 
> Main goal of this patchset is to allow handling scenarios like keyphrase
> detection - where DSP analyses incoming signal and wakes userspace to
> consume stream only when keyphrase is detected.
> 
> I'm sending this as RFC so we can discuss if this is the way to go or if
> there is perhaps another preferred way of adding such interface.
> Userspace part of implementation is available at
> https://github.com/amadeuszslawinski-intel/alsa-lib/tree/rfc_detect

It looks like a complex extension with a low benefit. We have already layer 
which can signalize stream readiness - an element in the control API. PCM 
driver may just return silence samples when the condition is not met for 
security reasons.

				Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.