[alsa-devel] [PATCH 0/7 v2] ASoC: soc-core cleanup step8
Hi Mark
These are v2 of soc-core cleanup step8. There is no relationship between each patches.
- [6/7] renamed function name - [7/7] new patch which cares SND_SOC_DAPM_STREAM_STOP
Kuninori Morimoto (7): 1) ASoC: soc-core: remove snd_soc_rtdcom_list 2) ASoC: soc-core: care .ignore_suspend for Component suspend 3) ASoC: soc-core: remove duplicate pinctrl operation when suspend 4) ASoC: soc-core: do pinctrl_pm_select_xxx() as component 5) ASoC: soc-core: add snd_soc_close_delayed_work() 6) ASoC: soc-dapm: add snd_soc_dapm_stream_stop() 7) ASoC: use snd_soc_dapm_stream_stop() for SND_SOC_DAPM_STREAM_STOP
include/sound/soc-dapm.h | 1 + include/sound/soc.h | 19 +++--- sound/soc/soc-component.c | 33 +++++----- sound/soc/soc-compress.c | 112 +++++++++------------------------ sound/soc/soc-core.c | 157 +++++++++++++++++++++++----------------------- sound/soc/soc-dapm.c | 23 +++++++ sound/soc/soc-pcm.c | 115 ++++++++------------------------- 7 files changed, 181 insertions(+), 279 deletions(-)
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 --- v2 -> v3
- no change
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)
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)
Morimoto-san, Thanks for this change. This is a lot more intuitive. But may I suggest that we should also possibly consider changing the function name above? How about snd_soc_rtd_add_component() maybe?
Thanks, Ranjani
{
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; }
Hi Sridharan
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)Morimoto-san, Thanks for this change. This is a lot more intuitive. But may I suggest that we should also possibly consider changing the function name above? How about snd_soc_rtd_add_component() maybe?
Nice idea :) To avoid multi-features in 1 patch, I want to use additional patch for rename. Is that OK ? If so, I will post it if Mark applied it.
Thank you for your help !! Best regards --- Kuninori Morimoto
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 --- v2 -> v3
- no change
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: /*
On Tue, Dec 17, 2019 at 6:47 PM Kuninori Morimoto < kuninori.morimoto.gx@renesas.com> wrote:
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.
Morimoto-san,
I am a bit skeptical about this change but I could be wrong. I am not sure if the ignore_suspend field in the DAI link definitions was meant to be applicable for the components as well. Mark/Takashi would have to confirm this.
Thanks, Ranjani
Hi Sridharan Cc Mark
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.Morimoto-san,
I am a bit skeptical about this change but I could be wrong. I am not sure if the ignore_suspend field in the DAI link definitions was meant to be applicable for the components as well. Mark/Takashi would have to confirm this.
Hmm.. from naming point of view, it has no problem, but I'm not sure detail, either.
It is just one of cleanup patch, thus, I have no big objection if it was not accepted.
Mark I think this patch doesn't have effect to other patches. You can just ignore it if you can't accept. But, please let me know if you have some issues. I can post v3
Thank you for your help !! Best regards --- Kuninori Morimoto
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.Morimoto-san,
I am a bit skeptical about this change but I could be wrong. I am not sure if the ignore_suspend field in the DAI link definitions was meant to be applicable for the components as well. Mark/Takashi would have to confirm this.
Even for dai links, it's not clear to me what .ignore_suspend means.
For Intel machine drivers, the .ignore_suspend flag is used for DMIC links which may be used in S0ix/D0ix states. I don't believe this works if there are multiple target states, e.g. you would probably want to leave the link active in S0ix, but suspend it in S3?
I think the current .ignore_suspend settings only work with the assumption that applications will close all capture streams before going to S3.
On Wed, Dec 18, 2019 at 08:54:27AM -0600, Pierre-Louis Bossart wrote:
Even for dai links, it's not clear to me what .ignore_suspend means.
It means exactly what it says, don't do anything on suspend.
For Intel machine drivers, the .ignore_suspend flag is used for DMIC links which may be used in S0ix/D0ix states. I don't believe this works if there are multiple target states, e.g. you would probably want to leave the link active in S0ix, but suspend it in S3?
I think the current .ignore_suspend settings only work with the assumption that applications will close all capture streams before going to S3.
These numbered suspend states are a platform specific concept, other systems will typically just suspend or not suspend. It sounds like this feature does not map onto your systems.
On 12/18/19 11:14 AM, Mark Brown wrote:
On Wed, Dec 18, 2019 at 08:54:27AM -0600, Pierre-Louis Bossart wrote:
Even for dai links, it's not clear to me what .ignore_suspend means.
It means exactly what it says, don't do anything on suspend.
For Intel machine drivers, the .ignore_suspend flag is used for DMIC links which may be used in S0ix/D0ix states. I don't believe this works if there are multiple target states, e.g. you would probably want to leave the link active in S0ix, but suspend it in S3?
I think the current .ignore_suspend settings only work with the assumption that applications will close all capture streams before going to S3.
These numbered suspend states are a platform specific concept, other systems will typically just suspend or not suspend. It sounds like this feature does not map onto your systems.
Correct. What is not clear to me is how we can specify a behavior that would be dependent on the target states.
On Wed, Dec 18, 2019 at 11:48:18AM -0600, Pierre-Louis Bossart wrote:
On 12/18/19 11:14 AM, Mark Brown wrote:
These numbered suspend states are a platform specific concept, other systems will typically just suspend or not suspend. It sounds like this feature does not map onto your systems.
Correct. What is not clear to me is how we can specify a behavior that would be dependent on the target states.
You can't currently, you'd need to extend the framework to support that.
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 --- v2 -> v3
- no change
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)
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 --- v2 -> v3
- no change
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; }
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 --- v2 -> v3
- no change
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;
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_dapm_stream_stop(), and share same code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v2 -> v3
- rename function - snd_soc_stream_stop() + snd_soc_dapm_stream_stop() - implement it at soc-dapm.c
include/sound/soc-dapm.h | 1 + sound/soc/soc-compress.c | 18 +----------------- sound/soc/soc-dapm.c | 23 +++++++++++++++++++++++ sound/soc/soc-pcm.c | 19 +------------------ 4 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 6e8a312..1b6afbc 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -434,6 +434,7 @@ void snd_soc_dapm_reset_cache(struct snd_soc_dapm_context *dapm); /* dapm events */ void snd_soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream, int event); +void snd_soc_dapm_stream_stop(struct snd_soc_pcm_runtime *rtd, int stream); void snd_soc_dapm_shutdown(struct snd_soc_card *card);
/* external DAPM widget events */ diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 7bd574f..f37fc61 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_dapm_stream_stop(rtd, stream);
mutex_unlock(&rtd->card->pcm_mutex); return 0; diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index b6378f0..442846f 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -4447,6 +4447,29 @@ void snd_soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream, mutex_unlock(&card->dapm_mutex); }
+void snd_soc_dapm_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_dapm_stream_stop); + /** * snd_soc_dapm_enable_pin_unlocked - enable pin. * @dapm: DAPM context diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ad908e0..dfff2dd 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_dapm_stream_stop(rtd, substream->stream);
mutex_unlock(&rtd->card->pcm_mutex);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
dpcm_fe_dai_shutdown() / soc_compr_free_fe() didn't care pmdown_time. We already have snd_soc_dapm_stream_stop() for it. Let's use common method.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v2 -> v3
- new patch
sound/soc/soc-compress.c | 2 +- sound/soc/soc-pcm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index f37fc61..64b1a25 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -274,7 +274,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream) for_each_dpcm_be(fe, stream, dpcm) dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
- dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP); + snd_soc_dapm_stream_stop(fe, stream);
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index dfff2dd..74d340d 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1966,7 +1966,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) soc_pcm_close(substream);
/* run the stream event for each BE */ - dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP); + snd_soc_dapm_stream_stop(fe, stream);
fe->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
On Wed, 2019-12-18 at 11:44 +0900, Kuninori Morimoto wrote:
Hi Mark
These are v2 of soc-core cleanup step8. There is no relationship between each patches.
- [6/7] renamed function name
- [7/7] new patch which cares SND_SOC_DAPM_STREAM_STOP
Kuninori Morimoto (7):
- 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-dapm: add snd_soc_dapm_stream_stop()
- ASoC: use snd_soc_dapm_stream_stop() for
Thanks, Morimoto-San. Other than the Patch 2 in the series, all others look good. The patch itself looks OK I'm just not sure if the direction is correct.
Reviewed-By: Ranjani Sridharan ranjani.sridharan@linux.intel.com
Hi Mark
I noticed that this patchset is not enough for latest for-5.6 branch. I will post v3 for latest for-5.6.
These are v2 of soc-core cleanup step8. There is no relationship between each patches.
- [6/7] renamed function name
- [7/7] new patch which cares SND_SOC_DAPM_STREAM_STOP
Kuninori Morimoto (7):
- 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-dapm: add snd_soc_dapm_stream_stop()
- ASoC: use snd_soc_dapm_stream_stop() for SND_SOC_DAPM_STREAM_STOP
include/sound/soc-dapm.h | 1 + include/sound/soc.h | 19 +++--- sound/soc/soc-component.c | 33 +++++----- sound/soc/soc-compress.c | 112 +++++++++------------------------ sound/soc/soc-core.c | 157 +++++++++++++++++++++++----------------------- sound/soc/soc-dapm.c | 23 +++++++ sound/soc/soc-pcm.c | 115 ++++++++------------------------- 7 files changed, 181 insertions(+), 279 deletions(-)
-- 2.7.4
participants (5)
-
Kuninori Morimoto -
Mark Brown -
Pierre-Louis Bossart -
Ranjani Sridharan -
Sridharan, Ranjani