From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 12A32A0C50; Fri, 16 Jul 2021 14:48:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EDC3440DDB; Fri, 16 Jul 2021 14:48:35 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 4CAA44067B for ; Fri, 16 Jul 2021 14:48:34 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10046"; a="197988352" X-IronPort-AV: E=Sophos;i="5.84,244,1620716400"; d="scan'208";a="197988352" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jul 2021 05:48:31 -0700 X-IronPort-AV: E=Sophos;i="5.84,244,1620716400"; d="scan'208";a="460741047" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.26.216]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 16 Jul 2021 05:48:28 -0700 Date: Fri, 16 Jul 2021 13:48:24 +0100 From: Bruce Richardson To: Jerin Jacob Cc: fengchengwen , Thomas Monjalon , Ferruh Yigit , Jerin Jacob , Andrew Rybchenko , dpdk-dev , Morten =?iso-8859-1?Q?Br=F8rup?= , Nipun Gupta , Hemant Agrawal , Maxime Coquelin , Honnappa Nagarahalli , David Marchand , Satananda Burla , Prasun Kapoor , "Ananyev, Konstantin" Message-ID: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1626363661-51103-1-git-send-email-fengchengwen@huawei.com> <0aa82990-566b-f471-129b-31fbc2410ecc@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v4] dmadev: introduce DMA device library X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 Fri, Jul 16, 2021 at 06:10:48PM +0530, Jerin Jacob wrote: > On Fri, Jul 16, 2021 at 8:34 AM fengchengwen wrote: > > > > On 2021/7/16 0:33, Bruce Richardson wrote: > > > On Fri, Jul 16, 2021 at 12:04:33AM +0800, fengchengwen wrote: > > >> @burce, jerin Some unmodified review comments are returned here: > > >> > > > > [snip] > > > > > > > >> 2. COMMENT: > + * @see struct rte_dmadev_info::dev_capa > > >>> + */ > > >> Drop this flag as unnecessary. All devices either always provide ordering > > >> guarantee - in which case it's a no-op - or else support the flag. > > >> > > >> REPLY: I prefer define it, it could let user know whether support fence. > > >> > > > I don't see it that way. The flag is pointless because the application > > > can't use it to make any decisions. If two operations require ordering the > > > application must use the fence flag because if the device doesn't guarantee > > > ordering it's necessary, and if the device does guarantee ordering it's > > > better and easier to just specify the flag than to put in code branches. > > > Having this as a capability is just going to confuse the user - better to > > > just say that if you need ordering, put in a fence. > > > > > > > If driver don't support fence, and application set the fence flag, What's > > driving behavior like? return error or implement fence at driver layer ? > > > > If expose the fence capability to application, then application could decide > > which option to use. e.g. commit the operations before the fence and make sure it completes, > > or use another collaborative approach. > > > > I think in this manner, the driver implementation can be simplified. > > > > [snip] > > > > >> 4. > > >> COMMENT: > + * @see RTE_DMA_DEV_TO_MEM > > >>> + * @see RTE_DMA_DEV_TO_DEV > > >> Since we can set of only one direction per vchan . Should be we make > > >> it as enum to > > >> make it clear. > > >> > > >> REPLY: May some devices support it. I think it's OK for future use. > > >> > > > +1 > > > That may need a capability flag though, to indicate if a device supports > > > multi-direction in a single vchannel. > > > > There are a lot of combinations, and I tend not to add a capability for multi-direction. > > Currently, no device supports multiple directions, So can we delay that definition? > > Yes. IMO, we need to change the comment from "Set of supported > transfer directions" to > "Transfer direction" > > If channel supports multiple directions, then in fastpath we need > another flag to select > which direction to use. > > IMO, Since none of the devices supports this mode, I think, we should > change the comment > to "Transfer direction" > Ok for me.