Question about OF-graph ports

Question about OF-graph ports

[Kuninori Morimoto]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about OF-graph ports

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Question about OF-graph ports

[Rob Herring]

On Thu, Jan 19, 2017 at 12:07 AM, Kuninori Morimoto
<kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about OF-graph ports

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about OF-graph ports

[Rob Herring]

On Thu, Jan 19, 2017 at 7:46 PM, Kuninori Morimoto
<kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about OF-graph ports

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH] ASoC: add simple-graph-card document

[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>;
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Sylwester Nawrocki]

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>;
> +	};

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Kuninori Morimoto]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Sylwester Nawrocki]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Sylwester Nawrocki]

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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Kuninori Morimoto]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html