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

Message ID 20220820023157.189047-35-hernan.vargas@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series baseband/acc100: changes for 22.11 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hernan Vargas Aug. 20, 2022, 2:31 a.m. UTC
  Update meson files with FlexRAN SDK dependency.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc100/meson.build | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
  

Comments

Maxime Coquelin Sept. 15, 2022, 10:31 a.m. UTC | #1
On 8/20/22 04:31, Hernan Vargas wrote:
> Update meson files with FlexRAN SDK dependency.
> 
> 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.

It would even open the possibility to have open-source implementations
of these libraries if community feel the need.

What do you think?

Thanks,
Maxime
  
Thomas Monjalon Sept. 15, 2022, 10:57 a.m. UTC | #2
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.

> > 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?
> 
> Thanks,
> Maxime
  
Chautru, Nicolas Sept. 16, 2022, 12:39 a.m. UTC | #3
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
> 
>
  

Patch

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')