[RFC PATCH v3 2/2] ALSA: pcm: refresh capture pointers in the drain action

[Joonho Lee <mynameisjune111@gmail.com>]

Capture drain decides whether to retain unread data from the cached
hardware pointer. A final subperiod can be omitted if the driver has
advanced without a period notification or another pointer query.

Refresh each running capture pointer in snd_pcm_do_drain_init(), with
the normal drain action holding the stream and linked-group locks.
Allow pointer accounting to return an error without issuing a nested
XRUN or drain-complete action. Guard both the driver XRUN sentinel and
the stop-threshold path; otherwise a linked drain can recursively enter
the group stop action while its member locks are held.

Let drain init own the stop: an update error ends that member in SETUP,
while a successful update retains available capture frames in DRAINING.
Ordinary pointer queries and period notifications retain their existing
XRUN and drain-completion actions.

Suggested-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/all/8733u8g0kh.wl-tiwai@suse.de/
Link: https://lore.kernel.org/all/87y0c0el29.wl-tiwai@suse.de/
Assisted-by: LLM
Signed-off-by: Joonho Lee <mynameisjune111@gmail.com>
---
 sound/core/pcm_lib.c    | 34 ++++++++++++++++++++++++----------
 sound/core/pcm_local.h  |  2 ++
 sound/core/pcm_native.c | 10 ++++++++--
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index de952f639d3b..2bb24d7dd2a1 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -206,8 +206,9 @@ void __snd_pcm_xrun(struct snd_pcm_substream *substream)
 
 #endif
 
-int snd_pcm_update_state(struct snd_pcm_substream *substream,
-			 struct snd_pcm_runtime *runtime)
+static int do_pcm_update_state(struct snd_pcm_substream *substream,
+			       struct snd_pcm_runtime *runtime,
+			       bool notify)
 {
 	snd_pcm_uframes_t avail;
 
@@ -216,12 +217,14 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 		runtime->avail_max = avail;
 	if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
 		if (avail >= runtime->buffer_size) {
-			snd_pcm_drain_done(substream);
+			if (notify)
+				snd_pcm_drain_done(substream);
 			return -EPIPE;
 		}
 	} else {
 		if (avail >= runtime->stop_threshold) {
-			__snd_pcm_xrun(substream);
+			if (notify)
+				__snd_pcm_xrun(substream);
 			return -EPIPE;
 		}
 	}
@@ -233,6 +236,12 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+int snd_pcm_update_state(struct snd_pcm_substream *substream,
+			 struct snd_pcm_runtime *runtime)
+{
+	return do_pcm_update_state(substream, runtime, true);
+}
+
 static void update_audio_tstamp(struct snd_pcm_substream *substream,
 				struct timespec64 *curr_tstamp,
 				struct timespec64 *audio_tstamp)
@@ -283,8 +292,12 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream,
 	runtime->driver_tstamp = driver_tstamp;
 }
 
-static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
-				  unsigned int in_interrupt)
+/*
+ * Call with the stream lock held.  With notify=false the caller handles
+ * stopping the stream, so pointer errors must not start a nested action.
+ */
+int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
+			   bool in_interrupt, bool notify)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_uframes_t pos;
@@ -322,7 +335,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 	}
 
 	if (pos == SNDRV_PCM_POS_XRUN) {
-		__snd_pcm_xrun(substream);
+		if (notify)
+			__snd_pcm_xrun(substream);
 		return -EPIPE;
 	}
 	if (pos >= runtime->buffer_size) {
@@ -490,13 +504,13 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 
 	update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
 
-	return snd_pcm_update_state(substream, runtime);
+	return do_pcm_update_state(substream, runtime, notify);
 }
 
 /* CAUTION: call it with irq disabled */
 int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream)
 {
-	return snd_pcm_update_hw_ptr0(substream, 0);
+	return snd_pcm_update_hw_ptr0(substream, false, true);
 }
 
 /**
@@ -1906,7 +1920,7 @@ void snd_pcm_period_elapsed_under_stream_lock(struct snd_pcm_substream *substrea
 	runtime = substream->runtime;
 
 	if (!snd_pcm_running(substream) ||
-	    snd_pcm_update_hw_ptr0(substream, 1) < 0)
+	    snd_pcm_update_hw_ptr0(substream, true, true) < 0)
 		goto _end;
 
 #ifdef CONFIG_SND_PCM_TIMER
diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h
index ecb21697ae3a..12bb6e75329a 100644
--- a/sound/core/pcm_local.h
+++ b/sound/core/pcm_local.h
@@ -28,6 +28,8 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream,
 int snd_pcm_update_state(struct snd_pcm_substream *substream,
 			 struct snd_pcm_runtime *runtime);
 int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream);
+int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
+			   bool in_interrupt, bool notify);
 
 void snd_pcm_playback_silence(struct snd_pcm_substream *substream,
 			      snd_pcm_uframes_t new_hw_ptr);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index defbb2977efe..91ae46de3521 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2094,6 +2094,8 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 				 snd_pcm_state_t state)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		switch (runtime->state) {
 		case SNDRV_PCM_STATE_PREPARED:
@@ -2119,8 +2121,12 @@ static int snd_pcm_do_drain_init(struct snd_pcm_substream *substream,
 		if (runtime->state == SNDRV_PCM_STATE_RUNNING) {
 			snd_pcm_state_t new_state;
 
-			new_state = snd_pcm_capture_avail(runtime) > 0 ?
-				SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP;
+			err = snd_pcm_update_hw_ptr0(substream, false, false);
+			if (err < 0)
+				new_state = SNDRV_PCM_STATE_SETUP;
+			else
+				new_state = snd_pcm_capture_avail(runtime) > 0 ?
+					SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP;
 			snd_pcm_do_stop(substream, new_state);
 			snd_pcm_post_stop(substream, new_state);
 		}
-- 
2.54.0 (Apple Git-157)