2022-06-22 20:49:31

by Ben Walker

[permalink] [raw]
Subject: [PATCH v4 00/15] dmaengine: Support polling for out of order completions

This series adds support for polling async transactions for completion
even if interrupts are disabled and transactions can complete out of
order.

To do this, all DMA client assumptions about the behavior of
dma_cookie_t have to be removed. Prior to this series, dma_cookie_t was
a monotonically increasing integer and cookies could be compared to one
another to determine if earlier operations had completed (up until the
cookie wraps around, then it would break).

Fortunately, only one out of the many, many DMA clients had any
dependency on dma_cookie_t being anything more than an opaque handle.
This is the pxa_camera driver and it is dealt with in patch 7 of this
series.

The series also does some API clean up and documents how dma_cookie_t
should behave (i.e. there are no rules, it's just a handle).

This closes out by adding support for .device_tx_status() to the idxd
driver and then reverting the DMA_OUT_OF_ORDER patch that previously
allowed idxd to opt-out of support for polling, which I think is a nice
overall simplification to the dmaengine API.

Changes since version 3:
- Fixed Message-Id in emails. Sorry they were all stripped! Won't
happen again.

Changes since version 2:
- None. Rebased as requested without conflict.

Changes since version 1:
- Broke up the change to remove dma_async_is_tx_complete into a single
patch for each driver
- Renamed dma_async_is_tx_complete to dmaengine_async_is_tx_complete.

Ben Walker (15):
dmaengine: Remove dma_async_is_complete from client API
dmaengine: Move dma_set_tx_state to the provider API header
dmaengine: Add dmaengine_async_is_tx_complete
crypto: stm32/hash: Use dmaengine_async_is_tx_complete
media: omap_vout: Use dmaengine_async_is_tx_complete
rapidio: Use dmaengine_async_is_tx_complete
media: pxa_camera: Use dmaengine_async_is_tx_complete
dmaengine: Remove dma_async_is_tx_complete
dmaengine: Remove last, used from dma_tx_state
dmaengine: Providers should prefer dma_set_residue over
dma_set_tx_state
dmaengine: Remove dma_set_tx_state
dmaengine: Add provider documentation on cookie assignment
dmaengine: idxd: idxd_desc.id is now a u16
dmaengine: idxd: Support device_tx_status
dmaengine: Revert "cookie bypass for out of order completion"

Documentation/driver-api/dmaengine/client.rst | 24 ++----
.../driver-api/dmaengine/provider.rst | 64 ++++++++------
drivers/crypto/stm32/stm32-hash.c | 3 +-
drivers/dma/amba-pl08x.c | 1 -
drivers/dma/at_hdmac.c | 3 +-
drivers/dma/dmaengine.c | 2 +-
drivers/dma/dmaengine.h | 12 ++-
drivers/dma/dmatest.c | 14 +--
drivers/dma/idxd/device.c | 1 +
drivers/dma/idxd/dma.c | 86 ++++++++++++++++++-
drivers/dma/idxd/idxd.h | 3 +-
drivers/dma/imx-sdma.c | 3 +-
drivers/dma/mmp_tdma.c | 3 +-
drivers/dma/mxs-dma.c | 3 +-
drivers/media/platform/intel/pxa_camera.c | 15 +++-
.../media/platform/ti/omap/omap_vout_vrfb.c | 2 +-
drivers/rapidio/devices/rio_mport_cdev.c | 3 +-
include/linux/dmaengine.h | 58 +------------
18 files changed, 164 insertions(+), 136 deletions(-)

Cc:[email protected]
Cc:[email protected]
Cc:[email protected]
Cc:[email protected]
--
2.35.1


2022-06-22 20:51:53

by Ben Walker

[permalink] [raw]
Subject: [PATCH v4 09/15] dmaengine: Remove last, used from dma_tx_state

Nothing uses these and they don't convey usable information.

Signed-off-by: Ben Walker <[email protected]>
---
drivers/dma/dmaengine.h | 4 ----
include/linux/dmaengine.h | 4 ----
2 files changed, 8 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index e72876a512a39..08c7bd7cfc229 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -74,8 +74,6 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
complete = chan->completed_cookie;
barrier();
if (state) {
- state->last = complete;
- state->used = used;
state->residue = 0;
state->in_flight_bytes = 0;
}
@@ -96,8 +94,6 @@ static inline void dma_set_tx_state(struct dma_tx_state *st,
if (!st)
return;

- st->last = last;
- st->used = used;
st->residue = residue;
}

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c34f21d19c423..e3e5311b6bb64 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -716,16 +716,12 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr
/**
* struct dma_tx_state - filled in to report the status of
* a transfer.
- * @last: last completed DMA cookie
- * @used: last issued DMA cookie (i.e. the one in progress)
* @residue: the remaining number of bytes left to transmit
* on the selected transfer for states DMA_IN_PROGRESS and
* DMA_PAUSED if this is implemented in the driver, else 0
* @in_flight_bytes: amount of data in bytes cached by the DMA.
*/
struct dma_tx_state {
- dma_cookie_t last;
- dma_cookie_t used;
u32 residue;
u32 in_flight_bytes;
};
--
2.35.1

2022-06-22 20:54:07

by Ben Walker

[permalink] [raw]
Subject: [PATCH v4 01/15] dmaengine: Remove dma_async_is_complete from client API

This is never actually used by any existing DMA clients. It is only
used, via dma_cookie_status, by providers.

Signed-off-by: Ben Walker <[email protected]>
---
Documentation/driver-api/dmaengine/client.rst | 5 ++--
drivers/dma/amba-pl08x.c | 1 -
drivers/dma/at_hdmac.c | 3 +-
drivers/dma/dmaengine.h | 10 ++++++-
include/linux/dmaengine.h | 28 ++-----------------
5 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index bfd057b21a000..85ecec2c40005 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -346,9 +346,8 @@ Further APIs
the documentation in include/linux/dmaengine.h for a more complete
description of this API.

- This can be used in conjunction with dma_async_is_complete() and
- the cookie returned from dmaengine_submit() to check for
- completion of a specific DMA transaction.
+ This can be used with the cookie returned from dmaengine_submit()
+ to check for completion of a specific DMA transaction.

.. note::

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index a4a794e62ac26..bd361aee07db8 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1536,7 +1536,6 @@ static void pl08x_free_chan_resources(struct dma_chan *chan)
}

