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 36C93A0C48; Tue, 13 Jul 2021 10:59:37 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1D884411BD; Tue, 13 Jul 2021 10:59:37 +0200 (CEST) Received: from mail-io1-f54.google.com (mail-io1-f54.google.com [209.85.166.54]) by mails.dpdk.org (Postfix) with ESMTP id 70AA44069E for ; Tue, 13 Jul 2021 10:59:36 +0200 (CEST) Received: by mail-io1-f54.google.com with SMTP id g22so26273592iom.1 for ; Tue, 13 Jul 2021 01:59:36 -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=gGaArWMUGHJXIR+QRJG4Zlx5vidBvmNVoO7ZttKNpno=; b=Pp7Dn50K1umVIX3JYbK6i+ljatdA8NM+/ztCLvl6f4+Xp4LrxSP0dc2v6poxNYKV74 RfDdE+6SYHW5/rYqKuXtverzRz1DMepH25cTLI4SBwW+G4YXCDPmn1Je97ShJvqcSPOn F6mnlPnP3SjA+TMJj+tOfCmaZp3DlI1qXalqT1EjQNupVD6x0xANTOwOHiDVEglr6UjI AFs0zwzBQLpTFIviY0sR0+xPd1ymNE5hgbleDYhhcm14S0ZGVP1Sn2tlo3082PzpOP8t FxYQ0WwGnN6RWlBvodR+3Q4HUU9PWBvo4FzCtxafpbrMLACDYjSSK6+4kjt+mqTDqYd4 Lehg== 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=gGaArWMUGHJXIR+QRJG4Zlx5vidBvmNVoO7ZttKNpno=; b=lXZc++xzrr0oP1OGWh67qOHrdVBjwBXi7lC+SZcwzJJbC6vrMVJDiLj3vUarVtrzXM vYfsbtmeMm1ltDNQ/jbwXvTh/v59sSqYU3gLW3Buq595P+hnYcaGA/+b7cOQwC25N5Gi /++Oj1b/EIs1IgYq3aJl8JvXXYDzyNpkLOCS0iELW/d+RwwcY0t7XnYEWrXofMB3d7hz yp1uqZnmKt8HXmflV0rs256/OwdBrhZW1z5RaARw5+obRlrF/GWrJ1/WdUZF1OKxDrfz 17s2j/3Fm4XSVl8IAlWWoyGVla6RDSha01+LF4In6svDE2ncxpyOJtKRohV6CF+TX9LF /l4A== X-Gm-Message-State: AOAM530lGkm1uKFa42W7Grfcs/RYkDUUyrvzVMcn8pIq6L37wIcm4d/i TDgzNMCqNBTu3cdVfEzNkjYs7We1gr6iF7Ag9x4= X-Google-Smtp-Source: ABdhPJyEE1R+xB2/RHy7LfTbnelU9bS2ckVUu5/pamxPsIReC/hAXJQWWf75tUgIPt4rWWfFBGRe4V31RyaW/LIrMEg= X-Received: by 2002:a5e:a816:: with SMTP id c22mr2488324ioa.94.1626166775737; Tue, 13 Jul 2021 01:59:35 -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: Tue, 13 Jul 2021 14:29:09 +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 10:30 PM Bruce Richardson wrote: > > On Mon, Jul 12, 2021 at 10:04:07PM +0530, Jerin Jacob wrote: > > 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. > > Depending on HW, our completions can be written back to a separate memory > area - a completion ring, if you will - but I've generally found it works > as well to reuse the enqueue ring for that purpose. However, with a > separate memory area for completions, why do you need to allocate 64K > entries for the completions? Would nb_desc entries not be enough? Is that > to allow the user to have more than nb_desc jobs outstanding before calling > "get_completions" API? Yes. That's what I thought. Thats where my question on what is the max number of outstanding completions. I thought it can be up to 64K. Agree to keep it implementation-specific and not need to highlight this in the documentation. > > > > > 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? > > Yes, we are masking to do that. Actually, for simplicity and perf we should > only allow power-of-2 ring sizes. Having to use modulus instead of masking > could be a problem. [Alternatively, I suppose we can allow drivers to round > up the ring sizes to the next power of 2, but I prefer just documenting it > as a limitation]. OK. > > /Bruce