Skip to contentSkip to conversation

dma: pl330: revert commit 04abf5daf7d

PATCH New Jassi Brar 3 participants 3 messages Dec 5, 2014 dmaengine

lore
Share & tools
lore

Jassi Brar

author
#0 lore raw

PATCH 1/1 dma: pl330: revert commit 04abf5daf7d

Quoted 1 line

From Documentation/crypto/async-tx-api.txt

"... Once a driver-specific threshold is met the driver
automatically issues pending operations. An application can force this event by calling async_tx_issue_pending_all() ..."
That is, the DMA controller drivers may buffer transfer requests
for optimization. However it is perfectly legal to start dma as soon as the user calls .tx_submit() on the descriptor, as the documentation specifies in include/linux/dmaengine.h

It is also consistent with the expected behaviour. For example, if a channel is already active when a client submits a request, we do not expect the channel to stop after the current request is done and wait for explicit issue_pending() from the user (the user has already done prep() and submit()) on the last request.

Signed-off-by Jassi Brar <[email protected]>
---
 drivers/dma/pl330.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)
drivers/dma/pl330.c +4 -20
index 19a9974..8ec5f1f 100644
@@ -411,9 +411,7 @@ struct dma_pl330_chan {
 	/* DMA-Engine Channel */
 	struct dma_chan chan;
 
-	/* List of submitted descriptors */
-	struct list_head submitted_list;
-	/* List of issued descriptors */
+	/* List of to be xfered descriptors */
 	struct list_head work_list;
 	/* List of completed descriptors */
 	struct list_head completed_list;

@@ -2084,11 +2082,6 @@ static int pl330_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, unsigned
 		pch->thread->req_running = -1;
 
 		/* Mark all desc done */
-		list_for_each_entry(desc, &pch->submitted_list, node) {
-			desc->status = FREE;
-			dma_cookie_complete(&desc->txd);
-		}
-
 		list_for_each_entry(desc, &pch->work_list , node) {
 			desc->status = FREE;
 			dma_cookie_complete(&desc->txd);

@@ -2099,7 +2092,6 @@ static int pl330_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, unsigned
 			dma_cookie_complete(&desc->txd);
 		}
 
-		list_splice_tail_init(&pch->submitted_list, &pl330->desc_pool);
 		list_splice_tail_init(&pch->work_list, &pl330->desc_pool);
 		list_splice_tail_init(&pch->completed_list, &pl330->desc_pool);
 		spin_unlock_irqrestore(&pch->lock, flags);

@@ -2158,14 +2150,7 @@ pl330_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 
 static void pl330_issue_pending(struct dma_chan *chan)
 {
-	struct dma_pl330_chan *pch = to_pchan(chan);
-	unsigned long flags;
-
-	spin_lock_irqsave(&pch->lock, flags);
-	list_splice_tail_init(&pch->submitted_list, &pch->work_list);
-	spin_unlock_irqrestore(&pch->lock, flags);
-
-	pl330_tasklet((unsigned long)pch);
+	pl330_tasklet((unsigned long) to_pchan(chan));
 }
 
 /*

@@ -2192,11 +2177,11 @@ static dma_cookie_t pl330_tx_submit(struct dma_async_tx_descriptor *tx)
 
 		dma_cookie_assign(&desc->txd);
 
-		list_move_tail(&desc->node, &pch->submitted_list);
+		list_move_tail(&desc->node, &pch->work_list);
 	}
 
 	cookie = dma_cookie_assign(&last->txd);
-	list_add_tail(&last->node, &pch->submitted_list);
+	list_add_tail(&last->node, &pch->work_list);
 	spin_unlock_irqrestore(&pch->lock, flags);
 
 	return cookie;

@@ -2680,7 +2665,6 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		else
 			pch->chan.private = adev->dev.of_node;
 
-		INIT_LIST_HEAD(&pch->submitted_list);
 		INIT_LIST_HEAD(&pch->work_list);
 		INIT_LIST_HEAD(&pch->completed_list);
 		spin_lock_init(&pch->lock);
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 dma: pl330: revert commit 04abf5daf7d

Quoted 8 lines · Jassi Brar

From Documentation/crypto/async-tx-api.txt
"... Once a driver-specific threshold is met the driver
automatically issues pending operations. An application can force this event by calling async_tx_issue_pending_all() ..."
That is, the DMA controller drivers may buffer transfer requests
for optimization. However it is perfectly legal to start dma as soon as the user calls .tx_submit() on the descriptor, as the documentation specifies in include/linux/dmaengine.h

It's not according to what is in the DMAengine documentation (Documentation/dmaengine.txt) and what we have been telling people for the last couple of years.

There are supposed to be two different queues one for pending descriptors and one for active descriptors. submit() adds a descriptor to the pending list and issue_pending() moves all descriptors from the pending list to the active list. Especially the driver must not automatically start transferring a descriptor after it has been submitted but before issue_pending() has been called.

- Lars

#2 lore raw

PATCH 1/1 dma: pl330: revert commit 04abf5daf7d

Quoted 9 lines · Lars-Peter Clausen
Quoted 8 lines · Jassi Brar

From Documentation/crypto/async-tx-api.txt
"... Once a driver-specific threshold is met the driver
automatically issues pending operations. An application can force this event by calling async_tx_issue_pending_all() ..."
That is, the DMA controller drivers may buffer transfer requests
for optimization. However it is perfectly legal to start dma as soon as the user calls .tx_submit() on the descriptor, as the documentation specifies in include/linux/dmaengine.h

Noooo, submit does not mean that as all

Quoted 11 lines

It's not according to what is in the DMAengine documentation (Documentation/dmaengine.txt) and what we have been telling people for the last couple of years.

There are supposed to be two different queues one for pending descriptors and one for active descriptors. submit() adds a descriptor to the pending list and issue_pending() moves all descriptors from the pending list to the active list. Especially the driver must not automatically start transferring a descriptor after it has been submitted but before issue_pending() has been called.

right

to quote the Documentation:

+ 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.
* device_issue_pending
- Takes the first transaction descriptor in the pending queue, and starts the transfer. Whenever that transfer is done, it should move to the next transaction in the list.
- This function can be called in an interrupt context

Pls see Documentation/dmaengine/provider.txt in dmaengine-next

Signature · 1 line
~Vinod