/*
- * Code accessing dma_async_is_complete() in a tight loop may give problems.
* If slaves are relying on interrupts to signal completion this function
* must not be called with interrupts disabled.
*/
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 5a50423b7378e..5ec9a36074771 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1491,8 +1491,7 @@ static int atc_terminate_all(struct dma_chan *chan)
* @txstate: if not %NULL updated with transaction state
*
* If @txstate is passed in, upon return it reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * internal state.
*/
static enum dma_status
atc_tx_status(struct dma_chan *chan,
diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f00294..a2ce377e9ed0f 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -79,7 +79,15 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
state->residue = 0;
state->in_flight_bytes = 0;
}
- return dma_async_is_complete(cookie, complete, used);
+
+ if (complete <= used) {
+ if ((cookie <= complete) || (cookie > used))
+ return DMA_COMPLETE;
+ } else {
+ if ((cookie <= complete) && (cookie > used))
+ return DMA_COMPLETE;
+ }
+ return DMA_IN_PROGRESS;
}

static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b46b88e6aa0d1..ea6ec2666eb15 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1446,9 +1446,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
* @last: returns last completed cookie, can be NULL
* @used: returns last issued cookie, can be NULL
*
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
*/
static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1464,28 +1464,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
return status;
}

-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
- dma_cookie_t last_complete, dma_cookie_t last_used)
-{
- if (last_complete <= last_used) {
- if ((cookie <= last_complete) || (cookie > last_used))
- return DMA_COMPLETE;
- } else {
- if ((cookie <= last_complete) && (cookie > last_used))
- return DMA_COMPLETE;
- }
- return DMA_IN_PROGRESS;
-}
-
static inline void
dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
{
--
2.35.1

2022-06-22 20:55:29

by Ben Walker

[permalink] [raw]
Subject: [PATCH v4 05/15] media: omap_vout: Use dmaengine_async_is_tx_complete

Replace dma_async_is_tx_complete with dmaengine_async_is_tx_complete.
The previous API will be removed in favor of the new one.

Cc: [email protected]
Signed-off-by: Ben Walker <[email protected]>
---
drivers/media/platform/ti/omap/omap_vout_vrfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti/omap/omap_vout_vrfb.c b/drivers/media/platform/ti/omap/omap_vout_vrfb.c
index 0cfa0169875f0..b9d252d5ced7a 100644
--- a/drivers/media/platform/ti/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/ti/omap/omap_vout_vrfb.c
@@ -289,7 +289,7 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
vout->vrfb_dma_tx.tx_status == 1,
VRFB_TX_TIMEOUT);

- status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
+ status = dmaengine_async_is_tx_complete(chan, cookie);

if (vout->vrfb_dma_tx.tx_status == 0) {
pr_err("%s: Timeout while waiting for DMA\n", __func__);
--
2.35.1

2022-06-27 06:32:40

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v4 00/15] dmaengine: Support polling for out of order completions

Hi Ben,


On 22-06-22, 12:37, Ben Walker wrote:
> This series adds support for polling async transactions for completion
> even if interrupts are disabled and transactions can complete out of
> order.

Pls cc relevant mainatiners and list on cover as well please so that
they get the context of this work. Also, I notice some patches are cced
to lists but no maintainers, that needs to be updated... People may not
look, you need to make it easy for them...

--
~Vinod

2022-08-29 20:50:48

by Ben Walker

[permalink] [raw]
Subject: [PATCH v5 0/7] dmaengine: Support polling for out of order completions

This series adds support for polling async transactions for completion
even if interrupts are disabled and transactions can complete out of
order.

Prior to this series, dma_cookie_t was a monotonically increasing integer and
cookies could be compared to one another to determine if earlier operations had
completed (up until the cookie wraps around, then it would break). Now, cookies
are treated as opaque handles. The series also does some API clean up and
documents how dma_cookie_t should behave.

This closes out by adding support for .device_tx_status() to the idxd
driver and then reverting the DMA_OUT_OF_ORDER patch that previously
allowed idxd to opt-out of support for polling, which I think is a nice
overall simplification to the dmaengine API.

Changes since version 4:
- Rebased
- Removed updates to the various drivers that call dma_async_is_tx_complete.
These clean ups will be spun off into a separate patch series since they need
acks from other maintainers.

Changes since version 3:
- Fixed Message-Id in emails. Sorry they were all stripped! Won't
happen again.

Changes since version 2:
- None. Rebased as requested without conflict.

Changes since version 1:
- Broke up the change to remove dma_async_is_tx_complete into a single
patch for each driver
- Renamed dma_async_is_tx_complete to dmaengine_async_is_tx_complete.

Ben Walker (7):
dmaengine: Remove dma_async_is_complete from client API
dmaengine: Move dma_set_tx_state to the provider API header
dmaengine: Add dmaengine_async_is_tx_complete
dmaengine: Add provider documentation on cookie assignment
dmaengine: idxd: idxd_desc.id is now a u16
dmaengine: idxd: Support device_tx_status
dmaengine: Revert "cookie bypass for out of order completion"

Documentation/driver-api/dmaengine/client.rst | 24 ++----
.../driver-api/dmaengine/provider.rst | 64 ++++++++------
drivers/dma/dmaengine.c | 2 +-
drivers/dma/dmaengine.h | 21 ++++-
drivers/dma/dmatest.c | 14 +--
drivers/dma/idxd/device.c | 1 +
drivers/dma/idxd/dma.c | 86 ++++++++++++++++++-
drivers/dma/idxd/idxd.h | 3 +-
include/linux/dmaengine.h | 43 +++-------
9 files changed, 164 insertions(+), 94 deletions(-)

--
2.37.1

2022-08-29 20:51:25

by Ben Walker

[permalink] [raw]
Subject: [PATCH v5 3/7] dmaengine: Add dmaengine_async_is_tx_complete

This is the replacement for dma_async_is_tx_complete with two changes:
1) The name prefix is 'dmaengine' as per convention
2) It no longer reports the 'last' or 'used' cookie

Drivers should convert to using dmaengine_async_is_tx_complete.

Signed-off-by: Ben Walker <[email protected]>
---
Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
.../driver-api/dmaengine/provider.rst | 6 +++---
drivers/dma/dmaengine.c | 2 +-
drivers/dma/dmatest.c | 3 +--
include/linux/dmaengine.h | 16 ++++++++++++++++
5 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index 85ecec2c40005..9ae489a4ca97f 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -259,8 +259,8 @@ The details of these operations are:

dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)

- This returns a cookie can be used to check the progress of DMA engine
- activity via other DMA engine calls not covered in this document.
+ This returns a cookie that can be used to check the progress of a transaction
+ via dmaengine_async_is_tx_complete().

dmaengine_submit() will not start the DMA operation, it merely adds
it to the pending queue. For this, see step 5, dma_async_issue_pending.
@@ -339,23 +339,12 @@ Further APIs

.. code-block:: c

- enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
- dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
-
- This can be used to check the status of the channel. Please see
- the documentation in include/linux/dmaengine.h for a more complete
- description of this API.
+ enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
+ dma_cookie_t cookie)

This can be used with the cookie returned from dmaengine_submit()
to check for completion of a specific DMA transaction.

- .. note::
-
- Not all DMA engine drivers can return reliable information for
- a running DMA channel. It is recommended that DMA engine users
- pause or stop (via dmaengine_terminate_all()) the channel before
- using this API.
-
5. Synchronize termination API

.. code-block:: c
diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index ceac2a300e328..1d0da2777921d 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -539,10 +539,10 @@ where to put them)

dma_cookie_t

-- it's a DMA transaction ID that will increment over time.
+- it's a DMA transaction ID.

-- Not really relevant any more since the introduction of ``virt-dma``
- that abstracts it away.
+- The value can be chosen by the provider, or use the helper APIs
+ such as dma_cookie_assign() and dma_cookie_complete().

DMA_CTRL_ACK

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index c741b6431958c..2816b8f492dab 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)

dma_async_issue_pending(chan);
do {
- status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
+ status = dmaengine_async_is_tx_complete(chan, cookie);
if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
dev_err(chan->device->dev, "%s: timeout!\n", __func__);
return DMA_ERROR;
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 9fe2ae7943169..dde7b9b626336 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -831,8 +831,7 @@ static int dmatest_func(void *data)
done->done,
msecs_to_jiffies(params->timeout));

- status = dma_async_is_tx_complete(chan, cookie, NULL,
- NULL);
+ status = dmaengine_async_is_tx_complete(chan, cookie);
}

if (!done->done) {
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 5ae881729b620..0ee21887b3924 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1426,6 +1426,8 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
* @last: returns last completed cookie, can be NULL
* @used: returns last issued cookie, can be NULL
*
+ * Note: This is deprecated. Use dmaengine_async_is_tx_complete instead.
+ *
* If @last and @used are passed in, upon return they reflect the most
* recently submitted (used) cookie and the most recently completed
* cookie.
@@ -1444,6 +1446,20 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
return status;
}

+/**
+ * dmaengine_async_is_tx_complete - poll for transaction completion
+ * @chan: DMA channel
+ * @cookie: transaction identifier to check status of
+ *
+ */
+static inline enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
+ dma_cookie_t cookie)
+{
+ struct dma_tx_state state;
+
+ return chan->device->device_tx_status(chan, cookie, &state);
+}
+
#ifdef CONFIG_DMA_ENGINE
struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
--
2.37.1

2022-08-29 20:54:58

by Ben Walker

[permalink] [raw]
Subject: [PATCH v5 5/7] dmaengine: idxd: idxd_desc.id is now a u16

This is going to be packed into the cookie. It does not need to be
negative or larger than u16.

Cc: Dave Jiang <[email protected]>
Cc: Fenghua Yu <[email protected]>
Signed-off-by: Ben Walker <[email protected]>
---
drivers/dma/idxd/idxd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index fed0dfc1eaa83..bd93ada32c89d 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -325,7 +325,7 @@ struct idxd_desc {
struct dma_async_tx_descriptor txd;
struct llist_node llnode;
struct list_head list;
- int id;
+ u16 id;
int cpu;
struct idxd_wq *wq;
};
--
2.37.1

2022-08-29 20:54:58

by Ben Walker

[permalink] [raw]
Subject: [PATCH v5 7/7] dmaengine: Revert "cookie bypass for out of order completion"

This reverts commit 47ec7f09bc107720905c96bc37771e4ed1ff0aed.

This is no longer necessary now that all assumptions about the order of
completions have been removed from the dmaengine client API.

Signed-off-by: Ben Walker <[email protected]>
---
.../driver-api/dmaengine/provider.rst | 19 -------------------
drivers/dma/dmatest.c | 11 +----------
drivers/dma/idxd/dma.c | 1 -
include/linux/dmaengine.h | 2 --
4 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index a5539f816d125..8d1510c8cb66f 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -258,22 +258,6 @@ Currently, the types available are:
want to transfer a portion of uncompressed data directly to the
display to print it

-- DMA_COMPLETION_NO_ORDER
-
- - The device does not support in order completion.
-
- - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
- the device is setting this capability.
-
- - All cookie tracking and checking API should be treated as invalid if
- the device exports this capability.
-
- - At this point, this is incompatible with polling option for dmatest.
-
- - If this cap is set, the user is recommended to provide an unique
- identifier for each descriptor sent to the DMA device in order to
- properly track the completion.
-
- DMA_REPEAT

- The device supports repeated transfers. A repeated transfer, indicated by
@@ -457,9 +441,6 @@ supported.
- In the case of a cyclic transfer, it should only take into
account the total size of the cyclic buffer.

- - Should return DMA_OUT_OF_ORDER if the device does not support in order
- completion and is completing the operation out of order.
-
- This function can be called in an interrupt context.

- device_config
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index dde7b9b626336..47db4748d3e51 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -838,10 +838,7 @@ static int dmatest_func(void *data)
result("test timed out", total_tests, src->off, dst->off,
len, 0);
goto error_unmap_continue;
- } else if (status != DMA_COMPLETE &&
- !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
- dev->cap_mask) &&
- status == DMA_OUT_OF_ORDER)) {
+ } else if (status != DMA_COMPLETE) {
result(status == DMA_ERROR ?
"completion error status" :
"completion busy status", total_tests, src->off,
@@ -1019,12 +1016,6 @@ static int dmatest_add_channel(struct dmatest_info *info,
dtc->chan = chan;
INIT_LIST_HEAD(&dtc->threads);

- if (dma_has_cap(DMA_COMPLETION_NO_ORDER, dma_dev->cap_mask) &&
- info->params.polled) {
- info->params.polled = false;
- pr_warn("DMA_COMPLETION_NO_ORDER, polled disabled\n");
- }
-
if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
if (dmatest == 0) {
cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index dda5342d273f4..49e863abd50cd 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -297,7 +297,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)

dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
dma_cap_set(DMA_PRIVATE, dma->cap_mask);
- dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
dma->device_release = idxd_dma_release;

dma->device_prep_dma_interrupt = idxd_dma_prep_interrupt;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0ee21887b3924..abaf24d953849 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,7 +39,6 @@ enum dma_status {
DMA_IN_PROGRESS,
DMA_PAUSED,
DMA_ERROR,
- DMA_OUT_OF_ORDER,
};

/**
@@ -62,7 +61,6 @@ enum dma_transaction_type {
DMA_SLAVE,
DMA_CYCLIC,
DMA_INTERLEAVE,
- DMA_COMPLETION_NO_ORDER,
DMA_REPEAT,
DMA_LOAD_EOT,
/* last transaction type for creation of the capabilities mask */
--
2.37.1

2022-09-01 17:37:14

by Dave Jiang

[permalink] [raw]
Subject: Re: [PATCH v5 0/7] dmaengine: Support polling for out of order completions


On 8/29/2022 1:35 PM, Ben Walker wrote:
> This series adds support for polling async transactions for completion
> even if interrupts are disabled and transactions can complete out of
> order.
>
> Prior to this series, dma_cookie_t was a monotonically increasing integer and
> cookies could be compared to one another to determine if earlier operations had
> completed (up until the cookie wraps around, then it would break). Now, cookies
> are treated as opaque handles. The series also does some API clean up and
> documents how dma_cookie_t should behave.
>
> This closes out by adding support for .device_tx_status() to the idxd
> driver and then reverting the DMA_OUT_OF_ORDER patch that previously
> allowed idxd to opt-out of support for polling, which I think is a nice
> overall simplification to the dmaengine API.
>
> Changes since version 4:
> - Rebased
> - Removed updates to the various drivers that call dma_async_is_tx_complete.
> These clean ups will be spun off into a separate patch series since they need
> acks from other maintainers.
>
> Changes since version 3:
> - Fixed Message-Id in emails. Sorry they were all stripped! Won't
> happen again.
>
> Changes since version 2:
> - None. Rebased as requested without conflict.
>
> Changes since version 1:
> - Broke up the change to remove dma_async_is_tx_complete into a single
> patch for each driver
> - Renamed dma_async_is_tx_complete to dmaengine_async_is_tx_complete.
>
> Ben Walker (7):
> dmaengine: Remove dma_async_is_complete from client API
> dmaengine: Move dma_set_tx_state to the provider API header
> dmaengine: Add dmaengine_async_is_tx_complete
> dmaengine: Add provider documentation on cookie assignment
> dmaengine: idxd: idxd_desc.id is now a u16
> dmaengine: idxd: Support device_tx_status
> dmaengine: Revert "cookie bypass for out of order completion"
>
> Documentation/driver-api/dmaengine/client.rst | 24 ++----
> .../driver-api/dmaengine/provider.rst | 64 ++++++++------
> drivers/dma/dmaengine.c | 2 +-
> drivers/dma/dmaengine.h | 21 ++++-
> drivers/dma/dmatest.c | 14 +--
> drivers/dma/idxd/device.c | 1 +
> drivers/dma/idxd/dma.c | 86 ++++++++++++++++++-
> drivers/dma/idxd/idxd.h | 3 +-
> include/linux/dmaengine.h | 43 +++-------
> 9 files changed, 164 insertions(+), 94 deletions(-)
Besides the stray white space, Reviewed-by: Dave Jiang
<[email protected]>

2022-10-19 17:10:09

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v5 3/7] dmaengine: Add dmaengine_async_is_tx_complete

On 29-08-22, 13:35, Ben Walker wrote:
> This is the replacement for dma_async_is_tx_complete with two changes:
> 1) The name prefix is 'dmaengine' as per convention
> 2) It no longer reports the 'last' or 'used' cookie

Thanks for this cleanup. This is good :)

But, why should we retain async is API here. I think lets cleanup
properly and rename it dmaengine_is_tx_complete()

we _really_ need to drop async and have everything dmaengine_*

>
> Drivers should convert to using dmaengine_async_is_tx_complete.
>
> Signed-off-by: Ben Walker <[email protected]>
> ---
> Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
> .../driver-api/dmaengine/provider.rst | 6 +++---
> drivers/dma/dmaengine.c | 2 +-
> drivers/dma/dmatest.c | 3 +--
> include/linux/dmaengine.h | 16 ++++++++++++++++
> 5 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
> index 85ecec2c40005..9ae489a4ca97f 100644
> --- a/Documentation/driver-api/dmaengine/client.rst
> +++ b/Documentation/driver-api/dmaengine/client.rst
> @@ -259,8 +259,8 @@ The details of these operations are:
>
> dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)
>
> - This returns a cookie can be used to check the progress of DMA engine
> - activity via other DMA engine calls not covered in this document.
> + This returns a cookie that can be used to check the progress of a transaction
> + via dmaengine_async_is_tx_complete().
>
> dmaengine_submit() will not start the DMA operation, it merely adds
> it to the pending queue. For this, see step 5, dma_async_issue_pending.
> @@ -339,23 +339,12 @@ Further APIs
>
> .. code-block:: c
>
> - enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
> - dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
> -
> - This can be used to check the status of the channel. Please see
> - the documentation in include/linux/dmaengine.h for a more complete
> - description of this API.
> + enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
> + dma_cookie_t cookie)
>
> This can be used with the cookie returned from dmaengine_submit()
> to check for completion of a specific DMA transaction.
>
> - .. note::
> -
> - Not all DMA engine drivers can return reliable information for
> - a running DMA channel. It is recommended that DMA engine users
> - pause or stop (via dmaengine_terminate_all()) the channel before
> - using this API.
> -
> 5. Synchronize termination API
>
> .. code-block:: c
> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> index ceac2a300e328..1d0da2777921d 100644
> --- a/Documentation/driver-api/dmaengine/provider.rst
> +++ b/Documentation/driver-api/dmaengine/provider.rst
> @@ -539,10 +539,10 @@ where to put them)
>
> dma_cookie_t
>
> -- it's a DMA transaction ID that will increment over time.
> +- it's a DMA transaction ID.
>
> -- Not really relevant any more since the introduction of ``virt-dma``
> - that abstracts it away.
> +- The value can be chosen by the provider, or use the helper APIs
> + such as dma_cookie_assign() and dma_cookie_complete().
>
> DMA_CTRL_ACK
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index c741b6431958c..2816b8f492dab 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
>
> dma_async_issue_pending(chan);
> do {
> - status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
> + status = dmaengine_async_is_tx_complete(chan, cookie);
> if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
> dev_err(chan->device->dev, "%s: timeout!\n", __func__);
> return DMA_ERROR;
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 9fe2ae7943169..dde7b9b626336 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -831,8 +831,7 @@ static int dmatest_func(void *data)
> done->done,
> msecs_to_jiffies(params->timeout));
>
> - status = dma_async_is_tx_complete(chan, cookie, NULL,
> - NULL);
> + status = dmaengine_async_is_tx_complete(chan, cookie);
> }
>
> if (!done->done) {
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 5ae881729b620..0ee21887b3924 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1426,6 +1426,8 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
> * @last: returns last completed cookie, can be NULL
> * @used: returns last issued cookie, can be NULL
> *
> + * Note: This is deprecated. Use dmaengine_async_is_tx_complete instead.
> + *
> * If @last and @used are passed in, upon return they reflect the most
> * recently submitted (used) cookie and the most recently completed
> * cookie.
> @@ -1444,6 +1446,20 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
> return status;
> }
>
> +/**
> + * dmaengine_async_is_tx_complete - poll for transaction completion
> + * @chan: DMA channel
> + * @cookie: transaction identifier to check status of
> + *
> + */
> +static inline enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
> + dma_cookie_t cookie)
> +{
> + struct dma_tx_state state;
> +
> + return chan->device->device_tx_status(chan, cookie, &state);
> +}
> +
> #ifdef CONFIG_DMA_ENGINE
> struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
> --
> 2.37.1

