[PATCH v4] dmaengine: cookie bypass for out of order completion

[PATCH v4] dmaengine: cookie bypass for out of order completion

[Dave Jiang]

The cookie tracking in dmaengine expects all submissions completed in
order. Some DMA devices like Intel DSA can complete submissions out of
order, especially if configured with a work queue sharing multiple DMA
engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
those DMA devices. The user should use callbacks to track the completion
rather than the DMA cookie. This would address the issue of dmatest
complaining that descriptors are "busy" when the cookie count goes
backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
DMA capability to allow the driver to flag the device's ability to complete
operations out of order.

Reported-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Tested-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
---
v4:
- Add block for polled in dmatest. Need better enabling in future to solve
  per channel capability for out of order vs poll. (peter)
v3:
- v2 mailed wrong patch
v2:
- Add DMA capability (vinod)
- Add documentation (vinod)

 Documentation/driver-api/dmaengine/provider.rst |   11 +++++++++++
 drivers/dma/dmatest.c                           |   11 ++++++++++-
 drivers/dma/idxd/dma.c                          |    3 ++-
 include/linux/dmaengine.h                       |    2 ++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
index 56e5833e8a07..783ca141e147 100644
--- a/Documentation/driver-api/dmaengine/provider.rst
+++ b/Documentation/driver-api/dmaengine/provider.rst
@@ -239,6 +239,14 @@ 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 supports out of order completion of the operations.
+
+  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
+    the device supports out of order completion and the completion is
+    is expected to be completed out of order.
+
 These various types will also affect how the source and destination
 addresses change over time.
 
@@ -399,6 +407,9 @@ 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 supports out of 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 a2cadfa2e6d7..93c7c56af5f2 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -821,7 +821,10 @@ 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) {
+		} else if (status != DMA_COMPLETE &&
+			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
+					 dev->cap_mask) &&
+			     status == DMA_OUT_OF_ORDER)) {
 			result(status == DMA_ERROR ?
 			       "completion error status" :
 			       "completion busy status", total_tests, src->off,
@@ -999,6 +1002,12 @@ 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 c64c1429d160..0c892cbd72e0 100644
--- a/drivers/dma/idxd/dma.c
+++ b/drivers/dma/idxd/dma.c
@@ -133,7 +133,7 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
 					  dma_cookie_t cookie,
 					  struct dma_tx_state *txstate)
 {
-	return dma_cookie_status(dma_chan, cookie, txstate);
+	return DMA_OUT_OF_ORDER;
 }
 
 /*
@@ -174,6 +174,7 @@ int idxd_register_dma_device(struct idxd_device *idxd)
 	INIT_LIST_HEAD(&dma->channels);
 	dma->dev = &idxd->pdev->dev;
 
+	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 21065c04c4ac..1123f4d15bae 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -39,6 +39,7 @@ enum dma_status {
 	DMA_IN_PROGRESS,
 	DMA_PAUSED,
 	DMA_ERROR,
+	DMA_OUT_OF_ORDER,
 };
 
 /**
@@ -61,6 +62,7 @@ enum dma_transaction_type {
 	DMA_SLAVE,
 	DMA_CYCLIC,
 	DMA_INTERLEAVE,
+	DMA_COMPLETION_NO_ORDER,
 /* last transaction type for creation of the capabilities mask */
 	DMA_TX_TYPE_END,
 };

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Peter Ujfalusi]

