diff mbox series

[4/7] ASoC: soc-component: add mark for snd_soc_pcm_component_pm_runtime_get/put()

Message ID 87h7ribwnb.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: merge soc_pcm_open() rollback and soc_pcm_close() | expand

Commit Message

Kuninori Morimoto Sept. 28, 2020, 12:01 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_open() does rollback when failed (A),
but, it is almost same as soc_pcm_close().

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

 ^	config_err:
 |		...
 |	rtd_startup_err:
(A)		...
 |	component_err:
 |		...
 v		return ret;
	}

The difference is
soc_pcm_close() 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_close() and rollback.

Now, soc_pcm_open/close() are handling
	1) snd_soc_dai_startup/shutdown()
	2) snd_soc_link_startup/shutdown()
	3) snd_soc_component_module_get/put()
	4) snd_soc_component_open/close()
=>	5) pm_runtime_put/get()

This patch is for 5) pm_runtime_put/get().

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 get() was
succeeded. If rollback needed, it will check rollback flag and marked
substream pointer.

One note here is that it cares *current* get() 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-component.h |  5 +++++
 sound/soc/soc-component.c     | 38 +++++++++++++++++++++++++++++++++++
 sound/soc/soc-compress.c      | 30 ++++++++-------------------
 sound/soc/soc-pcm.c           | 17 ++++++----------
 4 files changed, 57 insertions(+), 33 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index a812202ec595..2c790ce95259 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -220,6 +220,7 @@  struct snd_soc_component {
 	/* function mark */
 	struct snd_pcm_substream *mark_module;
 	struct snd_pcm_substream *mark_open;
+	void *mark_pm;
 
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs_root;
@@ -464,5 +465,9 @@  void snd_soc_pcm_component_hw_free(struct snd_pcm_substream *substream,
 				   struct snd_soc_component *last);
 int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
 				  int cmd);
+int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
+					 void *stream);
+void snd_soc_pcm_component_pm_runtime_put(struct snd_soc_pcm_runtime *rtd,
+					  void *stream, int rollback);
 
 #endif /* __SOC_COMPONENT_H */
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 147b46e29ff4..728e93f35ffb 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -9,6 +9,7 @@ 
 // Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
 //
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <sound/soc.h>
 
 #define soc_component_ret(dai, ret) _soc_component_ret(dai, __func__, ret)
@@ -836,3 +837,40 @@  int snd_soc_pcm_component_trigger(struct snd_pcm_substream *substream,
 
 	return 0;
 }
+
+int snd_soc_pcm_component_pm_runtime_get(struct snd_soc_pcm_runtime *rtd,
+					 void *stream)
+{
+	struct snd_soc_component *component;
+	int i, ret;
+
+	for_each_rtd_components(rtd, i, component) {
+		ret = pm_runtime_get_sync(component->dev);
+		if (ret < 0 && ret != -EACCES) {
+			pm_runtime_put_noidle(component->dev);
+			return soc_component_ret(component, ret);
+		}
+		/* mark stream if succeeded */
+		soc_component_mark_push(component, stream, pm);
+	}
+
+	return 0;
+}
+
+void snd_soc_pcm_component_pm_runtime_put(struct snd_soc_pcm_runtime *rtd,
+					  void *stream, int rollback)
+{
+	struct snd_soc_component *component;
+	int i;
+
+	for_each_rtd_components(rtd, i, component) {
+		if (rollback && !soc_component_mark_match(component, stream, pm))
+			continue;
+
+		pm_runtime_mark_last_busy(component->dev);
+		pm_runtime_put_autosuspend(component->dev);
+
+		/* remove marked stream */
+		soc_component_mark_pop(component, stream, pm);
+	}
+}
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 415510909a82..3a6a60215e81 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -73,18 +73,13 @@  static int soc_compr_components_free(struct snd_compr_stream *cstream,
 static int soc_compr_open(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component = NULL, *save = NULL;
+	struct snd_soc_component *component = NULL;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
-	int ret, i;
+	int ret;
 
-	for_each_rtd_components(rtd, i, component) {
-		ret = pm_runtime_get_sync(component->dev);
-		if (ret < 0 && ret != -EACCES) {
-			pm_runtime_put_noidle(component->dev);
-			save = component;
-			goto pm_err;
-		}
-	}
+	ret = snd_soc_pcm_component_pm_runtime_get(rtd, cstream);
+	if (ret < 0)
+		goto pm_err;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -113,12 +108,7 @@  static int soc_compr_open(struct snd_compr_stream *cstream)
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
 pm_err:
-	for_each_rtd_components(rtd, i, component) {
-		if (component == save)
-			break;
-		pm_runtime_mark_last_busy(component->dev);
-		pm_runtime_put_autosuspend(component->dev);
-	}
+	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 1);
 
 	return ret;
 }
@@ -205,10 +195,9 @@  static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	int stream, i;
+	int stream;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -237,10 +226,7 @@  static int soc_compr_free(struct snd_compr_stream *cstream)
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	for_each_rtd_components(rtd, i, component) {
-		pm_runtime_mark_last_busy(component->dev);
-		pm_runtime_put_autosuspend(component->dev);
-	}
+	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 0);
 
 	return 0;
 }
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index d9f9c0a243df..0934a06a9cc9 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -678,10 +678,7 @@  static int soc_pcm_close(struct snd_pcm_substream *substream)
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	for_each_rtd_components(rtd, i, component) {
-		pm_runtime_mark_last_busy(component->dev);
-		pm_runtime_put_autosuspend(component->dev);
-	}
+	snd_soc_pcm_component_pm_runtime_put(rtd, substream, 0);
 
 	for_each_rtd_components(rtd, i, component)
 		if (!snd_soc_component_active(component))
@@ -708,8 +705,9 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 	for_each_rtd_components(rtd, i, component)
 		pinctrl_pm_select_default_state(component->dev);
 
-	for_each_rtd_components(rtd, i, component)
-		pm_runtime_get_sync(component->dev);
+	ret = snd_soc_pcm_component_pm_runtime_get(rtd, substream);
+	if (ret < 0)
+		goto pm_err;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -806,11 +804,8 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 	soc_pcm_components_close(substream, 1);
 component_err:
 	mutex_unlock(&rtd->card->pcm_mutex);
-
-	for_each_rtd_components(rtd, i, component) {
-		pm_runtime_mark_last_busy(component->dev);
-		pm_runtime_put_autosuspend(component->dev);
-	}
+pm_err:
+	snd_soc_pcm_component_pm_runtime_put(rtd, substream, 1);
 
 	for_each_rtd_components(rtd, i, component)
 		if (!snd_soc_component_active(component))