diff mbox series

[5/6] ASoC: soc-dai: add mark for snd_soc_dai_hw_params/free()

Message ID 87imbxgqai.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State Accepted
Commit c304c9acb6e60bcc5c4d4b5a72763ca3bdf7d76b
Headers show
Series ASoC: merge soc_pcm_hw_param() rollback and soc_pcm_hw_free() | expand

Commit Message

Kuninori Morimoto Sept. 29, 2020, 4:31 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_hw_params() does rollback when failed (A),
but, it is almost same as soc_pcm_hw_free().

	static int soc_pcm_hw_params(xxx)
	{
		...
		if (ret < 0)
			goto xxx_err;
		...
		return ret;

 ^	component_err:
 |		...
 |	interface_err:
(A)		...
 |	codec_err:
 |		...
 v		return ret;
	}

The difference is
soc_pcm_hw_free() is for all dai/component/substream,
rollback          is for succeeded part only.

This kind of duplicated code can be a hotbed of bugs,
thus, we want to share soc_pcm_hw_free() and rollback.

Now, soc_pcm_hw_params/free() are handling
	1) snd_soc_link_hw_params/free()
	2) snd_soc_pcm_component_hw_params/free()
=>	3) snd_soc_dai_hw_params/free()

This patch is for 3) snd_soc_dai_hw_params/free().

The idea of having bit-flag or counter is not enough for this purpose.
For example if one DAI is used for 2xPlaybacks for some reasons,
and if 1st Playback was succeeded but 2nd Playback was failed,
2nd Playback rollback doesn't need to call shutdown.
But it has succeeded bit-flag or counter via 1st Playback,
thus, 2nd Playback rollback will call unneeded shutdown.
And 1st Playback's necessary shutdown will not be called,
because bit-flag or counter was cleared by wrong 2nd Playback rollback.

To avoid such case, this patch marks substream pointer when hw_params() was
succeeded. If rollback needed, it will check rollback flag and marked
substream pointer.

One note here is that it cares *previous* hw_params() only now,
but we might want to check *whole* marked substream in the future.
This patch is using macro named "push/pop", so that it can be easily
update.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dai.h |  4 +++-
 sound/soc/soc-dai.c     | 13 ++++++++++++-
 sound/soc/soc-dapm.c    |  4 ++--
 sound/soc/soc-pcm.c     |  6 +++---
 4 files changed, 20 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 2150bd4c7a05..7a85a6f83ca8 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -149,7 +149,8 @@  int snd_soc_dai_hw_params(struct snd_soc_dai *dai,
 			  struct snd_pcm_substream *substream,
 			  struct snd_pcm_hw_params *params);
 void snd_soc_dai_hw_free(struct snd_soc_dai *dai,
-			 struct snd_pcm_substream *substream);
+			 struct snd_pcm_substream *substream,
+			 int rollback);
 int snd_soc_dai_startup(struct snd_soc_dai *dai,
 			struct snd_pcm_substream *substream);
 void snd_soc_dai_shutdown(struct snd_soc_dai *dai,
@@ -390,6 +391,7 @@  struct snd_soc_dai {
 
 	/* function mark */
 	struct snd_pcm_substream *mark_startup;
+	struct snd_pcm_substream *mark_hw_params;
 
 	/* bit field */
 	unsigned int probed:1;
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index 4705c3da6280..2686a566649b 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -335,16 +335,27 @@  int snd_soc_dai_hw_params(struct snd_soc_dai *dai,
 	if (dai->driver->ops &&
 	    dai->driver->ops->hw_params)
 		ret = dai->driver->ops->hw_params(substream, params, dai);
+
+	/* mark substream if succeeded */
+	if (ret == 0)
+		soc_dai_mark_push(dai, substream, hw_params);
 end:
 	return soc_dai_ret(dai, ret);
 }
 
 void snd_soc_dai_hw_free(struct snd_soc_dai *dai,
-			 struct snd_pcm_substream *substream)
+			 struct snd_pcm_substream *substream,
+			 int rollback)
 {
+	if (rollback && !soc_dai_mark_match(dai, substream, hw_params))
+		return;
+
 	if (dai->driver->ops &&
 	    dai->driver->ops->hw_free)
 		dai->driver->ops->hw_free(substream, dai);
+
+	/* remove marked substream */
+	soc_dai_mark_pop(dai, substream, hw_params);
 }
 
 int snd_soc_dai_startup(struct snd_soc_dai *dai,
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 980f2c330b87..471ac8d9b669 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3955,13 +3955,13 @@  static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
 		substream->stream = SNDRV_PCM_STREAM_CAPTURE;
 		snd_soc_dapm_widget_for_each_source_path(w, path) {
 			source = path->source->priv;
-			snd_soc_dai_hw_free(source, substream);
+			snd_soc_dai_hw_free(source, substream, 0);
 		}
 
 		substream->stream = SNDRV_PCM_STREAM_PLAYBACK;
 		snd_soc_dapm_widget_for_each_sink_path(w, path) {
 			sink = path->sink->priv;
-			snd_soc_dai_hw_free(sink, substream);
+			snd_soc_dai_hw_free(sink, substream, 0);
 		}
 
 		substream->stream = SNDRV_PCM_STREAM_CAPTURE;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 5d4d0a95891a..80337d0cd0d2 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -895,7 +895,7 @@  static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
 		if (!snd_soc_dai_stream_valid(dai, substream->stream))
 			continue;
 
-		snd_soc_dai_hw_free(dai, substream);
+		snd_soc_dai_hw_free(dai, substream, 0);
 	}
 
 	mutex_unlock(&rtd->card->pcm_mutex);
@@ -1011,7 +1011,7 @@  static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		if (!snd_soc_dai_stream_valid(cpu_dai, substream->stream))
 			continue;
 
-		snd_soc_dai_hw_free(cpu_dai, substream);
+		snd_soc_dai_hw_free(cpu_dai, substream, 1);
 		cpu_dai->rate = 0;
 	}
 
@@ -1022,7 +1022,7 @@  static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
 			continue;
 
-		snd_soc_dai_hw_free(codec_dai, substream);
+		snd_soc_dai_hw_free(codec_dai, substream, 1);
 		codec_dai->rate = 0;
 	}