--
~Vinod

2022-10-19 17:35:29

by Ben Walker

[permalink] [raw]
Subject: Re: [PATCH v5 3/7] dmaengine: Add dmaengine_async_is_tx_complete

On 10/19/2022 9:31 AM, Vinod Koul wrote:
> On 29-08-22, 13:35, Ben Walker wrote:
>> This is the replacement for dma_async_is_tx_complete with two changes:
>> 1) The name prefix is 'dmaengine' as per convention
>> 2) It no longer reports the 'last' or 'used' cookie
>
> Thanks for this cleanup. This is good :)
>
> But, why should we retain async is API here. I think lets cleanup
> properly and rename it dmaengine_is_tx_complete()
>
> we _really_ need to drop async and have everything dmaengine_*

Ack. Misunderstood earlier feedback to change dma -> dmaengine. I agree
the async needs to go. Fixed in next version.

>
>>
>> Drivers should convert to using dmaengine_async_is_tx_complete.
>>
>> Signed-off-by: Ben Walker <[email protected]>
>> ---
>> Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
>> .../driver-api/dmaengine/provider.rst | 6 +++---
>> drivers/dma/dmaengine.c | 2 +-
>> drivers/dma/dmatest.c | 3 +--
>> include/linux/dmaengine.h | 16 ++++++++++++++++
>> 5 files changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
>> index 85ecec2c40005..9ae489a4ca97f 100644
>> --- a/Documentation/driver-api/dmaengine/client.rst
>> +++ b/Documentation/driver-api/dmaengine/client.rst
>> @@ -259,8 +259,8 @@ The details of these operations are:
>>
>> dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)
>>
>> - This returns a cookie can be used to check the progress of DMA engine
>> - activity via other DMA engine calls not covered in this document.
>> + This returns a cookie that can be used to check the progress of a transaction
>> + via dmaengine_async_is_tx_complete().
>>
>> dmaengine_submit() will not start the DMA operation, it merely adds
>> it to the pending queue. For this, see step 5, dma_async_issue_pending.
>> @@ -339,23 +339,12 @@ Further APIs
>>
>> .. code-block:: c
>>
>> - enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
>> - dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
>> -
>> - This can be used to check the status of the channel. Please see
>> - the documentation in include/linux/dmaengine.h for a more complete
>> - description of this API.
>> + enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
>> + dma_cookie_t cookie)
>>
>> This can be used with the cookie returned from dmaengine_submit()
>> to check for completion of a specific DMA transaction.
>>
>> - .. note::
>> -
>> - Not all DMA engine drivers can return reliable information for
>> - a running DMA channel. It is recommended that DMA engine users
>> - pause or stop (via dmaengine_terminate_all()) the channel before
>> - using this API.
>> -
>> 5. Synchronize termination API
>>
>> .. code-block:: c
>> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
>> index ceac2a300e328..1d0da2777921d 100644
>> --- a/Documentation/driver-api/dmaengine/provider.rst
>> +++ b/Documentation/driver-api/dmaengine/provider.rst
>> @@ -539,10 +539,10 @@ where to put them)
>>
>> dma_cookie_t
>>
>> -- it's a DMA transaction ID that will increment over time.
>> +- it's a DMA transaction ID.
>>
>> -- Not really relevant any more since the introduction of ``virt-dma``
>> - that abstracts it away.
>> +- The value can be chosen by the provider, or use the helper APIs
>> + such as dma_cookie_assign() and dma_cookie_complete().
>>
>> DMA_CTRL_ACK
>>
>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>> index c741b6431958c..2816b8f492dab 100644
>> --- a/drivers/dma/dmaengine.c
>> +++ b/drivers/dma/dmaengine.c
>> @@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
>>
>> dma_async_issue_pending(chan);
>> do {
>> - status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
>> + status = dmaengine_async_is_tx_complete(chan, cookie);
>> if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
>> dev_err(chan->device->dev, "%s: timeout!\n", __func__);
>> return DMA_ERROR;
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index 9fe2ae7943169..dde7b9b626336 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -831,8 +831,7 @@ static int dmatest_func(void *data)
>> done->done,
>> msecs_to_jiffies(params->timeout));
>>
>> - status = dma_async_is_tx_complete(chan, cookie, NULL,
>> - NULL);
>> + status = dmaengine_async_is_tx_complete(chan, cookie);
>> }
>>
>> if (!done->done) {
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index 5ae881729b620..0ee21887b3924 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -1426,6 +1426,8 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
>> * @last: returns last completed cookie, can be NULL
>> * @used: returns last issued cookie, can be NULL
>> *
>> + * Note: This is deprecated. Use dmaengine_async_is_tx_complete instead.
>> + *
>> * If @last and @used are passed in, upon return they reflect the most
>> * recently submitted (used) cookie and the most recently completed
>> * cookie.
>> @@ -1444,6 +1446,20 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
>> return status;
>> }
>>
>> +/**
>> + * dmaengine_async_is_tx_complete - poll for transaction completion
>> + * @chan: DMA channel
>> + * @cookie: transaction identifier to check status of
>> + *
>> + */
>> +static inline enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
>> + dma_cookie_t cookie)
>> +{
>> + struct dma_tx_state state;
>> +
>> + return chan->device->device_tx_status(chan, cookie, &state);
>> +}
>> +
>> #ifdef CONFIG_DMA_ENGINE
>> struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
>> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
>> --
>> 2.37.1
>

