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    [W:0.029 / U:63.144 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site