Re: [PATCH RFC 2/3] ASoC: audio-graph-card2: support explicitly disabled links

[Mihalcea Laurentiu <laurentiumihalcea111@gmail.com>]

On 19.05.2025 04:15, Kuninori Morimoto wrote:
> Hi Mihalcea
>
> Thank you for clarify details.
>
>> [snippet from base.dts]
>>
>> 	my_card: card {
>> 		compatible = "audio-graph-card2";
>> 		links = <&l2>; /* here, we're only allowed to specify links that exist */
>> 		routing = "Headphones", "C20",
>> 			  "Headphones", "C21",
>> 			  "Line", "C01";
>> 	};
> (snip)
>> &my_card {
>> 	/* here we're forced to also specify l2 even if this is already done
>>          * in base.dts. This is because DT overlays don't support appending to
>> 	 * properties.
>>          */
>> 	remote-endpoint = <&l0>, <&l1>, <&l2>;
>> };
> This is very nit pickking, but I need to confirm.
> You want to indicate here is this ?
>
> 	&my_card {
> -		remote-endpoint = <&l0>, <&l1>, <&l2>;
> +		links = <&l0>, <&l1>, <&l2>;
> 	};


yes, I'm very sorry for the mistakes....


>
>> &l0_ep {
>> 	remote-endpoint = <&c1_ep>;
>> };
>>
>> &l1_ep {
>> 	remote-endpoint = <&c2_ep>;
>> };
>>
>> c0: codec@0 {
>> 	port { c0_ep: endpoint { remote-endpoint = <&l0_ep>; } };
>> };
>>
>> c1: codec@1 {
>> 	port { c1_ep: endpoint { remote-endpoint = <&l1_ep>; } };
>> };
> This is also nit pickking, but I think above is wrong.
> I guess you want to indicate is...
>
> 	&l0_ep {
> -		remote-endpoint = <&c1_ep>;
> +		remote-endpoint = <&c0_ep>;
> 	};
>
> 	&l1_ep {
> -		remote-endpoint = <&c2_ep>;
> +		remote-endpoint = <&c1_ep>;
> 	};
>
>
> Your are indicating very confusable naming, so I want to understand
> correctly as much as possible.


yep, you're right. Again, very sorry.


>
>>>> 	CODEC A has widgets A0, A1.
>>>> 	CODEC B has widgets B0, B1.
>>>>
>>>> 	my-card {
>>>> 		compatible = "audio-graph-card2":
>>>> 		label = "my-label";
>>>> 		links = <&cpu0_port>, <&cpu1_port>;
>>>> 		routing = "A0", "A1",
>>>> 		          "B0", "B1";
>>>> 	};
>>>>
>>>> 	CODEC A's DT node was disabled.
>>>> 	CODEC B's DT node is enabled.
>>>> 	CPU0's DT node is enabled.
>>>> 	CPU1's DT node is enabled.
> (snip)
>> Assume that we also have BASE1 that is compatible with PLUGIN but
>> C0 and C1 are connected to BASE1's D0 and D5. Since there's no D1-C1
>> connection that means you'll have to create another DT overlay. Thus,
>> the scalability of plugin.dtso decreases.
>>
>> Now, for our particular case, we have BASE0 and BASE1 with the following
>> DAIs and CODECS (these are physically present on the base boards):
>>
>> BASE0 has DAIs D0, D1 and CODEC C0
>> BASE1 has DAIs D0, D1 and CODEC C1
>>
>> Both of these boards are compatible with plugin PLUGIN that has codec C2.
>> The possible DAI links are:
>>
>> For BASE0:
>>
>> D0 <----> C0
>> D1 <----> C2 (only possible with PLUGIN connected)
>>
>> For BASE1:
>>
>> D0 <----> C1
>> D1 <----> C2 (only possible with PLUGIN connected)
>>
>> Since the D1 <---> C2 connection is valid for both BASE0-PLUGIN and
>> BASE1-PLUGIN combinations I think we can make do without the support
>> for explicitly disabled links. But I don't think this is ideal because:
> Let's avoid BASE3 case here to avoid _if_ story.
> You are now indicating too many complex/cofusable situations with wrong
> setting samples (D0/D1/D2..., C0/C1/C2. BASEx has no D1-C1..., etc, etc...)
>
> I noticed that why you need to add disabled Codec routing on BASE DT ?
> It is the reason why you can't detect BASE only sound card, right ?


