[PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Liam Girdwood]

In preparation for Dynamic PCM (AKA DSP) support.

Add a DAPM API call to determine whether a DAPM audio path is valid between
source and sink widgets and return the path widgets. This also takes into
account all kcontrol mux and mixer settings in between the source and sink
widgets to validate the audio path.

This will be used by the Dynamic PCM core to determine the runtime DAI mappings
between FE and BE DAIs in order to run PCM operations.

This modifies the existing is_connected_input_ep() and is_connected_output_ep() by
adding a new parameter to store any valid widgets in a list. We also add some extra
debug to provide further insight into the DAPM path and widget discovery.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 include/sound/soc-dapm.h |    6 ++
 sound/soc/soc-dapm.c     |  184 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 176 insertions(+), 14 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 5833f01..a56ead2 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -310,6 +310,7 @@ struct snd_soc_dapm_path;
 struct snd_soc_dapm_pin;
 struct snd_soc_dapm_route;
 struct snd_soc_dapm_context;
+struct snd_soc_dapm_widget_list;
 
 int dapm_reg_event(struct snd_soc_dapm_widget *w,
 		   struct snd_kcontrol *kcontrol, int event);
@@ -375,6 +376,11 @@ int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm,
 int snd_soc_dapm_ignore_suspend(struct snd_soc_dapm_context *dapm,
 				const char *pin);
 
+/* dapm path query */
+int snd_soc_dapm_get_connected_widgets(struct snd_soc_dapm_context *dapm,
+		const char *stream_name, struct snd_soc_dapm_widget_list **list,
+		int stream);
+
 /* dapm widget types */
 enum snd_soc_dapm_type {
 	snd_soc_dapm_input = 0,		/* input pin */
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 80f97b2..120df5b 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -605,11 +605,51 @@ static int snd_soc_dapm_suspend_check(struct snd_soc_dapm_widget *widget)
 	}
 }
 
+/* add widget to list if it's not already in the list */
+static int dapm_list_add_widget(struct snd_soc_dapm_widget_list **list,
+	struct snd_soc_dapm_widget *w)
+{
+	struct snd_soc_dapm_widget_list *wlist;
+	int wlistsize, wlistentries, i;
+
+	if (*list == NULL)
+		return -EINVAL;
+
+	wlist = *list;
+
+	/* is this widget already in the list */
+	for (i = 0; i < wlist->num_widgets; i++) {
+		if (wlist->widgets[i] == w)
+			return 0;
+	}
+
+	/* allocate some new space */
+	wlistentries = wlist->num_widgets + 1;
+	wlistsize = sizeof(struct snd_soc_dapm_widget_list) +
+			wlistentries * sizeof(struct snd_soc_dapm_widget *);
+	*list = krealloc(wlist, wlistsize, GFP_KERNEL);
+	if (*list == NULL) {
+		dev_err(w->dapm->dev, "can't allocate widget list for %s\n",
+			w->name);
+		return -ENOMEM;
+	}
+	wlist = *list;
+
+	/* insert the widget */
+	dev_dbg(w->dapm->dev, "added %s in widget list pos %d\n",
+			w->name, wlist->num_widgets);
+
+	wlist->widgets[wlist->num_widgets] = w;
+	wlist->num_widgets++;
+	return 1;
+}
+
 /*
  * Recursively check for a completed path to an active or physically connected
  * output widget. Returns number of complete paths.
  */
-static int is_connected_output_ep(struct snd_soc_dapm_widget *widget)
+static int is_connected_output_ep(struct snd_soc_dapm_widget *widget,
+	struct snd_soc_dapm_widget_list **list)
 {
 	struct snd_soc_dapm_path *path;
 	int con = 0;
@@ -638,15 +678,35 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget)
 	}
 
 	list_for_each_entry(path, &widget->sinks, list_source) {
-		if (path->weak)
-			continue;
 
 		if (path->walked)
 			continue;
 
+		dev_dbg(widget->dapm->dev," %c : %s -> %s -> %s : %c\n",
+				path->sink && path->connect ? '*' : ' ',
+				widget->name, path->name, path->sink->name,
+				path->weak ? 'w': ' ');
+
+		if (path->weak)
+			continue;
+
 		if (path->sink && path->connect) {
+
 			path->walked = 1;
-			con += is_connected_output_ep(path->sink);
+
+			/* do we need to add this widget to the list ? */
+			if (list) {
+				int err;
+
+				err = dapm_list_add_widget(list, path->sink);
+				if (err < 0) {
+					dev_err(widget->dapm->dev, "could not add widget %s\n",
+						widget->name);
+					return con;
+				}
+			}
+
+			con += is_connected_output_ep(path->sink, list);
 		}
 	}
 
