[RESEND 00/10] Support polling for out of order completions

[RESEND 00/10] 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 3 of this
series. Please take an extra critical look at that change.

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.

Ben Walker (10):
  dmaengine: Remove dma_async_is_complete from client API
  dmaengine: Move dma_set_tx_state to the provider API header
  dmaengine: Remove the last, used parameters in
    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 | 22 ++---
 .../driver-api/dmaengine/provider.rst         | 64 ++++++++------
 drivers/crypto/stm32/stm32-hash.c             |  3 +-
 drivers/dma/amba-pl08x.c                      |  1 -
 drivers/dma/at_hdmac.c                        |  3 +-
 drivers/dma/dmaengine.c                       |  2 +-
 drivers/dma/dmaengine.h                       | 12 ++-
 drivers/dma/dmatest.c                         | 14 +--
 drivers/dma/idxd/device.c                     |  1 +
 drivers/dma/idxd/dma.c                        | 86 ++++++++++++++++++-
 drivers/dma/idxd/idxd.h                       |  3 +-
 drivers/dma/imx-sdma.c                        |  3 +-
 drivers/dma/mmp_tdma.c                        |  3 +-
 drivers/dma/mxs-dma.c                         |  3 +-
 drivers/media/platform/omap/omap_vout_vrfb.c  |  2 +-
 drivers/media/platform/pxa_camera.c           | 13 ++-
 drivers/rapidio/devices/rio_mport_cdev.c      |  3 +-
 include/linux/dmaengine.h                     | 55 +-----------
 18 files changed, 160 insertions(+), 133 deletions(-)

-- 
2.33.1

[RESEND 01/10] 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 a24882ba37643..fd52556c3edb4 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -1544,7 +1544,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_interrupt(
 }
 
 /*
- * Code accessing dma_async_is_complete() in a tight loop may give problems.
  * If slaves are relying on interrupts to signal completion this function
  * must not be called with interrupts disabled.
  */
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 30ae36124b1db..bc0e1af2296c9 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1483,8 +1483,7 @@ static int atc_terminate_all(struct dma_chan *chan)
  * @txstate: if not %NULL updated with transaction state
  *
  * If @txstate is passed in, upon return it reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * internal state.
  */
 static enum dma_status
 atc_tx_status(struct dma_chan *chan,
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 842d4f7ca752d..194e334b33bbc 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1439,9 +1439,9 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
  * @last: returns last completed cookie, can be NULL
  * @used: returns last issued cookie, can be NULL
  *
- * If @last and @used are passed in, upon return they reflect the driver
- * internal state and can be used with dma_async_is_complete() to check
- * the status of multiple cookies without re-checking hardware state.
+ * If @last and @used are passed in, upon return they reflect the most
+ * recently submitted (used) cookie and the most recently completed
+ * cookie.
  */
 static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
@@ -1457,28 +1457,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-/**
- * dma_async_is_complete - test a cookie against chan state
- * @cookie: transaction identifier to test status of
- * @last_complete: last know completed transaction
- * @last_used: last cookie value handed out
- *
- * dma_async_is_complete() is used in dma_async_is_tx_complete()
- * the test logic is separated for lightweight testing of multiple cookies
- */
-static inline enum dma_status dma_async_is_complete(dma_cookie_t cookie,
-			dma_cookie_t last_complete, dma_cookie_t last_used)
-{
-	if (last_complete <= last_used) {
-		if ((cookie <= last_complete) || (cookie > last_used))
-			return DMA_COMPLETE;
-	} else {
-		if ((cookie <= last_complete) && (cookie > last_used))
-			return DMA_COMPLETE;
-	}
-	return DMA_IN_PROGRESS;
-}
-
 static inline void
 dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
 {
-- 
2.33.1

[RESEND 02/10] 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 194e334b33bbc..8c4934bc038ec 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1457,17 +1457,6 @@ static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
 	return status;
 }
 
-static inline void
-dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, u32 residue)
-{
-	if (!st)
-		return;
-
-	st->last = last;
-	st->used = used;
-	st->residue = residue;
-}
-
 #ifdef CONFIG_DMA_ENGINE
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
 enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie);
-- 
2.33.1

[RESEND 03/10] dmaengine: Remove the last, used parameters in dma_async_is_tx_complete

[Ben Walker]

