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 C4880A0A0C; Fri, 2 Jul 2021 15:45:22 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4955C41363; Fri, 2 Jul 2021 15:45:22 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id F0DBD41353 for ; Fri, 2 Jul 2021 15:45:19 +0200 (CEST) Received: from dggemv703-chm.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4GGbq45r86zZjqJ; Fri, 2 Jul 2021 21:42:08 +0800 (CST) Received: from dggpeml500024.china.huawei.com (7.185.36.10) by dggemv703-chm.china.huawei.com (10.3.19.46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Fri, 2 Jul 2021 21:45:17 +0800 Received: from [127.0.0.1] (10.40.190.165) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Fri, 2 Jul 2021 21:45:16 +0800 To: Jerin Jacob CC: Bruce Richardson , Jerin Jacob , =?UTF-8?Q?Morten_Br=c3=b8rup?= , Nipun Gupta , "Thomas Monjalon" , Ferruh Yigit , dpdk-dev , Hemant Agrawal , "Maxime Coquelin" , Honnappa Nagarahalli , David Marchand , Satananda Burla , Prasun Kapoor References: <1623763327-30987-1-git-send-email-fengchengwen@huawei.com> <25d29598-c26d-8497-2867-9b650c79df49@huawei.com> <3db2eda0-4490-2b8f-c65d-636bcf794494@huawei.com> From: fengchengwen Message-ID: Date: Fri, 2 Jul 2021 21:45:15 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.40.190.165] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] dmadev discussion summary 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 2021/7/1 23:01, Jerin Jacob wrote: >> [key point]: >> ----------- ----------- >> | channel | | channel | >> ----------- ----------- >> \ / >> \ / >> \ / >> ------------ >> | HW-queue | >> ------------ >> | >> -------- >> |rawdev| >> -------- >> 1) User could create one channel by init context(dpi_dma_queue_ctx_s), >> this interface is not standardized and needs to be implemented by >> users. >> 2) Different channels can support different transmissions, e.g. one for >> inner m2m, and other for inbound copy. >> >> Overall, I think the 'channel' is similar the 'virt-queue' of dpaa2_qdma. >> The difference is that dpaa2_qdma supports multiple hardware queues. The >> 'channel' has following > > If dpaa2_qdma supports more than one HW queue, I think, it is good to > have the queue notion > in DPDK just like other DPDK device classes. It will be good to have > confirmation from dpaa2 folks, @Hemant Agrawal, > if there are really more than 1 HW queue in dppa device. > > > IMO, Channel is a better name than a virtual queue. The reason is, > virtual queue is more > implementation-specific notation. No need to have this in API specification. > In the DPDK framework, many data-plane API names contain queues. e.g. eventdev/crypto.. The concept of virt queues has continuity. >> [dma_copy/memset/sg]: all has vq_id input parameter. >> Note: I notice dpaa can't support single and sg in one virt-queue, and >> I think it's maybe software implement policy other than HW >> restriction because virt-queue could share the same HW-queue. >> Here we use vq_id to tackle different scenario, like local-to-local/ >> local-to-host and etc. > > IMO, The index representation has an additional overhead as one needs > to translate it > to memory pointer. I prefer to avoid by having object handle and use > _lookup() API get to make it work > in multi-process cases to avoid the additional indirection. Like mempool object. > This solution was first considered, similar to rte_hash returning a handle. It is not intuitive and has no obvious performance advantage. The number of jump times of the data-plane API index-driven callback function is not optimized. >> 5) And the dmadev public data-plane API (just prototype): >> dma_cookie_t rte_dmadev_memset(dev, vq_id, pattern, dst, len, flags) >> -- flags: used as an extended parameter, it could be uint32_t >> dma_cookie_t rte_dmadev_memcpy(dev, vq_id, src, dst, len, flags) >> dma_cookie_t rte_dmadev_memcpy_sg(dev, vq_id, sg, sg_len, flags) >> -- sg: struct dma_scatterlist array >> uint16_t rte_dmadev_completed(dev, vq_id, dma_cookie_t *cookie, >> uint16_t nb_cpls, bool *has_error) >> -- nb_cpls: indicate max process operations number >> -- has_error: indicate if there is an error >> -- return value: the number of successful completed operations. >> -- example: >> 1) If there are already 32 completed ops, and 4th is error, and >> nb_cpls is 32, then the ret will be 3(because 1/2/3th is OK), and >> has_error will be true. >> 2) If there are already 32 completed ops, and all successful >> completed, then the ret will be min(32, nb_cpls), and has_error >> will be false. >> 3) If there are already 32 completed ops, and all failed completed, >> then the ret will be 0, and has_error will be true. > > +1. IMO, it is better to call ring_idx instead of a cookie. To enforce > that it the ring index. > >> uint16_t rte_dmadev_completed_status(dev_id, vq_id, dma_cookie_t *cookie, >> uint16_t nb_status, uint32_t *status) >> -- return value: the number of failed completed operations. > > See above. Here we are assuming it is an index otherwise we need to > pass an array > cookies. > >> And here I agree with Morten: we should design API which adapts to DPDK >> service scenarios. So we don't support some sound-cards DMA, and 2D memory >> copy which mainly used in video scenarios. >> 6) The dma_cookie_t is signed int type, when <0 it mean error, it's >> monotonically increasing base on HW-queue (other than virt-queue). The >> driver needs to make sure this because the damdev framework don't manage >> the dma_cookie's creation. > > +1 and see above. > >> 7) Because data-plane APIs are not thread-safe, and user could determine >> virt-queue to HW-queue's map (at the queue-setup stage), so it is user's >> duty to ensure thread-safe. > > +1. But I am not sure how easy for the fast-path application to have this logic, > Instead, I think, it is better to tell the capa for queue by driver > and in channel configuration, > the application can request for requirement (Is multiple producers enq > to the same HW queue or not). > Based on the request, the implementation can pick the correct function > pointer for enq.(lock vs lockless version if HW does not support > lockless) > already redesigned. Please check the latest patch. > >> 8) One example: >> vq_id = rte_dmadev_queue_setup(dev, config.{HW-queue-index=x, opaque}); >> if (vq_id < 0) { >> // create virt-queue failed >> return; >> } >> // submit memcpy task >> cookit = rte_dmadev_memcpy(dev, vq_id, src, dst, len, flags); >> if (cookie < 0) { >> // submit failed >> return; >> } >> // get complete task >> ret = rte_dmadev_completed(dev, vq_id, &cookie, 1, has_error); >> if (!has_error && ret == 1) { >> // the memcpy successful complete >> } > > +1 > >> 9) As octeontx2_dma support sg-list which has many valid buffers in >> dpi_dma_buf_ptr_s, it could call the rte_dmadev_memcpy_sg API. > > +1 > >> 10) As ioat, it could delcare support one HW-queue at dev_configure stage, and >> only support create one virt-queue. >> 11) As dpaa2_qdma, I think it could migrate to new framework, but still wait >> for dpaa2_qdma guys feedback. >> 12) About the prototype src/dst parameters of rte_dmadev_memcpy API, we have >> two candidates which are iova and void *, how about introduce dma_addr_t >> type which could be va or iova ? > > I think, conversion looks ugly, better to have void * and share the > constraints of void * > as limitation/capability using flag. So that driver can update it. > already change to void * > >> > > . >