RE: [PATCH v2 34/37] baseband/acc100: update meson file sdk dependency

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

Hi Thomas, 

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, September 15, 2022 3:58 AM
> To: Vargas, Hernan <hernan.vargas@intel.com>
> Cc: dev@dpdk.org; gakhil@marvell.com; trix@redhat.com; Chautru, Nicolas
> <nicolas.chautru@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Maxime
> Coquelin <maxime.coquelin@redhat.com>
> Subject: Re: [PATCH v2 34/37] baseband/acc100: update meson file sdk
> dependency
> 
> 15/09/2022 12:31, Maxime Coquelin:
> >
> > On 8/20/22 04:31, Hernan Vargas wrote:
> > > Update meson files with FlexRAN SDK dependency.
> 
> There is no reason for this commit.
> If the reason is that you need these dependencies for some features, it is
> better to introduce the dependency when you use it.
> Patches should be split per features, not per file.

Agreed this is a mistake and this change is actually required by patch 15 in that serie. 

> 
> > > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > > ---
> > >   drivers/baseband/acc100/meson.build | 21 +++++++++++++++++++++
> > >   1 file changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/baseband/acc100/meson.build
> > > b/drivers/baseband/acc100/meson.build
> > > index 9a1a3b8b07..3b934a25ca 100644
> > > --- a/drivers/baseband/acc100/meson.build
> > > +++ b/drivers/baseband/acc100/meson.build
> > > @@ -1,6 +1,27 @@
> > >   # SPDX-License-Identifier: BSD-3-Clause
> > >   # Copyright(c) 2020 Intel Corporation
> > >
> > > +# check for FlexRAN SDK libraries
> > > +dep_dec5g = dependency('flexran_sdk_ldpc_decoder_5gnr', required:
> > > +false)
> > > +
> > > +if dep_dec5g.found()
> > > +    ext_deps += cc.find_library('libstdc++', required: true)
> > > +    ext_deps += cc.find_library('libirc', required: true)
> > > +    ext_deps += cc.find_library('libimf', required: true)
> > > +    ext_deps += cc.find_library('libipps', required: true)
> > > +    ext_deps += cc.find_library('libsvml', required: true)
> > > +    ext_deps += dep_dec5g
> > > +    ext_deps += dependency('flexran_sdk_ldpc_encoder_5gnr', required:
> true)
> > > +    ext_deps += dependency('flexran_sdk_LDPC_ratematch_5gnr',
> required: true)
> > > +    ext_deps += dependency('flexran_sdk_rate_dematching_5gnr',
> required: true)
> > > +    ext_deps += dependency('flexran_sdk_turbo', required: true)
> > > +    ext_deps += dependency('flexran_sdk_crc', required: true)
> > > +    ext_deps += dependency('flexran_sdk_rate_matching', required: true)
> > > +    ext_deps += dependency('flexran_sdk_common', required: true)
> > > +    cflags += ['-DRTE_BBDEV_SDK_AVX2']
> > > +    cflags += ['-DRTE_BBDEV_SDK_AVX512'] endif
> > > +
> > >   deps += ['bbdev', 'bus_vdev', 'ring', 'pci', 'bus_pci']
> > >
> > >   sources = files('rte_acc100_pmd.c')
> >
> > I think we should improve build coverage with stubs.
> >
> > For example, we could stub bblib_rate_dematching_5gnr(), and so all
> > the code under RTE_BBDEV_SDK_AVX512 ifdef in
> > enqueue_ldpc_dec_one_op_cb() would be built.
> 
> Yes, having code built even when the proprietary dependency is missing,
> would help to track some issues.
> 
> > It would even open the possibility to have open-source implementations
> > of these libraries if community feel the need.
> >
> > What do you think?

We need to assess at compile time whether the SDK is present or not, as the processing would be different (or in the case of the turbo_sw PMD using the same dependency to talk out some dependency).
So I don't think the stub is an option except if I miss the meaning of your suggestion. 
Note that other libraries could be used instead of the Intel ones as long as the same prototype is used (this is notably used by 3rd party company to provide their own implementation of these modules).


> >
> > Thanks,
> > Maxime
> 
>