dmaengine: Support polling for out of order completions

PATCH Superseded Ben Walker 4 participants 59 messages May 3, 2022 – Oct 28, 2022

Subsystems (9)

Documentation Media (V4L/DVB) Arm i.MX and Layerscape Crypto API +5 more subsystems

lore

Share & tools

lore

Ben Walker

author

2022-05-03 20:08 UTC #0 lore raw

v2 cover dmaengine: Support polling for out of order completions

This series adds support for polling async transactions for completion even if interrupts are disabled and trasactions 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 damengine API.

Herbert, David - Need an ack on patch 4.

Mauro - Need an ack on patches 5 and 7.

Matt, Alexandre - Need an ack on patch 6.

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/omap/omap_vout_vrfb.c  |  2 +-
 drivers/media/platform/pxa_camera.c           | 15 +++-
 drivers/rapidio/devices/rio_mport_cdev.c      |  3 +-
 include/linux/dmaengine.h                     | 58 +------------
 18 files changed, 164 insertions(+), 136 deletions(-)

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #1 lore raw

PATCH v2 1/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(-)

Documentation/driver-api/dmaengine/client.rst +2 -3

index bfd057b21a000..85ecec2c40005 100644
@@ -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::

drivers/dma/amba-pl08x.c +0 -1

index a24882ba37643..fd52556c3edb4 100644
@@ -1544,7 +1544,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_interrupt(
 }
 
 /*
- * 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.
  */

drivers/dma/at_hdmac.c +1 -2

index 30ae36124b1db..bc0e1af2296c9 100644
@@ -1483,8 +1483,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,

drivers/dma/dmaengine.h +9 -1

index 53f16d3f00294..a2ce377e9ed0f 100644
@@ -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)

include/linux/dmaengine.h +3 ~~-25~~

index 842d4f7ca752d..194e334b33bbc 100644
@@ -1439,9 +1439,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)

@@ -1457,28 +1457,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)
 {

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #2 lore raw

PATCH v2 2/15 dmaengine: Move dma_set_tx_state to the provider API header

In reply to Ben Walker

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

---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

drivers/dma/dmaengine.h +11 -0

index a2ce377e9ed0f..e72876a512a39 100644
@@ -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)

include/linux/dmaengine.h +0 ~~-11~~

index 194e334b33bbc..8c4934bc038ec 100644
@@ -1457,17 +1457,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);

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #3 lore raw

PATCH v2 3/15 dmaengine: Add dmaengine_async_is_tx_complete

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9ae489a4ca97f 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index 0072c9c7efd34..e9e9de18d6b02 100644
@@ -543,10 +543,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

drivers/dma/dmaengine.c +1 -1

index 2cfa8458b51be..62669d229cd53 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index f696246f57fdb..aa36359883242 100644
@@ -832,8 +832,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) {

include/linux/dmaengine.h +16 -0

index 8c4934bc038ec..7143b2ecdd451 100644
@@ -1439,6 +1439,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.

@@ -1457,6 +1459,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);

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #4 lore raw

PATCH v2 4/15 crypto: stm32/hash: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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

Signed-off-by Ben Walker <[email protected]>

---
 drivers/crypto/stm32/stm32-hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/crypto/stm32/stm32-hash.c +1 -2

index d33006d43f761..aef447847c499 100644
@@ -453,8 +453,7 @@ static int stm32_hash_xmit_dma(struct stm32_hash_dev *hdev,
 					 msecs_to_jiffies(100)))
 		err = -ETIMEDOUT;
 
-	if (dma_async_is_tx_complete(hdev->dma_lch, cookie,
-				     NULL, NULL) != DMA_COMPLETE)
+	if (dmaengine_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
 		err = -ETIMEDOUT;
 
 	if (err) {

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #5 lore raw

PATCH v2 5/15 media: omap_vout: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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

Signed-off-by Ben Walker <[email protected]>

---
 drivers/media/platform/omap/omap_vout_vrfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/platform/omap/omap_vout_vrfb.c +1 -1

index 0cfa0169875f0..b9d252d5ced7a 100644
@@ -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__);

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #6 lore raw

PATCH v2 6/15 rapidio: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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

Signed-off-by Ben Walker <[email protected]>

---
 drivers/rapidio/devices/rio_mport_cdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/rapidio/devices/rio_mport_cdev.c +1 -2

index 7df466e222820..ca66dbfef9c59 100644
@@ -597,8 +597,7 @@ static void dma_xfer_callback(void *param)
 	struct mport_dma_req *req = (struct mport_dma_req *)param;
 	struct mport_cdev_priv *priv = req->priv;
 
-	req->status = dma_async_is_tx_complete(priv->dmach, req->cookie,
-					       NULL, NULL);
+	req->status = dmaengine_async_is_tx_complete(priv->dmach, req->cookie);
 	complete(&req->req_comp);
 	kref_put(&req->refcount, dma_req_free);
 }

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:09 UTC #7 lore raw

PATCH v2 7/15 media: pxa_camera: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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

Signed-off-by Ben Walker <[email protected]>

---
 drivers/media/platform/pxa_camera.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/media/platform/pxa_camera.c +12 -3

index 3ba00b0f93200..29406b4c77406 100644
@@ -1048,9 +1048,18 @@ static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
 	}
 	last_buf = list_entry(pcdev->capture.prev,
 			      struct pxa_buffer, queue);
