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

[Ranjani Sridharan]

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:
> > > 
> > > 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.

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