All of lore.kernel.org
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8
@ 2019-12-17  1:25 Kuninori Morimoto
  2019-12-17  1:25 ` [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list Kuninori Morimoto
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:25 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


Hi Mark

These are soc-core cleanup step8.
There is no relationship between each patches.

Kuninori Morimoto (6):
  ASoC: soc-core: remove snd_soc_rtdcom_list
  ASoC: soc-core: care .ignore_suspend for Component suspend
  ASoC: soc-core: remove duplicate pinctrl operation when suspend
  ASoC: soc-core: do pinctrl_pm_select_xxx() as component
  ASoC: soc-core: add snd_soc_close_delayed_work()
  ASoC: soc-core: add snd_soc_stream_stop()

 include/sound/soc.h       |  20 +++---
 sound/soc/soc-component.c |  33 +++++----
 sound/soc/soc-compress.c  | 110 ++++++++--------------------
 sound/soc/soc-core.c      | 180 +++++++++++++++++++++++++---------------------
 sound/soc/soc-pcm.c       | 113 +++++++----------------------
 5 files changed, 179 insertions(+), 277 deletions(-)

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
@ 2019-12-17  1:25 ` Kuninori Morimoto
  2019-12-17  1:25 ` [alsa-devel] [PATCH 2/6] ASoC: soc-core: care .ignore_suspend for Component suspend Kuninori Morimoto
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:25 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current ALSA SoC is using struct snd_soc_rtdcom_list to
connecting component to rtd by using list_head.

	struct snd_soc_rtdcom_list {
		struct snd_soc_component *component;
		struct list_head list; /* rtd::component_list */
	};

	struct snd_soc_pcm_runtime {
		...
		struct list_head component_list; /* list of connected components */
		...
	};

The CPU/Codec/Platform component which will be connected to rtd (a)
is indicated via dai_link at snd_soc_add_pcm_runtime()

	int snd_soc_add_pcm_runtime(...)
	{
		...
		/* Find CPU from registered CPUs */
		rtd->cpu_dai = snd_soc_find_dai(dai_link->cpus);
		...
(a)		snd_soc_rtdcom_add(rtd, rtd->cpu_dai->component);
		...

		/* Find CODEC from registered CODECs */
(b)		for_each_link_codecs(dai_link, i, codec) {
			rtd->codec_dais[i] = snd_soc_find_dai(codec);
			...
(a)			snd_soc_rtdcom_add(rtd, rtd->codec_dais[i]->component);
		}
		...

		/* Find PLATFORM from registered PLATFORMs */
(b)		for_each_link_platforms(dai_link, i, platform) {
			for_each_component(component) {
				...
(a)				snd_soc_rtdcom_add(rtd, component);
			}
		}

	}

It shows, it is possible to know how many components will be
connected to rtd by using

	dai_link->num_cpus
	dai_link->num_codecs
	dai_link->num_platforms

If so, we can use component pointer array instead of list_head,
in such case, code can be more simple.
This patch removes struct snd_soc_rtdcom_list that is only
of temporary value, and convert to pointer array.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h       | 18 ++++++--------
 sound/soc/soc-component.c | 33 ++++++++++++-------------
 sound/soc/soc-compress.c  | 63 +++++++++++++++++++----------------------------
 sound/soc/soc-core.c      | 56 ++++++++++++++++-------------------------
 sound/soc/soc-pcm.c       | 43 ++++++++++++--------------------
 5 files changed, 86 insertions(+), 127 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 9787c80..0513f30 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -736,19 +736,9 @@ struct snd_soc_compr_ops {
 	int (*trigger)(struct snd_compr_stream *);
 };
 
-struct snd_soc_rtdcom_list {
-	struct snd_soc_component *component;
-	struct list_head list; /* rtd::component_list */
-};
 struct snd_soc_component*
 snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 		       const char *driver_name);
-#define for_each_rtd_components(rtd, rtdcom, _component)		\
-	for (rtdcom = list_first_entry(&(rtd)->component_list,		\
-				       typeof(*rtdcom), list);		\
-	     (&rtdcom->list != &(rtd)->component_list) &&		\
-		     (_component = rtdcom->component);			\
-	     rtdcom = list_next_entry(rtdcom, list))
 
 struct snd_soc_dai_link_component {
 	const char *name;
@@ -1150,12 +1140,18 @@ struct snd_soc_pcm_runtime {
 
 	unsigned int num; /* 0-based and monotonic increasing */
 	struct list_head list; /* rtd list of the soc card */
-	struct list_head component_list; /* list of connected components */
 
 	/* bit field */
 	unsigned int pop_wait:1;
 	unsigned int fe_compr:1; /* for Dynamic PCM */
+
+	int num_components;
+	struct snd_soc_component *components[0]; /* CPU/Codec/Platform */
 };
+#define for_each_rtd_components(rtd, i, component)			\
+	for ((i) = 0;							\
+	     ((i) < rtd->num_components) && ((component) = rtd->components[i]);\
+	     (i)++)
 #define for_each_rtd_codec_dai(rtd, i, dai)\
 	for ((i) = 0;						       \
 	     ((i) < rtd->num_codecs) && ((dai) = rtd->codec_dais[i]); \
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 9054558..c06ca7d 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -418,10 +418,10 @@ int snd_soc_pcm_component_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
+	int i;
 
 	/* FIXME: use 1st pointer */
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		if (component->driver->pointer)
 			return component->driver->pointer(component, substream);
 
@@ -433,10 +433,10 @@ int snd_soc_pcm_component_ioctl(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
+	int i;
 
 	/* FIXME: use 1st ioctl */
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		if (component->driver->ioctl)
 			return component->driver->ioctl(component, substream,
 							cmd, arg);
@@ -448,10 +448,9 @@ int snd_soc_pcm_component_sync_stop(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret;
+	int i, ret;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component->driver->ioctl) {
 			ret = component->driver->sync_stop(component,
 							   substream);
@@ -468,11 +467,11 @@ int snd_soc_pcm_component_copy_user(struct snd_pcm_substream *substream,
 				    void __user *buf, unsigned long bytes)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
+	int i;
 
 	/* FIXME. it returns 1st copy now */
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		if (component->driver->copy_user)
 			return component->driver->copy_user(
 				component, substream, channel, pos, buf, bytes);
@@ -484,12 +483,12 @@ struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream,
 					unsigned long offset)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 	struct page *page;
+	int i;
 
 	/* FIXME. it returns 1st page now */
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component->driver->page) {
 			page = component->driver->page(component,
 						       substream, offset);
@@ -505,11 +504,11 @@ int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream,
 			       struct vm_area_struct *vma)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
+	int i;
 
 	/* FIXME. it returns 1st mmap now */
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		if (component->driver->mmap)
 			return component->driver->mmap(component,
 						       substream, vma);
@@ -519,11 +518,11 @@ int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream,
 
 int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 	int ret;
+	int i;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component->driver->pcm_construct) {
 			ret = component->driver->pcm_construct(component, rtd);
 			if (ret < 0)
@@ -536,10 +535,10 @@ int snd_soc_pcm_component_new(struct snd_soc_pcm_runtime *rtd)
 
 void snd_soc_pcm_component_free(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
+	int i;
 
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		if (component->driver->pcm_destruct)
 			component->driver->pcm_destruct(component, rtd->pcm);
 }
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 6615ef6..2e905a7 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -25,10 +25,9 @@ static int soc_compr_components_open(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret;
+	int i, ret;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->open)
 			continue;
@@ -53,9 +52,9 @@ static int soc_compr_components_free(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
+	int i;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component == last)
 			break;
 
@@ -344,10 +343,9 @@ static int soc_compr_components_trigger(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret;
+	int i, ret;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->trigger)
 			continue;
@@ -447,10 +445,9 @@ static int soc_compr_components_set_params(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret;
+	int i, ret;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->set_params)
 			continue;
@@ -579,9 +576,8 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0;
+	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -591,7 +587,7 @@ static int soc_compr_get_params(struct snd_compr_stream *cstream,
 			goto err;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->get_params)
 			continue;
@@ -610,12 +606,11 @@ static int soc_compr_get_caps(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0;
+	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->get_caps)
 			continue;
@@ -633,12 +628,11 @@ static int soc_compr_get_codec_caps(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0;
+	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->get_codec_caps)
 			continue;
@@ -656,9 +650,8 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret = 0;
+	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -668,7 +661,7 @@ static int soc_compr_ack(struct snd_compr_stream *cstream, size_t bytes)
 			goto err;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->ack)
 			continue;
@@ -688,8 +681,7 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0;
+	int i, ret = 0;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
@@ -697,7 +689,7 @@ static int soc_compr_pointer(struct snd_compr_stream *cstream,
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->pointer)
 		cpu_dai->driver->cops->pointer(cstream, tstamp, cpu_dai);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->pointer)
 			continue;
@@ -715,12 +707,11 @@ static int soc_compr_copy(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret = 0;
+	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->copy)
 			continue;
@@ -738,9 +729,8 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret;
+	int i, ret;
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->set_metadata) {
 		ret = cpu_dai->driver->cops->set_metadata(cstream, metadata, cpu_dai);
@@ -748,7 +738,7 @@ static int soc_compr_set_metadata(struct snd_compr_stream *cstream,
 			return ret;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->set_metadata)
 			continue;
@@ -767,9 +757,8 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	int ret;
+	int i, ret;
 
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->get_metadata) {
 		ret = cpu_dai->driver->cops->get_metadata(cstream, metadata, cpu_dai);
@@ -777,7 +766,7 @@ static int soc_compr_get_metadata(struct snd_compr_stream *cstream,
 			return ret;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->get_metadata)
 			continue;
@@ -830,7 +819,6 @@ static struct snd_compr_ops soc_compr_dyn_ops = {
 int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 {
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_compr *compr;
@@ -838,6 +826,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	char new_name[64];
 	int ret = 0, direction = 0;
 	int playback = 0, capture = 0;
+	int i;
 
 	if (rtd->num_codecs > 1) {
 		dev_err(rtd->card->dev,
@@ -906,7 +895,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 		memcpy(compr->ops, &soc_compr_ops, sizeof(soc_compr_ops));
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->compr_ops ||
 		    !component->driver->compr_ops->copy)
 			continue;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0bd2cb2..8e49fb8 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -261,34 +261,18 @@ static inline void snd_soc_debugfs_exit(void)
 static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd,
 			      struct snd_soc_component *component)
 {
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *comp;
+	int i;
 
-	for_each_rtd_components(rtd, rtdcom, comp) {
+	for_each_rtd_components(rtd, i, comp) {
 		/* already connected */
 		if (comp == component)
 			return 0;
 	}
 
-	/*
-	 * created rtdcom here will be freed when rtd->dev was freed.
-	 * see
-	 *	soc_free_pcm_runtime() :: device_unregister(rtd->dev)
-	 */
-	rtdcom = devm_kzalloc(rtd->dev, sizeof(*rtdcom), GFP_KERNEL);
-	if (!rtdcom)
-		return -ENOMEM;
-
-	rtdcom->component = component;
-	INIT_LIST_HEAD(&rtdcom->list);
-
-	/*
-	 * When rtd was freed, created rtdcom here will be
-	 * also freed.
-	 * And we don't need to call list_del(&rtdcom->list)
-	 * when freed, because rtd is also freed.
-	 */
-	list_add_tail(&rtdcom->list, &rtd->component_list);
+	/* see for_each_rtd_components */
+	rtd->components[rtd->num_components] = component;
+	rtd->num_components++;
 
 	return 0;
 }
@@ -296,8 +280,8 @@ static int snd_soc_rtdcom_add(struct snd_soc_pcm_runtime *rtd,
 struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 						const char *driver_name)
 {
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
+	int i;
 
 	if (!driver_name)
 		return NULL;
@@ -310,7 +294,7 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 	 * But, if many components which have same driver name are connected
 	 * to 1 rtd, this function will return 1st found component.
 	 */
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		const char *component_name = component->driver->name;
 
 		if (!component_name)
@@ -318,7 +302,7 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 
 		if ((component_name == driver_name) ||
 		    strcmp(component_name, driver_name) == 0)
-			return rtdcom->component;
+			return component;
 	}
 
 	return NULL;
@@ -418,6 +402,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	struct snd_soc_card *card, struct snd_soc_dai_link *dai_link)
 {
 	struct snd_soc_pcm_runtime *rtd;
+	struct snd_soc_component *component;
 	struct device *dev;
 	int ret;
 
@@ -443,7 +428,12 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	/*
 	 * for rtd
 	 */
-	rtd = devm_kzalloc(dev, sizeof(*rtd), GFP_KERNEL);
+	rtd = devm_kzalloc(dev,
+			   sizeof(*rtd) +
+			   sizeof(*component) * (dai_link->num_cpus +
+						 dai_link->num_codecs +
+						 dai_link->num_platforms),
+			   GFP_KERNEL);
 	if (!rtd)
 		goto free_rtd;
 
@@ -463,7 +453,6 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	/*
 	 * rtd remaining settings
 	 */
-	INIT_LIST_HEAD(&rtd->component_list);
 	INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].be_clients);
 	INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_CAPTURE].be_clients);
 	INIT_LIST_HEAD(&rtd->dpcm[SNDRV_PCM_STREAM_PLAYBACK].fe_clients);
@@ -1108,9 +1097,8 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card,
 {
 	struct snd_soc_dai_link *dai_link = rtd->dai_link;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
-	int ret, num;
+	int ret, num, i;
 
 	/* set default power off timeout */
 	rtd->pmdown_time = pmdown_time;
@@ -1141,7 +1129,7 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card,
 	 * topology based drivers can use the DAI link id field to set PCM
 	 * device number and then use rtd + a base offset of the BEs.
 	 */
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (!component->driver->use_dai_pcm_id)
 			continue;
 
@@ -1406,12 +1394,11 @@ static void soc_remove_link_components(struct snd_soc_card *card)
 {
 	struct snd_soc_component *component;
 	struct snd_soc_pcm_runtime *rtd;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int order;
+	int i, order;
 
 	for_each_comp_order(order) {
 		for_each_card_rtds(card, rtd) {
-			for_each_rtd_components(rtd, rtdcom, component) {
+			for_each_rtd_components(rtd, i, component) {
 				if (component->driver->remove_order != order)
 					continue;
 
@@ -1425,12 +1412,11 @@ static int soc_probe_link_components(struct snd_soc_card *card)
 {
 	struct snd_soc_component *component;
 	struct snd_soc_pcm_runtime *rtd;
-	struct snd_soc_rtdcom_list *rtdcom;
-	int ret, order;
+	int i, ret, order;
 
 	for_each_comp_order(order) {
 		for_each_card_rtds(card, rtd) {
-			for_each_rtd_components(rtd, rtdcom, component) {
+			for_each_rtd_components(rtd, i, component) {
 				if (component->driver->probe_order != order)
 					continue;
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 01e7bc0..9c6c753 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -111,14 +111,14 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream)
  */
 bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd)
 {
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 	bool ignore = true;
+	int i;
 
 	if (!rtd->pmdown_time || rtd->dai_link->ignore_pmdown_time)
 		return true;
 
-	for_each_rtd_components(rtd, rtdcom, component)
+	for_each_rtd_components(rtd, i, component)
 		ignore &= !component->driver->use_pmdown_time;
 
 	return ignore;
@@ -428,11 +428,10 @@ static int soc_pcm_components_open(struct snd_pcm_substream *substream,
 				   struct snd_soc_component **last)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
-	int ret = 0;
+	int i, ret = 0;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		*last = component;
 
 		ret = snd_soc_component_module_get_when_open(component);
@@ -459,11 +458,10 @@ static int soc_pcm_components_close(struct snd_pcm_substream *substream,
 				    struct snd_soc_component *last)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
-	int ret = 0;
+	int i, ret = 0;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component == last)
 			break;
 
@@ -484,7 +482,6 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	const char *codec_dai_name = "multicodec";
@@ -494,9 +491,8 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	for_each_rtd_codec_dai(rtd, i, codec_dai)
 		pinctrl_pm_select_default_state(codec_dai->dev);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component)
 		pm_runtime_get_sync(component->dev);
-	}
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
@@ -617,7 +613,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		pm_runtime_mark_last_busy(component->dev);
 		pm_runtime_put_autosuspend(component->dev);
 	}
@@ -677,7 +673,6 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	int i;
@@ -728,7 +723,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		pm_runtime_mark_last_busy(component->dev);
 		pm_runtime_put_autosuspend(component->dev);
 	}
@@ -752,7 +747,6 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	int i, ret = 0;
@@ -768,7 +762,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 		}
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		ret = snd_soc_component_prepare(component, substream);
 		if (ret < 0) {
 			dev_err(component->dev,
@@ -829,11 +823,10 @@ static int soc_pcm_components_hw_free(struct snd_pcm_substream *substream,
 				      struct snd_soc_component *last)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
-	int ret = 0;
+	int i, ret = 0;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		if (component == last)
 			break;
 
@@ -853,7 +846,6 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	int i, ret = 0;
@@ -932,7 +924,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 
 	snd_soc_dapm_update_dai(substream, params, cpu_dai);
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		ret = snd_soc_component_hw_params(component, substream, params);
 		if (ret < 0) {
 			dev_err(component->dev,
@@ -1033,7 +1025,6 @@ static int soc_pcm_trigger_start(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	int i, ret;
@@ -1044,7 +1035,7 @@ static int soc_pcm_trigger_start(struct snd_pcm_substream *substream, int cmd)
 			return ret;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		ret = snd_soc_component_trigger(component, substream, cmd);
 		if (ret < 0)
 			return ret;
@@ -1067,7 +1058,6 @@ static int soc_pcm_trigger_stop(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
 	struct snd_soc_dai *codec_dai;
 	int i, ret;
@@ -1082,7 +1072,7 @@ static int soc_pcm_trigger_stop(struct snd_pcm_substream *substream, int cmd)
 	if (ret < 0)
 		return ret;
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		ret = snd_soc_component_trigger(component, substream, cmd);
 		if (ret < 0)
 			return ret;
@@ -2897,7 +2887,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 {
 	struct snd_soc_dai *codec_dai;
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-	struct snd_soc_rtdcom_list *rtdcom;
 	struct snd_soc_component *component;
 	struct snd_pcm *pcm;
 	char new_name[64];
@@ -3007,7 +2996,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 		rtd->ops.pointer	= soc_pcm_pointer;
 	}
 
-	for_each_rtd_components(rtd, rtdcom, component) {
+	for_each_rtd_components(rtd, i, component) {
 		const struct snd_soc_component_driver *drv = component->driver;
 
 		if (drv->ioctl)
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 2/6] ASoC: soc-core: care .ignore_suspend for Component suspend
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
  2019-12-17  1:25 ` [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list Kuninori Morimoto
@ 2019-12-17  1:25 ` Kuninori Morimoto
  2019-12-17  1:26 ` [alsa-devel] [PATCH 3/6] ASoC: soc-core: remove duplicate pinctrl operation when suspend Kuninori Morimoto
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:25 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Card dai_link has .ignore_suspend, and ALSA SoC cares it when suspend.
For example, like this

	for_each_card_rtds(card, rtd) {
		if (rtd->dai_link->ignore_suspend)
			continue;
		...
	}

But in snd_soc_suspend(), it doesn't care about
it when suspending Component. This patch cares it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8e49fb8..c10b9af 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -562,15 +562,25 @@ int snd_soc_suspend(struct device *dev)
 	snd_soc_dapm_sync(&card->dapm);
 
 	/* suspend all COMPONENTs */
-	for_each_card_components(card, component) {
-		struct snd_soc_dapm_context *dapm =
+	for_each_card_rtds(card, rtd) {
+
+		if (rtd->dai_link->ignore_suspend)
+			continue;
+
+		for_each_rtd_components(rtd, i, component) {
+			struct snd_soc_dapm_context *dapm =
 				snd_soc_component_get_dapm(component);
 
-		/*
-		 * If there are paths active then the COMPONENT will be held
-		 * with bias _ON and should not be suspended.
-		 */
-		if (!snd_soc_component_is_suspended(component)) {
+			/*
+			 * ignore if component was already suspended
+			 */
+			if (snd_soc_component_is_suspended(component))
+				continue;
+
+			/*
+			 * If there are paths active then the COMPONENT will be
+			 * held with bias _ON and should not be suspended.
+			 */
 			switch (snd_soc_dapm_get_bias_level(dapm)) {
 			case SND_SOC_BIAS_STANDBY:
 				/*
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 3/6] ASoC: soc-core: remove duplicate pinctrl operation when suspend
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
  2019-12-17  1:25 ` [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list Kuninori Morimoto
  2019-12-17  1:25 ` [alsa-devel] [PATCH 2/6] ASoC: soc-core: care .ignore_suspend for Component suspend Kuninori Morimoto
@ 2019-12-17  1:26 ` Kuninori Morimoto
  2019-12-17  1:26 ` [alsa-devel] [PATCH 4/6] ASoC: soc-core: do pinctrl_pm_select_xxx() as component Kuninori Morimoto
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:26 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

snd_soc_suspend() are doing below for pinctrl_pm_select_sleep_state()

	int snd_soc_suspend(struct device *dev)
	{
		...
		for_each_card_components(card, component) {
			...
(1)			pinctrl_pm_select_sleep_state(component->dev);
		}

		for_each_card_rtds(card, rtd) {
			...
(2)			pinctrl_pm_select_sleep_state(cpu_dai->dev);
		}
	}

(1) is called for all component (CPU/Codec/Platform), and
(2) is called for CPU DAIs.
Here, component->dev is same as dai->dev.
This means, it is called in duplicate on CPU case.
This patch removes (2).

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c10b9af..9b94a5a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -619,9 +619,6 @@ int snd_soc_suspend(struct device *dev)
 
 		if (cpu_dai->driver->bus_control)
 			snd_soc_dai_suspend(cpu_dai);
-
-		/* deactivate pins to sleep state */
-		pinctrl_pm_select_sleep_state(cpu_dai->dev);
 	}
 
 	if (card->suspend_post)
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 4/6] ASoC: soc-core: do pinctrl_pm_select_xxx() as component
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2019-12-17  1:26 ` [alsa-devel] [PATCH 3/6] ASoC: soc-core: remove duplicate pinctrl operation when suspend Kuninori Morimoto
@ 2019-12-17  1:26 ` Kuninori Morimoto
  2019-12-17  1:26 ` [alsa-devel] [PATCH 5/6] ASoC: soc-core: add snd_soc_close_delayed_work() Kuninori Morimoto
  2019-12-17  1:26 ` [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop() Kuninori Morimoto
  5 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:26 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ALSA SoC need to care pinctrl_pm_select_xxx().
It is called at soc-core and soc-pcm.
soc-pcm  is controlling it for activate DAI.
soc-core is controlling it for whole system
(= suspend/resume/probe/poweroff).

If we focus to soc-core side, it need to care about BIAS level.
Then, snd_soc_suspend() only is controlling it by Component base (a).
Other functions are DAI base (b).

(a)	pinctrl_pm_select_xxx(component->dev, xxx);
(b)	pinctrl_pm_select_xxx(dai->dev, xxx);

Because of these unbalance, the code is confusable.
Here, dai->dev and component->dev are same pointer.
Thus, we can replace it component base.

One note here is that it cared DAI (= CPU/Codec) pin before this patch,
after this patch, it cares Component (= CPU/Codec/Platform) pin.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 46 +++++++++++-----------------------------------
 sound/soc/soc-pcm.c  | 23 ++++++++---------------
 2 files changed, 19 insertions(+), 50 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 9b94a5a..b86d5c5 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -727,25 +727,16 @@ int snd_soc_resume(struct device *dev)
 	struct snd_soc_card *card = dev_get_drvdata(dev);
 	bool bus_control = false;
 	struct snd_soc_pcm_runtime *rtd;
-	struct snd_soc_dai *codec_dai;
-	int i;
+	struct snd_soc_component *component;
 
 	/* If the card is not initialized yet there is nothing to do */
 	if (!card->instantiated)
 		return 0;
 
 	/* activate pins from sleep state */
-	for_each_card_rtds(card, rtd) {
-		struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-
-		if (cpu_dai->active)
-			pinctrl_pm_select_default_state(cpu_dai->dev);
-
-		for_each_rtd_codec_dai(rtd, i, codec_dai) {
-			if (codec_dai->active)
-				pinctrl_pm_select_default_state(codec_dai->dev);
-		}
-	}
+	for_each_card_components(card, component)
+		if (component->active)
+			pinctrl_pm_select_default_state(component->dev);
 
 	/*
 	 * DAIs that also act as the control bus master might have other drivers
@@ -1883,6 +1874,7 @@ static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
 static int snd_soc_bind_card(struct snd_soc_card *card)
 {
 	struct snd_soc_pcm_runtime *rtd;
+	struct snd_soc_component *component;
 	struct snd_soc_dai_link *dai_link;
 	int ret, i, card_probed = 0;
 
@@ -2034,17 +2026,9 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 	snd_soc_dapm_sync(&card->dapm);
 
 	/* deactivate pins to sleep state */
-	for_each_card_rtds(card, rtd) {
-		struct snd_soc_dai *dai;
-
-		for_each_rtd_codec_dai(rtd, i, dai) {
-			if (!dai->active)
-				pinctrl_pm_select_sleep_state(dai->dev);
-		}
-
-		if (!rtd->cpu_dai->active)
-			pinctrl_pm_select_sleep_state(rtd->cpu_dai->dev);
-	}
+	for_each_card_components(card, component)
+		if (!component->active)
+			pinctrl_pm_select_sleep_state(component->dev);
 
 probe_end:
 	if (ret < 0)
@@ -2090,7 +2074,7 @@ static int soc_remove(struct platform_device *pdev)
 int snd_soc_poweroff(struct device *dev)
 {
 	struct snd_soc_card *card = dev_get_drvdata(dev);
-	struct snd_soc_pcm_runtime *rtd;
+	struct snd_soc_component *component;
 
 	if (!card->instantiated)
 		return 0;
@@ -2104,16 +2088,8 @@ int snd_soc_poweroff(struct device *dev)
 	snd_soc_dapm_shutdown(card);
 
 	/* deactivate pins to sleep state */
-	for_each_card_rtds(card, rtd) {
-		struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-		struct snd_soc_dai *codec_dai;
-		int i;
-
-		pinctrl_pm_select_sleep_state(cpu_dai->dev);
-		for_each_rtd_codec_dai(rtd, i, codec_dai) {
-			pinctrl_pm_select_sleep_state(codec_dai->dev);
-		}
-	}
+	for_each_card_components(card, component)
+		pinctrl_pm_select_sleep_state(component->dev);
 
 	return 0;
 }
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 9c6c753..68f7205 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -487,9 +487,8 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	const char *codec_dai_name = "multicodec";
 	int i, ret = 0;
 
-	pinctrl_pm_select_default_state(cpu_dai->dev);
-	for_each_rtd_codec_dai(rtd, i, codec_dai)
-		pinctrl_pm_select_default_state(codec_dai->dev);
+	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);
@@ -618,12 +617,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 		pm_runtime_put_autosuspend(component->dev);
 	}
 
-	for_each_rtd_codec_dai(rtd, i, codec_dai) {
-		if (!codec_dai->active)
-			pinctrl_pm_select_sleep_state(codec_dai->dev);
-	}
-	if (!cpu_dai->active)
-		pinctrl_pm_select_sleep_state(cpu_dai->dev);
+	for_each_rtd_components(rtd, i, component)
+		if (!component->active)
+			pinctrl_pm_select_sleep_state(component->dev);
 
 	return ret;
 }
@@ -728,12 +724,9 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 		pm_runtime_put_autosuspend(component->dev);
 	}
 
-	for_each_rtd_codec_dai(rtd, i, codec_dai) {
-		if (!codec_dai->active)
-			pinctrl_pm_select_sleep_state(codec_dai->dev);
-	}
-	if (!cpu_dai->active)
-		pinctrl_pm_select_sleep_state(cpu_dai->dev);
+	for_each_rtd_components(rtd, i, component)
+		if (!component->active)
+			pinctrl_pm_select_sleep_state(component->dev);
 
 	return 0;
 }
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 5/6] ASoC: soc-core: add snd_soc_close_delayed_work()
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2019-12-17  1:26 ` [alsa-devel] [PATCH 4/6] ASoC: soc-core: do pinctrl_pm_select_xxx() as component Kuninori Morimoto
@ 2019-12-17  1:26 ` Kuninori Morimoto
  2019-12-17  1:26 ` [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop() Kuninori Morimoto
  5 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:26 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We need to setup rtd->close_delayed_work_func.
It will be set at snd_soc_dai_compress_new() or soc_new_pcm().
But these setups close_delayed_work() which is same name /
same implemantaion, but different local code.
To reduce duplicate code, this patch moves it as
snd_soc_close_delayed_work() and share same code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h      |  1 +
 sound/soc/soc-compress.c | 29 +----------------------------
 sound/soc/soc-core.c     | 28 ++++++++++++++++++++++++++++
 sound/soc/soc-pcm.c      | 28 +---------------------------
 4 files changed, 31 insertions(+), 55 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 0513f30..f0e4f36 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1159,6 +1159,7 @@ struct snd_soc_pcm_runtime {
 #define for_each_rtd_codec_dai_rollback(rtd, i, dai)		\
 	for (; ((--i) >= 0) && ((dai) = rtd->codec_dais[i]);)
 
+void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
 
 /* mixer control */
 struct soc_mixer_control {
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 2e905a7..7bd574f 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -208,33 +208,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	return ret;
 }
 
-/*
- * Power down the audio subsystem pmdown_time msecs after close is called.
- * This is to ensure there are no pops or clicks in between any music tracks
- * due to DAPM power cycling.
- */
-static void close_delayed_work(struct snd_soc_pcm_runtime *rtd)
-{
-	struct snd_soc_dai *codec_dai = rtd->codec_dai;
-
-	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
-
-	dev_dbg(rtd->dev,
-		"Compress ASoC: pop wq checking: %s status: %s waiting: %s\n",
-		codec_dai->driver->playback.stream_name,
-		codec_dai->playback_active ? "active" : "inactive",
-		rtd->pop_wait ? "yes" : "no");
-
-	/* are we waiting on this codec DAI stream */
-	if (rtd->pop_wait == 1) {
-		rtd->pop_wait = 0;
-		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
-					  SND_SOC_DAPM_STREAM_STOP);
-	}
-
-	mutex_unlock(&rtd->card->pcm_mutex);
-}
-
 static int soc_compr_free(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
@@ -916,7 +889,7 @@ int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
 	}
 
 	/* DAPM dai link stream work */
-	rtd->close_delayed_work_func = close_delayed_work;
+	rtd->close_delayed_work_func = snd_soc_close_delayed_work;
 
 	rtd->compr = compr;
 	compr->private_data = rtd;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b86d5c5..f8ed927 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -359,6 +359,34 @@ struct snd_soc_pcm_runtime
 }
 EXPORT_SYMBOL_GPL(snd_soc_get_pcm_runtime);
 
+/*
+ * Power down the audio subsystem pmdown_time msecs after close is called.
+ * This is to ensure there are no pops or clicks in between any music tracks
+ * due to DAPM power cycling.
+ */
+void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+
+	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
+
+	dev_dbg(rtd->dev,
+		"ASoC: pop wq checking: %s status: %s waiting: %s\n",
+		codec_dai->driver->playback.stream_name,
+		codec_dai->playback_active ? "active" : "inactive",
+		rtd->pop_wait ? "yes" : "no");
+
+	/* are we waiting on this codec DAI stream */
+	if (rtd->pop_wait == 1) {
+		rtd->pop_wait = 0;
+		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
+					  SND_SOC_DAPM_STREAM_STOP);
+	}
+
+	mutex_unlock(&rtd->card->pcm_mutex);
+}
+EXPORT_SYMBOL_GPL(snd_soc_close_delayed_work);
+
 static void soc_release_rtd_dev(struct device *dev)
 {
 	/* "dev" means "rtd->dev" */
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 68f7205..ad908e0 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -624,32 +624,6 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	return ret;
 }
 
-/*
- * Power down the audio subsystem pmdown_time msecs after close is called.
- * This is to ensure there are no pops or clicks in between any music tracks
- * due to DAPM power cycling.
- */
-static void close_delayed_work(struct snd_soc_pcm_runtime *rtd)
-{
-	struct snd_soc_dai *codec_dai = rtd->codec_dais[0];
-
-	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
-
-	dev_dbg(rtd->dev, "ASoC: pop wq checking: %s status: %s waiting: %s\n",
-		 codec_dai->driver->playback.stream_name,
-		 codec_dai->playback_active ? "active" : "inactive",
-		 rtd->pop_wait ? "yes" : "no");
-
-	/* are we waiting on this codec DAI stream */
-	if (rtd->pop_wait == 1) {
-		rtd->pop_wait = 0;
-		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_PLAYBACK,
-					  SND_SOC_DAPM_STREAM_STOP);
-	}
-
-	mutex_unlock(&rtd->card->pcm_mutex);
-}
-
 static void codec2codec_close_delayed_work(struct snd_soc_pcm_runtime *rtd)
 {
 	/*
@@ -2956,7 +2930,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 	if (rtd->dai_link->params)
 		rtd->close_delayed_work_func = codec2codec_close_delayed_work;
 	else
-		rtd->close_delayed_work_func = close_delayed_work;
+		rtd->close_delayed_work_func = snd_soc_close_delayed_work;
 
 	pcm->nonatomic = rtd->dai_link->nonatomic;
 	rtd->pcm = pcm;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
                   ` (4 preceding siblings ...)
  2019-12-17  1:26 ` [alsa-devel] [PATCH 5/6] ASoC: soc-core: add snd_soc_close_delayed_work() Kuninori Morimoto
@ 2019-12-17  1:26 ` Kuninori Morimoto
  2019-12-17  3:22   ` Pierre-Louis Bossart
  5 siblings, 1 reply; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  1:26 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

When we stop stream, if it was Playback, we might need to care
about power down time. In such case, we need to use delayed work.

We have same implementation for it at soc-pcm.c and soc-compress.c,
but we don't want to have duplicate code.
This patch adds snd_soc_stream_stop(), and share same code.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h      |  1 +
 sound/soc/soc-compress.c | 18 +-----------------
 sound/soc/soc-core.c     | 23 +++++++++++++++++++++++
 sound/soc/soc-pcm.c      | 19 +------------------
 4 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f0e4f36..575ec11 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1160,6 +1160,7 @@ struct snd_soc_pcm_runtime {
 	for (; ((--i) >= 0) && ((dai) = rtd->codec_dais[i]);)
 
 void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
+void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream);
 
 /* mixer control */
 struct soc_mixer_control {
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 7bd574f..eb19df3 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -240,23 +240,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (cpu_dai->driver->cops && cpu_dai->driver->cops->shutdown)
 		cpu_dai->driver->cops->shutdown(cstream, cpu_dai);
 
-	if (cstream->direction == SND_COMPRESS_PLAYBACK) {
-		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
-			snd_soc_dapm_stream_event(rtd,
-						  SNDRV_PCM_STREAM_PLAYBACK,
-						  SND_SOC_DAPM_STREAM_STOP);
-		} else {
-			rtd->pop_wait = 1;
-			queue_delayed_work(system_power_efficient_wq,
-					   &rtd->delayed_work,
-					   msecs_to_jiffies(rtd->pmdown_time));
-		}
-	} else {
-		/* capture streams can be powered down now */
-		snd_soc_dapm_stream_event(rtd,
-					  SNDRV_PCM_STREAM_CAPTURE,
-					  SND_SOC_DAPM_STREAM_STOP);
-	}
+	snd_soc_stream_stop(rtd, stream);
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 	return 0;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f8ed927..28ebfad 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -387,6 +387,29 @@ void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd)
 }
 EXPORT_SYMBOL_GPL(snd_soc_close_delayed_work);
 
+void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream)
+{
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
+			/* powered down playback stream now */
+			snd_soc_dapm_stream_event(rtd,
+						  SNDRV_PCM_STREAM_PLAYBACK,
+						  SND_SOC_DAPM_STREAM_STOP);
+		} else {
+			/* start delayed pop wq here for playback streams */
+			rtd->pop_wait = 1;
+			queue_delayed_work(system_power_efficient_wq,
+					   &rtd->delayed_work,
+					   msecs_to_jiffies(rtd->pmdown_time));
+		}
+	} else {
+		/* capture streams can be powered down now */
+		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
+					  SND_SOC_DAPM_STREAM_STOP);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_soc_stream_stop);
+
 static void soc_release_rtd_dev(struct device *dev)
 {
 	/* "dev" means "rtd->dev" */
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index ad908e0..c9ff50e 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -672,24 +672,7 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 
 	soc_pcm_components_close(substream, NULL);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
-			/* powered down playback stream now */
-			snd_soc_dapm_stream_event(rtd,
-						  SNDRV_PCM_STREAM_PLAYBACK,
-						  SND_SOC_DAPM_STREAM_STOP);
-		} else {
-			/* start delayed pop wq here for playback streams */
-			rtd->pop_wait = 1;
-			queue_delayed_work(system_power_efficient_wq,
-					   &rtd->delayed_work,
-					   msecs_to_jiffies(rtd->pmdown_time));
-		}
-	} else {
-		/* capture streams can be powered down now */
-		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
-					  SND_SOC_DAPM_STREAM_STOP);
-	}
+	snd_soc_stream_stop(rtd, substream->stream);
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17  1:26 ` [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop() Kuninori Morimoto
@ 2019-12-17  3:22   ` Pierre-Louis Bossart
  2019-12-17  3:54     ` Kuninori Morimoto
  0 siblings, 1 reply; 14+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-17  3:22 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA

Hi Morimoto-san,


> +void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream);

> +void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream)
> +{
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
> +			/* powered down playback stream now */
> +			snd_soc_dapm_stream_event(rtd,
> +						  SNDRV_PCM_STREAM_PLAYBACK,
> +						  SND_SOC_DAPM_STREAM_STOP);
> +		} else {
> +			/* start delayed pop wq here for playback streams */
> +			rtd->pop_wait = 1;
> +			queue_delayed_work(system_power_efficient_wq,
> +					   &rtd->delayed_work,
> +					   msecs_to_jiffies(rtd->pmdown_time));
> +		}
> +	} else {
> +		/* capture streams can be powered down now */
> +		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
> +					  SND_SOC_DAPM_STREAM_STOP);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_stream_stop);

I am not a big fan of naming conventions for the new helper. We don't 
have any other snd_soc_stream function, but we have a concept of stream 
for SoundWire. we also have snd_soc_dapm_stream_ functions..

snd_soc_dapm_stream_stop() maybe?

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17  3:22   ` Pierre-Louis Bossart
@ 2019-12-17  3:54     ` Kuninori Morimoto
  2019-12-17  4:41       ` Kuninori Morimoto
  0 siblings, 1 reply; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  3:54 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown


Hi Pierre-Louis

Thank you for your review

> > +void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream)
> > +{
> > +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > +		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
> > +			/* powered down playback stream now */
> > +			snd_soc_dapm_stream_event(rtd,
> > +						  SNDRV_PCM_STREAM_PLAYBACK,
> > +						  SND_SOC_DAPM_STREAM_STOP);
> > +		} else {
> > +			/* start delayed pop wq here for playback streams */
> > +			rtd->pop_wait = 1;
> > +			queue_delayed_work(system_power_efficient_wq,
> > +					   &rtd->delayed_work,
> > +					   msecs_to_jiffies(rtd->pmdown_time));
> > +		}
> > +	} else {
> > +		/* capture streams can be powered down now */
> > +		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
> > +					  SND_SOC_DAPM_STREAM_STOP);
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(snd_soc_stream_stop);
> 
> I am not a big fan of naming conventions for the new helper. We don't
> have any other snd_soc_stream function, but we have a concept of
> stream for SoundWire. we also have snd_soc_dapm_stream_ functions..
> 
> snd_soc_dapm_stream_stop() maybe?

Ahh, make sense !
I will post v2 tomorrow

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17  3:54     ` Kuninori Morimoto
@ 2019-12-17  4:41       ` Kuninori Morimoto
  2019-12-17 12:13         ` Mark Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-17  4:41 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Mark Brown; +Cc: Linux-ALSA


