Thread (52 messages) 52 messages, 6 authors, 2009-02-06

Re: [PATCH 07/13] dmaengine: introduce dma_request_channel and private channels

From: Dan Williams <hidden>
Date: 2008-12-02 19:10:27
Also in: lkml
Subsystem: dma generic offload engine subsystem, the rest · Maintainers: Vinod Koul, Linus Torvalds

On Tue, 2008-12-02 at 10:27 -0700, Guennadi Liakhovetski wrote:
Ooh... Do you really think registering 32 dma-devices is a better solution 
than allowing non-equal dma-channels? As I explained in the commit 
comment, this is a specialised Image Processing DMA Controller, and each 
its channel has a fixed role. So, each client has to get a specific 
channel.
I see your point.  Rather than doing driver gymnastics we can simply
have dmaengine do the following (basically your patch reformatted a
bit):
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index e2ccfd0..66d0ae7 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -445,10 +445,10 @@ static void dma_channel_rebalance(void)
 		}
 }
 
-static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_device *dev)
+static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_device *dev,
+					  dma_filter_fn fn, void *fn_param)
 {
 	struct dma_chan *chan;
-	struct dma_chan *ret = NULL;
 
 	/* devices with multiple channels need special handling as we need to
 	 * ensure that all channels are either private or public.
@@ -471,11 +471,15 @@ static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_devic
 				 __func__, dma_chan_name(chan));
 			continue;
 		}
-		ret = chan;
-		break;
+		if (fn && !fn(chan, fn_param)) {
+			pr_debug("%s: %s filter said false\n",
+				 __func__, dma_chan_name(chan));
+			continue;
+		}
+		return chan;
 	}
 
-	return ret;
+	return NULL;
 }
 
 /**
@@ -488,22 +492,13 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
 {
 	struct dma_device *device, *_d;
 	struct dma_chan *chan = NULL;
-	bool ack;
 	int err;
 
 	/* Find a channel */
 	mutex_lock(&dma_list_mutex);
 	list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
-		chan = private_candidate(mask, device);
-		if (!chan)
-			continue;
-
-		if (fn)
-			ack = fn(chan, fn_param);
-		else
-			ack = true;
-
-		if (ack) {
+		chan = private_candidate(mask, device, fn, fn_param);
+		if (chan) {
 			/* Found a suitable channel, try to grab, prep, and
 			 * return it.  We first set DMA_PRIVATE to disable
 			 * balance_ref_count as this channel will not be
@@ -521,10 +516,8 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
 				       dma_chan_name(chan), err);
 			else
 				break;
-		} else
-			pr_debug("%s: %s filter said false\n",
-				 __func__, dma_chan_name(chan));
-		chan = NULL;
+			chan = NULL;
+		}
 	}
 	mutex_unlock(&dma_list_mutex);
 

quoted
quoted
Another problem I encountered with my framebuffer is the initialisation
order. You initialise dmaengine per subsys_initcall(), whereas the only
way to guarantee the order:

dmaengine
dma-device driver
framebuffer
hmm... can the framebuffer be moved to late_initcall?
I assumed, that one wants to register the framebuffer as early as 
possible...
Yes, but I'm hesitant to escalate the initcall level.  It sounds like
the channel(s?) for the framebuffer are an integral part of the
framebuffer device so maybe they should not be registered separately?
But that runs into issues if you want the channels to return to the
general pool when the framebuffer driver is unloaded.

--
Dan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help