lkml.org 
[lkml]   [2008]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 07/13] dmaengine: introduce dma_request_channel and private channels
On Tue, 2 Dec 2008, Dan Williams wrote:

> 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):

Good, so, would you commit it?

> 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.

You mean whether it makes sense at all to manage these framebuffer
channels outside of the framebuffer driver in a dma driver? I think yes.
Simply because these 2 channels used by the fb share code and _registers_
with the rest 30 channels, which are all also quite specialised.

As for excalating the initcall level - the dmaengine init function doesn't
do much - it just registers a device class and initialises a mutex -
shouldn't be a problem to do it earlier?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

\
 
 \ /
  Last update: 2008-12-02 22:31    [from the cache]
©2003-2011 Jasper Spaans