[alsa-devel] [Test Request] Component level pcm_new/pcm_free v2

newer
[alsa-devel] ALC225 headset mode...

older
[alsa-devel] [PATCH v3 00/12] AC97...

Kuninori Morimoto

20 Jul 2017 20 Jul '17

4:50 a.m.

Hi Mark, Johan, Brian

These are v2 of Component level pcm_new/free patches. I think current ALSA SoC need to cleanup and move to new style. Then, current all CPU/Codec/Platform features needs to be merged into "component" (and remove CPU/Codec/Platform categorize). My previous pcm_new/free patch had wrong operation, thus Johan, Brian got damage from it. I'm so sorry about that.

Current ALSA SoC has rtd->platform, but it should be removed in new style, I think. So, this patch adds new "component list" for rtd (= I named it as rtdcom, maybe we want to have more nice naming). This list includes all components (= CPU/Codec/Platform), not only platform component.

I'm thinking that rtd->platform related operation will be replaced like below

- if(rtd->platform->xxx) - rtd->platform->xxx(yyy);

+ for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; /* rtd connected CPU/Codec/Platform */ + if (component->xxx) + component->xxx(yyy); + }

Here, xxxx is pcm_new/free on this patch. If my understanding was correct, this v2 pcm_new/free works well for you, but I don't know. So, can you please test these patches ? These are based on mark/fix/multi-pcm

Kuninori Morimoto (3): ASoC: soc-core: add snd_soc_rtdcom_xxx() ASoC: use snd_soc_rtdcom_add() and convert to consistent operation ASoC: add Component level pcm_new/pcm_free v2

Best regards --- Kuninori Morimoto

Show replies by date

Kuninori Morimoto

20 Jul 20 Jul

4:51 a.m.

New subject: [alsa-devel] [Test Request 1/3] ASoC: soc-core: add snd_soc_rtdcom_xxx()

Current snd_soc_pcm_runtime has platform / codec pointers, and we could use these specific pointer. But these will be replaced to more generic "component" soon, and will need more generic method to get each connected component pointer from rtd.

This patch adds new snd_soc_rtdcom_xxx() to connect/disconnect component to rtd. It means same as previous "platform" / "codec" pointer style, but more generic. We can find necessary component pointer from rtd by using component driver name on snd_soc_rtdcom_lookup().

Here, the reason why it uses "driver name" is that "component name" was created by fmt_single_name() and difficult to use it from driver. Driver of course knows its "driver name", thus, using it is more easy.

Not-yet-Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 13 +++++++++++++ sound/soc/soc-core.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 38038cf..07d3eeb 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -891,6 +891,18 @@ struct snd_soc_component { #endif };

+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_rtdcom(rtd, rtdcom) \ + list_for_each_entry(rtdcom, &(rtd)->component_list, list) +#define for_each_rtdcom_safe(rtd, rtdcom1, rtdcom2) \ + list_for_each_entry_safe(rtdcom1, rtdcom2, &(rtd)->component_list, list) + /* SoC Audio Codec device */ struct snd_soc_codec { struct device *dev; @@ -1250,6 +1262,7 @@ 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 dev_registered:1; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2b8e9de..474050f 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -550,6 +550,54 @@ static inline void snd_soc_debugfs_exit(void)

#endif

+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_rtdcom_list *new_rtdcom; + + for_each_rtdcom(rtd, rtdcom) { + /* already connected */ + if (rtdcom->component == component) + return 0; + } + + new_rtdcom = kmalloc(sizeof(*new_rtdcom), GFP_KERNEL); + if (!new_rtdcom) + return -ENOMEM; + + new_rtdcom->component = component; + INIT_LIST_HEAD(&new_rtdcom->list); + + list_add_tail(&new_rtdcom->list, &rtd->component_list); + + return 0; +} + +static void snd_soc_rtdcom_del_all(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_rtdcom_list *rtdcom1, *rtdcom2; + + for_each_rtdcom_safe(rtd, rtdcom1, rtdcom2) + kfree(rtdcom1); + + INIT_LIST_HEAD(&rtd->component_list); +} + +struct snd_soc_component* snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd, + const char *driver_name) +{ + struct snd_soc_rtdcom_list *rtdcom; + + for_each_rtdcom(rtd, rtdcom) { + if ((rtdcom->component->driver->name == driver_name) || + strcmp(rtdcom->component->driver->name, driver_name) == 0) + return rtdcom->component; + } + + return NULL; +} + struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card, const char *dai_link, int stream) { @@ -574,6 +622,7 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime( if (!rtd) return NULL;

+ INIT_LIST_HEAD(&rtd->component_list); rtd->card = card; rtd->dai_link = dai_link; rtd->codec_dais = kzalloc(sizeof(struct snd_soc_dai *) * @@ -591,6 +640,7 @@ static void soc_free_pcm_runtime(struct snd_soc_pcm_runtime *rtd) { if (rtd && rtd->codec_dais) kfree(rtd->codec_dais); + snd_soc_rtdcom_del_all(rtd); kfree(rtd); }

-- 1.9.1

Kuninori Morimoto

4:51 a.m.

New subject: [alsa-devel] [Test Request 2/3] ASoC: use snd_soc_rtdcom_add() and convert to consistent operation

Basically, current ALSA SoC framework is based on CPU/Codec/Platform, but its operation doesn't have consistent. Thus, source code was unreadable, and difficult to understand. This patch connects each component (= CPU/Codec/Platform) to rtd by using snd_soc_rtdcom_add(), and convert uneven operations to consistent operation.

Not-yet-Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 2 +- sound/soc/soc-core.c | 61 +++++++++++++++++++++++----------------------------- sound/soc/soc-pcm.c | 24 +++++++++++---------- 3 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 07d3eeb..963eef6 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1247,7 +1247,7 @@ struct snd_soc_pcm_runtime { struct snd_pcm *pcm; struct snd_compr *compr; struct snd_soc_codec *codec; - struct snd_soc_platform *platform; + struct snd_soc_platform *platform; /* will be removed */ struct snd_soc_dai *codec_dai; struct snd_soc_dai *cpu_dai;

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 474050f..646c722 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1099,6 +1099,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd; struct snd_soc_dai_link_component *codecs = dai_link->codecs; struct snd_soc_dai_link_component cpu_dai_component; + struct snd_soc_component *component; struct snd_soc_dai **codec_dais; struct snd_soc_platform *platform; struct device_node *platform_of_node; @@ -1126,6 +1127,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, dai_link->cpu_dai_name); goto _err_defer; } + snd_soc_rtdcom_add(rtd, rtd->cpu_dai->component);

rtd->num_codecs = dai_link->num_codecs;

@@ -1138,6 +1140,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, codecs[i].dai_name); goto _err_defer; } + snd_soc_rtdcom_add(rtd, codec_dais[i]->component); }