These are only used by a single driver (pxa_camera) which knows exactly
which DMA engine it is dealing with and therefore knows the behavior
of the cookie values. It can grab the value it needs directly from
the dma_chan instead. No other DMA client assumes anything about
the behavior of the cookie values, so the cookie becomes an opaque
handle after this patch.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
 Documentation/driver-api/dmaengine/client.rst  | 17 +++--------------
 .../driver-api/dmaengine/provider.rst          |  6 +++---
 drivers/crypto/stm32/stm32-hash.c              |  3 +--
 drivers/dma/dmaengine.c                        |  2 +-
 drivers/dma/dmatest.c                          |  3 +--
 drivers/media/platform/omap/omap_vout_vrfb.c   |  2 +-
 drivers/media/platform/pxa_camera.c            | 13 +++++++++++--
 drivers/rapidio/devices/rio_mport_cdev.c       |  3 +--
 include/linux/dmaengine.h                      | 18 +++---------------
 9 files changed, 25 insertions(+), 42 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index 85ecec2c40005..6e43c9c428e68 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 dma_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.
@@ -340,22 +340,11 @@ 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.
+		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 0072c9c7efd34..e9e9de18d6b02 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -543,10 +543,10 @@ where to put them)
 
 dma_cookie_t
 
-- it's a DMA transaction ID that will increment over time.
+- it's a DMA transaction ID.
 
-- Not really relevant any more since the introduction of ``virt-dma``
-  that abstracts it away.
+- The value can be chosen by the provider, or use the helper APIs
+  such as dma_cookie_assign() and dma_cookie_complete().
 
 DMA_CTRL_ACK
 
diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
index d33006d43f761..9c3b6526e39f8 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 (dma_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
 		err = -ETIMEDOUT;
 
 	if (err) {
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 2cfa8458b51be..590f238a0671a 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 = dma_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 f696246f57fdb..0574090a80c8c 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -832,8 +832,7 @@ static int dmatest_func(void *data)
 					done->done,
 					msecs_to_jiffies(params->timeout));
 
-			status = dma_async_is_tx_complete(chan, cookie, NULL,
-							  NULL);
+			status = dma_async_is_tx_complete(chan, cookie);
 		}
 
 		if (!done->done) {
diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
index 0cfa0169875f0..d762939ffa5de 100644
--- a/drivers/media/platform/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/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 = dma_async_is_tx_complete(chan, cookie);
 
 	if (vout->vrfb_dma_tx.tx_status == 0) {
 		pr_err("%s: Timeout while waiting for DMA\n", __func__);
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index 3ba00b0f93200..6d5c36ea6622a 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -1049,8 +1049,17 @@ 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_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",
diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
index 7df466e222820..790e0c7a3306c 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 = dma_async_is_tx_complete(priv->dmach, req->cookie);
 	complete(&req->req_comp);
 	kref_put(&req->refcount, dma_req_free);
 }
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 8c4934bc038ec..5f884fffe74cc 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1436,25 +1436,13 @@ static inline void dma_async_issue_pending(struct dma_chan *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
- *
- * 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)
+	dma_cookie_t cookie)
 {
 	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;
+
+	return chan->device->device_tx_status(chan, cookie, &state);
 }
 
 #ifdef CONFIG_DMA_ENGINE
-- 
2.33.1

Re: [RESEND 03/10] dmaengine: Remove the last, used parameters in dma_async_is_tx_complete

[Vinod Koul]

On 01-02-22, 13:38, Ben Walker wrote:
> These are only used by a single driver (pxa_camera) which knows exactly
> which DMA engine it is dealing with and therefore knows the behavior
> of the cookie values. It can grab the value it needs directly from
> the dma_chan instead. No other DMA client assumes anything about
> the behavior of the cookie values, so the cookie becomes an opaque
> handle after this patch.
> 
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
>  Documentation/driver-api/dmaengine/client.rst  | 17 +++--------------
>  .../driver-api/dmaengine/provider.rst          |  6 +++---
>  drivers/crypto/stm32/stm32-hash.c              |  3 +--
>  drivers/dma/dmaengine.c                        |  2 +-
>  drivers/dma/dmatest.c                          |  3 +--
>  drivers/media/platform/omap/omap_vout_vrfb.c   |  2 +-
>  drivers/media/platform/pxa_camera.c            | 13 +++++++++++--
>  drivers/rapidio/devices/rio_mport_cdev.c       |  3 +--
>  include/linux/dmaengine.h                      | 18 +++---------------

This needs to be split per subsysytem and cc respective maintainers so
that they can review and ack it...

>  9 files changed, 25 insertions(+), 42 deletions(-)
> 
> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
> index 85ecec2c40005..6e43c9c428e68 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 dma_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.
> @@ -340,22 +340,11 @@ 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.
> +		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 0072c9c7efd34..e9e9de18d6b02 100644
> --- a/Documentation/driver-api/dmaengine/provider.rst
> +++ b/Documentation/driver-api/dmaengine/provider.rst
> @@ -543,10 +543,10 @@ where to put them)
>  
>  dma_cookie_t
>  
> -- it's a DMA transaction ID that will increment over time.
> +- it's a DMA transaction ID.

why drop this?

>  
> -- 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/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
> index d33006d43f761..9c3b6526e39f8 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 (dma_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
>  		err = -ETIMEDOUT;
>  
>  	if (err) {
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 2cfa8458b51be..590f238a0671a 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 = dma_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 f696246f57fdb..0574090a80c8c 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -832,8 +832,7 @@ static int dmatest_func(void *data)
>  					done->done,
>  					msecs_to_jiffies(params->timeout));
>  
> -			status = dma_async_is_tx_complete(chan, cookie, NULL,
> -							  NULL);
> +			status = dma_async_is_tx_complete(chan, cookie);
>  		}
>  
>  		if (!done->done) {
> diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
> index 0cfa0169875f0..d762939ffa5de 100644
> --- a/drivers/media/platform/omap/omap_vout_vrfb.c
> +++ b/drivers/media/platform/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 = dma_async_is_tx_complete(chan, cookie);
>  
>  	if (vout->vrfb_dma_tx.tx_status == 0) {
>  		pr_err("%s: Timeout while waiting for DMA\n", __func__);
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index 3ba00b0f93200..6d5c36ea6622a 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -1049,8 +1049,17 @@ 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_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",
> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> index 7df466e222820..790e0c7a3306c 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 = dma_async_is_tx_complete(priv->dmach, req->cookie);
>  	complete(&req->req_comp);
>  	kref_put(&req->refcount, dma_req_free);
>  }
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 8c4934bc038ec..5f884fffe74cc 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1436,25 +1436,13 @@ static inline void dma_async_issue_pending(struct dma_chan *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
> - *
> - * 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)
> +	dma_cookie_t cookie)

With this I think we should take this opprtunity to rename the api to
dmaengine_async_is_tx_complete()

-- 
~Vinod

Re: [RESEND 03/10] dmaengine: Remove the last, used parameters in dma_async_is_tx_complete

[Walker, Benjamin]

On 5/3/2022 7:13 AM, Vinod Koul wrote:
> On 01-02-22, 13:38, Ben Walker wrote:
>> These are only used by a single driver (pxa_camera) which knows exactly
>> which DMA engine it is dealing with and therefore knows the behavior
>> of the cookie values. It can grab the value it needs directly from
>> the dma_chan instead. No other DMA client assumes anything about
>> the behavior of the cookie values, so the cookie becomes an opaque
>> handle after this patch.
>>
>> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
>> ---
>>   Documentation/driver-api/dmaengine/client.rst  | 17 +++--------------
>>   .../driver-api/dmaengine/provider.rst          |  6 +++---
>>   drivers/crypto/stm32/stm32-hash.c              |  3 +--
>>   drivers/dma/dmaengine.c                        |  2 +-
>>   drivers/dma/dmatest.c                          |  3 +--
>>   drivers/media/platform/omap/omap_vout_vrfb.c   |  2 +-
>>   drivers/media/platform/pxa_camera.c            | 13 +++++++++++--
>>   drivers/rapidio/devices/rio_mport_cdev.c       |  3 +--
>>   include/linux/dmaengine.h                      | 18 +++---------------
> 
> This needs to be split per subsysytem and cc respective maintainers so
> that they can review and ack it...

Ack. Will split.

