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

[Keyon Jie]

On 2018年05月22日 14:26, Ranjani Sridharan wrote:
> 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.

I also found this issue just now when I am debugging it, then I refer 
back to the patch I made before 1.1, it actually should be id, not index 
here.

Thanks for the correction, will send patch with this once everyone 
aligned to use this solution.

Thanks,
~Keyon

> 
> 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
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>