Hi

> > > +void snd_soc_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream)
> > > +{
> > > +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> > > +		if (snd_soc_runtime_ignore_pmdown_time(rtd)) {
> > > +			/* powered down playback stream now */
> > > +			snd_soc_dapm_stream_event(rtd,
> > > +						  SNDRV_PCM_STREAM_PLAYBACK,
> > > +						  SND_SOC_DAPM_STREAM_STOP);
> > > +		} else {
> > > +			/* start delayed pop wq here for playback streams */
> > > +			rtd->pop_wait = 1;
> > > +			queue_delayed_work(system_power_efficient_wq,
> > > +					   &rtd->delayed_work,
> > > +					   msecs_to_jiffies(rtd->pmdown_time));
> > > +		}
> > > +	} else {
> > > +		/* capture streams can be powered down now */
> > > +		snd_soc_dapm_stream_event(rtd, SNDRV_PCM_STREAM_CAPTURE,
> > > +					  SND_SOC_DAPM_STREAM_STOP);
> > > +	}
> > > +}
> > > +EXPORT_SYMBOL_GPL(snd_soc_stream_stop);

Hmm...
I noticed that soc_compr_free_fe(), dpcm_fe_dai_shutdown() are directly
calling SND_SOC_DAPM_STREAM_STOP without caring pmdown time / delayed work.
Can we use snd_soc_dapm_stream_stop() for these, too ?

	static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream)
	{
		...
		/* run the stream event for each BE */
=>		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
		...
	}

	static int soc_compr_free_fe(struct snd_compr_stream *cstream)
	{
		...
=>		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
		...
	}

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17  4:41       ` Kuninori Morimoto
@ 2019-12-17 12:13         ` Mark Brown
  2019-12-17 13:56           ` Pierre-Louis Bossart
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2019-12-17 12:13 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Pierre-Louis Bossart


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

On Tue, Dec 17, 2019 at 01:41:09PM +0900, Kuninori Morimoto wrote:

> I noticed that soc_compr_free_fe(), dpcm_fe_dai_shutdown() are directly
> calling SND_SOC_DAPM_STREAM_STOP without caring pmdown time / delayed work.
> Can we use snd_soc_dapm_stream_stop() for these, too ?

That does seem like an oversight...  

[-- 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17 12:13         ` Mark Brown
@ 2019-12-17 13:56           ` Pierre-Louis Bossart
  2019-12-18  1:51             ` Kuninori Morimoto
  2019-12-18 11:50             ` Mark Brown
  0 siblings, 2 replies; 14+ messages in thread
From: Pierre-Louis Bossart @ 2019-12-17 13:56 UTC (permalink / raw)
  To: Mark Brown, Kuninori Morimoto; +Cc: Linux-ALSA

On 12/17/19 6:13 AM, Mark Brown wrote:
> On Tue, Dec 17, 2019 at 01:41:09PM +0900, Kuninori Morimoto wrote:
> 
>> I noticed that soc_compr_free_fe(), dpcm_fe_dai_shutdown() are directly
>> calling SND_SOC_DAPM_STREAM_STOP without caring pmdown time / delayed work.
>> Can we use snd_soc_dapm_stream_stop() for these, too ?
> 
> That does seem like an oversight...

What would the rationale for deferring a FE shutdown though?

IIRC in the Intel machine drivers we only use .pm_ignore_downtime to 
backends to keep some clocks on, and even that is questionable (the 
clock dependencies should be properly modeled instead of hidden with an 
obscure dailink setting). When I asked why exactly this flag was set in 
most cases the reason why 'because others did so in the past'...


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17 13:56           ` Pierre-Louis Bossart
@ 2019-12-18  1:51             ` Kuninori Morimoto
  2019-12-18 11:50             ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2019-12-18  1:51 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown


Hi Pierre-Louis

> >> I noticed that soc_compr_free_fe(), dpcm_fe_dai_shutdown() are directly
> >> calling SND_SOC_DAPM_STREAM_STOP without caring pmdown time / delayed work.
> >> Can we use snd_soc_dapm_stream_stop() for these, too ?
> > 
> > That does seem like an oversight...
> 
> What would the rationale for deferring a FE shutdown though?
> 
> IIRC in the Intel machine drivers we only use .pm_ignore_downtime to
> backends to keep some clocks on, and even that is questionable (the
> clock dependencies should be properly modeled instead of hidden with
> an obscure dailink setting). When I asked why exactly this flag was
> set in most cases the reason why 'because others did so in the
> past'...

Not only "ignore_pmdown_time", but many flags are used
for such reasons, I guess...
But, it is "driver side setting issue", and same things happen
even if we have properly modeled clock dependencies, I guess unfortunately...

From framework side point of view, it should care settings anyway
if it was added/selected even though if it was added by copy-and-paste.
"Following settings case-by-case" is a little bit strange for me.
Maybe FE which has ignore_pmdown_time has some rationale, somehow ?

So, I will post v2 which renames function name that is pointed from you.
And, post additional patch which cares above +2 STREAM_STOP.
Feel free to post NACK it if you don't like it.
Then, Mark can just ignore it.

Thank you for your help !!
Best regards
---
Kuninori Morimoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()
  2019-12-17 13:56           ` Pierre-Louis Bossart
  2019-12-18  1:51             ` Kuninori Morimoto
@ 2019-12-18 11:50             ` Mark Brown
  1 sibling, 0 replies; 14+ messages in thread