2022-10-28 20:56:06

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 0/7] dmaengine: Support polling for out of order completions

This series adds support for polling async transactions for completion
even if interrupts are disabled and transactions can complete out of
order.

Prior to this series, dma_cookie_t was a monotonically increasing integer and
cookies could be compared to one another to determine if earlier operations had
completed (up until the cookie wraps around, then it would break). Now, cookies
are treated as opaque handles. The series also does some API clean up and
documents how dma_cookie_t should behave.

This closes out by adding support for .device_tx_status() to the idxd
driver and then reverting the DMA_OUT_OF_ORDER patch that previously
allowed idxd to opt-out of support for polling, which I think is a nice
overall simplification to the dmaengine API.

Changes since version 5:
- Rebased to 6.1
- Renamed dmaengine_async_is_tx_complete to dmaengine_is_tx_complete
- Fixed stray blank line above idxd_dma_tx_status
- Added Reviewed-by from Dave Jiang

Changes since version 4:
- Rebased
- Removed updates to the various drivers that call dma_async_is_tx_complete.
These clean ups will be spun off into a separate patch series since they need
acks from other maintainers.

Changes since version 3:
- Fixed Message-Id in emails. Sorry they were all stripped! Won't
happen again.

Changes since version 2:
- None. Rebased as requested without conflict.

Changes since version 1:
- Broke up the change to remove dma_async_is_tx_complete into a single
patch for each driver
- Renamed dma_async_is_tx_complete to dmaengine_async_is_tx_complete.

Ben Walker (7):
dmaengine: Remove dma_async_is_complete from client API
dmaengine: Move dma_set_tx_state to the provider API header
dmaengine: Add dmaengine_async_is_tx_complete
dmaengine: Add provider documentation on cookie assignment
dmaengine: idxd: idxd_desc.id is now a u16
dmaengine: idxd: Support device_tx_status
dmaengine: Revert "cookie bypass for out of order completion"

Documentation/driver-api/dmaengine/client.rst | 24 ++----
.../driver-api/dmaengine/provider.rst | 64 ++++++++------
drivers/dma/dmaengine.c | 2 +-
drivers/dma/dmaengine.h | 21 ++++-
drivers/dma/dmatest.c | 14 +--
drivers/dma/idxd/device.c | 1 +
drivers/dma/idxd/dma.c | 86 ++++++++++++++++++-
drivers/dma/idxd/idxd.h | 3 +-
include/linux/dmaengine.h | 43 +++-------
9 files changed, 164 insertions(+), 94 deletions(-)

--
2.37.1


2022-10-28 20:56:59

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 5/7] dmaengine: idxd: idxd_desc.id is now a u16

This is going to be packed into the cookie. It does not need to be
negative or larger than u16.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
drivers/dma/idxd/idxd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index 1196ab342f011..d9096dfb27422 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -333,7 +333,7 @@ struct idxd_desc {
struct dma_async_tx_descriptor txd;
struct llist_node llnode;
struct list_head list;
- int id;
+ u16 id;
int cpu;
struct idxd_wq *wq;
};
--
2.37.3


2022-10-28 20:57:54

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 7/7] dmaengine: Revert "cookie bypass for out of order completion"

This reverts commit 47ec7f09bc107720905c96bc37771e4ed1ff0aed.

This is no longer necessary now that all assumptions about the order of
completions have been removed from the dmaengine client API.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
.../driver-api/dmaengine/provider.rst | 19 -------------------
drivers/dma/dmatest.c | 11 +----------
drivers/dma/idxd/dma.c | 1 -
include/linux/dmaengine.h | 2 --
4 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index a5539f816d125..8d1510c8cb66f 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -258,22 +258,6 @@ Currently, the types available are:
want to transfer a portion of uncompressed data directly to the
display to print it

-- DMA_COMPLETION_NO_ORDER
-
- - The device does not support in order completion.
-
- - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
- the device is setting this capability.
-
- - All cookie tracking and checking API should be treated as invalid if
- the device exports this capability.
-
- - At this point, this is incompatible with polling option for dmatest.
-
- - If this cap is set, the user is recommended to provide an unique
- identifier for each descriptor sent to the DMA device in order to
- properly track the completion.
-
- DMA_REPEAT

- The device supports repeated transfers. A repeated transfer, indicated by
@@ -457,9 +441,6 @@ supported.
- In the case of a cyclic transfer, it should only take into
account the total size of the cyclic buffer.

- - Should return DMA_OUT_OF_ORDER if the device does not support in order
- completion and is completing the operation out of order.
-
- This function can be called in an interrupt context.

- device_config
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 76a027e95d2aa..2febc179a7074 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -838,10 +838,7 @@ static int dmatest_func(void *data)
result("test timed out", total_tests, src->off, dst->off,
len, 0);
goto error_unmap_continue;
- } else if (status != DMA_COMPLETE &&
- !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
- dev->cap_mask) &&
- status == DMA_OUT_OF_ORDER)) {
+ } else if (status != DMA_COMPLETE) {
result(status == DMA_ERROR ?
"completion error status" :
"completion busy status", total_tests, src->off,
@@ -1019,12 +1016,6 @@ static int dmatest_add_channel(struct dmatest_info *info,
dtc->chan = chan;
INIT_LIST_HEAD(&dtc->threads);

- if (dma_has_cap(DMA_COMPLETION_NO_ORDER, dma_dev->cap_mask) &&
- info->params.polled) {
- info->params.polled = false;
- pr_warn("DMA_COMPLETION_NO_ORDER, polled disabled\n");
- }
-
if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
if (dmatest == 0) {
cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index 87749efec311b..071aafec3de1b 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -296,7 +296,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)

dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
dma_cap_set(DMA_PRIVATE, dma->cap_mask);
- dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
dma->device_release = idxd_dma_release;

dma->device_prep_dma_interrupt = idxd_dma_prep_interrupt;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 72b7d51fe41de..59c7f67cb3b5a 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,7 +39,6 @@ enum dma_status {
DMA_IN_PROGRESS,
DMA_PAUSED,
DMA_ERROR,
- DMA_OUT_OF_ORDER,
};

/**
@@ -62,7 +61,6 @@ enum dma_transaction_type {
DMA_SLAVE,
DMA_CYCLIC,
DMA_INTERLEAVE,
- DMA_COMPLETION_NO_ORDER,
DMA_REPEAT,
DMA_LOAD_EOT,
/* last transaction type for creation of the capabilities mask */
--
2.37.3


2022-10-28 21:03:55

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 2/7] dmaengine: Move dma_set_tx_state to the provider API header

This is only used by DMA providers, not DMA clients. Move it next
to the other cookie utility functions.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
drivers/dma/dmaengine.h | 11 +++++++++++
include/linux/dmaengine.h | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index a2ce377e9ed0f..e72876a512a39 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -90,6 +90,17 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
return DMA_IN_PROGRESS;
}

+static inline void dma_set_tx_state(struct dma_tx_state *st,
+ dma_cookie_t last, dma_cookie_t used, u32 residue)
+{
+ if (!st)
+ return;
+
+ st->last = last;
+ st->used = used;
+ st->residue = residue;
+}
+
static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
{
if (state)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c55dcae7dc620..5ae881729b620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1444,17 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
return status;
}

-static inline void
-dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
-{
- if (!st)
- return;
-
- st->last = last;
- st->used = used;
- st->residue = residue;
-}
-
#ifdef CONFIG_DMA_ENGINE
struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
--
2.37.3


2022-10-28 21:06:46

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 1/7] dmaengine: Remove dma_async_is_complete from client API

This is never actually used by any existing DMA clients. It is only
used, via dma_cookie_status, by providers.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
Documentation/driver-api/dmaengine/client.rst | 5 ++--
drivers/dma/dmaengine.h | 10 ++++++-
include/linux/dmaengine.h | 28 ++-----------------
3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index bfd057b21a000..85ecec2c40005 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -346,9 +346,8 @@ Further APIs
the documentation in include/linux/dmaengine.h for a more complete
description of this API.

- This can be used in conjunction with dma_async_is_complete() and
- the cookie returned from dmaengine_submit() to check for
- completion of a specific DMA transaction.
+ This can be used with the cookie returned from dmaengine_submit()
+ to check for completion of a specific DMA transaction.

.. note::

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f00294..a2ce377e9ed0f 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -79,7 +79,15 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
state->residue = 0;
state->in_flight_bytes = 0;
}
- return dma_async_is_complete(cookie, complete, used);
+
+ if (complete <= used) {
+ if ((cookie <= complete) || (cookie > used))
+ return DMA_COMPLETE;
+ } else {
+ if ((cookie <= complete) && (cookie > used))
+ return DMA_COMPLETE;
+ }
+ return DMA_IN_PROGRESS;
}

static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c923f4e60f240..c55dcae7dc620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1426,9 +1426,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
* @last: returns last completed cookie, can be NULL
* @used: returns last issued cookie, can be NULL
*
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
*/
static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1444,28 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
return status;
}

-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
- dma_cookie_t last_complete, dma_cookie_t last_used)
-{
- if (last_complete <= last_used) {
- if ((cookie <= last_complete) || (cookie > last_used))
- return DMA_COMPLETE;
- } else {
- if ((cookie <= last_complete) && (cookie > last_used))
- return DMA_COMPLETE;
- }
- return DMA_IN_PROGRESS;
-}
-
static inline void
dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
{
--
2.37.3


2022-10-28 21:07:35

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 6/7] dmaengine: idxd: Support device_tx_status

This can now be supported even for devices that complete operations out
of order. Add support for directly polling transactions.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
drivers/dma/idxd/device.c | 1 +
drivers/dma/idxd/dma.c | 84 ++++++++++++++++++++++++++++++++++++++-
drivers/dma/idxd/idxd.h | 1 +
3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 2c1e6f6daa628..870e7adfdd240 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -148,6 +148,7 @@ int idxd_wq_alloc_resources(struct idxd_wq *wq)
desc->iax_completion = &wq->iax_compls[i];
desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
desc->id = i;
+ desc->gen = 1;
desc->wq = wq;
desc->cpu = -1;
}
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index e0874cb4721c8..87749efec311b 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -12,6 +12,23 @@
#include "registers.h"
#include "idxd.h"

+
+#define DMA_COOKIE_BITS (sizeof(dma_cookie_t) * 8)
+/*
+ * The descriptor id takes the lower 16 bits of the cookie.
+ */
+#define DESC_ID_BITS 16
+#define DESC_ID_MASK ((1 << DESC_ID_BITS) - 1)
+/*
+ * The 'generation' is in the upper half of the cookie. But dma_cookie_t
+ * is signed, so we leave the upper-most bit for the sign. Further, we
+ * need to flag whether a cookie corresponds to an operation that is
+ * being completed via interrupt to avoid polling it, which takes
+ * the second most upper bit. So we subtract two bits from the upper half.
+ */
+#define DESC_GEN_MAX ((1 << (DMA_COOKIE_BITS - DESC_ID_BITS - 2)) - 1)
+#define DESC_INTERRUPT_FLAG (1 << (DMA_COOKIE_BITS - 2))
+
static inline struct idxd_wq *to_idxd_wq(struct dma_chan *c)
{
struct idxd_dma_chan *idxd_chan;
@@ -162,9 +179,62 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
dma_cookie_t cookie,
struct dma_tx_state *txstate)
{
- return DMA_OUT_OF_ORDER;
+ u8 status;
+ struct idxd_wq *wq;
+ struct idxd_desc *desc;
+ u32 idx;
+
+ memset(txstate, 0, sizeof(*txstate));
+
+ if (dma_submit_error(cookie))
+ return DMA_ERROR;
+
+ wq = to_idxd_wq(dma_chan);
+
+ idx = cookie & DESC_ID_MASK;
+ if (idx >= wq->num_descs)
+ return DMA_ERROR;
+
+ desc = wq->descs[idx];
+
+ if (desc->txd.cookie != cookie) {
+ /*
+ * The user asked about an old transaction
+ */
+ return DMA_COMPLETE;
+ }
+
+ /*
+ * For descriptors completed via interrupt, we can't go
+ * look at the completion status directly because it races
+ * with the IRQ handler recyling the descriptor. However,
+ * since in this case we can rely on the interrupt handler
+ * to invalidate the cookie when the command completes we
+ * know that if we get here, the command is still in
+ * progress.
+ */
+ if ((cookie & DESC_INTERRUPT_FLAG) != 0)
+ return DMA_IN_PROGRESS;
+
+ status = desc->completion->status & DSA_COMP_STATUS_MASK;
+
+ if (status) {
+ /*
+ * Check against the original status as ABORT is software defined
+ * and 0xff, which DSA_COMP_STATUS_MASK can mask out.
+ */
+ if (unlikely(desc->completion->status == IDXD_COMP_DESC_ABORT))
+ idxd_dma_complete_txd(desc, IDXD_COMPLETE_ABORT, true);
+ else
+ idxd_dma_complete_txd(desc, IDXD_COMPLETE_NORMAL, true);
+
+ return DMA_COMPLETE;
+ }
+
+ return DMA_IN_PROGRESS;
}

