[alsa-devel] [PATCH 0/8] ASoC: simple-card: DPCM support
Hi Mark
These patches are DPCM support for simple-card, and milestone is (*)
* 1. simple DPCM support on DT 2. sampling rate convert support on DPCM 3. multi FE/BE support on DT 4. rsnd multi block IP use multi DPCM
1) - 4) are tidyup patches of simple-card 5) adds DPCM support on simple-card 6) tidyups asoc_simple_card_sub_parse_of() (based on 5) 7) is requred DPCM on Renesas R-Car driver 8) is DT support on dummy driver
5) and 8) are main patch for DPCM on simple-card.
I'm still not 100% understand about DPCM, but 8) patch was needed to DPCM FE/BE route, but is it wrong solution ?
About 5) patch, I know Lars-Peter pointed DT idea for DPCM via http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/media... My 5) patch used this idea, but, not 100%. Because DPCM can use multi FE-BE connection (?) So, it is just pointing BE endpoint via "remote".
/* FrontEnd */ simple-audio-card,dai-link@0 {
remote = <&endpoint>;
cpu { ... }; codec { ... }; };
/* BackEnd */ endpoint: simple-audio-card,dai-link@1 {
cpu { ... }; codec { ... }; };
We can list many DAI in future ?
remote = <&endpoint0, &endpoint1, &endpoint2>;
Kuninori Morimoto (8): 1) ASoC: simple-card: use asoc_simple_xxx prefix 2) ASoC: simple-card: remove dai_link->cpu_dai_name when DT 3) ASoC: simple-card: dai_link->init should be cared when multi DAI 4) ASoC: simple-card: use common for_each_child_of_node() for loop 5) ASoC: simple-card: add DPCM support when DT case 6) ASoC: simple-card: remove is_top_level_node from asoc_simple_card_sub_parse_of() 7) ASoC: rsnd: add dai_link stream name 8) ASoC: add snd-soc-dummy DT support
.../devicetree/bindings/sound/simple-card.txt | 35 +++++++++ .../devicetree/bindings/sound/snd-soc-dummy | 13 +++ sound/soc/generic/simple-card.c | 83 +++++++++++++------- sound/soc/sh/rcar/core.c | 8 ++ sound/soc/sh/rcar/rsnd.h | 1 + sound/soc/soc-utils.c | 12 ++- 6 files changed, 120 insertions(+), 32 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/snd-soc-dummy
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
simple-card driver is using asoc_simple_xxx() prefix. simple_card_dai_link_of() should be asoc_simple_card_dai_link_of().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/generic/simple-card.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 159e517f..f60c68d 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -163,11 +163,11 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return 0; }
-static int simple_card_dai_link_of(struct device_node *node, - struct device *dev, - struct snd_soc_dai_link *dai_link, - struct simple_dai_props *dai_props, - bool is_top_level_node) +static int asoc_simple_card_dai_link_of(struct device_node *node, + struct device *dev, + struct snd_soc_dai_link *dai_link, + struct simple_dai_props *dai_props, + bool is_top_level_node) { struct device_node *np = NULL; struct device_node *bitclkmaster = NULL; @@ -337,16 +337,18 @@ static int asoc_simple_card_parse_of(struct device_node *node, int i; for (i = 0; (np = of_get_next_child(node, np)); i++) { dev_dbg(dev, "\tlink %d:\n", i); - ret = simple_card_dai_link_of(np, dev, dai_link + i, - dai_props + i, false); + ret = asoc_simple_card_dai_link_of(np, dev, + dai_link + i, + dai_props + i, + false); if (ret < 0) { of_node_put(np); return ret; } } } else { - ret = simple_card_dai_link_of(node, dev, dai_link, dai_props, - true); + ret = asoc_simple_card_dai_link_of(node, dev, + dai_link, dai_props, true); if (ret < 0) return ret; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
f687d900d30a61dda38db2a99239f5284a86a309 (ASoC: simple-card: cpu_dai_name creates confusion when DT case) removed dai_link->cpu_dai_name when DT case, since it uses DT phand in soc_bind_dai_link(). This binding will fail if it has cpu_dai_name.
6a91a17bd7b92b2d2aa9ece85457f52a62fd7708 (ASoC: simple-card: Handle many DAI links) added multi DAI link support to simple-card driver. Then, removing cpu_dai_name was cared only single DAI. But, it is needed in all DT cases. This patch moves it to asoc_simple_card_dai_link_of() so that care about all DAIs.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/generic/simple-card.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index f60c68d..b2ca545 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -285,6 +285,17 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_props->codec_dai.fmt, dai_props->codec_dai.sysclk);
+ /* + * soc_bind_dai_link() will check cpu name + * after of_node matching if dai_link has cpu_dai_name. + * but, it will never match if name was created by fmt_single_name() + * remove cpu_dai_name to escape name matching. + * see + * fmt_single_name() + * fmt_multiple_name() + */ + dai_link->cpu_dai_name = NULL; + dai_link_of_err: if (np) of_node_put(np); @@ -429,18 +440,6 @@ static int asoc_simple_card_probe(struct platform_device *pdev) goto err; }
- /* - * soc_bind_dai_link() will check cpu name - * after of_node matching if dai_link has cpu_dai_name. - * but, it will never match if name was created by fmt_single_name() - * remove cpu_dai_name to escape name matching. - * see - * fmt_single_name() - * fmt_multiple_name() - */ - if (num_links == 1) - dai_link->cpu_dai_name = NULL; - } else { struct asoc_simple_card_info *cinfo;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
6a91a17bd7b92b2d2aa9ece85457f52a62fd7708 (ASoC: simple-card: Handle many DAI links) added multi DAI support on simple-card. This means priv->dai_link might be pointer of multi DAI. dai_link->init is needed for all DAI. This patch cares it for all DAIs on DT/non-DT
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/generic/simple-card.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2ca545..de6e631 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -274,6 +274,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_link->codec_dai_name); dai_link->name = dai_link->stream_name = name; dai_link->ops = &asoc_simple_card_ops; + dai_link->init = asoc_simple_card_dai_init;
dev_dbg(dev, "\tname : %s\n", dai_link->stream_name); dev_dbg(dev, "\tcpu : %s / %04x / %d\n", @@ -465,6 +466,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev) dai_link->codec_name = cinfo->codec; dai_link->cpu_dai_name = cinfo->cpu_dai.name; dai_link->codec_dai_name = cinfo->codec_dai.name; + dai_link->init = asoc_simple_card_dai_init; memcpy(&priv->dai_props->cpu_dai, &cinfo->cpu_dai, sizeof(priv->dai_props->cpu_dai)); memcpy(&priv->dai_props->codec_dai, &cinfo->codec_dai, @@ -474,11 +476,6 @@ static int asoc_simple_card_probe(struct platform_device *pdev) priv->dai_props->codec_dai.fmt |= cinfo->daifmt; }
- /* - * init snd_soc_dai_link - */ - dai_link->init = asoc_simple_card_dai_init; - snd_soc_card_set_drvdata(&priv->snd_card, priv);
ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/generic/simple-card.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index de6e631..60d277a 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -346,8 +346,9 @@ static int asoc_simple_card_parse_of(struct device_node *node,
if (multi) { struct device_node *np = NULL; - int i; - for (i = 0; (np = of_get_next_child(node, np)); i++) { + int i = 0; + + for_each_child_of_node(node, np) { dev_dbg(dev, "\tlink %d:\n", i); ret = asoc_simple_card_dai_link_of(np, dev, dai_link + i, @@ -357,6 +358,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, of_node_put(np); return ret; } + i++; } } else { ret = asoc_simple_card_dai_link_of(node, dev,
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch adds DPCM support on simple-card when DT case
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- .../devicetree/bindings/sound/simple-card.txt | 35 ++++++++++++++++++++ sound/soc/generic/simple-card.c | 33 ++++++++++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index c2e9841..60ce432 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -51,6 +51,8 @@ Optional dai-link subnode properties: dai-link uses bit clock inversion. - frame-inversion : bool property. Add this if the dai-link uses frame clock inversion. +- remote : DPCM phandle to backend DAI entry. + and this DAI will be frontend entry.
For backward compatibility the frame-master and bitclock-master properties can be used as booleans in codec subnode to indicate if the @@ -149,3 +151,36 @@ sound { }; }; }; + +Example 3 - DPCM: + +sound { + compatible = "simple-audio-card"; + + /* Front End <-> Back End route */ + simple-audio-card,routing = + "yyy Playback", "xxx Playback", + "xxx Capture", "yyy Capture"; + + simple-audio-card,dai-link@0 { /* Front End */ + ... + remote = <&backend>; + + cpu { + sound-dai = <xxx>; + }; + codec { + sound-dai = <xxx>; + }; + }; + + backend: simple-audio-card,dai-link@1 { /* Back End */ + ... + cpu { + sound-dai = <yyy>; + }; + codec { + sound-dai = <yyy>; + }; + }; +}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 60d277a..9481e70 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -163,7 +163,8 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return 0; }
-static int asoc_simple_card_dai_link_of(struct device_node *node, +static int asoc_simple_card_dai_link_of(struct device_node *parent_node, + struct device_node *node, struct device *dev, struct snd_soc_dai_link *dai_link, struct simple_dai_props *dai_props, @@ -276,6 +277,32 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_link->ops = &asoc_simple_card_ops; dai_link->init = asoc_simple_card_dai_init;
+ /* For DPCM */ + if (parent_node) { + struct device_node *e; + struct device_node *h; + + for_each_child_of_node(parent_node, e) { + h = of_parse_phandle(e, "remote", 0); + if (!h) + continue; + + /* Front End */ + if (e == node) { + dai_link->dynamic = 1; + dai_link->dpcm_playback = 1; /* FIXME */ + dai_link->dpcm_capture = 1; /* FIXME */ + } + + /* Back End */ + if (h == node) { + dai_link->no_pcm = 1; + dai_link->dpcm_playback = 1; /* FIXME */ + dai_link->dpcm_capture = 1; /* FIXME */ + } + } + } + dev_dbg(dev, "\tname : %s\n", dai_link->stream_name); dev_dbg(dev, "\tcpu : %s / %04x / %d\n", dai_link->cpu_dai_name, @@ -350,7 +377,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
for_each_child_of_node(node, np) { dev_dbg(dev, "\tlink %d:\n", i); - ret = asoc_simple_card_dai_link_of(np, dev, + ret = asoc_simple_card_dai_link_of(node, np, dev, dai_link + i, dai_props + i, false); @@ -361,7 +388,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, i++; } } else { - ret = asoc_simple_card_dai_link_of(node, dev, + ret = asoc_simple_card_dai_link_of(NULL, node, dev, dai_link, dai_props, true); if (ret < 0) return ret;
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
asoc_simple_card_sub_parse_of() has parent_node. We can use it instead of is_top_level_node.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/generic/simple-card.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 9481e70..d3aa472 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -167,8 +167,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *parent_node, struct device_node *node, struct device *dev, struct snd_soc_dai_link *dai_link, - struct simple_dai_props *dai_props, - bool is_top_level_node) + struct simple_dai_props *dai_props) { struct device_node *np = NULL; struct device_node *bitclkmaster = NULL; @@ -179,7 +178,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *parent_node, char *prefix = ""; int ret;
- if (is_top_level_node) + if (!parent_node) prefix = "simple-audio-card,";
daifmt = snd_soc_of_parse_daifmt(node, prefix, @@ -379,8 +378,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, dev_dbg(dev, "\tlink %d:\n", i); ret = asoc_simple_card_dai_link_of(node, np, dev, dai_link + i, - dai_props + i, - false); + dai_props + i); if (ret < 0) { of_node_put(np); return ret; @@ -389,7 +387,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, } } else { ret = asoc_simple_card_dai_link_of(NULL, node, dev, - dai_link, dai_props, true); + dai_link, dai_props); if (ret < 0) return ret; }
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch adds missing dai_link stream_name which is used when DPCM
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/rcar/core.c | 8 ++++++++ sound/soc/sh/rcar/rsnd.h | 1 + 2 files changed, 9 insertions(+)
diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 1922ec5..d25b2d7 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -854,19 +854,27 @@ static int rsnd_dai_probe(struct platform_device *pdev, drv[i].name = rdai[i].name; drv[i].ops = &rsnd_soc_dai_ops; if (pmod) { + snprintf(rdai[i].playback.name, RSND_DAI_NAME_SIZE, + "DAI%d Playback", i); + drv[i].playback.rates = RSND_RATES; drv[i].playback.formats = RSND_FMTS; drv[i].playback.channels_min = 2; drv[i].playback.channels_max = 2; + drv[i].playback.stream_name = rdai[i].playback.name;
rdai[i].playback.info = &info->dai_info[i].playback; rsnd_path_init(priv, &rdai[i], &rdai[i].playback); } if (cmod) { + snprintf(rdai[i].capture.name, RSND_DAI_NAME_SIZE, + "DAI%d Capture", i); + drv[i].capture.rates = RSND_RATES; drv[i].capture.formats = RSND_FMTS; drv[i].capture.channels_min = 2; drv[i].capture.channels_max = 2; + drv[i].capture.stream_name = rdai[i].capture.name;
rdai[i].capture.info = &info->dai_info[i].capture; rsnd_path_init(priv, &rdai[i], &rdai[i].capture); diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index d119adf..56b93a0 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -231,6 +231,7 @@ char *rsnd_mod_dma_name(struct rsnd_mod *mod); */ #define RSND_DAI_NAME_SIZE 16 struct rsnd_dai_stream { + char name[RSND_DAI_NAME_SIZE]; struct snd_pcm_substream *substream; struct rsnd_mod *mod[RSND_MOD_MAX]; struct rsnd_dai_path_info *info; /* rcar_snd.h */
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch adds snd-soc-dummy DT binding support. It removes .stream_name from dummy_dai, because "Playback" / "Capture" is very popular naming.
The DAPM will lost correct route settings if other CPU/Codec was using same stream name. And it will be problem when DPCM case. Like below
FE CPU (rsnd): "DAI0 Playback" Codec (dummy): "Playback"
BE CPU (dummy): "Playback" Codec (ak4642): "Playback"
simple-audio-card,routing = "Playback", "DAI0 Playback";
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- .../devicetree/bindings/sound/snd-soc-dummy | 13 +++++++++++++ sound/soc/soc-utils.c | 12 +++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/snd-soc-dummy
diff --git a/Documentation/devicetree/bindings/sound/snd-soc-dummy b/Documentation/devicetree/bindings/sound/snd-soc-dummy new file mode 100644 index 0000000..ea3fe0c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/snd-soc-dummy @@ -0,0 +1,13 @@ +snd-soc-dummy: + +ALSA SoC dummy codec. + +Required properties: + + - compatible : "alsa-soc-dummy" + +Example: + +sound_dummy { + compatible = "alsa-soc-dummy"; +}; diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 7f22ca3..df11010 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -12,7 +12,8 @@ * Free Software Foundation; either version 2 of the License, or (at your * option) any later version. */ - +#include <linux/module.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/export.h> #include <sound/core.h> @@ -104,14 +105,12 @@ static struct snd_soc_codec_driver dummy_codec; static struct snd_soc_dai_driver dummy_dai = { .name = "snd-soc-dummy-dai", .playback = { - .stream_name = "Playback", .channels_min = 1, .channels_max = 384, .rates = STUB_RATES, .formats = STUB_FORMATS, }, .capture = { - .stream_name = "Capture", .channels_min = 1, .channels_max = 384, .rates = STUB_RATES, @@ -151,10 +150,17 @@ static int snd_soc_dummy_remove(struct platform_device *pdev) return 0; }
+static struct of_device_id soc_dummy_of_match[] = { + { .compatible = "alsa-soc-dummy" }, + {}, +}; +MODULE_DEVICE_TABLE(of, soc_dummy_of_match); + static struct platform_driver soc_dummy_driver = { .driver = { .name = "snd-soc-dummy", .owner = THIS_MODULE, + .of_match_table = soc_dummy_of_match, }, .probe = snd_soc_dummy_probe, .remove = snd_soc_dummy_remove,
On 08/20/2014 09:13 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This patch adds snd-soc-dummy DT binding support.
The devicetree describes the hardware, the snd-soc-dummy is a software only thing and should not be found in the devicetree.
It removes .stream_name from dummy_dai, because "Playback" / "Capture" is very popular naming.
The DAPM will lost correct route settings if other CPU/Codec was using same stream name. And it will be problem when DPCM case. Like below
FE CPU (rsnd): "DAI0 Playback" Codec (dummy): "Playback"
BE CPU (dummy): "Playback" Codec (ak4642): "Playback"
That's something we need to fix, but I don't think removing the stream names is the right way to do this. In a multi CODEC environment you'll quite likely end up with widgets of the same name. Ideally a route endpoint would be expressed by a tuple of DT node and pin name. But I don't think it is possible to mix integer and string elements in a property.
Hi Lars
Thank you for your feedback
This patch adds snd-soc-dummy DT binding support.
The devicetree describes the hardware, the snd-soc-dummy is a software only thing and should not be found in the devicetree.
Hmm... indeed OK. I will re-consider about this
FE CPU (rsnd): "DAI0 Playback" Codec (dummy): "Playback"
BE CPU (dummy): "Playback" Codec (ak4642): "Playback"
That's something we need to fix, but I don't think removing the stream names is the right way to do this. In a multi CODEC environment you'll quite likely end up with widgets of the same name. Ideally a route endpoint would be expressed by a tuple of DT node and pin name. But I don't think it is possible to mix integer and string elements in a property.
Thank you for your advice. "DT node and name" seems nice idea, but it works on DT case only ? Anyway, I re-consider about this too. It can be trial and error...
Best regards --- Kuninori Morimoto
On Mon, Aug 25, 2014 at 05:04:20PM -0700, Kuninori Morimoto wrote:
That's something we need to fix, but I don't think removing the stream names is the right way to do this. In a multi CODEC environment you'll quite likely end up with widgets of the same name. Ideally a route endpoint would be expressed by a tuple of DT node and pin name. But I don't think it is possible to mix integer and string elements in a property.
It's not. Now that we have preprocessor support it's a lot easier to just use numbers though - the legibility problems from just using raw numbers in big tables don't apply so much any more.
Thank you for your advice. "DT node and name" seems nice idea, but it works on DT case only ? Anyway, I re-consider about this too. It can be trial and error...
I think that for hardware which has fairly monolithic audio blocks using DPCM it might be worth thinking about providing a way for the DT to look like the DT for a simple I2S DAI with the driver for the IP in the SoC filling in all the structure needed by DPCM.
Hi Mark, Lars
That's something we need to fix, but I don't think removing the stream names is the right way to do this. In a multi CODEC environment you'll quite likely end up with widgets of the same name. Ideally a route endpoint would be expressed by a tuple of DT node and pin name. But I don't think it is possible to mix integer and string elements in a property.
It's not. Now that we have preprocessor support it's a lot easier to just use numbers though - the legibility problems from just using raw numbers in big tables don't apply so much any more.
Thank you for your advice. "DT node and name" seems nice idea, but it works on DT case only ? Anyway, I re-consider about this too. It can be trial and error...
I think that for hardware which has fairly monolithic audio blocks using DPCM it might be worth thinking about providing a way for the DT to look like the DT for a simple I2S DAI with the driver for the IP in the SoC filling in all the structure needed by DPCM.
I expended route setting method like below, but what do you think ? I guess it can use not only DT case, and it is readable ?
-------------------------- Subject: [PATCH] ASoC: dapm: enable DAI name on DAPM route
DAPM route setting is using name matching. but, it can't match correctly if codec/platform driver have same name. This can be very serious issue on DAPM.
FE CPU (ec500000.rcar_sound): "DAI0 Playback" Codec (snd-soc-dummy-dai): "Playback"
BE CPU (snd-soc-dummy-dai): "Playback" Codec (ak4642-hifi): "Playback"
This patch expand route setting by using DAI name. You can select "ak4642-hifi" side "Playback" by below.
DT simple-audio-card,routing = "ak4642-hifi Playback", "DAI0 Playback";
non DT struct snd_soc_dapm_route route[] = { { "ak4642-hifi Playback", NULL, "DAI0 Playback"}, }
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/soc-dapm.c | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index b24f70a..084f15b 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2395,6 +2395,30 @@ err: return ret; }
+static void snd_soc_dapm_route_scan(struct snd_soc_dapm_widget *w, + struct snd_soc_dapm_widget **wsource, + struct snd_soc_dapm_widget **wsink, + struct snd_soc_dapm_widget **wtsource, + struct snd_soc_dapm_widget **wtsink, + struct snd_soc_dapm_context *dapm, + const char *sink, + const char *source, + const char *name) +{ + if (!*wsink && !(strcmp(name, sink))) { + *wtsink = w; + if (w->dapm == dapm) + *wsink = w; + return; + } + + if (!*wsource && !(strcmp(name, source))) { + *wtsource = w; + if (w->dapm == dapm) + *wsource = w; + } +} + static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, const struct snd_soc_dapm_route *route) { @@ -2425,17 +2449,20 @@ static int snd_soc_dapm_add_route(struct snd_soc_dapm_context *dapm, * current DAPM context */ list_for_each_entry(w, &dapm->card->widgets, list) { - if (!wsink && !(strcmp(w->name, sink))) { - wtsink = w; - if (w->dapm == dapm) - wsink = w; - continue; - } - if (!wsource && !(strcmp(w->name, source))) { - wtsource = w; - if (w->dapm == dapm) - wsource = w; + if (w->dai) { + char w_name[80]; + + snprintf(w_name, sizeof(w_name), "%s %s", + w->dai->name, w->name); + + snd_soc_dapm_route_scan(w, &wsource, &wsink, + &wtsource, &wtsink, dapm, + sink, source, w_name); } + + snd_soc_dapm_route_scan(w, &wsource, &wsink, + &wtsource, &wtsink, dapm, + sink, source, w->name); } /* use widget from another DAPM context if not found from this */ if (!wsink)
Best regards --- Kuninori Morimoto
On Tue, Aug 26, 2014 at 01:34:21AM -0700, Kuninori Morimoto wrote:
This patch expand route setting by using DAI name. You can select "ak4642-hifi" side "Playback" by below.
DT simple-audio-card,routing = "ak4642-hifi Playback", "DAI0 Playback";
non DT struct snd_soc_dapm_route route[] = { { "ak4642-hifi Playback", NULL, "DAI0 Playback"}, }
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
On 08/26/2014 11:11 AM, Mark Brown wrote:
On Tue, Aug 26, 2014 at 01:34:21AM -0700, Kuninori Morimoto wrote:
This patch expand route setting by using DAI name. You can select "ak4642-hifi" side "Playback" by below.
DT simple-audio-card,routing = "ak4642-hifi Playback", "DAI0 Playback";
non DT struct snd_soc_dapm_route route[] = { { "ak4642-hifi Playback", NULL, "DAI0 Playback"}, }
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
Yes, and I think we shouldn't use anything except for datahsheet pin names in the devicetree routing, because otherwise we are leaking driver implementation details.
On Tue, Aug 26, 2014 at 11:19:01AM +0200, Lars-Peter Clausen wrote:
On 08/26/2014 11:11 AM, Mark Brown wrote:
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
Yes, and I think we shouldn't use anything except for datahsheet pin names in the devicetree routing, because otherwise we are leaking driver implementation details.
While I agree with the sentiment for this when it comes to DAIs we probably want to use the name the interfaces get in the documentation rather than pin names since they involve multiple pins working together.
Hi Mark, Lars
I'm confusing
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
Yes, and I think we shouldn't use anything except for datahsheet pin names in the devicetree routing, because otherwise we are leaking driver implementation details.
It came from snd_soc_of_parse_audio_routing() Do you mean this function itself is not good ?
While I agree with the sentiment for this when it comes to DAIs we probably want to use the name the interfaces get in the documentation rather than pin names since they involve multiple pins working together.
Sorry, but what does your "interfaces get in the documentation" mean ?
On Tue, Aug 26, 2014 at 03:31:44AM -0700, Kuninori Morimoto wrote:
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
Yes, and I think we shouldn't use anything except for datahsheet pin names in the devicetree routing, because otherwise we are leaking driver implementation details.
It came from snd_soc_of_parse_audio_routing() Do you mean this function itself is not good ?
That's intended to be routing analogue pins to each other, not for DAI links in DPCM - for DAI links we should be getting this information from elsewhere.
While I agree with the sentiment for this when it comes to DAIs we probably want to use the name the interfaces get in the documentation rather than pin names since they involve multiple pins working together.
Sorry, but what does your "interfaces get in the documentation" mean ?
If the documentation refers to the interface as for example "I2S0" then the DT should refer to it as I2S0 too.
Hi Mark
If we're already specifying the DAI links for the (D)PCM code it seems like we shouldn't also have to put DAPM routes for them in DT as well.
Yes, and I think we shouldn't use anything except for datahsheet pin names in the devicetree routing, because otherwise we are leaking driver implementation details.
It came from snd_soc_of_parse_audio_routing() Do you mean this function itself is not good ?
That's intended to be routing analogue pins to each other, not for DAI links in DPCM - for DAI links we should be getting this information from elsewhere.
While I agree with the sentiment for this when it comes to DAIs we probably want to use the name the interfaces get in the documentation rather than pin names since they involve multiple pins working together.
Sorry, but what does your "interfaces get in the documentation" mean ?
If the documentation refers to the interface as for example "I2S0" then the DT should refer to it as I2S0 too.
Hmm... This means we need update DPCM interface ? In my understanding, DPCM needs DAPM routing information somehow in final stage. But, we want to specify it as "DAI link" like interface.
Now, I have many questions.
If my understand is correct, my prev patch is OK for "DAPM final stage", but we need wrapper for "DPCM interface" ? It will exchanges "I2S0" to "ak4642 Playback" internally. (And exchanges format too ?)
Is this needed as "DT interface" ? Can non-DT code use "ak4642 Playback" directly ? I'm wondering that some driver is using DPCM already (in non-DT) in upstream.
If we can use "I2S0" interface, it is understandable if FE/BE was
FE cpu: CPU-A codec: dummy
BE cpu: dummy codec: Codec-A
But, How about this case ?
FE cpu: CPU-A codec: Codec-A
BE cpu: CPU-B codec: Codec-B
Hi Mark again
But, How about this case ?
FE cpu: CPU-A codec: Codec-A
BE cpu: CPU-B codec: Codec-B
I found 1 method. I can create it if we can assume that "simple-card doen't support above style",
If the documentation refers to the interface as for example "I2S0" then the DT should refer to it as I2S0 too.
simple-card is using "format" property now, and I remember that someone want to exchange format in DPCM.
My 1st DPCM patch used "remote" property for specify FE/BE. And, we can get DAI stream_name if we can update snd_soc_of_get_dai_name() This means, we can use DPCM like below if you can accept my previous "ASoC: dapm: enable DAI name on DAPM route" What do you think ?
sound { compatible = "simple-audio-card";
/* FrontEnd */ simple-audio-card,dai-link@0 { ... format = "left_j"; remote = <&endpoint>;
cpu { sound-dai = <&rcar_sound 0>; }; codec { /* dummy */ }; };
/* BackEnd */ endpoint: simple-audio-card,dai-link@1 { ... format = "left_j";
cpu { /* dummy */ }; codec1: codec { sound-dai = <&ak4643>; }; }; };
Hi Mark
- are tidyup patches of simple-card
- adds DPCM support on simple-card
- tidyups asoc_simple_card_sub_parse_of() (based on 5)
- is requred DPCM on Renesas R-Car driver
- is DT support on dummy driver
(snip)
Kuninori Morimoto (8):
- ASoC: simple-card: use asoc_simple_xxx prefix
- ASoC: simple-card: remove dai_link->cpu_dai_name when DT
- ASoC: simple-card: dai_link->init should be cared when multi DAI
- ASoC: simple-card: use common for_each_child_of_node() for loop
- ASoC: simple-card: add DPCM support when DT case
- ASoC: simple-card: remove is_top_level_node from asoc_simple_card_sub_parse_of()
- ASoC: rsnd: add dai_link stream name
- ASoC: add snd-soc-dummy DT support
I will re-consider about DPCM DT support. but, can you please care about 1) - 4) only at this point ? These are bug fix patches. I will resend v2 patches for 5) - 8) And I'm very happy if I can get some feedback about 5)
Best regards --- Kuninori Morimoto
Hi Mark
- are tidyup patches of simple-card
- adds DPCM support on simple-card
- tidyups asoc_simple_card_sub_parse_of() (based on 5)
- is requred DPCM on Renesas R-Car driver
- is DT support on dummy driver
(snip)
Kuninori Morimoto (8):
- ASoC: simple-card: use asoc_simple_xxx prefix
- ASoC: simple-card: remove dai_link->cpu_dai_name when DT
- ASoC: simple-card: dai_link->init should be cared when multi DAI
- ASoC: simple-card: use common for_each_child_of_node() for loop
- ASoC: simple-card: add DPCM support when DT case
- ASoC: simple-card: remove is_top_level_node from asoc_simple_card_sub_parse_of()
- ASoC: rsnd: add dai_link stream name
- ASoC: add snd-soc-dummy DT support
I will re-consider about DPCM DT support. but, can you please care about 1) - 4) only at this point ? These are bug fix patches. I will resend v2 patches for 5) - 8) And I'm very happy if I can get some feedback about 5)
There is no response. I will re-post above 1) - 4) as v2 patch-set
Best regards --- Kuninori Morimoto
Hi Mark
These are fixup/tidyup patches for simple-card DT. Some sound card can't probe without these patches.
Kuninori Morimoto (4): ASoC: simple-card: use asoc_simple_xxx prefix ASoC: simple-card: remove dai_link->cpu_dai_name when DT ASoC: simple-card: dai_link->init should be cared when multi DAI ASoC: simple-card: use common for_each_child_of_node() for loop
sound/soc/generic/simple-card.c | 56 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 28 deletions(-)
Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
simple-card driver is using asoc_simple_xxx() prefix. simple_card_dai_link_of() should be asoc_simple_card_dai_link_of().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- no change
sound/soc/generic/simple-card.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 21b0ea2..c5445b0 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -163,11 +163,11 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return 0; }
-static int simple_card_dai_link_of(struct device_node *node, - struct device *dev, - struct snd_soc_dai_link *dai_link, - struct simple_dai_props *dai_props, - bool is_top_level_node) +static int asoc_simple_card_dai_link_of(struct device_node *node, + struct device *dev, + struct snd_soc_dai_link *dai_link, + struct simple_dai_props *dai_props, + bool is_top_level_node) { struct device_node *np = NULL; struct device_node *bitclkmaster = NULL; @@ -337,16 +337,18 @@ static int asoc_simple_card_parse_of(struct device_node *node, int i; for (i = 0; (np = of_get_next_child(node, np)); i++) { dev_dbg(dev, "\tlink %d:\n", i); - ret = simple_card_dai_link_of(np, dev, dai_link + i, - dai_props + i, false); + ret = asoc_simple_card_dai_link_of(np, dev, + dai_link + i, + dai_props + i, + false); if (ret < 0) { of_node_put(np); return ret; } } } else { - ret = simple_card_dai_link_of(node, dev, dai_link, dai_props, - true); + ret = asoc_simple_card_dai_link_of(node, dev, + dai_link, dai_props, true); if (ret < 0) return ret; }
On Wed, Aug 27, 2014 at 08:07:46PM -0700, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
simple-card driver is using asoc_simple_xxx() prefix. simple_card_dai_link_of() should be asoc_simple_card_dai_link_of().
Applied, thanks.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
f687d900d30a61dda38db2a99239f5284a86a309 (ASoC: simple-card: cpu_dai_name creates confusion when DT case) removed dai_link->cpu_dai_name when DT case, since it uses DT phand in soc_bind_dai_link(). This binding will fail if it has cpu_dai_name.
6a91a17bd7b92b2d2aa9ece85457f52a62fd7708 (ASoC: simple-card: Handle many DAI links) added multi DAI link support to simple-card driver. Then, removing cpu_dai_name was cared only single DAI. But, it is needed in all DT cases. This patch moves it to asoc_simple_card_dai_link_of() so that care about all DAIs.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- no change
sound/soc/generic/simple-card.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index c5445b0..e8185a0 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -285,6 +285,17 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_props->codec_dai.fmt, dai_props->codec_dai.sysclk);
+ /* + * soc_bind_dai_link() will check cpu name + * after of_node matching if dai_link has cpu_dai_name. + * but, it will never match if name was created by fmt_single_name() + * remove cpu_dai_name to escape name matching. + * see + * fmt_single_name() + * fmt_multiple_name() + */ + dai_link->cpu_dai_name = NULL; + dai_link_of_err: if (np) of_node_put(np); @@ -429,18 +440,6 @@ static int asoc_simple_card_probe(struct platform_device *pdev) goto err; }
- /* - * soc_bind_dai_link() will check cpu name - * after of_node matching if dai_link has cpu_dai_name. - * but, it will never match if name was created by fmt_single_name() - * remove cpu_dai_name to escape name matching. - * see - * fmt_single_name() - * fmt_multiple_name() - */ - if (num_links == 1) - dai_link->cpu_dai_name = NULL; - } else { struct asoc_simple_card_info *cinfo;
On Wed, Aug 27, 2014 at 08:08:06PM -0700, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
f687d900d30a61dda38db2a99239f5284a86a309 (ASoC: simple-card: cpu_dai_name creates confusion when DT case) removed dai_link->cpu_dai_name when DT case, since it uses DT phand in soc_bind_dai_link(). This binding will fail if it has cpu_dai_name.
Applied, thanks.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
6a91a17bd7b92b2d2aa9ece85457f52a62fd7708 (ASoC: simple-card: Handle many DAI links) added multi DAI support on simple-card. This means priv->dai_link might be pointer of multi DAI. dai_link->init is needed for all DAI. This patch cares it for all DAIs on DT/non-DT
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- no change
sound/soc/generic/simple-card.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index e8185a0..8902704 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -274,6 +274,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_link->codec_dai_name); dai_link->name = dai_link->stream_name = name; dai_link->ops = &asoc_simple_card_ops; + dai_link->init = asoc_simple_card_dai_init;
dev_dbg(dev, "\tname : %s\n", dai_link->stream_name); dev_dbg(dev, "\tcpu : %s / %04x / %d\n", @@ -465,6 +466,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev) dai_link->codec_name = cinfo->codec; dai_link->cpu_dai_name = cinfo->cpu_dai.name; dai_link->codec_dai_name = cinfo->codec_dai.name; + dai_link->init = asoc_simple_card_dai_init; memcpy(&priv->dai_props->cpu_dai, &cinfo->cpu_dai, sizeof(priv->dai_props->cpu_dai)); memcpy(&priv->dai_props->codec_dai, &cinfo->codec_dai, @@ -474,11 +476,6 @@ static int asoc_simple_card_probe(struct platform_device *pdev) priv->dai_props->codec_dai.fmt |= cinfo->daifmt; }
- /* - * init snd_soc_dai_link - */ - dai_link->init = asoc_simple_card_dai_init; - snd_soc_card_set_drvdata(&priv->snd_card, priv);
ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
On Wed, Aug 27, 2014 at 08:08:27PM -0700, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
6a91a17bd7b92b2d2aa9ece85457f52a62fd7708 (ASoC: simple-card: Handle many DAI links) added multi DAI support on simple-card.
Applied, thanks.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- no change
sound/soc/generic/simple-card.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 8902704..fd8b045 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -346,8 +346,9 @@ static int asoc_simple_card_parse_of(struct device_node *node,
if (multi) { struct device_node *np = NULL; - int i; - for (i = 0; (np = of_get_next_child(node, np)); i++) { + int i = 0; + + for_each_child_of_node(node, np) { dev_dbg(dev, "\tlink %d:\n", i); ret = asoc_simple_card_dai_link_of(np, dev, dai_link + i, @@ -357,6 +358,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, of_node_put(np); return ret; } + i++; } } else { ret = asoc_simple_card_dai_link_of(node, dev,
On Wed, Aug 27, 2014 at 08:08:47PM -0700, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
applied, thanks.
participants (4)
-
Kuninori Morimoto -
Kuninori Morimoto -
Lars-Peter Clausen -
Mark Brown