[-- Attachment #1: Type: text/plain, Size: 6764 bytes --]



On 12/05/2020 2.47, Dave Jiang wrote:
> The cookie tracking in dmaengine expects all submissions completed in
> order. Some DMA devices like Intel DSA can complete submissions out of
> order, especially if configured with a work queue sharing multiple DMA
> engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
> those DMA devices. The user should use callbacks to track the completion
> rather than the DMA cookie. This would address the issue of dmatest
> complaining that descriptors are "busy" when the cookie count goes
> backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
> DMA capability to allow the driver to flag the device's ability to complete
> operations out of order.

I'm still a bit hesitant around this.
If the DMA only support out of order completion then it is mandatory
that each descriptor must have unique callback parameter in order the
client could know which transfer has been completed.
Kind of obvious, not sure if it worth documenting.

It was/is on my to do list to support out of order completion as well,
but the use case and setup is different and would need additional
features for DMAengine (like 'classification' of a descriptor).
I have planned to add optional DMA device level of cookie handling so I
could track non ordered cookie statuses.
In my case I mostly know which way the descriptor is going to go, but if
I understand right in your case you have a pool of DMAs and it is mostly
random which one is going to pick up (and complete) the descriptors,
thus leading to cases when smaller transfers overtaking longer ones in
completion.

I also believe this would not block optional DMA device level of cookie
handling.

> Reported-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Tested-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
> ---
> v4:
> - Add block for polled in dmatest. Need better enabling in future to solve
>   per channel capability for out of order vs poll. (peter)

Thanks, it is just too bad that dma_has_cap() is so widely used to be
easily repurposed :(

> v3:
> - v2 mailed wrong patch
> v2:
> - Add DMA capability (vinod)
> - Add documentation (vinod)
> 
>  Documentation/driver-api/dmaengine/provider.rst |   11 +++++++++++
>  drivers/dma/dmatest.c                           |   11 ++++++++++-
>  drivers/dma/idxd/dma.c                          |    3 ++-
>  include/linux/dmaengine.h                       |    2 ++
>  4 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
> index 56e5833e8a07..783ca141e147 100644
> --- a/Documentation/driver-api/dmaengine/provider.rst
> +++ b/Documentation/driver-api/dmaengine/provider.rst
> @@ -239,6 +239,14 @@ 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 supports out of order completion of the operations.
> +
> +  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
> +    the device supports out of order completion and the completion is
> +    is expected to be completed out of order.

It is not just supports out of order, but does not support in order
completion. Iow it can not guarantee in order completion.
As a not: all cookie tracking and checking API should be treated as
invalid. For example dma_sync_wait() is unusable.

and one extra 'is'

> +
>  These various types will also affect how the source and destination
>  addresses change over time.
>  
> @@ -399,6 +407,9 @@ 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 supports out of order
> +    completion and is completing the operation out of order.

Same here.

> +
>    - This function can be called in an interrupt context.
>  
>  - device_config
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index a2cadfa2e6d7..93c7c56af5f2 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -821,7 +821,10 @@ 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) {
> +		} else if (status != DMA_COMPLETE &&
> +			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
> +					 dev->cap_mask) &&
> +			     status == DMA_OUT_OF_ORDER)) {
>  			result(status == DMA_ERROR ?
>  			       "completion error status" :
>  			       "completion busy status", total_tests, src->off,
> @@ -999,6 +1002,12 @@ 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 c64c1429d160..0c892cbd72e0 100644
> --- a/drivers/dma/idxd/dma.c
> +++ b/drivers/dma/idxd/dma.c
> @@ -133,7 +133,7 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
>  					  dma_cookie_t cookie,
>  					  struct dma_tx_state *txstate)
>  {
> -	return dma_cookie_status(dma_chan, cookie, txstate);
> +	return DMA_OUT_OF_ORDER;
>  }
>  
>  /*
> @@ -174,6 +174,7 @@ int idxd_register_dma_device(struct idxd_device *idxd)
>  	INIT_LIST_HEAD(&dma->channels);
>  	dma->dev = &idxd->pdev->dev;
>  
> +	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 21065c04c4ac..1123f4d15bae 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -39,6 +39,7 @@ enum dma_status {
>  	DMA_IN_PROGRESS,
>  	DMA_PAUSED,
>  	DMA_ERROR,
> +	DMA_OUT_OF_ORDER,
>  };
>  
>  /**
> @@ -61,6 +62,7 @@ enum dma_transaction_type {
>  	DMA_SLAVE,
>  	DMA_CYCLIC,
>  	DMA_INTERLEAVE,
> +	DMA_COMPLETION_NO_ORDER,
>  /* last transaction type for creation of the capabilities mask */
>  	DMA_TX_TYPE_END,
>  };
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1783 bytes --]

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Dave Jiang]

On 5/13/2020 12:30 AM, Peter Ujfalusi wrote:
> 
> 
> On 12/05/2020 2.47, Dave Jiang wrote:
>> The cookie tracking in dmaengine expects all submissions completed in
>> order. Some DMA devices like Intel DSA can complete submissions out of
>> order, especially if configured with a work queue sharing multiple DMA
>> engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
>> those DMA devices. The user should use callbacks to track the completion
>> rather than the DMA cookie. This would address the issue of dmatest
>> complaining that descriptors are "busy" when the cookie count goes
>> backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
>> DMA capability to allow the driver to flag the device's ability to complete
>> operations out of order.
> 
> I'm still a bit hesitant around this.
> If the DMA only support out of order completion then it is mandatory
> that each descriptor must have unique callback parameter in order the
> client could know which transfer has been completed.
> Kind of obvious, not sure if it worth documenting.

Agreed. I'll add a bit more notes to remind the reader.

> 
> It was/is on my to do list to support out of order completion as well,
> but the use case and setup is different and would need additional
> features for DMAengine (like 'classification' of a descriptor).
> I have planned to add optional DMA device level of cookie handling so I
> could track non ordered cookie statuses.
> In my case I mostly know which way the descriptor is going to go, but if
> I understand right in your case you have a pool of DMAs and it is mostly
> random which one is going to pick up (and complete) the descriptors,
> thus leading to cases when smaller transfers overtaking longer ones in
> completion.
> 
> I also believe this would not block optional DMA device level of cookie
> handling.

Looking forward to your enhancements.

> 
>> Reported-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> Tested-by: Swathi Kovvuri <swathi.kovvuri@intel.com>
>> ---
>> v4:
>> - Add block for polled in dmatest. Need better enabling in future to solve
>>    per channel capability for out of order vs poll. (peter)
> 
> Thanks, it is just too bad that dma_has_cap() is so widely used to be
> easily repurposed :(

yes...

> 
>> v3:
>> - v2 mailed wrong patch
>> v2:
>> - Add DMA capability (vinod)
>> - Add documentation (vinod)
>>
>>   Documentation/driver-api/dmaengine/provider.rst |   11 +++++++++++
>>   drivers/dma/dmatest.c                           |   11 ++++++++++-
>>   drivers/dma/idxd/dma.c                          |    3 ++-
>>   include/linux/dmaengine.h                       |    2 ++
>>   4 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dmaengine/provider.rst b/Documentation/driver-api/dmaengine/provider.rst
>> index 56e5833e8a07..783ca141e147 100644
>> --- a/Documentation/driver-api/dmaengine/provider.rst
>> +++ b/Documentation/driver-api/dmaengine/provider.rst
>> @@ -239,6 +239,14 @@ 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 supports out of order completion of the operations.
>> +
>> +  - The driver should return DMA_OUT_OF_ORDER for device_tx_status if
>> +    the device supports out of order completion and the completion is
>> +    is expected to be completed out of order.
> 
> It is not just supports out of order, but does not support in order
> completion. Iow it can not guarantee in order completion.
> As a not: all cookie tracking and checking API should be treated as
> invalid. For example dma_sync_wait() is unusable.
> 
> and one extra 'is'

I will clarify and add the additional note.

> 
>> +
>>   These various types will also affect how the source and destination
>>   addresses change over time.
>>   
>> @@ -399,6 +407,9 @@ 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 supports out of order
>> +    completion and is completing the operation out of order.
> 
> Same here.
> 
>> +
>>     - This function can be called in an interrupt context.
>>   
>>   - device_config
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index a2cadfa2e6d7..93c7c56af5f2 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -821,7 +821,10 @@ 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) {
>> +		} else if (status != DMA_COMPLETE &&
>> +			   !(dma_has_cap(DMA_COMPLETION_NO_ORDER,
>> +					 dev->cap_mask) &&
>> +			     status == DMA_OUT_OF_ORDER)) {
>>   			result(status == DMA_ERROR ?
>>   			       "completion error status" :
>>   			       "completion busy status", total_tests, src->off,
>> @@ -999,6 +1002,12 @@ 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 c64c1429d160..0c892cbd72e0 100644
>> --- a/drivers/dma/idxd/dma.c
>> +++ b/drivers/dma/idxd/dma.c
>> @@ -133,7 +133,7 @@ static enum dma_status idxd_dma_tx_status(struct dma_chan *dma_chan,
>>   					  dma_cookie_t cookie,
>>   					  struct dma_tx_state *txstate)
>>   {
>> -	return dma_cookie_status(dma_chan, cookie, txstate);
>> +	return DMA_OUT_OF_ORDER;
>>   }
>>   
>>   /*
>> @@ -174,6 +174,7 @@ int idxd_register_dma_device(struct idxd_device *idxd)
>>   	INIT_LIST_HEAD(&dma->channels);
>>   	dma->dev = &idxd->pdev->dev;
>>   
>> +	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 21065c04c4ac..1123f4d15bae 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -39,6 +39,7 @@ enum dma_status {
>>   	DMA_IN_PROGRESS,
>>   	DMA_PAUSED,
>>   	DMA_ERROR,
>> +	DMA_OUT_OF_ORDER,
>>   };
>>   
>>   /**
>> @@ -61,6 +62,7 @@ enum dma_transaction_type {
>>   	DMA_SLAVE,
>>   	DMA_CYCLIC,
>>   	DMA_INTERLEAVE,
>> +	DMA_COMPLETION_NO_ORDER,
>>   /* last transaction type for creation of the capabilities mask */
>>   	DMA_TX_TYPE_END,
>>   };
>>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Vinod Koul]

On 13-05-20, 09:35, Dave Jiang wrote:
> 
> 
> On 5/13/2020 12:30 AM, Peter Ujfalusi wrote:
> > 
> > 
> > On 12/05/2020 2.47, Dave Jiang wrote:
> > > The cookie tracking in dmaengine expects all submissions completed in
> > > order. Some DMA devices like Intel DSA can complete submissions out of
> > > order, especially if configured with a work queue sharing multiple DMA
> > > engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
> > > those DMA devices. The user should use callbacks to track the completion
> > > rather than the DMA cookie. This would address the issue of dmatest
> > > complaining that descriptors are "busy" when the cookie count goes
> > > backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
> > > DMA capability to allow the driver to flag the device's ability to complete
> > > operations out of order.
> > 
> > I'm still a bit hesitant around this.
> > If the DMA only support out of order completion then it is mandatory
> > that each descriptor must have unique callback parameter in order the
> > client could know which transfer has been completed.

Maybe we can still use the cookie to indicate that, or leave it to users
to manage? They can add an id in the callback params?

Using former is easy, but still user needs to keep track... later can be
possibly more suited here?

-- 
~Vinod

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Dave Jiang]

On 5/14/2020 11:48 PM, Vinod Koul wrote:
> On 13-05-20, 09:35, Dave Jiang wrote:
>>
>>
>> On 5/13/2020 12:30 AM, Peter Ujfalusi wrote:
>>>
>>>
>>> On 12/05/2020 2.47, Dave Jiang wrote:
>>>> The cookie tracking in dmaengine expects all submissions completed in
>>>> order. Some DMA devices like Intel DSA can complete submissions out of
>>>> order, especially if configured with a work queue sharing multiple DMA
>>>> engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
>>>> those DMA devices. The user should use callbacks to track the completion
>>>> rather than the DMA cookie. This would address the issue of dmatest
>>>> complaining that descriptors are "busy" when the cookie count goes
>>>> backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
>>>> DMA capability to allow the driver to flag the device's ability to complete
>>>> operations out of order.
>>>
>>> I'm still a bit hesitant around this.
>>> If the DMA only support out of order completion then it is mandatory
>>> that each descriptor must have unique callback parameter in order the
>>> client could know which transfer has been completed.
> 
> Maybe we can still use the cookie to indicate that, or leave it to users
> to manage? They can add an id in the callback params?
> 
> Using former is easy, but still user needs to keep track... later can be
> possibly more suited here?
> 

