[alsa-devel] [PATCH 0/6] ASoC: soc-core cleanup step8

newer
[alsa-devel] [BUG] ALSA: soc: sti:...

older
[alsa-devel] Undecalared Variable...

Kuninori Morimoto

17 Dec 2019 17 Dec '19

2:25 a.m.

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

Show replies by date

Kuninori Morimoto

17 Dec 17 Dec

2:25 a.m.

New subject: [alsa-devel] [PATCH 1/6] ASoC: soc-core: remove snd_soc_rtdcom_list

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; }

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; }

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;

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

Kuninori Morimoto

2:25 a.m.

New subject: [alsa-devel] [PATCH 2/6] ASoC: soc-core: care .ignore_suspend for Component suspend

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

Kuninori Morimoto

2:26 a.m.

New subject: [alsa-devel] [PATCH 3/6] ASoC: soc-core: remove duplicate pinctrl operation when suspend

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

Kuninori Morimoto

2:26 a.m.

New subject: [alsa-devel] [PATCH 4/6] ASoC: soc-core: do pinctrl_pm_select_xxx() as component

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); }

return 0; }

-- 2.7.4

Kuninori Morimoto

2:26 a.m.

New subject: [alsa-devel] [PATCH 5/6] ASoC: soc-core: add snd_soc_close_delayed_work()

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

Kuninori Morimoto

2:26 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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

Pierre-Louis Bossart

4:22 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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

Kuninori Morimoto

4:54 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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

Kuninori Morimoto

5:41 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

...

+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

Mark Brown

1:13 p.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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

Pierre-Louis Bossart

2:56 p.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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'...

Kuninori Morimoto

18 Dec 18 Dec

2:51 a.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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

Mark Brown

12:50 p.m.

New subject: [alsa-devel] [PATCH 6/6] ASoC: soc-core: add snd_soc_stream_stop()

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.

1834

Age (days ago)

1835

Last active (days ago)

List overview

Download

13 comments

3 participants

tags (0)

participants (3)

Kuninori Morimoto
Mark Brown
Pierre-Louis Bossart