[PATCH 1/3] ASoC: Explicitly say if we're powering up or down

[PATCH 1/3] ASoC: Explicitly say if we're powering up or down

[Mark Brown]

Rather than passing the sequence to use for DAPM widgets around by reference
explicitly say if we're powering up or down until the point where we need
the sequence itself. This should make no practical difference in itself but
supports future refactoring.

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

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 499730a..57e1c9f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -726,8 +726,15 @@ static int dapm_supply_check_power(struct snd_soc_dapm_widget *w)
 
 static int dapm_seq_compare(struct snd_soc_dapm_widget *a,
 			    struct snd_soc_dapm_widget *b,
-			    int sort[])
+			    bool power_up)
 {
+	int *sort;
+
+	if (power_up)
+		sort = dapm_up_seq;
+	else
+		sort = dapm_down_seq;
+
 	if (sort[a->id] != sort[b->id])
 		return sort[a->id] - sort[b->id];
 	if (a->reg != b->reg)
@@ -741,12 +748,12 @@ static int dapm_seq_compare(struct snd_soc_dapm_widget *a,
 /* Insert a widget in order into a DAPM power sequence. */
 static void dapm_seq_insert(struct snd_soc_dapm_widget *new_widget,
 			    struct list_head *list,
-			    int sort[])
+			    bool power_up)
 {
 	struct snd_soc_dapm_widget *w;
 
 	list_for_each_entry(w, list, power_list)
-		if (dapm_seq_compare(new_widget, w, sort) < 0) {
+		if (dapm_seq_compare(new_widget, w, power_up) < 0) {
 			list_add_tail(&new_widget->power_list, &w->power_list);
 			return;
 		}
@@ -857,7 +864,7 @@ static void dapm_seq_run_coalesced(struct snd_soc_dapm_context *dapm,
  * handled.
  */
 static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
-			 struct list_head *list, int event, int sort[])
+			 struct list_head *list, int event, bool power_up)
 {
 	struct snd_soc_dapm_widget *w, *n;
 	LIST_HEAD(pending);
@@ -865,6 +872,12 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 	int cur_reg = SND_SOC_NOPM;
 	struct snd_soc_dapm_context *cur_dapm = NULL;
 	int ret;
+	int *sort;
+
+	if (power_up)
+		sort = dapm_up_seq;
+	else
+		sort = dapm_down_seq;
 
 	list_for_each_entry_safe(w, n, list, power_list) {
 		ret = 0;
@@ -1002,10 +1015,10 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event)
 	list_for_each_entry(w, &card->widgets, list) {
 		switch (w->id) {
 		case snd_soc_dapm_pre:
-			dapm_seq_insert(w, &down_list, dapm_down_seq);
+			dapm_seq_insert(w, &down_list, false);
 			break;
 		case snd_soc_dapm_post:
-			dapm_seq_insert(w, &up_list, dapm_up_seq);
+			dapm_seq_insert(w, &up_list, true);
 			break;
 
 		default:
@@ -1025,9 +1038,9 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event)
 			trace_snd_soc_dapm_widget_power(w, power);
 
 			if (power)
-				dapm_seq_insert(w, &up_list, dapm_up_seq);
+				dapm_seq_insert(w, &up_list, true);
 			else
-				dapm_seq_insert(w, &down_list, dapm_down_seq);
+				dapm_seq_insert(w, &down_list, false);
 
 			w->power = power;
 			break;
@@ -1086,12 +1099,12 @@ static int dapm_power_widgets(struct snd_soc_dapm_context *dapm, int event)
 	}
 
 	/* Power down widgets first; try to avoid amplifying pops. */
-	dapm_seq_run(dapm, &down_list, event, dapm_down_seq);
+	dapm_seq_run(dapm, &down_list, event, false);
 
 	dapm_widget_update(dapm);
 
 	/* Now power up. */
-	dapm_seq_run(dapm, &up_list, event, dapm_up_seq);
+	dapm_seq_run(dapm, &up_list, event, true);
 
 	list_for_each_entry(d, &dapm->card->dapm_list, list) {
 		/* If we just powered the last thing off drop to standby bias */
@@ -2372,7 +2385,7 @@ static void soc_dapm_shutdown_codec(struct snd_soc_dapm_context *dapm)
 		if (w->dapm != dapm)
 			continue;
 		if (w->power) {
-			dapm_seq_insert(w, &down_list, dapm_down_seq);
+			dapm_seq_insert(w, &down_list, false);
 			w->power = 0;
 			powerdown = 1;
 		}
@@ -2383,7 +2396,7 @@ static void soc_dapm_shutdown_codec(struct snd_soc_dapm_context *dapm)
 	 */
 	if (powerdown) {
 		snd_soc_dapm_set_bias_level(NULL, dapm, SND_SOC_BIAS_PREPARE);
-		dapm_seq_run(dapm, &down_list, 0, dapm_down_seq);
+		dapm_seq_run(dapm, &down_list, 0, false);
 		snd_soc_dapm_set_bias_level(NULL, dapm, SND_SOC_BIAS_STANDBY);
 	}
 }
-- 
1.7.2.3

[PATCH 2/3] ASoC: Add support for sequencing within

[Mark Brown]

With larger devices there may be many widgets of the same type in series
in an audio path. Allow drivers to specify an additional level of ordering
within each widget type by adding a subsequence number to widgets and then
splitting operations on widgets so that widgets of the same type but
different sequence numbers are processed separately.  A typical example
would be a supply widget which requires that another widget be enabled
to provide power or clocking.

SND_SOC_DAPM_PGA_S() and SND_SOC_DAPM_SUPPLY_S() macros are provided
allowing this to be used with PGAs and supplies as these are the most
commonly affected widgets.

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

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 8031769..a3760c9 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -157,6 +157,18 @@
 	.invert = winvert, .kcontrols = wcontrols, .num_kcontrols = 1, \
 	.event = wevent, .event_flags = wflags}
 
+/* additional sequencing control within an event type */
+#define SND_SOC_DAPM_PGA_S(wname, wsubseq, wreg, wshift, winvert, wcontrols, \
+	wncontrols, wevent, wflags) \
+{	.id = snd_soc_dapm_pga, .name = wname, .reg = wreg, .shift = wshift, \
+	.invert = winvert, .kcontrols = wcontrols, .num_kcontrols = wncontrols, \
+	.event = wevent, .event_flags = wflags, .subseq = wsubseq}
+#define SND_SOC_DAPM_SUPPLY_S(wname, wsubseq, wreg, wshift, winvert, wevent, \
+	wflags)	\
+{	.id = snd_soc_dapm_supply, .name = wname, .reg = wreg,	\
+	.shift = wshift, .invert = winvert, .event = wevent, \
+	.event_flags = wflags, .subseq = wsubseq}
+
 /* Simplified versions of above macros, assuming wncontrols = ARRAY_SIZE(wcontrols) */
 #define SOC_PGA_E_ARRAY(wname, wreg, wshift, winvert, wcontrols, \
 	wevent, wflags) \
