All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] DMAEngine: Define generic transfer request api
       [not found] <CAGsJ_4wXURUwbf-fcNOq1m5-NJ9+VMuDq+9OJpBjFZK4C_X3cw@mail.gmail.com>
@ 2011-09-12 16:26 ` Barry Song
  2011-09-12 16:54   ` Jassi Brar
  0 siblings, 1 reply; 21+ messages in thread
From: Barry Song @ 2011-09-12 16:26 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

> Define a new api that could be used for doing fancy data transfers
> like interleaved to contiguous copy and vice-versa.
> Traditional SG_list based transfers tend to be very inefficient in
> such cases as where the interleave and chunk are only a few bytes,
> which call for a very condensed api to convey pattern of the transfer.
>
> This api supports all 4 variants of scatter-gather and contiguous transfer.
> Besides, it could also represent common operations like
>        device_prep_dma_{cyclic, memset, memcpy}
> and maybe some more that I am not sure of.
>
> Of course, neither can this api help transfers that don't lend to DMA by
> nature, i.e, scattered tiny read/writes with no periodic pattern.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

anyway, this API needs a real user to prove why it needs to exist.

prima2 can be the 1st(?, 2nd if TI uses) user of this API. let's try
to see what the driver will be with this api. Then we might figure out
more about what it should be.

