On 2021/7/16 0:33, Bruce Richardson wrote:

On Fri, Jul 16, 2021 at 12:04:33AM +0800, fengchengwen wrote:

quoted

@burce, jerin  Some unmodified review comments are returned here:

[snip]

quoted

2.  COMMENT: > + * @see struct rte_dmadev_info::dev_capa

quoted

+ */

Drop this flag as unnecessary. All devices either always provide ordering
guarantee - in which case it's a no-op - or else support the flag.

REPLY: I prefer define it, it could let user know whether support fence.

I don't see it that way. The flag is pointless because the application
can't use it to make any decisions. If two operations require ordering the
application must use the fence flag because if the device doesn't guarantee
ordering it's necessary, and if the device does guarantee ordering it's
better and easier to just specify the flag than to put in code branches.
Having this as a capability is just going to confuse the user - better to
just say that if you need ordering, put in a fence.

If driver don't support fence, and application set the fence flag, What's
driving behavior like? return error or implement fence at driver layer ?

If expose the fence capability to application, then application could decide
which option to use. e.g. commit the operations before the fence and make sure it completes,
or use another collaborative approach.

I think in this manner, the driver implementation can be simplified.

[snip]

quoted

4.
COMMENT: > +        * @see RTE_DMA_DEV_TO_MEM

quoted

+        * @see RTE_DMA_DEV_TO_DEV

Since we can set of only one direction per vchan . Should be we make
it as enum to
make it clear.

REPLY: May some devices support it. I think it's OK for future use.

+1
That may need a capability flag though, to indicate if a device supports
multi-direction in a single vchannel.

There are a lot of combinations, and I tend not to add a capability for multi-direction.
Currently, no device supports multiple directions, So can we delay that definition?

[snip]

thanks