exactly!!!


>
> 	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
> [snippet from base.dts]
>
> 	my_card: card {
> 		compatible = "audio-graph-card2";
> 		links = <&l2>; /* here, we're only allowed to specify links that exist */
> 		routing = "Headphones", "C20",
> 			  "Headphones", "C21",
> =>			  "Line", "C01";
> 	};
>
> 	---- 8< ---- 8< ---- 8< ---- 8< ---- 8< ----
>
> If my understanding was correct, your system can be indicated like below
> (It is not same as your D0/D1/D2 sample, but I think same things, and
>  not confusable sample)
>
> 	BASE			  PLUGIN
> 	+-----------------+
> 	| CPU0 <-> Codec0 |     +--------+
> 	| CPU1		  | <-> | Codec1 |
> 	| CPU2		  | <-> | Codec2 |
> 	+-----------------+     +--------+


pretty much. The only difference would be that PLUGIN has only 1 codec

in our case. I think it'll be much easier to just stick to your naming conventions...


>
> How it works by below ?
>
> BASE
> 	/*
> 	 * detect CPU0-Codec0 connection only
> 	 * Codec1/2 are disabled, but not related to BASE
> 	 */
> 	my_card: card {
> 		links = <&cpu0>;
> 		routing = "Headphone0", "Codec0"; /* for CPU0-Codec0 */
> 	};
>
> PLUGIN
> 	/* detect all
> 	 * CPU0-Codec0 connection
> 	 * CPU1-Codec1 connection
> 	 * CPU2-Codec2 connection, and its routings */
> 	&my_card {
> 		links = <&cpu0>, <&cpu1>, <&cpu2>;
> 		routing = "Headphone0", "Codec0", /* for CPU0-Codec0 */
> 			  "Headphone1", "Codec1", /* for CPU1-Codec1 */
> 			  "Headphone2", "Codec2"; /* for CPU2-Codec2 */
> 	};


so, the problem with this is the fact that (assuming you've used a DT overlay

for the PLUGIN) you won't be able to use the DT overlay on other boards because

you've also added the "Headphone0", "Codec0" route which is specific to BASE's

Codec0. We have multiple boards so our system would look like this:


	BASE0			  PLUGIN
	+-----------------+
	| CPU0 <-> Codec0 |     +--------+
	| CPU1		  | <-> | Codec1 |
	+-----------------+     +--------+


	BASE1			  PLUGIN
	+-----------------+
	| CPU0 <-> Codec3 |     +--------+
	| CPU1		  | <-> | Codec1 |
	+-----------------+     +--------+


The plugin is the same. The only difference between BASE1 and BASE0 is the fact that CPU0

is connected to Codec0 on BASE0, while, on BASE1, CPU0 is connected to a different codec: Codec3.


>
>
> And/Or your situation is similar as mine (I should have noticed
> about this sooner).
>
> 	d70be079c3cf34bd91e1c8f7b4bc760356c9150c
> 	("arm64: dts: renesas: ulcb/kf: Use multi Component sound")
>
> 	547b02f74e4ac1e7d295a6266d5bc93a647cd4ac
> 	("ASoC: rsnd: enable multi Component support for Audio Graph Card/Card2")
>
> 	45655ec69cb954d7fa594054bec33d6d5b99f8d5
> 	("ASoC: soc-core.c: enable multi Component")
>
> My board is handling above sample as 2 cards, by using "multi Component"
>
> 	BASE			  PLUGIN
> 	+-----------------+			^
> 	| CPU0 <-> Codec0 |			| Card1
> 	|		  |			v
> 	|		  |     +--------+	^
> 	| CPU1		  | <-> | Codec1 |	| Card2
> 	| CPU2		  | <-> | Codec2 |	|
> 	+-----------------+     +--------+	v


one important thing to note here is the fact that we can only

have 1 sound card because all DAIs (CPU0, CPU1, CPU2) belong

to the same component.


>
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto