From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 6357EA0C4C;
	Mon, 12 Jul 2021 15:32:48 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id DE6214069D;
	Mon, 12 Jul 2021 15:32:47 +0200 (CEST)
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by mails.dpdk.org (Postfix) with ESMTP id 9515D40685
 for <dev@dpdk.org>; Mon, 12 Jul 2021 15:32:46 +0200 (CEST)
X-IronPort-AV: E=McAfee;i="6200,9189,10042"; a="273814575"
X-IronPort-AV: E=Sophos;i="5.84,234,1620716400"; d="scan'208";a="273814575"
Received: from orsmga004.jf.intel.com ([10.7.209.38])
 by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 12 Jul 2021 06:32:45 -0700
X-IronPort-AV: E=Sophos;i="5.84,234,1620716400"; d="scan'208";a="561746987"
Received: from bricha3-mobl.ger.corp.intel.com ([10.252.5.145])
 by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA;
 12 Jul 2021 06:32:41 -0700
Date: Mon, 12 Jul 2021 14:32:34 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Jerin Jacob <jerinjacobk@gmail.com>
Cc: Chengwen Feng <fengchengwen@huawei.com>,
 Thomas Monjalon <thomas@monjalon.net>,
 Ferruh Yigit <ferruh.yigit@intel.com>,
 Jerin Jacob <jerinj@marvell.com>, dpdk-dev <dev@dpdk.org>,
 Morten =?iso-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>,
 Nipun Gupta <nipun.gupta@nxp.com>, Hemant Agrawal <hemant.agrawal@nxp.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>,
 Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>,
 David Marchand <david.marchand@redhat.com>,
 Satananda Burla <sburla@marvell.com>, Prasun Kapoor <pkapoor@marvell.com>,
 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, liangma@liangbit.com
Message-ID: <YOxEclYFxI3fh9Je@bricha3-MOBL.ger.corp.intel.com>
References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com>
 <1625995556-41473-1-git-send-email-fengchengwen@huawei.com>
 <CALBAE1M+9+0Lbn3aYD1Y7CTLk0u6DMc6T7KdUkyP5vWQz4dXBg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CALBAE1M+9+0Lbn3aYD1Y7CTLk0u6DMc6T7KdUkyP5vWQz4dXBg@mail.gmail.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Jul 12, 2021 at 03:29:27PM +0530, Jerin Jacob wrote:
> On Sun, Jul 11, 2021 at 2:59 PM Chengwen Feng <fengchengwen@huawei.com> 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 <fengchengwen@huawei.com>
> > ---
> > +/**
> > + * @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
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.

> 
> 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.

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