[alsa-devel] [PATCH] ASoC: generic: add generic compound card with DT support
Some audio cards are built from different hardware components. When such compound cards don't need specific code, this driver creates them with the required DAI links and routes from a DT.
Signed-off-by: Jean-Francois Moine moinejf@free.fr --- This code was first developped on the generic simple card, but its recent DT extension cannot be easily extended again to support compound cards as the one in the Cubox. Note also that the example relies on a proposed patch of mine aiming to render the codec name / OF node optional in DAI links (http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070082.ht...). --- .../devicetree/bindings/sound/compound-card.txt | 95 ++++++++++++ sound/soc/generic/Kconfig | 6 + sound/soc/generic/Makefile | 2 + sound/soc/generic/compound-card.c | 247 +++++++++++++++++++++++++ 4 file changed, 350 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/compound-card.txt b/Documentation/devicetree/bindings/sound/compound-card.txt new file mode 100644 index 0000000..554a796 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/compound-card.txt @@ -0,0 +1,95 @@ +Device-Tree bindings for compound audio card + +Compound audio card describes the links between the different parts +of an audio card built from different hardware components. + +Required properties: + - compatible: should be "compound-audio-card" + - audio-controller: phandle of the audio controller + +Optional properties: + - routes: list of couple of strings (sink, source) + +Required subnodes: + - link: DAI link subnode + At least one link must be specified. + +Required link subnode properties: + - link-name: names of the DAI link and of the stream + - cpu-dai-name: name of the CPU or CODEC DAI + An empty string indicates that the CPU DAI is + the same as the audio controller. + - codec-dai-name: name of the CODEC DAI + +Optional link subnode properties: + - audio-codec or codec-name: phandle or name of the CODEC + in case the codec-dai-name is not unique + - format: DAI format. One of: + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" + - front-end or back-end: present if the DAI link describes resp. + a front-end CPU DAI or a back-end CODEC DAI + - playback or capture: present if the DAI link is used for + playback or capture only + +Example node: + +The audio subsystem found in the SolidRun Cubox is composed of: + +- an audio controller which outputs playback streams on either one or both + of I2S and S/PDIF, + +- a HDMI transmitter which can get its input from either I2S or S/PDIF, + +- an optical S/PDIF connector. + + sound { + compatible = "compound-audio-card"; + audio-controller = <&audio1>; + routes = + "I2S Playback", "System Playback", + "SPDIF Playback", "System Playback", + "hdmi-out", "I2S Playback", + "hdmi-out", "SPDIF Playback", + "spdif-out", "SPDIF Playback"; + link@0 { + link-name = "System audio"; + front-end; + cpu-dai-name = "mvebu-audio"; + codec-dai-name = "snd-soc-dummy-dai"; + format = "i2s"; + playback; + }; + link@1 { + link-name = "hdmi-i2s"; + back-end; + cpu-dai-name = "i2s"; + codec-dai-name = "i2s-hifi"; + playback; + }; + link@2 { + link-name = "hdmi-spdif"; + back-end; + cpu-dai-name = "spdif"; + codec-dai-name = "spdif-hifi"; + playback; + }; + link@3 { + link-name = "spdif"; + back-end; + cpu-dai-name = "spdif"; + codec-dai-name = "dit-hifi"; + playback; + }; + }; + + hdmi_codec: hdmi-codec { + compatible = "nxp,tda998x-codec"; + audio-ports = <0x03>, <0x04>; + }; + + spdif_codec: spdif-codec { + compatible = "linux,spdif-dit"; + }; + + diff --git a/sound/soc/generic/Kconfig b/sound/soc/generic/Kconfig index 610f612..f2678ae 100644 --- a/sound/soc/generic/Kconfig +++ b/sound/soc/generic/Kconfig @@ -2,3 +2,9 @@ config SND_SIMPLE_CARD tristate "ASoC Simple sound card support" help This option enables generic simple sound card support + +config SND_COMPOUND_CARD + tristate "ASoC Compound sound card support" + depends on OF + help + This option enables the generic compound sound card support. diff --git a/sound/soc/generic/Makefile b/sound/soc/generic/Makefile index 9c3b246..434cb79 100644 --- a/sound/soc/generic/Makefile +++ b/sound/soc/generic/Makefile @@ -1,3 +1,5 @@ snd-soc-simple-card-objs := simple-card.o +snd-soc-compound-card-objs := compound-card.o
obj-$(CONFIG_SND_SIMPLE_CARD) += snd-soc-simple-card.o +obj-$(CONFIG_SND_COMPOUND_CARD) += snd-soc-compound-card.o diff --git a/sound/soc/generic/compound-card.c b/sound/soc/generic/compound-card.c new file mode 100644 index 0000000..f57e159 --- /dev/null +++ b/sound/soc/generic/compound-card.c @@ -0,0 +1,247 @@ +/* + * ASoC compound sound card support + * + * Copyright (C) 2013 Jean-Francois Moine moinejf@free.fr + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/platform_device.h> +#include <linux/module.h> +#include <linux/of.h> +#include <sound/soc.h> + +// link types +#define LINK_FE 0 /* front-end */ +#define LINK_BE 1 /* back-end */ +#define LINK_CC 2 /* (controller or codec) to codec */ + +struct compound_card_data { + struct snd_soc_card snd_card; + struct snd_soc_dai_link *snd_links; +}; + +/* get a DAI link */ +static int get_link(struct compound_card_data *priv, + struct snd_soc_dai_link *link, + struct device_node *ctrl_np, + struct device_node *np) +{ + struct device_node *codec_np; + const char *cpu_dai_name; + int type, ret; + + /* optional link-name */ + of_property_read_string(np, "link-name", &link->name); + link->stream_name = link->name; + + /* link type */ + if (of_property_read_bool(np, "front-end")) + type = LINK_FE; + else if (of_property_read_bool(np, "back-end")) + type = LINK_BE; + else + type = LINK_CC; + + /* cpu-dai-name */ + ret = of_property_read_string(np, "cpu-dai-name", + &cpu_dai_name); + if (ret) { + dev_err(priv->snd_card.dev, + "No valid cpu-dai-name found\n"); + return ret; + } + + /* an empty cpu-dai-name forces the audio-controller */ + if (cpu_dai_name[0] == '\0') + link->cpu_of_node = ctrl_np; + else + link->cpu_dai_name = cpu_dai_name; + + /* the codec may be omitted or specified by name or phandle */ + codec_np = of_parse_phandle(np, "audio-codec", 0); + if (codec_np) + link->codec_of_node = codec_np; + else + of_property_read_string(np, "codec-name", + &link->codec_name); + + /* codec-dai-name */ + ret = of_property_read_string(np, "codec-dai-name", + &link->codec_dai_name); + if (ret) { + dev_err(priv->snd_card.dev, + "Failed to parse codec-dai-name\n"); + return ret; + } + + if (type == LINK_BE) { + link->no_pcm = 1; + /* don't set the platform, otherwise, the platform functions + * are called many times */ + } else { + link->platform_of_node = ctrl_np; + if (type == LINK_FE) + link->dynamic = 1; + } + + /* DAI format */ + link->dai_fmt = snd_soc_of_parse_daifmt(np, NULL) & + SND_SOC_DAIFMT_FORMAT_MASK; + + if (of_property_read_bool(np, "playback")) + link->playback_only = 1; + else if (of_property_read_bool(np, "capture")) + link->capture_only = 1; + + return 0; +} + +static int compound_card_dt_probe(struct platform_device *pdev, + struct compound_card_data *priv) +{ + struct device_node *np, *ctrl_np, *link_np; + struct snd_soc_dai_link *link; + int ret, num_links; + + np = pdev->dev.of_node; + + ctrl_np = of_parse_phandle(np, "audio-controller", 0); + if (!ctrl_np) { + dev_err(&pdev->dev, "No valid audio-controller phandle found\n"); + return -EINVAL; + } + + /* get the optional routes */ + if (of_property_count_strings(np, "routes")) + snd_soc_of_parse_audio_routing(&priv->snd_card, "routes"); + + priv->snd_card.name = "ASoC compound sound card"; + + /* check the number of links */ + num_links = 0; + for_each_child_of_node(np, link_np) + num_links++; + + if (num_links == 0) { + dev_err(&pdev->dev, "No DAI link\n"); + return -EINVAL; + } + + link = devm_kzalloc(&pdev->dev, + sizeof(struct snd_soc_dai_link) * num_links, + GFP_KERNEL); + if (!link) { + ret = -ENOMEM; + goto err; + } + priv->snd_links = link; + priv->snd_card.num_links = num_links; + for_each_child_of_node(np, link_np) { + ret = get_link(priv, link, ctrl_np, link_np); + if (ret) + goto err; + link++; + } + + return 0; + +err: + if (link_np) + of_node_put(link_np); + if (ctrl_np) + of_node_put(ctrl_np); + for (num_links = 0, link = priv->snd_links; + num_links < priv->snd_card.num_links; + num_links++, link++) { + struct device_node *codec_np; + + codec_np = (struct device_node *) link->codec_of_node; + if (!codec_np) + break; + of_node_put(codec_np); + } + return ret; +} + +static int compound_card_dt_remove(struct platform_device *pdev, + struct compound_card_data *priv) +{ + struct device_node *codec_np; + struct snd_soc_dai_link *link; + int num_links; + + if (!priv->snd_links) + return 0; + if (priv->snd_links->platform_of_node) + of_node_put((struct device_node *) + priv->snd_links->platform_of_node); + for (num_links = 0, link = priv->snd_links; + num_links < priv->snd_card.num_links; + num_links++, link++) { + codec_np = (struct device_node *) link->codec_of_node; + if (!codec_np) + break; + of_node_put(codec_np); + } + return 0; +} + +static int compound_card_probe(struct platform_device *pdev) +{ + struct compound_card_data *priv; + int ret; + + priv = devm_kzalloc(&pdev->dev, sizeof *priv, GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->snd_card.owner = THIS_MODULE; + priv->snd_card.dev = &pdev->dev; + + ret = compound_card_dt_probe(pdev, priv); + + if (ret) + return ret; + + priv->snd_card.dai_link = priv->snd_links; + + dev_set_drvdata(&pdev->dev, priv); + + return snd_soc_register_card(&priv->snd_card); +} + +static int compound_card_remove(struct platform_device *pdev) +{ + struct compound_card_data *priv = dev_get_drvdata(&pdev->dev); + + snd_soc_unregister_card(&priv->snd_card); + + compound_card_dt_remove(pdev, priv); + + return 0; +} + +static const struct of_device_id compound_card_of_dev_ids[] = { + { .compatible = "compound-audio-card", }, + {} +}; +MODULE_DEVICE_TABLE(of, compound_card_of_dev_ids); + +static struct platform_driver compound_card = { + .driver = { + .name = "asoc-compound-card", + .owner = THIS_MODULE, + .of_match_table = compound_card_of_dev_ids, + }, + .probe = compound_card_probe, + .remove = compound_card_remove, +}; + +module_platform_driver(compound_card); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("ASoC Compound Sound Card"); +MODULE_AUTHOR("Jean-Francois Moine moinejf@free.fr");
On Tue, Dec 31, 2013 at 11:31:38AM +0100, Jean-Francois Moine wrote:
This code was first developped on the generic simple card, but its recent DT extension cannot be easily extended again to support compound cards as the one in the Cubox.
It would have been useful to have provided that feedback at the time rather than waiting until after it had been merged - it was in review for long enough. It would also be good to articulate the issues with the binding rather than simply stating they exist, and to consider adding a second binding to the existing generic card rather than adding a totally new card.
Please also remember that all DT patches need to be reviewed by the DT people, you've not CCed either them or the list.
- front-end or back-end: present if the DAI link describes resp.
a front-end CPU DAI or a back-end CODEC DAI
These are Linux-internal concepts which shouldn't appear in a DT binding or at the very least need definition. One thing to consider here is that these things are all about the internals of a SoC and you'd therefore expect that they would be defined separately from the card so as to avoid having to replicate information in every card using a given SoC.
On Tue, 31 Dec 2013 11:59:27 +0000 Mark Brown broonie@kernel.org wrote:
This code was first developped on the generic simple card, but its recent DT extension cannot be easily extended again to support compound cards as the one in the Cubox.
It would have been useful to have provided that feedback at the time rather than waiting until after it had been merged - it was in review for long enough. It would also be good to articulate the issues with the binding rather than simply stating they exist, and to consider adding a second binding to the existing generic card rather than adding a totally new card.
Sorry, I spent a lot of time on DPCM, and I was not yet ready to propose something when you accepted Kuninori's patch.
Please also remember that all DT patches need to be reviewed by the DT people, you've not CCed either them or the list.
- front-end or back-end: present if the DAI link describes resp.
a front-end CPU DAI or a back-end CODEC DAIThese are Linux-internal concepts which shouldn't appear in a DT binding or at the very least need definition. One thing to consider here is that these things are all about the internals of a SoC and you'd therefore expect that they would be defined separately from the card so as to avoid having to replicate information in every card using a given SoC.
Do you mean that, as DPCM cannot be in the DT, there should be a specific driver for the Cubox audio card (Marvell Armada 510 + NXP HDMI transmitter)?
On Tue, Dec 31, 2013 at 01:36:10PM +0100, Jean-Francois Moine wrote:
Mark Brown broonie@kernel.org wrote:
It would have been useful to have provided that feedback at the time rather than waiting until after it had been merged - it was in review for long enough. It would also be good to articulate the issues with
Sorry, I spent a lot of time on DPCM, and I was not yet ready to propose something when you accepted Kuninori's patch.
It'd still have been worthwhile to say that you weren't sure it'd work even if you didn't have an alternative; you could perhaps have worked on it together.
These are Linux-internal concepts which shouldn't appear in a DT binding or at the very least need definition. One thing to consider here is that these things are all about the internals of a SoC and you'd therefore expect that they would be defined separately from the card so as to avoid having to replicate information in every card using a given SoC.
Do you mean that, as DPCM cannot be in the DT, there should be a specific driver for the Cubox audio card (Marvell Armada 510 + NXP HDMI transmitter)?
I'm not saying it can't be in the DT, I'm saying that this doesn't look like the right way to do things. For example we could have a way of specifying parts of the card separately so the SoC can be referenced as a whole by the card, or we could have the internals behind the DAI hidden from the DT entirely so the DT just links the DAIs together and the SoC internals come from the implementation of the DAI devices.
If you can't figure something out a more specific binding is fine, but if you're trying to do a generic binding for everything then these things need to be considered.
On 12/31/2013 11:31 AM, Jean-Francois Moine wrote:
Some audio cards are built from different hardware components. When such compound cards don't need specific code, this driver creates them with the required DAI links and routes from a DT.
Signed-off-by: Jean-Francois Moine moinejf@free.fr
This code was first developped on the generic simple card, but its recent DT extension cannot be easily extended again to support compound cards as the one in the Cubox. Note also that the example relies on a proposed patch of mine aiming to render the codec name / OF node optional in DAI links (http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070082.ht...).
.../devicetree/bindings/sound/compound-card.txt | 95 ++++++++++++ sound/soc/generic/Kconfig | 6 + sound/soc/generic/Makefile | 2 + sound/soc/generic/compound-card.c | 247 +++++++++++++++++++++++++ 4 file changed, 350 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/compound-card.txt b/Documentation/devicetree/bindings/sound/compound-card.txt new file mode 100644 index 0000000..554a796 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/compound-card.txt @@ -0,0 +1,95 @@ +Device-Tree bindings for compound audio card
+Compound audio card describes the links between the different parts +of an audio card built from different hardware components.
+Required properties:
- compatible: should be "compound-audio-card"
- audio-controller: phandle of the audio controller
+Optional properties:
- routes: list of couple of strings (sink, source)
+Required subnodes:
- link: DAI link subnode
- At least one link must be specified.
+Required link subnode properties:
- link-name: names of the DAI link and of the stream
- cpu-dai-name: name of the CPU or CODEC DAI
An empty string indicates that the CPU DAI isthe same as the audio controller.
- codec-dai-name: name of the CODEC DAI
+Optional link subnode properties:
- audio-codec or codec-name: phandle or name of the CODEC
in case the codec-dai-name is not unique
- format: DAI format. One of:
"i2s", "right_j", "left_j" , "dsp_a""dsp_b", "ac97", "pdm", "msb", "lsb"
- front-end or back-end: present if the DAI link describes resp.
a front-end CPU DAI or a back-end CODEC DAI
- playback or capture: present if the DAI link is used for
playback or capture only
As Mark also said, this binding definitely leaks way too much internals of the current ASoC implementation. In my opinion the way forward for ASoC is to stop to distinguish between different types of components. This is on one hand CODECS and CPU-DAIs and on the other hand also front-end and beck-end DAIs. The first steps in this direction have already been take by the start of the component-fication, but its still a long way to go. Exposing those concepts via the devicetree will only make it harder to get rid of them later. The bindings for a compound card should essentially describe which components are involved and how the fabric between and around them looks like. If the type of the component is needed in the ASoC implementation it should be possible to auto-discover it. Also I think we want to align the devicetree bindings with what the media people have been doing[1]. Audio and video are not that different in this regard and there will also be boards where the audio and video fabric will be intermingled (e.g. like on your board with HDMI).
- Lars
On Wed, 01 Jan 2014 20:05:05 +0100 Lars-Peter Clausen lars@metafoo.de wrote:
As Mark also said, this binding definitely leaks way too much internals of the current ASoC implementation. In my opinion the way forward for ASoC is to stop to distinguish between different types of components. This is on one hand CODECS and CPU-DAIs and on the other hand also front-end and beck-end DAIs. The first steps in this direction have already been take by the start of the component-fication, but its still a long way to go. Exposing those concepts via the devicetree will only make it harder to get rid of them later. The bindings for a compound card should essentially describe which components are involved and how the fabric between and around them looks like. If the type of the component is needed in the ASoC implementation it should be possible to auto-discover it. Also I think we want to align the devicetree bindings with what the media people have been doing[1].
(you forgot the [1] reference)
Audio and video are not that different in this regard and there will also be boards where the audio and video fabric will be intermingled (e.g. like on your board with HDMI).
I found a way to discover the DAI link types for my system: when simple DAPM, the kirkwood CPU DAIs have an ID != 0. For the Cubox, the CPU DAI of the first link (system playback) has the ID 0, so I can move to DPCM setting the 'dynamic' and 'no_pcm' flags in the DAI links at snd platform probe time.
Then, after some extensions of the simple-card, the DT would look like (not tested yet):
sound { compatible = "simple-audio-card"; simple-audio-routing = "HDMI I2S Playback", "System Playback", "HDMI SPDIF Playback", "System Playback", "SPDIF Playback", "System Playback",
"hdmi-out-i2s", "HDMI I2S Playback", "hdmi-out-spdif", "HDMI SPDIF Playback", "spdif-out", "SPDIF Playback";
simple-audio-card,cpu@0 { link-name = "System audio"; /* extension */ sound-dai = <&audio1 0>; playback; /* extension */ format = "i2s"; }; simple-audio-card,codec@0 { sound-dai-name = "snd-soc-dummy-dai"; };
/* multi-links extension */ simple-audio-card,cpu@1 { link-name = "hdmi-i2s"; platform-name = "snd-soc-dummy"; /* extension */ sound-dai = <&audio1 1>; playback; }; simple-audio-card,codec@1 { sound-dai = <&hdmi_codec 0>; };
simple-audio-card,cpu@2 { link-name = "hdmi-spdif"; platform-name = "snd-soc-dummy"; sound-dai = <&audio1 2>; playback; }; simple-audio-card,codec@2 { sound-dai = <&hdmi_codec 1>; };
simple-audio-card,cpu@3 { link-name = "spdif"; platform-name = "snd-soc-dummy"; sound-dai = <&audio1 1>; playback; }; simple-audio-card,codec@3 { sound-dai = <&spdif_codec>; }; };
May I go to this direction?
On 01/01/2014 09:08 PM, Jean-Francois Moine wrote:
On Wed, 01 Jan 2014 20:05:05 +0100 Lars-Peter Clausen lars@metafoo.de wrote:
As Mark also said, this binding definitely leaks way too much internals of the current ASoC implementation. In my opinion the way forward for ASoC is to stop to distinguish between different types of components. This is on one hand CODECS and CPU-DAIs and on the other hand also front-end and beck-end DAIs. The first steps in this direction have already been take by the start of the component-fication, but its still a long way to go. Exposing those concepts via the devicetree will only make it harder to get rid of them later. The bindings for a compound card should essentially describe which components are involved and how the fabric between and around them looks like. If the type of the component is needed in the ASoC implementation it should be possible to auto-discover it. Also I think we want to align the devicetree bindings with what the media people have been doing[1].
(you forgot the [1] reference)
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/media...
On Wed, 01 Jan 2014 21:11:23 +0100 Lars-Peter Clausen lars@metafoo.de wrote:
On 01/01/2014 09:08 PM, Jean-Francois Moine wrote:
On Wed, 01 Jan 2014 20:05:05 +0100 Lars-Peter Clausen lars@metafoo.de wrote:
As Mark also said, this binding definitely leaks way too much internals of the current ASoC implementation. In my opinion the way forward for ASoC is to stop to distinguish between different types of components. This is on one hand CODECS and CPU-DAIs and on the other hand also front-end and beck-end DAIs. The first steps in this direction have already been take by the start of the component-fication, but its still a long way to go. Exposing those concepts via the devicetree will only make it harder to get rid of them later. The bindings for a compound card should essentially describe which components are involved and how the fabric between and around them looks like. If the type of the component is needed in the ASoC implementation it should be possible to auto-discover it. Also I think we want to align the devicetree bindings with what the media people have been doing[1].
(you forgot the [1] reference)
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/media...
I see the idea. So here is a proposal of DT for the Cubox (audio + video) which should work without too many difficulties.
/* video / audio transmitter */
tda998x: hdmi-encoder { compatible = "nxp,tda998x"; ...
/* video */ port@0 { tda998x_0: endpoint { reg = <0x230145>; remote-endpoint = <&lcd0_0>; }; };
/* audio */ port@1 { hdmi_i2s_audio: endpoint@0 { reg = <0x03>; remote-endpoint = <&audio_hdmi_i2s>; }; hdmi_spdif_audio: endpoint@1 { reg = <0x04>; remote-endpoint = <&audio_hdmi_spdif>; }; }; };
toslink: spdif { compatible = "linux,spdif-dit"; port { spdif_audio: endpoint { remote-endpoint = <&audio_spdif>; }; }; };
/* video */
lcd0: video-controller { compatible = "marvell,dove-lcd"; ... port { lcd0_0: endpoint { remote-endpoint = <&tda998x_0>; }; }; };
video { compatible = "media-video"; video-controller = <&lcd0>; };
/* audio */
audio1: audio-controller { compatible = "marvell,dove-audio"; ... port@0 { audio_hdmi_i2s: endpoint { remote-endpoint = <&hdmi_i2s_audio>; }; };
port@1 { audio_hdmi_spdif: endpoint@0 { remote-endpoint = <&hdmi_spdif_audio>; }; audio_spdif: endpoint@1 { remote-endpoint = <&spdif_audio>; }; }; };
sound { compatible = "media-audio"; audio-controller = <&audio1>; };
On Thu, Jan 02, 2014 at 10:26:47AM +0100, Jean-Francois Moine wrote:
/* audio */ port@1 { hdmi_i2s_audio: endpoint@0 { reg = <0x03>; remote-endpoint = <&audio_hdmi_i2s>; };
I think we want an explicit object in the card representing the DAIs. This will both be useful for making it easy to find the configuration for the link and will be more extensible for the cases where multiple devices are connected, you can't just assume there's just two.
On Thu, 2 Jan 2014 11:10:56 +0000 Mark Brown broonie@kernel.org wrote:
On Thu, Jan 02, 2014 at 10:26:47AM +0100, Jean-Francois Moine wrote:
/* audio */ port@1 { hdmi_i2s_audio: endpoint@0 { reg = <0x03>; remote-endpoint = <&audio_hdmi_i2s>; };I think we want an explicit object in the card representing the DAIs. This will both be useful for making it easy to find the configuration for the link and will be more extensible for the cases where multiple devices are connected, you can't just assume there's just two.
I don't see the problem: the 'port' is the DAI. The associated endpoints give the DAI links and the routing information.
As the DT definition has been done for video, some properties may be added at will for audio.
What kind of object were you thinking of?
On Thu, Jan 02, 2014 at 12:43:31PM +0100, Jean-Francois Moine wrote:
Mark Brown broonie@kernel.org wrote:
I think we want an explicit object in the card representing the DAIs. This will both be useful for making it easy to find the configuration for the link and will be more extensible for the cases where multiple devices are connected, you can't just assume there's just two.
I don't see the problem: the 'port' is the DAI. The associated endpoints give the DAI links and the routing information.
As the DT definition has been done for video, some properties may be added at will for audio.
What kind of object were you thinking of?
Like I say multiple devices on the same link - if you're just listing a single remote device there can't be more than one.
On Thu, 2 Jan 2014 11:56:18 +0000 Mark Brown broonie@kernel.org wrote:
On Thu, Jan 02, 2014 at 12:43:31PM +0100, Jean-Francois Moine wrote:
Mark Brown broonie@kernel.org wrote:
I think we want an explicit object in the card representing the DAIs. This will both be useful for making it easy to find the configuration for the link and will be more extensible for the cases where multiple devices are connected, you can't just assume there's just two.
I don't see the problem: the 'port' is the DAI. The associated endpoints give the DAI links and the routing information.
As the DT definition has been done for video, some properties may be added at will for audio.
What kind of object were you thinking of?
Like I say multiple devices on the same link - if you're just listing a single remote device there can't be more than one.
I still don't understand. There is already such cases in the Cubox: the S/PDIF output from the kirkwood audio controller is connected to both the HDMI transmitter and the S/PDIF TOSLINK. So, in the audio controller, the port @1 defines the S/PDIF DAI and the endpoints @0 and @1 point to the remote DAIs, creating 2 snd DAI links:
port@1 { audio_hdmi_spdif: endpoint@0 { remote-endpoint = <&hdmi_spdif_audio>; }; audio_spdif: endpoint@1 { remote-endpoint = <&spdif_audio>; }; };
in the snd card:
- DAI link 1 = 'audio controller spdif out' <=> 'hdmi spdif' - DAI link 2 = 'audio controller spdif out' <=> 'spdif'
If I am wrong, may you give us an example for which such a DT would not work?
On Thu, Jan 02, 2014 at 01:44:37PM +0100, Jean-Francois Moine wrote:
I still don't understand. There is already such cases in the Cubox: the S/PDIF output from the kirkwood audio controller is connected to both the HDMI transmitter and the S/PDIF TOSLINK. So, in the audio controller, the port @1 defines the S/PDIF DAI and the endpoints @0 and @1 point to the remote DAIs, creating 2 snd DAI links:
port@1 { audio_hdmi_spdif: endpoint@0 { remote-endpoint = <&hdmi_spdif_audio>; }; audio_spdif: endpoint@1 { remote-endpoint = <&spdif_audio>; }; };
Oh, so the endpoints are virtual and that's supposed to be three things wired together rather than a single device with multiple links? That's really not very clear from reading the above and seems cumbersome - every device will want to explicitly identify every other device on the link and any configuration is going to either need to be replicated on every device or we'll need to check lots of places for the configuation. It seems like this will be hard to work with.
On Thu, 2 Jan 2014 13:10:45 +0000 Mark Brown broonie@kernel.org wrote:
On Thu, Jan 02, 2014 at 01:44:37PM +0100, Jean-Francois Moine wrote:
I still don't understand. There is already such cases in the Cubox: the S/PDIF output from the kirkwood audio controller is connected to both the HDMI transmitter and the S/PDIF TOSLINK. So, in the audio controller, the port @1 defines the S/PDIF DAI and the endpoints @0 and @1 point to the remote DAIs, creating 2 snd DAI links:
port@1 { audio_hdmi_spdif: endpoint@0 { remote-endpoint = <&hdmi_spdif_audio>; }; audio_spdif: endpoint@1 { remote-endpoint = <&spdif_audio>; }; };
Oh, so the endpoints are virtual and that's supposed to be three things wired together rather than a single device with multiple links? That's really not very clear from reading the above and seems cumbersome - every device will want to explicitly identify every other device on the link and any configuration is going to either need to be replicated on every device or we'll need to check lots of places for the configuation. It seems like this will be hard to work with.
No, the 'endpoint' <=> 'remote-endpoint' is a point to point relation. Even if the sources and sinks are not explicitly defined, the way the stream flows is easy to find: the main source is always in the 'audio-controller' node
sound { compatible = "media-audio"; audio-controller = <&audio1>; };
and, then, the controller ports are sources (CPU DAIs) and the associated remote ports are sinks (CODEC DAIs).
With many levels, once the remote (sink) ports are identified, in the devices where such sinks exist, the remaining ports are sources.
Usually, the devices don't have to know to which other device they are connected, and, yes, the reverse pointer sink to source is not useful.
But the way (link) the audio stream comes from may be important to know. This is the case for the HDMI CODEC which must tell the HDMI transmitter from which hardware port(s) ('reg') it may get the audio stream. That's why, the HDMI encoder has two endpoints in its audio port, each endpoint being a different CODEC DAI.
On Thu, Jan 02, 2014 at 06:50:55PM +0100, Jean-Francois Moine wrote:
No, the 'endpoint' <=> 'remote-endpoint' is a point to point relation. Even if the sources and sinks are not explicitly defined, the way the stream flows is easy to find: the main source is always in the 'audio-controller' node
But the links in question aren't point to point links and may be bidirectional...
Usually, the devices don't have to know to which other device they are connected, and, yes, the reverse pointer sink to source is not useful.
This may be the case for the systems you've looked at but other designs are quite common.
But the way (link) the audio stream comes from may be important to know. This is the case for the HDMI CODEC which must tell the HDMI transmitter from which hardware port(s) ('reg') it may get the audio stream. That's why, the HDMI encoder has two endpoints in its audio port, each endpoint being a different CODEC DAI.
Obviously if there are multiple DAIs on a device then it needs to be possible to represent them separately but that seems orthogonal to the rest of the discussion (and resolved already)?
participants (3)
-
Jean-Francois Moine -
Lars-Peter Clausen -
Mark Brown