@@ -657,7 +717,8 @@ static int is_connected_output_ep(struct snd_soc_dapm_widget *widget)
  * Recursively check for a completed path to an active or physically connected
  * input widget. Returns number of complete paths.
  */
-static int is_connected_input_ep(struct snd_soc_dapm_widget *widget)
+static int is_connected_input_ep(struct snd_soc_dapm_widget *widget,
+	struct snd_soc_dapm_widget_list **list)
 {
 	struct snd_soc_dapm_path *path;
 	int con = 0;
@@ -691,21 +752,116 @@ static int is_connected_input_ep(struct snd_soc_dapm_widget *widget)
 	}
 
 	list_for_each_entry(path, &widget->sources, list_sink) {
-		if (path->weak)
-			continue;
 
 		if (path->walked)
 			continue;
 
+		if (path->source)
+			dev_vdbg(widget->dapm->dev," %c : %s <- %s <- %s : %c\n",
+				path->source && path->connect ? '*' : ' ',
+				widget->name, path->name, path->source->name,
+				path->weak ? 'w': ' ');
+
+		if (path->weak)
+			continue;
+
 		if (path->source && path->connect) {
+
 			path->walked = 1;
-			con += is_connected_input_ep(path->source);
+
+			/* do we need to add this widget to the list ? */
+			if (list) {
+				int err;
+
+				err = dapm_list_add_widget(list, path->source);
+				if (err < 0) {
+					dev_err(widget->dapm->dev, "could not add widget %s\n",
+						widget->name);
+					return con;
+				}
+			}
+
+			con += is_connected_input_ep(path->source, list);
 		}
 	}
 
 	return con;
 }
 
+static int dapm_get_playback_paths(struct snd_soc_dapm_context *dapm,
+		struct snd_soc_dapm_widget *root,
+		struct snd_soc_dapm_widget_list **list)
+{
+	int paths;
+
+	dev_dbg(dapm->dev, "Playback: checking paths from %s\n",root->name);
+	paths = is_connected_output_ep(root, list);
+	dev_dbg(dapm->dev, "Playback: found %d paths from %s\n", paths, root->name);
+
+	return paths;
+}
+
+static int dapm_get_capture_paths(struct snd_soc_dapm_context *dapm,
+		struct snd_soc_dapm_widget *root,
+		struct snd_soc_dapm_widget_list **list)
+{
+	int paths;
+
+	dev_dbg(dapm->dev, "Capture: checking paths from %s\n",root->name);
+	paths = is_connected_input_ep(root, list);
+	dev_dbg(dapm->dev, "Capture: found %d paths from %s\n", paths, root->name);
+
+	return paths;
+}
+
+/**
+ * snd_soc_dapm_get_connected_widgets - query audio path and it's widgets.
+ * @dapm: the dapm context.
+ * @stream_name: stream name.
+ * @list: list of active widgets for this stream.
+ * @stream: stream direction.
+ *
+ * Queries DAPM graph as to whether an valid audio stream path exists for
+ * the initial stream specified by name. This takes into account
+ * current mixer and mux kcontrol settings. Creates list of valid widgets.
+ *
+ * Returns the number of valid paths or negative error.
+ */
+int snd_soc_dapm_get_connected_widgets(struct snd_soc_dapm_context *dapm,
+	const char *stream_name, struct snd_soc_dapm_widget_list **list,
+	int stream)
+{
+	struct snd_soc_dapm_widget *w;
+	enum snd_soc_dapm_type type;
+	int paths;
+
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		type = snd_soc_dapm_aif_in;
+	else
+		type = snd_soc_dapm_aif_out;
+
+	/* get stream root widget AIF from stream string and direction */
+	list_for_each_entry(w, &dapm->card->widgets, list) {
+
+		if (w->id != type)
+			continue;
+
+		if (!strcmp(w->sname, stream_name))
+			goto found;
+	}
+	dev_err(dapm->dev, "root widget for %s not found\n", stream_name);
+	return 0;
+
+found:
+	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+		paths = dapm_get_playback_paths(dapm, w, list);
+	else
+		paths = dapm_get_capture_paths(dapm, w, list);
+
+	dapm_clear_walk(dapm);
+	return paths;
+}
+
 /*
  * Handler for generic register modifier widget.
  */
@@ -732,9 +888,9 @@ static int dapm_generic_check_power(struct snd_soc_dapm_widget *w)
 {
 	int in, out;
 
-	in = is_connected_input_ep(w);
+	in = is_connected_input_ep(w, NULL);
 	dapm_clear_walk(w->dapm);
-	out = is_connected_output_ep(w);
+	out = is_connected_output_ep(w, NULL);
 	dapm_clear_walk(w->dapm);
 	return out != 0 && in != 0;
 }
@@ -745,7 +901,7 @@ static int dapm_adc_check_power(struct snd_soc_dapm_widget *w)
 	int in;
 
 	if (w->active) {
-		in = is_connected_input_ep(w);
+		in = is_connected_input_ep(w, NULL);
 		dapm_clear_walk(w->dapm);
 		return in != 0;
 	} else {
@@ -759,7 +915,7 @@ static int dapm_dac_check_power(struct snd_soc_dapm_widget *w)
 	int out;
 
 	if (w->active) {
-		out = is_connected_output_ep(w);
+		out = is_connected_output_ep(w, NULL);
 		dapm_clear_walk(w->dapm);
 		return out != 0;
 	} else {
@@ -1315,9 +1471,9 @@ static ssize_t dapm_widget_power_read_file(struct file *file,
 	if (!buf)
 		return -ENOMEM;
 
-	in = is_connected_input_ep(w);
+	in = is_connected_input_ep(w, NULL);
 	dapm_clear_walk(w->dapm);
-	out = is_connected_output_ep(w);
+	out = is_connected_output_ep(w, NULL);
 	dapm_clear_walk(w->dapm);
 
 	ret = snprintf(buf, PAGE_SIZE, "%s: %s  in %d out %d",
-- 
1.7.4.1

Re: [PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Mark Brown]

On Mon, Jul 25, 2011 at 11:15:26AM +0100, Liam Girdwood wrote:

> +int snd_soc_dapm_get_connected_widgets(struct snd_soc_dapm_context *dapm,
> +	const char *stream_name, struct snd_soc_dapm_widget_list **list,
> +	int stream)

> +	/* get stream root widget AIF from stream string and direction */
> +	list_for_each_entry(w, &dapm->card->widgets, list) {
> +
> +		if (w->id != type)
> +			continue;
> +
> +		if (!strcmp(w->sname, stream_name))
> +			goto found;
> +	}

Hrm, so I must be missing something here but this still has the issue
with only finding the first widget if we've got more than one widget for
a stream.  For example, the tlv320aic3x driver defines:

   SND_SOC_DAPM_DAC("Left DAC", "Left Playback", DAC_PWR, 7, 0),
   SND_SOC_DAPM_DAC("Right DAC", "Right Playback", DAC_PWR, 6, 0),

and hooks them both up to a single "Playback" stream (most devices don't
even have the distinct names at the widget level).  This makes sense as
DAPM really does want mono streams to work with but if we only look at
the first match we'll fail to enumerate some of the widgets connected to
the stream.  I think the found: block wants to be inside the if
statement and probably just drop the dev_err(), or do it by checking to
see if the list we come up with is empty.

Re: [PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Liam Girdwood]

On Mon, 2011-07-25 at 23:16 +0200, Mark Brown wrote:
> On Mon, Jul 25, 2011 at 11:15:26AM +0100, Liam Girdwood wrote:
> 
> > +int snd_soc_dapm_get_connected_widgets(struct snd_soc_dapm_context *dapm,
> > +	const char *stream_name, struct snd_soc_dapm_widget_list **list,
> > +	int stream)
> 
> > +	/* get stream root widget AIF from stream string and direction */
> > +	list_for_each_entry(w, &dapm->card->widgets, list) {
> > +
> > +		if (w->id != type)
> > +			continue;
> > +
> > +		if (!strcmp(w->sname, stream_name))
> > +			goto found;
> > +	}
> 
> Hrm, so I must be missing something here but this still has the issue
> with only finding the first widget if we've got more than one widget for
> a stream.  For example, the tlv320aic3x driver defines:
> 
>    SND_SOC_DAPM_DAC("Left DAC", "Left Playback", DAC_PWR, 7, 0),
>    SND_SOC_DAPM_DAC("Right DAC", "Right Playback", DAC_PWR, 6, 0),
> 
> and hooks them both up to a single "Playback" stream (most devices don't
> even have the distinct names at the widget level).  

This will only match on the exact name here. So we can retrieve both
left and right stream widgets by making two separate calls. We should
also not have two AIF widgets with the same stream name within a DAPM
context.

> This makes sense as
> DAPM really does want mono streams to work with but if we only look at
> the first match we'll fail to enumerate some of the widgets connected to
> the stream.  

This will only happen if we have AIF widgets with exactly the same
stream names in a dapm context so there should only ever be 1 match.

Liam

Re: [PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Mark Brown]

On Tue, Jul 26, 2011 at 12:16:36PM +0100, Liam Girdwood wrote:
> On Mon, 2011-07-25 at 23:16 +0200, Mark Brown wrote:

> > Hrm, so I must be missing something here but this still has the issue
> > with only finding the first widget if we've got more than one widget for
> > a stream.  For example, the tlv320aic3x driver defines:

> >    SND_SOC_DAPM_DAC("Left DAC", "Left Playback", DAC_PWR, 7, 0),
> >    SND_SOC_DAPM_DAC("Right DAC", "Right Playback", DAC_PWR, 6, 0),

> > and hooks them both up to a single "Playback" stream (most devices don't
> > even have the distinct names at the widget level).  

> This will only match on the exact name here. So we can retrieve both
> left and right stream widgets by making two separate calls. We should
> also not have two AIF widgets with the same stream name within a DAPM
> context.

OK, right.  If we want to do that then a substantial proportion of the
existing CODEC drivers are broken - they're routinely putting in the
stream name they want to match against rather than adding extra text to
the stream to give unique names that are never used.

It does feel like this ought to be looking things up by widget rather
than by stream, though.  Widget names are already guaranteed unique and
if the users do want to look things up for a single widget only it seems
to make sense to ask for that widget rather than ask for a stream as
it's not how we're currently using streams.  It feels like if we're
asking for a stream we should do the same substring multi-match that we
do when pushing events into them.

Re: [PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Liam Girdwood]

On Tue, 2011-07-26 at 13:41 +0200, Mark Brown wrote:
> On Tue, Jul 26, 2011 at 12:16:36PM +0100, Liam Girdwood wrote:
> > On Mon, 2011-07-25 at 23:16 +0200, Mark Brown wrote:
> 
> > > Hrm, so I must be missing something here but this still has the issue
> > > with only finding the first widget if we've got more than one widget for
> > > a stream.  For example, the tlv320aic3x driver defines:
> 
> > >    SND_SOC_DAPM_DAC("Left DAC", "Left Playback", DAC_PWR, 7, 0),
> > >    SND_SOC_DAPM_DAC("Right DAC", "Right Playback", DAC_PWR, 6, 0),
> 
> > > and hooks them both up to a single "Playback" stream (most devices don't
> > > even have the distinct names at the widget level).  
> 
> > This will only match on the exact name here. So we can retrieve both
> > left and right stream widgets by making two separate calls. We should
> > also not have two AIF widgets with the same stream name within a DAPM
> > context.
> 
> OK, right.  If we want to do that then a substantial proportion of the
> existing CODEC drivers are broken - they're routinely putting in the
> stream name they want to match against rather than adding extra text to
> the stream to give unique names that are never used.
> 

They should all be OK, the stream event performs a strstr() on each
stream widget so will match left/right etc streams based on the DAI
stream name. 

> It does feel like this ought to be looking things up by widget rather
> than by stream, though.  Widget names are already guaranteed unique and
> if the users do want to look things up for a single widget only it seems
> to make sense to ask for that widget rather than ask for a stream as
> it's not how we're currently using streams.  It feels like if we're
> asking for a stream we should do the same substring multi-match that we
> do when pushing events into them.

V2 did the substring match, although now that more complex hardware is
appearing I wonder if we should connect DAIs to widgets with the widget
name (rather than just relying on the substring). i.e. each DAI could
have a list of AIF widgets (most would have 1 or 2). Both methods could
co-exist for a while with the substring method being deprecated.

Liam 

Re: [PATCH v3] ASoC: dapm - Add API call to query the widgets of valid DAPM paths.

[Mark Brown]

On Tue, Jul 26, 2011 at 03:26:54PM +0100, Liam Girdwood wrote:
> On Tue, 2011-07-26 at 13:41 +0200, Mark Brown wrote:

> > OK, right.  If we want to do that then a substantial proportion of the
> > existing CODEC drivers are broken - they're routinely putting in the
> > stream name they want to match against rather than adding extra text to
> > the stream to give unique names that are never used.

> They should all be OK, the stream event performs a strstr() on each
> stream widget so will match left/right etc streams based on the DAI
> stream name. 

The case I'm worried about is the case where we've got two widgets with
identical stream names - left and right AIF both call their playback
widget stream "Playback" or whatever.

> > It does feel like this ought to be looking things up by widget rather
> > than by stream, though.  Widget names are already guaranteed unique and
> > if the users do want to look things up for a single widget only it seems
> > to make sense to ask for that widget rather than ask for a stream as
> > it's not how we're currently using streams.  It feels like if we're
> > asking for a stream we should do the same substring multi-match that we
> > do when pushing events into them.

> V2 did the substring match, although now that more complex hardware is
> appearing I wonder if we should connect DAIs to widgets with the widget
> name (rather than just relying on the substring). i.e. each DAI could
> have a list of AIF widgets (most would have 1 or 2). Both methods could
> co-exist for a while with the substring method being deprecated.

Yes, there's definitely room to improve here - one thing I'd like to see
is a system for mapping individual channels in the played audio so if
for example we're playing mono data (or stereo data on a 5.1 CODEC) then
we can figure out that lots of the channels aren't actually in use and
not power them on.