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

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

[Ben Walker]

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

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

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

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

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

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

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

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

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

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

Cc:linux-crypto@vger.kernel.org
Cc:linux-stm32@st-md-mailman.stormreply.com
Cc:linux-media@vger.kernel.org
Cc:mporter@kernel.crashing.org
-- 
2.35.1

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

[Ben Walker]

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 <benjamin.walker@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst |  5 ++--
 drivers/dma/amba-pl08x.c                      |  1 -
 drivers/dma/at_hdmac.c                        |  3 +-
 drivers/dma/dmaengine.h                       | 10 ++++++-
 include/linux/dmaengine.h                     | 28 ++-----------------
 5 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index bfd057b21a000..85ecec2c40005 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -346,9 +346,8 @@ Further APIs
    the documentation in include/linux/dmaengine.h for a more complete
    description of this API.
 
-   This can be used in conjunction with dma_async_is_complete() and
-   the cookie returned from dmaengine_submit() to check for
-   completion of a specific DMA transaction.
+   This can be used with the cookie returned from dmaengine_submit()
+   to check for completion of a specific DMA transaction.
 
    .. note::
 
diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index a4a794e62ac26..bd361aee07db8 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1536,7 +1536,6 @@ static void pl08x_free_chan_resources(struct dma_chan *chan)
 }
 
 /*
- * Code accessing dma_async_is_complete() in a tight loop may give problems.
  * If slaves are relying on interrupts to signal completion this function
  * must not be called with interrupts disabled.
  */
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 5a50423b7378e..5ec9a36074771 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1491,8 +1491,7 @@ static int atc_terminate_all(struct dma_chan *chan)
  * @txstate: if not %NULL updated with transaction state
  *
  * If @txstate is passed in, upon return it reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * internal state.
  */
 static enum dma_status
 atc_tx_status(struct dma_chan *chan,
diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f00294..a2ce377e9ed0f 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -79,7 +79,15 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 		state->residue = 0;
 		state->in_flight_bytes = 0;
 	}
-	return dma_async_is_complete(cookie, complete, used);
+
+	if (complete <= used) {
+		if ((cookie <= complete) || (cookie > used))
+			return DMA_COMPLETE;
+	} else {
+		if ((cookie <= complete) && (cookie > used))
+			return DMA_COMPLETE;
+	}
+	return DMA_IN_PROGRESS;
 }
 
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b46b88e6aa0d1..ea6ec2666eb15 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1446,9 +1446,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
  * @last: returns last completed cookie, can be NULL
  * @used: returns last issued cookie, can be NULL
  *
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
  */
 static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1464,28 +1464,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
