From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1BE85A04DC; Tue, 20 Oct 2020 17:35:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F3E1DAD4A; Tue, 20 Oct 2020 17:35:03 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id C7E24ACDF for ; Tue, 20 Oct 2020 17:35:01 +0200 (CEST) IronPort-SDR: ixz7XK1NFpXhdIPp6QldR3ohA+umdjo453v/xnlaXFpzNdGZDW2n8PRSVnoBMvC/ETyJiBVuw4 wDNG5G7Y2vYw== X-IronPort-AV: E=McAfee;i="6000,8403,9779"; a="167319790" X-IronPort-AV: E=Sophos;i="5.77,397,1596524400"; d="scan'208";a="167319790" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2020 08:34:57 -0700 IronPort-SDR: g41M24dYykFJt2VvL2NQ7iTLU+KdGr+QCdtEQDt3hyGEPCeHw6joygPziEVRSnaWWfIW4Lg6Ws KnkS8YdF1V0A== X-IronPort-AV: E=Sophos;i="5.77,397,1596524400"; d="scan'208";a="465969585" Received: from bricha3-mobl.ger.corp.intel.com ([10.213.248.1]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 20 Oct 2020 08:34:55 -0700 Date: Tue, 20 Oct 2020 16:34:51 +0100 From: Bruce Richardson To: "McDaniel, Timothy" Cc: Jerin Jacob , "Mcnamara, John" , "Kovacevic, Marko" , Ray Kinsella , Neil Horman , dpdk-dev , "Carrillo, Erik G" , "Eads, Gage" , "Van Haaren, Harry" , Jerin Jacob , Thomas Monjalon Message-ID: <20201020153451.GI558@bricha3-MOBL.ger.corp.intel.com> References: <1599855987-25976-2-git-send-email-timothy.mcdaniel@intel.com> <1602958879-8558-1-git-send-email-timothy.mcdaniel@intel.com> <1602958879-8558-2-git-send-email-timothy.mcdaniel@intel.com> <20201019083337.GA649@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2 01/22] event/dlb2: add documentation and meson build infrastructure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Oct 20, 2020 at 04:17:13PM +0100, McDaniel, Timothy wrote: > > > > -----Original Message----- > > From: Bruce Richardson > > Sent: Monday, October 19, 2020 3:34 AM > > To: Jerin Jacob > > Cc: McDaniel, Timothy ; Mcnamara, John > > ; Kovacevic, Marko > > ; Ray Kinsella ; Neil Horman > > ; dpdk-dev ; Carrillo, Erik G > > ; Eads, Gage ; Van Haaren, > > Harry ; Jerin Jacob ; > > Thomas Monjalon > > 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 > > > 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 > > > > Reviewed-by: Gage Eads > > > > --- > > > > 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. > > > > > > > > > + > > > > > > --- 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