> ---
>  include/linux/dmaengine.h |   73 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 8fbf40e..74f3ae0 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -76,6 +76,76 @@ enum dma_transaction_type {
>  /* last transaction type for creation of the capabilities mask */
>  #define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
>
> +/**
> + * Generic Transfer Request
> + * ------------------------
> + * A chunk is collection of contiguous bytes to be transfered.
> + * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
> + * ICGs may or maynot change between chunks.
> + * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
> + *  that when repeated an integral number of times, specifies the transfer.
> + * A transfer template is specification of a Frame, the number of times
> + *  it is to be repeated and other per-transfer attributes.
> + *
> + * Practically, a client driver would have ready a template for each
> + *  type of transfer it is going to need during its lifetime and
> + *  set only 'src_start' and 'dst_start' before submitting the requests.
> + *
> + *
> + *  |      Frame-1        |       Frame-2       | ~ |       Frame-'numf'  |
> + *  |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
> + *
> + *    ==  Chunk size
> + *    ... ICG
> + */
> +
> +/**
> + * struct data_chunk - Element of scatter-gather list that makes a frame.
> + * @size: Number of bytes to read from source.
> + *       size_dst := fn(op, size_src), so doesn't mean much for destination.
> + * @icg: Number of bytes to jump after last src/dst address of this
> + *      chunk and before first src/dst address for next chunk.
> + *      Ignored for dst(assumed 0), if dst_inc is true and dst_sgl is false.
> + *      Ignored for src(assumed 0), if src_inc is true and src_sgl is false.
> + */
> +struct data_chunk {
> +       size_t size;
> +       size_t icg;
> +};
> +
> +/**
> + * struct xfer_template - Template to convey DMAC the transfer pattern
> + *      and attributes.
> + * @op: The operation to perform on source data before writing it on
> + *      to destination address.
> + * @src_start: Bus address of source for the first chunk.
> + * @dst_start: Bus address of destination for the first chunk.
> + * @src_inc: If the source address increments after reading from it.
> + * @dst_inc: If the destination address increments after writing to it.
> + * @src_sgl: If the 'icg' of sgl[] applies to Source (scattered read).
> + *             Otherwise, source is read contiguously (icg ignored).
> + *             Ignored if src_inc is false.
> + * @dst_sgl: If the 'icg' of sgl[] applies to Destination (scattered write).
> + *             Otherwise, destination is filled contiguously (icg ignored).
> + *             Ignored if dst_inc is false.
> + * @frm_irq: If the client expects DMAC driver to do callback after each frame.
> + * @numf: Number of frames in this template.
> + * @frame_size: Number of chunks in a frame i.e, size of sgl[].
> + * @sgl: Array of {chunk,icg} pairs that make up a frame.
> + */
> +struct xfer_template {
> +       enum dma_transaction_type op;
> +       dma_addr_t src_start;
> +       dma_addr_t dst_start;
> +       bool src_inc;
> +       bool dst_inc;
> +       bool src_sgl;
> +       bool dst_sgl;
> +       bool frm_irq;
> +       size_t numf;
> +       size_t frame_size;
> +       struct data_chunk sgl[0];
> +};
>
>  /**
>  * enum dma_ctrl_flags - DMA flags to augment operation preparation,
> @@ -432,6 +502,7 @@ struct dma_tx_state {
>  * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
>  *     The function takes a buffer of size buf_len. The callback function will
>  *     be called after period_len bytes have been transferred.
> + * @device_prep_dma_genxfer: Transfer expression in a generic way.
>  * @device_control: manipulate all pending operations on a channel, returns
>  *     zero or error code
>  * @device_tx_status: poll for transaction completion, the optional
> @@ -496,6 +567,8 @@ struct dma_device {
>        struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
>                struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>                size_t period_len, enum dma_data_direction direction);
> +       struct dma_async_tx_descriptor *(*device_prep_dma_genxfer)(
> +               struct dma_chan *chan, struct xfer_template *xt);
>        int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>                unsigned long arg);
>
> --
> 1.7.4.1
>
-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-12 16:26 ` [PATCH] DMAEngine: Define generic transfer request api Barry Song
@ 2011-09-12 16:54   ` Jassi Brar
  2011-09-13  1:21     ` Barry Song
  0 siblings, 1 reply; 21+ messages in thread
From: Jassi Brar @ 2011-09-12 16:54 UTC (permalink / raw)
  To: Barry Song
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

On 12 September 2011 21:56, Barry Song <21cnbao@gmail.com> wrote:
>> Define a new api that could be used for doing fancy data transfers
>> like interleaved to contiguous copy and vice-versa.
>> Traditional SG_list based transfers tend to be very inefficient in
>> such cases as where the interleave and chunk are only a few bytes,
>> which call for a very condensed api to convey pattern of the transfer.
>>
>> This api supports all 4 variants of scatter-gather and contiguous transfer.
>> Besides, it could also represent common operations like
>>        device_prep_dma_{cyclic, memset, memcpy}
>> and maybe some more that I am not sure of.
>>
>> Of course, neither can this api help transfers that don't lend to DMA by
>> nature, i.e, scattered tiny read/writes with no periodic pattern.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>
> anyway, this API needs a real user to prove why it needs to exist.
>
> prima2 can be the 1st(?, 2nd if TI uses) user of this API. let's try
> to see what the driver will be with this api. Then we might figure out
> more about what it should be.
>
Did you discover any issue with the api?
Because only three days ago you said
{
Jassi, you might think my reply as an ACK to "[PATCH] DMAEngine:
Define generic transfer request api".
}

The api met your requirements easily not because I know them already,
but because I designed the api to be as generic as practically possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-12 16:54   ` Jassi Brar
@ 2011-09-13  1:21     ` Barry Song
  2011-09-13  7:46       ` Barry Song
  0 siblings, 1 reply; 21+ messages in thread
From: Barry Song @ 2011-09-13  1:21 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
> On 12 September 2011 21:56, Barry Song <21cnbao@gmail.com> wrote:
>>> Define a new api that could be used for doing fancy data transfers
>>> like interleaved to contiguous copy and vice-versa.
>>> Traditional SG_list based transfers tend to be very inefficient in
>>> such cases as where the interleave and chunk are only a few bytes,
>>> which call for a very condensed api to convey pattern of the transfer.
>>>
>>> This api supports all 4 variants of scatter-gather and contiguous transfer.
>>> Besides, it could also represent common operations like
>>>        device_prep_dma_{cyclic, memset, memcpy}
>>> and maybe some more that I am not sure of.
>>>
>>> Of course, neither can this api help transfers that don't lend to DMA by
>>> nature, i.e, scattered tiny read/writes with no periodic pattern.
>>>
>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>>
>> anyway, this API needs a real user to prove why it needs to exist.
>>
>> prima2 can be the 1st(?, 2nd if TI uses) user of this API. let's try
>> to see what the driver will be with this api. Then we might figure out
>> more about what it should be.
>>
> Did you discover any issue with the api?

no until now, but i need to test as i said since there is nobody else
has used it before. so i just hold the formal ACK for a moment.

> Because only three days ago you said
> {
> Jassi, you might think my reply as an ACK to "[PATCH] DMAEngine:
> Define generic transfer request api".
> }

if test pass, to the patch, and even for the moment, to the API's idea
Acked-by: Barry Song <baohua.song@csr.com>

>
> The api met your requirements easily not because I know them already,
> but because I designed the api to be as generic as practically possible.
>

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-13  1:21     ` Barry Song
@ 2011-09-13  7:46       ` Barry Song
  2011-09-13  8:43         ` Jassi Brar
  0 siblings, 1 reply; 21+ messages in thread
From: Barry Song @ 2011-09-13  7:46 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/13 Barry Song <21cnbao@gmail.com>:
> 2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
>> On 12 September 2011 21:56, Barry Song <21cnbao@gmail.com> wrote:
>>>> Define a new api that could be used for doing fancy data transfers
>>>> like interleaved to contiguous copy and vice-versa.
>>>> Traditional SG_list based transfers tend to be very inefficient in
>>>> such cases as where the interleave and chunk are only a few bytes,
>>>> which call for a very condensed api to convey pattern of the transfer.
>>>>
>>>> This api supports all 4 variants of scatter-gather and contiguous transfer.
>>>> Besides, it could also represent common operations like
>>>>        device_prep_dma_{cyclic, memset, memcpy}
>>>> and maybe some more that I am not sure of.
>>>>
>>>> Of course, neither can this api help transfers that don't lend to DMA by
>>>> nature, i.e, scattered tiny read/writes with no periodic pattern.
>>>>
>>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>>>
>>> anyway, this API needs a real user to prove why it needs to exist.
>>>
>>> prima2 can be the 1st(?, 2nd if TI uses) user of this API. let's try
>>> to see what the driver will be with this api. Then we might figure out
>>> more about what it should be.
>>>
>> Did you discover any issue with the api?
>
> no until now, but i need to test as i said since there is nobody else
> has used it before. so i just hold the formal ACK for a moment.
>
>> Because only three days ago you said
>> {
>> Jassi, you might think my reply as an ACK to "[PATCH] DMAEngine:
>> Define generic transfer request api".
>> }
>
> if test pass, to the patch, and even for the moment, to the API's idea
> Acked-by: Barry Song <baohua.song@csr.com>

one issue i noticed is with a device_prep_dma_genxfer, i don't need
device_prep_slave_sg any more, then the validation check in
dma_async_device_register():

        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
                !device->device_prep_slave_sg);

is wrong to me.

How about:

       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
-               !device->device_prep_dma_memcpy);
+               !device->device_prep_dma_memcpy &&
+               !device->device_prep_dma_genxfer);

        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
 -               !device->device_prep_slave_sg);
+               !device->device_prep_slave_sg &&
+               !device->device_prep_dma_genxfer);

>
>>
>> The api met your requirements easily not because I know them already,
>> but because I designed the api to be as generic as practically possible.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-13  7:46       ` Barry Song
@ 2011-09-13  8:43         ` Jassi Brar
  2011-09-13  8:58           ` Barry Song
  0 siblings, 1 reply; 21+ messages in thread
From: Jassi Brar @ 2011-09-13  8:43 UTC (permalink / raw)
  To: Barry Song
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>> if test pass, to the patch, and even for the moment, to the API's idea
>> Acked-by: Barry Song <baohua.song@csr.com>
>
> one issue i noticed is with a device_prep_dma_genxfer, i don't need
> device_prep_slave_sg any more,
Yeah, the damengine would need to adapt to the fact that these
interleaved tranfers could be Mem->Mem as well as Mem<->Dev
(even though yours could be only one type, but some dmacs could
do both).

> How about:
>
>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
> -               !device->device_prep_dma_memcpy);
> +               !device->device_prep_dma_memcpy &&
> +               !device->device_prep_dma_genxfer);
>
>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>  -               !device->device_prep_slave_sg);
> +               !device->device_prep_slave_sg &&
> +               !device->device_prep_dma_genxfer);
>
Seems ok, but please modify in a way you think is best and submit a patch
on top of this new api. Then it'll be easier to evaluate everything.

thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-13  8:43         ` Jassi Brar
@ 2011-09-13  8:58           ` Barry Song
  2011-09-15  6:31             ` Barry Song
  0 siblings, 1 reply; 21+ messages in thread
From: Barry Song @ 2011-09-13  8:58 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
> On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>>> if test pass, to the patch, and even for the moment, to the API's idea
>>> Acked-by: Barry Song <baohua.song@csr.com>
>>
>> one issue i noticed is with a device_prep_dma_genxfer, i don't need
>> device_prep_slave_sg any more,
> Yeah, the damengine would need to adapt to the fact that these
> interleaved tranfers could be Mem->Mem as well as Mem<->Dev
> (even though yours could be only one type, but some dmacs could
> do both).
>
>> How about:
>>
>>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
>> -               !device->device_prep_dma_memcpy);
>> +               !device->device_prep_dma_memcpy &&
>> +               !device->device_prep_dma_genxfer);
>>
>>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>>  -               !device->device_prep_slave_sg);
>> +               !device->device_prep_slave_sg &&
>> +               !device->device_prep_dma_genxfer);
>>
> Seems ok, but please modify in a way you think is best and submit a patch
> on top of this new api. Then it'll be easier to evaluate everything.

i think it should be handled by this patch but not a new one.

>
> thanks.
>

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-13  8:58           ` Barry Song
@ 2011-09-15  6:31             ` Barry Song
  2011-09-15  6:43               ` Jassi Brar
  0 siblings, 1 reply; 21+ messages in thread
From: Barry Song @ 2011-09-15  6:31 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/13 Barry Song <21cnbao@gmail.com>:
> 2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
>> On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>>>> if test pass, to the patch, and even for the moment, to the API's idea
>>>> Acked-by: Barry Song <baohua.song@csr.com>
>>>
>>> one issue i noticed is with a device_prep_dma_genxfer, i don't need
>>> device_prep_slave_sg any more,
>> Yeah, the damengine would need to adapt to the fact that these
>> interleaved tranfers could be Mem->Mem as well as Mem<->Dev
>> (even though yours could be only one type, but some dmacs could
>> do both).
>>
>>> How about:
>>>
>>>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
>>> -               !device->device_prep_dma_memcpy);
>>> +               !device->device_prep_dma_memcpy &&
>>> +               !device->device_prep_dma_genxfer);
>>>
>>>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>>>  -               !device->device_prep_slave_sg);
>>> +               !device->device_prep_slave_sg &&
>>> +               !device->device_prep_dma_genxfer);
>>>
>> Seems ok, but please modify in a way you think is best and submit a patch
>> on top of this new api. Then it'll be easier to evaluate everything.
>
> i think it should be handled by this patch but not a new one.

and i also think xfer_template is a bad name for a structure which is
an API. i'd like to add namespace for it and rename it to dma_genxfer.
or have any good suggestion?

i'd like to send this together with "BUG_ON(dma_has_cap(DMA_SLAVE,
device->cap_mask) &&!device->device_prep_dma_genxfer)" as v2.

-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-15  6:31             ` Barry Song
@ 2011-09-15  6:43               ` Jassi Brar
  2011-09-15  6:49                 ` Barry Song
  2011-09-15  8:17                 ` Barry Song
  0 siblings, 2 replies; 21+ messages in thread
From: Jassi Brar @ 2011-09-15  6:43 UTC (permalink / raw)
  To: Barry Song
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

On 15 September 2011 12:01, Barry Song <21cnbao@gmail.com> wrote:
> 2011/9/13 Barry Song <21cnbao@gmail.com>:
>> 2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
>>> On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>>>>> if test pass, to the patch, and even for the moment, to the API's idea
>>>>> Acked-by: Barry Song <baohua.song@csr.com>
>>>>
>>>> one issue i noticed is with a device_prep_dma_genxfer, i don't need
>>>> device_prep_slave_sg any more,
>>> Yeah, the damengine would need to adapt to the fact that these
>>> interleaved tranfers could be Mem->Mem as well as Mem<->Dev
>>> (even though yours could be only one type, but some dmacs could
>>> do both).
>>>
>>>> How about:
>>>>
>>>>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
>>>> -               !device->device_prep_dma_memcpy);
>>>> +               !device->device_prep_dma_memcpy &&
>>>> +               !device->device_prep_dma_genxfer);
>>>>
>>>>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>>>>  -               !device->device_prep_slave_sg);
>>>> +               !device->device_prep_slave_sg &&
>>>> +               !device->device_prep_dma_genxfer);
>>>>
>>> Seems ok, but please modify in a way you think is best and submit a patch
>>> on top of this new api. Then it'll be easier to evaluate everything.
>>
>> i think it should be handled by this patch but not a new one.
>
> and i also think xfer_template is a bad name for a structure which is
> an API. i'd like to add namespace for it and rename it to dma_genxfer.
> or have any good suggestion?
I think xfer_template is better - which stresses the usage as having prepared
templates of transfers and only change src/dst address before submitting.
'device_prep_dma_genxfer' is the API which is already named so.

> i'd like to send this together with "BUG_ON(dma_has_cap(DMA_SLAVE,
> device->cap_mask) &&!device->device_prep_dma_genxfer)" as v2.
Is there no change other than skipping check for SLAVE when using this api ?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-15  6:43               ` Jassi Brar
@ 2011-09-15  6:49                 ` Barry Song
  2011-09-15  8:17                 ` Barry Song
  1 sibling, 0 replies; 21+ messages in thread
From: Barry Song @ 2011-09-15  6:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/15 Jassi Brar <jaswinder.singh@linaro.org>:
> On 15 September 2011 12:01, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/13 Barry Song <21cnbao@gmail.com>:
>>> 2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
>>>> On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>>>>>> if test pass, to the patch, and even for the moment, to the API's idea
>>>>>> Acked-by: Barry Song <baohua.song@csr.com>
>>>>>
>>>>> one issue i noticed is with a device_prep_dma_genxfer, i don't need
>>>>> device_prep_slave_sg any more,
>>>> Yeah, the damengine would need to adapt to the fact that these
>>>> interleaved tranfers could be Mem->Mem as well as Mem<->Dev
>>>> (even though yours could be only one type, but some dmacs could
>>>> do both).
>>>>
>>>>> How about:
>>>>>
>>>>>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
>>>>> -               !device->device_prep_dma_memcpy);
>>>>> +               !device->device_prep_dma_memcpy &&
>>>>> +               !device->device_prep_dma_genxfer);
>>>>>
>>>>>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>>>>>  -               !device->device_prep_slave_sg);
>>>>> +               !device->device_prep_slave_sg &&
>>>>> +               !device->device_prep_dma_genxfer);
>>>>>
>>>> Seems ok, but please modify in a way you think is best and submit a patch
>>>> on top of this new api. Then it'll be easier to evaluate everything.
>>>
>>> i think it should be handled by this patch but not a new one.
>>
>> and i also think xfer_template is a bad name for a structure which is
>> an API. i'd like to add namespace for it and rename it to dma_genxfer.
>> or have any good suggestion?
> I think xfer_template is better - which stresses the usage as having prepared
> templates of transfers and only change src/dst address before submitting.
> 'device_prep_dma_genxfer' is the API which is already named so.

sorry i can't agree that.
device_prep_dma_genxfer is an API, xfer_template is a data structure
which will be seen by users, client drivers. it at least needs a
namespace. Otherwise, people someday maybe add another xfer.

>
>> i'd like to send this together with "BUG_ON(dma_has_cap(DMA_SLAVE,
>> device->cap_mask) &&!device->device_prep_dma_genxfer)" as v2.
> Is there no change other than skipping check for SLAVE when using this api ?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-09-15  6:43               ` Jassi Brar
  2011-09-15  6:49                 ` Barry Song
@ 2011-09-15  8:17                 ` Barry Song
  1 sibling, 0 replies; 21+ messages in thread
From: Barry Song @ 2011-09-15  8:17 UTC (permalink / raw)
  To: Jassi Brar
  Cc: dan.j.williams, sundaram, linus.walleij, vinod.koul, rmk+kernel,
	linux-omap, DL-SHA-WorkGroupLinux

2011/9/15 Jassi Brar <jaswinder.singh@linaro.org>:
> On 15 September 2011 12:01, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/13 Barry Song <21cnbao@gmail.com>:
>>> 2011/9/13 Jassi Brar <jaswinder.singh@linaro.org>:
>>>> On 13 September 2011 13:16, Barry Song <21cnbao@gmail.com> wrote:
>>>>>> if test pass, to the patch, and even for the moment, to the API's idea
>>>>>> Acked-by: Barry Song <baohua.song@csr.com>
>>>>>
>>>>> one issue i noticed is with a device_prep_dma_genxfer, i don't need
>>>>> device_prep_slave_sg any more,
>>>> Yeah, the damengine would need to adapt to the fact that these
>>>> interleaved tranfers could be Mem->Mem as well as Mem<->Dev
>>>> (even though yours could be only one type, but some dmacs could
>>>> do both).
>>>>
>>>>> How about:
>>>>>
>>>>>       BUG_ON(dma_has_cap(DMA_MEMCPY, device->cap_mask) &&
>>>>> -               !device->device_prep_dma_memcpy);
>>>>> +               !device->device_prep_dma_memcpy &&
>>>>> +               !device->device_prep_dma_genxfer);
>>>>>
>>>>>        BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>>>>>  -               !device->device_prep_slave_sg);
>>>>> +               !device->device_prep_slave_sg &&
>>>>> +               !device->device_prep_dma_genxfer);
>>>>>
>>>> Seems ok, but please modify in a way you think is best and submit a patch
>>>> on top of this new api. Then it'll be easier to evaluate everything.
>>>
>>> i think it should be handled by this patch but not a new one.
>>
>> and i also think xfer_template is a bad name for a structure which is
>> an API. i'd like to add namespace for it and rename it to dma_genxfer.
>> or have any good suggestion?
> I think xfer_template is better - which stresses the usage as having prepared
> templates of transfers and only change src/dst address before submitting.
> 'device_prep_dma_genxfer' is the API which is already named so.
>
>> i'd like to send this together with "BUG_ON(dma_has_cap(DMA_SLAVE,
>> device->cap_mask) &&!device->device_prep_dma_genxfer)" as v2.
> Is there no change other than skipping check for SLAVE when using this api ?

another change i want to do is a simple xfer alloc helper so that
every driver doesn't need a long line to alloc this struct with a zero
length array:

struct xfer_template  *alloc_xfer_template(size_t frame_size)
{
        kzalloc(sizeof(struct xfer_template) +
                sizeof(struct data_chunk) * frame_size);
}

Then client can fill
xt.sgl[0].size
xt.sgl[0].icg
xt.sgl[1].size
xt.sgl[1].icg
...
xt.sgl[x].size
xt.sgl[x].icg

but xfer_template and data_chunk will have namespace.

>
-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-19 13:43     ` Koul, Vinod
@ 2011-08-23 14:43         ` Matt Porter
  2011-08-23 14:43         ` Matt Porter
  1 sibling, 0 replies; 21+ messages in thread
From: Matt Porter @ 2011-08-23 14:43 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Linus Walleij, Jassi Brar, sundaram, dan.j.williams,
	linux-kernel, linux-omap, rmk+kernel, nsekhar

On Aug 19, 2011, at 9:43 AM, Koul, Vinod wrote:

> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> 
>>> Currently we have two approaches to solve this problem first being the
>>> DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
>>> approach as this can give client ability to configure each transfer
>>> rather than set for the channel. Linus W, do you agree?
>> 
>> I think Sundaram is in the position of doing some heavy work on
>> using one or the other of the API:s, and I think he is better
>> suited than anyone else of us to select what scheme to use,
>> in the end he's going to write the first code using the API.
> And Unfortunately TI folks don't seem to care about this discussion :(
> Haven't seen anything on this from them, or on previous RFC by Jassi

IIRC, Sundaram went on holiday. I suspect he cares a lot about this
discussion as he has a session on the topic at LPC.

-Matt

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
@ 2011-08-23 14:43         ` Matt Porter
  0 siblings, 0 replies; 21+ messages in thread
From: Matt Porter @ 2011-08-23 14:43 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Linus Walleij, Jassi Brar, sundaram, dan.j.williams,
	linux-kernel, linux-omap, rmk+kernel, nsekhar

On Aug 19, 2011, at 9:43 AM, Koul, Vinod wrote:

> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> 
>>> Currently we have two approaches to solve this problem first being the
>>> DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
>>> approach as this can give client ability to configure each transfer
>>> rather than set for the channel. Linus W, do you agree?
>> 
>> I think Sundaram is in the position of doing some heavy work on
>> using one or the other of the API:s, and I think he is better
>> suited than anyone else of us to select what scheme to use,
>> in the end he's going to write the first code using the API.
> And Unfortunately TI folks don't seem to care about this discussion :(
> Haven't seen anything on this from them, or on previous RFC by Jassi

IIRC, Sundaram went on holiday. I suspect he cares a lot about this
discussion as he has a session on the topic at LPC.

-Matt

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-19 17:28           ` Koul, Vinod
@ 2011-08-19 18:45             ` Jassi Brar
  0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2011-08-19 18:45 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Linus Walleij, sundaram, Williams, Dan J, linux-kernel,
	linux-omap, rmk+kernel, nsekhar

On 19 August 2011 22:58, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Fri, 2011-08-19 at 21:16 +0530, Jassi Brar wrote:
>> On 19 August 2011 19:49, Linus Walleij <linus.ml.walleij@gmail.com> wrote:
>> > 2011/8/19 Koul, Vinod <vinod.koul@intel.com>:
>> >> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
>> >>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>> >
>> >>> I think Sundaram is in the position of doing some heavy work on
>> >>> using one or the other of the API:s, and I think he is better
>> >>> suited than anyone else of us to select what scheme to use,
>> >>> in the end he's going to write the first code using the API.
>> >
>> >> And Unfortunately TI folks don't seem to care about this discussion :(
>> >> Haven't seen anything on this from them, or on previous RFC by Jassi
>> >
>> > Well if there is no code usig the API then there is no rush
>> > in merging it either I guess. Whenever someone (TI or
>> > Samsung) cook some driver patches they can choose their
>> > approach.
>> >
>> No, it's not a matter of "choice".
>> If that were the case, Sundaram already proposed a TI specific
>> flag. Why wait for him to tell his choice again?
>>
>> You might, but I can't molest my sensibility to believe that a Vendor
>> specific flag could be better than a generic solution.
>> Not here at least, where the overhead due to generality is not much.
>> (though I can trim some 'futuristic' members from the 'struct xfer_template')
> Who said anything about adding a vendor flag solution,
Not you, but to whom I replied - LinusW   See https://lkml.org/lkml/2011/7/11/74

> since TI are potential users of the API it would good to know i this fits there needs
> are not.
I am super-interested to hear from TI guys.
The generic api here rather supports the case Sundaram projected as
'most' general case. Look at the figure at end of
https://lkml.org/lkml/2011/6/9/343

>> Maintainers might wait as long as they want, but there should never
>> be an option to have vendor specific hacks.
> to me API looks decent after reading some specs of DMACs which support
> this mode. Pls send updated patch along with one driver which uses it.
> Should be good to go...
That has been one problem with DMAEngine. People patch the API as they need
stuff, rather than having had solid thought-out API that could be
extended consistently.
For ex, we ended up having _ten_ device_prep_dma_* callbacks, where as
we could have done having had originally 1 (or maybe 2)  'generic'
prepare callback with a 'enum dma_transaction_type op' argument.
IMO it's rather good that I designed this API for a theoretical
highly-capable DMAC,
and not just the DMAC I've worked on - which would have constrained
the api in future
for other DMACs.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-19 15:46         ` Jassi Brar
@ 2011-08-19 17:28           ` Koul, Vinod
  2011-08-19 18:45             ` Jassi Brar
  0 siblings, 1 reply; 21+ messages in thread
From: Koul, Vinod @ 2011-08-19 17:28 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Linus Walleij, sundaram, Williams, Dan J, linux-kernel,
	linux-omap, rmk+kernel, nsekhar

On Fri, 2011-08-19 at 21:16 +0530, Jassi Brar wrote:
> On 19 August 2011 19:49, Linus Walleij <linus.ml.walleij@gmail.com> wrote:
> > 2011/8/19 Koul, Vinod <vinod.koul@intel.com>:
> >> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
> >>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> >
> >>> I think Sundaram is in the position of doing some heavy work on
> >>> using one or the other of the API:s, and I think he is better
> >>> suited than anyone else of us to select what scheme to use,
> >>> in the end he's going to write the first code using the API.
> >
> >> And Unfortunately TI folks don't seem to care about this discussion :(
> >> Haven't seen anything on this from them, or on previous RFC by Jassi
> >
> > Well if there is no code usig the API then there is no rush
> > in merging it either I guess. Whenever someone (TI or
> > Samsung) cook some driver patches they can choose their
> > approach.
> >
> No, it's not a matter of "choice".
> If that were the case, Sundaram already proposed a TI specific
> flag. Why wait for him to tell his choice again?
> 
> You might, but I can't molest my sensibility to believe that a Vendor
> specific flag could be better than a generic solution.
> Not here at least, where the overhead due to generality is not much.
> (though I can trim some 'futuristic' members from the 'struct xfer_template')
Who said anything about adding a vendor flag solution, since TI are
potential users of the API it would good to know i this fits there needs
are not. If they don't care, we can't help it...

> 
> Maintainers might wait as long as they want, but there should never
> be an option to have vendor specific hacks.
to me API looks decent after reading some specs of DMACs which support
this mode. Pls send updated patch along with one driver which uses it.
Should be good to go...


-- 
~Vinod


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-19 14:19       ` Linus Walleij
@ 2011-08-19 15:46         ` Jassi Brar
  2011-08-19 17:28           ` Koul, Vinod
  0 siblings, 1 reply; 21+ messages in thread
From: Jassi Brar @ 2011-08-19 15:46 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Koul, Vinod, sundaram, dan.j.williams, linux-kernel, linux-omap,
	rmk+kernel, nsekhar

On 19 August 2011 19:49, Linus Walleij <linus.ml.walleij@gmail.com> wrote:
> 2011/8/19 Koul, Vinod <vinod.koul@intel.com>:
>> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
>>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
>
>>> I think Sundaram is in the position of doing some heavy work on
>>> using one or the other of the API:s, and I think he is better
>>> suited than anyone else of us to select what scheme to use,
>>> in the end he's going to write the first code using the API.
>
>> And Unfortunately TI folks don't seem to care about this discussion :(
>> Haven't seen anything on this from them, or on previous RFC by Jassi
>
> Well if there is no code usig the API then there is no rush
> in merging it either I guess. Whenever someone (TI or
> Samsung) cook some driver patches they can choose their
> approach.
>
No, it's not a matter of "choice".
If that were the case, Sundaram already proposed a TI specific
flag. Why wait for him to tell his choice again?

You might, but I can't molest my sensibility to believe that a Vendor
specific flag could be better than a generic solution.
Not here at least, where the overhead due to generality is not much.
(though I can trim some 'futuristic' members from the 'struct xfer_template')

Maintainers might wait as long as they want, but there should never
be an option to have vendor specific hacks.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-19 13:43     ` Koul, Vinod
@ 2011-08-19 14:19       ` Linus Walleij
  2011-08-19 15:46         ` Jassi Brar
  2011-08-23 14:43         ` Matt Porter
  1 sibling, 1 reply; 21+ messages in thread
From: Linus Walleij @ 2011-08-19 14:19 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Jassi Brar, sundaram, dan.j.williams, linux-kernel, linux-omap,
	rmk+kernel, nsekhar

2011/8/19 Koul, Vinod <vinod.koul@intel.com>:
> On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
>> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:

>> I think Sundaram is in the position of doing some heavy work on
>> using one or the other of the API:s, and I think he is better
>> suited than anyone else of us to select what scheme to use,
>> in the end he's going to write the first code using the API.

> And Unfortunately TI folks don't seem to care about this discussion :(
> Haven't seen anything on this from them, or on previous RFC by Jassi

Well if there is no code usig the API then there is no rush
in merging it either I guess. Whenever someone (TI or
Samsung) cook some driver patches they can choose their
approach.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-16 13:06   ` Linus Walleij
@ 2011-08-19 13:43     ` Koul, Vinod
  2011-08-19 14:19       ` Linus Walleij
  2011-08-23 14:43         ` Matt Porter
  0 siblings, 2 replies; 21+ messages in thread
From: Koul, Vinod @ 2011-08-19 13:43 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jassi Brar, sundaram, dan.j.williams, linux-kernel, linux-omap,
	rmk+kernel, nsekhar

On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
> On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:
> 
> > Currently we have two approaches to solve this problem first being the
> > DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
> > approach as this can give client ability to configure each transfer
> > rather than set for the channel. Linus W, do you agree?
> 
> I think Sundaram is in the position of doing some heavy work on
> using one or the other of the API:s, and I think he is better
> suited than anyone else of us to select what scheme to use,
> in the end he's going to write the first code using the API.
And Unfortunately TI folks don't seem to care about this discussion :(
Haven't seen anything on this from them, or on previous RFC by Jassi

-- 
~Vinod


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-16 12:56 ` Koul, Vinod
  2011-08-16 13:06   ` Linus Walleij
@ 2011-08-16 14:32   ` Jassi Brar
  1 sibling, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2011-08-16 14:32 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: sundaram, dan.j.williams, linux-kernel, linus.walleij,
	linux-omap, rmk+kernel, nsekhar

On 16 August 2011 18:26, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Fri, 2011-08-12 at 16:44 +0530, Jassi Brar wrote:
>> Define a new api that could be used for doing fancy data transfers
>> like interleaved to contiguous copy and vice-versa.
>> Traditional SG_list based transfers tend to be very inefficient in
>> such cases as where the interleave and chunk are only a few bytes,
>> which call for a very condensed api to convey pattern of the transfer.
>>
>> This api supports all 4 variants of scatter-gather and contiguous transfer.
>> Besides, it could also represent common operations like
>>         device_prep_dma_{cyclic, memset, memcpy}
>> and maybe some more that I am not sure of.
>>
>> Of course, neither can this api help transfers that don't lend to DMA by
>> nature, i.e, scattered tiny read/writes with no periodic pattern.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>> ---
>>  include/linux/dmaengine.h |   73 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 73 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index 8fbf40e..74f3ae0 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -76,6 +76,76 @@ enum dma_transaction_type {
>>  /* last transaction type for creation of the capabilities mask */
>>  #define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
>>
>> +/**
>> + * Generic Transfer Request
>> + * ------------------------
>> + * A chunk is collection of contiguous bytes to be transfered.
>> + * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
>> + * ICGs may or maynot change between chunks.
>> + * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
>> + *  that when repeated an integral number of times, specifies the transfer.
>> + * A transfer template is specification of a Frame, the number of times
>> + *  it is to be repeated and other per-transfer attributes.
>> + *
>> + * Practically, a client driver would have ready a template for each
>> + *  type of transfer it is going to need during its lifetime and
>> + *  set only 'src_start' and 'dst_start' before submitting the requests.
>> + *
>> + *
>> + *  |      Frame-1        |       Frame-2       | ~ |       Frame-'numf'  |
>> + *  |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
>> + *
>> + *    ==  Chunk size
>> + *    ... ICG
>> + */
>> +
>> +/**
>> + * struct data_chunk - Element of scatter-gather list that makes a frame.
>> + * @size: Number of bytes to read from source.
>> + *     size_dst := fn(op, size_src), so doesn't mean much for destination.
>> + * @icg: Number of bytes to jump after last src/dst address of this
>> + *    chunk and before first src/dst address for next chunk.
>> + *    Ignored for dst(assumed 0), if dst_inc is true and dst_sgl is false.
>> + *    Ignored for src(assumed 0), if src_inc is true and src_sgl is false.
>> + */
>> +struct data_chunk {
>> +     size_t size;
>> +     size_t icg;
>> +};
>> +
>> +/**
>> + * struct xfer_template - Template to convey DMAC the transfer pattern
>> + *    and attributes.
>> + * @op: The operation to perform on source data before writing it on
>> + *    to destination address.
>> + * @src_start: Bus address of source for the first chunk.
>> + * @dst_start: Bus address of destination for the first chunk.
>> + * @src_inc: If the source address increments after reading from it.
>> + * @dst_inc: If the destination address increments after writing to it.
>> + * @src_sgl: If the 'icg' of sgl[] applies to Source (scattered read).
>> + *           Otherwise, source is read contiguously (icg ignored).
>> + *           Ignored if src_inc is false.
>> + * @dst_sgl: If the 'icg' of sgl[] applies to Destination (scattered write).
>> + *           Otherwise, destination is filled contiguously (icg ignored).
>> + *           Ignored if dst_inc is false.
>> + * @frm_irq: If the client expects DMAC driver to do callback after each frame.
>> + * @numf: Number of frames in this template.
>> + * @frame_size: Number of chunks in a frame i.e, size of sgl[].
>> + * @sgl: Array of {chunk,icg} pairs that make up a frame.
>> + */
>> +struct xfer_template {
>> +     enum dma_transaction_type op;
>> +     dma_addr_t src_start;
>> +     dma_addr_t dst_start;
>> +     bool src_inc;
>> +     bool dst_inc;
>> +     bool src_sgl;
>> +     bool dst_sgl;
>> +     bool frm_irq;
>> +     size_t numf;
>> +     size_t frame_size;
>> +     struct data_chunk sgl[0];
>> +};
>>
>>  /**
>>   * enum dma_ctrl_flags - DMA flags to augment operation preparation,
>> @@ -432,6 +502,7 @@ struct dma_tx_state {
>>   * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
>>   *   The function takes a buffer of size buf_len. The callback function will
>>   *   be called after period_len bytes have been transferred.
>> + * @device_prep_dma_genxfer: Transfer expression in a generic way.
>>   * @device_control: manipulate all pending operations on a channel, returns
>>   *   zero or error code
>>   * @device_tx_status: poll for transaction completion, the optional
>> @@ -496,6 +567,8 @@ struct dma_device {
>>       struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
>>               struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>>               size_t period_len, enum dma_data_direction direction);
>> +     struct dma_async_tx_descriptor *(*device_prep_dma_genxfer)(
>> +             struct dma_chan *chan, struct xfer_template *xt);
>>       int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>>               unsigned long arg);
>>
> Do you have a driver which is using the API, would help to evaluate this
> API when we see the usage :)
Sorry, not in public and not for DMAENGINE.
It will help if you see it as a generic[1] solution to the requirement of
supporting interleaved transfers with DMACs like TI's EDMA, SDMA,
ARM's PL330 and many more in future.

> Currently we have two approaches to solve this problem first being the
> DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
> approach as this can give client ability to configure each transfer
> rather than set for the channel.
To me, doing it via a control command vs new transfer type, is secondary
to doing it in a generic way. And yes, I too believe this way is better because
it is more flexible.


[1] Btw, I haven't yet added option to specify byte/half-word/word swap
while data transfer - which I once had to do with PL330 in a weird setup.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-16 12:56 ` Koul, Vinod
@ 2011-08-16 13:06   ` Linus Walleij
  2011-08-19 13:43     ` Koul, Vinod
  2011-08-16 14:32   ` Jassi Brar
  1 sibling, 1 reply; 21+ messages in thread
From: Linus Walleij @ 2011-08-16 13:06 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Jassi Brar, sundaram, dan.j.williams, linux-kernel, linux-omap,
	rmk+kernel, nsekhar

On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod <vinod.koul@intel.com> wrote:

> Currently we have two approaches to solve this problem first being the
> DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
> approach as this can give client ability to configure each transfer
> rather than set for the channel. Linus W, do you agree?

I think Sundaram is in the position of doing some heavy work on
using one or the other of the API:s, and I think he is better
suited than anyone else of us to select what scheme to use,
in the end he's going to write the first code using the API.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] DMAEngine: Define generic transfer request api
  2011-08-12 11:14 Jassi Brar
@ 2011-08-16 12:56 ` Koul, Vinod
  2011-08-16 13:06   ` Linus Walleij
  2011-08-16 14:32   ` Jassi Brar
  0 siblings, 2 replies; 21+ messages in thread
From: Koul, Vinod @ 2011-08-16 12:56 UTC (permalink / raw)
  To: Jassi Brar, sundaram
  Cc: dan.j.williams, linux-kernel, linus.walleij, linux-omap,
	rmk+kernel, nsekhar

On Fri, 2011-08-12 at 16:44 +0530, Jassi Brar wrote:
> Define a new api that could be used for doing fancy data transfers
> like interleaved to contiguous copy and vice-versa.
> Traditional SG_list based transfers tend to be very inefficient in
> such cases as where the interleave and chunk are only a few bytes,
> which call for a very condensed api to convey pattern of the transfer.
> 
> This api supports all 4 variants of scatter-gather and contiguous transfer.
> Besides, it could also represent common operations like
>         device_prep_dma_{cyclic, memset, memcpy}
> and maybe some more that I am not sure of.
> 
> Of course, neither can this api help transfers that don't lend to DMA by
> nature, i.e, scattered tiny read/writes with no periodic pattern.
> 
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  include/linux/dmaengine.h |   73 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 8fbf40e..74f3ae0 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -76,6 +76,76 @@ enum dma_transaction_type {
>  /* last transaction type for creation of the capabilities mask */
>  #define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
>  
> +/**
> + * Generic Transfer Request
> + * ------------------------
> + * A chunk is collection of contiguous bytes to be transfered.
> + * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
> + * ICGs may or maynot change between chunks.
> + * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
> + *  that when repeated an integral number of times, specifies the transfer.
> + * A transfer template is specification of a Frame, the number of times
> + *  it is to be repeated and other per-transfer attributes.
> + *
> + * Practically, a client driver would have ready a template for each
> + *  type of transfer it is going to need during its lifetime and
> + *  set only 'src_start' and 'dst_start' before submitting the requests.
> + *
> + *
> + *  |      Frame-1        |       Frame-2       | ~ |       Frame-'numf'  |
> + *  |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
> + *
> + *    ==  Chunk size
> + *    ... ICG
> + */
> +
> +/**
> + * struct data_chunk - Element of scatter-gather list that makes a frame.
> + * @size: Number of bytes to read from source.
> + *	  size_dst := fn(op, size_src), so doesn't mean much for destination.
> + * @icg: Number of bytes to jump after last src/dst address of this
> + *	 chunk and before first src/dst address for next chunk.
> + *	 Ignored for dst(assumed 0), if dst_inc is true and dst_sgl is false.
> + *	 Ignored for src(assumed 0), if src_inc is true and src_sgl is false.
> + */
> +struct data_chunk {
> +	size_t size;
> +	size_t icg;
> +};
> +
> +/**
> + * struct xfer_template - Template to convey DMAC the transfer pattern
> + *	 and attributes.
> + * @op: The operation to perform on source data before writing it on
> + *	 to destination address.
> + * @src_start: Bus address of source for the first chunk.
> + * @dst_start: Bus address of destination for the first chunk.
> + * @src_inc: If the source address increments after reading from it.
> + * @dst_inc: If the destination address increments after writing to it.
> + * @src_sgl: If the 'icg' of sgl[] applies to Source (scattered read).
> + *		Otherwise, source is read contiguously (icg ignored).
> + *		Ignored if src_inc is false.
> + * @dst_sgl: If the 'icg' of sgl[] applies to Destination (scattered write).
> + *		Otherwise, destination is filled contiguously (icg ignored).
> + *		Ignored if dst_inc is false.
> + * @frm_irq: If the client expects DMAC driver to do callback after each frame.
> + * @numf: Number of frames in this template.
> + * @frame_size: Number of chunks in a frame i.e, size of sgl[].
> + * @sgl: Array of {chunk,icg} pairs that make up a frame.
> + */
> +struct xfer_template {
> +	enum dma_transaction_type op;
> +	dma_addr_t src_start;
> +	dma_addr_t dst_start;
> +	bool src_inc;
> +	bool dst_inc;
> +	bool src_sgl;
> +	bool dst_sgl;
> +	bool frm_irq;
> +	size_t numf;
> +	size_t frame_size;
> +	struct data_chunk sgl[0];
> +};
>  
>  /**
>   * enum dma_ctrl_flags - DMA flags to augment operation preparation,
> @@ -432,6 +502,7 @@ struct dma_tx_state {
>   * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
>   *	The function takes a buffer of size buf_len. The callback function will
>   *	be called after period_len bytes have been transferred.
> + * @device_prep_dma_genxfer: Transfer expression in a generic way.
>   * @device_control: manipulate all pending operations on a channel, returns
>   *	zero or error code
>   * @device_tx_status: poll for transaction completion, the optional
> @@ -496,6 +567,8 @@ struct dma_device {
>  	struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
>  		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
>  		size_t period_len, enum dma_data_direction direction);
> +	struct dma_async_tx_descriptor *(*device_prep_dma_genxfer)(
> +		struct dma_chan *chan, struct xfer_template *xt);
>  	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  		unsigned long arg);
>  
Do you have a driver which is using the API, would help to evaluate this
API when we see the usage :)

Currently we have two approaches to solve this problem first being the
DMA_STRIDE_CONFIG proposed by Linus W, I feel this one is better
approach as this can give client ability to configure each transfer
rather than set for the channel. Linus W, do you agree?

Sundaram, Does this fit to the usage you folks wanted?


-- 
~Vinod


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] DMAEngine: Define generic transfer request api
@ 2011-08-12 11:14 Jassi Brar
  2011-08-16 12:56 ` Koul, Vinod
  0 siblings, 1 reply; 21+ messages in thread
From: Jassi Brar @ 2011-08-12 11:14 UTC (permalink / raw)
  To: dan.j.williams, vkoul, linux-kernel
  Cc: sundaram, linus.walleij, linux-omap, rmk+kernel, nsekhar, Jassi Brar

Define a new api that could be used for doing fancy data transfers
like interleaved to contiguous copy and vice-versa.
Traditional SG_list based transfers tend to be very inefficient in
such cases as where the interleave and chunk are only a few bytes,
which call for a very condensed api to convey pattern of the transfer.

This api supports all 4 variants of scatter-gather and contiguous transfer.
Besides, it could also represent common operations like
        device_prep_dma_{cyclic, memset, memcpy}
and maybe some more that I am not sure of.

Of course, neither can this api help transfers that don't lend to DMA by
nature, i.e, scattered tiny read/writes with no periodic pattern.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 include/linux/dmaengine.h |   73 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 8fbf40e..74f3ae0 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -76,6 +76,76 @@ enum dma_transaction_type {
 /* last transaction type for creation of the capabilities mask */
 #define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
 
+/**
+ * Generic Transfer Request
+ * ------------------------
+ * A chunk is collection of contiguous bytes to be transfered.
+ * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
+ * ICGs may or maynot change between chunks.
+ * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
+ *  that when repeated an integral number of times, specifies the transfer.
+ * A transfer template is specification of a Frame, the number of times
+ *  it is to be repeated and other per-transfer attributes.
+ *
+ * Practically, a client driver would have ready a template for each
+ *  type of transfer it is going to need during its lifetime and
+ *  set only 'src_start' and 'dst_start' before submitting the requests.
+ *
+ *
+ *  |      Frame-1        |       Frame-2       | ~ |       Frame-'numf'  |
+ *  |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
+ *
+ *    ==  Chunk size
+ *    ... ICG
+ */
+
+/**
+ * struct data_chunk - Element of scatter-gather list that makes a frame.
+ * @size: Number of bytes to read from source.
+ *	  size_dst := fn(op, size_src), so doesn't mean much for destination.
+ * @icg: Number of bytes to jump after last src/dst address of this
+ *	 chunk and before first src/dst address for next chunk.
+ *	 Ignored for dst(assumed 0), if dst_inc is true and dst_sgl is false.
+ *	 Ignored for src(assumed 0), if src_inc is true and src_sgl is false.
+ */
+struct data_chunk {
+	size_t size;
+	size_t icg;
+};
+
+/**
+ * struct xfer_template - Template to convey DMAC the transfer pattern
+ *	 and attributes.
+ * @op: The operation to perform on source data before writing it on
+ *	 to destination address.
+ * @src_start: Bus address of source for the first chunk.
+ * @dst_start: Bus address of destination for the first chunk.
+ * @src_inc: If the source address increments after reading from it.
+ * @dst_inc: If the destination address increments after writing to it.
+ * @src_sgl: If the 'icg' of sgl[] applies to Source (scattered read).
+ *		Otherwise, source is read contiguously (icg ignored).
+ *		Ignored if src_inc is false.
+ * @dst_sgl: If the 'icg' of sgl[] applies to Destination (scattered write).
+ *		Otherwise, destination is filled contiguously (icg ignored).
+ *		Ignored if dst_inc is false.
+ * @frm_irq: If the client expects DMAC driver to do callback after each frame.
+ * @numf: Number of frames in this template.
+ * @frame_size: Number of chunks in a frame i.e, size of sgl[].
+ * @sgl: Array of {chunk,icg} pairs that make up a frame.
+ */
+struct xfer_template {
+	enum dma_transaction_type op;
+	dma_addr_t src_start;
+	dma_addr_t dst_start;
+	bool src_inc;
+	bool dst_inc;
+	bool src_sgl;
+	bool dst_sgl;
+	bool frm_irq;
+	size_t numf;
+	size_t frame_size;
+	struct data_chunk sgl[0];
+};
 
 /**
  * enum dma_ctrl_flags - DMA flags to augment operation preparation,
@@ -432,6 +502,7 @@ struct dma_tx_state {
  * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
  *	The function takes a buffer of size buf_len. The callback function will
  *	be called after period_len bytes have been transferred.
+ * @device_prep_dma_genxfer: Transfer expression in a generic way.
  * @device_control: manipulate all pending operations on a channel, returns
  *	zero or error code
  * @device_tx_status: poll for transaction completion, the optional
@@ -496,6 +567,8 @@ struct dma_device {
 	struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
 		struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
 		size_t period_len, enum dma_data_direction direction);
+	struct dma_async_tx_descriptor *(*device_prep_dma_genxfer)(
+		struct dma_chan *chan, struct xfer_template *xt);
 	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 		unsigned long arg);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2011-09-15  8:18 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAGsJ_4wXURUwbf-fcNOq1m5-NJ9+VMuDq+9OJpBjFZK4C_X3cw@mail.gmail.com>
2011-09-12 16:26 ` [PATCH] DMAEngine: Define generic transfer request api Barry Song
2011-09-12 16:54   ` Jassi Brar
2011-09-13  1:21     ` Barry Song
2011-09-13  7:46       ` Barry Song
2011-09-13  8:43         ` Jassi Brar
2011-09-13  8:58           ` Barry Song
2011-09-15  6:31             ` Barry Song
2011-09-15  6:43               ` Jassi Brar
2011-09-15  6:49                 ` Barry Song
2011-09-15  8:17                 ` Barry Song
2011-08-12 11:14 Jassi Brar
2011-08-16 12:56 ` Koul, Vinod
2011-08-16 13:06   ` Linus Walleij
2011-08-19 13:43     ` Koul, Vinod
2011-08-19 14:19       ` Linus Walleij
2011-08-19 15:46         ` Jassi Brar
2011-08-19 17:28           ` Koul, Vinod
2011-08-19 18:45             ` Jassi Brar
2011-08-23 14:43       ` Matt Porter
2011-08-23 14:43         ` Matt Porter
2011-08-16 14:32   ` Jassi Brar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.