On Tue, 2018-05-22 at 14:33 +0800, Keyon Jie wrote:
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.
Keyon I've been thinking about this a little bit more and I have a couple of questions:
1. What will the ID be? Could it be the same as the port number? 2. Pierre brought this up in another email, will the ID's be sequential across different DAI types? Or will they start at 0?
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-firmw are
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar e