[PATCH 00/12] ASoC: dapm: Cleanups and speed-ups

[PATCH 00/12] ASoC: dapm: Cleanups and speed-ups

[Lars-Peter Clausen]

Hi,

This series contains a few cleanups and speed-ups for the DAPM
implementation.  It starts with removing some dead or outdated code and
optimizing some lesser used operations.

The first larger change introduced by this series is the categorization of
widgets into sinks, sources, supplies and normal widgets. This is all the
information the main DAPM algorithm needs and using these categories makes
the code simpler and faster and hopefully easier to understand.

The second major change is to use more aggressive caching on the connected
input and output paths of a widget. It takes advantage of the fact that
typically only as subsection of the DAPM graph changes per operation. This
allows to drastically reduce the number of neighbor and path checks for some
DAPM operations. The change is most effective when the system is idle and
when changing the graph routing for a widget with multiple inputs. This e.g.
happens a lot when switching the audio profile. It still has quite good
improvements for other operations as well and typically leads to at least a
50% reduction of path and neighbor checks.

Some before and after DAPM stats. All testing was done with the ADAU1761.
Before is asoc/for-next, After~1 is asoc/for-next + this series except the
last patch, After is asoc/for-next + this series.

Changing a single control on a 5 input mixer while the system is idle:

          Power  Path  Neighbour
Before:   2      25    32
After~1:  2      12    30
After:    2       1     2

Cumulative numbers for switching the audio profile which changes 7 controls
while the system is idle:

          Power  Path  Neighbour
Before:   16     141   188
After~1:  16      80   170
After:    16       7    23

Cumulative numbers for switching the audio profile which changes 7 controls
while playback is active:

          Power  Path  Neighbour
Before:   52     207   297
After~1:  51     123   273
After:    51      29   109

Starting (or stopping) the playback stream:

          Power  Path  Neighbour
Before:   35     80    125
After~1:  34     34    117
After:    34     17     69

- Lars

Lars-Peter Clausen (12):
  ASoC: dapm: Reduce number of checked paths in
    dapm_widget_in_card_paths()
  ASoC: dapm: Remove always true path source/sink checks
  ASoC: dapm: Only mark paths dirty when the connection status changed
  ASoC: dapm: Do not add un-muxed paths to MUX control
  ASoC: dapm: Do not pretend to support controls for non mixer/mux
    widgets
  ASoC: dapm: Remove special DAI widget power check functions
  ASoC: dapm: Remove path 'walked' flag
  ASoC: dapm: Introduce toplevel widget categories
  ASoC: dapm: Add a flag to mark paths connected to supply widgets
  ASoC: dapm: Mark endpoints instead of IO widgets dirty during
    suspend/resume
  ASoC: dapm: Add a few supply widget sanity checks
  ASoC: dapm: Use more aggressive caching

 include/sound/soc-dapm.h |   9 +-
 sound/soc/soc-core.c     |   8 +-
 sound/soc/soc-dapm.c     | 755 +++++++++++++++++++++++++----------------------
 3 files changed, 409 insertions(+), 363 deletions(-)

-- 
1.8.0

[PATCH 01/12] ASoC: dapm: Reduce number of checked paths in dapm_widget_in_card_paths()

[Lars-Peter Clausen]

Each widget has a list of all the paths that it is connected to. There is no
need to iterate over all paths when we are only interested in the paths of a
specific widget.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 65 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c61cb9c..b8b44c9 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3788,35 +3788,54 @@ int snd_soc_dapm_ignore_suspend(struct snd_soc_dapm_context *dapm,
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_ignore_suspend);
 
+/**
+ * dapm_is_external_path() - Checks if a path is a external path
+ * @card: The card the path belongs to
+ * @path: The path to check
+ *
+ * Returns true if the path is either between two different DAPM contexts or
+ * between two external pins of the same DAPM context. Otherwise returns
+ * false.
+ */
+static bool dapm_is_external_path(struct snd_soc_card *card,
+	struct snd_soc_dapm_path *path)
+{
+	dev_dbg(card->dev,
+		"... Path %s(id:%d dapm:%p) - %s(id:%d dapm:%p)\n",
+		path->source->name, path->source->id, path->source->dapm,
+		path->sink->name, path->sink->id, path->sink->dapm);
+
+	/* Connection between two different DAPM contexts */
+	if (path->source->dapm != path->sink->dapm)
+		return true;
+
+	/* Loopback connection from external pin to external pin */
+	if (path->sink->id == snd_soc_dapm_input) {
+		switch (path->source->id) {
+		case snd_soc_dapm_output:
+		case snd_soc_dapm_micbias:
+			return true;
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static bool snd_soc_dapm_widget_in_card_paths(struct snd_soc_card *card,
 					      struct snd_soc_dapm_widget *w)
 {
 	struct snd_soc_dapm_path *p;
 
-	list_for_each_entry(p, &card->paths, list) {
-		if ((p->source == w) || (p->sink == w)) {
-			dev_dbg(card->dev,
-			    "... Path %s(id:%d dapm:%p) - %s(id:%d dapm:%p)\n",
-			    p->source->name, p->source->id, p->source->dapm,
-			    p->sink->name, p->sink->id, p->sink->dapm);
+	list_for_each_entry(p, &w->sources, list_sink) {
+		if (dapm_is_external_path(card, p))
+			return true;
+	}
 
-			/* Connected to something other than the codec */
-			if (p->source->dapm != p->sink->dapm)
-				return true;
-			/*
-			 * Loopback connection from codec external pin to
-			 * codec external pin
-			 */
-			if (p->sink->id == snd_soc_dapm_input) {
-				switch (p->source->id) {
-				case snd_soc_dapm_output:
-				case snd_soc_dapm_micbias:
-					return true;
-				default:
-					break;
-				}
-			}
-		}
+	list_for_each_entry(p, &w->sinks, list_source) {
+		if (dapm_is_external_path(card, p))
+			return true;
 	}
 
 	return false;
-- 
1.8.0

[PATCH 02/12] ASoC: dapm: Remove always true path source/sink checks

[Lars-Peter Clausen]

A path has always a valid source and a valid sink otherwise we wouldn't add
it in the first place. Hence all tests that check if sink/source is non NULL
always evaluate to true and can be removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b8b44c9..12f9f5f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -909,7 +909,7 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
 
 		trace_snd_soc_dapm_output_path(widget, path);
 
-		if (path->sink && path->connect) {
+		if (path->connect) {
 			path->walked = 1;
 			path->walking = 1;
 
@@ -1017,7 +1017,7 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
 
 		trace_snd_soc_dapm_input_path(widget, path);
 
-		if (path->source && path->connect) {
+		if (path->connect) {
 			path->walked = 1;
 			path->walking = 1;
 
@@ -1219,9 +1219,6 @@ static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
 		    !path->connected(path->source, path->sink))
 			continue;
 
-		if (!path->sink)
-			continue;
-
 		if (dapm_widget_power_check(path->sink))
 			return 1;
 	}
@@ -1636,12 +1633,9 @@ static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
 	/* If we changed our power state perhaps our neigbours changed
 	 * also.
 	 */
-	list_for_each_entry(path, &w->sources, list_sink) {
-		if (path->source) {
-			dapm_widget_set_peer_power(path->source, power,
-						   path->connect);
-		}
-	}
+	list_for_each_entry(path, &w->sources, list_sink)
+		dapm_widget_set_peer_power(path->source, power, path->connect);
+
 	switch (w->id) {
 	case snd_soc_dapm_supply:
 	case snd_soc_dapm_regulator_supply:
@@ -1650,12 +1644,9 @@ static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
 		/* Supplies can't affect their outputs, only their inputs */
 		break;
 	default:
-		list_for_each_entry(path, &w->sinks, list_source) {
-			if (path->sink) {
-				dapm_widget_set_peer_power(path->sink, power,
-							   path->connect);
-			}
-		}
+		list_for_each_entry(path, &w->sinks, list_source)
+			dapm_widget_set_peer_power(path->sink, power,
+						   path->connect);
 		break;
 	}
 
-- 
1.8.0

[PATCH 03/12] ASoC: dapm: Only mark paths dirty when the connection status changed

[Lars-Peter Clausen]

Rework soc_dapm_{mixer,mux}_update_power() to only mark a path dirty if the
connect state if the path has actually changed. This avoids unnecessary
power state checks for the widgets involved.

Also factor out the common code that is involved in this into a helper
function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 12f9f5f..518c532 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2002,12 +2002,35 @@ static inline void dapm_debugfs_cleanup(struct snd_soc_dapm_context *dapm)
 
 #endif
 
+/**
+ * soc_dapm_connect_path() - Connects or disconnects a path
+ * @path: The path to update
+ * @connect: The new connect state of the path. True if the path is connected,
+ *  false if it is disconneted.
+ * @reason: The reason why the path changed (for debugging only)
+ *
+ * Returns true if the path connect state changed, false otherwise.
+ */
+static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
+	bool connect, const char *reason)
+{
+	if (path->connect == connect)
+		return false;
+
+	path->connect = connect;
+	dapm_mark_dirty(path->source, reason);
+	dapm_mark_dirty(path->sink, reason);
+
+	return true;
+}
+
 /* test and update the power status of a mux widget */
 static int soc_dapm_mux_update_power(struct snd_soc_card *card,
 				 struct snd_kcontrol *kcontrol, int mux, struct soc_enum *e)
 {
 	struct snd_soc_dapm_path *path;
 	int found = 0;
+	bool connect;
 
 	lockdep_assert_held(&card->dapm_mutex);
 
@@ -2018,16 +2041,12 @@ static int soc_dapm_mux_update_power(struct snd_soc_card *card,
 
 		found = 1;
 		/* we now need to match the string in the enum to the path */
-		if (!(strcmp(path->name, e->texts[mux]))) {
-			path->connect = 1; /* new connection */
-			dapm_mark_dirty(path->source, "mux connection");
-		} else {
-			if (path->connect)
-				dapm_mark_dirty(path->source,
-						"mux disconnection");
-			path->connect = 0; /* old connection must be powered down */
-		}
-		dapm_mark_dirty(path->sink, "mux change");
+		if (!(strcmp(path->name, e->texts[mux])))
+			connect = true;
+		else
+			connect = false;
+
+		soc_dapm_connect_path(path, connect, "mux update");
 	}
 
 	if (found)
@@ -2066,9 +2085,7 @@ static int soc_dapm_mixer_update_power(struct snd_soc_card *card,
 	/* find dapm widget path assoc with kcontrol */
 	dapm_kcontrol_for_each_path(path, kcontrol) {
 		found = 1;
-		path->connect = connect;
-		dapm_mark_dirty(path->source, "mixer connection");
-		dapm_mark_dirty(path->sink, "mixer update");
+		soc_dapm_connect_path(path, connect, "mixer update");
 	}
 
 	if (found)
-- 
1.8.0

[PATCH 04/12] ASoC: dapm: Do not add un-muxed paths to MUX control

[Lars-Peter Clausen]

Paths that are directly connected to a MUX widget are not affected by
changes to the MUX's control. Rather than checking if a path is directly
connected each time the MUX is updated do it only once when MUX is created.

We can also remove the check for e->texts[mux] != NULL, since if that
condition was true the code would have had already crashed much earlier (And
generally speaking if a enum's 'texts' entry is NULL it's a bug in the
driver).

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 518c532..bc8329b 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -738,8 +738,10 @@ static int dapm_new_mux(struct snd_soc_dapm_widget *w)
 	if (ret < 0)
 		return ret;
 
-	list_for_each_entry(path, &w->sources, list_sink)
-		dapm_kcontrol_add_path(w->kcontrols[0], path);
+	list_for_each_entry(path, &w->sources, list_sink) {
+		if (path->name)
+			dapm_kcontrol_add_path(w->kcontrols[0], path);
+	}
 
 	return 0;
 }
@@ -2036,9 +2038,6 @@ static int soc_dapm_mux_update_power(struct snd_soc_card *card,
 
 	/* find dapm widget path assoc with kcontrol */
 	dapm_kcontrol_for_each_path(path, kcontrol) {
-		if (!path->name || !e->texts[mux])
-			continue;
-
 		found = 1;
 		/* we now need to match the string in the enum to the path */
 		if (!(strcmp(path->name, e->texts[mux])))
-- 
1.8.0

[PATCH 05/12] ASoC: dapm: Do not pretend to support controls for non mixer/mux widgets

[Lars-Peter Clausen]

Controls on a path only have an effect if the sink on the path is either a
mixer or mux widget. Currently we sort of silently ignore controls on other
paths, but since they don't do anything having them on other paths does not
make much sense and it is probably safe to assume that if we see such a path
it is a mistake in the driver that registered the path. This patch modifies
snd_soc_dapm_add_path() to report an error if a path with and control is
encountered where we didn't expect a control. This also allows to simplify
the code quite a bit.

The patch also moves the connecting of the path lists out of
dapm_connect_mux() and dapm_connect_mixer() into snd_soc_dapm_add_path().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 112 +++++++++++++++++----------------------------------
 1 file changed, 37 insertions(+), 75 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index bc8329b..0e1ae6f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -469,10 +469,9 @@ out:
 
 /* connect mux widget to its interconnecting audio paths */
 static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
-	struct snd_soc_dapm_widget *src, struct snd_soc_dapm_widget *dest,
-	struct snd_soc_dapm_path *path, const char *control_name,
-	const struct snd_kcontrol_new *kcontrol)
+	struct snd_soc_dapm_path *path, const char *control_name)
 {
+	const struct snd_kcontrol_new *kcontrol = &path->sink->kcontrol_news[0];
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int val, item;
 	int i;
@@ -493,9 +492,6 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 
 	for (i = 0; i < e->items; i++) {
 		if (!(strcmp(control_name, e->texts[i]))) {
-			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];
 			if (i == item)
 				path->connect = 1;
@@ -509,11 +505,10 @@ static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 }
 
 /* set up initial codec paths */
-static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w,
-	struct snd_soc_dapm_path *p, int i)
+static void dapm_set_mixer_path_status(struct snd_soc_dapm_path *p, int i)
 {
 	struct soc_mixer_control *mc = (struct soc_mixer_control *)
-		w->kcontrol_news[i].private_value;
+		p->sink->kcontrol_news[i].private_value;
 	unsigned int reg = mc->reg;
 	unsigned int shift = mc->shift;
 	unsigned int max = mc->max;
@@ -522,7 +517,7 @@ static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w,
 	unsigned int val;
 
 	if (reg != SND_SOC_NOPM) {
-		soc_dapm_read(w->dapm, reg, &val);
+		soc_dapm_read(p->sink->dapm, reg, &val);
 		val = (val >> shift) & mask;
 		if (invert)
 			val = max - val;
@@ -534,19 +529,15 @@ static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w,
 
 /* connect mixer widget to its interconnecting audio paths */
 static int dapm_connect_mixer(struct snd_soc_dapm_context *dapm,
-	struct snd_soc_dapm_widget *src, struct snd_soc_dapm_widget *dest,
 	struct snd_soc_dapm_path *path, const char *control_name)
 {
 	int i;
 
 	/* search for mixer kcontrol */
-	for (i = 0; i < dest->num_kcontrols; i++) {
-		if (!strcmp(control_name, dest->kcontrol_news[i].name)) {
-			list_add(&path->list, &dapm->card->paths);
-			list_add(&path->list_sink, &dest->sources);
-			list_add(&path->list_source, &src->sinks);
-			path->name = dest->kcontrol_news[i].name;
-			dapm_set_mixer_path_status(dest, path, i);
+	for (i = 0; i < path->sink->num_kcontrols; i++) {
+		if (!strcmp(control_name, path->sink->kcontrol_news[i].name)) {
+			path->name = path->sink->kcontrol_news[i].name;
+			dapm_set_mixer_path_status(path, i);
 			return 0;
 		}
 	}
@@ -2353,69 +2344,40 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 			wsource->ext = 1;
 	}
 
-	dapm_mark_dirty(wsource, "Route added");
-	dapm_mark_dirty(wsink, "Route added");
-
 	/* connect static paths */
 	if (control == NULL) {
-		list_add(&path->list, &dapm->card->paths);
-		list_add(&path->list_sink, &wsink->sources);
-		list_add(&path->list_source, &wsource->sinks);
 		path->connect = 1;
-		return 0;
-	}
-
-	/* connect dynamic paths */
-	switch (wsink->id) {
-	case snd_soc_dapm_adc:
-	case snd_soc_dapm_dac:
-	case snd_soc_dapm_pga:
-	case snd_soc_dapm_out_drv:
-	case snd_soc_dapm_input:
-	case snd_soc_dapm_output:
-	case snd_soc_dapm_siggen:
-	case snd_soc_dapm_micbias:
-	case snd_soc_dapm_vmid:
-	case snd_soc_dapm_pre:
-	case snd_soc_dapm_post:
-	case snd_soc_dapm_supply:
-	case snd_soc_dapm_regulator_supply:
-	case snd_soc_dapm_clock_supply:
-	case snd_soc_dapm_aif_in:
-	case snd_soc_dapm_aif_out:
-	case snd_soc_dapm_dai_in:
-	case snd_soc_dapm_dai_out:
-	case snd_soc_dapm_dai_link:
-	case snd_soc_dapm_kcontrol:
-		list_add(&path->list, &dapm->card->paths);
-		list_add(&path->list_sink, &wsink->sources);
-		list_add(&path->list_source, &wsource->sinks);
-		path->connect = 1;
-		return 0;
-	case snd_soc_dapm_mux:
-		ret = dapm_connect_mux(dapm, wsource, wsink, path, control,
-			&wsink->kcontrol_news[0]);
-		if (ret != 0)
-			goto err;
-		break;
-	case snd_soc_dapm_switch:
-	case snd_soc_dapm_mixer:
-	case snd_soc_dapm_mixer_named_ctl:
-		ret = dapm_connect_mixer(dapm, wsource, wsink, path, control);
-		if (ret != 0)
+	} else {
+		/* connect dynamic paths */
+		switch (wsink->id) {
+		case snd_soc_dapm_mux:
+			ret = dapm_connect_mux(dapm, path, control);
+			if (ret != 0)
+				goto err;
+			break;
+		case snd_soc_dapm_switch:
+		case snd_soc_dapm_mixer:
+		case snd_soc_dapm_mixer_named_ctl:
+			ret = dapm_connect_mixer(dapm, path, control);
+			if (ret != 0)
+				goto err;
+			break;
+		default:
+			dev_err(dapm->dev,
+				"Control not supported for path %s -> [%s] -> %s\n",
+				wsource->name, control, wsink->name);
+			ret = -EINVAL;
 			goto err;
-		break;
-	case snd_soc_dapm_hp:
-	case snd_soc_dapm_mic:
-	case snd_soc_dapm_line:
-	case snd_soc_dapm_spk:
-		list_add(&path->list, &dapm->card->paths);
-		list_add(&path->list_sink, &wsink->sources);
-		list_add(&path->list_source, &wsource->sinks);
-		path->connect = 0;
-		return 0;
+		}
 	}
 
+	list_add(&path->list, &dapm->card->paths);
+	list_add(&path->list_sink, &wsink->sources);
+	list_add(&path->list_source, &wsource->sinks);
+
+	dapm_mark_dirty(wsource, "Route added");
+	dapm_mark_dirty(wsink, "Route added");
+
 	return 0;
 err:
 	kfree(path);
-- 
1.8.0

[PATCH 06/12] ASoC: dapm: Remove special DAI widget power check functions

[Lars-Peter Clausen]

dapm_adc_check_power() checks if the widget is active, if yes it only checks
whether there are any connected input paths. Otherwise it calls
dapm_generic_check_power() which will check for both connected input and
output paths. But the function that checks for connected output paths will
return true if the widget is a active sink. Which means the generic power
check function will work just fine and there is no need for a special power
check function.

The same applies for dapm_dac_check_power(), but with input and output paths
reversed.

This patch removes both dapm_adc_check_power() and dapm_dac_check_power()
and replace their usage with dapm_generic_check_power().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 40 ++--------------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 0e1ae6f..8f40c35 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -1162,38 +1162,6 @@ static int dapm_generic_check_power(struct snd_soc_dapm_widget *w)
 	return out != 0 && in != 0;
 }
 
-/* Check to see if an ADC has power */
-static int dapm_adc_check_power(struct snd_soc_dapm_widget *w)
-{
-	int in;
-
-	DAPM_UPDATE_STAT(w, power_checks);
-
-	if (w->active) {
-		in = is_connected_input_ep(w, NULL);
-		dapm_clear_walk_input(w->dapm, &w->sources);
-		return in != 0;
-	} else {
-		return dapm_generic_check_power(w);
-	}
-}
-
-/* Check to see if a DAC has power */
-static int dapm_dac_check_power(struct snd_soc_dapm_widget *w)
-{
-	int out;
-
-	DAPM_UPDATE_STAT(w, power_checks);
-
-	if (w->active) {
-		out = is_connected_output_ep(w, NULL);
-		dapm_clear_walk_output(w->dapm, &w->sinks);
-		return out != 0;
-	} else {
-		return dapm_generic_check_power(w);
-	}
-}
-
 /* Check to see if a power supply is needed */
 static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
 {
@@ -3064,12 +3032,6 @@ snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
 	case snd_soc_dapm_mux:
 		w->power_check = dapm_generic_check_power;
 		break;
-	case snd_soc_dapm_dai_out:
-		w->power_check = dapm_adc_check_power;
-		break;
-	case snd_soc_dapm_dai_in:
-		w->power_check = dapm_dac_check_power;
-		break;
 	case snd_soc_dapm_adc:
 	case snd_soc_dapm_aif_out:
 	case snd_soc_dapm_dac:
@@ -3084,6 +3046,8 @@ snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
 	case snd_soc_dapm_mic:
 	case snd_soc_dapm_line:
 	case snd_soc_dapm_dai_link:
+	case snd_soc_dapm_dai_out:
+	case snd_soc_dapm_dai_in:
 		w->power_check = dapm_generic_check_power;
 		break;
 	case snd_soc_dapm_supply:
-- 
1.8.0

[PATCH 07/12] ASoC: dapm: Remove path 'walked' flag

[Lars-Peter Clausen]

The 'walked' flag was used to avoid walking paths that have already been
walked. But since we started caching the number of inputs and outputs of a
path we never actually get into a situation where we try to walk a path that
has the 'walked' flag set.

There are two cases in which we can end up walking a path multiple times
within a single run of is_connected_output_ep() or is_connected_input_ep().

1) If a path splits up and rejoins later:

	     .--> C ---v
	A -> B         E --> F
	     '--> D ---^

When walking from A to F we'll end up at E twice, once via C and once via D.
But since we do a depth first search we'll fully discover the path and
initialize the number of outputs/inputs of the widget the first time we get
there. The second time we get there we'll use the cached value and not
bother to check any of the paths again. So we'll never see a path where
'walked' is set in this case.

2) If there is a circle:

	A --> B <-- C <-.--> F
	      '--> D ---'

When walking from A to F we'll end up twice at B. But since there is a
circle the 'walking' flag will still be set on B once we get there the
second time. This means we won't look at any of it's outgoing paths. So in
this case we won't ever see a path where 'walked' is set either.

So it is safe to remove the flag. This on one hand means we remove some
always true checks from one of the hottest paths of the DAPM algorithm and
on the other hand means we do not have to do the tedious clearing of the
flag after checking the number inputs or outputs of a widget.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc-dapm.h |  1 -
 sound/soc/soc-dapm.c     | 49 ++----------------------------------------------
 2 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 3a4d7da..ebb93f2 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -508,7 +508,6 @@ struct snd_soc_dapm_path {
 
 	/* status */
 	u32 connect:1;	/* source and sink widgets are connected */
-	u32 walked:1;	/* path has been walked */
 	u32 walking:1;  /* path is in the process of being walked */
 	u32 weak:1;	/* path ignored for power management */
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 8f40c35..b5f2f4d8 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -747,34 +747,6 @@ static int dapm_new_pga(struct snd_soc_dapm_widget *w)
 	return 0;
 }
 
-/* reset 'walked' bit for each dapm path */
-static void dapm_clear_walk_output(struct snd_soc_dapm_context *dapm,
-				   struct list_head *sink)
-{
-	struct snd_soc_dapm_path *p;
-
-	list_for_each_entry(p, sink, list_source) {
-		if (p->walked) {
-			p->walked = 0;
-			dapm_clear_walk_output(dapm, &p->sink->sinks);
-		}
-	}
-}
-
-static void dapm_clear_walk_input(struct snd_soc_dapm_context *dapm,
-				  struct list_head *source)
-{
-	struct snd_soc_dapm_path *p;
-
-	list_for_each_entry(p, source, list_sink) {
-		if (p->walked) {
-			p->walked = 0;
-			dapm_clear_walk_input(dapm, &p->source->sources);
-		}
-	}
-}
-
-
 /* We implement power down on suspend by checking the power state of
  * the ALSA card - when we are suspending the ALSA state for the card
  * is set to D3.
@@ -897,13 +869,9 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
 		if (path->walking)
 			return 1;
 
-		if (path->walked)
-			continue;
-
 		trace_snd_soc_dapm_output_path(widget, path);
 
 		if (path->connect) {
-			path->walked = 1;
 			path->walking = 1;
 
 			/* do we need to add this widget to the list ? */
@@ -1005,13 +973,9 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
 		if (path->walking)
 			return 1;
 
-		if (path->walked)
-			continue;
-
 		trace_snd_soc_dapm_input_path(widget, path);
 
 		if (path->connect) {
-			path->walked = 1;
 			path->walking = 1;
 
 			/* do we need to add this widget to the list ? */
@@ -1059,15 +1023,10 @@ int snd_soc_dapm_dai_get_connected_widgets(struct snd_soc_dai *dai, int stream,
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
 	dapm_reset(card);
 
-	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 		paths = is_connected_output_ep(dai->playback_widget, list);
-		dapm_clear_walk_output(&card->dapm,
-				       &dai->playback_widget->sinks);
-	} else {
+	else
 		paths = is_connected_input_ep(dai->capture_widget, list);
-		dapm_clear_walk_input(&card->dapm,
-				      &dai->capture_widget->sources);
-	}
 
 	trace_snd_soc_dapm_connected(paths, stream);
 	mutex_unlock(&card->dapm_mutex);
@@ -1156,9 +1115,7 @@ static int dapm_generic_check_power(struct snd_soc_dapm_widget *w)
 	DAPM_UPDATE_STAT(w, power_checks);
 
 	in = is_connected_input_ep(w, NULL);
-	dapm_clear_walk_input(w->dapm, &w->sources);
 	out = is_connected_output_ep(w, NULL);
-	dapm_clear_walk_output(w->dapm, &w->sinks);
 	return out != 0 && in != 0;
 }
 
@@ -1816,9 +1773,7 @@ static ssize_t dapm_widget_power_read_file(struct file *file,
 		return -ENOMEM;
 
 	in = is_connected_input_ep(w, NULL);
-	dapm_clear_walk_input(w->dapm, &w->sources);
 	out = is_connected_output_ep(w, NULL);
-	dapm_clear_walk_output(w->dapm, &w->sinks);
 
 	ret = snprintf(buf, PAGE_SIZE, "%s: %s%s  in %d out %d",
 		       w->name, w->power ? "On" : "Off",
-- 
1.8.0

[PATCH 08/12] ASoC: dapm: Introduce toplevel widget categories

[Lars-Peter Clausen]

DAPM widgets can be classified into four categories:
	* supply: Supply widgets do not affect the power state of their
		non-supply widget neighbors and unlike other widgets a
		supply widget is not powered up when it is on an active
		path, but when at least on of its neighbors is powered up.
	* source: A source is a widget that receives data from outside the
		DAPM graph or generates data. This can for example be a
		microphone, the playback DMA or a signal generator. A source
		widget will be considered powered up if there is an active
		path to a sink widget.
	* sink: A sink is a widget that transmits data to somewhere outside
		of the DAPM graph. This can e.g. be a speaker or the capture
		DMA. A sink widget will be considered powered up if there is
		an active path from a source widget.
	* normal: Normal widgets are widgets not covered by the categories
		above. A normal widget will be considered powered up if it
		is on an active path between a source widget and a sink
		widget.

The way the number of input and output paths for a widget is calculated
depends on its category. There are a bunch of factors which decide which
category a widget is. Currently there is no formal classification of these
categories and we calculate the category of the widget based on these
factors whenever we want to know it. This is at least once for every widget
during each power update sequence. The factors which determine the category
of the widgets are mostly static though and if at all change rather seldom.
This patch introduces three new per widget flags, one for each of non-normal
widgets categories. Instead of re-computing the category each time we want
to know them the flags will be checked. For the majority of widgets the
category is solely determined by the widget id, which means it never changes
and only has to be set once when the widget is created. The only widgets
with dynamic categories are:

	snd_soc_dapm_dai_out: Is considered a sink iff the capture stream is
		active, otherwise normal.
	snd_soc_dapm_dai_in: Is considered a source iff the playback stream
		is active, otherwise normal.
	snd_soc_dapm_input: Is considered a sink iff it has no outgoing
		paths, otherwise normal.
	snd_soc_dapm_output: Is considered a source iff it has no incoming
		paths, otherwise normal.
	snd_soc_dapm_line: Is considered a sink iff it has no outgoing paths
		and is considered a source iff it has no incoming paths,
		otherwise normal.

For snd_soc_dapm_dai_out/snd_soc_dapm_dai_in widgets the category will be
updated when a stream is started or stopped. For the other dynamic widgets
the category will be updated when a path connecting to it is added or
removed.

Introducing those new widget categories allows to make
is_connected_{output,input}_ep, which are among the hottest paths of the
DAPM algorithm, more generic and significantly shorter.

The before and after sizes for is_connected_{output,input}_ep are:

On ARM (defconfig + CONFIG_SND_SOC):
	function                                     old     new   delta
	is_connected_output_ep                       480     340    -140
	is_connected_input_ep                        456     352    -104

On amd64 (defconfig + CONFIG_SND_SOC):
	function                                     old     new   delta
	is_connected_output_ep                       579     427    -152
	is_connected_input_ep                        563     427    -136

Which is about a 25%-30% decrease, other architectures are expected to have
similar numbers. At the same time the size of the snd_soc_dapm_widget struct
does not change since the new flags are stored in the same word as the
existing flags.

Note: that since the per widget 'ext' flag was only used to decide whether a
snd_soc_dapm_input or snd_soc_dapm_output widget was a source or a sink it
is now unused and can be removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc-dapm.h |   4 +-
 sound/soc/soc-dapm.c     | 216 +++++++++++++++++++++--------------------------
 2 files changed, 100 insertions(+), 120 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index ebb93f2..8cf3aad 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -543,11 +543,13 @@ struct snd_soc_dapm_widget {
 	unsigned char active:1;			/* active stream on DAC, ADC's */
 	unsigned char connected:1;		/* connected codec pin */
 	unsigned char new:1;			/* cnew complete */
-	unsigned char ext:1;			/* has external widgets */
 	unsigned char force:1;			/* force state */
 	unsigned char ignore_suspend:1;         /* kept enabled over suspend */
 	unsigned char new_power:1;		/* power from this run */
 	unsigned char power_checked:1;		/* power checked this run */
+	unsigned char is_supply:1;		/* Widget is a supply type widget */
+	unsigned char is_sink:1;		/* Widget is a sink type widget */
+	unsigned char is_source:1;		/* Widget is a source type widget */
 	int subseq;				/* sort within widget type */
 
 	int (*power_check)(struct snd_soc_dapm_widget *w);
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b5f2f4d8..578c119 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -821,43 +821,12 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
 
 	DAPM_UPDATE_STAT(widget, path_checks);
 
-	switch (widget->id) {
-	case snd_soc_dapm_supply:
-	case snd_soc_dapm_regulator_supply:
-	case snd_soc_dapm_clock_supply:
-	case snd_soc_dapm_kcontrol:
+	if (widget->is_supply)
 		return 0;
-	default:
-		break;
-	}
-
-	switch (widget->id) {
-	case snd_soc_dapm_adc:
-	case snd_soc_dapm_aif_out:
-	case snd_soc_dapm_dai_out:
-		if (widget->active) {
-			widget->outputs = snd_soc_dapm_suspend_check(widget);
-			return widget->outputs;
-		}
-	default:
-		break;
-	}
 
-	if (widget->connected) {
-		/* connected pin ? */
-		if (widget->id == snd_soc_dapm_output && !widget->ext) {
-			widget->outputs = snd_soc_dapm_suspend_check(widget);
-			return widget->outputs;
-		}
-
-		/* connected jack or spk ? */
-		if (widget->id == snd_soc_dapm_hp ||
-		    widget->id == snd_soc_dapm_spk ||
-		    (widget->id == snd_soc_dapm_line &&
-		     !list_empty(&widget->sources))) {
-			widget->outputs = snd_soc_dapm_suspend_check(widget);
-			return widget->outputs;
-		}
+	if (widget->is_sink && widget->connected) {
+		widget->outputs = snd_soc_dapm_suspend_check(widget);
+		return widget->outputs;
 	}
 
 	list_for_each_entry(path, &widget->sinks, list_source) {
@@ -913,55 +882,12 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
 
 	DAPM_UPDATE_STAT(widget, path_checks);
 
-	switch (widget->id) {
-	case snd_soc_dapm_supply:
-	case snd_soc_dapm_regulator_supply:
-	case snd_soc_dapm_clock_supply:
-	case snd_soc_dapm_kcontrol:
+	if (widget->is_supply)
 		return 0;
-	default:
-		break;
-	}
-
-	/* active stream ? */
-	switch (widget->id) {
-	case snd_soc_dapm_dac:
-	case snd_soc_dapm_aif_in:
-	case snd_soc_dapm_dai_in:
-		if (widget->active) {
-			widget->inputs = snd_soc_dapm_suspend_check(widget);
-			return widget->inputs;
-		}
-	default:
-		break;
-	}
-
-	if (widget->connected) {
-		/* connected pin ? */
-		if (widget->id == snd_soc_dapm_input && !widget->ext) {
-			widget->inputs = snd_soc_dapm_suspend_check(widget);
-			return widget->inputs;
-		}
-
-		/* connected VMID/Bias for lower pops */
-		if (widget->id == snd_soc_dapm_vmid) {
-			widget->inputs = snd_soc_dapm_suspend_check(widget);
-			return widget->inputs;
-		}
 
-		/* connected jack ? */
-		if (widget->id == snd_soc_dapm_mic ||
-		    (widget->id == snd_soc_dapm_line &&
-		     !list_empty(&widget->sinks))) {
-			widget->inputs = snd_soc_dapm_suspend_check(widget);
-			return widget->inputs;
-		}
-
-		/* signal generator */
-		if (widget->id == snd_soc_dapm_siggen) {
-			widget->inputs = snd_soc_dapm_suspend_check(widget);
-			return widget->inputs;
-		}
+	if (widget->is_source && widget->connected) {
+		widget->inputs = snd_soc_dapm_suspend_check(widget);
+		return widget->inputs;
 	}
 
 	list_for_each_entry(path, &widget->sources, list_sink) {
@@ -1554,18 +1480,11 @@ static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
 	list_for_each_entry(path, &w->sources, list_sink)
 		dapm_widget_set_peer_power(path->source, power, path->connect);
 
-	switch (w->id) {
-	case snd_soc_dapm_supply:
-	case snd_soc_dapm_regulator_supply:
-	case snd_soc_dapm_clock_supply:
-	case snd_soc_dapm_kcontrol:
-		/* Supplies can't affect their outputs, only their inputs */
-		break;
-	default:
+	/* Supplies can't affect their outputs, only their inputs */
+	if (!w->is_supply) {
 		list_for_each_entry(path, &w->sinks, list_source)
 			dapm_widget_set_peer_power(path->sink, power,
 						   path->connect);
-		break;
 	}
 
 	if (power)
@@ -2230,6 +2149,53 @@ int snd_soc_dapm_sync(struct snd_soc_dapm_context *dapm)
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_sync);
 
+/**
+ * dapm_update_widget_flags() - Re-compute widget sink and source flags
+ * @w: The widgets for which to update the flags
+ *
+ * Some widgets have a dynamic category which depends on to which neighbors they
+ * are connected. This function update the category for these widgets.
+ *
+ * This function must be called whenever a path is added or removed to a widget.
+ */
+static void dapm_update_widget_flags(struct snd_soc_dapm_widget *w)
+{
+	struct snd_soc_dapm_path *p;
+
+	switch (w->id) {
+	case snd_soc_dapm_input:
+		w->is_source = 1;
+		list_for_each_entry(p, &w->sources, list_sink) {
+			if (p->source->id == snd_soc_dapm_micbias ||
+				p->source->id == snd_soc_dapm_mic ||
+				p->source->id == snd_soc_dapm_line ||
+				p->source->id == snd_soc_dapm_output) {
+					w->is_source = 0;
+					break;
+			}
+		}
+		break;
+	case snd_soc_dapm_output:
+		w->is_sink = 1;
+		list_for_each_entry(p, &w->sinks, list_source) {
+			if (p->sink->id == snd_soc_dapm_spk ||
+				p->sink->id == snd_soc_dapm_hp ||
+				p->sink->id == snd_soc_dapm_line ||
+				p->sink->id == snd_soc_dapm_input) {
+					w->is_sink = 0;
+					break;
+			}
+		}
+		break;
+	case snd_soc_dapm_line:
+		w->is_sink = !list_empty(&w->sources);
+		w->is_source = !list_empty(&w->sinks);
+		break;
+	default:
+		break;
+	}
+}
+
 static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	struct snd_soc_dapm_widget *wsource, struct snd_soc_dapm_widget *wsink,
 	const char *control,
@@ -2251,22 +2217,6 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	INIT_LIST_HEAD(&path->list_source);
 	INIT_LIST_HEAD(&path->list_sink);
 
-	/* check for external widgets */
-	if (wsink->id == snd_soc_dapm_input) {
-		if (wsource->id == snd_soc_dapm_micbias ||
-			wsource->id == snd_soc_dapm_mic ||
-			wsource->id == snd_soc_dapm_line ||
-			wsource->id == snd_soc_dapm_output)
-			wsink->ext = 1;
-	}
-	if (wsource->id == snd_soc_dapm_output) {
-		if (wsink->id == snd_soc_dapm_spk ||
-			wsink->id == snd_soc_dapm_hp ||
-			wsink->id == snd_soc_dapm_line ||
-			wsink->id == snd_soc_dapm_input)
-			wsource->ext = 1;
-	}
-
 	/* connect static paths */
 	if (control == NULL) {
 		path->connect = 1;
@@ -2298,6 +2248,9 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	list_add(&path->list_sink, &wsink->sources);
 	list_add(&path->list_source, &wsource->sinks);
 
+	dapm_update_widget_flags(wsource);
+	dapm_update_widget_flags(wsink);
+
 	dapm_mark_dirty(wsource, "Route added");
 	dapm_mark_dirty(wsink, "Route added");
 
@@ -2382,6 +2335,7 @@ static int snd_soc_dapm_del_route(struct snd_soc_dapm_context *dapm,
 				  const struct snd_soc_dapm_route *route)
 {
 	struct snd_soc_dapm_path *path, *p;
+	struct snd_soc_dapm_widget *wsource, *wsink;
 	const char *sink;
 	const char *source;
 	char prefixed_sink[80];
@@ -2418,10 +2372,17 @@ static int snd_soc_dapm_del_route(struct snd_soc_dapm_context *dapm,
 	}
 
 	if (path) {
-		dapm_mark_dirty(path->source, "Route removed");
-		dapm_mark_dirty(path->sink, "Route removed");
+		wsource = path->source;
+		wsink = path->sink;
+
+		dapm_mark_dirty(wsource, "Route removed");
+		dapm_mark_dirty(wsink, "Route removed");
 
 		dapm_free_path(path);
+
+		/* Update any path related flags */
+		dapm_update_widget_flags(wsource);
+		dapm_update_widget_flags(wsink);
 	} else {
 		dev_warn(dapm->dev, "ASoC: Route %s->%s does not exist\n",
 			 source, sink);
@@ -2979,27 +2940,38 @@ snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
 	}
 
 	switch (w->id) {
-	case snd_soc_dapm_switch:
-	case snd_soc_dapm_mixer:
-	case snd_soc_dapm_mixer_named_ctl:
+	case snd_soc_dapm_mic:
+	case snd_soc_dapm_input:
+		w->is_source = 1;
 		w->power_check = dapm_generic_check_power;
 		break;
-	case snd_soc_dapm_mux:
+	case snd_soc_dapm_spk:
+	case snd_soc_dapm_hp:
+	case snd_soc_dapm_output:
+		w->is_sink = 1;
 		w->power_check = dapm_generic_check_power;
 		break;
+	case snd_soc_dapm_line:
+		w->is_sink = 0;
+		w->is_source = 0;
+		w->power_check = dapm_generic_check_power;
+		break;
+	case snd_soc_dapm_vmid:
+	case snd_soc_dapm_siggen:
+		w->is_source = 1;
+		w->power_check = dapm_always_on_check_power;
+		break;
+	case snd_soc_dapm_mux:
+	case snd_soc_dapm_switch:
+	case snd_soc_dapm_mixer:
+	case snd_soc_dapm_mixer_named_ctl:
 	case snd_soc_dapm_adc:
 	case snd_soc_dapm_aif_out:
 	case snd_soc_dapm_dac:
 	case snd_soc_dapm_aif_in:
 	case snd_soc_dapm_pga:
 	case snd_soc_dapm_out_drv:
-	case snd_soc_dapm_input:
-	case snd_soc_dapm_output:
 	case snd_soc_dapm_micbias:
-	case snd_soc_dapm_spk:
-	case snd_soc_dapm_hp:
-	case snd_soc_dapm_mic:
-	case snd_soc_dapm_line:
 	case snd_soc_dapm_dai_link:
 	case snd_soc_dapm_dai_out:
 	case snd_soc_dapm_dai_in:
@@ -3009,6 +2981,7 @@ snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
 	case snd_soc_dapm_regulator_supply:
 	case snd_soc_dapm_clock_supply:
 	case snd_soc_dapm_kcontrol:
+		w->is_supply = 1;
 		w->power_check = dapm_supply_check_power;
 		break;
 	default:
@@ -3372,6 +3345,11 @@ static void soc_dapm_dai_stream_event(struct snd_soc_dai *dai, int stream,
 		case SND_SOC_DAPM_STREAM_PAUSE_RELEASE:
 			break;
 		}
+
+		if (w->id == snd_soc_dapm_dai_in)
+			w->is_source = w->active;
+		else
+			w->is_sink = w->active;
 	}
 }
 
-- 
1.8.0

[PATCH 09/12] ASoC: dapm: Add a flag to mark paths connected to supply widgets

[Lars-Peter Clausen]

Supply widgets do not count towards the input and output widgets of their
neighbors and for supply widgets themselves we do not care for the number
of input or output paths. This means that a path that connects to a supply
widget effectively behaves the same as a path that as the weak property set.
This patch adds a new path flag that gets set to true when the path is
connected to at least one supply widget. If a path with the flag set is
encountered in is_connected_{input,output}_ep() is is skipped in the same
way that weak paths are skipped. This slightly brings down the number of
path checks.

Since both the weak and the supply flag are implemented as bitfields which
are stored in the same word there is no runtime overhead due to checking
both rather than just one and also the size of the path struct is not
increased by this patch. Another advantage is that we do not have to handle
supply widgets in is_connected_{input,output}_ep() anymore since it will
never be called for supply widgets. The only exception is from
dapm_widget_power_read_file() where a check is added to special case supply
widgets.

Testing with the ADAU1761, which has a handful of supply widgets, shows the
following changes in the DAPM stats for a playback stream start.

         Power  Path  Neighbour
Before:  34     78    117
After:   34     48    117

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc-dapm.h |  1 +
 sound/soc/soc-dapm.c     | 23 +++++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 8cf3aad..e7ebeb7 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -510,6 +510,7 @@ struct snd_soc_dapm_path {
 	u32 connect:1;	/* source and sink widgets are connected */
 	u32 walking:1;  /* path is in the process of being walked */
 	u32 weak:1;	/* path ignored for power management */
+	u32 is_supply:1;	/* At least one of the connected widgets is a supply */
 
 	int (*connected)(struct snd_soc_dapm_widget *source,
 			 struct snd_soc_dapm_widget *sink);
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 578c119..a7d5acf 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -821,9 +821,6 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
 
 	DAPM_UPDATE_STAT(widget, path_checks);
 
-	if (widget->is_supply)
-		return 0;
-
 	if (widget->is_sink && widget->connected) {
 		widget->outputs = snd_soc_dapm_suspend_check(widget);
 		return widget->outputs;
@@ -832,7 +829,7 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
 	list_for_each_entry(path, &widget->sinks, list_source) {
 		DAPM_UPDATE_STAT(widget, neighbour_checks);
 
-		if (path->weak)
+		if (path->weak || path->is_supply)
 			continue;
 
 		if (path->walking)
@@ -882,9 +879,6 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
 
 	DAPM_UPDATE_STAT(widget, path_checks);
 
-	if (widget->is_supply)
-		return 0;
-
 	if (widget->is_source && widget->connected) {
 		widget->inputs = snd_soc_dapm_suspend_check(widget);
 		return widget->inputs;
@@ -893,7 +887,7 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
 	list_for_each_entry(path, &widget->sources, list_sink) {
 		DAPM_UPDATE_STAT(widget, neighbour_checks);
 
-		if (path->weak)
+		if (path->weak || path->is_supply)
 			continue;
 
 		if (path->walking)
@@ -1691,8 +1685,14 @@ static ssize_t dapm_widget_power_read_file(struct file *file,
 	if (!buf)
 		return -ENOMEM;
 
-	in = is_connected_input_ep(w, NULL);
-	out = is_connected_output_ep(w, NULL);
+	/* Supply widgets are not handled by is_connected_{input,output}_ep() */
+	if (w->is_supply) {
+		in = 0;
+		out = 0;
+	} else {
+		in = is_connected_input_ep(w, NULL);
+		out = is_connected_output_ep(w, NULL);
+	}
 
 	ret = snprintf(buf, PAGE_SIZE, "%s: %s%s  in %d out %d",
 		       w->name, w->power ? "On" : "Off",
@@ -2217,6 +2217,9 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	INIT_LIST_HEAD(&path->list_source);
 	INIT_LIST_HEAD(&path->list_sink);
 
+	if (wsource->is_supply || wsink->is_supply)
+		path->is_supply = 1;
+
 	/* connect static paths */
 	if (control == NULL) {
 		path->connect = 1;
-- 
1.8.0

[PATCH 10/12] ASoC: dapm: Mark endpoints instead of IO widgets dirty during suspend/resume

[Lars-Peter Clausen]

The state of endpoint widgets is affected by that card's power state.
Endpoint widgets that do no have the ignore_suspend flag set will be
considered inactive during suspend. So they have to be re-checked and marked
dirty after the card's power state changes. Currently the input and output
widgets are marked dirty instead, this works most of the time since
typically a path from one endpoint to another will go via a input or output
widget. But marking the endpoints dirty is technically more correct and will
also work for odd corner cases.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc-dapm.h |  2 +-
 sound/soc/soc-core.c     |  8 ++++----
 sound/soc/soc-dapm.c     | 15 ++++-----------
 3 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index e7ebeb7..43ca165 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -435,7 +435,7 @@ void snd_soc_dapm_auto_nc_pins(struct snd_soc_card *card);
 unsigned int dapm_kcontrol_get_value(const struct snd_kcontrol *kcontrol);
 
 /* Mostly internal - should not normally be used */
-void dapm_mark_io_dirty(struct snd_soc_dapm_context *dapm);
+void dapm_mark_endpoints_dirty(struct snd_soc_card *card);
 
 /* dapm path query */
 int snd_soc_dapm_dai_get_connected_widgets(struct snd_soc_dai *dai, int stream,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 4c8f8a2..443be00 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -629,8 +629,8 @@ int snd_soc_suspend(struct device *dev)
 					  SND_SOC_DAPM_STREAM_SUSPEND);
 	}
 
-	/* Recheck all analogue paths too */
-	dapm_mark_io_dirty(&card->dapm);
+	/* Recheck all endpoints too, their state is affected by suspend */
+	dapm_mark_endpoints_dirty(card);
 	snd_soc_dapm_sync(&card->dapm);
 
 	/* suspend all CODECs */
@@ -796,8 +796,8 @@ static void soc_resume_deferred(struct work_struct *work)
 	/* userspace can access us now we are back as we were before */
 	snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0);
 
-	/* Recheck all analogue paths too */
-	dapm_mark_io_dirty(&card->dapm);
+	/* Recheck all endpoints too, their state is affected by suspend */
+	dapm_mark_endpoints_dirty(card);
 	snd_soc_dapm_sync(&card->dapm);
 }
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index a7d5acf..8792e47 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -159,27 +159,20 @@ static void dapm_mark_dirty(struct snd_soc_dapm_widget *w, const char *reason)
 	}
 }
 
-void dapm_mark_io_dirty(struct snd_soc_dapm_context *dapm)
+void dapm_mark_endpoints_dirty(struct snd_soc_card *card)
 {
-	struct snd_soc_card *card = dapm->card;
 	struct snd_soc_dapm_widget *w;
 
 	mutex_lock(&card->dapm_mutex);
 
 	list_for_each_entry(w, &card->widgets, list) {
-		switch (w->id) {
-		case snd_soc_dapm_input:
-		case snd_soc_dapm_output:
-			dapm_mark_dirty(w, "Rechecking inputs and outputs");
-			break;
-		default:
-			break;
-		}
+		if (w->is_sink || w->is_source)
+			dapm_mark_dirty(w, "Rechecking endpoints");
 	}
 
 	mutex_unlock(&card->dapm_mutex);
 }
-EXPORT_SYMBOL_GPL(dapm_mark_io_dirty);
+EXPORT_SYMBOL_GPL(dapm_mark_endpoints_dirty);
 
 /* create a new dapm widget */
 static inline struct snd_soc_dapm_widget *dapm_cnew_widget(
-- 
1.8.0

[PATCH 11/12] ASoC: dapm: Add a few supply widget sanity checks

[Lars-Peter Clausen]

Supply widgets are somewhat special and not all kinds of paths to or from
supply widgets make sense. This patch adds a few sanity checks that errors
out during the path instantiation for those invalid paths. This will prevent
drivers to depend on weird behavior resulting from such paths as well as
will allow the DAPM algorithms to assume that they never see such paths.

This patch adds checks for the following three invalid types of paths:
	* A path with a non-supply widget as a source connected to a supply
	  widget as a sink. Such a path has no effect on either of the two
	  connected widgets.
	* Paths with a connected() callback that have a non-supply widget as a
	  sink. The DAPM algorithm only uses the conneceted() callback for
	  supply widget power checks. And since it prevents caching of the DAPM
	  state there is no intention to make it more generic as it has
	  negative performance implications.
	* Paths which connect a supply to a mixer or mux via a control. Controls
	  are only meant to affect the routing of audio data.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-dapm.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 8792e47..38ab643 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2198,6 +2198,27 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	struct snd_soc_dapm_path *path;
 	int ret;
 
+	if (wsink->is_supply && !wsource->is_supply) {
+		dev_err(dapm->dev,
+			"Connecting non-supply widget to supply widget is not supported (%s -> %s)\n",
+			wsource->name, wsink->name);
+		return -EINVAL;
+	}
+
+	if (connected && !wsource->is_supply) {
+		dev_err(dapm->dev,
+			"connected() callback only supported for supply widgets (%s -> %s)\n",
+			wsource->name, wsink->name);
+		return -EINVAL;
+	}
+
+	if (wsource->is_supply && control) {
+		dev_err(dapm->dev,
+			"Conditional paths are not supported for supply widgets (%s -> [%s] -> %s)\n",
+			wsource->name, control, wsink->name);
+		return -EINVAL;
+	}
+
 	path = kzalloc(sizeof(struct snd_soc_dapm_path), GFP_KERNEL);
 	if (!path)
 		return -ENOMEM;
-- 
1.8.0

[PATCH 12/12] ASoC: dapm: Use more aggressive caching

[Lars-Peter Clausen]

Currently we cache the number of input and output paths going to/from a
widget only within a power update sequence. But not in between power update
sequences.

But we know how changes to the DAPM graph affect the number of input (form a
source) and output (to a sink) paths of a widget and only need to
recalculate them if a operation has been performed that might have changed
them.
	* Adding/removing or connecting/disconnecting a path means that the for
	  the source of the path the number of output paths can change and for
	  the sink the number of input paths can change.
	* Connecting/disconnecting a widget has the same effect has connecting/
	  disconnecting all paths of the widget. So for the widget itself the
	  number of inputs and outputs can change, for all sinks of the widget
	  the number of inputs can change and for all sources of the widget the
	  number of outputs can change.
	* Activating/Deactivating a stream can either change the number of
	  outputs on the sources of the widget associated with the stream or the
	  number of inputs on the sinks.

Instead of always invalidating all cached numbers of input and output paths
for each power up or down sequence this patch restructures the code to only
invalidate the cached numbers when a operation that might change them has
been performed. This can greatly reduce the number of DAPM power checks for
some very common operations.

The biggest effect is on widgets with multiple connected inputs and output
(e.g. mixers probably being the most widespread form of this). Previously we
had to re-calculate the number of inputs and outputs on all input and output
paths. With this change we only have to re-calculate the number of outputs
on the input path that got changed and the number of inputs on the output
paths.

E.g. imagine the following example:

	A --> B ----.
	            v
	M --> N --> Z <-- S <-- R
	            |
	            v
	            X

Widget Z has multiple input paths, if any change was made that cause Z to be
marked as dirty the power state of Z has to be re-computed. This requires to
know the number of inputs and outputs of Z, which requires to know the
number of inputs and outputs of all widgets on all paths from or to Z.
Previously this meant re-computing all inputs and outputs of all the path
going into or out of Z. With this patch in place only paths that actually
have changed need to be re-computed.

Even for widgets with only one sink or source there is still a improvement
since typically only the number of inputs or the number of outputs has to be
re-calculated, but not both.

When loading a state file or switching between different profiles typically
multiple mixer and mux settings are changed, so we see the benefit of this
patch multiplied for these kinds of operations.

Testing with the ADAU1761 shows the following changes in DAPM stats for
changing a single Mixer switch for a Mixer with 5 inputs while the DAPM
context is idle.

         Power  Path  Neighbour
Before:  2      12    30
After:   2       1     2

For the same switch, but with a active playback stream the stat changed are
as follows.

         Power  Path  Neighbour
Before:  10     20    54
After:   10      7    21

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc-dapm.h |   1 +
 sound/soc/soc-dapm.c     | 157 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 150 insertions(+), 8 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 43ca165..89823cf 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -569,6 +569,7 @@ struct snd_soc_dapm_widget {
 	struct list_head sinks;
 
 	/* used during DAPM updates */
+	struct list_head work_list;
 	struct list_head power_list;
 	struct list_head dirty;
 	int inputs;
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 38ab643..7052e77 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -159,6 +159,112 @@ static void dapm_mark_dirty(struct snd_soc_dapm_widget *w, const char *reason)
 	}
 }
 
+/**
+ * dapm_widget_invalidate_input_paths() - Invalidate the cached number of input
+ *  paths
+ * @w: The widget for which to invalidate the cached number of input paths
+ *
+ * The function resets the cached number of inputs for the specified widget and
+ * all widgets that can be reached via outgoing paths from the widget.
+ *
+ * This function must be called if the number of input paths for a widget might
+ * have changed. E.g. if the source state of a widget changes or a path is added
+ * or activated with the widget as the sink.
+ */
+static void dapm_widget_invalidate_input_paths(struct snd_soc_dapm_widget *w)
+{
+	struct snd_soc_dapm_widget *sink;
+	struct snd_soc_dapm_path *p;
+	LIST_HEAD(list);
+
+	if (w->inputs == -1)
+		return;
+
+	w->inputs = -1;
+	list_add_tail(&w->work_list, &list);
+
+	list_for_each_entry(w, &list, work_list) {
+		list_for_each_entry(p, &w->sinks, list_source) {
+			if (p->is_supply || p->weak || !p->connect)
+				continue;
+			sink = p->sink;
+			if (sink->inputs != -1) {
+				sink->inputs = -1;
+				list_add_tail(&sink->work_list, &list);
+			}
+		}
+	}
+}
+
+/**
+ * dapm_widget_invalidate_output_paths() - Invalidate the cached number of
+ *  output paths
+ * @w: The widget for which to invalidate the cached number of output paths
+ *
+ * Resets the cached number of outputs for the specified widget and all widgets
+ * that can be reached via incoming paths from the widget.
+ *
+ * This function must be called if the number of output paths for a widget might
+ * have changed. E.g. if the sink state of a widget changes or a path is added
+ * or activated with the widget as the source.
+ */
+static void dapm_widget_invalidate_output_paths(struct snd_soc_dapm_widget *w)
+{
+	struct snd_soc_dapm_widget *source;
+	struct snd_soc_dapm_path *p;
+	LIST_HEAD(list);
+
+	if (w->outputs == -1)
+		return;
+
+	w->outputs = -1;
+	list_add_tail(&w->work_list, &list);
+
+	list_for_each_entry(w, &list, work_list) {
+		list_for_each_entry(p, &w->sources, list_sink) {
+			if (p->is_supply || p->weak || !p->connect)
+				continue;
+			source = p->source;
+			if (source->outputs != -1) {
+				source->outputs = -1;
+				list_add_tail(&source->work_list, &list);
+			}
+		}
+	}
+}
+
+/**
+ * dapm_path_invalidate() - Invalidates the cached number of inputs and outputs
+ *  for the widgets connected to a path
+ * @p: The path to invalidate
+ *
+ * Resets the cached number of inputs for the sink of the path and the cached
+ * number of outputs for the source of the path.
+ *
+ * This function must be called when a path is added, removed or the connected
+ * state changes.
+ */
+static void dapm_path_invalidate(struct snd_soc_dapm_path *p)
+{
+	/*
+	 * Weak paths or supply paths do not influence the number of input or
+	 * output paths of their neighbors.
+	 */
+	if (p->weak || p->is_supply)
+		return;
+
+	/*
+	 * The number of connected endpoints is the sum of the number of
+	 * connected endpoints of all neighbors. If a node with 0 connected
+	 * endpoints is either connected or disconnected that sum won't change,
+	 * so there is no need to re-check the path.
+	 */
+	if (p->source->inputs != 0)
+		dapm_widget_invalidate_input_paths(p->sink);
+	if (p->sink->outputs != 0)
+		dapm_widget_invalidate_output_paths(p->source);
+}
+
 void dapm_mark_endpoints_dirty(struct snd_soc_card *card)
 {
 	struct snd_soc_dapm_widget *w;
@@ -166,8 +272,13 @@ void dapm_mark_endpoints_dirty(struct snd_soc_card *card)
 	mutex_lock(&card->dapm_mutex);
 
 	list_for_each_entry(w, &card->widgets, list) {
-		if (w->is_sink || w->is_source)
+		if (w->is_sink || w->is_source) {
 			dapm_mark_dirty(w, "Rechecking endpoints");
+			if (w->is_sink)
+				dapm_widget_invalidate_output_paths(w);
+			if (w->is_source)
+				dapm_widget_invalidate_input_paths(w);
+		}
 	}
 
 	mutex_unlock(&card->dapm_mutex);
@@ -379,8 +490,6 @@ static void dapm_reset(struct snd_soc_card *card)
 	list_for_each_entry(w, &card->widgets, list) {
 		w->new_power = w->power;
 		w->power_checked = false;
-		w->inputs = -1;
-		w->outputs = -1;
 	}
 }
 
@@ -931,10 +1040,19 @@ int snd_soc_dapm_dai_get_connected_widgets(struct snd_soc_dai *dai, int stream,
 	struct snd_soc_dapm_widget_list **list)
 {
 	struct snd_soc_card *card = dai->card;
+	struct snd_soc_dapm_widget *w;
 	int paths;
 
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
-	dapm_reset(card);
+
+	/*
+	 * For is_connected_{output,input}_ep fully discover the graph we need
+	 * to reset the cached number of inputs and outputs.
+	 */
+	list_for_each_entry(w, &card->widgets, list) {
+		w->inputs = -1;
+		w->outputs = -1;
+	}
 
 	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
 		paths = is_connected_output_ep(dai->playback_widget, list);
@@ -1848,6 +1966,7 @@ static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
 	path->connect = connect;
 	dapm_mark_dirty(path->source, reason);
 	dapm_mark_dirty(path->sink, reason);
+	dapm_path_invalidate(path);
 
 	return true;
 }
@@ -2088,8 +2207,11 @@ static int snd_soc_dapm_set_pin(struct snd_soc_dapm_context *dapm,
 		return -EINVAL;
 	}
 
-	if (w->connected != status)
+	if (w->connected != status) {
 		dapm_mark_dirty(w, "pin configuration");
+		dapm_widget_invalidate_input_paths(w);
+		dapm_widget_invalidate_output_paths(w);
+	}
 
 	w->connected = status;
 	if (status == 0)
@@ -2271,6 +2393,9 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 	dapm_mark_dirty(wsource, "Route added");
 	dapm_mark_dirty(wsink, "Route added");
 
+	if (path->connect)
+		dapm_path_invalidate(path);
+
 	return 0;
 err:
 	kfree(path);
@@ -2394,6 +2519,8 @@ static int snd_soc_dapm_del_route(struct snd_soc_dapm_context *dapm,
 
 		dapm_mark_dirty(wsource, "Route removed");
 		dapm_mark_dirty(wsink, "Route removed");
+		if (path->connect)
+			dapm_path_invalidate(path);
 
 		dapm_free_path(path);
 
@@ -3015,6 +3142,9 @@ snd_soc_dapm_new_control(struct snd_soc_dapm_context *dapm,
 	INIT_LIST_HEAD(&w->dirty);
 	list_add(&w->list, &dapm->card->widgets);
 
+	w->inputs = -1;
+	w->outputs = -1;
+
 	/* machine layer set ups unconnected pins and insertions */
 	w->connected = 1;
 	return w;
@@ -3363,10 +3493,13 @@ static void soc_dapm_dai_stream_event(struct snd_soc_dai *dai, int stream,
 			break;
 		}
 
-		if (w->id == snd_soc_dapm_dai_in)
+		if (w->id == snd_soc_dapm_dai_in) {
 			w->is_source = w->active;
-		else
+			dapm_widget_invalidate_input_paths(w);
+		} else {
 			w->is_sink = w->active;
+			dapm_widget_invalidate_output_paths(w);
+		}
 	}
 }
 
@@ -3493,7 +3626,15 @@ int snd_soc_dapm_force_enable_pin_unlocked(struct snd_soc_dapm_context *dapm,
 	}
 
 	dev_dbg(w->dapm->dev, "ASoC: force enable pin %s\n", pin);
-	w->connected = 1;
+	if (!w->connected) {
+		/*
+		 * w->force does not affect the number of input or output paths,
+		 * so we only have to recheck if w->connected is changed
+		 */
+		dapm_widget_invalidate_input_paths(w);
+		dapm_widget_invalidate_output_paths(w);
+		w->connected = 1;
+	}
 	w->force = 1;
 	dapm_mark_dirty(w, "force enable");
 
-- 
1.8.0

Re: [PATCH 03/12] ASoC: dapm: Only mark paths dirty when the connection status changed

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 230 bytes --]

On Mon, Oct 20, 2014 at 07:36:35PM +0200, Lars-Peter Clausen wrote:

> +static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
> +	bool connect, const char *reason)

Why is this returning a value which is then ignored?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 01/12] ASoC: dapm: Reduce number of checked paths in dapm_widget_in_card_paths()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 514 bytes --]

On Mon, Oct 20, 2014 at 07:36:33PM +0200, Lars-Peter Clausen wrote:

> +/**
> + * dapm_is_external_path() - Checks if a path is a external path
> + * @card: The card the path belongs to
> + * @path: The path to check

We should probably not be marking internal only functions up using
kerneldoc, this will cause them to appear in generated documentation
which would doubtless be confusing for readers.  Just omitting the /**
and only using /* should be enough if kerneldoc doesn't have a specific
markup for this.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 01/12] ASoC: dapm: Reduce number of checked paths in dapm_widget_in_card_paths()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 263 bytes --]

On Mon, Oct 20, 2014 at 07:36:33PM +0200, Lars-Peter Clausen wrote:
> Each widget has a list of all the paths that it is connected to. There is no
> need to iterate over all paths when we are only interested in the paths of a
> specific widget.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 02/12] ASoC: dapm: Remove always true path source/sink checks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 289 bytes --]

On Mon, Oct 20, 2014 at 07:36:34PM +0200, Lars-Peter Clausen wrote:
> A path has always a valid source and a valid sink otherwise we wouldn't add
> it in the first place. Hence all tests that check if sink/source is non NULL
> always evaluate to true and can be removed.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/12] ASoC: dapm: Remove path 'walked' flag

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 346 bytes --]

On Mon, Oct 20, 2014 at 07:36:39PM +0200, Lars-Peter Clausen wrote:
> The 'walked' flag was used to avoid walking paths that have already been
> walked. But since we started caching the number of inputs and outputs of a
> path we never actually get into a situation where we try to walk a path that
> has the 'walked' flag set.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 06/12] ASoC: dapm: Remove special DAI widget power check functions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 659 bytes --]

On Mon, Oct 20, 2014 at 07:36:38PM +0200, Lars-Peter Clausen wrote:
> dapm_adc_check_power() checks if the widget is active, if yes it only checks
> whether there are any connected input paths. Otherwise it calls
> dapm_generic_check_power() which will check for both connected input and
> output paths. But the function that checks for connected output paths will
> return true if the widget is a active sink. Which means the generic power
> check function will work just fine and there is no need for a special power
> check function.

Applied, thanks.  IIRC it did make a difference when we weren't hooking
things into DAPM properly but that's bitrot now.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 11/12] ASoC: dapm: Add a few supply widget sanity checks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 355 bytes --]

On Mon, Oct 20, 2014 at 07:36:43PM +0200, Lars-Peter Clausen wrote:

> 	* Paths with a connected() callback that have a non-supply widget as a
> 	  sink. The DAPM algorithm only uses the conneceted() callback for

This change is OK but I think you mean source rather than sink here -
supplies are normally the source of a route (except in supply chains).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 00/12] ASoC: dapm: Cleanups and speed-ups

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]

On Mon, Oct 20, 2014 at 07:36:32PM +0200, Lars-Peter Clausen wrote:

> Changing a single control on a 5 input mixer while the system is idle:

>           Power  Path  Neighbour
> Before:   2      25    32
> After~1:  2      12    30
> After:    2       1     2

> Cumulative numbers for switching the audio profile which changes 7 controls
> while the system is idle:

>           Power  Path  Neighbour
> Before:   16     141   188
> After~1:  16      80   170
> After:    16       7    23

This series is all basically OK, a couple of small comments one of which
was very near the start of the series or I'd have applied more.  

Just as a general comment the numbers you're showing here are all for
very small use cases and while they do show an order of magnitude
improvement if they do scale out (it looks like this is mostly linear or
constant factor gains but some of those are scale with the total number of
widgets or paths which can be interesting) since that makes things much
more exciting.  It's also good (though not at all essential) to show the
benchmarks on individual commits since this both keeps them around and
helps people trying to solve performance problems in production find
relevant upstream optimisations to backport.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 03/12] ASoC: dapm: Only mark paths dirty when the connection status changed

[Lars-Peter Clausen]

On 10/22/2014 01:00 PM, Mark Brown wrote:
> On Mon, Oct 20, 2014 at 07:36:35PM +0200, Lars-Peter Clausen wrote:
>
>> +static bool soc_dapm_connect_path(struct snd_soc_dapm_path *path,
>> +	bool connect, const char *reason)
>
> Why is this returning a value which is then ignored?
>

I was initially using it to not schedule the dapm_power_widgets(), but there 
is potential for problems when the DAPM graph and the hardware state are 
out-of-sync, in which case we still want to apply the register update. There 
are some corner cases where this can happen and which need to be fixed first.

I can drop the return type for now.

- Lars

Re: [PATCH 03/12] ASoC: dapm: Only mark paths dirty when the connection status changed

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 747 bytes --]

On Wed, Oct 22, 2014 at 02:10:29PM +0200, Lars-Peter Clausen wrote:
> On 10/22/2014 01:00 PM, Mark Brown wrote:

> >Why is this returning a value which is then ignored?

> I was initially using it to not schedule the dapm_power_widgets(), but there
> is potential for problems when the DAPM graph and the hardware state are
> out-of-sync, in which case we still want to apply the register update. There
> are some corner cases where this can happen and which need to be fixed
> first.

> I can drop the return type for now.

Probably best, I was looking for a user and didn't find one.  Seems like
a bit of a microoptimisation either way.

If it was just returning an error code and we were ignoring that I'd not
have worried!  

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 00/12] ASoC: dapm: Cleanups and speed-ups

[Lars-Peter Clausen]

On 10/22/2014 01:36 PM, Mark Brown wrote:
> On Mon, Oct 20, 2014 at 07:36:32PM +0200, Lars-Peter Clausen wrote:
>
>> Changing a single control on a 5 input mixer while the system is idle:
>
>>            Power  Path  Neighbour
>> Before:   2      25    32
>> After~1:  2      12    30
>> After:    2       1     2
>
>> Cumulative numbers for switching the audio profile which changes 7 controls
>> while the system is idle:
>
>>            Power  Path  Neighbour
>> Before:   16     141   188
>> After~1:  16      80   170
>> After:    16       7    23
>
> This series is all basically OK, a couple of small comments one of which
> was very near the start of the series or I'd have applied more.
>
> Just as a general comment the numbers you're showing here are all for
> very small use cases and while they do show an order of magnitude
> improvement if they do scale out (it looks like this is mostly linear or
> constant factor gains but some of those are scale with the total number of
> widgets or paths which can be interesting) since that makes things much
> more exciting.  It's also good (though not at all essential) to show the
> benchmarks on individual commits since this both keeps them around and
> helps people trying to solve performance problems in production find
> relevant upstream optimisations to backport.
>

There are two patches in this series which have direct impact on the stats. 
The first one is the one that marks paths from a supply as a supply path. 
Those paths are skipped in is_connected_{input,output}_ep and hence supply 
widgets no longer appear in the path checks stat. While this might look 
pretty good in the stats the actual performance improvement is not that big 
since is_connected_{input,output_ep} will return right away if it encounters 
a supply widget. The change is still worth doing since the check if a path 
is connected to a supply comes essentially for free and we can also remove 
the code that handles supply widgets from is_connected_{input,output_ep}.

The second change is the last patch in this series which caches the 
connected input and output path numbers between multiple runs of 
dapm_power_widgets() and only re-calculates those numbers if they could have 
changed. Since per DAPM operation typically only either changes the number 
of inputs or outputs the number of path checks is reduced by 50%. The number 
of neighbor checks is also reduced about the same percentage, but since the 
number of neighbors encountered when walking from sink to source is not the 
same as when walking from source to sink the actual numbers will slightly 
vary from card to card (e.g. for a mixer we see 1 neighbor when walking from 
source to sink, but the number of inputs neighbors when walking from source 
to sink).

Things get better than 50% when there are widgets on the path that got 
changed that have multiple inputs and outputs since we only need to re-check 
the one path that has changed rather than rechecking all input and output 
paths.  E.g. if you have a mixer with many enabled inputs this can be quite 
substantial.

If the system is idle (or the part of the system affected by the changed 
path) the number of path checks drops to either 0 or 1, regardless of how 
large or complex the DAPM context is. 0 if there is no connected sink and no 
connected source. 1 if there is either a connected source or sink, but not 
both. The number of neighbor checks again will scale accordingly and will be 
a constant number that is the number of inputs or outputs of the widget for 
which we did the path check.

- Lars

Re: [PATCH 00/12] ASoC: dapm: Cleanups and speed-ups

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1844 bytes --]

On Sat, Oct 25, 2014 at 03:58:21PM +0200, Lars-Peter Clausen wrote:

> There are two patches in this series which have direct impact on the stats.
> The first one is the one that marks paths from a supply as a supply path.
> Those paths are skipped in is_connected_{input,output}_ep and hence supply
> widgets no longer appear in the path checks stat. While this might look
> pretty good in the stats the actual performance improvement is not that big
> since is_connected_{input,output_ep} will return right away if it encounters
> a supply widget. The change is still worth doing since the check if a path
> is connected to a supply comes essentially for free and we can also remove
> the code that handles supply widgets from is_connected_{input,output_ep}.

> The second change is the last patch in this series which caches the
> connected input and output path numbers between multiple runs of
> dapm_power_widgets() and only re-calculates those numbers if they could have
> changed. Since per DAPM operation typically only either changes the number
> of inputs or outputs the number of path checks is reduced by 50%. The number
> of neighbor checks is also reduced about the same percentage, but since the
> number of neighbors encountered when walking from sink to source is not the
> same as when walking from source to sink the actual numbers will slightly
> vary from card to card (e.g. for a mixer we see 1 neighbor when walking from
> source to sink, but the number of inputs neighbors when walking from source
> to sink).

Right, I get all this - my point was more that I had to think about it
rather than being able to see it from the changelog and with performance
it's particularly important since it's the sort of thing people end up
looking for when they run into problems in production so giving them a
helping hand is good.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]