-			dma_cookie_t last_complete, dma_cookie_t last_used)
-{
-	if (last_complete <= last_used) {
-		if ((cookie <= last_complete) || (cookie > last_used))
-			return DMA_COMPLETE;
-	} else {
-		if ((cookie <= last_complete) && (cookie > last_used))
-			return DMA_COMPLETE;
-	}
-	return DMA_IN_PROGRESS;
-}
-
 static inline void
 dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
 {
-- 
2.35.1

[PATCH v4 02/15] dmaengine: Move dma_set_tx_state to the provider API header

[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 <benjamin.walker@intel.com>
---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index a2ce377e9ed0f..e72876a512a39 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -90,6 +90,17 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 	return DMA_IN_PROGRESS;
 }
 
+static inline void dma_set_tx_state(struct dma_tx_state *st,
+	dma_cookie_t last, dma_cookie_t used, u32 residue)
+{
+	if (!st)
+		return;
+
+	st->last = last;
+	st->used = used;
+	st->residue = residue;
+}
+
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
 {
 	if (state)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index ea6ec2666eb15..f968f7671e22c 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -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);
-- 
2.35.1

[PATCH v4 03/15] dmaengine: Add dmaengine_async_is_tx_complete

[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 <benjamin.walker@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
 .../driver-api/dmaengine/provider.rst         |  6 +++---
 drivers/dma/dmaengine.c                       |  2 +-
 drivers/dma/dmatest.c                         |  3 +--
 include/linux/dmaengine.h                     | 16 ++++++++++++++++
 5 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index 85ecec2c40005..9ae489a4ca97f 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -259,8 +259,8 @@ The details of these operations are:
 
       dma_cookie_t dmaengine_submit(struct dma_async_tx_descriptor *desc)
 
-   This returns a cookie can be used to check the progress of DMA engine
-   activity via other DMA engine calls not covered in this document.
+   This returns a cookie that can be used to check the progress of a transaction
+   via dmaengine_async_is_tx_complete().
 
    dmaengine_submit() will not start the DMA operation, it merely adds
    it to the pending queue. For this, see step 5, dma_async_issue_pending.
@@ -339,23 +339,12 @@ Further APIs
 
    .. code-block:: c
 
-      enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
-		dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
-
-   This can be used to check the status of the channel. Please see
-   the documentation in include/linux/dmaengine.h for a more complete
-   description of this API.
+      enum dma_status dmaengine_async_is_tx_complete(struct dma_chan *chan,
+		dma_cookie_t cookie)
 
    This can be used with the cookie returned from dmaengine_submit()
    to check for completion of a specific DMA transaction.
 
-   .. note::
-
-      Not all DMA engine drivers can return reliable information for
-      a running DMA channel. It is recommended that DMA engine users
-      pause or stop (via dmaengine_terminate_all()) the channel before
-      using this API.
-
 5. Synchronize termination API
 
    .. code-block:: c
diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index 1e0f1f85d10e5..610309276bc9c 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -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
 
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index e80feeea0e018..f67a13c29d3f9 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -523,7 +523,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
 
 	dma_async_issue_pending(chan);
 	do {
-		status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
+		status = dmaengine_async_is_tx_complete(chan, cookie);
 		if (time_after_eq(jiffies, dma_sync_wait_timeout)) {
 			dev_err(chan->device->dev, "%s: timeout!\n", __func__);
 			return DMA_ERROR;
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 0a2168a4ccb0c..3ee47a72bf9d7 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -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) {
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index f968f7671e22c..44eb1df433e61 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -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);
-- 
2.35.1

[PATCH v4 04/15] crypto: stm32/hash: Use dmaengine_async_is_tx_complete

[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: linux-crypto@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/crypto/stm32/stm32-hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
index d33006d43f761..aef447847c499 100644
--- a/drivers/crypto/stm32/stm32-hash.c
+++ b/drivers/crypto/stm32/stm32-hash.c
@@ -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) {
-- 
2.35.1

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

[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: linux-media@vger.kernel.org
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/media/platform/ti/omap/omap_vout_vrfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti/omap/omap_vout_vrfb.c b/drivers/media/platform/ti/omap/omap_vout_vrfb.c
index 0cfa0169875f0..b9d252d5ced7a 100644
--- a/drivers/media/platform/ti/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/ti/omap/omap_vout_vrfb.c
@@ -289,7 +289,7 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
 					 vout->vrfb_dma_tx.tx_status == 1,
 					 VRFB_TX_TIMEOUT);
 
-	status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
+	status = dmaengine_async_is_tx_complete(chan, cookie);
 
 	if (vout->vrfb_dma_tx.tx_status == 0) {
 		pr_err("%s: Timeout while waiting for DMA\n", __func__);
-- 
2.35.1

[PATCH v4 06/15] rapidio: Use dmaengine_async_is_tx_complete

[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: mporter@kernel.crashing.org
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/rapidio/devices/rio_mport_cdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index 2cdc054e53a53..d4108ff2bb74c 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -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);
 }
-- 
2.35.1

[PATCH v4 07/15] media: pxa_camera: Use dmaengine_async_is_tx_complete

[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: linux-media@vger.kernel.org
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/media/platform/intel/pxa_camera.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/intel/pxa_camera.c b/drivers/media/platform/intel/pxa_camera.c
index 35145e3348f0e..0880bf2baa339 100644
--- a/drivers/media/platform/intel/pxa_camera.c
+++ b/drivers/media/platform/intel/pxa_camera.c
@@ -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",
-- 
2.35.1

[PATCH v4 08/15] dmaengine: Remove dma_async_is_tx_complete

[Ben Walker]

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

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 include/linux/dmaengine.h | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 44eb1df433e61..c34f21d19c423 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -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
-- 
2.35.1

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

[Ben Walker]

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

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/dma/dmaengine.h   | 4 ----
 include/linux/dmaengine.h | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index e72876a512a39..08c7bd7cfc229 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -74,8 +74,6 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 	complete = chan->completed_cookie;
 	barrier();
 	if (state) {
-		state->last = complete;
-		state->used = used;
 		state->residue = 0;
 		state->in_flight_bytes = 0;
 	}
@@ -96,8 +94,6 @@ static inline void dma_set_tx_state(struct dma_tx_state *st,
 	if (!st)
 		return;
 
-	st->last = last;
-	st->used = used;
 	st->residue = residue;
 }
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c34f21d19c423..e3e5311b6bb64 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -716,16 +716,12 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr
 /**
  * struct dma_tx_state - filled in to report the status of
  * a transfer.
- * @last: last completed DMA cookie
- * @used: last issued DMA cookie (i.e. the one in progress)
  * @residue: the remaining number of bytes left to transmit
  *	on the selected transfer for states DMA_IN_PROGRESS and
  *	DMA_PAUSED if this is implemented in the driver, else 0
  * @in_flight_bytes: amount of data in bytes cached by the DMA.
  */
 struct dma_tx_state {
-	dma_cookie_t last;
-	dma_cookie_t used;
 	u32 residue;
 	u32 in_flight_bytes;
 };
-- 
2.35.1

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

[Ben Walker]

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

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 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(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8535018ee7a2e..f2de1836dbd34 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -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;
 }
diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
index a262e0eb4cc94..753b431ca206b 100644
--- a/drivers/dma/mmp_tdma.c
+++ b/drivers/dma/mmp_tdma.c
@@ -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;
 }
diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 994fc4d2aca42..ab9eca6d682dc 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -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;
 }
-- 
2.35.1

[PATCH v4 11/15] dmaengine: Remove dma_set_tx_state

[Ben Walker]

Nothing calls this anymore.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/dma/dmaengine.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 08c7bd7cfc229..7a5203175e6a8 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -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)
-- 
2.35.1

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

[Ben Walker]

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index 610309276bc9c..db019ec492b58 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -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
-- 
2.35.1

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

[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 <benjamin.walker@intel.com>
---
 drivers/dma/idxd/idxd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[PATCH v4 14/15] dmaengine: idxd: Support device_tx_status

[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 <benjamin.walker@intel.com>
---
 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(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index ff0ea60051f0c..3c61eac0b05c7 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -148,6 +148,7 @@ int idxd_wq_alloc_resources(struct idxd_wq *wq)
 			desc->iax_completion = &wq->iax_compls[i];
 		desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
 		desc->id = i;
+		desc->gen = 1;
 		desc->wq = wq;
 		desc->cpu = -1;
 	}
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index e0874cb4721c8..dda5342d273f4 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -12,6 +12,23 @@
 #include "registers.h"
 #include "idxd.h"
 
+
+#define DMA_COOKIE_BITS (sizeof(dma_cookie_t) * 8)
+/*
+ * The descriptor id takes the lower 16 bits of the cookie.
+ */
+#define DESC_ID_BITS 16
+#define DESC_ID_MASK ((1 << DESC_ID_BITS) - 1)
+/*
+ * The 'generation' is in the upper half of the cookie. But dma_cookie_t
+ * is signed, so we leave the upper-most bit for the sign. Further, we
+ * need to flag whether a cookie corresponds to an operation that is
+ * being completed via interrupt to avoid polling it, which takes
+ * the second most upper bit. So we subtract two bits from the upper half.
+ */
+#define DESC_GEN_MAX ((1 << (DMA_COOKIE_BITS - DESC_ID_BITS - 2)) - 1)
+#define DESC_INTERRUPT_FLAG (1 << (DMA_COOKIE_BITS - 2))
+
 static inline struct idxd_wq *to_idxd_wq(struct dma_chan *c)
 {
 	struct idxd_dma_chan *idxd_chan;
@@ -158,13 +175,67 @@ static void idxd_dma_free_chan_resources(struct dma_chan *chan)
 		idxd_wq_refcount(wq));
 }
 
+
 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) {
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index bd93ada32c89d..d4f0227895075 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -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;
 };
-- 
2.35.1

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

[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 <benjamin.walker@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 19 -------------------
 drivers/dma/dmatest.c                         | 11 +----------
 drivers/dma/idxd/dma.c                        |  1 -
 include/linux/dmaengine.h                     |  2 --
 4 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index db019ec492b58..8565241270a62 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -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
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 3ee47a72bf9d7..d34e7a9b63d89 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -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);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index dda5342d273f4..49e863abd50cd 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -297,7 +297,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)
 
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->cap_mask);
-	dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
 	dma->device_release = idxd_dma_release;
 
 	dma->device_prep_dma_interrupt = idxd_dma_prep_interrupt;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index e3e5311b6bb64..136c7afbcc385 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,7 +39,6 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