From: Mark Brown @ 2019-12-18 11:50 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Kuninori Morimoto


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

On Tue, Dec 17, 2019 at 07:56:28AM -0600, Pierre-Louis Bossart wrote:
> On 12/17/19 6:13 AM, Mark Brown wrote:
> > On Tue, Dec 17, 2019 at 01:41:09PM +0900, Kuninori Morimoto wrote:

> > > Can we use snd_soc_dapm_stream_stop() for these, too ?

> > That does seem like an oversight...

> What would the rationale for deferring a FE shutdown though?

> IIRC in the Intel machine drivers we only use .pm_ignore_downtime to
> backends to keep some clocks on, and even that is questionable (the clock
> dependencies should be properly modeled instead of hidden with an obscure
> dailink setting). When I asked why exactly this flag was set in most cases
> the reason why 'because others did so in the past'...

It's there as a hack to keep clocks live or to avoid pops and clicks as
the datastream goes to zero during shutdown if the CODEC doesn't support
mute or otherwise handle the input going away cleanly (which is just a
more general case of keeping the clocks live usually).  You'd do it for
front ends as well to make sure things are consistent.

[-- 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-12-18 11:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17  1:25 [alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8 Kuninori Morimoto
2019-12-17  1:25 ` [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list Kuninori Morimoto
2019-12-17  1:25 ` [alsa-devel] [PATCH 2/6] ASoC: soc-core: care .ignore_suspend for Component suspend Kuninori Morimoto
2019-12-17  1:26 ` [alsa-devel] [PATCH 3/6] ASoC: soc-core: remove duplicate pinctrl operation when suspend Kuninori Morimoto
2019-12-17  1:26 ` [alsa-devel] [PATCH 4/6] ASoC: soc-core: do pinctrl_pm_select_xxx() as component Kuninori Morimoto
2019-12-17  1:26 ` [alsa-devel] [PATCH 5/6] ASoC: soc-core: add snd_soc_close_delayed_work() Kuninori Morimoto
2019-12-17  1:26 ` [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop() Kuninori Morimoto
2019-12-17  3:22   ` Pierre-Louis Bossart
2019-12-17  3:54     ` Kuninori Morimoto
2019-12-17  4:41       ` Kuninori Morimoto
2019-12-17 12:13         ` Mark Brown
2019-12-17 13:56           ` Pierre-Louis Bossart
2019-12-18  1:51             ` Kuninori Morimoto
2019-12-18 11:50             ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.