Is there anything else I need to do with this patch?

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Vinod Koul]

On 15-05-20, 09:21, Dave Jiang wrote:
> 
> 
> On 5/14/2020 11:48 PM, Vinod Koul wrote:
> > On 13-05-20, 09:35, Dave Jiang wrote:
> > > 
> > > 
> > > On 5/13/2020 12:30 AM, Peter Ujfalusi wrote:
> > > > 
> > > > 
> > > > On 12/05/2020 2.47, Dave Jiang wrote:
> > > > > The cookie tracking in dmaengine expects all submissions completed in
> > > > > order. Some DMA devices like Intel DSA can complete submissions out of
> > > > > order, especially if configured with a work queue sharing multiple DMA
> > > > > engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
> > > > > those DMA devices. The user should use callbacks to track the completion
> > > > > rather than the DMA cookie. This would address the issue of dmatest
> > > > > complaining that descriptors are "busy" when the cookie count goes
> > > > > backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
> > > > > DMA capability to allow the driver to flag the device's ability to complete
> > > > > operations out of order.
> > > > 
> > > > I'm still a bit hesitant around this.
> > > > If the DMA only support out of order completion then it is mandatory
> > > > that each descriptor must have unique callback parameter in order the
> > > > client could know which transfer has been completed.
> > 
> > Maybe we can still use the cookie to indicate that, or leave it to users
> > to manage? They can add an id in the callback params?
> > 
> > Using former is easy, but still user needs to keep track... later can be
> > possibly more suited here?
> > 
> 
> Is there anything else I need to do with this patch?

Not really atm, but i am going to defer this after merge window.

-- 
~Vinod

Re: [PATCH v4] dmaengine: cookie bypass for out of order completion

[Dave Jiang]

On 5/19/2020 9:59 AM, Vinod Koul wrote:
> On 15-05-20, 09:21, Dave Jiang wrote:
>>
>>
>> On 5/14/2020 11:48 PM, Vinod Koul wrote:
>>> On 13-05-20, 09:35, Dave Jiang wrote:
>>>>
>>>>
>>>> On 5/13/2020 12:30 AM, Peter Ujfalusi wrote:
>>>>>
>>>>>
>>>>> On 12/05/2020 2.47, Dave Jiang wrote:
>>>>>> The cookie tracking in dmaengine expects all submissions completed in
>>>>>> order. Some DMA devices like Intel DSA can complete submissions out of
>>>>>> order, especially if configured with a work queue sharing multiple DMA
>>>>>> engines. Add a status DMA_OUT_OF_ORDER that tx_status can be returned for
>>>>>> those DMA devices. The user should use callbacks to track the completion
>>>>>> rather than the DMA cookie. This would address the issue of dmatest
>>>>>> complaining that descriptors are "busy" when the cookie count goes
>>>>>> backwards due to out of order completion. Add DMA_COMPLETION_NO_ORDER
>>>>>> DMA capability to allow the driver to flag the device's ability to complete
>>>>>> operations out of order.
>>>>>
>>>>> I'm still a bit hesitant around this.
>>>>> If the DMA only support out of order completion then it is mandatory
>>>>> that each descriptor must have unique callback parameter in order the
>>>>> client could know which transfer has been completed.
>>>
>>> Maybe we can still use the cookie to indicate that, or leave it to users
>>> to manage? They can add an id in the callback params?
>>>
>>> Using former is easy, but still user needs to keep track... later can be
>>> possibly more suited here?
>>>
>>
>> Is there anything else I need to do with this patch?
> 
> Not really atm, but i am going to defer this after merge window.
> 

Fair enough. Thanks.