> 
>>   9 files changed, 25 insertions(+), 42 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
>> index 85ecec2c40005..6e43c9c428e68 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 dma_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.
>> @@ -340,22 +340,11 @@ 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.
>> +		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 0072c9c7efd34..e9e9de18d6b02 100644
>> --- a/Documentation/driver-api/dmaengine/provider.rst
>> +++ b/Documentation/driver-api/dmaengine/provider.rst
>> @@ -543,10 +543,10 @@ where to put them)
>>   
>>   dma_cookie_t
>>   
>> -- it's a DMA transaction ID that will increment over time.
>> +- it's a DMA transaction ID.
> 
> why drop this?

The exact behavior of the cookie (incrementing over time) is no longer a 
part of the API after this is removed. It becomes an opaque handle.

> 
>>   
>> -- 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/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
>> index d33006d43f761..9c3b6526e39f8 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 (dma_async_is_tx_complete(hdev->dma_lch, cookie) != DMA_COMPLETE)
>>   		err = -ETIMEDOUT;
>>   
>>   	if (err) {
>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>> index 2cfa8458b51be..590f238a0671a 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 = dma_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 f696246f57fdb..0574090a80c8c 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -832,8 +832,7 @@ static int dmatest_func(void *data)
>>   					done->done,
>>   					msecs_to_jiffies(params->timeout));
>>   
>> -			status = dma_async_is_tx_complete(chan, cookie, NULL,
>> -							  NULL);
>> +			status = dma_async_is_tx_complete(chan, cookie);
>>   		}
>>   
>>   		if (!done->done) {
>> diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
>> index 0cfa0169875f0..d762939ffa5de 100644
>> --- a/drivers/media/platform/omap/omap_vout_vrfb.c
>> +++ b/drivers/media/platform/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 = dma_async_is_tx_complete(chan, cookie);
>>   
>>   	if (vout->vrfb_dma_tx.tx_status == 0) {
>>   		pr_err("%s: Timeout while waiting for DMA\n", __func__);
>> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
>> index 3ba00b0f93200..6d5c36ea6622a 100644
>> --- a/drivers/media/platform/pxa_camera.c
>> +++ b/drivers/media/platform/pxa_camera.c
>> @@ -1049,8 +1049,17 @@ 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_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",
>> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
>> index 7df466e222820..790e0c7a3306c 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 = dma_async_is_tx_complete(priv->dmach, req->cookie);
>>   	complete(&req->req_comp);
>>   	kref_put(&req->refcount, dma_req_free);
>>   }
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index 8c4934bc038ec..5f884fffe74cc 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -1436,25 +1436,13 @@ static inline void dma_async_issue_pending(struct dma_chan *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
>> - *
>> - * 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)
>> +	dma_cookie_t cookie)
> 
> With this I think we should take this opprtunity to rename the api to
> dmaengine_async_is_tx_complete()
> 

Ok, I can add a patch in my v2 series to do the rename.

[RESEND 04/10] 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 5f884fffe74cc..3c545b42723ec 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.33.1

[RESEND 05/10] 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 75ec0754d4ad4..efb218221ba81 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1734,8 +1734,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.33.1

[RESEND 06/10] 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.33.1

[RESEND 07/10] 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 e9e9de18d6b02..7c8ace703c96f 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -421,7 +421,9 @@ supported.
 
     - tx_submit: A pointer to a function you have to implement,
       that is supposed to push the current transaction descriptor to a
-      pending queue, waiting for issue_pending to be called.
+      pending queue, waiting for issue_pending to be called. Each
+      descriptor is given a cookie to identify it. See the section
+      "Cookie Management" below.
 
   - In this structure the function pointer callback_result can be
     initialized in order for the submitter to be notified that a
@@ -526,6 +528,40 @@ supported.
 
   - May sleep.
 
+Cookie Management
+------------------
+
+When a transaction is queued for submission via tx_submit(), the provider
+must assign that transaction a cookie (dma_cookie_t) to uniquely identify it.
+The provider is allowed to perform this assignment however it wants, but for
+convenience the following utility functions are available to create
+monotonically increasing cookies
+
+  .. code-block:: c
+
+    void dma_cookie_init(struct dma_chan *chan);
+
+  Called once at channel creation
+
+  .. code-block:: c
+
+    dma_cookie_t dma_cookie_assign(struct dma_async_tx_descriptor *tx);
+
+  Assign a cookie to the given descriptor
+
+  .. code-block:: c
+
+    void dma_cookie_complete(struct dma_async_tx_descriptor *tx);
+
+  Mark the descriptor as complete and invalidate the cookie
+
+  .. code-block:: c
+
+    enum dma_status dma_cookie_status(struct dma_chan *chan,
+      dma_cookie_t cookie, struct dma_tx_state *state);
+
+  Report the status of the cookie and filling in state, if not NULL.
+
 
 Misc notes
 ==========
