[alsa-devel] [PATCH v2 1/5] ASoC: dapm: Skip CODEC<->CODEC links in connect_dai_link_widgets()
For CODEC to CODEC DAI links the paths are created in snd_soc_dapm_new_pcm(). Also for CODEC to CODEC links the widgets are connected cross-over via a DAI link widget, meaning that the capture widget of one CODEC will be connected to the playback widget of the other and vice versa. Whereas snd_soc_dapm_connect_dai_link_widgets() directly connects the playback widget of the CPU DAI to the playback widget of the CODEC DAI and the capture widget of the CPU DAI to the capture widget of the CODEC DAI. So not skipping CODEC<->CODEC links in snd_soc_dapm_connect_dai_link_widgets() will create incorrect connections between the two CODECs which will cause DAPM to detect active paths where there are none and unnecessarily power up widgets.
Fixes: b893ea5 ("ASoC: sapm: Automatically connect DAI link widgets in DAPM graph.") Cc: stable@vger.kernel.org (for 3.14+) Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-dapm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index fb6c7b7..142a738 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3410,8 +3410,11 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) cpu_dai = rtd->cpu_dai; codec_dai = rtd->codec_dai;
- /* dynamic FE links have no fixed DAI mapping */ - if (rtd->dai_link->dynamic) + /* + * dynamic FE links have no fixed DAI mapping. + * CODEC<->CODEC links have no direct connection. + */ + if (rtd->dai_link->dynamic || rtd->dai_link->params) continue;
/* there is no point in connecting BE DAI links with dummies */
This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
The patch claims that the patch is necessary to avoid double prefix addition when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets(). But snd_soc_dapm_add_route() is called with the card's DAPM context, which does not have a prefix, which means there is no prefix that could be added a second time.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- I might be missing something. My best guess is that this was needed in some vendor tree, but not in upstream. --- sound/soc/soc-dapm.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 142a738..08d869c 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2381,8 +2381,7 @@ err: }
static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, - const struct snd_soc_dapm_route *route, - unsigned int is_prefixed) + const struct snd_soc_dapm_route *route) { struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w; struct snd_soc_dapm_widget *wtsource = NULL, *wtsink = NULL; @@ -2392,7 +2391,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, char prefixed_source[80]; int ret;
- if (dapm->codec && dapm->codec->name_prefix && !is_prefixed) { + if (dapm->codec && dapm->codec->name_prefix) { snprintf(prefixed_sink, sizeof(prefixed_sink), "%s %s", dapm->codec->name_prefix, route->sink); sink = prefixed_sink; @@ -2520,7 +2519,7 @@ int snd_soc_dapm_add_routes(struct snd_soc_dapm_context *dapm,
mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_INIT); for (i = 0; i < num; i++) { - r = snd_soc_dapm_add_route(dapm, route, false); + r = snd_soc_dapm_add_route(dapm, route); if (r < 0) { dev_err(dapm->dev, "ASoC: Failed to add route %s -> %s -> %s\n", route->source, @@ -3430,7 +3429,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) cpu_dai->codec->name, r.source, codec_dai->platform->name, r.sink);
- snd_soc_dapm_add_route(&card->dapm, &r, true); + snd_soc_dapm_add_route(&card->dapm, &r); }
/* connect BE DAI capture if widgets are valid */ @@ -3441,7 +3440,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) codec_dai->codec->name, r.source, cpu_dai->platform->name, r.sink);
- snd_soc_dapm_add_route(&card->dapm, &r, true); + snd_soc_dapm_add_route(&card->dapm, &r); }
}
On 05/07/2014 04:20 PM, Lars-Peter Clausen wrote:
This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
The patch claims that the patch is necessary to avoid double prefix addition when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets(). But snd_soc_dapm_add_route() is called with the card's DAPM context, which does not have a prefix, which means there is no prefix that could be added a second time.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
I might be missing something. My best guess is that this was needed in some vendor tree, but not in upstream.
Hi Songhee, Arun,
Can you take a look at this? I think the reason why you had this patch is because this was needed to prevent double prefix addition in snd_soc_dapm_link_dai_widgets(). All other places where dapm_add_route() is/was used do not suffer the problem of double prefix addition. But snd_soc_dapm_link_dai_widgets() was changed in commit commit 2553628e ("ASoC: dapm: Add snd_soc_dapm_add_path() helper function") to use dapm_add_path which does not have the problem since it doesn't try to add a prefix. v3.12 was the first kernel that had this commit and I think that you forward ported this change from a vendor tree that was based on an earlier version. So in summery I think the issue that you tried to fix this patch was already fixed by the time you submitted the patch.
Thanks, - Lars
sound/soc/soc-dapm.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 142a738..08d869c 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2381,8 +2381,7 @@ err: }
static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
const struct snd_soc_dapm_route *route,unsigned int is_prefixed)
const struct snd_soc_dapm_route *route)@@ -2392,7 +2391,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, char prefixed_source[80]; int ret;
- if (dapm->codec && dapm->codec->name_prefix && !is_prefixed) {
- if (dapm->codec && dapm->codec->name_prefix) { snprintf(prefixed_sink, sizeof(prefixed_sink), "%s %s", dapm->codec->name_prefix, route->sink); sink = prefixed_sink;
@@ -2520,7 +2519,7 @@ int snd_soc_dapm_add_routes(struct snd_soc_dapm_context *dapm,
mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_INIT); for (i = 0; i < num; i++) {
r = snd_soc_dapm_add_route(dapm, route, false);
r = snd_soc_dapm_add_route(dapm, route);@@ -3430,7 +3429,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) cpu_dai->codec->name, r.source, codec_dai->platform->name, r.sink);
snd_soc_dapm_add_route(&card->dapm, &r, true);
snd_soc_dapm_add_route(&card->dapm, &r);}
/* connect BE DAI capture if widgets are valid */
@@ -3441,7 +3440,7 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) codec_dai->codec->name, r.source, cpu_dai->platform->name, r.sink);
snd_soc_dapm_add_route(&card->dapm, &r, true);
snd_soc_dapm_add_route(&card->dapm, &r);}
}
-----Original Message----- From: Lars-Peter Clausen [mailto:lars@metafoo.de] Sent: Monday, May 12, 2014 8:00 AM To: Lars-Peter Clausen Cc: Mark Brown; Liam Girdwood; Songhee Baek; Arun Shamanna Lakshmi; alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH v2 2/5] ASoC: Revert "ASoC: dapm: Fix double prefix addition"
On 05/07/2014 04:20 PM, Lars-Peter Clausen wrote:
This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
The patch claims that the patch is necessary to avoid double prefix addition when calling snd_soc_dapm_add_route() from
snd_soc_dapm_connect_dai_link_widgets().
But snd_soc_dapm_add_route() is called with the card's DAPM context, which does not have a prefix, which means there is no prefix that could be added a second time.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
I might be missing something. My best guess is that this was needed in some vendor tree, but not in upstream.
Hi Songhee, Arun,
Can you take a look at this? I think the reason why you had this patch is because this was needed to prevent double prefix addition in snd_soc_dapm_link_dai_widgets(). All other places where dapm_add_route() is/was used do not suffer the problem of double prefix addition. But snd_soc_dapm_link_dai_widgets() was changed in commit commit 2553628e ("ASoC: dapm: Add snd_soc_dapm_add_path() helper function") to use dapm_add_path which does not have the problem since it doesn't try to add a prefix. v3.12 was the first kernel that had this commit and I think that you forward ported this change from a vendor tree that was based on an earlier version. So in summery I think the issue that you tried to fix this patch was already fixed by the time you submitted the patch.
Thanks,
- Lars
We tested the latest upstream kernel and it works without our patch. So, it's okay to revert this and sorry to push this patch in the first place.
sound/soc/soc-dapm.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
On Wed, May 07, 2014 at 04:20:25PM +0200, Lars-Peter Clausen wrote:
This reverts commit bd23c5b661858446267f4d6b2fb4edd8eb710dda.
The patch claims that the patch is necessary to avoid double prefix addition when calling snd_soc_dapm_add_route() from snd_soc_dapm_connect_dai_link_widgets().
Applied, thanks. Please take care over the word wrapping in your commit logs - they're bumping right at the 80 column limit usually.
We already know which two widgets should be connected, so use snd_soc_dapm_add_path() instead of snd_soc_dapm_add_route() in snd_soc_dapm_connect_dai_link_widgets().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-dapm.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 08d869c..e28ce91 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3397,12 +3397,10 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card) void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card) { struct snd_soc_pcm_runtime *rtd = card->rtd; + struct snd_soc_dapm_widget *sink, *source; struct snd_soc_dai *cpu_dai, *codec_dai; - struct snd_soc_dapm_route r; int i;
- memset(&r, 0, sizeof(r)); - /* for each BE DAI link... */ for (i = 0; i < card->num_rtd; i++) { rtd = &card->rtd[i]; @@ -3423,26 +3421,27 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
/* connect BE DAI playback if widgets are valid */ if (codec_dai->playback_widget && cpu_dai->playback_widget) { - r.source = cpu_dai->playback_widget->name; - r.sink = codec_dai->playback_widget->name; + source = cpu_dai->playback_widget; + sink = codec_dai->playback_widget; dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n", - cpu_dai->codec->name, r.source, - codec_dai->platform->name, r.sink); + cpu_dai->codec->name, source->name, + codec_dai->platform->name, sink->name);
- snd_soc_dapm_add_route(&card->dapm, &r); + snd_soc_dapm_add_path(&card->dapm, source, sink, + NULL, NULL); }
/* connect BE DAI capture if widgets are valid */ if (codec_dai->capture_widget && cpu_dai->capture_widget) { - r.source = codec_dai->capture_widget->name; - r.sink = cpu_dai->capture_widget->name; + source = codec_dai->capture_widget; + sink = cpu_dai->capture_widget; dev_dbg(rtd->dev, "connected DAI link %s:%s -> %s:%s\n", - codec_dai->codec->name, r.source, - cpu_dai->platform->name, r.sink); + codec_dai->codec->name, source->name, + cpu_dai->platform->name, sink->name);
- snd_soc_dapm_add_route(&card->dapm, &r); + snd_soc_dapm_add_path(&card->dapm, source, sink, + NULL, NULL); } - } }
On Wed, May 07, 2014 at 04:20:26PM +0200, Lars-Peter Clausen wrote:
We already know which two widgets should be connected, so use snd_soc_dapm_add_path() instead of snd_soc_dapm_add_route() in snd_soc_dapm_connect_dai_link_widgets().
Applied, thanks.
We already know the widgets we want to connect, so use snd_soc_dapm_add_path() instead of snd_soc_dapm_add_route() in snd_soc_dapm_new_pcm().
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-dapm.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index e28ce91..d7958f4 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3242,11 +3242,11 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card, struct snd_soc_dapm_widget *source, struct snd_soc_dapm_widget *sink) { - struct snd_soc_dapm_route routes[2]; struct snd_soc_dapm_widget template; struct snd_soc_dapm_widget *w; size_t len; char *link_name; + int ret;
len = strlen(source->name) + strlen(sink->name) + 2; link_name = devm_kzalloc(card->dev, len, GFP_KERNEL); @@ -3273,15 +3273,10 @@ int snd_soc_dapm_new_pcm(struct snd_soc_card *card,
w->params = params;
- memset(&routes, 0, sizeof(routes)); - - routes[0].source = source->name; - routes[0].sink = link_name; - routes[1].source = link_name; - routes[1].sink = sink->name; - - return snd_soc_dapm_add_routes(&card->dapm, routes, - ARRAY_SIZE(routes)); + ret = snd_soc_dapm_add_path(&card->dapm, source, w, NULL, NULL); + if (ret) + return ret; + return snd_soc_dapm_add_path(&card->dapm, w, sink, NULL, NULL); }
int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
On Wed, May 07, 2014 at 04:20:27PM +0200, Lars-Peter Clausen wrote:
We already know the widgets we want to connect, so use snd_soc_dapm_add_path() instead of snd_soc_dapm_add_route() in snd_soc_dapm_new_pcm().
Applied, thanks.
If we find a widget who's stream name matches the name of a DAI widget then thats the one it should be connected to. Based on the widget id we can say in which direction the path should be. No need to go back to the DAI and check the stream names.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-dapm.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index d7958f4..bed17a3 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3334,6 +3334,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card) { struct snd_soc_dapm_widget *dai_w, *w; + struct snd_soc_dapm_widget *src, *sink; struct snd_soc_dai *dai;
/* For each DAI widget... */ @@ -3364,25 +3365,15 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card) if (!w->sname || !strstr(w->sname, dai_w->name)) continue;
- if (dai->driver->playback.stream_name && - strstr(w->sname, - dai->driver->playback.stream_name)) { - dev_dbg(dai->dev, "%s -> %s\n", - dai->playback_widget->name, w->name); - - snd_soc_dapm_add_path(w->dapm, - dai->playback_widget, w, NULL, NULL); - } - - if (dai->driver->capture.stream_name && - strstr(w->sname, - dai->driver->capture.stream_name)) { - dev_dbg(dai->dev, "%s -> %s\n", - w->name, dai->capture_widget->name); - - snd_soc_dapm_add_path(w->dapm, w, - dai->capture_widget, NULL, NULL); + if (dai_w->id == snd_soc_dapm_dai_in) { + src = dai_w; + sink = w; + } else { + src = w; + sink = dai_w; } + dev_dbg(dai->dev, "%s -> %s\n", src->name, sink->name); + snd_soc_dapm_add_path(w->dapm, src, sink, NULL, NULL); } }
On Wed, May 07, 2014 at 04:20:28PM +0200, Lars-Peter Clausen wrote:
If we find a widget who's stream name matches the name of a DAI widget then thats the one it should be connected to. Based on the widget id we can say in which direction the path should be. No need to go back to the DAI and check the stream names.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
sound/soc/soc-dapm.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index d7958f4..bed17a3 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3334,6 +3334,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card) { struct snd_soc_dapm_widget *dai_w, *w;
- struct snd_soc_dapm_widget *src, *sink; struct snd_soc_dai *dai;
I think you have removed all the uses of this variable (dai) so you can remove this as well.
Thanks, Charles
On 05/12/2014 04:37 PM, Charles Keepax wrote:
On Wed, May 07, 2014 at 04:20:28PM +0200, Lars-Peter Clausen wrote:
If we find a widget who's stream name matches the name of a DAI widget then thats the one it should be connected to. Based on the widget id we can say in which direction the path should be. No need to go back to the DAI and check the stream names.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
sound/soc/soc-dapm.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index d7958f4..bed17a3 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3334,6 +3334,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card) { struct snd_soc_dapm_widget *dai_w, *w;
- struct snd_soc_dapm_widget *src, *sink; struct snd_soc_dai *dai;
I think you have removed all the uses of this variable (dai) so you can remove this as well.
It is still used in the dev_dbg() call.
On Wed, May 07, 2014 at 04:20:28PM +0200, Lars-Peter Clausen wrote:
If we find a widget who's stream name matches the name of a DAI widget then thats the one it should be connected to. Based on the widget id we can say in which direction the path should be. No need to go back to the DAI and check the stream names.
Applied, thanks.
On Wed, May 07, 2014 at 04:20:24PM +0200, Lars-Peter Clausen wrote:
For CODEC to CODEC DAI links the paths are created in snd_soc_dapm_new_pcm(). Also for CODEC to CODEC links the widgets are connected cross-over via a DAI link widget, meaning that the capture widget of one CODEC will be connected to
Applied, thanks.
participants (4)
-
Arun Shamanna Lakshmi -
Charles Keepax -
Lars-Peter Clausen -
Mark Brown