[Sound-open-firmware] about multiple BE dai_links support

[Ranjani Sridharan]

On Tue, 2018-05-22 at 12:50 +0800, Keyon Jie wrote:
> Hi all,
> 
> As we are using link->id for BE dai_link match in 
> snd_soc_find_dai_link(), so the link->id define in machine driver is 
> crucial.
> 
> Previously we set all link->id to be 0s in machine drivers(e.g. 
> nocodec.c, bytcr_rt5651.c, cnl_rt274.c, bxt_pcm512x.c...), but in
> many 
> other cAVS machine drivers(skl_rt286.c, kbl_rt5663_max98927.c, 
> bxt_tdf8532.c, ...), they are using different link->id for different
> BE 
> dai_links.
> 
> So, to reuse those machine driver and BE dai_links for SOF, we have
> 2 
> solution to fix this:
> 
> 1. remove the hardcode(to 0) of link->id in topology and add a param
> for 
> that. I provide a RFC patch below.
> 
> 2. change snd_soc_find_dai_link(), remove the link->id comparison.
> 
> Personally I don't like the #2 solution, it will lead to backward 
> compatibility issue for those exist drivers, and also is mismatch
> with 
> the comment of snd_soc_find_dai_link():
>   * @id: DAI link ID to match
>   * @name: DAI link name to match, optional
>   * @stream_name: DAI link stream name to match, optional
> 
> Hi Pierre/Liam, any comments on this? I hope we can align this soon
> then 
> I can refine and send machine driver for GPMRB.
> 
> 
> Thanks,
> ~Keyon
> 
> diff --git a/topology/m4/dai.m4 b/topology/m4/dai.m4
> index 0b030af..0a79364 100644
> --- a/topology/m4/dai.m4
> +++ b/topology/m4/dai.m4
> @@ -140,17 +140,17 @@ define(`DAI_TDM',
>   dnl DAI Config)
>   define(`N_DAI_CONFIG', `DAICONFIG.'$1)
> 
> -dnl DAI_CONFIG(type, idx, name, format, valid bits, mclk, bclk,
> fsync, tdm)
> +dnl DAI_CONFIG(type, idx, link_id, name, format, valid bits, mclk, 
> bclk, fsync, tdm)
>   define(`DAI_CONFIG',
>   `SectionHWConfig."'$1$2`" {'
>   `'
>   `      id              "'$2`"'
>   `      format          "'$4`"'
>   `'
> -`      '$6
>   `      '$7
>   `      '$8
>   `      '$9
> +`      '$10
>   `}'
>   `SectionVendorTuples."'N_DAI_CONFIG($1$2)`_tuples_str" {'
>   `      tokens "sof_dai_tokens"'
> @@ -164,15 +164,15 @@ define(`DAI_CONFIG',
>   `SectionVendorTuples."'N_DAI_CONFIG($1$2)`_tuples" {'
>   `      tokens "sof_dai_tokens"'
>   `      tuples."word" {'
> -`              SOF_TKN_DAI_SAMPLE_BITS'        STR($5)
> +`              SOF_TKN_DAI_SAMPLE_BITS'        STR($6)
>   `      }'
>   `}'
>   `SectionData."'N_DAI_CONFIG($1$2)`_data" {'
>   `      tuples "'N_DAI_CONFIG($1$2)`_tuples"'
>   `}'
>   `'
> -`SectionBE."'$3`" {'
> -`      index "0"'
> +`SectionBE."'$4`" {'
> +`      index "'$3`"'
Keyon, I may be incorrect but I have always associated index with
Pipeline_ID. So I am not sure this is the right way to assign the id. 

Why the choice of index instead of the ID in SectionHWConfig?

>   `      default_hw_conf_id      "'$2`"'
>   `'
>   `      hw_configs ['
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware