dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: dmatest: Add support for completion polling
@ 2019-05-29  8:37 Peter Ujfalusi
  2019-05-31  6:54 ` Peter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Ujfalusi @ 2019-05-29  8:37 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

With the polled parameter the DMA drivers can be tested if they can work
correctly when no completion is requested (no DMA_PREP_INTERRUPT and no
callback is provided).

If polled mode is selected then use dma_sync_wait() to execute the test
iteration instead of relying on the completion callback.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/dma/dmatest.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index b96814a7dceb..088086d041e9 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -75,6 +75,10 @@ static bool norandom;
 module_param(norandom, bool, 0644);
 MODULE_PARM_DESC(norandom, "Disable random offset setup (default: random)");
 
+static bool polled;
+module_param(polled, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
+
 static bool verbose;
 module_param(verbose, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
@@ -113,6 +117,7 @@ struct dmatest_params {
 	bool		norandom;
 	int		alignment;
 	unsigned int	transfer_size;
+	bool		polled;
 };
 
 /**
@@ -654,7 +659,10 @@ static int dmatest_func(void *data)
 	/*
 	 * src and dst buffers are freed by ourselves below
 	 */
-	flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
+	if (params->polled)
+		flags = DMA_CTRL_ACK;
+	else
+		flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
 
 	ktime = ktime_get();
 	while (!kthread_should_stop()
@@ -783,8 +791,10 @@ static int dmatest_func(void *data)
 		}
 
 		done->done = false;
-		tx->callback = dmatest_callback;
-		tx->callback_param = done;
+		if (!params->polled) {
+			tx->callback = dmatest_callback;
+			tx->callback_param = done;
+		}
 		cookie = tx->tx_submit(tx);
 
 		if (dma_submit_error(cookie)) {
@@ -793,12 +803,22 @@ static int dmatest_func(void *data)
 			msleep(100);
 			goto error_unmap_continue;
 		}
-		dma_async_issue_pending(chan);
 
-		wait_event_freezable_timeout(thread->done_wait, done->done,
-					     msecs_to_jiffies(params->timeout));
+		if (params->polled) {
+			status = dma_sync_wait(chan, cookie);
+			dmaengine_terminate_sync(chan);
+			if (status == DMA_COMPLETE)
+				done->done = true;
+		} else {
+			dma_async_issue_pending(chan);
+
+			wait_event_freezable_timeout(thread->done_wait,
+					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, NULL,
+							  NULL);
+		}
 
 		if (!done->done) {
 			result("test timed out", total_tests, src->off, dst->off,
@@ -1068,6 +1088,7 @@ static void add_threaded_test(struct dmatest_info *info)
 	params->norandom = norandom;
 	params->alignment = alignment;
 	params->transfer_size = transfer_size;
+	params->polled = polled;
 
 	request_channels(info, DMA_MEMCPY);
 	request_channels(info, DMA_MEMSET);
-- 
Peter

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-05-29  8:37 [PATCH] dmaengine: dmatest: Add support for completion polling Peter Ujfalusi
@ 2019-05-31  6:54 ` Peter Ujfalusi
  2019-06-03  7:05   ` Peter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Ujfalusi @ 2019-05-31  6:54 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko


On 29/05/2019 11.37, Peter Ujfalusi wrote:
> With the polled parameter the DMA drivers can be tested if they can work
> correctly when no completion is requested (no DMA_PREP_INTERRUPT and no
> callback is provided).
> 
> If polled mode is selected then use dma_sync_wait() to execute the test
> iteration instead of relying on the completion callback.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/dma/dmatest.c | 35 ++++++++++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index b96814a7dceb..088086d041e9 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -75,6 +75,10 @@ static bool norandom;
>  module_param(norandom, bool, 0644);
>  MODULE_PARM_DESC(norandom, "Disable random offset setup (default: random)");
>  
> +static bool polled;
> +module_param(polled, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
> +
>  static bool verbose;
>  module_param(verbose, bool, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
> @@ -113,6 +117,7 @@ struct dmatest_params {
>  	bool		norandom;
>  	int		alignment;
>  	unsigned int	transfer_size;
> +	bool		polled;
>  };
>  
>  /**
> @@ -654,7 +659,10 @@ static int dmatest_func(void *data)
>  	/*
>  	 * src and dst buffers are freed by ourselves below
>  	 */
> -	flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> +	if (params->polled)
> +		flags = DMA_CTRL_ACK;
> +	else
> +		flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>  
>  	ktime = ktime_get();
>  	while (!kthread_should_stop()
> @@ -783,8 +791,10 @@ static int dmatest_func(void *data)
>  		}
>  
>  		done->done = false;
> -		tx->callback = dmatest_callback;
> -		tx->callback_param = done;
> +		if (!params->polled) {
> +			tx->callback = dmatest_callback;
> +			tx->callback_param = done;
> +		}
>  		cookie = tx->tx_submit(tx);
>  
>  		if (dma_submit_error(cookie)) {
> @@ -793,12 +803,22 @@ static int dmatest_func(void *data)
>  			msleep(100);
>  			goto error_unmap_continue;
>  		}
> -		dma_async_issue_pending(chan);
>  
> -		wait_event_freezable_timeout(thread->done_wait, done->done,
> -					     msecs_to_jiffies(params->timeout));
> +		if (params->polled) {
> +			status = dma_sync_wait(chan, cookie);
> +			dmaengine_terminate_sync(chan);
> +			if (status == DMA_COMPLETE)
> +				done->done = true;

I think the main question is how polling for completion should be
handled when client does not request for completion interrupt, thus we
will have no callback in the DMA driver when the transfer is completed.

If DMA_PREP_INTERRUPT is set for the tx_descriptor then the polling will
wait until the DMA driver internally receives the interrupt that the
transfer is done and sets the cookie to completed state.

However if DMA_PREP_INTERRUPT is not set, the DMA driver will not get
notification from the HW that is the transfer is done, the only way to
know is to check the tx_status and based on the residue (if it is 0 then
it is done) decide what to tell the client.

Should the client call dmaengine_terminate_* after the polling returned
unconditionally to free up the descriptor?

Or client should only call dmaengine_terminate_* in case the polling
returned with !DMA_COMPLETE and the DMA driver must clean things up
before returning if the transfer is completed (residue == 0)?

> +		} else {
> +			dma_async_issue_pending(chan);
> +
> +			wait_event_freezable_timeout(thread->done_wait,
> +					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, NULL,
> +							  NULL);
> +		}
>  
>  		if (!done->done) {
>  			result("test timed out", total_tests, src->off, dst->off,
> @@ -1068,6 +1088,7 @@ static void add_threaded_test(struct dmatest_info *info)
>  	params->norandom = norandom;
>  	params->alignment = alignment;
>  	params->transfer_size = transfer_size;
> +	params->polled = polled;
>  
>  	request_channels(info, DMA_MEMCPY);
>  	request_channels(info, DMA_MEMSET);
> 

- Péter

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-05-31  6:54 ` Peter Ujfalusi
@ 2019-06-03  7:05   ` Peter Ujfalusi
  2019-06-04 12:45     ` Vinod Koul
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Ujfalusi @ 2019-06-03  7:05 UTC (permalink / raw)
  To: vkoul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

Hi,

On 31/05/2019 9.54, Peter Ujfalusi wrote:
> 
> On 29/05/2019 11.37, Peter Ujfalusi wrote:
>> With the polled parameter the DMA drivers can be tested if they can work
>> correctly when no completion is requested (no DMA_PREP_INTERRUPT and no
>> callback is provided).
>>
>> If polled mode is selected then use dma_sync_wait() to execute the test
>> iteration instead of relying on the completion callback.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/dma/dmatest.c | 35 ++++++++++++++++++++++++++++-------
>>  1 file changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
>> index b96814a7dceb..088086d041e9 100644
>> --- a/drivers/dma/dmatest.c
>> +++ b/drivers/dma/dmatest.c
>> @@ -75,6 +75,10 @@ static bool norandom;
>>  module_param(norandom, bool, 0644);
>>  MODULE_PARM_DESC(norandom, "Disable random offset setup (default: random)");
>>  
>> +static bool polled;
>> +module_param(polled, bool, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
>> +
>>  static bool verbose;
>>  module_param(verbose, bool, S_IRUGO | S_IWUSR);
>>  MODULE_PARM_DESC(verbose, "Enable \"success\" result messages (default: off)");
>> @@ -113,6 +117,7 @@ struct dmatest_params {
>>  	bool		norandom;
>>  	int		alignment;
>>  	unsigned int	transfer_size;
>> +	bool		polled;
>>  };
>>  
>>  /**
>> @@ -654,7 +659,10 @@ static int dmatest_func(void *data)
>>  	/*
>>  	 * src and dst buffers are freed by ourselves below
>>  	 */
>> -	flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>> +	if (params->polled)
>> +		flags = DMA_CTRL_ACK;
>> +	else
>> +		flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>>  
>>  	ktime = ktime_get();
>>  	while (!kthread_should_stop()
>> @@ -783,8 +791,10 @@ static int dmatest_func(void *data)
>>  		}
>>  
>>  		done->done = false;
>> -		tx->callback = dmatest_callback;
>> -		tx->callback_param = done;
>> +		if (!params->polled) {
>> +			tx->callback = dmatest_callback;
>> +			tx->callback_param = done;
>> +		}
>>  		cookie = tx->tx_submit(tx);
>>  
>>  		if (dma_submit_error(cookie)) {
>> @@ -793,12 +803,22 @@ static int dmatest_func(void *data)
>>  			msleep(100);
>>  			goto error_unmap_continue;
>>  		}
>> -		dma_async_issue_pending(chan);
>>  
>> -		wait_event_freezable_timeout(thread->done_wait, done->done,
>> -					     msecs_to_jiffies(params->timeout));
>> +		if (params->polled) {
>> +			status = dma_sync_wait(chan, cookie);
>> +			dmaengine_terminate_sync(chan);
>> +			if (status == DMA_COMPLETE)
>> +				done->done = true;
> 
> I think the main question is how polling for completion should be
> handled when client does not request for completion interrupt, thus we
> will have no callback in the DMA driver when the transfer is completed.
> 
> If DMA_PREP_INTERRUPT is set for the tx_descriptor then the polling will
> wait until the DMA driver internally receives the interrupt that the
> transfer is done and sets the cookie to completed state.
> 
> However if DMA_PREP_INTERRUPT is not set, the DMA driver will not get
> notification from the HW that is the transfer is done, the only way to
> know is to check the tx_status and based on the residue (if it is 0 then
> it is done) decide what to tell the client.
> 
> Should the client call dmaengine_terminate_* after the polling returned
> unconditionally to free up the descriptor?

This is how omap-dma is handling the polled memcpy support.

> Or client should only call dmaengine_terminate_* in case the polling
> returned with !DMA_COMPLETE and the DMA driver must clean things up
> before returning if the transfer is completed (residue == 0)?
> 
>> +		} else {
>> +			dma_async_issue_pending(chan);
>> +
>> +			wait_event_freezable_timeout(thread->done_wait,
>> +					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, NULL,
>> +							  NULL);
>> +		}
>>  
>>  		if (!done->done) {
>>  			result("test timed out", total_tests, src->off, dst->off,
>> @@ -1068,6 +1088,7 @@ static void add_threaded_test(struct dmatest_info *info)
>>  	params->norandom = norandom;
>>  	params->alignment = alignment;
>>  	params->transfer_size = transfer_size;
>> +	params->polled = polled;
>>  
>>  	request_channels(info, DMA_MEMCPY);
>>  	request_channels(info, DMA_MEMSET);
>>
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-03  7:05   ` Peter Ujfalusi
@ 2019-06-04 12:45     ` Vinod Koul
  2019-06-04 13:35       ` Peter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2019-06-04 12:45 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

On 03-06-19, 10:05, Peter Ujfalusi wrote:

> > I think the main question is how polling for completion should be
> > handled when client does not request for completion interrupt, thus we
> > will have no callback in the DMA driver when the transfer is completed.
> > 
> > If DMA_PREP_INTERRUPT is set for the tx_descriptor then the polling will
> > wait until the DMA driver internally receives the interrupt that the
> > transfer is done and sets the cookie to completed state.
> > 
> > However if DMA_PREP_INTERRUPT is not set, the DMA driver will not get
> > notification from the HW that is the transfer is done, the only way to
> > know is to check the tx_status and based on the residue (if it is 0 then
> > it is done) decide what to tell the client.
> > 
> > Should the client call dmaengine_terminate_* after the polling returned
> > unconditionally to free up the descriptor?
> 
> This is how omap-dma is handling the polled memcpy support.

Yes that is a good question. Even if the client does not set
DMA_PREP_INTERRUPT would there be no interrupt generated by controller
on txn completion? If not how will next txn be submitted to the
hardware.

I think we should view DMA_PREP_INTERRUPT from client pov, but
controller cannot get away with disabling interrupts IMO.

Assuming I had enough caffeine before I thought process, then client would
poll descriptor status using cookie and should free up once the cookie
is freed, makes sense?

-- 
~Vinod

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-04 12:45     ` Vinod Koul
@ 2019-06-04 13:35       ` Peter Ujfalusi
  2019-06-10  7:04         ` Vinod Koul
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Ujfalusi @ 2019-06-04 13:35 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko



On 04/06/2019 15.45, Vinod Koul wrote:
> On 03-06-19, 10:05, Peter Ujfalusi wrote:
> 
>>> I think the main question is how polling for completion should be
>>> handled when client does not request for completion interrupt, thus we
>>> will have no callback in the DMA driver when the transfer is completed.
>>>
>>> If DMA_PREP_INTERRUPT is set for the tx_descriptor then the polling will
>>> wait until the DMA driver internally receives the interrupt that the
>>> transfer is done and sets the cookie to completed state.
>>>
>>> However if DMA_PREP_INTERRUPT is not set, the DMA driver will not get
>>> notification from the HW that is the transfer is done, the only way to
>>> know is to check the tx_status and based on the residue (if it is 0 then
>>> it is done) decide what to tell the client.
>>>
>>> Should the client call dmaengine_terminate_* after the polling returned
>>> unconditionally to free up the descriptor?
>>
>> This is how omap-dma is handling the polled memcpy support.
> 
> Yes that is a good question. Even if the client does not set
> DMA_PREP_INTERRUPT would there be no interrupt generated by controller
> on txn completion? If not how will next txn be submitted to the
> hardware.
> 
> I think we should view DMA_PREP_INTERRUPT from client pov, but
> controller cannot get away with disabling interrupts IMO.

What happens if client is issuing a DMA memcpy (short one) while
interrupts are disabled?

The user for this is:
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c

commit: f5b9930b85dc6319fd6bcc259e447eff62fc691c

The interrupt based completion is not going to work in some cases, the
DMA driver should obey that the missing DMA_PREP_INTERRUPT really
implies that interrupts can not be used.

> Assuming I had enough caffeine before I thought process, then client would
> poll descriptor status using cookie and should free up once the cookie
> is freed, makes sense?

OK, so clients are expected to call dmaengine_terminate_*
unconditionally after the transfer is completed, right?

If we use interrupts then the handler would anyway free up the
descriptor, so terminating should not do any harm, if we can not have
interrupts then terminate will clear up the completed descriptor
proactively.

In any case I have updated the EDMA patch to do the same thing in case
of polling w/o interrupts as it would do in the completion irq handler,
and similar approach prepared for omap-dma as well.

- Péter

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-04 13:35       ` Peter Ujfalusi
@ 2019-06-10  7:04         ` Vinod Koul
  2019-06-10 11:12           ` Peter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2019-06-10  7:04 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

On 04-06-19, 16:35, Peter Ujfalusi wrote:
> 
> 
> On 04/06/2019 15.45, Vinod Koul wrote:
> > On 03-06-19, 10:05, Peter Ujfalusi wrote:
> > 
> >>> I think the main question is how polling for completion should be
> >>> handled when client does not request for completion interrupt, thus we
> >>> will have no callback in the DMA driver when the transfer is completed.
> >>>
> >>> If DMA_PREP_INTERRUPT is set for the tx_descriptor then the polling will
> >>> wait until the DMA driver internally receives the interrupt that the
> >>> transfer is done and sets the cookie to completed state.
> >>>
> >>> However if DMA_PREP_INTERRUPT is not set, the DMA driver will not get
> >>> notification from the HW that is the transfer is done, the only way to
> >>> know is to check the tx_status and based on the residue (if it is 0 then
> >>> it is done) decide what to tell the client.
> >>>
> >>> Should the client call dmaengine_terminate_* after the polling returned
> >>> unconditionally to free up the descriptor?
> >>
> >> This is how omap-dma is handling the polled memcpy support.
> > 
> > Yes that is a good question. Even if the client does not set
> > DMA_PREP_INTERRUPT would there be no interrupt generated by controller
> > on txn completion? If not how will next txn be submitted to the
> > hardware.
> > 
> > I think we should view DMA_PREP_INTERRUPT from client pov, but
> > controller cannot get away with disabling interrupts IMO.
> 
> What happens if client is issuing a DMA memcpy (short one) while
> interrupts are disabled?
> 
> The user for this is:
> drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> 
> commit: f5b9930b85dc6319fd6bcc259e447eff62fc691c
> 
> The interrupt based completion is not going to work in some cases, the
> DMA driver should obey that the missing DMA_PREP_INTERRUPT really
> implies that interrupts can not be used.

well yes but how do we *assume* completion and issue subsequent txns?
Does driver create a task and poll?

> 
> > Assuming I had enough caffeine before I thought process, then client would
> > poll descriptor status using cookie and should free up once the cookie
> > is freed, makes sense?
> 
> OK, so clients are expected to call dmaengine_terminate_*
> unconditionally after the transfer is completed, right?

How do you know/detect transfer is completed?
> 
> If we use interrupts then the handler would anyway free up the
> descriptor, so terminating should not do any harm, if we can not have
> interrupts then terminate will clear up the completed descriptor
> proactively.

yes terminate part is fine.

> In any case I have updated the EDMA patch to do the same thing in case
> of polling w/o interrupts as it would do in the completion irq handler,
> and similar approach prepared for omap-dma as well.
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
~Vinod

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-10  7:04         ` Vinod Koul
@ 2019-06-10 11:12           ` Peter Ujfalusi
  2019-06-11  4:47             ` Vinod Koul
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Ujfalusi @ 2019-06-10 11:12 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko



On 10/06/2019 10.04, Vinod Koul wrote:
>>> I think we should view DMA_PREP_INTERRUPT from client pov, but
>>> controller cannot get away with disabling interrupts IMO.
>>
>> What happens if client is issuing a DMA memcpy (short one) while
>> interrupts are disabled?
>>
>> The user for this is:
>> drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
>>
>> commit: f5b9930b85dc6319fd6bcc259e447eff62fc691c
>>
>> The interrupt based completion is not going to work in some cases, the
>> DMA driver should obey that the missing DMA_PREP_INTERRUPT really
>> implies that interrupts can not be used.
> 
> well yes but how do we *assume* completion and issue subsequent txns?
> Does driver create a task and poll?

The client driver will poll on tx_status, like using dma_sync_wait().
The DMA driver is expected to check if the transfer is completed by
other means than relying on the interrupt for transfer completion.

>>
>>> Assuming I had enough caffeine before I thought process, then client would
>>> poll descriptor status using cookie and should free up once the cookie
>>> is freed, makes sense?
>>
>> OK, so clients are expected to call dmaengine_terminate_*
>> unconditionally after the transfer is completed, right?
> 
> How do you know/detect transfer is completed?

This is a bit tricky and depends on the DMA hardware.
For sDMA (omap-dma) we already do this by checking the channel status.
The channel will be switched to idle if the transfer is completed.

EDMA on the other hand does not provide straight forward way to check if
the transfer is completed w/o interrupts, however we can see it if the
CC loaded the closing dummy paRAM slot (address is 0).

If I want to enable this for UDMAP then I would check the return ring if
I got back the descriptor or not.

>> If we use interrupts then the handler would anyway free up the
>> descriptor, so terminating should not do any harm, if we can not have
>> interrupts then terminate will clear up the completed descriptor
>> proactively.
> 
> yes terminate part is fine.

OK, so I don't need to change this patch for dmatest, right?

I'll prepare the EDMA patch and an update for omap-dma as well.

>> In any case I have updated the EDMA patch to do the same thing in case
>> of polling w/o interrupts as it would do in the completion irq handler,
>> and similar approach prepared for omap-dma as well.
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-10 11:12           ` Peter Ujfalusi
@ 2019-06-11  4:47             ` Vinod Koul
  2019-06-11 13:41               ` Peter Ujfalusi
  0 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2019-06-11  4:47 UTC (permalink / raw)
  To: Peter Ujfalusi; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

On 10-06-19, 14:12, Peter Ujfalusi wrote:
> 
> 
> On 10/06/2019 10.04, Vinod Koul wrote:
> >>> I think we should view DMA_PREP_INTERRUPT from client pov, but
> >>> controller cannot get away with disabling interrupts IMO.
> >>
> >> What happens if client is issuing a DMA memcpy (short one) while
> >> interrupts are disabled?
> >>
> >> The user for this is:
> >> drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> >>
> >> commit: f5b9930b85dc6319fd6bcc259e447eff62fc691c
> >>
> >> The interrupt based completion is not going to work in some cases, the
> >> DMA driver should obey that the missing DMA_PREP_INTERRUPT really
> >> implies that interrupts can not be used.
> > 
> > well yes but how do we *assume* completion and issue subsequent txns?
> > Does driver create a task and poll?
> 
> The client driver will poll on tx_status, like using dma_sync_wait().
> The DMA driver is expected to check if the transfer is completed by
> other means than relying on the interrupt for transfer completion.
> 
> >>
> >>> Assuming I had enough caffeine before I thought process, then client would
> >>> poll descriptor status using cookie and should free up once the cookie
> >>> is freed, makes sense?
> >>
> >> OK, so clients are expected to call dmaengine_terminate_*
> >> unconditionally after the transfer is completed, right?
> > 
> > How do you know/detect transfer is completed?
> 
> This is a bit tricky and depends on the DMA hardware.
> For sDMA (omap-dma) we already do this by checking the channel status.
> The channel will be switched to idle if the transfer is completed.

Well we are talking about DMA and doing this kind of things doesn't make
sense to me. Why not do memcpy/pio instead. DMA is designed to finish
txn fast and move to next one, if we cannot do that, I would say lets
not use dmaengine in those cases! Keeping dmaengine idle is not really
good design.

> EDMA on the other hand does not provide straight forward way to check if
> the transfer is completed w/o interrupts, however we can see it if the
> CC loaded the closing dummy paRAM slot (address is 0).
> 
> If I want to enable this for UDMAP then I would check the return ring if
> I got back the descriptor or not.
> 
> >> If we use interrupts then the handler would anyway free up the
> >> descriptor, so terminating should not do any harm, if we can not have
> >> interrupts then terminate will clear up the completed descriptor
> >> proactively.
> > 
> > yes terminate part is fine.
> 
> OK, so I don't need to change this patch for dmatest, right?
> 
> I'll prepare the EDMA patch and an update for omap-dma as well.
> 
> >> In any case I have updated the EDMA patch to do the same thing in case
> >> of polling w/o interrupts as it would do in the completion irq handler,
> >> and similar approach prepared for omap-dma as well.
> >>
> >> - Péter
> >>
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> > 
> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 
~Vinod

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dmaengine: dmatest: Add support for completion polling
  2019-06-11  4:47             ` Vinod Koul
@ 2019-06-11 13:41               ` Peter Ujfalusi
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Ujfalusi @ 2019-06-11 13:41 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, andriy.shevchenko

On 11/06/2019 7.47, Vinod Koul wrote:
> On 10-06-19, 14:12, Peter Ujfalusi wrote:
>>> How do you know/detect transfer is completed?
>>
>> This is a bit tricky and depends on the DMA hardware.
>> For sDMA (omap-dma) we already do this by checking the channel status.
>> The channel will be switched to idle if the transfer is completed.
> 
> Well we are talking about DMA and doing this kind of things doesn't make
> sense to me. Why not do memcpy/pio instead. DMA is designed to finish
> txn fast and move to next one, if we cannot do that, I would say lets
> not use dmaengine in those cases! Keeping dmaengine idle is not really
> good design.

The dra7 case is a special one: we have one errata (i878) for which the
workaround is to use something else than MCU to access DMM registers,
otherwise bus/system lockup can happen. The only feasible non MCU access
is to use DMA memcpy to read/write registers.

I've also encountered user who want to swap CPU memcpy with DMA based
one as it tends to lower the CPU load and it is faster.

- Péter

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-06-11 13:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-29  8:37 [PATCH] dmaengine: dmatest: Add support for completion polling Peter Ujfalusi
2019-05-31  6:54 ` Peter Ujfalusi
2019-06-03  7:05   ` Peter Ujfalusi
2019-06-04 12:45     ` Vinod Koul
2019-06-04 13:35       ` Peter Ujfalusi
2019-06-10  7:04         ` Vinod Koul
2019-06-10 11:12           ` Peter Ujfalusi
2019-06-11  4:47             ` Vinod Koul
2019-06-11 13:41               ` Peter Ujfalusi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).