> -----Original Message-----
> From: Bruce Richardson <bruce.richard...@intel.com>
> Sent: Monday, October 19, 2020 3:34 AM
> To: Jerin Jacob <jerinjac...@gmail.com>
> Cc: McDaniel, Timothy <timothy.mcdan...@intel.com>; Mcnamara, John
> <john.mcnam...@intel.com>; Kovacevic, Marko
> <marko.kovace...@intel.com>; Ray Kinsella <m...@ashroe.eu>; Neil Horman
> <nhor...@tuxdriver.com>; dpdk-dev <dev@dpdk.org>; Carrillo, Erik G
> <erik.g.carri...@intel.com>; Eads, Gage <gage.e...@intel.com>; Van Haaren,
> Harry <harry.van.haa...@intel.com>; Jerin Jacob <jer...@marvell.com>;
> Thomas Monjalon <tho...@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v2 01/22] event/dlb2: add documentation and
> meson build infrastructure
>
> On Sun, Oct 18, 2020 at 02:18:32PM +0530, Jerin Jacob wrote:
> > On Sat, Oct 17, 2020 at 11:50 PM Timothy McDaniel
> > <timothy.mcdan...@intel.com> wrote:
> > >
> > > Adds the meson build infrastructure, which includes
> > > compile-time constants in rte_config.h. DLB2 is
> > > only supported on Linux X86 platforms at this time.
> > >
> > > Signed-off-by: Timothy McDaniel <timothy.mcdan...@intel.com>
> > > Reviewed-by: Gage Eads <gage.e...@intel.com>
> > > ---
> > > config/rte_config.h | 7 +
> > > doc/guides/eventdevs/dlb2.rst | 330
> > > ++++++++++++++++++++++
> > > doc/guides/eventdevs/index.rst | 1 +
> > > drivers/event/dlb2/meson.build | 7 +
> > > drivers/event/dlb2/rte_pmd_dlb2_event_version.map | 3 +
> > > drivers/event/meson.build | 3 +
> > > 6 files changed, 351 insertions(+)
> > > create mode 100644 doc/guides/eventdevs/dlb2.rst
> > > create mode 100644 drivers/event/dlb2/meson.build
> > > create mode 100644 drivers/event/dlb2/rte_pmd_dlb2_event_version.map
> > >
> > > diff --git a/config/rte_config.h b/config/rte_config.h
> > > index 0bae630..fd1b3c3 100644
> > > --- a/config/rte_config.h
> > > +++ b/config/rte_config.h
> > > @@ -131,4 +131,11 @@
> > > /* QEDE PMD defines */
> > > #define RTE_LIBRTE_QEDE_FW ""
> > >
> > > +/* DLB2 defines */
> > > +#define RTE_LIBRTE_PMD_DLB2_POLL_INTERVAL 1000
> > > +#define RTE_LIBRTE_PMD_DLB2_UMWAIT_CTL_STATE 0
> > > +#undef RTE_LIBRTE_PMD_DLB2_QUELL_STATS
> > > +#define RTE_LIBRTE_PMD_DLB2_SW_CREDIT_QUANTA 32
> > > +#define RTE_PMD_DLB2_DEFAULT_DEPTH_THRESH 256
> >
> > We are going way with GLOBAL compile time options for drivers.
> > I think, we should move to driver-specific folder not pollute top
> > level config file.
> > @Richardson, Bruce @Thomas Monjalon Thoughts?
> >
> Yes, we do need a better way to pass driver-specific build config options.
> How likely is it that the user may want to tweak these values? If it's not
> likely having them just as regular defines in the driver folder may be
> better, though they are not particularly problematic in rte_config.h given
> the number of other values already there.
>
We were careful to only place those options that were likely to be modified by
the end user
here.
> >
> > > +
> <snip>
> > > --- a/drivers/event/meson.build
> > > +++ b/drivers/event/meson.build
> > > @@ -10,6 +10,9 @@ if not (toolchain == 'gcc' and
> cc.version().version_compare('<4.8.6') and
> > > dpdk_conf.has('RTE_ARCH_ARM64'))
> > > drivers += 'octeontx'
> > > endif
> > > +if (dpdk_conf.has('RTE_ARCH_X86_64') and is_linux)
> > > + drivers += 'dlb2'
> > > +endif
> >
> > Please add the message in "Content Skipped" section,
> > Reference: grep "reason" in drivers/vdpa/mlx5/meson.build
> >
>
> The octeontx case is also wrong in this file, IMHO. Rather than checking
> things in the event level and adding things to the list, the list should
> just be static. If something should be optionally compiled, then check the
> conditions in the driver meson.build file itself and add "build=false" to
> disable, setting "reason" to the cause of it being disabled. This keeps all
> the logic about a driver in the other file, rather than someone having to
> look in multiple places for why something is or isn't getting built
> properly.
>
> Regards,
> /Bruce