-	DMA_OUT_OF_ORDER,
 };
 
 /**
@@ -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 */
-- 
2.35.1

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

[Vinod Koul]

Hi Ben,


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

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

-- 
~Vinod

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

[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"

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

-- 
2.37.1

[PATCH v5 1/7] dmaengine: Remove dma_async_is_complete from client API

[Ben Walker]

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 <benjamin.walker@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst |  5 ++--
 drivers/dma/dmaengine.h                       | 10 ++++++-
 include/linux/dmaengine.h                     | 28 ++-----------------
 3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index bfd057b21a000..85ecec2c40005 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -346,9 +346,8 @@ Further APIs
    the documentation in include/linux/dmaengine.h for a more complete
    description of this API.
 
-   This can be used in conjunction with dma_async_is_complete() and
-   the cookie returned from dmaengine_submit() to check for
-   completion of a specific DMA transaction.
+   This can be used with the cookie returned from dmaengine_submit()
+   to check for completion of a specific DMA transaction.
 
    .. note::
 
diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f00294..a2ce377e9ed0f 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -79,7 +79,15 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 		state->residue = 0;
 		state->in_flight_bytes = 0;
 	}
-	return dma_async_is_complete(cookie, complete, used);
+
+	if (complete <= used) {
+		if ((cookie <= complete) || (cookie > used))
+			return DMA_COMPLETE;
+	} else {
+		if ((cookie <= complete) && (cookie > used))
+			return DMA_COMPLETE;
+	}
+	return DMA_IN_PROGRESS;
 }
 
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c923f4e60f240..c55dcae7dc620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1426,9 +1426,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
  * @last: returns last completed cookie, can be NULL
  * @used: returns last issued cookie, can be NULL
  *
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
  */
 static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1444,28 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