-	last_status = dma_async_is_tx_complete(pcdev->dma_chans[chan],
-					       last_buf->cookie[chan],
-					       NULL, &last_issued);
+	last_status = dmaengine_async_is_tx_complete(pcdev->dma_chans[chan],
+					       last_buf->cookie[chan]);
+	/*
+	 * Peek into the channel and read the last cookie that was issued.
+	 * This is a layering violation - the dmaengine API does not officially
+	 * provide this information. Since this camera driver is tightly coupled
+	 * with a specific DMA device we know exactly how this cookie value will
+	 * behave. Otherwise, this wouldn't be safe.
+	 */
+	last_issued = pcdev->dma_chans[chan]->cookie;
+	barrier();
+
 	if (camera_status & overrun &&
 	    last_status != DMA_COMPLETE) {
 		dev_dbg(pcdev_to_dev(pcdev), "FIFO overrun! CISR: %x\n",

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #8 lore raw

PATCH v2 8/15 dmaengine: Remove dma_async_is_tx_complete

In reply to Ben Walker

Everything has now been converted over to dmaengine_async_is_tx_complete, so this can be removed.

Signed-off-by Ben Walker <[email protected]>

---
 include/linux/dmaengine.h | 27 ---------------------------
 1 file changed, 27 deletions(-)

include/linux/dmaengine.h +0 ~~-27~~

index 7143b2ecdd451..17f210adc14cb 100644
@@ -1432,33 +1432,6 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
 	chan->device->device_issue_pending(chan);
 }
 
-/**
- * dma_async_is_tx_complete - poll for transaction completion
- * @chan: DMA channel
- * @cookie: transaction identifier to check status of
- * @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.
- */
-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)
-{
-	struct dma_tx_state state;
-	enum dma_status status;
-
-	status = chan->device->device_tx_status(chan, cookie, &state);
-	if (last)
-		*last = state.last;
-	if (used)
-		*used = state.used;
-	return status;
-}
-
 /**
  * dmaengine_async_is_tx_complete - poll for transaction completion
  * @chan: DMA channel

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:09 UTC #9 lore raw

PATCH v2 9/15 dmaengine: Remove last, used from dma_tx_state

In reply to Ben Walker

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(-)

drivers/dma/dmaengine.h +0 -4

index e72876a512a39..08c7bd7cfc229 100644
@@ -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;
 }

include/linux/dmaengine.h +0 -4

index 17f210adc14cb..827007146eb94 100644
@@ -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;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:09 UTC #10 lore raw

PATCH v2 10/15 dmaengine: Providers should prefer dma_set_residue over dma_set_tx_state

In reply to Ben Walker

The dma_set_tx_state function will go away shortly. The two functions are functionally equivalent.

Signed-off-by Ben Walker <[email protected]>

---
 drivers/dma/imx-sdma.c | 3 +--
 drivers/dma/mmp_tdma.c | 3 +--
 drivers/dma/mxs-dma.c  | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

drivers/dma/imx-sdma.c +1 -2

index 70c0aa931ddf4..f1ef059da4652 100644
@@ -1750,8 +1750,7 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
 
 	spin_unlock_irqrestore(&sdmac->vc.lock, flags);
 
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			 residue);
+	dma_set_residue(txstate, residue);
 
 	return sdmac->status;
 }

drivers/dma/mmp_tdma.c +1 -2

index a262e0eb4cc94..753b431ca206b 100644
@@ -539,8 +539,7 @@ static enum dma_status mmp_tdma_tx_status(struct dma_chan *chan,
 	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
 
 	tdmac->pos = mmp_tdma_get_pos(tdmac);
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			 tdmac->buf_len - tdmac->pos);
+	dma_set_residue(txstate, tdmac->buf_len - tdmac->pos);
 
 	return tdmac->status;
 }

drivers/dma/mxs-dma.c +1 -2

index 994fc4d2aca42..ab9eca6d682dc 100644
@@ -664,8 +664,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 		residue -= bar;
 	}
 
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			residue);
+	dma_set_residue(txstate, residue);
 
 	return mxs_chan->status;
 }

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #11 lore raw

PATCH v2 11/15 dmaengine: Remove dma_set_tx_state

In reply to Ben Walker

Nothing calls this anymore.

Signed-off-by Ben Walker <[email protected]>

---
 drivers/dma/dmaengine.h | 9 ---------
 1 file changed, 9 deletions(-)

drivers/dma/dmaengine.h +0 -9

index 08c7bd7cfc229..7a5203175e6a8 100644
@@ -88,15 +88,6 @@ 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->residue = residue;
-}
-
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
 {
 	if (state)

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:09 UTC #12 lore raw

PATCH v2 12/15 dmaengine: Add provider documentation on cookie assignment

In reply to Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +37 -8

index e9e9de18d6b02..7c8ace703c96f 100644
@@ -421,7 +421,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a

@@ -526,6 +528,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========

@@ -541,13 +577,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
 - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #13 lore raw

PATCH v2 13/15 dmaengine: idxd: idxd_desc.id is now a u16

In reply to Ben Walker

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

---
 drivers/dma/idxd/idxd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/dma/idxd/idxd.h +1 -1

index a9021e332a5b4..40835160ef883 100644
@@ -328,7 +328,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;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #14 lore raw

PATCH v2 14/15 dmaengine: idxd: Support device_tx_status

In reply to Ben Walker

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

---
 drivers/dma/idxd/device.c |  1 +
 drivers/dma/idxd/dma.c    | 85 ++++++++++++++++++++++++++++++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 3 files changed, 85 insertions(+), 2 deletions(-)

drivers/dma/idxd/device.c +1 -0

index f98dd443a4edb..48424693c7a49 100644
@@ -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;
 	}

drivers/dma/idxd/dma.c +83 -2

109 lines not on this page · raw message

drivers/dma/idxd/idxd.h +1 -0

index 40835160ef883..d06ddfddd8612 100644
@@ -329,6 +329,7 @@ struct idxd_desc {
 	struct llist_node llnode;
 	struct list_head list;
 	u16 id;
+	u16 gen;
 	int cpu;
 	struct idxd_wq *wq;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-05-03 20:08 UTC #15 lore raw

PATCH v2 15/15 dmaengine: Revert "cookie bypass for out of order completion"

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/provider.rst +0 ~~-19~~

index 7c8ace703c96f..5bb4738ece0c2 100644
@@ -262,22 +262,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

@@ -461,9 +445,6 @@ supported.
   - In the case of a cyclic transfer, it should only take into
     account the current period.
 
-  - 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

drivers/dma/dmatest.c +1 ~~-10~~

index aa36359883242..e78b315d31f83 100644
@@ -839,10 +839,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,

@@ -1020,12 +1017,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);

drivers/dma/idxd/dma.c +0 -1

index 7bb9ecad43ec5..4d35597cf5315 100644
@@ -289,7 +289,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)
 	dma->dev = dev;
 
 	dma_cap_set(DMA_PRIVATE, dma->cap_mask);
-	dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
 	dma->device_release = idxd_dma_release;
 
 	if (idxd->hw.opcap.bits[0] & IDXD_OPCAP_MEMMOVE) {

include/linux/dmaengine.h +0 -2

index 827007146eb94..ba568d0373dec 100644
@@ -39,7 +39,6 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
-	DMA_OUT_OF_ORDER,
 };
 
 /**

@@ -63,7 +62,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 */

Signature · 1 line

2.35.1

Vinod Koul

maintainer

2022-06-09 06:53 UTC #16 lore raw

In reply to Ben Walker

Quoted 23 lines · Ben Walker

This series adds support for polling async transactions for completion even if interrupts are disabled and trasactions 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 damengine API.

Quoted 5 lines

Herbert, David - Need an ack on patch 4.
Mauro - Need an ack on patches 5 and 7.
Matt, Alexandre - Need an ack on patch 6.

Can you rebase and resend this, hopefully folks can ack the change...

Quoted 46 lines

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/omap/omap_vout_vrfb.c  |  2 +-
 drivers/media/platform/pxa_camera.c           | 15 +++-
 drivers/rapidio/devices/rio_mport_cdev.c      |  3 +-
 include/linux/dmaengine.h                     | 58 +------------
 18 files changed, 164 insertions(+), 136 deletions(-)
Signature · 1 line
2.35.1

Signature · 1 line

~Vinod

Mauro Carvalho Chehab

maintainer

2022-06-22 06:30 UTC #17 lore raw

Em Thu, 9 Jun 2022 12:23:05 +0530
Vinod Koul <[email protected]> escreveu:

Quoted 24 lines · Ben Walker

Quoted 23 lines
This series adds support for polling async transactions for completion even if interrupts are disabled and trasactions 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 damengine API.

Quoted 7 lines

Quoted 5 lines
Herbert, David - Need an ack on patch 4.
Mauro - Need an ack on patches 5 and 7.
Matt, Alexandre - Need an ack on patch 6.
Can you rebase and resend this, hopefully folks can ack the change...

The mailing lists for each subsystem should also be c/c, in order to allow usual reviewers to take a look at the patches. In case of patches 5 and 7, they both should be c/c to [email protected].

It also makes sense to c/c LKML, in case some of the reviewers want to see the full patch series.

Regards,
Mauro

Quoted 48 lines

Quoted 47 lines
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/omap/omap_vout_vrfb.c  |  2 +-
 drivers/media/platform/pxa_camera.c           | 15 +++-
 drivers/rapidio/devices/rio_mport_cdev.c      |  3 +-
 include/linux/dmaengine.h                     | 58 +------------
 18 files changed, 164 insertions(+), 136 deletions(-)
Signature · 1 line
2.35.1  

Ben Walker

author

2022-06-22 19:38 UTC #18 lore raw

v4 cover dmaengine: Support polling for out of order completions

In reply to Ben Walker

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

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

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.

 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]

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:38 UTC #19 lore raw

PATCH v4 1/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(-)

Documentation/driver-api/dmaengine/client.rst +2 -3

index bfd057b21a000..85ecec2c40005 100644
@@ -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::

drivers/dma/amba-pl08x.c +0 -1

index a4a794e62ac26..bd361aee07db8 100644
@@ -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.
  */

drivers/dma/at_hdmac.c +1 -2

index 5a50423b7378e..5ec9a36074771 100644
@@ -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,

drivers/dma/dmaengine.h +9 -1

index 53f16d3f00294..a2ce377e9ed0f 100644
@@ -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)

include/linux/dmaengine.h +3 ~~-25~~

index b46b88e6aa0d1..ea6ec2666eb15 100644
@@ -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)
 {

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:38 UTC #20 lore raw

PATCH v4 2/15 dmaengine: Move dma_set_tx_state to the provider API header

In reply to Ben Walker

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

---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

drivers/dma/dmaengine.h +11 -0

index a2ce377e9ed0f..e72876a512a39 100644
@@ -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)

include/linux/dmaengine.h +0 ~~-11~~

index ea6ec2666eb15..f968f7671e22c 100644
@@ -1464,17 +1464,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);

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:38 UTC #21 lore raw

PATCH v4 3/15 dmaengine: Add dmaengine_async_is_tx_complete

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9ae489a4ca97f 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index 1e0f1f85d10e5..610309276bc9c 100644
@@ -549,10 +549,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

drivers/dma/dmaengine.c +1 -1

index e80feeea0e018..f67a13c29d3f9 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index 0a2168a4ccb0c..3ee47a72bf9d7 100644
@@ -838,8 +838,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) {

include/linux/dmaengine.h +16 -0

index f968f7671e22c..44eb1df433e61 100644
@@ -1446,6 +1446,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.

@@ -1464,6 +1466,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);

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #22 lore raw

PATCH v4 4/15 crypto: stm32/hash: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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/crypto/stm32/stm32-hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/crypto/stm32/stm32-hash.c +1 -2

index d33006d43f761..aef447847c499 100644
@@ -453,8 +453,7 @@ static int stm32_hash_xmit_dma(struct stm32_hash_dev *hdev,
 					 msecs_to_jiffies(100)))
 		err = -ETIMEDOUT;
 
-	if (dma_async_is_tx_complete(hdev->dma_lch, cookie,
-				     NULL, NULL) != DMA_COMPLETE)
+	if (dmaengine_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
 		err = -ETIMEDOUT;
 
 	if (err) {

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:38 UTC #23 lore raw

PATCH v4 5/15 media: omap_vout: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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(-)

drivers/media/platform/ti/omap/omap_vout_vrfb.c +1 -1

index 0cfa0169875f0..b9d252d5ced7a 100644
@@ -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__);

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #24 lore raw

PATCH v4 6/15 rapidio: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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/rapidio/devices/rio_mport_cdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/rapidio/devices/rio_mport_cdev.c +1 -2

index 2cdc054e53a53..d4108ff2bb74c 100644
@@ -597,8 +597,7 @@ static void dma_xfer_callback(void *param)
 	struct mport_dma_req *req = (struct mport_dma_req *)param;
 	struct mport_cdev_priv *priv = req->priv;
 
-	req->status = dma_async_is_tx_complete(priv->dmach, req->cookie,
-					       NULL, NULL);
+	req->status = dmaengine_async_is_tx_complete(priv->dmach, req->cookie);
 	complete(&req->req_comp);
 	kref_put(&req->refcount, dma_req_free);
 }

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #25 lore raw

PATCH v4 7/15 media: pxa_camera: Use dmaengine_async_is_tx_complete

In reply to Ben Walker

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

Cc [email protected]

Signed-off-by Ben Walker <[email protected]>

---
 drivers/media/platform/intel/pxa_camera.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/media/platform/intel/pxa_camera.c +12 -3

index 35145e3348f0e..0880bf2baa339 100644
@@ -1048,9 +1048,18 @@ static void pxa_camera_dma_irq(struct pxa_camera_dev *pcdev,
 	}
 	last_buf = list_entry(pcdev->capture.prev,
 			      struct pxa_buffer, queue);
-	last_status = dma_async_is_tx_complete(pcdev->dma_chans[chan],
-					       last_buf->cookie[chan],
-					       NULL, &last_issued);
+	last_status = dmaengine_async_is_tx_complete(pcdev->dma_chans[chan],
+					       last_buf->cookie[chan]);
+	/*
+	 * Peek into the channel and read the last cookie that was issued.
+	 * This is a layering violation - the dmaengine API does not officially
+	 * provide this information. Since this camera driver is tightly coupled
+	 * with a specific DMA device we know exactly how this cookie value will
+	 * behave. Otherwise, this wouldn't be safe.
+	 */
+	last_issued = pcdev->dma_chans[chan]->cookie;
+	barrier();
+
 	if (camera_status & overrun &&
 	    last_status != DMA_COMPLETE) {
 		dev_dbg(pcdev_to_dev(pcdev), "FIFO overrun! CISR: %x\n",

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #26 lore raw

PATCH v4 8/15 dmaengine: Remove dma_async_is_tx_complete

In reply to Ben Walker

Everything has now been converted over to dmaengine_async_is_tx_complete, so this can be removed.

Signed-off-by Ben Walker <[email protected]>

---
 include/linux/dmaengine.h | 27 ---------------------------
 1 file changed, 27 deletions(-)

include/linux/dmaengine.h +0 ~~-27~~

index 44eb1df433e61..c34f21d19c423 100644
@@ -1439,33 +1439,6 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
 	chan->device->device_issue_pending(chan);
 }
 
-/**
- * dma_async_is_tx_complete - poll for transaction completion
- * @chan: DMA channel
- * @cookie: transaction identifier to check status of
- * @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.
- */
-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)
-{
-	struct dma_tx_state state;
-	enum dma_status status;
-
-	status = chan->device->device_tx_status(chan, cookie, &state);
-	if (last)
-		*last = state.last;
-	if (used)
-		*used = state.used;
-	return status;
-}
-
 /**
  * dmaengine_async_is_tx_complete - poll for transaction completion
  * @chan: DMA channel

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 20:05 UTC #27 lore raw

PATCH v4 9/15 dmaengine: Remove last, used from dma_tx_state

In reply to Ben Walker

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(-)

drivers/dma/dmaengine.h +0 -4

index e72876a512a39..08c7bd7cfc229 100644
@@ -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;
 }

include/linux/dmaengine.h +0 -4

index c34f21d19c423..e3e5311b6bb64 100644
@@ -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;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #28 lore raw

PATCH v4 10/15 dmaengine: Providers should prefer dma_set_residue over dma_set_tx_state

In reply to Ben Walker

The dma_set_tx_state function will go away shortly. The two functions are functionally equivalent.

Signed-off-by Ben Walker <[email protected]>

---
 drivers/dma/imx-sdma.c | 3 +--
 drivers/dma/mmp_tdma.c | 3 +--
 drivers/dma/mxs-dma.c  | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

drivers/dma/imx-sdma.c +1 -2

index 8535018ee7a2e..f2de1836dbd34 100644
@@ -1816,8 +1816,7 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
 
 	spin_unlock_irqrestore(&sdmac->vc.lock, flags);
 
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			 residue);
+	dma_set_residue(txstate, residue);
 
 	return sdmac->status;
 }

drivers/dma/mmp_tdma.c +1 -2

index a262e0eb4cc94..753b431ca206b 100644
@@ -539,8 +539,7 @@ static enum dma_status mmp_tdma_tx_status(struct dma_chan *chan,
 	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
 
 	tdmac->pos = mmp_tdma_get_pos(tdmac);
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			 tdmac->buf_len - tdmac->pos);
+	dma_set_residue(txstate, tdmac->buf_len - tdmac->pos);
 
 	return tdmac->status;
 }

drivers/dma/mxs-dma.c +1 -2

index 994fc4d2aca42..ab9eca6d682dc 100644
@@ -664,8 +664,7 @@ static enum dma_status mxs_dma_tx_status(struct dma_chan *chan,
 		residue -= bar;
 	}
 
-	dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
-			residue);
+	dma_set_residue(txstate, residue);
 
 	return mxs_chan->status;
 }

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #29 lore raw

PATCH v4 11/15 dmaengine: Remove dma_set_tx_state

In reply to Ben Walker

Nothing calls this anymore.

Signed-off-by Ben Walker <[email protected]>

---
 drivers/dma/dmaengine.h | 9 ---------
 1 file changed, 9 deletions(-)

drivers/dma/dmaengine.h +0 -9

index 08c7bd7cfc229..7a5203175e6a8 100644
@@ -88,15 +88,6 @@ 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->residue = residue;
-}
-
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
 {
 	if (state)

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #30 lore raw

PATCH v4 12/15 dmaengine: Add provider documentation on cookie assignment

In reply to Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +37 -8

index 610309276bc9c..db019ec492b58 100644
@@ -427,7 +427,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a

@@ -532,6 +534,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========

@@ -547,13 +583,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
 - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #31 lore raw

PATCH v4 13/15 dmaengine: idxd: idxd_desc.id is now a u16

In reply to Ben Walker

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

---
 drivers/dma/idxd/idxd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/dma/idxd/idxd.h +1 -1

index fed0dfc1eaa83..bd93ada32c89d 100644
@@ -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;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #32 lore raw

PATCH v4 14/15 dmaengine: idxd: Support device_tx_status

In reply to Ben Walker

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

---
 drivers/dma/idxd/device.c |  1 +
 drivers/dma/idxd/dma.c    | 85 ++++++++++++++++++++++++++++++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 3 files changed, 85 insertions(+), 2 deletions(-)

drivers/dma/idxd/device.c +1 -0

index ff0ea60051f0c..3c61eac0b05c7 100644
@@ -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;
 	}

drivers/dma/idxd/dma.c +83 -2

109 lines not on this page · raw message

drivers/dma/idxd/idxd.h +1 -0

index bd93ada32c89d..d4f0227895075 100644
@@ -326,6 +326,7 @@ struct idxd_desc {
 	struct llist_node llnode;
 	struct list_head list;
 	u16 id;
+	u16 gen;
 	int cpu;
 	struct idxd_wq *wq;
 };

Signature · 1 line

2.35.1

Ben Walker

author

2022-06-22 19:39 UTC #33 lore raw

PATCH v4 15/15 dmaengine: Revert "cookie bypass for out of order completion"

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/provider.rst +0 ~~-19~~

index db019ec492b58..8565241270a62 100644
@@ -268,22 +268,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

@@ -467,9 +451,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

drivers/dma/dmatest.c +1 ~~-10~~

index 3ee47a72bf9d7..d34e7a9b63d89 100644
@@ -845,10 +845,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,

@@ -1027,12 +1024,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);

drivers/dma/idxd/dma.c +0 -1

index dda5342d273f4..49e863abd50cd 100644
@@ -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;

include/linux/dmaengine.h +0 -2

index e3e5311b6bb64..136c7afbcc385 100644
@@ -39,7 +39,6 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
-	DMA_OUT_OF_ORDER,
 };
 
 /**

@@ -63,7 +62,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 */

Signature · 1 line

2.35.1

Vinod Koul

maintainer

2022-06-27 06:29 UTC #34 lore raw

In reply to Ben Walker

Hi Ben,

Quoted 3 lines · Ben Walker

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

Signature · 1 line

~Vinod

Ben Walker

author

2022-08-29 20:35 UTC #35 lore raw

v5 cover dmaengine: Support polling for out of order completions

In reply to Ben Walker

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.

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(-)

Signature · 1 line

2.37.1

Ben Walker

author

2022-08-29 20:35 UTC #36 lore raw

PATCH v5 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]>

---
 Documentation/driver-api/dmaengine/client.rst |  5 ++--
 drivers/dma/dmaengine.h                       | 10 ++++++-
 include/linux/dmaengine.h                     | 28 ++-----------------
 3 files changed, 14 insertions(+), 29 deletions(-)

Documentation/driver-api/dmaengine/client.rst +2 -3

index bfd057b21a000..85ecec2c40005 100644
@@ -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::

drivers/dma/dmaengine.h +9 -1

index 53f16d3f00294..a2ce377e9ed0f 100644
@@ -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)

include/linux/dmaengine.h +3 ~~-25~~

index c923f4e60f240..c55dcae7dc620 100644
@@ -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)
 {

Signature · 1 line

2.37.1

Ben Walker

author

2022-08-29 20:36 UTC #37 lore raw

PATCH v5 2/7 dmaengine: Move dma_set_tx_state to the provider API header

In reply to Ben Walker

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

---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

drivers/dma/dmaengine.h +11 -0

index a2ce377e9ed0f..e72876a512a39 100644
@@ -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)

include/linux/dmaengine.h +0 ~~-11~~

index c55dcae7dc620..5ae881729b620 100644
@@ -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);

Signature · 1 line

2.37.1

Ben Walker

author

2022-08-29 20:36 UTC #38 lore raw

PATCH v5 3/7 dmaengine: Add dmaengine_async_is_tx_complete

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9ae489a4ca97f 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index ceac2a300e328..1d0da2777921d 100644
@@ -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

drivers/dma/dmaengine.c +1 -1

index c741b6431958c..2816b8f492dab 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index 9fe2ae7943169..dde7b9b626336 100644
@@ -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) {

include/linux/dmaengine.h +16 -0

index 5ae881729b620..0ee21887b3924 100644
@@ -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);

Signature · 1 line

2.37.1

Vinod Koul

maintainer

2022-10-19 16:31 UTC #39 lore raw

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

Quoted 3 lines · Ben Walker

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_*

Quoted 134 lines

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9ae489a4ca97f 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index ceac2a300e328..1d0da2777921d 100644
@@ -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

drivers/dma/dmaengine.c +1 -1

index c741b6431958c..2816b8f492dab 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index 9fe2ae7943169..dde7b9b626336 100644
@@ -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) {

include/linux/dmaengine.h +16 -0

index 5ae881729b620..0ee21887b3924 100644
@@ -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);

Signature · 1 line

2.37.1

Signature · 1 line

~Vinod

Walker, Benjamin

author

2022-10-19 17:09 UTC #40 lore raw

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

Quoted 11 lines · Vinod Koul

Quoted 3 lines · Ben Walker
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.

Quoted 136 lines

Quoted 135 lines

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9ae489a4ca97f 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index ceac2a300e328..1d0da2777921d 100644
@@ -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

drivers/dma/dmaengine.c +1 -1

index c741b6431958c..2816b8f492dab 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index 9fe2ae7943169..dde7b9b626336 100644
@@ -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) {

include/linux/dmaengine.h +16 -0

index 5ae881729b620..0ee21887b3924 100644
@@ -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);

Signature · 1 line

2.37.1

Ben Walker

author

2022-08-29 20:36 UTC #41 lore raw

PATCH v5 4/7 dmaengine: Add provider documentation on cookie assignment

In reply to Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +37 -8

index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a

@@ -522,6 +524,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========

@@ -537,13 +573,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
 - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.37.1

Vinod Koul

maintainer

2022-10-19 16:34 UTC #42 lore raw

Re: [PATCH v5 4/7] dmaengine: Add provider documentation on cookie assignment

Quoted 23 lines · Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +3 -1

index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a

Documentation/driver-api/dmaengine/provider.rst supported. · line 532

Quoted 9 lines

Documentation/driver-api/dmaengine/provider.rst +6 -0

@@ -522,6 +524,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for

We assumes that we have monotonically increasing cookie and if cookie 10 is marked complete cookie 8 is assumed complete too...

Completion is always in order unless we specify DMA_COMPLETION_NO_ORDER

Quoted 47 lines

Documentation/driver-api/dmaengine/provider.rst +28 -7

+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========

@@ -537,13 +573,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
 - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.37.1

Signature · 1 line

~Vinod

Walker, Benjamin

author

2022-10-19 17:21 UTC #43 lore raw

Re: [PATCH v5 4/7] dmaengine: Add provider documentation on cookie assignment

Quoted 27 lines · Vinod Koul

Quoted 26 lines · Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

---
  .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
  1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +3 -1

index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
 
      - tx_submit: A pointer to a function you have to implement,
        that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
    - In this structure the function pointer callback_result can be
      initialized in order for the submitter to be notified that a

@@ -522,6 +524,40 @@ supported.
 
    - May sleep.

Quoted 9 lines

Documentation/driver-api/dmaengine/provider.rst supported. · line 532
Quoted 6 lines
Documentation/driver-api/dmaengine/provider.rst +6 -0
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
We assumes that we have monotonically increasing cookie and if cookie 10 is marked complete cookie 8 is assumed complete too...

That's exactly what this patch series is changing. The earlier patches make changes to no longer report to the client the "last" or "used" cookie (to compare against) in the client APIs, and it turns out that nothing in the kernel actually cares about this behavior. So it's simply a documentation change to indicate that the client no longer has any visibility into the cookie behavior.

Immediately below here the documentation then says that there's some convenience functions that providers can use that do produce monotonically increasing cookies. These are now optional for providers to use, if they find them useful, rather than the required way to manage the cookies.

Quoted 2 lines

Completion is always in order unless we specify DMA_COMPLETION_NO_ORDER

The final patch in this series eliminates DMA_COMPLETION_NO_ORDER entirely. It was only used by the IDXD driver, and the reason I'm doing these patches is so that we can poll the IDXD driver for completions even though it can complete out of order.

Quoted 49 lines

Quoted 48 lines

Documentation/driver-api/dmaengine/provider.rst +28 -7

+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
  Misc notes
  ==========

@@ -537,13 +573,6 @@ where to put them)
  - Makes sure that dependent operations are run before marking it
    as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
  - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.37.1

Vinod Koul

maintainer

2022-10-20 04:12 UTC #44 lore raw

Re: [PATCH v5 4/7] dmaengine: Add provider documentation on cookie assignment

Quoted 34 lines · Benjamin

Quoted 24 lines · Vinod Koul
Quoted 23 lines · Ben Walker
Clarify the rules on assigning cookies to DMA transactions.
Signed-off-by Ben Walker <[email protected]>
---
  .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
  1 file changed, 37 insertions(+), 8 deletions(-)
Documentation/driver-api/dmaengine/provider.rst +4 -1
index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
      - tx_submit: A pointer to a function you have to implement,
        that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
    - In this structure the function pointer callback_result can be
      initialized in order for the submitter to be notified that a
@@ -522,6 +524,40 @@ supported.
    - May sleep.
+Cookie Management
Quoted 8 lines
Documentation/driver-api/dmaengine/provider.rst supported. · line 532
Quoted 5 lines
Documentation/driver-api/dmaengine/provider.rst +5 -0
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
We assumes that we have monotonically increasing cookie and if cookie 10 is marked complete cookie 8 is assumed complete too...

Quoted 6 lines

That's exactly what this patch series is changing. The earlier patches make changes to no longer report to the client the "last" or "used" cookie (to compare against) in the client APIs, and it turns out that nothing in the kernel actually cares about this behavior. So it's simply a documentation change to indicate that the client no longer has any visibility into the cookie behavior.

Not really, there are some engines which will notify that descriptor X completed which also implies that all descriptors before X have completed as well...

If we change the default behaviour, we risk breaking those.

Quoted 13 lines

Immediately below here the documentation then says that there's some convenience functions that providers can use that do produce monotonically increasing cookies. These are now optional for providers to use, if they find them useful, rather than the required way to manage the cookies.
Quoted 2 lines
Completion is always in order unless we specify DMA_COMPLETION_NO_ORDER
The final patch in this series eliminates DMA_COMPLETION_NO_ORDER entirely. It was only used by the IDXD driver, and the reason I'm doing these patches is so that we can poll the IDXD driver for completions even though it can complete out of order.

Signature · 1 line

~Vinod

Walker, Benjamin

author

2022-10-21 17:34 UTC #45 lore raw

Re: [PATCH v5 4/7] dmaengine: Add provider documentation on cookie assignment

Quoted 42 lines · Vinod Koul

Quoted 34 lines · Benjamin
Quoted 24 lines · Vinod Koul
Quoted 23 lines · Ben Walker
Clarify the rules on assigning cookies to DMA transactions.
Signed-off-by Ben Walker <[email protected]>
---
   .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
   1 file changed, 37 insertions(+), 8 deletions(-)
Documentation/driver-api/dmaengine/provider.rst +4 -1
index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
       - tx_submit: A pointer to a function you have to implement,
         that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
     - In this structure the function pointer callback_result can be
       initialized in order for the submitter to be notified that a
@@ -522,6 +524,40 @@ supported.
     - May sleep.
+Cookie Management
Quoted 8 lines
Documentation/driver-api/dmaengine/provider.rst supported. · line 532
Quoted 5 lines
Documentation/driver-api/dmaengine/provider.rst +5 -0
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
We assumes that we have monotonically increasing cookie and if cookie 10 is marked complete cookie 8 is assumed complete too...
Quoted 6 lines
That's exactly what this patch series is changing. The earlier patches make changes to no longer report to the client the "last" or "used" cookie (to compare against) in the client APIs, and it turns out that nothing in the kernel actually cares about this behavior. So it's simply a documentation change to indicate that the client no longer has any visibility into the cookie behavior.

Quoted 5 lines

Not really, there are some engines which will notify that descriptor X completed which also implies that all descriptors before X have completed as well...
If we change the default behaviour, we risk breaking those.

I actually don't believe it's true that any clients rely on this behavior today. Certainly, that's the defined behavior prior to this patch series and a client could have relied on that. But I did a big audit and I don't believe any of them actually do. Prior to submitting this patch series I was thinking I needed to create new APIs that code could opt into and convert over to gradually, but it seems we're fortunate enough to get away with just changing the documentation.

As a quick justification, it's worth doing the work to audit and confirm all of this because this is such an important change for the future usefulness of the dmaengine framework. Modern DMA devices are best used by polling for completions, and they certainly can complete out of order. As more of the kernel moves to performing asynchronous operations (mostly via io_uring), this is becoming very important. The rest of this email is me repeating my big audit and taking notes along the way. I apologize if it's long, but it's important to document the findings.

If we look at the client-facing API, we can identify all of the points at which a cookie is returned to the user or consumed by the the API as input.

As input:
dma_submit_error
dmaengine_tx_status
dma_async_is_tx_complete
dmaengine_is_tx_complete
dma_sync_wait

As a returned value:
dmaengine_submit
dmaengine_tx_status (via the returned state parameter) dma_async_is_tx_complete (via last/user parameters)

It's also in the following data structures (which are visible to clients):
dma_chan
dma_async_tx_descriptor (returned from the tx_submit function pointer) dma_tx_state (only returned by dmaengine_tx_status)

So auditing all of those uses:
- dma_submit_error doesn't assume it's monotonically increasing

- dmaengine_tx_status itself doesn't assume (in the generic dmaengine code) it's monotonically increasing. Providers implementing this call may assume that, but they're in control of making it so. This call can also return cookies via the optional state parameter. Except every call either passes NULL for state to ignore it, or allocates state on the stack and never stores it outside of the local function. Within those functions, only state.residue is ever used - the cookies are never touched.

- dma_sync_wait is called in 5 places. In 3 places it's called immediately after a dmaengine_submit and the cookie is only ever on the stack and never compared to anything. The other two spots are during shutdown in ntb_transport_free_queue(). All it's doing here is waiting for the last *submitted* cookie to finish, then aborting anything that is still outstanding. This driver already works with devices that complete out of order (idxd), so it has a comment saying that waiting for the last submitted may not wait for all, and that's why it does the abort. No issue there.

- dmaengine_is_tx_complete isn't used anywhere. We just added it in this series. It's intended to replace dma_async_is_tx_complete.

- dma_async_is_tx_complete is called in 4 places:
-- stm32-hash.c calls dmaengine_submit to get a cookie, then calls dma_async_is_tx_complete with that value. The cookie only exists on the stack and it's value is never compared with anything. The last/used return values are not used.

-- rio_mport_cdev.c calls dmaengine_submit and stores the cookie into a request object. That's then passed into dma_async_is_tx_complete later and the last/used parameters are not captured. This cookie is only compared to other cookies using equality, so this one is safe.

-- omap_vout_vrfb.c is the same story as stm32-hash.c. The cookie is only used within a single function and it's never compared to another cookie.

-- pxa_camera.c does appear to rely on cookie values monotonically increasing. Except we get off easy here, because this driver only works with dma channels for one specific provider. It can't use just any provider. This particular provider elects to make it's cookies monotonically increasing still, so nothing breaks. In general, I have some real concerns on layering in this driver since the DMA engine it's using does not appear to be generic and instead only works with this particular camera device. I don't feel like it should be using the dmaengine framework at all.

- dmaengine_submit returns a cookie to the user, and the remaining uses of the cookie are embedded into structs. To audit these I created a patch that changes the type of dma_cookie_t to a struct like so:

typedef struct {
    s32 val;
} dma_cookie_t;

I then fixed up the utility functions in the dmaengine framework, commented out all of the printk stuff that was complaining about casting a struct to %d, and let the compiler find all of the places where math or comparisons were performed on it. Filtering out comparisons against 0 to detect errors, assignments to negative values, equality comparisons to other cookies, and any uses by DMA providers, which all still work after this patch series, we're left with... nothing.

So the summary is:

- pxa_camera is the only client that cares about the cookie behavior, but it's tied in to exactly one provider that happens to do the cookies the way it wants. This patch series doesn't force any provider to change what it does currently.

It really is the case that none of the clients care about the cookie behavior, and we really can just make a documentation change to modify cookies to become opaque handles.

Quoted 13 lines

Quoted 13 lines
Immediately below here the documentation then says that there's some convenience functions that providers can use that do produce monotonically increasing cookies. These are now optional for providers to use, if they find them useful, rather than the required way to manage the cookies.
Quoted 2 lines
Completion is always in order unless we specify DMA_COMPLETION_NO_ORDER
The final patch in this series eliminates DMA_COMPLETION_NO_ORDER entirely. It was only used by the IDXD driver, and the reason I'm doing these patches is so that we can poll the IDXD driver for completions even though it can complete out of order.

Ben Walker

author

2022-08-29 20:36 UTC #46 lore raw

PATCH v5 5/7 dmaengine: idxd: idxd_desc.id is now a u16

In reply to Ben Walker

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(-)

drivers/dma/idxd/idxd.h +1 -1

index fed0dfc1eaa83..bd93ada32c89d 100644
@@ -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;
 };

Signature · 1 line

2.37.1

Ben Walker

author

2022-08-29 20:36 UTC #47 lore raw

PATCH v5 6/7 dmaengine: idxd: Support device_tx_status

In reply to Ben Walker

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

Cc Dave Jiang <[email protected]>

Cc Fenghua Yu <[email protected]>

Signed-off-by Ben Walker <[email protected]>

---
 drivers/dma/idxd/device.c |  1 +
 drivers/dma/idxd/dma.c    | 85 ++++++++++++++++++++++++++++++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 3 files changed, 85 insertions(+), 2 deletions(-)

drivers/dma/idxd/device.c +1 -0

index 5a8cc52c1abfd..752544bef4551 100644
@@ -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;
 	}

drivers/dma/idxd/dma.c +83 -2

109 lines not on this page · raw message

drivers/dma/idxd/idxd.h +1 -0

index bd93ada32c89d..d4f0227895075 100644
@@ -326,6 +326,7 @@ struct idxd_desc {
 	struct llist_node llnode;
 	struct list_head list;
 	u16 id;
+	u16 gen;
 	int cpu;
 	struct idxd_wq *wq;
 };

Signature · 1 line

2.37.1

Dave Jiang

reviewer

2022-09-01 17:24 UTC #48 lore raw

Re: [PATCH v5 6/7] dmaengine: idxd: Support device_tx_status

Quoted 53 lines · Ben Walker

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

Cc Dave Jiang <[email protected]>

Cc Fenghua Yu <[email protected]>

Signed-off-by Ben Walker <[email protected]>

