[alsa-devel] regression: bad sound card type detection in audio graph

[Arnaud Pouliquen]

On 3/29/19 3:03 AM, Kuninori Morimoto wrote:
> 
> Hi Arnaud
> 
>> Thanks for your answer and my apologize. I found the issue: it was
>> located between my keyboard and my seat...
> 
> OK, no problem.
> 
>> ports {
>>        #address-cells = <1>;
>>         #size-cells = <0>;
>>        wm8994_tx_endpoint: endpoint at 0 {
>>                reg = <0>;
>>                 remote-endpoint = <&sai2a_endpoint>;
>>        };
>>        wm8994_rx_endpoint: endpoint at 1 {
>>                reg = <1>;
>>                 remote-endpoint = <&sai2b_endpoint>;
>>        };
>> };
> 
> Not a big deal, but I think you want to use "port" not "ports" here.
> 
>> This trig me an extra question: how to manage a configuration using only
>> one codec DAI (bidir) connected to 2 unidirectional CPU-DAI?
>>   Playback: CPU-DAI (slave)-> (master)CODEC-DAI/port0
>>   Record: CPU-DAI (slave)<- (master)CODEC-DAI/port0
> 
> A little bit confused.
> 
>         // 1 CPU, 1 Codec
>         Playback: CPU-DAI(slave) -> (master)CODEC-DAI/port0
>         Record:   CPU-DAI(slave) <- (master)CODEC-DAI/port0
> or
>         // 2 CPU, 1 Codec
>         Playback: CPU-A-DAI(slave) -> (master)CODEC-DAI/port0
>         Record:   CPU-B-DAI(slave) <- (master)CODEC-DAI/port0
> 
> 1st one is very normal at ALSA SoC I think.
> 1 endpoint is 1 DAI, and it has 1 playback and 1 capture.
> 2nd one is indeed special case.
> 
>> In this case it seems logic to declare for the codec, one port with 2
>> endpoints so this imply to use DPCM mechanism... right?
> 
> So far, yes
> 
>> In this case we should use a static routing to conncet FE and BE...?
>> Do you know if this kind of configuration has been tested?
> 
> I assume it is above "2nd" case,
> unfortunately, this case is not assumed on audio-graph-card so far.
Yes here i was speaking about the second case.
This confirms that we need to look deeper in code on our side to find
and propose a solution for this type of links.

> 
>> FYi, This configuration is available on the stm32mp157c-dk2 board. Today
>> we have not an upstreamable solution (we have hacked the codec to
>> duplicate the DAI to create 2 ports with one endpoint, instead of one
>> port with 2 endpoints).
> 
> I'm not sure, but if we can judge
> "it has multi endpoints, but it is not a DPCM connection" somehow,
> we can solve your issue ?
> The issue here is this judgment should be DT acceptable.>
> Hmm...
> Easy solution I noticed now is that compatible = "audio-graph-card" case
> should only select non DPCM case, same as before.
> compatible = "audio-graph-scu-card" can be both "non DPCM" and "DPCM".
> I think only user of "audio-graph-scu-card" is me, and it is local use.
> I think it is still DT backward compatible (?)
> I didn't test this patch, but maybe similar can be solution ?

Yes i think that is a good proposal with advantage of ensuring
compatibility with existing code.

Thanks for your clarifications!

Best regards,
Arnaud

> 
> ------------------
> diff --git a/sound/soc/generic/audio-graph-card.c
> b/sound/soc/generic/audio-graph-card.c
> index b75b0f1..0b39d85 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -402,6 +402,7 @@ static int graph_for_each_link(struct
> asoc_simple_priv *priv,
>          struct device_node *codec_port_old = NULL;
>          struct asoc_simple_data adata;
>          int rc, ret;
> +       int dpcm_selectable = (int)of_device_get_match_data(dev);
>  
>          /* loop for all listed CPU port */
>          of_for_each_phandle(&it, rc, node, "dais", NULL, 0) {
> @@ -431,7 +432,8 @@ static int graph_for_each_link(struct
> asoc_simple_priv *priv,
>                           * if Codec port has many endpoints,
>                           * or has convert-xxx property
>                           */
> -                       if ((of_get_child_count(codec_port) > 1) ||
> +                       if ((dpcm_selectable &&
> +                            of_get_child_count(codec_port) > 1) ||
>                              adata.convert_rate || adata.convert_channels)
>                                  ret = func_dpcm(priv, cpu_ep, codec_ep, li,
>                                                  (codec_port_old ==
> codec_port));
> @@ -664,8 +666,8 @@ static int graph_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id graph_of_match[] = {
> -       { .compatible = "audio-graph-card", },
> -       { .compatible = "audio-graph-scu-card", },
> +       { .compatible = "audio-graph-card",     .data = 0 },
> +       { .compatible = "audio-graph-scu-card", .data = 1 },
>          {},
>  };
>  MODULE_DEVICE_TABLE(of, graph_of_match);
> ------------------
> 
> Best regards
> ---
> Kuninori Morimoto