-			dma_cookie_t last_complete, dma_cookie_t last_used)
-{
-	if (last_complete <= last_used) {
-		if ((cookie <= last_complete) || (cookie > last_used))
-			return DMA_COMPLETE;
-	} else {
-		if ((cookie <= last_complete) && (cookie > last_used))
-			return DMA_COMPLETE;
-	}
-	return DMA_IN_PROGRESS;
-}
-
 static inline void
 dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
 {
-- 
2.37.1

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

[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 <benjamin.walker@intel.com>
---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index a2ce377e9ed0f..e72876a512a39 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -90,6 +90,17 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 	return DMA_IN_PROGRESS;
 }
 
+static inline void dma_set_tx_state(struct dma_tx_state *st,
+	dma_cookie_t last, dma_cookie_t used, u32 residue)
+{
+	if (!st)
+		return;
+
+	st->last = last;
+	st->used = used;
+	st->residue = residue;
+}
+
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
 {
 	if (state)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c55dcae7dc620..5ae881729b620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1444,17 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-static inline void
-dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
-{
-	if (!st)
-		return;
-
-	st->last = last;
-	st->used = used;
-	st->residue = residue;
-}
-
 #ifdef CONFIG_DMA_ENGINE
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
 enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
-- 
2.37.1

[PATCH v5 3/7] dmaengine: Add dmaengine_async_is_tx_complete

[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 <benjamin.walker@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
 .../driver-api/dmaengine/provider.rst         |  6 +++---
 drivers/dma/dmaengine.c                       |  2 +-
 drivers/dma/dmatest.c                         |  3 +--
 include/linux/dmaengine.h                     | 16 ++++++++++++++++
 5 files changed, 25 insertions(+), 21 deletions(-)

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

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

[Vinod Koul]

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

Thanks for this cleanup. This is good :)

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

we _really_ need to drop async and have everything dmaengine_*

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

-- 
~Vinod

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

[Walker, Benjamin]

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

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

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

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

[Ben Walker]

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index 1d0da2777921d..a5539f816d125 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -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
-- 
2.37.1

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

[Vinod Koul]

On 29-08-22, 13:35, Ben Walker wrote:
> Clarify the rules on assigning cookies to DMA transactions.
> 
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
>  .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
>  1 file changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> index 1d0da2777921d..a5539f816d125 100644
> --- a/Documentation/driver-api/dmaengine/provider.rst
> +++ b/Documentation/driver-api/dmaengine/provider.rst
> @@ -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

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

> +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
> -- 
> 2.37.1

-- 
~Vinod

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

[Walker, Benjamin]

On 10/19/2022 9:34 AM, Vinod Koul wrote:
> On 29-08-22, 13:35, Ben Walker wrote:
>> Clarify the rules on assigning cookies to DMA transactions.
>>
>> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
>> ---
>>   .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
>>   1 file changed, 37 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
>> index 1d0da2777921d..a5539f816d125 100644
>> --- a/Documentation/driver-api/dmaengine/provider.rst
>> +++ b/Documentation/driver-api/dmaengine/provider.rst
>> @@ -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
> 
> 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.

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

> 
>> +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
>> -- 
>> 2.37.1
> 

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

[Vinod Koul]

On 19-10-22, 10:21, Walker, Benjamin wrote:
> On 10/19/2022 9:34 AM, Vinod Koul wrote:
> > On 29-08-22, 13:35, Ben Walker wrote:
> > > Clarify the rules on assigning cookies to DMA transactions.
> > > 
> > > Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> > > ---
> > >   .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
> > >   1 file changed, 37 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> > > index 1d0da2777921d..a5539f816d125 100644
> > > --- a/Documentation/driver-api/dmaengine/provider.rst
> > > +++ b/Documentation/driver-api/dmaengine/provider.rst
> > > @@ -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
> > 
> > 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.

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

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

[Walker, Benjamin]

On 10/19/2022 9:12 PM, Vinod Koul wrote:
> On 19-10-22, 10:21, Walker, Benjamin wrote:
>> On 10/19/2022 9:34 AM, Vinod Koul wrote:
>>> On 29-08-22, 13:35, Ben Walker wrote:
>>>> Clarify the rules on assigning cookies to DMA transactions.
>>>>
>>>> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
>>>> ---
>>>>    .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
>>>>    1 file changed, 37 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
>>>> index 1d0da2777921d..a5539f816d125 100644
>>>> --- a/Documentation/driver-api/dmaengine/provider.rst
>>>> +++ b/Documentation/driver-api/dmaengine/provider.rst
>>>> @@ -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
>>>
>>> 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.
> 
> 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.

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

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

[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 <dave.jiang@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 drivers/dma/idxd/idxd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[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 <dave.jiang@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 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(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 5a8cc52c1abfd..752544bef4551 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -148,6 +148,7 @@ int idxd_wq_alloc_resources(struct idxd_wq *wq)
 			desc->iax_completion = &wq->iax_compls[i];
 		desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
 		desc->id = i;
+		desc->gen = 1;
 		desc->wq = wq;
 		desc->cpu = -1;
 	}
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index e0874cb4721c8..dda5342d273f4 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -12,6 +12,23 @@
 #include "registers.h"
 #include "idxd.h"
 
+
+#define DMA_COOKIE_BITS (sizeof(dma_cookie_t) * 8)
+/*
+ * The descriptor id takes the lower 16 bits of the cookie.
+ */
+#define DESC_ID_BITS 16
+#define DESC_ID_MASK ((1 << DESC_ID_BITS) - 1)
+/*
+ * The 'generation' is in the upper half of the cookie. But dma_cookie_t
+ * is signed, so we leave the upper-most bit for the sign. Further, we
+ * need to flag whether a cookie corresponds to an operation that is
+ * being completed via interrupt to avoid polling it, which takes
+ * the second most upper bit. So we subtract two bits from the upper half.
+ */
+#define DESC_GEN_MAX ((1 << (DMA_COOKIE_BITS - DESC_ID_BITS - 2)) - 1)
+#define DESC_INTERRUPT_FLAG (1 << (DMA_COOKIE_BITS - 2))
+
 static inline struct idxd_wq *to_idxd_wq(struct dma_chan *c)
 {
 	struct idxd_dma_chan *idxd_chan;
@@ -158,13 +175,67 @@ static void idxd_dma_free_chan_resources(struct dma_chan *chan)
 		idxd_wq_refcount(wq));
 }
 
+
 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) {
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index bd93ada32c89d..d4f0227895075 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -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;
 };
-- 
2.37.1

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

[Dave Jiang]

On 8/29/2022 1:35 PM, Ben Walker wrote:
> This can now be supported even for devices that complete operations out
> of order. Add support for directly polling transactions.
>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
>   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(-)
>
> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
> index 5a8cc52c1abfd..752544bef4551 100644
> --- a/drivers/dma/idxd/device.c
> +++ b/drivers/dma/idxd/device.c
> @@ -148,6 +148,7 @@ int idxd_wq_alloc_resources(struct idxd_wq *wq)
>   			desc->iax_completion = &wq->iax_compls[i];
>   		desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
>   		desc->id = i;
> +		desc->gen = 1;
>   		desc->wq = wq;
>   		desc->cpu = -1;
>   	}
> diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
> index e0874cb4721c8..dda5342d273f4 100644
> --- a/drivers/dma/idxd/dma.c
> +++ b/drivers/dma/idxd/dma.c
> @@ -12,6 +12,23 @@
>   #include "registers.h"
>   #include "idxd.h"
>   
> +
> +#define DMA_COOKIE_BITS (sizeof(dma_cookie_t) * 8)
> +/*
> + * The descriptor id takes the lower 16 bits of the cookie.
> + */
> +#define DESC_ID_BITS 16
> +#define DESC_ID_MASK ((1 << DESC_ID_BITS) - 1)
> +/*
> + * The 'generation' is in the upper half of the cookie. But dma_cookie_t
> + * is signed, so we leave the upper-most bit for the sign. Further, we
> + * need to flag whether a cookie corresponds to an operation that is
> + * being completed via interrupt to avoid polling it, which takes
> + * the second most upper bit. So we subtract two bits from the upper half.
> + */
> +#define DESC_GEN_MAX ((1 << (DMA_COOKIE_BITS - DESC_ID_BITS - 2)) - 1)
> +#define DESC_INTERRUPT_FLAG (1 << (DMA_COOKIE_BITS - 2))
> +
>   static inline struct idxd_wq *to_idxd_wq(struct dma_chan *c)
>   {
>   	struct idxd_dma_chan *idxd_chan;
> @@ -158,13 +175,67 @@ static void idxd_dma_free_chan_resources(struct dma_chan *chan)
>   		idxd_wq_refcount(wq));
>   }
>   
> +