---
  drivers/dma/idxd/device.c |  1 +
  drivers/dma/idxd/dma.c    | 85 ++++++++++++++++++++++++++++++++++++++-
  drivers/dma/idxd/idxd.h   |  1 +
  3 files changed, 85 insertions(+), 2 deletions(-)

drivers/dma/idxd/device.c +1 -0

index 5a8cc52c1abfd..752544bef4551 100644
@@ -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;
  	}

drivers/dma/idxd/dma.c +17 -0

index e0874cb4721c8..dda5342d273f4 100644
@@ -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;

drivers/dma/idxd/dma.c static void idxd_dma_free_chan_resources(struct dma_chan *chan) · line 178

Quoted 4 lines

drivers/dma/idxd/dma.c +1 -0

@@ -158,13 +175,67 @@ static void idxd_dma_free_chan_resources(struct dma_chan *chan)
  		idxd_wq_refcount(wq));
  }
 
+

Stray blank line

Quoted 95 lines

drivers/dma/idxd/dma.c +65 -2

  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 +252,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) {

drivers/dma/idxd/idxd.h +1 -0

index bd93ada32c89d..d4f0227895075 100644
@@ -326,6 +326,7 @@ struct idxd_desc {
  	struct llist_node llnode;
  	struct list_head list;
  	u16 id;
+	u16 gen;
  	int cpu;
  	struct idxd_wq *wq;
  };

Ben Walker

author

2022-08-29 20:36 UTC #49 lore raw

PATCH v5 7/7 dmaengine: Revert "cookie bypass for out of order completion"

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/provider.rst +0 ~~-19~~

index a5539f816d125..8d1510c8cb66f 100644
@@ -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

drivers/dma/dmatest.c +1 ~~-10~~

index dde7b9b626336..47db4748d3e51 100644
@@ -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);

drivers/dma/idxd/dma.c +0 -1

index dda5342d273f4..49e863abd50cd 100644
@@ -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;

include/linux/dmaengine.h +0 -2

index 0ee21887b3924..abaf24d953849 100644
@@ -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 */

Signature · 1 line

2.37.1

Dave Jiang

reviewer

2022-09-01 17:26 UTC #50 lore raw

In reply to Ben Walker

Quoted 42 lines · Ben Walker

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"

Quoted 10 lines

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

Ben Walker

author

2022-10-28 20:48 UTC #51 lore raw

v6 cover dmaengine: Support polling for out of order completions

In reply to Ben Walker

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

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

 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(-)

Signature · 1 line

2.37.1

Ben Walker

author

2022-10-28 20:48 UTC #52 lore raw

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(-)

Documentation/driver-api/dmaengine/client.rst +2 -3

index bfd057b21a000..85ecec2c40005 100644
@@ -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::

drivers/dma/dmaengine.h +9 -1

index 53f16d3f00294..a2ce377e9ed0f 100644
@@ -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)

include/linux/dmaengine.h +3 ~~-25~~

index c923f4e60f240..c55dcae7dc620 100644
@@ -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)
 {

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:48 UTC #53 lore raw

PATCH v6 2/7 dmaengine: Move dma_set_tx_state to the provider API header

In reply to Ben Walker

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(-)

drivers/dma/dmaengine.h +11 -0

index a2ce377e9ed0f..e72876a512a39 100644
@@ -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)

include/linux/dmaengine.h +0 ~~-11~~

index c55dcae7dc620..5ae881729b620 100644
@@ -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);

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:48 UTC #54 lore raw

PATCH v6 3/7 dmaengine: Add dmaengine_is_tx_complete

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/client.rst +4 ~~-15~~

index 85ecec2c40005..9e737041d65ea 100644
@@ -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

Documentation/driver-api/dmaengine/provider.rst +3 -3

index ceac2a300e328..1d0da2777921d 100644
@@ -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

drivers/dma/dmaengine.c +1 -1

index c741b6431958c..74bc92e51a5a7 100644
@@ -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;

drivers/dma/dmatest.c +1 -2

index ffe621695e472..76a027e95d2aa 100644
@@ -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) {

include/linux/dmaengine.h +16 -0

index 5ae881729b620..72b7d51fe41de 100644
@@ -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);

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:48 UTC #55 lore raw

PATCH v6 4/7 dmaengine: Add provider documentation on cookie assignment

In reply to Ben Walker

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by Ben Walker <[email protected]>

Reviewed-by Dave Jiang <[email protected]>

---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

Documentation/driver-api/dmaengine/provider.rst +37 -8

index 1d0da2777921d..a5539f816d125 100644
@@ -417,7 +417,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a

@@ -522,6 +524,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========

@@ -537,13 +573,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- 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
 
 - If clear, the descriptor cannot be reused by provider until the

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:48 UTC #56 lore raw

PATCH v6 5/7 dmaengine: idxd: idxd_desc.id is now a u16

In reply to Ben Walker

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(-)

drivers/dma/idxd/idxd.h +1 -1

index 1196ab342f011..d9096dfb27422 100644
@@ -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;
 };

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:48 UTC #57 lore raw

PATCH v6 6/7 dmaengine: idxd: Support device_tx_status

In reply to Ben Walker

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(-)

drivers/dma/idxd/device.c +1 -0

index 2c1e6f6daa628..870e7adfdd240 100644
@@ -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;
 	}

drivers/dma/idxd/dma.c +82 -2

104 lines not on this page · raw message

drivers/dma/idxd/idxd.h +1 -0

index d9096dfb27422..739c55e56502c 100644
@@ -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;
 };

Signature · 1 line

2.37.3

Ben Walker

author

2022-10-28 20:49 UTC #58 lore raw

PATCH v6 7/7 dmaengine: Revert "cookie bypass for out of order completion"

In reply to Ben Walker

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(-)

Documentation/driver-api/dmaengine/provider.rst +0 ~~-19~~

index a5539f816d125..8d1510c8cb66f 100644
@@ -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

drivers/dma/dmatest.c +1 ~~-10~~

index 76a027e95d2aa..2febc179a7074 100644
@@ -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);

drivers/dma/idxd/dma.c +0 -1

index 87749efec311b..071aafec3de1b 100644
@@ -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;

include/linux/dmaengine.h +0 -2

index 72b7d51fe41de..59c7f67cb3b5a 100644
@@ -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 */

Signature · 1 line

2.37.3