2008-11-11 09:36:34

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH 2/4 v1] i.MX31: Image Processing Unit DMA and IRQ drivers

(moved to LKML and added Haavard as the author of dw_dmac and DMA_SLAVE
support to cc:)

On Fri, 7 Nov 2008, Guennadi Liakhovetski wrote:

> Now to the problems I encountered with the dmaengine framework. Basically
> I was unable to find out how I can release specific transfer-descriptors
> without using ->device_terminate_all(), and channels without unregistering
> the client.
>
> I did see the DMA_CTRL_ACK flag, but it looks like it is used
> inconsistently in various drivers. In the header it is described as
>
> * @DMA_CTRL_ACK - the descriptor cannot be reused until the client
> * acknowledges receipt, i.e. has has a chance to establish any
> * dependency chains
>
> but I haven't found any way to actually "acknowledge receipt" of the
> descriptor. There are two macros in dmaengine.h: async_tx_ack() and
> async_tx_test_ack() but their implementation doesn't look right:
> async_tx_ack() does just
>
> tx->flags |= DMA_CTRL_ACK;
>
> and async_tx_test_ack()
>
> return (tx->flags & DMA_CTRL_ACK) == DMA_CTRL_ACK;
>
> So, instead of using this flag to say "do not release until acked" it now
> means "acked, can be released?" In ipu_idmac.c I have to be able to
> release specific transfer-descriptors, so, I added a .tx_free() pointer to
> struct dma_async_tx_descriptor.
>
> As for releasing channels - there is a dma_chan_cleanup() function, which
> calls the .device_free_chan_resources() method in struct dma_device and
> decremts channel's refcount, but this doesn't seem to lead to the
> chan->client_count counter being decremented, i.e.,
> dma_clients_notify_removed() never gets called as it, probably, should be.
> I have no solution to this problem, so, releasing specific channels in
> ipu_idmac.c currently doesn't work.

I still have no solution to this ->client_count problem. Is this a bug and
there should be a way to release a channel (including decrementint the
client_count) without unregistering the client, or am I missing something?

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


2008-11-11 15:37:46

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH 2/4 v1] i.MX31: Image Processing Unit DMA and IRQ drivers

On Tue, Nov 11, 2008 at 2:36 AM, Guennadi Liakhovetski
<[email protected]> wrote:
> I still have no solution to this ->client_count problem. Is this a bug and
> there should be a way to release a channel (including decrementint the
> client_count) without unregistering the client, or am I missing something?
>

Hi Guennadi,

I am currently working on improving this situation:

http://marc.info/?l=linux-kernel&m=122463374500746&w=2

Please have a look.

Thanks,
Dan

2008-11-11 18:13:46

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH 2/4 v1] i.MX31: Image Processing Unit DMA and IRQ drivers

Hi Dan,

On Tue, 11 Nov 2008, Dan Williams wrote:

> On Tue, Nov 11, 2008 at 2:36 AM, Guennadi Liakhovetski
> <[email protected]> wrote:
> > I still have no solution to this ->client_count problem. Is this a bug and
> > there should be a way to release a channel (including decrementint the
> > client_count) without unregistering the client, or am I missing something?
> >
>
> Hi Guennadi,
>
> I am currently working on improving this situation:
>
> http://marc.info/?l=linux-kernel&m=122463374500746&w=2
>
> Please have a look.

Thanks for the pointer, I looked at it.

1. ASYNC_TX_ACK: one either sets this flag immediately on a descriptor in
flags for ->device_prep_dma_memcpy() (like in
dma_async_memcpy_buf_to_buf()), which would mean "may be recycled
automatically" or you don't set it, then the descriptor is kept, until you
call async_tx_ack() on it. Are you going to preserve the async_tx_ack()
implementation? In this case dma-device drivers have to scan their
descriptors to do garbage-collection?... Wouldn't it be better to
introduce an explicit call to free a descriptor, like in this my patch:
http://marc.info/?l=linux-arm-kernel&m=122607472721145&w=2 (see the last
hank for include/linux/dmaengine.h)?

2. channel client-count: am I right, that with the current implementation
one cannot free a channel in a way, that its client_count got decremented,
and the ->device_free_chan_resources() method be called for it? Is this
going to be addressed too or I am missing something and channel freeing
has to be done differently?

3. you write

> 3b/ Remove the client registration infrastructure

which means, there should be an interface to free a channel explicitly?
because until now unregistering the client was the only way (that I found)
to free channels...

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