[alsa-devel] [PATCHv2 0/3] ASoC: Extend DAPM to handle cross-device paths
Hi
Here's updated version of previous RFC_ii/iv set:
http://mailman.alsa-project.org/pipermail/alsa-devel/2010-October/033196.htm...
Patches 1-2 have no remarkable changes, just modified on top of recent changes.
Patch 3 is simpler as it turns out that the dapm_seq_run_coalesced doesn't need any changes because it's enough to call it with a right DAPM context. One fix is in beginning of dapm_power_widgets that d->dev_power is zeroed only for those codecs that have widgets in order to avoid unnecessary widgetless codec bias shutdowns.
Minor changes are dapm.sys_power renaming to dev_power and long lines are cut.
Decoupling DAPM paths from DAPM context is a first prerequisite when extending ASoC core to cross-device paths. This patch is almost a nullop and does not allow to construct cross-device setup but the path clean-up part in dapm_free_widgets is prepared to remove cross-device paths between a device being removed and others.
Signed-off-by: Jarkko Nikula jhnikula@gmail.com --- include/sound/soc-dapm.h | 1 - include/sound/soc.h | 2 + sound/soc/codecs/tlv320aic3x.c | 2 +- sound/soc/soc-core.c | 2 +- sound/soc/soc-dapm.c | 41 ++++++++++++++++++++++++++------------- 5 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 78d3560..c674c91 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -470,7 +470,6 @@ struct snd_soc_dapm_widget { /* DAPM context */ struct snd_soc_dapm_context { struct list_head widgets; - struct list_head paths; enum snd_soc_bias_level bias_level; enum snd_soc_bias_level suspend_bias_level; struct delayed_work delayed_work; diff --git a/include/sound/soc.h b/include/sound/soc.h index af23f42..835087d 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -618,6 +618,8 @@ struct snd_soc_card { struct list_head platform_dev_list; struct list_head dai_dev_list;
+ struct list_head paths; + #ifdef CONFIG_DEBUG_FS struct dentry *debugfs_card_root; struct dentry *debugfs_pop_time; diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 6173c2b..0082cc3 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -183,7 +183,7 @@ static int snd_soc_dapm_put_volsw_aic3x(struct snd_kcontrol *kcontrol,
if (snd_soc_test_bits(widget->codec, reg, val_mask, val)) { /* find dapm widget path assoc with kcontrol */ - list_for_each_entry(path, &widget->dapm->paths, list) { + list_for_each_entry(path, &widget->dapm->card->paths, list) { if (path->kcontrol != kcontrol) continue;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 3d70ce5..d4800c8 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1696,6 +1696,7 @@ static int soc_probe(struct platform_device *pdev) INIT_LIST_HEAD(&card->dai_dev_list); INIT_LIST_HEAD(&card->codec_dev_list); INIT_LIST_HEAD(&card->platform_dev_list); + INIT_LIST_HEAD(&card->paths);
soc_init_card_debugfs(card);
@@ -3275,7 +3276,6 @@ int snd_soc_register_codec(struct device *dev, }
INIT_LIST_HEAD(&codec->dapm.widgets); - INIT_LIST_HEAD(&codec->dapm.paths); codec->dapm.bias_level = SND_SOC_BIAS_OFF; codec->dapm.dev = dev; codec->dapm.codec = codec; diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 8352430..fb2d3ce 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -266,7 +266,7 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
for (i = 0; i < e->max; i++) { if (!(strcmp(control_name, e->texts[i]))) { - list_add(&path->list, &dapm->paths); + list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &dest->sources); list_add(&path->list_source, &src->sinks); path->name = (char*)e->texts[i]; @@ -288,7 +288,7 @@ static int dapm_connect_mixer(struct snd_soc_dapm_context *dapm, /* search for mixer kcontrol */ for (i = 0; i < dest->num_kcontrols; i++) { if (!strcmp(control_name, dest->kcontrols[i].name)) { - list_add(&path->list, &dapm->paths); + list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &dest->sources); list_add(&path->list_source, &src->sinks); path->name = dest->kcontrols[i].name; @@ -447,7 +447,7 @@ static inline void dapm_clear_walk(struct snd_soc_dapm_context *dapm) { struct snd_soc_dapm_path *p;
- list_for_each_entry(p, &dapm->paths, list) + list_for_each_entry(p, &dapm->card->paths, list) p->walked = 0; }
@@ -1171,7 +1171,7 @@ static int dapm_mux_update_power(struct snd_soc_dapm_widget *widget, return 0;
/* find dapm widget path assoc with kcontrol */ - list_for_each_entry(path, &widget->dapm->paths, list) { + list_for_each_entry(path, &widget->dapm->card->paths, list) { if (path->kcontrol != kcontrol) continue;
@@ -1205,7 +1205,7 @@ static int dapm_mixer_update_power(struct snd_soc_dapm_widget *widget, return -ENODEV;
/* find dapm widget path assoc with kcontrol */ - list_for_each_entry(path, &widget->dapm->paths, list) { + list_for_each_entry(path, &widget->dapm->card->paths, list) { if (path->kcontrol != kcontrol) continue;
@@ -1295,14 +1295,27 @@ static void dapm_free_widgets(struct snd_soc_dapm_context *dapm)
list_for_each_entry_safe(w, next_w, &dapm->widgets, list) { list_del(&w->list); + /* + * remove source and sink paths associated to this widget. + * While removing the path, remove reference to it from both + * source and sink widgets so that path is removed only once. + */ + list_for_each_entry_safe(p, next_p, &w->sources, list_sink) { + list_del(&p->list_sink); + list_del(&p->list_source); + list_del(&p->list); + kfree(p->long_name); + kfree(p); + } + list_for_each_entry_safe(p, next_p, &w->sinks, list_source) { + list_del(&p->list_sink); + list_del(&p->list_source); + list_del(&p->list); + kfree(p->long_name); + kfree(p); + } kfree(w); } - - list_for_each_entry_safe(p, next_p, &dapm->paths, list) { - list_del(&p->list); - kfree(p->long_name); - kfree(p); - } }
static int snd_soc_dapm_set_pin(struct snd_soc_dapm_context *dapm, @@ -1395,7 +1408,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm,
/* connect static paths */ if (control == NULL) { - list_add(&path->list, &dapm->paths); + list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &wsink->sources); list_add(&path->list_source, &wsource->sinks); path->connect = 1; @@ -1416,7 +1429,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, case snd_soc_dapm_supply: case snd_soc_dapm_aif_in: case snd_soc_dapm_aif_out: - list_add(&path->list, &dapm->paths); + list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &wsink->sources); list_add(&path->list_source, &wsource->sinks); path->connect = 1; @@ -1439,7 +1452,7 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, case snd_soc_dapm_mic: case snd_soc_dapm_line: case snd_soc_dapm_spk: - list_add(&path->list, &dapm->paths); + list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &wsink->sources); list_add(&path->list_source, &wsource->sinks); path->connect = 0;
Decoupling widgets from DAPM context is required when extending the ASoC core to cross-device paths. Even the list of widgets are now kept in struct snd_soc_card, the widget listing in sysfs and debugs remain sorted per device.
This patch makes possible to build cross-device paths but does not extend yet the DAPM to handle codec bias and widget power changes of an another device.
Cross-device paths are registered by listing the widgets from device A in a map for device B. An example below shows a path that connects MONO out of A into Line In of B:
static const struct snd_soc_dapm_route mapA[] = { {"MONO", NULL, "DAC"}, };
static const struct snd_soc_dapm_route mapB[] = { {"Line In", NULL, "MONO"}, };
Signed-off-by: Jarkko Nikula jhnikula@gmail.com --- include/sound/soc-dapm.h | 2 +- include/sound/soc.h | 1 + sound/soc/codecs/wm8960.c | 4 ++- sound/soc/soc-core.c | 2 +- sound/soc/soc-dapm.c | 69 ++++++++++++++++++++++++++++++++------------- 5 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index c674c91..6714fc8 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -469,7 +469,7 @@ struct snd_soc_dapm_widget {
/* DAPM context */ struct snd_soc_dapm_context { - struct list_head widgets; + int n_widgets; /* number of widgets in this context */ enum snd_soc_bias_level bias_level; enum snd_soc_bias_level suspend_bias_level; struct delayed_work delayed_work; diff --git a/include/sound/soc.h b/include/sound/soc.h index 835087d..77be6b3 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -618,6 +618,7 @@ struct snd_soc_card { struct list_head platform_dev_list; struct list_head dai_dev_list;
+ struct list_head widgets; struct list_head paths;
#ifdef CONFIG_DEBUG_FS diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c index 0ea5788..b06c567 100644 --- a/sound/soc/codecs/wm8960.c +++ b/sound/soc/codecs/wm8960.c @@ -418,7 +418,9 @@ static int wm8960_add_widgets(struct snd_soc_codec *codec) * list each time to find the desired power state do so now * and save the result. */ - list_for_each_entry(w, &codec->dapm.widgets, list) { + list_for_each_entry(w, &codec->card->widgets, list) { + if (w->dapm != &codec->dapm) + continue; if (strcmp(w->name, "LOUT1 PGA") == 0) wm8960->lout1 = w; if (strcmp(w->name, "ROUT1 PGA") == 0) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d4800c8..e761a47 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1696,6 +1696,7 @@ static int soc_probe(struct platform_device *pdev) INIT_LIST_HEAD(&card->dai_dev_list); INIT_LIST_HEAD(&card->codec_dev_list); INIT_LIST_HEAD(&card->platform_dev_list); + INIT_LIST_HEAD(&card->widgets); INIT_LIST_HEAD(&card->paths);
soc_init_card_debugfs(card); @@ -3275,7 +3276,6 @@ int snd_soc_register_codec(struct device *dev, return -ENOMEM; }
- INIT_LIST_HEAD(&codec->dapm.widgets); codec->dapm.bias_level = SND_SOC_BIAS_OFF; codec->dapm.dev = dev; codec->dapm.codec = codec; diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index fb2d3ce..52ff087 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -934,7 +934,9 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) /* Check which widgets we need to power and store them in * lists indicating if they should be powered up or down. */ - list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &card->widgets, list) { + if (w->dapm != dapm) + continue; switch (w->id) { case snd_soc_dapm_pre: dapm_seq_insert(w, &down_list, dapm_down_seq); @@ -972,7 +974,7 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) /* If there are no DAPM widgets then try to figure out power from the * event type. */ - if (list_empty(&dapm->widgets)) { + if (!dapm->n_widgets) { switch (event) { case SND_SOC_DAPM_STREAM_START: case SND_SOC_DAPM_STREAM_RESUME: @@ -1136,8 +1138,8 @@ void snd_soc_dapm_debugfs_init(struct snd_soc_dapm_context *dapm) if (!dapm->debugfs_dapm) return;
- list_for_each_entry(w, &dapm->widgets, list) { - if (!w->name) + list_for_each_entry(w, &dapm->card->widgets, list) { + if (!w->name || w->dapm != dapm) continue;
d = debugfs_create_file(w->name, 0444, @@ -1232,7 +1234,9 @@ static ssize_t dapm_widget_show(struct device *dev, int count = 0; char *state = "not set";
- list_for_each_entry(w, &codec->dapm.widgets, list) { + list_for_each_entry(w, &codec->card->widgets, list) { + if (w->dapm != &codec->dapm) + continue;
/* only display widgets that burnm power */ switch (w->id) { @@ -1293,7 +1297,9 @@ static void dapm_free_widgets(struct snd_soc_dapm_context *dapm) struct snd_soc_dapm_widget *w, *next_w; struct snd_soc_dapm_path *p, *next_p;
- list_for_each_entry_safe(w, next_w, &dapm->widgets, list) { + list_for_each_entry_safe(w, next_w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; list_del(&w->list); /* * remove source and sink paths associated to this widget. @@ -1323,7 +1329,9 @@ static int snd_soc_dapm_set_pin(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; if (!strcmp(w->name, pin)) { dev_dbg(w->dapm->dev, "dapm: pin %s = %d\n", pin, status); @@ -1359,22 +1367,34 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_path *path; struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w; + struct snd_soc_dapm_widget *wtsource = NULL, *wtsink = NULL; const char *sink = route->sink; const char *control = route->control; const char *source = route->source; int ret = 0;
- /* find src and dest widgets */ - list_for_each_entry(w, &dapm->widgets, list) { - + /* + * find src and dest widgets over all widgets but favor a widget from + * current DAPM context + */ + list_for_each_entry(w, &dapm->card->widgets, list) { if (!wsink && !(strcmp(w->name, sink))) { - wsink = w; + wtsink = w; + if (w->dapm == dapm) + wsink = w; continue; } if (!wsource && !(strcmp(w->name, source))) { - wsource = w; + wtsource = w; + if (w->dapm == dapm) + wsource = w; } } + /* use widget from another DAPM context if not found from this */ + if (!wsink) + wsink = wtsink; + if (!wsource) + wsource = wtsource;
if (wsource == NULL || wsink == NULL) return -ENODEV; @@ -1511,7 +1531,7 @@ int snd_soc_dapm_new_widgets(struct snd_soc_dapm_context *dapm) { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) + list_for_each_entry(w, &dapm->card->widgets, list) { if (w->new) continue; @@ -1995,12 +2015,13 @@ int snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm, if ((w = dapm_cnew_widget(widget)) == NULL) return -ENOMEM;
+ dapm->n_widgets++; w->dapm = dapm; w->codec = dapm->codec; INIT_LIST_HEAD(&w->sources); INIT_LIST_HEAD(&w->sinks); INIT_LIST_HEAD(&w->list); - list_add(&w->list, &dapm->widgets); + list_add(&w->list, &dapm->card->widgets);
/* machine layer set ups unconnected pins and insertions */ w->connected = 1; @@ -2043,9 +2064,9 @@ static void soc_dapm_stream_event(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) + list_for_each_entry(w, &dapm->card->widgets, list) { - if (!w->sname) + if (!w->sname || w->dapm != dapm) continue; dev_dbg(w->dapm->dev, "widget %s\n %s stream %s event %d\n", w->name, w->sname, stream, event); @@ -2128,7 +2149,9 @@ int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; if (!strcmp(w->name, pin)) { dev_dbg(w->dapm->dev, "dapm: force enable pin %s\n", pin); @@ -2193,7 +2216,9 @@ int snd_soc_dapm_get_pin_status(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; if (!strcmp(w->name, pin)) return w->connected; } @@ -2218,7 +2243,9 @@ int snd_soc_dapm_ignore_suspend(struct snd_soc_dapm_context *dapm, { struct snd_soc_dapm_widget *w;
- list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; if (!strcmp(w->name, pin)) { w->ignore_suspend = 1; return 0; @@ -2249,7 +2276,9 @@ static void soc_dapm_shutdown_codec(struct snd_soc_dapm_context *dapm) LIST_HEAD(down_list); int powerdown = 0;
- list_for_each_entry(w, &dapm->widgets, list) { + list_for_each_entry(w, &dapm->card->widgets, list) { + if (w->dapm != dapm) + continue; if (w->power) { dapm_seq_insert(w, &down_list, dapm_down_seq); w->power = 0;
On Fri, Nov 12, 2010 at 11:23:25AM +0200, Jarkko Nikula wrote:
Cross-device paths are registered by listing the widgets from device A in a map for device B. An example below shows a path that connects MONO out of A into Line In of B:
static const struct snd_soc_dapm_route mapA[] = { {"MONO", NULL, "DAC"}, };
static const struct snd_soc_dapm_route mapB[] = { {"Line In", NULL, "MONO"}, };
This is going to fall over if we have widgets in two devices with the same name (and especially if we have two devices of the same kind in the system). Adding source device names to the paths would probably cover it with the same format but it should be considered and noted here.
struct snd_soc_dapm_context {
- struct list_head widgets;
- int n_widgets; /* number of widgets in this context */
I'm not sure why we need to count the number of widgets here;
index 835087d..77be6b3 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -618,6 +618,7 @@ struct snd_soc_card { struct list_head platform_dev_list; struct list_head dai_dev_list;
- struct list_head widgets; struct list_head paths;
If we keep moving stuff into the card it's questionable what the context is buying us...
On Fri, 12 Nov 2010 13:18:41 +0000 Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Nov 12, 2010 at 11:23:25AM +0200, Jarkko Nikula wrote:
Cross-device paths are registered by listing the widgets from device A in a map for device B. An example below shows a path that connects MONO out of A into Line In of B:
static const struct snd_soc_dapm_route mapA[] = { {"MONO", NULL, "DAC"}, };
static const struct snd_soc_dapm_route mapB[] = { {"Line In", NULL, "MONO"}, };
This is going to fall over if we have widgets in two devices with the same name (and especially if we have two devices of the same kind in the system). Adding source device names to the paths would probably cover it with the same format but it should be considered and noted here.
That's the reason why the code favors a widget from current DAPM context in snd_soc_dapm_add_route so that no current machine would (hopefully) break because of this patch.
Of course any new code implementing cross-device paths between codecs with conflicting widget names would need the name prefixing stuff also.
struct snd_soc_dapm_context {
- struct list_head widgets;
- int n_widgets; /* number of widgets in this context */
I'm not sure why we need to count the number of widgets here;
True, this should go the next patch actually.
index 835087d..77be6b3 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -618,6 +618,7 @@ struct snd_soc_card { struct list_head platform_dev_list; struct list_head dai_dev_list;
- struct list_head widgets; struct list_head paths;
If we keep moving stuff into the card it's questionable what the context is buying us...
DAPM context represent the parent device (codecs currently and other devices in someday) and its state so I don't see we'll be removing it in near future even now the widgets and paths are moved to card.
On Fri, 12 Nov 2010 15:57:56 +0200 Jarkko Nikula jhnikula@gmail.com wrote:
On Fri, 12 Nov 2010 13:18:41 +0000 Mark Brown broonie@opensource.wolfsonmicro.com wrote:
struct snd_soc_dapm_context {
- struct list_head widgets;
- int n_widgets; /* number of widgets in this context */
I'm not sure why we need to count the number of widgets here;
True, this should go the next patch actually.
Not true, it is actually need here. We cannot detect widgetless codecs by testing list_empty(&card->widgets) after moving widgets from dapm to card so we must keep count of widgets for each DAPM context (device).
- if (list_empty(&dapm->widgets)) { + if (!dapm->n_widgets) {
On Fri, Nov 12, 2010 at 03:57:56PM +0200, Jarkko Nikula wrote:
Mark Brown broonie@opensource.wolfsonmicro.com wrote:
If we keep moving stuff into the card it's questionable what the context is buying us...
DAPM context represent the parent device (codecs currently and other devices in someday) and its state so I don't see we'll be removing it in near future even now the widgets and paths are moved to card.
Yeah, but as we keep reducing the amount of per-device state we hold that could eventually go down to zero - we keep on looking at things and deciding they're better managed over the whole card.
Power change event like stream start/stop or kcontrol change in a cross-device path originates from one device but codec bias and widget power changes must be populated to another devices on that path as well.
This patch modifies the dapm_power_widgets so that all the widgets on a sound card are checked for a power change, not just those that are specific to originating device. Also bias management is extended to check all the devices. Only exception in bias management are widgetless codecs whose bias state is changed only if power change is originating from that context.
DAPM context test is added to dapm_seq_run to take care of if power sequence extends to an another device which requires separate register writes.
Signed-off-by: Jarkko Nikula jhnikula@gmail.com --- include/sound/soc-dapm.h | 5 ++ include/sound/soc.h | 1 + sound/soc/soc-core.c | 2 + sound/soc/soc-dapm.c | 107 ++++++++++++++++++++++++++------------------- 4 files changed, 70 insertions(+), 45 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 6714fc8..6af5f66 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -478,6 +478,11 @@ struct snd_soc_dapm_context { struct device *dev; /* from parent - for debug */ struct snd_soc_codec *codec; /* parent codec */ struct snd_soc_card *card; /* parent card */ + + /* used during DAPM updates */ + int dev_power; + struct list_head list; + #ifdef CONFIG_DEBUG_FS struct dentry *debugfs_dapm; #endif diff --git a/include/sound/soc.h b/include/sound/soc.h index 77be6b3..8470059 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -620,6 +620,7 @@ struct snd_soc_card {
struct list_head widgets; struct list_head paths; + struct list_head dapm_list;
#ifdef CONFIG_DEBUG_FS struct dentry *debugfs_card_root; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e761a47..3a16096 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1454,6 +1454,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num) /* mark codec as probed and add to card codec list */ codec->probed = 1; list_add(&codec->card_list, &card->codec_dev_list); + list_add(&codec->dapm.list, &card->dapm_list); }
/* probe the platform */ @@ -1698,6 +1699,7 @@ static int soc_probe(struct platform_device *pdev) INIT_LIST_HEAD(&card->platform_dev_list); INIT_LIST_HEAD(&card->widgets); INIT_LIST_HEAD(&card->paths); + INIT_LIST_HEAD(&card->dapm_list);
soc_init_card_debugfs(card);
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 52ff087..1ec43f9 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -841,19 +841,22 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm, LIST_HEAD(pending); int cur_sort = -1; int cur_reg = SND_SOC_NOPM; + struct snd_soc_dapm_context *cur_dapm = NULL; int ret;
list_for_each_entry_safe(w, n, list, power_list) { ret = 0;
/* Do we need to apply any queued changes? */ - if (sort[w->id] != cur_sort || w->reg != cur_reg) { + if (sort[w->id] != cur_sort || w->reg != cur_reg || + w->dapm != cur_dapm) { if (!list_empty(&pending)) - dapm_seq_run_coalesced(dapm, &pending); + dapm_seq_run_coalesced(cur_dapm, &pending);
INIT_LIST_HEAD(&pending); cur_sort = -1; cur_reg = SND_SOC_NOPM; + cur_dapm = NULL; }
switch (w->id) { @@ -897,6 +900,7 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm, /* Queue it up for application */ cur_sort = sort[w->id]; cur_reg = w->reg; + cur_dapm = w->dapm; list_move(&w->power_list, &pending); break; } @@ -923,20 +927,22 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) { struct snd_soc_card *card = dapm->codec->card; struct snd_soc_dapm_widget *w; + struct snd_soc_dapm_context *d; LIST_HEAD(up_list); LIST_HEAD(down_list); int ret = 0; int power; - int sys_power = 0;
trace_snd_soc_dapm_start(card);
+ list_for_each_entry(d, &card->dapm_list, list) + if (d->n_widgets) + d->dev_power = 0; + /* Check which widgets we need to power and store them in * lists indicating if they should be powered up or down. */ list_for_each_entry(w, &card->widgets, list) { - if (w->dapm != dapm) - continue; switch (w->id) { case snd_soc_dapm_pre: dapm_seq_insert(w, &down_list, dapm_down_seq); @@ -954,7 +960,7 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) else power = 1; if (power) - sys_power = 1; + w->dapm->dev_power = 1;
if (w->power == power) continue; @@ -978,19 +984,19 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) switch (event) { case SND_SOC_DAPM_STREAM_START: case SND_SOC_DAPM_STREAM_RESUME: - sys_power = 1; + dapm->dev_power = 1; break; case SND_SOC_DAPM_STREAM_SUSPEND: - sys_power = 0; + dapm->dev_power = 0; break; case SND_SOC_DAPM_STREAM_NOP: switch (dapm->bias_level) { case SND_SOC_BIAS_STANDBY: case SND_SOC_BIAS_OFF: - sys_power = 0; + dapm->dev_power = 0; break; default: - sys_power = 1; + dapm->dev_power = 1; break; } break; @@ -999,21 +1005,24 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) } }
- if (sys_power && dapm->bias_level == SND_SOC_BIAS_OFF) { - ret = snd_soc_dapm_set_bias_level(card, dapm, - SND_SOC_BIAS_STANDBY); - if (ret != 0) - dev_err(dapm->dev, - "Failed to turn on bias: %d\n", ret); - } + list_for_each_entry(d, &dapm->card->dapm_list, list) { + if (d->dev_power && d->bias_level == SND_SOC_BIAS_OFF) { + ret = snd_soc_dapm_set_bias_level(card, d, + SND_SOC_BIAS_STANDBY); + if (ret != 0) + dev_err(d->dev, + "Failed to turn on bias: %d\n", ret); + }
- /* If we're changing to all on or all off then prepare */ - if ((sys_power && dapm->bias_level == SND_SOC_BIAS_STANDBY) || - (!sys_power && dapm->bias_level == SND_SOC_BIAS_ON)) { - ret = snd_soc_dapm_set_bias_level(card, dapm, SND_SOC_BIAS_PREPARE); - if (ret != 0) - dev_err(dapm->dev, - "Failed to prepare bias: %d\n", ret); + /* If we're changing to all on or all off then prepare */ + if ((d->dev_power && d->bias_level == SND_SOC_BIAS_STANDBY) || + (!d->dev_power && d->bias_level == SND_SOC_BIAS_ON)) { + ret = snd_soc_dapm_set_bias_level(card, d, + SND_SOC_BIAS_PREPARE); + if (ret != 0) + dev_err(d->dev, + "Failed to prepare bias: %d\n", ret); + } }
/* Power down widgets first; try to avoid amplifying pops. */ @@ -1022,29 +1031,36 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event) /* Now power up. */ dapm_seq_run(dapm, &up_list, event, dapm_up_seq);
- /* If we just powered the last thing off drop to standby bias */ - if (dapm->bias_level == SND_SOC_BIAS_PREPARE && !sys_power) { - ret = snd_soc_dapm_set_bias_level(card, dapm, SND_SOC_BIAS_STANDBY); - if (ret != 0) - dev_err(dapm->dev, - "Failed to apply standby bias: %d\n", ret); - } + list_for_each_entry(d, &dapm->card->dapm_list, list) { + /* If we just powered the last thing off drop to standby bias */ + if (d->bias_level == SND_SOC_BIAS_PREPARE && !d->dev_power) { + ret = snd_soc_dapm_set_bias_level(card, d, + SND_SOC_BIAS_STANDBY); + if (ret != 0) + dev_err(d->dev, + "Failed to apply standby bias: %d\n", + ret); + }
- /* If we're in standby and can support bias off then do that */ - if (dapm->bias_level == SND_SOC_BIAS_STANDBY && - dapm->idle_bias_off) { - ret = snd_soc_dapm_set_bias_level(card, dapm, SND_SOC_BIAS_OFF); - if (ret != 0) - dev_err(dapm->dev, - "Failed to turn off bias: %d\n", ret); - } + /* If we're in standby and can support bias off then do that */ + if (d->bias_level == SND_SOC_BIAS_STANDBY && + d->idle_bias_off) { + ret = snd_soc_dapm_set_bias_level(card, d, + SND_SOC_BIAS_OFF); + if (ret != 0) + dev_err(d->dev, + "Failed to turn off bias: %d\n", ret); + }
- /* If we just powered up then move to active bias */ - if (dapm->bias_level == SND_SOC_BIAS_PREPARE && sys_power) { - ret = snd_soc_dapm_set_bias_level(card, dapm, SND_SOC_BIAS_ON); - if (ret != 0) - dev_err(dapm->dev, - "Failed to apply active bias: %d\n", ret); + /* If we just powered up then move to active bias */ + if (d->bias_level == SND_SOC_BIAS_PREPARE && d->dev_power) { + ret = snd_soc_dapm_set_bias_level(card, d, + SND_SOC_BIAS_ON); + if (ret != 0) + dev_err(d->dev, + "Failed to apply active bias: %d\n", + ret); + } }
pop_dbg(dapm->dev, card->pop_time, @@ -2267,6 +2283,7 @@ void snd_soc_dapm_free(struct snd_soc_dapm_context *dapm) { snd_soc_dapm_sys_remove(dapm->dev); dapm_free_widgets(dapm); + list_del(&dapm->list); } EXPORT_SYMBOL_GPL(snd_soc_dapm_free);
On Fri, Nov 12, 2010 at 11:23:26AM +0200, Jarkko Nikula wrote:
list_for_each_entry_safe(w, n, list, power_list) { ret = 0;
/* Do we need to apply any queued changes? */
if (sort[w->id] != cur_sort || w->reg != cur_reg) {
if (sort[w->id] != cur_sort || w->reg != cur_reg ||
w->dapm != cur_dapm) { if (!list_empty(&pending))
dapm_seq_run_coalesced(dapm, &pending);
dapm_seq_run_coalesced(cur_dapm, &pending); INIT_LIST_HEAD(&pending); cur_sort = -1; cur_reg = SND_SOC_NOPM;
We need a corresponding change in dapm_seq_compare() to ensure that the widgets for a given card are grouped together. I'll send out some patches to do that shortly since I just noticed there's an oversight there anyway.
On Fri, Nov 12, 2010 at 11:23:23AM +0200, Jarkko Nikula wrote:
One fix is in beginning of dapm_power_widgets that d->dev_power is zeroed only
Fix relative to the previous version, this is?
for those codecs that have widgets in order to avoid unnecessary widgetless codec bias shutdowns.
Hrm, I think we should probably consider just requring all devices to have at least stub DAPM support. It's probably simpler than having to special case them, we should be able to add a utility function which adds default stubs to minimise the code impact.
On Fri, Nov 12, 2010 at 01:51:44PM +0000, Mark Brown wrote:
for those codecs that have widgets in order to avoid unnecessary widgetless codec bias shutdowns.
Hrm, I think we should probably consider just requring all devices to have at least stub DAPM support. It's probably simpler than having to special case them, we should be able to add a utility function which adds default stubs to minimise the code impact.
...and on further reflection this is actually broken for simultaneous playback and record in mainline so we probably want to do a fix there anyway.
On Fri, 12 Nov 2010 13:54:12 +0000 Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Nov 12, 2010 at 01:51:44PM +0000, Mark Brown wrote:
for those codecs that have widgets in order to avoid unnecessary widgetless codec bias shutdowns.
Hrm, I think we should probably consider just requring all devices to have at least stub DAPM support. It's probably simpler than having to special case them, we should be able to add a utility function which adds default stubs to minimise the code impact.
...and on further reflection this is actually broken for simultaneous playback and record in mainline so we probably want to do a fix there anyway.
Are you thinking to have a fix before this set or let it to float if the issue has been there already some time? I'm thinking do we have any other widgetless codecs than some dummy bluetooth etc. codec stubs.
On Fri, Nov 12, 2010 at 04:15:42PM +0200, Jarkko Nikula wrote:
Mark Brown broonie@opensource.wolfsonmicro.com wrote:
...and on further reflection this is actually broken for simultaneous playback and record in mainline so we probably want to do a fix there anyway.
Are you thinking to have a fix before this set or let it to float if the issue has been there already some time? I'm thinking do we have any other widgetless codecs than some dummy bluetooth etc. codec stubs.
One or the other. The bluetooth/baseband stubs are something else again - they're DAIs with no associated device - but I'll need to think about how bad this really is and if insisting on DAPM really is the best way forward. I'd suggest just dropping the special casing of this from your series unless you feel inspired to change stuff for 2.6.37 (in which case do the change separately, obviously) as you're not making anything worse.
On Fri, 12 Nov 2010 13:51:44 +0000 Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Nov 12, 2010 at 11:23:23AM +0200, Jarkko Nikula wrote:
One fix is in beginning of dapm_power_widgets that d->dev_power is zeroed only
Fix relative to the previous version, this is?
Yep, I noticed that previous version had the bug that widgetless codec bias would shutdown if any other device initiates power change.
participants (2)
-
Jarkko Nikula
-
Mark Brown