Re: [RFC PATCH] soc: audio-graph-card2: use correct endpoint when getting link parameters

[Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>]

Hi Morimoto-san

On 14.01.25 г. 8:44 ч., Kuninori Morimoto wrote:
> 
> Hi Ivaylo
> 
> Sorry for the late review.
> 

And sorry for the noise on my side.

>> We may have multiple links between ports, with each link
>> having different parameters. Currently, no matter the topology,
>> it is always port endpoint 0 that is used when setting parameters.
>>
>> On a complex sound system, like the one found on Motorola droid4,
>> hifi and voice DAIs require differents formats (i2s vs dsp_a)
>> and curently it is impossible to use DT to set that.
>>   
>> Implementing the change leads to partially dropping of at least
>> 0dedbde5062d (ASoC: cpcap: Implement set_tdm_slot for voice call
>> support), as core does most of what is needed to configure voice DAI.
>>
>> We (on Maemo Leste ) use the patch (along with few others) to have
>> voice calls working properly on d4 through UCM.
>>
>> The patch is for linux 6.6, I want to know whether the
>> approach would be accepted before sending a proper patch for
>> current master.
>>
>> the original commit message follows:
>>
>> When link parameters are parsed, it is always endpoint@0 that is used and
>> parameters set to other endpoints are ignored.
>>
>> Fix that by using endpoint that is set in DT when parsing link parameters.
>>
>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> ---
> (snip)
>> @@ -684,7 +683,6 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv,
>>   {
>>   	struct device_node *ep = port_to_endpoint(lnk);
>>   	struct device_node *rep = of_graph_get_remote_endpoint(ep);
>> -	struct device_node *rport = of_graph_get_remote_port(ep);
>>   	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
>>   	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
>>   	int is_cpu = asoc_graph_is_ports0(lnk);
>> @@ -718,7 +716,7 @@ int audio_graph2_link_dpcm(struct asoc_simple_priv *priv,
>>   		dai_link->dynamic		= 1;
>>   		dai_link->dpcm_merged_format	= 1;
>>   
>> -		ret = graph_parse_node(priv, GRAPH_DPCM, rport, li, 1);
>> +		ret = graph_parse_node(priv, GRAPH_DPCM, rep, li, 1);
> 
> Please correct me if I was misunderstanding
> Is the main issue "remote" side endpoint ?
> 
> You want to parse "remote" endpoint (= rep) directly, but the function
> requests "port" (= rport), and it will use endpoint0 ( != rep).
> Is this the main issue you want to fix ?
> 

Yes, it is the 'remote' side endpoint, currently it is always remote 
endpoint0 that is used, because when you get 'port', it is endpoint0 of 
that port that core uses.

See:
https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi#L91

https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-mapphone-handset.dtsi#L65

and

https://github.com/maemo-leste/droid4-linux/blob/maemo-6.6.y/arch/arm/boot/dts/ti/omap/motorola-mapphone-common.dtsi#L476

as an example DTS that is using multiple endpoints per port and also

https://lkml.org/lkml/2018/3/27/1225

for what audio wiring looks like.

For voice calls the device does not use CPU, but we have C2C link 
between modem and cpcap instead. However, we must correctly set DAI 
format on cpcap side for for that link to work properly.

General speaking, we might have multiple endpoints connected for a 
single port and when getting "link properties" I think we should use 
remote endpoint that is linked to local endpoint, not always remote 
endpoint0.

If it is still not clear what $subject patch tries to achieve, please 
LMK and I'll try to elaborate even more, if possible.

Also, if you think core allows such 'routing' to be implemented without 
the $subject functionality, please elaborate. I spent a good amount of 
time back then with no luck.

Thanks and regards,
Ivo