/* Single codec links expect codec and codec_dai in runtime data */ @@ -1150,6 +1153,23 @@ static int soc_bind_dai_link(struct snd_soc_card *card, platform_name = "snd-soc-dummy";

/* find one from the set of registered platforms */ + list_for_each_entry(component, &component_list, list) { + platform_of_node = component->dev->of_node; + if (!platform_of_node && component->dev->parent->of_node) + platform_of_node = component->dev->parent->of_node; + + if (dai_link->platform_of_node) { + if (platform_of_node != dai_link->platform_of_node) + continue; + } else { + if (strcmp(component->name, platform_name)) + continue; + } + + snd_soc_rtdcom_add(rtd, component); + } + + /* find one from the set of registered platforms */ list_for_each_entry(platform, &platform_list, list) { platform_of_node = platform->dev->of_node; if (!platform_of_node && platform->dev->parent->of_node) @@ -1234,27 +1254,15 @@ static void soc_remove_link_dais(struct snd_soc_card *card, static void soc_remove_link_components(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd, int order) { - struct snd_soc_dai *cpu_dai = rtd->cpu_dai; - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_component *component; - int i; + struct snd_soc_rtdcom_list *rtdcom;

- /* remove the platform */ - if (platform && platform->component.driver->remove_order == order) - soc_remove_component(&platform->component); + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component;

- /* remove the CODEC-side CODEC */ - for (i = 0; i < rtd->num_codecs; i++) { - component = rtd->codec_dais[i]->component; if (component->driver->remove_order == order) soc_remove_component(component); } - - /* remove any CPU-side CODEC */ - if (cpu_dai) { - if (cpu_dai->component->driver->remove_order == order) - soc_remove_component(cpu_dai->component); - } }

static void soc_remove_dai_links(struct snd_soc_card *card) @@ -1606,21 +1614,13 @@ static int soc_probe_link_components(struct snd_soc_card *card, struct snd_soc_pcm_runtime *rtd, int order) { - struct snd_soc_platform *platform = rtd->platform; struct snd_soc_component *component; - int i, ret; + struct snd_soc_rtdcom_list *rtdcom; + int ret;

- /* probe the CPU-side component, if it is a CODEC */ - component = rtd->cpu_dai->component; - if (component->driver->probe_order == order) { - ret = soc_probe_component(card, component); - if (ret < 0) - return ret; - } + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component;

- /* probe the CODEC-side components */ - for (i = 0; i < rtd->num_codecs; i++) { - component = rtd->codec_dais[i]->component; if (component->driver->probe_order == order) { ret = soc_probe_component(card, component); if (ret < 0) @@ -1628,13 +1628,6 @@ static int soc_probe_link_components(struct snd_soc_card *card, } }

- /* probe the platform */ - if (platform->component.driver->probe_order == order) { - ret = soc_probe_component(card, &platform->component); - if (ret < 0) - return ret; - } - return 0; }

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 7d3859e..d801c93 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -454,6 +454,8 @@ 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_platform *platform = rtd->platform; + 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"; @@ -462,10 +464,12 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) pinctrl_pm_select_default_state(cpu_dai->dev); for (i = 0; i < rtd->num_codecs; i++) pinctrl_pm_select_default_state(rtd->codec_dais[i]->dev); - pm_runtime_get_sync(cpu_dai->dev); - for (i = 0; i < rtd->num_codecs; i++) - pm_runtime_get_sync(rtd->codec_dais[i]->dev); - pm_runtime_get_sync(platform->dev); + + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + pm_runtime_get_sync(component->dev); + }

mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);

@@ -603,15 +607,13 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) out: mutex_unlock(&rtd->pcm_mutex);

- pm_runtime_mark_last_busy(platform->dev); - pm_runtime_put_autosuspend(platform->dev); - for (i = 0; i < rtd->num_codecs; i++) { - pm_runtime_mark_last_busy(rtd->codec_dais[i]->dev); - pm_runtime_put_autosuspend(rtd->codec_dais[i]->dev); + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + pm_runtime_mark_last_busy(component->dev); + pm_runtime_put_autosuspend(component->dev); }

- pm_runtime_mark_last_busy(cpu_dai->dev); - pm_runtime_put_autosuspend(cpu_dai->dev); for (i = 0; i < rtd->num_codecs; i++) { if (!rtd->codec_dais[i]->active) pinctrl_pm_select_sleep_state(rtd->codec_dais[i]->dev);

-- 1.9.1

Kuninori Morimoto

4:52 a.m.

New subject: [alsa-devel] [Test Request 3/3] ASoC: add Component level pcm_new/pcm_free v2

In current ALSA SoC, Platform only has pcm_new/pcm_free feature, but it should be supported on Component level. This patch adds it.

The v1 was added commit 99b04f4c4051f7 ("ASoC: add Component level pcm_new/pcm_free") but it called all "card" connected component's pcm_new/free, it was wrong. This patch calls "rtd" connected component.

Not-yet-Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 6 ++++++ sound/soc/soc-core.c | 21 +++++++++++++++++++++ sound/soc/soc-pcm.c | 33 ++++++++++++++++++++++++++------- 3 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 963eef6..103ae43 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -806,6 +806,10 @@ struct snd_soc_component_driver { int (*suspend)(struct snd_soc_component *); int (*resume)(struct snd_soc_component *);

+ /* pcm creation and destruction */ + int (*pcm_new)(struct snd_soc_pcm_runtime *); + void (*pcm_free)(struct snd_pcm *); + /* DT */ int (*of_xlate_dai_name)(struct snd_soc_component *component, struct of_phandle_args *args, @@ -881,6 +885,8 @@ struct snd_soc_component { void (*remove)(struct snd_soc_component *); int (*suspend)(struct snd_soc_component *); int (*resume)(struct snd_soc_component *); + int (*pcm_new)(struct snd_soc_pcm_runtime *); + void (*pcm_free)(struct snd_pcm *);

/* machine specific init */ int (*init)(struct snd_soc_component *component); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 646c722..5459e5b 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3214,6 +3214,8 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, component->remove = component->driver->remove; component->suspend = component->driver->suspend; component->resume = component->driver->resume; + component->pcm_new = component->driver->pcm_new; + component->pcm_free= component->driver->pcm_free;

dapm = &component->dapm; dapm->dev = dev; @@ -3441,6 +3443,21 @@ static void snd_soc_platform_drv_remove(struct snd_soc_component *component) platform->driver->remove(platform); }

+static int snd_soc_platform_drv_pcm_new(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_platform *platform = rtd->platform; + + return platform->driver->pcm_new(rtd); +} + +static void snd_soc_platform_drv_pcm_free(struct snd_pcm *pcm) +{ + struct snd_soc_pcm_runtime *rtd = pcm->private_data; + struct snd_soc_platform *platform = rtd->platform; + + platform->driver->pcm_free(pcm); +} + /** * snd_soc_add_platform - Add a platform to the ASoC core * @dev: The parent device for the platform @@ -3464,6 +3481,10 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform, platform->component.probe = snd_soc_platform_drv_probe; if (platform_drv->remove) platform->component.remove = snd_soc_platform_drv_remove; + if (platform_drv->pcm_new) + platform->component.pcm_new = snd_soc_platform_drv_pcm_new; + if (platform_drv->pcm_free) + platform->component.pcm_free = snd_soc_platform_drv_pcm_free;

#ifdef CONFIG_DEBUG_FS platform->component.debugfs_prefix = "platform"; diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d801c93..36b3035 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2634,12 +2634,28 @@ static int dpcm_fe_dai_close(struct snd_pcm_substream *fe_substream) return ret; }

+static void soc_pcm_free(struct snd_pcm *pcm) +{ + struct snd_soc_pcm_runtime *rtd = pcm->private_data; + struct snd_soc_rtdcom_list *rtdcom; + struct snd_soc_component *component; + + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + if (component->pcm_free) + component->pcm_free(pcm); + } +} + /* create a new pcm */ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) { struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *codec_dai; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_component *component; + struct snd_soc_rtdcom_list *rtdcom; struct snd_pcm *pcm; char new_name[64]; int ret = 0, playback = 0, capture = 0; @@ -2749,17 +2765,20 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (capture) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops);

- if (platform->driver->pcm_new) { - ret = platform->driver->pcm_new(rtd); + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + if (!component->pcm_new) + continue; + + ret = component->pcm_new(rtd); if (ret < 0) { - dev_err(platform->dev, - "ASoC: pcm constructor failed: %d\n", - ret); + dev_err(component->dev, + "ASoC: pcm constructor failed: %d\n", ret); return ret; } } - - pcm->private_free = platform->driver->pcm_free; + pcm->private_free = soc_pcm_free; out: dev_info(rtd->card->dev, "%s <-> %s mapping ok\n", (rtd->num_codecs > 1) ? "multicodec" : rtd->codec_dai->name,

-- 1.9.1

Kuninori Morimoto

21 Jul 21 Jul

3:46 a.m.

Hi Mark, Johan, Brian again

I noticed this patch-set still have issue. Please drop it. Sorry for my noise

...

These are v2 of Component level pcm_new/free patches. I think current ALSA SoC need to cleanup and move to new style. Then, current all CPU/Codec/Platform features needs to be merged into "component" (and remove CPU/Codec/Platform categorize). My previous pcm_new/free patch had wrong operation, thus Johan, Brian got damage from it. I'm so sorry about that.

Current ALSA SoC has rtd->platform, but it should be removed in new style, I think. So, this patch adds new "component list" for rtd (= I named it as rtdcom, maybe we want to have more nice naming). This list includes all components (= CPU/Codec/Platform), not only platform component.

I'm thinking that rtd->platform related operation will be replaced like below
if(rtd->platform->xxx)
rtd->platform->xxx(yyy);
for_each_rtdcom(rtd, rtdcom) {
component = rtdcom->component; /* rtd connected CPU/Codec/Platform */
if (component->xxx)
	component->xxx(yyy);
}
Here, xxxx is pcm_new/free on this patch. If my understanding was correct, this v2 pcm_new/free works well for you, but I don't know. So, can you please test these patches ? These are based on mark/fix/multi-pcm

Kuninori Morimoto (3): ASoC: soc-core: add snd_soc_rtdcom_xxx() ASoC: use snd_soc_rtdcom_add() and convert to consistent operation ASoC: add Component level pcm_new/pcm_free v2

Best regards

Kuninori Morimoto

2711

Age (days ago)

2712

Last active (days ago)

List overview

Download

4 comments

1 participants

tags (0)

participants (1)

Kuninori Morimoto