+
/*
* issue_pending() does not need to do anything since tx_submit() does the job
* already.
@@ -181,7 +251,17 @@ static dma_cookie_t idxd_dma_tx_submit(struct dma_async_tx_descriptor *tx)
int rc;
struct idxd_desc *desc = container_of(tx, struct idxd_desc, txd);

- cookie = dma_cookie_assign(tx);
+ cookie = (desc->gen << DESC_ID_BITS) | (desc->id & DESC_ID_MASK);
+
+ if ((desc->hw->flags & IDXD_OP_FLAG_RCI) != 0)
+ cookie |= DESC_INTERRUPT_FLAG;
+
+ if (desc->gen == DESC_GEN_MAX)
+ desc->gen = 1;
+ else
+ desc->gen++;
+
+ tx->cookie = cookie;

rc = idxd_submit_desc(wq, desc);
if (rc < 0) {
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index d9096dfb27422..739c55e56502c 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -334,6 +334,7 @@ struct idxd_desc {
struct llist_node llnode;
struct list_head list;
u16 id;
+ u16 gen;
int cpu;
struct idxd_wq *wq;
};
--
2.37.3


2022-10-28 21:07:35

by Ben Walker

[permalink] [raw]
Subject: [PATCH v6 3/7] dmaengine: Add dmaengine_is_tx_complete

This is the replacement for dma_async_is_tx_complete with two changes:
1) The name prefix is 'dmaengine' as per convention
2) It no longer reports the 'last' or 'used' cookie

Drivers should convert to using dmaengine_is_tx_complete.

Signed-off-by: Ben Walker <[email protected]>
Reviewed-by: Dave Jiang <[email protected]>
---
Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
.../driver-api/dmaengine/provider.rst | 6 +++---
drivers/dma/dmaengine.c | 2 +-
drivers/dma/dmatest.c | 3 +--
include/linux/dmaengine.h | 16 ++++++++++++++++
5 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index 85ecec2c40005..9e737041d65ea 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -259,8 +259,8 @@ The details of these operations are:

dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)

- This returns a cookie can be used to check the progress of DMA engine
- activity via other DMA engine calls not covered in this document.
+ This returns a cookie that can be used to check the progress of a transaction
+ via dmaengine_is_tx_complete().

dmaengine_submit() will not start the DMA operation, it merely adds
it to the pending queue. For this, see step 5, dma_async_issue_pending.
@@ -339,23 +339,12 @@ Further APIs

.. code-block:: c

- enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
- dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
-
- This can be used to check the status of the channel. Please see
- the documentation in include/linux/dmaengine.h for a more complete
- description of this API.
+ enum dma_status dmaengine_is_tx_complete(struct dma_chan *chan,
+ dma_cookie_t cookie)

This can be used with the cookie returned from dmaengine_submit()
to check for completion of a specific DMA transaction.

- .. note::
-
- Not all DMA engine drivers can return reliable information for
- a running DMA channel. It is recommended that DMA engine users
- pause or stop (via dmaengine_terminate_all()) the channel before
- using this API.
-
5. Synchronize termination API

.. code-block:: c
diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index ceac2a300e328..1d0da2777921d 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -539,10 +539,10 @@ where to put them)

dma_cookie_t

-- it's a DMA transaction ID that will increment over time.
+- it's a DMA transaction ID.

-- Not really relevant any more since the introduction of ``virt-dma``
- that abstracts it away.
+- The value can be chosen by the provider, or use the helper APIs
+ such as dma_cookie_assign() and dma_cookie_complete().

DMA_CTRL_ACK

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index c741b6431958c..74bc92e51a5a7 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)

dma_async_issue_pending(chan);
do {
- status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
+ status = dmaengine_is_tx_complete(chan, cookie);
if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
dev_err(chan->device->dev, "%s: timeout!\n", __func__);
return DMA_ERROR;
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index ffe621695e472..76a027e95d2aa 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -831,8 +831,7 @@ static int dmatest_func(void *data)
done->done,
msecs_to_jiffies(params->timeout));

- status = dma_async_is_tx_complete(chan, cookie, NULL,
- NULL);
+ status = dmaengine_is_tx_complete(chan, cookie);
}

if (!done->done) {
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 5ae881729b620..72b7d51fe41de 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1426,6 +1426,8 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
* @last: returns last completed cookie, can be NULL
* @used: returns last issued cookie, can be NULL
*
+ * Note: This is deprecated. Use dmaengine_is_tx_complete instead.
+ *
* If @last and @used are passed in, upon return they reflect the most
* recently submitted (used) cookie and the most recently completed
* cookie.
@@ -1444,6 +1446,20 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
return status;
}

+/**
+ * dmaengine_is_tx_complete - poll for transaction completion
+ * @chan: DMA channel
+ * @cookie: transaction identifier to check status of
+ *
+ */
+static inline enum dma_status dmaengine_is_tx_complete(struct dma_chan *chan,
+ dma_cookie_t cookie)
+{
+ struct dma_tx_state state;
+
+ return chan->device->device_tx_status(chan, cookie, &state);
+}
+
#ifdef CONFIG_DMA_ENGINE
struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
--
2.37.3