> -----Original Message-----
> From: Bruce Richardson <bruce.richard...@intel.com>
> Sent: Tuesday, October 20, 2020 10:35 AM
> To: McDaniel, Timothy <timothy.mcdan...@intel.com>
> Cc: Jerin Jacob <jerinjac...@gmail.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 Tue, Oct 20, 2020 at 04:17:13PM +0100, McDaniel, Timothy wrote:
> >
> >
> > > -----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.
> > >
> > > >
> > > > > +
> > > <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
> >
> > Due to time constraints, I would prefer to take these issues up in a future
> release.
> >
> I'm not suggesting you need to fix the existing octeon case, but it's easy
> enough to add dlb2 to the existing list and add :
>
> if not dpdk_conf.has('RTE_ARCH_X86_64') or not is_linux
> build = false
> reason = "..."
> endif
>
> to the dlb2 meson.build file.
>
> Regards,
> /Bruce
Very well. I used the existing event/meson.build and conditional inclusion of
the octeon driver
as my guide for conditionally including DLB and DLB2. It's easy enough to
change, so I will do it.
