[PATCH] ASoC: core: Add platform DAI widget mapping.

[PATCH] ASoC: core: Add platform DAI widget mapping.

[Liam Girdwood]

Add platform driver support for CPU DAI DAPM widgets.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 include/sound/soc-dai.h |    1 +
 sound/soc/soc-core.c    |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index c429f24..3248fbc 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -241,6 +241,7 @@ struct snd_soc_dai {
 
 	struct snd_soc_dapm_widget *playback_widget;
 	struct snd_soc_dapm_widget *capture_widget;
+	struct snd_soc_dapm_context dapm;
 
 	/* DAI DMA data */
 	void *playback_dma_data;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 5aef34a..f466b9d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1074,6 +1074,7 @@ static int soc_probe_platform(struct snd_soc_card *card,
 {
 	int ret = 0;
 	const struct snd_soc_platform_driver *driver = platform->driver;
+	struct snd_soc_dai *dai;
 
 	platform->card = card;
 	platform->dapm.card = card;
@@ -1087,6 +1088,14 @@ static int soc_probe_platform(struct snd_soc_card *card,
 		snd_soc_dapm_new_controls(&platform->dapm,
 			driver->dapm_widgets, driver->num_dapm_widgets);
 
+	/* Create DAPM widgets for each DAI stream */
+	list_for_each_entry(dai, &dai_list, list) {
+		if (dai->dev != platform->dev)
+			continue;
+
+		snd_soc_dapm_new_dai_widgets(&platform->dapm, dai);
+	}
+
 	if (driver->probe) {
 		ret = driver->probe(platform);
 		if (ret < 0) {
@@ -1222,9 +1231,12 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num, int order)
 	/* probe the cpu_dai */
 	if (!cpu_dai->probed &&
 			cpu_dai->driver->probe_order == order) {
+		cpu_dai->dapm.card = card;
 		if (!try_module_get(cpu_dai->dev->driver->owner))
 			return -ENODEV;
 
+		snd_soc_dapm_new_dai_widgets(&cpu_dai->dapm, cpu_dai);
+
 		if (cpu_dai->driver->probe) {
 			ret = cpu_dai->driver->probe(cpu_dai);
 			if (ret < 0) {
@@ -3241,6 +3253,7 @@ int snd_soc_register_dai(struct device *dev,
 
 	dai->dev = dev;
 	dai->driver = dai_drv;
+	dai->dapm.dev = dev;
 	if (!dai->driver->ops)
 		dai->driver->ops = &null_dai_ops;
 
@@ -3317,6 +3330,7 @@ int snd_soc_register_dais(struct device *dev,
 			dai->id = dai->driver->id;
 		else
 			dai->id = i;
+		dai->dapm.dev = dev;
 		if (!dai->driver->ops)
 			dai->driver->ops = &null_dai_ops;
 
-- 
1.7.5.4

[PATCH] ASoC: core: Add support for DAI driver DAI widgets.

[Liam Girdwood]

Automatically create DAI widgets for DAI drivers. This allows us to connect
CPU DAIs into the DAPM graph. The widgets use the DAI driver dev name with
a "Playback" or "Capture" suffix.

i.e. "mcbsp.1 Playback"

Also add a snd_soc_dai member to snd_soc_widget so that we dont have to
use the widget private data.

Signed-off-by: Liam Girdwood <lrg@ti.com>
---
 include/sound/soc-dapm.h |    1 +
 sound/soc/soc-dapm.c     |   56 +++++++++++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 6727384..8cd3c74 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -482,6 +482,7 @@ struct snd_soc_dapm_widget {
 	const char *sname;	/* stream name */
 	struct snd_soc_codec *codec;
 	struct snd_soc_platform *platform;
+	struct snd_soc_dai *dai;
 	struct list_head list;
 	struct snd_soc_dapm_context *dapm;
 
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 5c83b83..d15a703 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -44,6 +44,7 @@
 
 #include <trace/events/asoc.h>
 
+#define NAME_SIZE	32
 #define DAPM_UPDATE_STAT(widget, val) widget->dapm->card->dapm_stats.val++;
 
 /* dapm power sequences - make this per codec in the future */
@@ -2911,6 +2912,7 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
 {
 	struct snd_soc_dapm_widget template;
 	struct snd_soc_dapm_widget *w;
+	char name[NAME_SIZE];
 
 	WARN_ON(dapm->dev != dai->dev);
 
@@ -2918,41 +2920,49 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
 	template.reg = SND_SOC_NOPM;
 
 	if (dai->driver->playback.stream_name) {
-		template.id = snd_soc_dapm_dai;
 		template.name = dai->driver->playback.stream_name;
 		template.sname = dai->driver->playback.stream_name;
+	} else {
+		snprintf(name, NAME_SIZE, "%s %s", dev_name(dai->dev), "Playback");
+		template.name = name;
+		template.sname = name;
+	}
 
-		dev_dbg(dai->dev, "adding %s widget\n",
-			template.name);
+	template.id = snd_soc_dapm_dai;
+	dev_dbg(dai->dev, "adding %s widget\n",
+		template.name);
 
-		w = snd_soc_dapm_new_control(dapm, &template);
-		if (!w) {
-			dev_err(dapm->dev, "Failed to create %s widget\n",
-				dai->driver->playback.stream_name);
-		}
-
-		w->priv = dai;
-		dai->playback_widget = w;
+	w = snd_soc_dapm_new_control(dapm, &template);
+	if (!w) {
+		dev_err(dapm->dev, "Failed to create %s widget\n",
+			dai->driver->playback.stream_name);
 	}
 
+	dai->playback_widget = w;
+	w->dai = dai;
+
 	if (dai->driver->capture.stream_name) {
-		template.id = snd_soc_dapm_dai;
 		template.name = dai->driver->capture.stream_name;
 		template.sname = dai->driver->capture.stream_name;
+	} else {
+		snprintf(name, NAME_SIZE, "%s %s", dev_name(dai->dev), "Capture");
+		template.name = name;
+		template.sname = name;
+	}
+	template.id = snd_soc_dapm_dai;
 
-		dev_dbg(dai->dev, "adding %s widget\n",
-			template.name);
+	dev_dbg(dai->dev, "adding %s widget\n",
+		template.name);
 
-		w = snd_soc_dapm_new_control(dapm, &template);
-		if (!w) {
-			dev_err(dapm->dev, "Failed to create %s widget\n",
-				dai->driver->capture.stream_name);
-		}
-
-		w->priv = dai;
-		dai->capture_widget = w;
+	w = snd_soc_dapm_new_control(dapm, &template);
+	if (!w) {
+		dev_err(dapm->dev, "Failed to create %s widget\n",
+			dai->driver->capture.stream_name);
 	}
 
+	dai->capture_widget = w;
+	w->dai = dai;
+
 	return 0;
 }
 
@@ -2969,7 +2979,7 @@ int snd_soc_dapm_link_dai_widgets(struct snd_soc_card *card)
 		if (dai_w->id != snd_soc_dapm_dai)
 			continue;
 
-		dai = dai_w->priv;
+		dai = dai_w->dai;
 
 		/* ...find all widgets with the same stream and link them */
 		list_for_each_entry(w, &card->widgets, list) {
-- 
1.7.5.4

Re: [PATCH] ASoC: core: Add platform DAI widget mapping.

[Mark Brown]

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

On Wed, Mar 07, 2012 at 11:47:41AM +0000, Liam Girdwood wrote:
> Add platform driver support for CPU DAI DAPM widgets.

Applied, thanks.

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

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

Re: [PATCH] ASoC: core: Add support for DAI driver DAI widgets.

[Mark Brown]

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

On Wed, Mar 07, 2012 at 11:47:42AM +0000, Liam Girdwood wrote:

> Automatically create DAI widgets for DAI drivers. This allows us to connect
> CPU DAIs into the DAPM graph. The widgets use the DAI driver dev name with
> a "Playback" or "Capture" suffix.

So what this is doing is the above which is really about synthesising a
name for streams where the driver didn't provide one.  This isn't
specific to CPU DAIs, it'll apply just as well to CODEC DAIs that don't
specify a per stream name for whatever reason.  

> Also add a snd_soc_dai member to snd_soc_widget so that we dont have to
> use the widget private data.

If we're going to do that I think we should just drop the private data
field, this is exactly the sort of stuff it was added for.  Currently
it's only used by DAIs and regulator supplies.

> +#define NAME_SIZE	32

32 bytes is enough for everyone!  :P  Actually it should be so no real
problem.

>  	if (dai->driver->playback.stream_name) {
> -		template.id = snd_soc_dapm_dai;
>  		template.name = dai->driver->playback.stream_name;
>  		template.sname = dai->driver->playback.stream_name;
> +	} else {
> +		snprintf(name, NAME_SIZE, "%s %s", dev_name(dai->dev), "Playback");
> +		template.name = name;
> +		template.sname = name;
> +	}

Isn't this going to get complicated to use when building up the DAPM
routes?  The device is going to need to know its own dev_name() to
supply a table of routes which would mean you'd have to dynamically
create the routing table (or just specify a name and skip the whole
thing I guess).  

Could we not just use plain old "Playback" and "Capture" here?  We could
then use the name_prefix mechanism which we've got to support multiple
CODECs with the CPU side stuff as well, that will then take care of the
DAPM routes in the same way it does for the CODECs - it'd allow them to
use simple data tables with no dev_name in them which seems like a win.
We'd want to supply a default name_prefix for CPU DAIs but that seems
OK.

This would also ensure that this mechanism works for multi-CODEC systems
which I've got a horrible feeling are going to run into trouble at some
point soon for the same reason you're adding in the dev_names here.

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

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

Re: [PATCH] ASoC: core: Add support for DAI driver DAI widgets.

[Liam Girdwood]

On Wed, 2012-03-07 at 20:37 +0000, Mark Brown wrote:
> On Wed, Mar 07, 2012 at 11:47:42AM +0000, Liam Girdwood wrote:
> 
> > Automatically create DAI widgets for DAI drivers. This allows us to connect
> > CPU DAIs into the DAPM graph. The widgets use the DAI driver dev name with
> > a "Playback" or "Capture" suffix.
> 
> So what this is doing is the above which is really about synthesising a
> name for streams where the driver didn't provide one.  This isn't
> specific to CPU DAIs, it'll apply just as well to CODEC DAIs that don't
> specify a per stream name for whatever reason.  

yes, but it's more about creating a DAPM DAI widget for DAIs that are
not named by their drivers. 

> 
> > Also add a snd_soc_dai member to snd_soc_widget so that we dont have to
> > use the widget private data.
> 
> If we're going to do that I think we should just drop the private data
> field, this is exactly the sort of stuff it was added for.  Currently
> it's only used by DAIs and regulator supplies.
> 

ok, we could add a regulator * in place of the void * in a separate
patch

> > +#define NAME_SIZE	32
> 
> 32 bytes is enough for everyone!  :P  Actually it should be so no real
> problem.
> 
> >  	if (dai->driver->playback.stream_name) {
> > -		template.id = snd_soc_dapm_dai;
> >  		template.name = dai->driver->playback.stream_name;
> >  		template.sname = dai->driver->playback.stream_name;
> > +	} else {
> > +		snprintf(name, NAME_SIZE, "%s %s", dev_name(dai->dev), "Playback");
> > +		template.name = name;
> > +		template.sname = name;
> > +	}
> 
> Isn't this going to get complicated to use when building up the DAPM
> routes?  The device is going to need to know its own dev_name() to
> supply a table of routes which would mean you'd have to dynamically
> create the routing table (or just specify a name and skip the whole
> thing I guess).  

I'm not needing to dynamically create the table, it all works as
before :-

	/* Connections between twl6040 and ABE */
	{"Headset Playback", NULL, "PDM_DL1"},
	{"Handsfree Playback", NULL, "PDM_DL2"},
	{"Vibra Playback", NULL, "PDM_VIB"},
	{"PDM_UL1", NULL, "PDM Capture"},

	/* Bluetooth <--> ABE*/
	{"omap-mcbsp-dai.0 Playback", NULL, "BT_VX_DL"},
	{"BT_VX_UL", NULL, "omap-mcbsp-dai.0 Capture"},

This is the machine driver DAPM table. Here we are connecting the DAI
widgets for twl6040 (using existing DAI widget codec code) to the ABE
and the ABE to the Bluetooth device (using this patch).

This does give us DAPM loopback support for codec<->codec as we will get
a widget event for this DAI link.

> 
> Could we not just use plain old "Playback" and "Capture" here?  We could
> then use the name_prefix mechanism which we've got to support multiple
> CODECs with the CPU side stuff as well, that will then take care of the
> DAPM routes in the same way it does for the CODECs - it'd allow them to
> use simple data tables with no dev_name in them which seems like a win.
> We'd want to supply a default name_prefix for CPU DAIs but that seems
> OK.

The intention here was for CPU DAIs only, the assumption was that the
codecs would have a DAPM mapping to DAI or DAC/ADC by default. I can
certainly change this to ignore the codec DAIs and leave them as is ?

> 
> This would also ensure that this mechanism works for multi-CODEC systems
> which I've got a horrible feeling are going to run into trouble at some
> point soon for the same reason you're adding in the dev_names here.


Liam

Re: [PATCH] ASoC: core: Add support for DAI driver DAI widgets.

[Mark Brown]

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

On Thu, Mar 08, 2012 at 12:07:00PM +0000, Liam Girdwood wrote:
> On Wed, 2012-03-07 at 20:37 +0000, Mark Brown wrote:

> > So what this is doing is the above which is really about synthesising a
> > name for streams where the driver didn't provide one.  This isn't
> > specific to CPU DAIs, it'll apply just as well to CODEC DAIs that don't
> > specify a per stream name for whatever reason.  

> yes, but it's more about creating a DAPM DAI widget for DAIs that are
> not named by their drivers. 

Yes, that's what I said :)  It just confused me because it applies to
any DAI.

> > > Also add a snd_soc_dai member to snd_soc_widget so that we dont have to
> > > use the widget private data.

> > If we're going to do that I think we should just drop the private data
> > field, this is exactly the sort of stuff it was added for.  Currently
> > it's only used by DAIs and regulator supplies.

> ok, we could add a regulator * in place of the void * in a separate
> patch

I guess.  If anything I'd rather be pulling stuff out of the core widget
into private per widget type data structures as we do end up with a fair
number of widgets but at this point we're a long way from doing that so
either way is fine.

> > Isn't this going to get complicated to use when building up the DAPM
> > routes?  The device is going to need to know its own dev_name() to
> > supply a table of routes which would mean you'd have to dynamically
> > create the routing table (or just specify a name and skip the whole
> > thing I guess).  

> I'm not needing to dynamically create the table, it all works as
> before :-

> This is the machine driver DAPM table. Here we are connecting the DAI
> widgets for twl6040 (using existing DAI widget codec code) to the ABE
> and the ABE to the Bluetooth device (using this patch).

This only works for the machine driver, not for the driver adding the
DAI itself.

> The intention here was for CPU DAIs only, the assumption was that the
> codecs would have a DAPM mapping to DAI or DAC/ADC by default. I can
> certainly change this to ignore the codec DAIs and leave them as is ?

Why not use the existing infrastructure?  It's more general in that it
covers other things like the regular controls which some CPUs have but
mostly don't currently use (digital gains and the like) as well as
allowing the DAI driver to map its own widgets if it wants to.

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

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