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 06B2AA0547; Thu, 9 Sep 2021 16:19:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9E4FA40E46; Thu, 9 Sep 2021 16:19:18 +0200 (CEST) Received: from new4-smtp.messagingengine.com (new4-smtp.messagingengine.com [66.111.4.230]) by mails.dpdk.org (Postfix) with ESMTP id 4A52040041 for ; Thu, 9 Sep 2021 16:19:17 +0200 (CEST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailnew.nyi.internal (Postfix) with ESMTP id 3BE1F580BC8; Thu, 9 Sep 2021 10:19:14 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Thu, 09 Sep 2021 10:19:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= /2pMVS+uRLMX8yvrwFndQaWgpuH+ykpSmokU2/b/gg4=; b=COottAdtBFubRtNo O52d82r2khRorfkBK7aIx8bivxlc4ommMZelxxs+iLCSk19wtvh3JAnDNUMQVo6/ Aeh5SjsMk1mbqVo4gnDUJUzpK1KZRTH0w48qnB7RlJIBdK0stXXrrT5R/kdfSxNG B9BXAd0LJ7bJ4nuvzck7eqfDZ2m1xseEMzpFsRhatsNO8Mw0mTLSx9jw+yrt6hQT CkyjQqfMWTpJZy0SBMVdhllRkmWCwpg8ej0QYZBzKaKllAtwQgtcoUM01uOojY1W slurjTTMVecSAE5e/4vSuHw0oa5/KK38ptSF2vwLYdYUCIBEWN6KoO34jJPZBn9R PdfDew== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=/2pMVS+uRLMX8yvrwFndQaWgpuH+ykpSmokU2/b/g g4=; b=vrjQMwpUHJIVaJdThRWBaA/1qlA2PTpXy1WBOOlUBeEQkLx9aeM0E1jYc iAMAdo/ruXPFoCHlTuwM9sLvaQoDoohWy9kaS+Ud8IaKpfILr9DpNaziGkXIbY93 LCBtCN6w7Rn4DcgF26U5dk19sGSMnki164f8cD6ehNXifqbuUBXqQwSl9D60Js/r nXd9IfVbD2CIoQuAcEn13fp83In7zg+/7Sddx2XcIF2qD4CKrMUwcK5rdlKBFH6z J4+pqBGC875sjC3sZV0Tu6BySyGpb0BfDCe28l+mVV9S5LMYDW9AzY9+2aCO8dsJ 4WG3hhat3otL70b5MS629f4EYRlCg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefledgjeehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpefhvffufffkjghfggfgtgesthfure dttddtvdenucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshes mhhonhhjrghlohhnrdhnvghtqeenucggtffrrghtthgvrhhnpeehtdevffejtddvffehhe dutdffleehudfftdekheehieejleehuefhvdeludfhffenucffohhmrghinhepkhgvrhhn vghlrdhorhhgpdhnohhunhhplhhushdrnhgvthdpughpughkrdhorhhgnecuvehluhhsth gvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhn jhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Sep 2021 10:19:10 -0400 (EDT) From: Thomas Monjalon To: fengchengwen Cc: bruce.richardson@intel.com, dev@dpdk.org, ferruh.yigit@intel.com, jerinj@marvell.com, jerinjacobk@gmail.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org, mb@smartsharesystems.com, nipun.gupta@nxp.com, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com, honnappa.nagarahalli@arm.com, david.marchand@redhat.com, sburla@marvell.com, pkapoor@marvell.com, konstantin.ananyev@intel.com, conor.walsh@intel.com, kevin.laatz@intel.com Date: Thu, 09 Sep 2021 16:19:07 +0200 Message-ID: <12745898.oiGctkSRRo@thomas> In-Reply-To: <76b99e6f-c732-1096-d0ee-3d92de87746e@huawei.com> References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1855788.BnNMhiXu0z@thomas> <76b99e6f-c732-1096-d0ee-3d92de87746e@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v21 1/7] dmadev: introduce DMA device library public APIs 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" 09/09/2021 15:33, fengchengwen: > On 2021/9/9 18:33, Thomas Monjalon wrote: > > 07/09/2021 14:56, Chengwen Feng: > >> + * The DMA controller could have multiple HW-DMA-channels (aka. HW-DMA-queues), > >> + * each HW-DMA-channel should be represented by a dmadev. > >> + * > >> + * The dmadev could create multiple virtual DMA channels, each virtual DMA > >> + * channel represents a different transfer context. The DMA operation request > >> + * must be submitted to the virtual DMA channel. e.g. Application could create > >> + * virtual DMA channel 0 for memory-to-memory transfer scenario, and create > >> + * virtual DMA channel 1 for memory-to-device transfer scenario. > > > > What is the benefit of virtual channels compared to have separate dmadevs > > for the same HW channel? > > This design is from the previous discussion [1]. If a dmadev is created for each > virtual channel, there are many associations between the dmadevs. For example, > channel operations of some devices need to interact with the kernel, the corresponding > kernel operation handles need to be shared among multiple dmadevs. It's going to get > more complicated. > > [1] https://lore.kernel.org/dpdk-dev/c4a0ee30-f7b8-f8a1-463c-8eedaec82aea@huawei.com/ OK thanks for the explanation. [...] > >> + * be released by rte_dmadev_pmd_release() during the PCI/SoC device removing > >> + * phase. > > > > Again what is this phase? > > I think freeing should be done only via the "close" function. > > OK > The allocate/release will be reconstructed with reference to rte_eth_dev_pci_generic_probe. You shouldn't always mimic ethdev, there can be some misconceptions :) I think you don't need PCI specific helper. [...] > >> + * @note If the dmadev works in silent mode (@see RTE_DMADEV_CAPA_SILENT), > >> + * application does not invoke the above two completed APIs. > >> + * > >> + * About the ring_idx which enqueue APIs (e.g. rte_dmadev_copy() > >> + * rte_dmadev_fill()) returned, the rules are as follows: > > > > I feel a word is missing above. > > Can you point it out specifically ? > PS: I specifically examined by access https://www.nounplus.net/grammarcheck/ and found > it prompts the 'enqueue' to 'enqueues or enqueued'. After second read, I think it is a tense problem. What about "returned" -> "return" ? [...] > >> +/**< DMA device support memory-to-memory transfer. > >> + * > >> + * @see struct rte_dmadev_info::dev_capa > >> + */ > > > > It is preferred to have documentation before the item. > > Is this particularly strong? > I use postfix comment style for whole doxygen comments. I think it's better to maintain a unified > style. In general prefix comment is preferred. Postfix comments are OK for short comments on the same line. [...] > > This series add one file per patch. > > Instead it would be better to have groups of features per patch, > > meaning the implementation and the driver interface should be > > in the same patch. > > You can split like this: > > 1/ device allocation > > 2/ configuration and start/stop > > 3/ dataplane functions > > > > I would suggest 2 more patches: > > 4/ event notification > > see https://patches.dpdk.org/project/dpdk/patch/20210730135533.417611-3-thomas@monjalon.net/ > > 5/ multi-process > > see https://patches.dpdk.org/project/dpdk/patch/20210730135533.417611-5-thomas@monjalon.net/ > > The split mode you recommend is better. > But is this particularly strong ? Yes, that's really better. > Because many acked-by and reviewed-by base on stand-alone file. > Does this division mean that a new acked/reviewed ? You can keep the acks which are commong to the first 4 patches I guess and ask for re-ack to others.