Stray blank line

DJ

>   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) {
> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
> index bd93ada32c89d..d4f0227895075 100644
> --- a/drivers/dma/idxd/idxd.h
> +++ b/drivers/dma/idxd/idxd.h
> @@ -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;
>   };

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

[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 <benjamin.walker@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 19 -------------------
 drivers/dma/dmatest.c                         | 11 +----------
 drivers/dma/idxd/dma.c                        |  1 -
 include/linux/dmaengine.h                     |  2 --
 4 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index a5539f816d125..8d1510c8cb66f 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -258,22 +258,6 @@ Currently, the types available are:
     want to transfer a portion of uncompressed data directly to the
     display to print it
 
-- DMA_COMPLETION_NO_ORDER
-
-  - The device does not support in order completion.
-
-  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
-    the device is setting this capability.
-
-  - All cookie tracking and checking API should be treated as invalid if
-    the device exports this capability.
-
-  - At this point, this is incompatible with polling option for dmatest.
-
-  - If this cap is set, the user is recommended to provide an unique
-    identifier for each descriptor sent to the DMA device in order to
-    properly track the completion.
-
 - DMA_REPEAT
 
   - The device supports repeated transfers. A repeated transfer, indicated by
@@ -457,9 +441,6 @@ supported.
   - In the case of a cyclic transfer, it should only take into
     account the total size of the cyclic buffer.
 
-  - Should return DMA_OUT_OF_ORDER if the device does not support in order
-    completion and is completing the operation out of order.
-
   - This function can be called in an interrupt context.
 
 - device_config
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index dde7b9b626336..47db4748d3e51 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -838,10 +838,7 @@ static int dmatest_func(void *data)
 			result("test timed out", total_tests, src->off, dst->off,
 			       len, 0);
 			goto error_unmap_continue;
-		} else if (status != DMA_COMPLETE &&
-			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
-					 dev->cap_mask) &&
-			     status == DMA_OUT_OF_ORDER)) {
+		} else if (status != DMA_COMPLETE) {
 			result(status == DMA_ERROR ?
 			       "completion error status" :
 			       "completion busy status", total_tests, src->off,
@@ -1019,12 +1016,6 @@ static int dmatest_add_channel(struct dmatest_info *info,
 	dtc->chan = chan;
 	INIT_LIST_HEAD(&dtc->threads);
 
-	if (dma_has_cap(DMA_COMPLETION_NO_ORDER, dma_dev->cap_mask) &&
-	    info->params.polled) {
-		info->params.polled = false;
-		pr_warn("DMA_COMPLETION_NO_ORDER, polled disabled\n");
-	}
-
 	if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
 		if (dmatest == 0) {
 			cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index dda5342d273f4..49e863abd50cd 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -297,7 +297,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)
 
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->cap_mask);
-	dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
 	dma->device_release = idxd_dma_release;
 
 	dma->device_prep_dma_interrupt = idxd_dma_prep_interrupt;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0ee21887b3924..abaf24d953849 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,7 +39,6 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
-	DMA_OUT_OF_ORDER,
 };
 
 /**
@@ -62,7 +61,6 @@ enum dma_transaction_type {
 	DMA_SLAVE,
 	DMA_CYCLIC,
 	DMA_INTERLEAVE,
-	DMA_COMPLETION_NO_ORDER,
 	DMA_REPEAT,
 	DMA_LOAD_EOT,
 /* last transaction type for creation of the capabilities mask */
-- 
2.37.1

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

[Dave Jiang]

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

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

[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 5:
 - Rebased to 6.1
 - Renamed dmaengine_async_is_tx_complete to dmaengine_is_tx_complete
 - Fixed stray blank line above idxd_dma_tx_status
 - Added Reviewed-by from Dave Jiang

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

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

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

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

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

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

-- 
2.37.1

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

[Ben Walker]

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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst |  5 ++--
 drivers/dma/dmaengine.h                       | 10 ++++++-
 include/linux/dmaengine.h                     | 28 ++-----------------
 3 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index bfd057b21a000..85ecec2c40005 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -346,9 +346,8 @@ Further APIs
    the documentation in include/linux/dmaengine.h for a more complete
    description of this API.
 
-   This can be used in conjunction with dma_async_is_complete() and
-   the cookie returned from dmaengine_submit() to check for
-   completion of a specific DMA transaction.
+   This can be used with the cookie returned from dmaengine_submit()
+   to check for completion of a specific DMA transaction.
 
    .. note::
 
diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index 53f16d3f00294..a2ce377e9ed0f 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -79,7 +79,15 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 		state->residue = 0;
 		state->in_flight_bytes = 0;
 	}
-	return dma_async_is_complete(cookie, complete, used);
+
+	if (complete <= used) {
+		if ((cookie <= complete) || (cookie > used))
+			return DMA_COMPLETE;
+	} else {
+		if ((cookie <= complete) && (cookie > used))
+			return DMA_COMPLETE;
+	}
+	return DMA_IN_PROGRESS;
 }
 
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c923f4e60f240..c55dcae7dc620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1426,9 +1426,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
  * @last: returns last completed cookie, can be NULL
  * @used: returns last issued cookie, can be NULL
  *
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
  */
 static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1444,28 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
