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