@@ -541,13 +577,6 @@ where to put them)
 - Makes sure that dependent operations are run before marking it
   as complete.
 
-dma_cookie_t
-
-- it's a DMA transaction ID.
-
-- The value can be chosen by the provider, or use the helper APIs
-  such as dma_cookie_assign() and dma_cookie_complete().
-
 DMA_CTRL_ACK
 
 - If clear, the descriptor cannot be reused by provider until the
-- 
2.33.1

[RESEND 08/10] 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 c53835584406a..b95299b723518 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -328,7 +328,7 @@ struct idxd_desc {
 	struct dma_async_tx_descriptor txd;
 	struct llist_node llnode;
 	struct list_head list;
-	int id;
+	u16 id;
 	int cpu;
 	struct idxd_wq *wq;
 };
-- 
2.33.1

[RESEND 09/10] 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 4b29b8ff21272..e71d926cfd7a9 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 1b6e5f373259c..15bd87c2811e4 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 b95299b723518..dd414930cd215 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -329,6 +329,7 @@ struct idxd_desc {
 	struct llist_node llnode;
 	struct list_head list;
 	u16 id;
+	u16 gen;
 	int cpu;
 	struct idxd_wq *wq;
 };
-- 
2.33.1

[RESEND 10/10] 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 7c8ace703c96f..5bb4738ece0c2 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -262,22 +262,6 @@ Currently, the types available are:
     want to transfer a portion of uncompressed data directly to the
     display to print it
 
-- DMA_COMPLETION_NO_ORDER
-
-  - The device does not support in order completion.
-
-  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
-    the device is setting this capability.
-
-  - All cookie tracking and checking API should be treated as invalid if
-    the device exports this capability.
-
-  - At this point, this is incompatible with polling option for dmatest.
-
-  - If this cap is set, the user is recommended to provide an unique
-    identifier for each descriptor sent to the DMA device in order to
-    properly track the completion.
-
 - DMA_REPEAT
 
   - The device supports repeated transfers. A repeated transfer, indicated by
@@ -461,9 +445,6 @@ supported.
   - In the case of a cyclic transfer, it should only take into
     account the current period.
 
-  - Should return DMA_OUT_OF_ORDER if the device does not support in order
-    completion and is completing the operation out of order.
-
   - This function can be called in an interrupt context.
 
 - device_config
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 0574090a80c8c..f8cbeeaf8ed03 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -839,10 +839,7 @@ static int dmatest_func(void *data)
 			result("test timed out", total_tests, src->off, dst->off,
 			       len, 0);
 			goto error_unmap_continue;
-		} else if (status != DMA_COMPLETE &&
-			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
-					 dev->cap_mask) &&
-			     status == DMA_OUT_OF_ORDER)) {
+		} else if (status != DMA_COMPLETE) {
 			result(status == DMA_ERROR ?
 			       "completion error status" :
 			       "completion busy status", total_tests, src->off,
@@ -1020,12 +1017,6 @@ static int dmatest_add_channel(struct dmatest_info *info,
 	dtc->chan = chan;
 	INIT_LIST_HEAD(&dtc->threads);
 
-	if (dma_has_cap(DMA_COMPLETION_NO_ORDER, dma_dev->cap_mask) &&
-	    info->params.polled) {
-		info->params.polled = false;
-		pr_warn("DMA_COMPLETION_NO_ORDER, polled disabled\n");
-	}
-
 	if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
 		if (dmatest == 0) {
 			cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
diff --git a/drivers/dma/idxd/dma.c b/drivers/dma/idxd/dma.c
index 15bd87c2811e4..3400f49ca3c95 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->dev = dev;
 
 	dma_cap_set(DMA_PRIVATE, dma->cap_mask);
-	dma_cap_set(DMA_COMPLETION_NO_ORDER, dma->cap_mask);
 	dma->device_release = idxd_dma_release;
 
 	if (idxd->hw.opcap.bits[0] & IDXD_OPCAP_MEMMOVE) {
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 3c545b42723ec..47b0f0db1ef77 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.33.1