@@ -450,6 +462,7 @@ 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 */
+	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 57e1c9f..eb7436c 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -737,6 +737,12 @@ static int dapm_seq_compare(struct snd_soc_dapm_widget *a,
 
 	if (sort[a->id] != sort[b->id])
 		return sort[a->id] - sort[b->id];
+	if (a->subseq != b->subseq) {
+		if (power_up)
+			return a->subseq - b->subseq;
+		else
+			return b->subseq - a->subseq;
+	}
 	if (a->reg != b->reg)
 		return a->reg - b->reg;
 	if (a->dapm != b->dapm)
@@ -869,6 +875,7 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 	struct snd_soc_dapm_widget *w, *n;
 	LIST_HEAD(pending);
 	int cur_sort = -1;
+	int cur_subseq = -1;
 	int cur_reg = SND_SOC_NOPM;
 	struct snd_soc_dapm_context *cur_dapm = NULL;
 	int ret;
@@ -884,12 +891,13 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 
 		/* Do we need to apply any queued changes? */
 		if (sort[w->id] != cur_sort || w->reg != cur_reg ||
-		    w->dapm != cur_dapm) {
+		    w->dapm != cur_dapm || w->subseq != cur_subseq) {
 			if (!list_empty(&pending))
 				dapm_seq_run_coalesced(cur_dapm, &pending);
 
 			INIT_LIST_HEAD(&pending);
 			cur_sort = -1;
+			cur_subseq = -1;
 			cur_reg = SND_SOC_NOPM;
 			cur_dapm = NULL;
 		}
@@ -934,6 +942,7 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 		default:
 			/* Queue it up for application */
 			cur_sort = sort[w->id];
+			cur_subseq = w->subseq;
 			cur_reg = w->reg;
 			cur_dapm = w->dapm;
 			list_move(&w->power_list, &pending);
-- 
1.7.2.3

Re: [PATCH 2/3] ASoC: Add support for sequencing within

[Peter Ujfalusi]

On 01/18/11 18:20, ext Mark Brown wrote:
> With larger devices there may be many widgets of the same type in series
> in an audio path. Allow drivers to specify an additional level of ordering
> within each widget type by adding a subsequence number to widgets and then
> splitting operations on widgets so that widgets of the same type but
> different sequence numbers are processed separately.  A typical example
> would be a supply widget which requires that another widget be enabled
> to provide power or clocking.
> 
> SND_SOC_DAPM_PGA_S() and SND_SOC_DAPM_SUPPLY_S() macros are provided
> allowing this to be used with PGAs and supplies as these are the most
> commonly affected widgets.

Not really for this series...
I have been also wondering on the power up/down sequence ordering in a
complex setup.
I might be totally wrong, but never the less:
I have been wondering, if we just go, and follow the sequence of the
DAPM widgets in on the path on powerup, and follow the reverse sequence
on powerdown.

Take something like this:
DAC->PGA1->MIXER1->PGA2->MUX->PGA3->MIXER2->OUTPUT->SPEAKER
 ^      ^                       ^
SUPPLY1 SUPPLY2                 SUPPLY3

The sequence now:
SUPPLY1->SUPPLY2->SUPPLY3->MUX->DAC->MIXER1->MIXER2->PGA1->PGA2->PGA3->SPEAKER

Now we are going to be able to change the order of PGA, and SUPPLY.
We were able to do that earlier by changing the order of the PGA/SUPPLY
within the snd_soc_dapm_widget struct (I know... it is not really explicit).

But if we actually follow the route we could have sequence of:
SUPPLY1->DAC->SUPPLY2->PGA1->MIXER1->PGA2->MUX->SUPPLY3->PGA3->MIXER2->SPEAKER

When we build up the DAPM tree, we usually follow the spec, basically
modeling the HW, so we can take care of the recommended sequences.

IMHO in this way we would have better control over the sequences, and we
might be in better position to combat pop noises, since we have more
explicit control over the sequence of events.

I have not looked how hard it would be to change the DAPM to do this.
Anyways I would not started to do this without asking... I know there
are really good reasons to have the current DAPM sequence handling, but
I do wonder, if it make sense to do this.

What do you think?

-- 
Péter

Re: [PATCH 2/3] ASoC: Add support for sequencing within

[Mark Brown]

On Wed, Jan 19, 2011 at 10:19:38AM +0200, Peter Ujfalusi wrote:

> I have been wondering, if we just go, and follow the sequence of the
> DAPM widgets in on the path on powerup, and follow the reverse sequence
> on powerdown.

It's certainly worth considering.  It's not unambiguously clear that
this is ideal - there's a few cases where you pretty much always want to
do things in a different order to the straight path, the primary one
being that since PGAs make everything more noisy you generally want to
power them on last.  There's also the issue that you don't want to
oversequence things, more often than not bringing things up in parallel
performs at least as well or even better.

> IMHO in this way we would have better control over the sequences, and we
> might be in better position to combat pop noises, since we have more
> explicit control over the sequence of events.

OOI, do we actually have issues here at the minute?  The only issue I'm
aware of is that we don't have a facility for keeping PGAs muted while
sequences are running.

> I have not looked how hard it would be to change the DAPM to do this.
> Anyways I would not started to do this without asking... I know there
> are really good reasons to have the current DAPM sequence handling, but
> I do wonder, if it make sense to do this.

It's potentially useful.  There are some considerations as above that
mean we don't want a straight graph walk but there's certainly some room
for using the infrastructure.

[PATCH 3/3] ASoC: Provide per widget type callback when executing DAPM sequences

[Mark Brown]

Many modern devices have features such as DC servos which take time to start.
Currently these are handled by per-widget events but this makes it difficult
to paralleise operations on multiple widgets, meaning delays can end up
being needlessly serialised. By providing a callback to drivers when all
widgets of a given type have been handled during a DAPM sequence the core
allows drivers to start operations separately and wait for them to complete
much more simply.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/sound/soc-dapm.h |    3 +++
 include/sound/soc.h      |    3 +++
 sound/soc/soc-core.c     |    1 +
 sound/soc/soc-dapm.c     |   16 +++++++++++++++-
 4 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index a3760c9..6c9ae23 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -500,6 +500,9 @@ struct snd_soc_dapm_context {
 
 	struct snd_soc_dapm_update *update;
 
+	void (*seq_notifier)(struct snd_soc_dapm_context *,
+			     enum snd_soc_dapm_type);
+
 	struct device *dev; /* from parent - for debug */
 	struct snd_soc_codec *codec; /* parent codec */
 	struct snd_soc_card *card; /* parent card */
diff --git a/include/sound/soc.h b/include/sound/soc.h
index d824e70..c0d2f4b 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -545,6 +545,9 @@ struct snd_soc_codec_driver {
 	/* codec bias level */
 	int (*set_bias_level)(struct snd_soc_codec *,
 			      enum snd_soc_bias_level level);
+
+	void (*seq_notifier)(struct snd_soc_dapm_context *,
+			     enum snd_soc_dapm_type);
 };
 
 /* SoC platform interface */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8305712..08d7b98 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3496,6 +3496,7 @@ int snd_soc_register_codec(struct device *dev,
 	codec->dapm.bias_level = SND_SOC_BIAS_OFF;
 	codec->dapm.dev = dev;
 	codec->dapm.codec = codec;
+	codec->dapm.seq_notifier = codec_drv->seq_notifier;
 	codec->dev = dev;
 	codec->driver = codec_drv;
 	codec->num_dai = num_dai;
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index eb7436c..37b376f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -878,7 +878,7 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 	int cur_subseq = -1;
 	int cur_reg = SND_SOC_NOPM;
 	struct snd_soc_dapm_context *cur_dapm = NULL;
-	int ret;
+	int ret, i;
 	int *sort;
 
 	if (power_up)
@@ -895,6 +895,13 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 			if (!list_empty(&pending))
 				dapm_seq_run_coalesced(cur_dapm, &pending);
 
+			if (cur_dapm && cur_dapm->seq_notifier) {
+				for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
+					if (sort[i] == cur_sort)
+						cur_dapm->seq_notifier(cur_dapm,
+								       i);
+			}
+
 			INIT_LIST_HEAD(&pending);
 			cur_sort = -1;
 			cur_subseq = -1;
@@ -956,6 +963,13 @@ static void dapm_seq_run(struct snd_soc_dapm_context *dapm,
 
 	if (!list_empty(&pending))
 		dapm_seq_run_coalesced(dapm, &pending);
+
+	if (cur_dapm && cur_dapm->seq_notifier) {
+		for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
+			if (sort[i] == cur_sort)
+				cur_dapm->seq_notifier(cur_dapm,
+						       i);
+	}
 }
 
 static void dapm_widget_update(struct snd_soc_dapm_context *dapm)
-- 
1.7.2.3