From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754735AbYLBTK1 (ORCPT ); Tue, 2 Dec 2008 14:10:27 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754231AbYLBTKM (ORCPT ); Tue, 2 Dec 2008 14:10:12 -0500 Received: from mga03.intel.com ([143.182.124.21]:11393 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751252AbYLBTKJ (ORCPT ); Tue, 2 Dec 2008 14:10:09 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.33,703,1220252400"; d="scan'208";a="85600802" Subject: Re: [PATCH 07/13] dmaengine: introduce dma_request_channel and private channels From: Dan Williams To: Guennadi Liakhovetski Cc: "linux-kernel@vger.kernel.org" , "netdev@vger.kernel.org" , "Sosnowski, Maciej" , "hskinnemoen@atmel.com" , "nicolas.ferre@atmel.com" In-Reply-To: References: <20081114213300.32354.1154.stgit@dwillia2-linux.ch.intel.com> <20081114213453.32354.53002.stgit@dwillia2-linux.ch.intel.com> Content-Type: text/plain Date: Tue, 02 Dec 2008 12:10:07 -0700 Message-Id: <1228245007.8408.20.camel@dwillia2-linux.ch.intel.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 (2.22.3.1-1.fc9) Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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); > > > 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