[alsa-devel] Question about OF-graph ports
Hi Rob, Mark and ALSA SoC ML
I'm working for OF-graph base sound card support. Now I want to know 1 things which can't solve by myself. It is OF-graph ports
ALSA SoC needs CPU, Codec, and Sound Card. If CPU <-> Card <-> Codec case, OF-graph will be
card { ports { port@0 { card_in: endpoint { remote-endpoint = <&cpu_out>; }; }; port@1 { card_out: endpoint { remote-endpoint = <&codec_in>; }; }; }; };
cpu { port { cpu_out: endpoint { remote-endpoint = <&card_in>; }; }; };
codec { port { codec_in: endpoint { remote-endpoint = <&card_out>; }; }; };
This is OK. If CPU has many ports, then ALSA SoC requests 1 Card which has many DAIs (= Digital Audio Interface). This case, cpu will have "ports" and many "port", this is OK.
SoC.0 Codec0 SoC.1 <-> Card <-> Codec1 SoC.2 Codec2
In "card", CPU/Codec pair is indicated by "ports" now (= port0 is CPU, port1 is Codec, etc)
My question is in this case, how to know CPU/Codec pair on "card" ? Can we have *multi ports*, like this ?
card { ports { ports@0 { port@0 { /* SoC.0 */ }; port@1 { /* Codec0*/ }; }; ports@1 { port@0 { /* SoC.1 */ }; port@1 { /* Codec1*/ }; }; ports@2 { port@0 { /* SoC.2 */ }; port@1 { /* Codec2*/ }; }; }; };
Or can I use grouping, like this ?
card { group = <port@0 port@1>, <port@2 port@3>, <port@4 port@5>; ports { port@0 { /* SoC.0 */ }; port@1 { /* SoC.1 */ }; port@2 { /* SoC.2 */ }; port@3 { /* Codec0*/ }; port@4 { /* Codec1*/ }; port@5 { /* Codec2*/ }; }; };
Best regards --- Kuninori Morimoto
On Thu, Jan 19, 2017 at 06:07:43AM +0000, Kuninori Morimoto wrote:
My question is in this case, how to know CPU/Codec pair on "card" ?
This is more of a DT question but, I don't think I have that strong an opinion based on the snippets you posted, they both seem readable.
On Thu, Jan 19, 2017 at 12:07 AM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Rob, Mark and ALSA SoC ML
I'm working for OF-graph base sound card support. Now I want to know 1 things which can't solve by myself. It is OF-graph ports
ALSA SoC needs CPU, Codec, and Sound Card. If CPU <-> Card <-> Codec case, OF-graph will be
card { ports { port@0 { card_in: endpoint { remote-endpoint = <&cpu_out>; }; }; port@1 { card_out: endpoint { remote-endpoint = <&codec_in>; }; }; }; }; cpu { port { cpu_out: endpoint { remote-endpoint = <&card_in>; }; }; }; codec { port { codec_in: endpoint { remote-endpoint = <&card_out>; }; }; };
This is OK. If CPU has many ports, then ALSA SoC requests 1 Card which has many DAIs (= Digital Audio Interface). This case, cpu will have "ports" and many "port", this is OK.
SoC.0 Codec0 SoC.1 <-> Card <-> Codec1 SoC.2 Codec2
This looks wrong to me. The card is just a container node to group things. The card should not be the middle of the graph. There is no data flow thru the card as it's not a hardware thing. Start with the codec's port connected to the node controlling the audio port on the SOC (In the simple case, that's an SSI controller) and expand the graph from there.
In "card", CPU/Codec pair is indicated by "ports" now (= port0 is CPU, port1 is Codec, etc)
My question is in this case, how to know CPU/Codec pair on "card" ?
The card should only have the entry (or exit) points of the graph. That may not even need to be graph nodes. It could just be a list of CPU DAI phandles (which then have graph nodes).
Look at the display side use of OF graph. We have display subsystem nodes (like a card) with a poorly named ports property (nothing to do with OF graph) listing display channels. Each phandle there is the entry point to the graph and is typically the front end display controller (perhaps a CSC and scaling unit). The graph then goes to a backend controller (for display timing), then to interface PHY (DSI/HDMI), then possibly and external bridge chip, then to a connector or panel node. That's a complex example. In simple cases, we just have a display controller connected to a panel.
Can we have *multi ports*, like this ?
card { ports { ports@0 { port@0 { /* SoC.0 */ };
This is not the OF graph binding, so don't do this. OF graph is flexible enough already to describe any topology. If you feel you need to extend it, then something is wrong.
port@1 { /* Codec0*/ }; }; ports@1 { port@0 { /* SoC.1 */ }; port@1 { /* Codec1*/ }; }; ports@2 { port@0 { /* SoC.2 */ }; port@1 { /* Codec2*/ }; }; };
};
Or can I use grouping, like this ?
card { group = <port@0 port@1>, <port@2 port@3>, <port@4 port@5>;
If it's not clear by now, the graph is supposed to define the connections. You are defining connections here in your own custom way.
ports { port@0 { /* SoC.0 */ }; port@1 { /* SoC.1 */ }; port@2 { /* SoC.2 */ }; port@3 { /* Codec0*/ }; port@4 { /* Codec1*/ }; port@5 { /* Codec2*/ }; };
};
Best regards
Kuninori Morimoto
Hi Rob
Thank you for your feedback
SoC.0 Codec0 SoC.1 <-> Card <-> Codec1 SoC.2 Codec2
(snip)
The card should only have the entry (or exit) points of the graph. That may not even need to be graph nodes. It could just be a list of CPU DAI phandles (which then have graph nodes).
Look at the display side use of OF graph. We have display subsystem nodes (like a card) with a poorly named ports property (nothing to do with OF graph) listing display channels. Each phandle there is the entry point to the graph and is typically the front end display controller (perhaps a CSC and scaling unit). The graph then goes to a backend controller (for display timing), then to interface PHY (DSI/HDMI), then possibly and external bridge chip, then to a connector or panel node. That's a complex example. In simple cases, we just have a display controller connected to a panel.
If my understanding here was correct, do you mean like this ?
sound_dai0 { ports { port { /* Card.0 */ } port { /* SoC.0 */ } port { /* Codec0 */ } }; };
sound_dai1 { ports { port { /* Card.1 */ } port { /* SoC.1 */ } port { /* Codec1 */ } }; };
sound_dai2 { ports { port { /* Card.2 */ } port { /* SoC.2 */ } port { /* Codec2 */ } }; };
card { ports { port { /* sound_dai0 */ } port { /* sound_dai1 */ } port { /* sound_dai2 */ } }; };
Or this ?
sound_dai0 { ports { port { /* SoC.0 */ } port { /* Codec0 */ } }; };
sound_dai1 { ports { port { /* SoC.1 */ } port { /* Codec1 */ } }; };
sound_dai2 { ports { port { /* SoC.2 */ } port { /* Codec2 */ } }; };
card { dais = <&sound_dai0 &sound_dai1 &sound_dai2>; };
Best regards --- Kuninori Morimoto
On Thu, Jan 19, 2017 at 7:46 PM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Rob
Thank you for your feedback
SoC.0 Codec0 SoC.1 <-> Card <-> Codec1 SoC.2 Codec2
(snip)
The card should only have the entry (or exit) points of the graph. That may not even need to be graph nodes. It could just be a list of CPU DAI phandles (which then have graph nodes).
Look at the display side use of OF graph. We have display subsystem nodes (like a card) with a poorly named ports property (nothing to do with OF graph) listing display channels. Each phandle there is the entry point to the graph and is typically the front end display controller (perhaps a CSC and scaling unit). The graph then goes to a backend controller (for display timing), then to interface PHY (DSI/HDMI), then possibly and external bridge chip, then to a connector or panel node. That's a complex example. In simple cases, we just have a display controller connected to a panel.
If my understanding here was correct, do you mean like this ?
I need a drawing of what the hw looks like to really tell you.
sound_dai0 { ports { port { /* Card.0 */ } port { /* SoC.0 */ } port { /* Codec0 */ }
This tells me that the DAI0 h/w block has 2 inputs and 1 output or has 1 input and 2 outputs. I assume that is not what you meant.
}; }; sound_dai1 { ports { port { /* Card.1 */ } port { /* SoC.1 */ } port { /* Codec1 */ } }; }; sound_dai2 { ports { port { /* Card.2 */ } port { /* SoC.2 */ } port { /* Codec2 */ } }; }; card { ports { port { /* sound_dai0 */ } port { /* sound_dai1 */ } port { /* sound_dai2 */ } }; };
Or this ?
sound_dai0 { ports { port { /* SoC.0 */ } port { /* Codec0 */ }
This is probably closer, but I don't know what SoC.0 is. The input should be some audio processor I guess.
}; }; sound_dai1 { ports { port { /* SoC.1 */ } port { /* Codec1 */ } }; }; sound_dai2 { ports { port { /* SoC.2 */ } port { /* Codec2 */ } }; }; card { dais = <&sound_dai0 &sound_dai1 &sound_dai2>; };
Best regards
Kuninori Morimoto
Hi Rob
sound_dai0 { ports { port { /* SoC.0 */ } port { /* Codec0 */ }
This is probably closer, but I don't know what SoC.0 is. The input should be some audio processor I guess.
Thanks. I created OF-graph sound card based on this DT binding. Will post soon
Best regards --- Kuninori Morimoto
Hi Rob, Mark
I created OF-graph base simple sound card. I want to post this patch-set, but it depends many things. Thus, before posting patch-set, I would like to know OF-graph sound DT binding was OK or not. Can you give me comments ?
--- .../bindings/sound/simple-graph-card.txt | 140 +++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-graph-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-graph-card.txt b/Documentation/devicetree/bindings/sound/simple-graph-card.txt new file mode 100644 index 0000000..4b29284 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt @@ -0,0 +1,140 @@ +Simple-Graph-Card: + +Simple-Graph-Card specifies audio DAI connections of SoC <-> codec. +It is based on common bindings for device graphs. +see ${LINUX}/Documentation/devicetree/bindings/graph.txt + +Basically, Simple-Graph-Card property is same as Simple-Card. +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt + +Below are same as Simple-Card. + +- simple-audio-card,name +- simple-audio-card,format +- simple-audio-card,frame-master +- simple-audio-card,bitclock-master +- simple-audio-card,bitclock-inversion +- simple-audio-card,frame-inversion +- simple-audio-card,dai-tdm-slot-num +- simple-audio-card,dai-tdm-slot-width +- clocks / system-clock-frequency + +These should be implemented +- simple-audio-card,widgets +- simple-audio-card,routing +- simple-audio-card,mclk-fs +- simple-audio-card,hp-det-gpio +- simple-audio-card,mic-det-gpio + +Required properties: + +- compatible : "asoc-simple-graph-card"; + +Optional properties: + +- dais : Sound DAI phandle list if Card has + multi DAI. see Example + +Each ports + +- port@0 : CPU setting +- port@1 : Codec setting + +Example: Single DAI case + + sound_card: sound { + compatible = "asoc-simple-graph-card"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + sound0_in: endpoint { + remote-endpoint = <&rsnd_port0>; + + simple-audio-card,format = "left_j"; + simple-audio-card,bitclock-master = <&rsnd_port0>; + simple-audio-card,frame-master = <&rsnd_port0>; + }; + }; + port@1 { + sound0_out: endpoint { + remote-endpoint = <&ak4613_port>; + }; + }; + }; + }; + + +Example: Multi DAI case + + sound_dai0: sound_dai@0 { + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + sound0_in: endpoint { + remote-endpoint = <&rsnd_port0>; + + simple-audio-card,format = "left_j"; + simple-audio-card,bitclock-master = <&rsnd_port0>; + simple-audio-card,frame-master = <&rsnd_port0>; + }; + }; + port@1 { + sound0_out: endpoint { + remote-endpoint = <&ak4613_port>; + }; + }; + }; + }; + + sound_dai1: sound_dai@1 { + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + sound1_in: endpoint { + remote-endpoint = <&rsnd_port1>; + + simple-audio-card,format = "i2s"; + simple-audio-card,bitclock-master = <&rsnd_port1>; + simple-audio-card,frame-master = <&rsnd_port1>; + }; + }; + port@1 { + sound1_out: endpoint { + remote-endpoint = <&rcar_dw_hdmi0_snd_in>; + }; + }; + }; + }; + + sound_dai2: sound_dai@2 { + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + sound2_in: endpoint { + remote-endpoint = <&rsnd_port2>; + + simple-audio-card,format = "i2s"; + simple-audio-card,bitclock-master = <&rsnd_port2>; + simple-audio-card,frame-master = <&rsnd_port2>; + }; + }; + port@1 { + sound2_out: endpoint { + remote-endpoint = <&rcar_dw_hdmi1_snd_in>; + }; + }; + }; + }; + + rsnd_ak4613: sound { + compatible = "asoc-simple-graph-card"; + + dais = <&sound_dai0 + &sound_dai1 + &sound_dai2>; + };
Hi Morimoto-san,
On 01/23/2017 08:33 AM, Kuninori Morimoto wrote:
Hi Rob, Mark
I created OF-graph base simple sound card. I want to post this patch-set, but it depends many things. Thus, before posting patch-set, I would like to know OF-graph sound DT binding was OK or not. Can you give me comments ?
.../bindings/sound/simple-graph-card.txt | 140 +++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-graph-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-graph-card.txt b/Documentation/devicetree/bindings/sound/simple-graph-card.txt new file mode 100644 index 0000000..4b29284 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt @@ -0,0 +1,140 @@ +Simple-Graph-Card:
This binding is likely going to cover most of existing sound system configuration, at least it looks as it has a potential to be scalable enough. So perhaps we could treat is as a more generic binding and could drop the "Simple" word now? I'm not 100% sure it's a good idea, but maybe we could make it "Generic Sound Card DT Binding"?
+Simple-Graph-Card specifies audio DAI connections of SoC <-> codec. +It is based on common bindings for device graphs. +see ${LINUX}/Documentation/devicetree/bindings/graph.txt
The DT bindings should be OS agnostic, I think we should drop the "${LINUX}/" part here and elsewhere in this documentation.
+Basically, Simple-Graph-Card property is same as Simple-Card. +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt
+Below are same as Simple-Card.
+- simple-audio-card,name +- simple-audio-card,format +- simple-audio-card,frame-master +- simple-audio-card,bitclock-master +- simple-audio-card,bitclock-inversion +- simple-audio-card,frame-inversion +- simple-audio-card,dai-tdm-slot-num +- simple-audio-card,dai-tdm-slot-width +- clocks / system-clock-frequency
+These should be implemented +- simple-audio-card,widgets +- simple-audio-card,routing +- simple-audio-card,mclk-fs +- simple-audio-card,hp-det-gpio +- simple-audio-card,mic-det-gpio
+Required properties:
+- compatible : "asoc-simple-graph-card";
"asoc" is Linux related, we shouldn't be putting this in the compatible string. Perhaps "generic-sound-card" ? Again, I'm sure if it is expected to have such a generic kind of DT binding.
+Optional properties:
+- dais : Sound DAI phandle list if Card has
multi DAI. see Example
+Each ports
+- port@0 : CPU setting +- port@1 : Codec setting
+Example: Single DAI case
- sound_card: sound {
compatible = "asoc-simple-graph-card";
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
sound0_in: endpoint {
remote-endpoint = <&rsnd_port0>;
simple-audio-card,format = "left_j";
simple-audio-card,bitclock-master = <&rsnd_port0>;
simple-audio-card,frame-master = <&rsnd_port0>;
};
};
port@1 {
sound0_out: endpoint {
remote-endpoint = <&ak4613_port>;
};
};
};
- };
I wouldn't be making a separate case for single DAI. The 'ports' node can be omitted, port@0, port@1 nodes could be put under respective device nodes and in the 'sound' node we would have 'dais' property pointing to the CPU DAI port, not the endpoint. The endpoint is supposed to describe one of possible configurations of the port. I think we want phandles to the 'port' nodes, not phandles to the 'endpoint' nodes in the 'sound' node.
+Example: Multi DAI case
- sound_dai0: sound_dai@0 {
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
sound0_in: endpoint {
remote-endpoint = <&rsnd_port0>;
simple-audio-card,format = "left_j";
simple-audio-card,bitclock-master = <&rsnd_port0>;
simple-audio-card,frame-master = <&rsnd_port0>;
};
};
port@1 {
sound0_out: endpoint {
remote-endpoint = <&ak4613_port>;
};
};
};
- };
- sound_dai1: sound_dai@1 {
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
sound1_in: endpoint {
remote-endpoint = <&rsnd_port1>;
simple-audio-card,format = "i2s";
simple-audio-card,bitclock-master = <&rsnd_port1>;
simple-audio-card,frame-master = <&rsnd_port1>;
};
};
port@1 {
sound1_out: endpoint {
remote-endpoint = <&rcar_dw_hdmi0_snd_in>;
};
};
};
- };
- rsnd_ak4613: sound {
compatible = "asoc-simple-graph-card";
dais = <&sound_dai0
&sound_dai1
&sound_dai2>;
- };
Hi Sylwester
Thank you for your feedback
+Simple-Graph-Card:
This binding is likely going to cover most of existing sound system configuration, at least it looks as it has a potential to be scalable enough. So perhaps we could treat is as a more generic binding and could drop the "Simple" word now? I'm not 100% sure it's a good idea, but maybe we could make it "Generic Sound Card DT Binding"?
(snip)
+Required properties:
+- compatible : "asoc-simple-graph-card";
"asoc" is Linux related, we shouldn't be putting this in the compatible string. Perhaps "generic-sound-card" ? Again, I'm sure if it is expected to have such a generic kind of DT binding.
This is OF-graph version of existing "simple-card" (sound/soc/generic/simple-card.c). As you know, ASoC sound card can have many features, but what this sound card can do is just CPU-Codec connection and some small additional things. You can use this card if you want to just connect CPU-Codec, but if you want to use more advanced feature on your sound card, then, you need to create your own sound card. Of course we can expand simple card, but has some limitation.
For example, we have simple-scu-card.c which is for DPCM version of simple card. As you can see, these are using almost same DT bindings, but using different feature because normal sound card and DPCM sound card are totally different. Thus, unfortunately, using "generic" in compatible is a little bit over-kill. So I and Mark had named it as "simple" card. Thus I want to keep this "simple" on this OF-graph version driver too.
+Example: Single DAI case
- sound_card: sound {
compatible = "asoc-simple-graph-card";
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
sound0_in: endpoint {
remote-endpoint = <&rsnd_port0>;
simple-audio-card,format = "left_j";
simple-audio-card,bitclock-master = <&rsnd_port0>;
simple-audio-card,frame-master = <&rsnd_port0>;
};
};
port@1 {
sound0_out: endpoint {
remote-endpoint = <&ak4613_port>;
};
};
};
- };
I wouldn't be making a separate case for single DAI. The 'ports' node can be omitted, port@0, port@1 nodes could be put under respective device nodes and in the 'sound' node we would have 'dais' property pointing to the CPU DAI port, not the endpoint. The endpoint is supposed to describe one of possible configurations of the port. I think we want phandles to the 'port' nodes, not phandles to the 'endpoint' nodes in the 'sound' node.
Last 2 line was unfortunately not clear for me. do you mean dais = <&cpu_port>; on card ? This driver want to handle both single/multi DAI connection, so, using same rule is more easy and simple. If my understanding was correct, do you mean like below ?
cpu: cpu { ... cpu_port: port { cpu_out: endpoint { remote-endpoint = <&codec_in>; }; }; };
codec: codec { ... codec_port: port { codec_in: endpoint { remote-endpoint = <&cpu_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu_port>; };
If so, I want Multi DAI like this
cpu: cpu { ... ports { cpu0_port: port { cpu0_out: endpoint { remote-endpoint = <&codec0_in>; }; }; cpu1_port: port { cpu1_out: endpoint { remote-endpoint = <&codec1_in>; }; }; }; };
codec0: codec@0 { ... codec0_port: port { codec0_in: endpoint { remote-endpoint = <&cpu0_out>; }; }; };
codec1: codec@1 { ... codec1_port: port { codec1_in: endpoint { remote-endpoint = <&cpu1_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu0_port &cpu1_port>; };
Best regards --- Kuninori Morimoto
On 01/24/2017 01:30 AM, Kuninori Morimoto wrote:
+Simple-Graph-Card:
This binding is likely going to cover most of existing sound system configuration, at least it looks as it has a potential to be scalable enough. So perhaps we could treat is as a more generic binding and could drop the "Simple" word now? I'm not 100% sure it's a good idea, but maybe we could make it "Generic Sound Card DT Binding"?
(snip)
+Required properties:
+- compatible : "asoc-simple-graph-card";
"asoc" is Linux related, we shouldn't be putting this in the compatible string. Perhaps "generic-sound-card" ? Again, I'm sure if it is expected to have such a generic kind of DT binding.
This is OF-graph version of existing "simple-card" (sound/soc/generic/simple-card.c). As you know, ASoC sound card can have many features, but what this sound card can do is just CPU-Codec connection and some small additional things. You can use this card if you want to just connect CPU-Codec, but if you want to use more advanced feature on your sound card, then, you need to create your own sound card. Of course we can expand simple card, but has some limitation.
For example, we have simple-scu-card.c which is for DPCM version of simple card. As you can see, these are using almost same DT bindings, but using different feature because normal sound card and DPCM sound card are totally different.
Is the main difference between "normal" and DPCM card that in case of the former the data flow routes are static and the latter allows dynamic reconfiguration of sound data routes? AFAIU DPCM stands here for Dynamic PCM [1], rather than Differential Pulse Code Modulation.
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
Is the SCU part in "ASoC simple SCU Sound Card" derived from "(S)ample Rate (C)onverter (U)nit" ?
Thus, unfortunately, using "generic" in compatible is a little bit over-kill. So I and Mark had named it as "simple" card. Thus I want to keep this "simple" on this OF-graph version driver too.
[...]
I wouldn't be making a separate case for single DAI. The 'ports' node can be omitted, port@0, port@1 nodes could be put under respective device nodes and in the 'sound' node we would have 'dais' property pointing to the CPU DAI port, not the endpoint. The endpoint is supposed to describe one of possible configurations of the port. I think we want phandles to the 'port' nodes, not phandles to the 'endpoint' nodes in the 'sound' node.
Last 2 line was unfortunately not clear for me. do you mean dais = <&cpu_port>; on card ? This driver want to handle both single/multi DAI connection, so, using same rule is more easy and simple. If my understanding was correct, do you mean like below ?
Yes, exactly.
cpu: cpu { ... cpu_port: port { cpu_out: endpoint { remote-endpoint = <&codec_in>; }; }; };
codec: codec { ... codec_port: port { codec_in: endpoint { remote-endpoint = <&cpu_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu_port>;
};
If so, I want Multi DAI like this
cpu: cpu { ... ports { cpu0_port: port { cpu0_out: endpoint { remote-endpoint = <&codec0_in>; }; }; cpu1_port: port { cpu1_out: endpoint { remote-endpoint = <&codec1_in>; }; }; }; };
codec0: codec@0 { ... codec0_port: port { codec0_in: endpoint { remote-endpoint = <&cpu0_out>; }; }; };
codec1: codec@1 { ... codec1_port: port { codec1_in: endpoint { remote-endpoint = <&cpu1_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu0_port &cpu1_port>;
};
[1] https://kernel.org/doc/html/latest/sound/soc/dpcm.html
-- Thanks, Sylwester
On Tue, Jan 24, 2017 at 03:10:48PM +0100, Sylwester Nawrocki wrote:
Is the main difference between "normal" and DPCM card that in case of the former the data flow routes are static and the latter allows dynamic reconfiguration of sound data routes? AFAIU DPCM stands here for Dynamic PCM [1], rather than Differential Pulse Code Modulation.
DPCM is a Linux internal abstraction that doesn't represent what's going on in a very generic fashion which makes it problematic when it appears directly in DT bindings. It's very SoC centric.
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
The simple card is creaking at the seams as it was only really designed to represent very simple use cases but has been extended rather beyond that. It's also not set up to cope with things like CODEC<->CODEC links that don't involve the CPU as it really only has the idea of point to point links between a CPU DAI and a CODEC DAI.
Is the SCU part in "ASoC simple SCU Sound Card" derived from "(S)ample Rate (C)onverter (U)nit" ?
Yes.
Hi Sylwester
Is the main difference between "normal" and DPCM card that in case of the former the data flow routes are static and the latter allows dynamic reconfiguration of sound data routes? AFAIU DPCM stands here for Dynamic PCM [1], rather than Differential Pulse Code Modulation.
DPCM is a Linux internal abstraction that doesn't represent what's going on in a very generic fashion which makes it problematic when it appears directly in DT bindings. It's very SoC centric.
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
The simple card is creaking at the seams as it was only really designed to represent very simple use cases but has been extended rather beyond that. It's also not set up to cope with things like CODEC<->CODEC links that don't involve the CPU as it really only has the idea of point to point links between a CPU DAI and a CODEC DAI.
Is the SCU part in "ASoC simple SCU Sound Card" derived from "(S)ample Rate (C)onverter (U)nit" ?
Yes.
The original driver was created for Renesas SCU feature which needed DPCM's .be_hw_params_fixup() for converting. It was created based on simple card. At first, it was created as Renesas specific sound card, but I changed/moved it to one of simple card. Then I named it as simple scu card.
Best regards --- Kuninori Morimoto
Hi Sylwester
Is the main difference between "normal" and DPCM card that in case of the former the data flow routes are static and the latter allows dynamic reconfiguration of sound data routes? AFAIU DPCM stands here for Dynamic PCM [1], rather than Differential Pulse Code Modulation.
DPCM is a Linux internal abstraction that doesn't represent what's going on in a very generic fashion which makes it problematic when it appears directly in DT bindings. It's very SoC centric.
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
The simple card is creaking at the seams as it was only really designed to represent very simple use cases but has been extended rather beyond that. It's also not set up to cope with things like CODEC<->CODEC links that don't involve the CPU as it really only has the idea of point to point links between a CPU DAI and a CODEC DAI.
Is the SCU part in "ASoC simple SCU Sound Card" derived from "(S)ample Rate (C)onverter (U)nit" ?
Yes.
The original driver was created for Renesas SCU feature which needed DPCM's .be_hw_params_fixup() for converting. It was created based on simple card. At first, it was created as Renesas specific sound card, but I changed/moved it to one of simple card. Then I named it as simple scu card.
Best regards --- Kuninori Morimoto
Hi Rob
Rob, can you agree about below Single/Multi OF-graph binding ? If these are OK, I will fix to it.
If my understanding was correct, do you mean like below ?
Yes, exactly.
cpu: cpu { ... cpu_port: port { cpu_out: endpoint { remote-endpoint = <&codec_in>; }; }; };
codec: codec { ... codec_port: port { codec_in: endpoint { remote-endpoint = <&cpu_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu_port>;
};
If so, I want Multi DAI like this
cpu: cpu { ... ports { cpu0_port: port { cpu0_out: endpoint { remote-endpoint = <&codec0_in>; }; }; cpu1_port: port { cpu1_out: endpoint { remote-endpoint = <&codec1_in>; }; }; }; };
codec0: codec@0 { ... codec0_port: port { codec0_in: endpoint { remote-endpoint = <&cpu0_out>; }; }; };
codec1: codec@1 { ... codec1_port: port { codec1_in: endpoint { remote-endpoint = <&cpu1_out>; }; }; };
card { compatible = "asoc-simple-graph-card";
dais = <&cpu0_port &cpu1_port>;
};
Best regards --- Kuninori Morimoto
Hi Sylwester, again
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
The big reason why I want to introduce OF-graph simple card is that I need to support HDMI sound. HDMI video side is already using OF-graph base DT binding. So it is useful if HDMI sound side can follow same style.
Best regards --- Kuninori Morimoto
On 01/25/2017 01:09 AM, Kuninori Morimoto wrote:
It seems the graph based binding could cover above both cases. Apologies if this has been explained before, but what are main reasons for introducing the graph based binding?
The big reason why I want to introduce OF-graph simple card is that I need to support HDMI sound. HDMI video side is already using OF-graph base DT binding. So it is useful if HDMI sound side can follow same style.
OK, thanks, as I understood something which covers more than simple CPU<->CODEC point-to-point connections is needed.
I have also been working on adding proper HDMI audio support for Exynos. One issue I can see with proposed binding is that when we use the graph binding for both video and audio with a device that supports both, e.g. HDMI transmitter we need to somehow differentiate the purpose of each port. To avoid the sound subsystem parsing video port/endpoint nodes and the other way around.
For instance in arch/arm/boot/dts/rk3036.dtsi we have already:
hdmi: hdmi@20034000 { compatible = "rockchip,rk3036-inno-hdmi"; reg = <0x20034000 0x4000>; ...
hdmi_in: port { #address-cells = <1>; #size-cells = <0>; hdmi_in_vop: endpoint@0 { reg = <0>; remote-endpoint = <&vop_out_hdmi>; }; }; };
And with the sound binding added it could have become:
hdmi: hdmi@20034000 { compatible = "rockchip,rk3036-inno-hdmi"; .. ports { #address-cells = <1>; #size-cells = <0>;
/* video */ hdmi_in: port@0 { reg = <0>; hdmi_in_vop: endpoint { remote-endpoint = <&vop_out_hdmi>; }; };
/* audio */ hdmi_audio_out: port@? { reg = <?>; endpoint { remote-endpoint = <&i2s>; }; }; };
i2s: i2s@10220000 { compatible = "rockchip,rk3036-i2s", "rockchip,rk3066-i2s"; reg = <0x10220000 0x4000>; ... };
Now within individual device bindings the type of the port (audio, video) could be determined by reg property, by assigning specific values for video and audio. But if we wanted to make this more generic we would probably need something like a property determining the port's purpose, e.g.
type = "audio";
Regarding compatible string for the card, how about "soc-sound-card-v1" instead of "asoc-simple-graph-card" ? Or "soc-simple-graph-card"? Appending version now could let us avoid inventing funny names when it turns we need some different binding in future.
Hi Sylwester
hdmi: hdmi@20034000 { compatible = "rockchip,rk3036-inno-hdmi"; .. ports { #address-cells = <1>; #size-cells = <0>;
/* video */ hdmi_in: port@0 { reg = <0>; hdmi_in_vop: endpoint { remote-endpoint = <&vop_out_hdmi>; }; }; /* audio */ hdmi_audio_out: port@? { reg = <?>; endpoint { remote-endpoint = <&i2s>; }; };
};
i2s: i2s@10220000 { compatible = "rockchip,rk3036-i2s", "rockchip,rk3066-i2s"; reg = <0x10220000 0x4000>; ... };
Now within individual device bindings the type of the port (audio, video) could be determined by reg property, by assigning specific values for video and audio. But if we wanted to make this more generic we would probably need something like a property determining the port's purpose, e.g.
type = "audio";
Regarding compatible string for the card, how about "soc-sound-card-v1" instead of "asoc-simple-graph-card" ? Or "soc-simple-graph-card"? Appending version now could let us avoid inventing funny names when it turns we need some different binding in future.
It seems you are fighting with this issue which I was fighting :) I guess your issue is related to getting dai_name ? Unfortunately, the idea which adding this "type" property on OF-graph was already rejected by Rob before. He said each driver should know each video/sound port somehow.
So, I'm creating new callback .of_xlate_dai_id on component driver. This callback will exchange reg number to sound dai id. In your case, I think your HDMI sound will be located as reg = <1>; ? If so, your driver can have like below. 1) Your HDMI driver can have .of_xlate_dai_id to exchange reg to sound dai id 2) (If you use simple-graph-card) simple card will call asoc_simple_card_parse_graph_dai() 3) It will try to get dai_id by using .of_xlate_dai_id() first. It will get gai_id simply counting DT ports if driver doesn't have callback. 4) you can get correct dai name
Of course this is not yet accepted/reviewed, but I'm ready to post this patch-set now. It includes both OF-graph simple card, and HDMI sound (= of_xlate_dai_id()) solution. But it is a little bit big volume. Thus, before posting it, I wanted to confirm DT bindings. This mail thread is for it. I'm waiting Rob's reply now
-- on your HDMI driver --
static int xxx_get_dai_id(struct snd_soc_component *component, struct device_node *endpoint) { struct of_endpoint of_ep; int ret;
ret = of_graph_parse_endpoint(endpoint, &of_ep); if (ret < 0) return ret;
/* * HDMI sound should be located as reg = <1> * Then, it is sound port 0 */ if (of_ep.port == 1) return 0;
return -EINVAL; }
--- on soc-core ---
int snd_soc_get_dai_id(struct device_node *ep) { struct snd_soc_component *pos; struct device_node *node; struct device_node *endpoint; int i, id; int ret;
node = of_graph_get_port_parent(ep);
/* * For example HDMI case, HDMI has video/sound port, * but ALSA SoC needs sound port number only. * Thus counting HDMI DT port/endpoint doesn't work. * Then, it should have .of_xlate_dai_id */ ret = -ENOTSUPP; mutex_lock(&client_mutex); list_for_each_entry(pos, &component_list, list) { struct device_node *component_of_node = pos->dev->of_node;
if (!component_of_node && pos->dev->parent) component_of_node = pos->dev->parent->of_node;
if (component_of_node != node) continue;
if (pos->driver->of_xlate_dai_id) ret = pos->driver->of_xlate_dai_id(pos, ep);
break; } mutex_unlock(&client_mutex);
if (ret != -ENOTSUPP) return ret;
/* * Non HDMI sound case, counting port/endpoint on its DT * is enough. Let's count it. */ i = 0; id = -1; for_each_endpoint_of_node(node, endpoint) { if (endpoint == ep) id = i; i++; } if (id < 0) return -ENODEV;
return id; } EXPORT_SYMBOL_GPL(snd_soc_get_dai_id);
--- simple-card-utils ----
int asoc_simple_card_parse_graph_dai(struct device_node *ep, struct device_node **dai_of_node, const char **dai_name) { struct device_node *node; struct of_phandle_args args; int ret;
if (!ep) return 0; if (!dai_name) return 0;
node = of_graph_get_port_parent(ep);
/* Get dai->name */ args.np = node; args.args[0] = snd_soc_get_dai_id(ep); args.args_count = (of_graph_get_endpoint_count(node) > 1);
ret = snd_soc_get_dai_name(&args, dai_name); if (ret < 0) return ret;
*dai_of_node = node;
return 0; } EXPORT_SYMBOL_GPL(asoc_simple_card_parse_graph_dai);
Best regards --- Kuninori Morimoto
participants (4)
-
Kuninori Morimoto
-
Mark Brown
-
Rob Herring
-
Sylwester Nawrocki