-			dma_cookie_t last_complete, dma_cookie_t last_used)
-{
-	if (last_complete <= last_used) {
-		if ((cookie <= last_complete) || (cookie > last_used))
-			return DMA_COMPLETE;
-	} else {
-		if ((cookie <= last_complete) && (cookie > last_used))
-			return DMA_COMPLETE;
-	}
-	return DMA_IN_PROGRESS;
-}
-
 static inline void
 dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
 {
-- 
2.37.3

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

[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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/dmaengine.h   | 11 +++++++++++
 include/linux/dmaengine.h | 11 -----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
index a2ce377e9ed0f..e72876a512a39 100644
--- a/drivers/dma/dmaengine.h
+++ b/drivers/dma/dmaengine.h
@@ -90,6 +90,17 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan,
 	return DMA_IN_PROGRESS;
 }
 
+static inline void dma_set_tx_state(struct dma_tx_state *st,
+	dma_cookie_t last, dma_cookie_t used, u32 residue)
+{
+	if (!st)
+		return;
+
+	st->last = last;
+	st->used = used;
+	st->residue = residue;
+}
+
 static inline void dma_set_residue(struct dma_tx_state *state, u32 residue)
 {
 	if (state)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c55dcae7dc620..5ae881729b620 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1444,17 +1444,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-static inline void
-dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
-{
-	if (!st)
-		return;
-
-	st->last = last;
-	st->used = used;
-	st->residue = residue;
-}
-
 #ifdef CONFIG_DMA_ENGINE
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
 enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
-- 
2.37.3

[PATCH v6 3/7] dmaengine: Add dmaengine_is_tx_complete

[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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst | 19 ++++---------------
 .../driver-api/dmaengine/provider.rst         |  6 +++---
 drivers/dma/dmaengine.c                       |  2 +-
 drivers/dma/dmatest.c                         |  3 +--
 include/linux/dmaengine.h                     | 16 ++++++++++++++++
 5 files changed, 25 insertions(+), 21 deletions(-)

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

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

[Ben Walker]

Clarify the rules on assigning cookies to DMA transactions.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 45 +++++++++++++++----
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index 1d0da2777921d..a5539f816d125 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -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
-- 
2.37.3

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

[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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/idxd/idxd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/idxd/device.c |  1 +
 drivers/dma/idxd/dma.c    | 84 ++++++++++++++++++++++++++++++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 2c1e6f6daa628..870e7adfdd240 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -148,6 +148,7 @@ int idxd_wq_alloc_resources(struct idxd_wq *wq)
 			desc->iax_completion = &wq->iax_compls[i];
 		desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
 		desc->id = i;
+		desc->gen = 1;
 		desc->wq = wq;
 		desc->cpu = -1;
 	}
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index e0874cb4721c8..87749efec311b 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -12,6 +12,23 @@
 #include "registers.h"
 #include "idxd.h"
 
+
+#define DMA_COOKIE_BITS (sizeof(dma_cookie_t) * 8)
+/*
+ * The descriptor id takes the lower 16 bits of the cookie.
+ */
+#define DESC_ID_BITS 16
+#define DESC_ID_MASK ((1 << DESC_ID_BITS) - 1)
+/*
+ * The 'generation' is in the upper half of the cookie. But dma_cookie_t
+ * is signed, so we leave the upper-most bit for the sign. Further, we
+ * need to flag whether a cookie corresponds to an operation that is
+ * being completed via interrupt to avoid polling it, which takes
+ * the second most upper bit. So we subtract two bits from the upper half.
+ */
+#define DESC_GEN_MAX ((1 << (DMA_COOKIE_BITS - DESC_ID_BITS - 2)) - 1)
+#define DESC_INTERRUPT_FLAG (1 << (DMA_COOKIE_BITS - 2))
+
 static inline struct idxd_wq *to_idxd_wq(struct dma_chan *c)
 {
 	struct idxd_dma_chan *idxd_chan;
@@ -162,9 +179,62 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
 					  dma_cookie_t cookie,
 					  struct dma_tx_state *txstate)
 {
-	return DMA_OUT_OF_ORDER;
+	u8 status;
+	struct idxd_wq *wq;
+	struct idxd_desc *desc;
+	u32 idx;
+
+	memset(txstate, 0, sizeof(*txstate));
+
+	if (dma_submit_error(cookie))
+		return DMA_ERROR;
+
+	wq = to_idxd_wq(dma_chan);
+
+	idx = cookie & DESC_ID_MASK;
+	if (idx >= wq->num_descs)
+		return DMA_ERROR;
+
+	desc = wq->descs[idx];
+
+	if (desc->txd.cookie != cookie) {
+		/*
+		 * The user asked about an old transaction
+		 */
+		return DMA_COMPLETE;
+	}
+
+	/*
+	 * For descriptors completed via interrupt, we can't go
+	 * look at the completion status directly because it races
+	 * with the IRQ handler recyling the descriptor. However,
+	 * since in this case we can rely on the interrupt handler
+	 * to invalidate the cookie when the command completes we
+	 * know that if we get here, the command is still in
+	 * progress.
+	 */
+	if ((cookie & DESC_INTERRUPT_FLAG) != 0)
+		return DMA_IN_PROGRESS;
+
+	status = desc->completion->status & DSA_COMP_STATUS_MASK;
+
+	if (status) {
+		/*
+		 * Check against the original status as ABORT is software defined
+		 * and 0xff, which DSA_COMP_STATUS_MASK can mask out.
+		 */
+		if (unlikely(desc->completion->status == IDXD_COMP_DESC_ABORT))
+			idxd_dma_complete_txd(desc, IDXD_COMPLETE_ABORT, true);
+		else
+			idxd_dma_complete_txd(desc, IDXD_COMPLETE_NORMAL, true);
+
+		return DMA_COMPLETE;
+	}
+
+	return DMA_IN_PROGRESS;
 }
 
+
 /*
  * issue_pending() does not need to do anything since tx_submit() does the job
  * already.
@@ -181,7 +251,17 @@ static dma_cookie_t idxd_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 	int rc;
 	struct idxd_desc *desc = container_of(tx, struct idxd_desc, txd);
 
-	cookie = dma_cookie_assign(tx);
+	cookie = (desc->gen << DESC_ID_BITS) | (desc->id & DESC_ID_MASK);
+
+	if ((desc->hw->flags & IDXD_OP_FLAG_RCI) != 0)
+		cookie |= DESC_INTERRUPT_FLAG;
+
+	if (desc->gen == DESC_GEN_MAX)
+		desc->gen = 1;
+	else
+		desc->gen++;
+
+	tx->cookie = cookie;
 
 	rc = idxd_submit_desc(wq, desc);
 	if (rc < 0) {
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index d9096dfb27422..739c55e56502c 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -334,6 +334,7 @@ struct idxd_desc {
 	struct llist_node llnode;
 	struct list_head list;
 	u16 id;
+	u16 gen;
 	int cpu;
 	struct idxd_wq *wq;
 };
-- 
2.37.3

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

[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 <benjamin.walker@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
---
 .../driver-api/dmaengine/provider.rst         | 19 -------------------
 drivers/dma/dmatest.c                         | 11 +----------
 drivers/dma/idxd/dma.c                        |  1 -
 include/linux/dmaengine.h                     |  2 --
 4 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index a5539f816d125..8d1510c8cb66f 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -258,22 +258,6 @@ Currently, the types available are:
     want to transfer a portion of uncompressed data directly to the
     display to print it
 
-- DMA_COMPLETION_NO_ORDER
-
-  - The device does not support in order completion.
-
-  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
-    the device is setting this capability.
-
-  - All cookie tracking and checking API should be treated as invalid if
-    the device exports this capability.
-
-  - At this point, this is incompatible with polling option for dmatest.
-
-  - If this cap is set, the user is recommended to provide an unique
-    identifier for each descriptor sent to the DMA device in order to
-    properly track the completion.
-
 - DMA_REPEAT
 
   - The device supports repeated transfers. A repeated transfer, indicated by
@@ -457,9 +441,6 @@ supported.
   - In the case of a cyclic transfer, it should only take into
     account the total size of the cyclic buffer.
 
-  - Should return DMA_OUT_OF_ORDER if the device does not support in order
-    completion and is completing the operation out of order.
-
   - This function can be called in an interrupt context.
 
 - device_config
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 76a027e95d2aa..2febc179a7074 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -838,10 +838,7 @@ static int dmatest_func(void *data)
 			result("test timed out", total_tests, src->off, dst->off,
 			       len, 0);
 			goto error_unmap_continue;
-		} else if (status != DMA_COMPLETE &&
-			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
-					 dev->cap_mask) &&
-			     status == DMA_OUT_OF_ORDER)) {
+		} else if (status != DMA_COMPLETE) {
 			result(status == DMA_ERROR ?
 			       "completion error status" :
 			       "completion busy status", total_tests, src->off,
@@ -1019,12 +1016,6 @@ static int dmatest_add_channel(struct dmatest_info *info,
 	dtc->chan = chan;
 	INIT_LIST_HEAD(&dtc->threads);
 
-	if (dma_has_cap(DMA_COMPLETION_NO_ORDER, dma_dev->cap_mask) &&
-	    info->params.polled) {
-		info->params.polled = false;
-		pr_warn("DMA_COMPLETION_NO_ORDER, polled disabled\n");
-	}
-
 	if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
 		if (dmatest == 0) {
 			cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index 87749efec311b..071aafec3de1b 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -296,7 +296,6 @@ int idxd_register_dma_device(struct idxd_device *idxd)
 
 	dma_cap_set(DMA_INTERRUPT, dma->cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->cap_mask);
-	dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
 	dma->device_release = idxd_dma_release;
 
 	dma->device_prep_dma_interrupt = idxd_dma_prep_interrupt;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 72b7d51fe41de..59c7f67cb3b5a 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,7 +39,6 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
-	DMA_OUT_OF_ORDER,
 };
 
 /**
@@ -62,7 +61,6 @@ enum dma_transaction_type {
 	DMA_SLAVE,
 	DMA_CYCLIC,
 	DMA_INTERLEAVE,
-	DMA_COMPLETION_NO_ORDER,
 	DMA_REPEAT,
 	DMA_LOAD_EOT,
 /* last transaction type for creation of the capabilities mask */
-- 
2.37.3