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:
- remove the hardcode(to 0) of link->id in topology and add a param
for that. I provide a RFC patch below.
- 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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware