Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] ASoC: Add verbose debugging showing why widgets get marked dirty
@ 2011-10-04 22:01 Mark Brown
  2011-10-04 22:01 ` [PATCH 2/4] ASoC: Only run power_check() on a widget once per run Mark Brown
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Mark Brown @ 2011-10-04 22:01 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: alsa-devel, patches, Mark Brown

Help diagnose why we're checking widgets by providing some logging when
we first dirty them. This should possibly be a trace point if it's useful
but can be absurdly verbose if enabled, we can always change it later if
desired.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/soc-dapm.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 214a709..e6a0882 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -124,10 +124,13 @@ static bool dapm_dirty_widget(struct snd_soc_dapm_widget *w)
 	return !list_empty(&w->dirty);
 }
 
-static void dapm_mark_dirty(struct snd_soc_dapm_widget *w)
+static void dapm_mark_dirty(struct snd_soc_dapm_widget *w, const char *reason)
 {
-	if (!dapm_dirty_widget(w))
+	if (!dapm_dirty_widget(w)) {
+		dev_vdbg(w->dapm->dev, "Marking %s dirty due to %s\n",
+			 w->name, reason);
 		list_add_tail(&w->dirty, &w->dapm->card->dapm_dirty);
+	}
 }
 
 /* create a new dapm widget */
@@ -1227,7 +1230,7 @@ static void dapm_widget_set_peer_power(struct snd_soc_dapm_widget *peer,
 	/* If the peer is already in the state we're moving to then we
 	 * won't have an impact on it. */
 	if (power != peer->power)
-		dapm_mark_dirty(peer);
+		dapm_mark_dirty(peer, "peer state change");
 }
 
 static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
@@ -1624,16 +1627,17 @@ static int dapm_mux_update_power(struct snd_soc_dapm_widget *widget,
 		/* 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);
+			dapm_mark_dirty(path->source, "mux connection");
 		} else {
 			if (path->connect)
-				dapm_mark_dirty(path->source);
+				dapm_mark_dirty(path->source,
+						"mux disconnection");
 			path->connect = 0; /* old connection must be powered down */
 		}
 	}
 
 	if (found) {
-		dapm_mark_dirty(widget);
+		dapm_mark_dirty(widget, "mux change");
 		dapm_power_widgets(widget->dapm, SND_SOC_DAPM_STREAM_NOP);
 	}
 
@@ -1660,11 +1664,11 @@ static int dapm_mixer_update_power(struct snd_soc_dapm_widget *widget,
 		/* found, now check type */
 		found = 1;
 		path->connect = connect;
-		dapm_mark_dirty(path->source);
+		dapm_mark_dirty(path->source, "mixer connection");
 	}
 
 	if (found) {
-		dapm_mark_dirty(widget);
+		dapm_mark_dirty(widget, "mixer update");
 		dapm_power_widgets(widget->dapm, SND_SOC_DAPM_STREAM_NOP);
 	}
 
@@ -1810,7 +1814,7 @@ static int snd_soc_dapm_set_pin(struct snd_soc_dapm_context *dapm,
 	w->connected = status;
 	if (status == 0)
 		w->force = 0;
-	dapm_mark_dirty(w);
+	dapm_mark_dirty(w, "pin configuration");
 
 	return 0;
 }
@@ -2699,7 +2703,7 @@ static void soc_dapm_stream_event(struct snd_soc_dapm_context *dapm,
 		dev_vdbg(w->dapm->dev, "widget %s\n %s stream %s event %d\n",
 			w->name, w->sname, stream, event);
 		if (strstr(w->sname, stream)) {
-			dapm_mark_dirty(w);
+			dapm_mark_dirty(w, "stream event");
 			switch(event) {
 			case SND_SOC_DAPM_STREAM_START:
 				w->active = 1;
@@ -2789,7 +2793,7 @@ int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm,
 	dev_dbg(w->dapm->dev, "dapm: force enable pin %s\n", pin);
 	w->connected = 1;
 	w->force = 1;
-	dapm_mark_dirty(w);
+	dapm_mark_dirty(w, "force enable");
 
 	return 0;
 }
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/4] ASoC: Only run power_check() on a widget once per run
  2011-10-04 22:01 [PATCH 1/4] ASoC: Add verbose debugging showing why widgets get marked dirty Mark Brown
@ 2011-10-04 22:01 ` Mark Brown
  2011-10-04 22:01 ` [PATCH 3/4] ASoC: Don't mark the outputs of supplies as dirty on state changes Mark Brown
  2011-10-04 22:01 ` [PATCH 4/4] ASoC: Stop checking for supplied widgets after we find the first Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2011-10-04 22:01 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: alsa-devel, patches, Mark Brown

Some widgets will get power_check() run on them more than once during a
DAPM run, most commonly due to supply widgets checking to see if their
consumers are powered up. It's wasteful to do this so cache the result
of power_check() during a run. For one system I tested this on I got an
improvement of:

           Power    Path   Neighbour
