From: "Walker, Benjamin" <benjamin.walker@intel.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: <dmaengine@vger.kernel.org>, <dave.jiang@intel.com>
Subject: Re: [RESEND 03/10] dmaengine: Remove the last, used parameters in dma_async_is_tx_complete
Date: Tue, 3 May 2022 10:56:25 -0700 [thread overview]
Message-ID: <abfd086d-0a43-5d68-3f14-07ec4e8f5da1@intel.com> (raw)
In-Reply-To: <YnE4jLGSOupviRWQ@matsya>
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.
next prev parent reply other threads:[~2022-05-03 17:56 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-01 20:38 [RESEND 00/10] Support polling for out of order completions Ben Walker
2022-02-01 20:38 ` [RESEND 01/10] dmaengine: Remove dma_async_is_complete from client API Ben Walker
2022-02-01 20:38 ` [RESEND 02/10] dmaengine: Move dma_set_tx_state to the provider API header Ben Walker
2022-02-01 20:38 ` [RESEND 03/10] dmaengine: Remove the last, used parameters in dma_async_is_tx_complete Ben Walker
2022-05-03 14:13 ` Vinod Koul
2022-05-03 17:56 ` Walker, Benjamin [this message]
2022-02-01 20:38 ` [RESEND 04/10] dmaengine: Remove last, used from dma_tx_state Ben Walker
2022-02-01 20:38 ` [RESEND 05/10] dmaengine: Providers should prefer dma_set_residue over dma_set_tx_state Ben Walker
2022-02-01 20:38 ` [RESEND 06/10] dmaengine: Remove dma_set_tx_state Ben Walker
2022-02-01 20:38 ` [RESEND 07/10] dmaengine: Add provider documentation on cookie assignment Ben Walker
2022-02-01 20:38 ` [RESEND 08/10] dmaengine: idxd: idxd_desc.id is now a u16 Ben Walker
2022-02-01 20:38 ` [RESEND 09/10] dmaengine: idxd: Support device_tx_status Ben Walker
2022-02-01 20:38 ` [RESEND 10/10] dmaengine: Revert "cookie bypass for out of order completion" Ben Walker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abfd086d-0a43-5d68-3f14-07ec4e8f5da1@intel.com \
--to=benjamin.walker@intel.com \
--cc=dave.jiang@intel.com \
--cc=dmaengine@vger.kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox