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 6758EA0C4E; Mon, 12 Jul 2021 18:34:36 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 125DE4069E; Mon, 12 Jul 2021 18:34:36 +0200 (CEST) Received: from mail-io1-f52.google.com (mail-io1-f52.google.com [209.85.166.52]) by mails.dpdk.org (Postfix) with ESMTP id C41F14069D for ; Mon, 12 Jul 2021 18:34:34 +0200 (CEST) Received: by mail-io1-f52.google.com with SMTP id k16so23384737ios.10 for ; Mon, 12 Jul 2021 09:34:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Pbi0WMTCD7KJxNhyiN8CrxyzOzOQzIt7S0vJoKTLu9k=; b=HiYHhkcJpDZwm6o/Tk3016ZYMrPvglgDGWEN76jZe8mWXoyUx5EMCuiOh4IcjsntNO Yu7TWq2Rtd2RZUwJOdNbwHpvK+G1tgBLTx90caoyZxD6bSmDKj2T9t1CRhZWHZwpMeVg jQsCyC5qVx3UoHcJdZtncW5c8SG/zx0MWjOey/pXmQlMxASv/Y2HBYRDGeKrrpLdPsO6 95LrI62bHnsZSLPcDASG0dlEnMG9ygg6GH2N2wpIYKFc5A4wrGxOHEfLFL7Wsl22OUSC DNXaObPZHR3Jdz6JOhl0KT2AIPPTH3e/ZbAaj6u5i1dHPeu+iH5CXB9TOlh/FHz+Jk1C gKqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Pbi0WMTCD7KJxNhyiN8CrxyzOzOQzIt7S0vJoKTLu9k=; b=nqKhfdop+xBNh+0IusYAMObTctKxSro3hQ9UNBMtJW6SQjsh4hmzax3A+n4cqYG3LR mvLNt29LOkcp6IN9gVAvGG0oStWP5ed0KsQjAhagk0MdfT4dDTwHoiCH62jFEAnP/p+R +zzK1AqlZuBhLQWFsOf2MSUtoUBA7E3NnLUuBjf1F3V5cCe/CFU8xtbTrAaNfUAy4XAp usgTsW3od0jXU/Xn3dMyDOhUAKIE1Aq3cYcNKHzqua5GhzGENCgk4SyFqTLbxDNTKOob 5X8GDtzOu80wV63//eLTEqx8M/fBZMQIVfjFcp8xHSrWK0Wiv334gTCAuVR8ND91dEpB Dzfg== X-Gm-Message-State: AOAM532qLAHG9BagZ9YXaXdR8jsfCoIy99XH0qjDrui5E7HPLbV6SPdd 1csW40re/nCXEwsPm2LrkSucY3PwYzaBmZrG8mw= X-Google-Smtp-Source: ABdhPJwKh2+QrkKKauzFZs6kyZ1ik4kskTqAVfWFwuulIU0aMeNclvISKRLdPJLr+NtiqHNuUiCaiSM5241ZoMM2ZXY= X-Received: by 2002:a02:7b22:: with SMTP id q34mr43241343jac.133.1626107673927; Mon, 12 Jul 2021 09:34:33 -0700 (PDT) MIME-Version: 1.0 References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1625995556-41473-1-git-send-email-fengchengwen@huawei.com> In-Reply-To: From: Jerin Jacob Date: Mon, 12 Jul 2021 22:04:07 +0530 Message-ID: To: Bruce Richardson Cc: Chengwen Feng , Thomas Monjalon , Ferruh Yigit , Jerin Jacob , dpdk-dev , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Nipun Gupta , Hemant Agrawal , Maxime Coquelin , Honnappa Nagarahalli , David Marchand , Satananda Burla , Prasun Kapoor , "Ananyev, Konstantin" , liangma@liangbit.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v2] 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 Mon, Jul 12, 2021 at 7:02 PM Bruce Richardson wrote: > > On Mon, Jul 12, 2021 at 03:29:27PM +0530, Jerin Jacob wrote: > > On Sun, Jul 11, 2021 at 2:59 PM Chengwen Feng wrote: > > > > > > This patch introduce 'dmadevice' which is a generic type of DMA > > > device. > > > > > > The APIs of dmadev library exposes some generic operations which can > > > enable configuration and I/O with the DMA devices. > > > > > > Signed-off-by: Chengwen Feng > > > --- > > > +/** > > > + * @warning > > > + * @b EXPERIMENTAL: this API may change without prior notice. > > > + * > > > + * Enqueue a fill operation onto the virtual DMA channel. > > > + * > > > + * This queues up a fill operation to be performed by hardware, but does not > > > + * trigger hardware to begin that operation. > > > + * > > > + * @param dev_id > > > + * The identifier of the device. > > > + * @param vchan > > > + * The identifier of virtual DMA channel. > > > + * @param pattern > > > + * The pattern to populate the destination buffer with. > > > + * @param dst > > > + * The address of the destination buffer. > > > + * @param length > > > + * The length of the destination buffer. > > > + * @param flags > > > + * An flags for this operation. > > > + * > > > + * @return > > > + * - 0..UINT16_MAX: index of enqueued copy job. > > > > fill job > > > > > + * - <0: Error code returned by the driver copy function. > > > + */ > > > +__rte_experimental > > > +static inline int > > > +rte_dmadev_fill(uint16_t dev_id, uint16_t vchan, uint64_t pattern, > > > + rte_iova_t dst, uint32_t length, uint64_t flags) > > > +{ > > > + struct rte_dmadev *dev = &rte_dmadevices[dev_id]; > > > +#ifdef RTE_DMADEV_DEBUG > > > + RTE_DMADEV_VALID_DEV_ID_OR_ERR_RET(dev_id, -EINVAL); > > > + RTE_FUNC_PTR_OR_ERR_RET(*dev->fill, -ENOTSUP); > > > + if (vchan >= dev->data->dev_conf.max_vchans) { > > > + RTE_DMADEV_LOG(ERR, "Invalid vchan %d\n", vchan); > > > + return -EINVAL; > > > + } > > > +#endif > > > + return (*dev->fill)(dev, vchan, pattern, dst, length, flags); > > > +} > > > + > > > > > +/** > > > + * @warning > > > + * @b EXPERIMENTAL: this API may change without prior notice. > > > + * > > > + * Returns the number of operations that have been successfully completed. > > > + * > > > + * @param dev_id > > > + * The identifier of the device. > > > + * @param vchan > > > + * The identifier of virtual DMA channel. > > > + * @param nb_cpls > > > + * The maximum number of completed operations that can be processed. > > > + * @param[out] last_idx > > > + * The last completed operation's index. > > > + * If not required, NULL can be passed in. > > > > This means the driver will be tracking the last index. > > > > Driver will be doing this anyway, no, since it needs to ensure we don't Yes. > wrap around? > > > Is that mean, the application needs to call this API periodically to > > consume the completion slot. > > I.e up to 64K (UINT16_MAX) outstanding jobs are possible. If the > > application fails to call this > > >64K outstand job then the subsequence enqueue will fail. > > Well, given that there will be a regular enqueue ring which will probably > be <= 64k in size, the completion call will need to be called frequently > anyway. I don't think we need to document this restriction as it's fairly > understood that you can't go beyond the size of the ring without cleanup. See below. > > > > > If so, we need to document this. > > > > One of the concerns of keeping UINT16_MAX as the limit is the > > completion memory will always not in cache. > > On the other hand, if we make this size programmable. it may introduce > > complexity in the application. > > > > Thoughts? > > The reason for using powers-of-2 sizes, e.g. 0 .. UINT16_MAX, is that the > ring can be any other power-of-2 size and we can index it just by masking. > In the sample app for dmadev, I expect the ring size used to be set the > same as the dmadev enqueue ring size, for simplicity. No question on not using power of 2. Aligned on that. At least in our HW, the size of the ring is rte_dmadev_vchan_conf::nb_desc. But completion happens in _different_ memory space. Currently, we are allocating UINT16_MAX entries to hold that. That's where cache miss aspects of completion aspects came. In your case, Is completion happens in the same ring memory(looks like one bit in job desc represents the job completed or not) ? And when application calls rte_dmadev_completed(), You are converting UINT16_MAX based index to rte_dmadev_vchan_conf::nb_desc. Right? > > In fact, I was thinking that in later versions we may be able to include > some macros to help make this whole process easier, of converting indexes > to arbitrary data structures. [The reason for using macros is so that the > actual rings we are indexing can be of user-defined type, rather than just > a ring of pointers]. > > /Bruce