Before:    106      970    1186
After:     69       727    905

from this.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/soc-dapm.h |    2 ++
 sound/soc/soc-dapm.c     |   15 +++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index c080635..e2853da 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -473,6 +473,8 @@ struct snd_soc_dapm_widget {
 	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 */
 	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 e6a0882..c39146d 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -787,10 +787,17 @@ EXPORT_SYMBOL_GPL(dapm_reg_event);
 
 static int dapm_widget_power_check(struct snd_soc_dapm_widget *w)
 {
+	if (w->power_checked)
+		return w->new_power;
+
 	if (w->force)
-		return 1;
+		w->new_power = 1;
 	else
-		return w->power_check(w);
+		w->new_power = w->power_check(w);
+
+	w->power_checked = true;
+
+	return w->new_power;
 }
 
 /* Generic check to see if a widget should be powered.
@@ -1322,6 +1329,10 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event)
 
 	memset(&card->dapm_stats, 0, sizeof(card->dapm_stats));
 
+	list_for_each_entry(w, &card->widgets, list) {
+		w->power_checked = false;
+	}
+
 	/* Check which widgets we need to power and store them in
 	 * lists indicating if they should be powered up or down.  We
 	 * only check widgets that have been flagged as dirty but note
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/4] ASoC: Don't mark the outputs of supplies as dirty on state changes
  2011-10-04 22:01 [PATCH 1/4] ASoC: Add verbose debugging showing why widgets get marked dirty Mark Brown
  2011-10-04 22:01 ` [PATCH 2/4] ASoC: Only run power_check() on a widget once per run Mark Brown
@ 2011-10-04 22:01 ` Mark Brown
  2011-10-04 22:01 ` [PATCH 4/4] ASoC: Stop checking for supplied widgets after we find the first Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2011-10-04 22:01 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: alsa-devel, patches, Mark Brown

The whole point of supply widgets is that they aren't inputs to their
sinks so a state change in a supply should never affect the state of the
widget being supplied and we don't need to mark them as dirty.

           Power    Path   Neighbour
Before:    69       727    905
After:     63       607    731

This is particularly useful where supplies affect large portions of the
chip (eg, a bandgap supplying the analogue sections).

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/soc-dapm.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c39146d..cbca1dd 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -1260,11 +1260,18 @@ static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
 						   path->connect);
 		}
 	}
-	list_for_each_entry(path, &w->sinks, list_source) {
-		if (path->sink) {
-			dapm_widget_set_peer_power(path->sink, power,
-						   path->connect);
+	switch (w->id) {
+	case snd_soc_dapm_supply:
+		/* 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);
+			}
 		}
+		break;
 	}
 
 	if (power)
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 4/4] ASoC: Stop checking for supplied widgets after we find the first
  2011-10-04 22:01 [PATCH 1/4] ASoC: Add verbose debugging showing why widgets get marked dirty Mark Brown
  2011-10-04 22:01 ` [PATCH 2/4] ASoC: Only run power_check() on a widget once per run Mark Brown
  2011-10-04 22:01 ` [PATCH 3/4] ASoC: Don't mark the outputs of supplies as dirty on state changes Mark Brown
@ 2011-10-04 22:01 ` Mark Brown
  2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2011-10-04 22:01 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: alsa-devel, patches, Mark Brown

We don't really care how many widgets a supply is supplying, we just care
if the number is non-zero. This didn't actually produce any improvement
in the test cases I've been using but seems obviously sensible enough that
I'm pushing it out anyway.

We could do a similar thing for other widgets but this may be unhelpful
for further refactorings Liam was working on aiming to allow us to
identify connected audio paths.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/soc-dapm.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index cbca1dd..82d93bf 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -851,7 +851,6 @@ static int dapm_dac_check_power(struct snd_soc_dapm_widget *w)
 static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
 {
 	struct snd_soc_dapm_path *path;
-	int power = 0;
 
 	DAPM_UPDATE_STAT(w, power_checks);
 
@@ -869,15 +868,13 @@ static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
 		if (!path->sink)
 			continue;
 
-		if (dapm_widget_power_check(path->sink)) {
-			power = 1;
-			break;
-		}
+		if (dapm_widget_power_check(path->sink))
+			return 1;
 	}
 
 	dapm_clear_walk(w->dapm);
 
-	return power;
+	return 0;
 }
 
 static int dapm_always_on_check_power(struct snd_soc_dapm_widget *w)
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-10-04 22:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-04 22:01 [PATCH 1/4] ASoC: Add verbose debugging showing why widgets get marked dirty Mark Brown
2011-10-04 22:01 ` [PATCH 2/4] ASoC: Only run power_check() on a widget once per run Mark Brown
2011-10-04 22:01 ` [PATCH 3/4] ASoC: Don't mark the outputs of supplies as dirty on state changes Mark Brown
2011-10-04 22:01 ` [PATCH 4/4] ASoC: Stop checking for supplied widgets after we find the first Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox