All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-10 15:03 ` Sundaram Raju
  0 siblings, 0 replies; 35+ messages in thread
From: Sundaram Raju @ 2011-07-10 15:03 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
	linux-lFZ/pmaqli7XmaaqVzeoHQ,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
controller specific configurations on how a buffer must be walked
through and how data is picked for transfer based on a specified
pattern over the channel.

The configuration passed is specific to the TI DMA controller used.

Signed-off-by: Sundaram Raju <sundaram-l0cyMroinI0@public.gmane.org>
---
 include/linux/dmaengine.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index eee7add..51dadc4 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -123,6 +123,10 @@ enum dma_ctrl_flags {
  * command.
  * @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
  * into external start mode.
+ * @TI_DMA_STRIDE_CONFIG: this command is only implemented by TI DMA
+ * controllers that need to pass special configuration on how to walk through
+ * the buffer to pick up data in a specified pattern to be transferred in
+ * the channel.
  */
 enum dma_ctrl_cmd {
 	DMA_TERMINATE_ALL,
@@ -130,6 +134,7 @@ enum dma_ctrl_cmd {
 	DMA_RESUME,
 	DMA_SLAVE_CONFIG,
 	FSLDMA_EXTERNAL_START,
+	TI_DMA_STRIDE_CONFIG,
 };
 
 /**
-- 
1.6.2.4

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-10 15:03 ` Sundaram Raju
  0 siblings, 0 replies; 35+ messages in thread
From: Sundaram Raju @ 2011-07-10 15:03 UTC (permalink / raw)
  To: linux-arm-kernel

Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
controller specific configurations on how a buffer must be walked
through and how data is picked for transfer based on a specified
pattern over the channel.

The configuration passed is specific to the TI DMA controller used.

Signed-off-by: Sundaram Raju <sundaram@ti.com>
---
 include/linux/dmaengine.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index eee7add..51dadc4 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -123,6 +123,10 @@ enum dma_ctrl_flags {
  * command.
  * @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
  * into external start mode.
+ * @TI_DMA_STRIDE_CONFIG: this command is only implemented by TI DMA
+ * controllers that need to pass special configuration on how to walk through
+ * the buffer to pick up data in a specified pattern to be transferred in
+ * the channel.
  */
 enum dma_ctrl_cmd {
 	DMA_TERMINATE_ALL,
@@ -130,6 +134,7 @@ enum dma_ctrl_cmd {
 	DMA_RESUME,
 	DMA_SLAVE_CONFIG,
 	FSLDMA_EXTERNAL_START,
+	TI_DMA_STRIDE_CONFIG,
 };
 
 /**
-- 
1.6.2.4

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-10 15:03 ` Sundaram Raju
@ 2011-07-11  9:28   ` Linus Walleij
  -1 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-11  9:28 UTC (permalink / raw)
  To: Sundaram Raju
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	dan.j.williams, linux-omap

2011/7/10 Sundaram Raju <sundaram@ti.com>:

> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
> controller specific configurations on how a buffer must be walked
> through and how data is picked for transfer based on a specified
> pattern over the channel.
>
> The configuration passed is specific to the TI DMA controller used.
>
> Signed-off-by: Sundaram Raju <sundaram@ti.com>

This is exactly how I think we should do this.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-11  9:28   ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-11  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

2011/7/10 Sundaram Raju <sundaram@ti.com>:

> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
> controller specific configurations on how a buffer must be walked
> through and how data is picked for transfer based on a specified
> pattern over the channel.
>
> The configuration passed is specific to the TI DMA controller used.
>
> Signed-off-by: Sundaram Raju <sundaram@ti.com>

This is exactly how I think we should do this.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-11  9:28   ` Linus Walleij
@ 2011-07-11 21:39     ` Dan Williams
  -1 siblings, 0 replies; 35+ messages in thread
From: Dan Williams @ 2011-07-11 21:39 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sundaram Raju, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, linux-omap

On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/7/10 Sundaram Raju <sundaram@ti.com>:
>
>> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
>> controller specific configurations on how a buffer must be walked
>> through and how data is picked for transfer based on a specified
>> pattern over the channel.
>>
>> The configuration passed is specific to the TI DMA controller used.

...and I suspect the slave device drivers that use TI DMA are not
expected to ever work with other dmaengines?  Likely the case, but
just wondering out loud.

>> Signed-off-by: Sundaram Raju <sundaram@ti.com>
>
> This is exactly how I think we should do this.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-11 21:39     ` Dan Williams
  0 siblings, 0 replies; 35+ messages in thread
From: Dan Williams @ 2011-07-11 21:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 2011/7/10 Sundaram Raju <sundaram@ti.com>:
>
>> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
>> controller specific configurations on how a buffer must be walked
>> through and how data is picked for transfer based on a specified
>> pattern over the channel.
>>
>> The configuration passed is specific to the TI DMA controller used.

...and I suspect the slave device drivers that use TI DMA are not
expected to ever work with other dmaengines?  Likely the case, but
just wondering out loud.

>> Signed-off-by: Sundaram Raju <sundaram@ti.com>
>
> This is exactly how I think we should do this.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-10 15:03 ` Sundaram Raju
@ 2011-07-12  4:17   ` Jassi Brar
  -1 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12  4:17 UTC (permalink / raw)
  To: Sundaram Raju
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	linus.walleij, dan.j.williams, linux-omap

On Sun, Jul 10, 2011 at 8:33 PM, Sundaram Raju <sundaram@ti.com> wrote:
> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
> controller specific configurations on how a buffer must be walked
> through and how data is picked for transfer based on a specified
> pattern over the channel.
>
> The configuration passed is specific to the TI DMA controller used.
>
> Signed-off-by: Sundaram Raju <sundaram@ti.com>
> ---
>  include/linux/dmaengine.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index eee7add..51dadc4 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -123,6 +123,10 @@ enum dma_ctrl_flags {
>  * command.
>  * @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
>  * into external start mode.
> + * @TI_DMA_STRIDE_CONFIG: this command is only implemented by TI DMA
> + * controllers that need to pass special configuration on how to walk through
> + * the buffer to pick up data in a specified pattern to be transferred in
> + * the channel.
>  */
>  enum dma_ctrl_cmd {
>        DMA_TERMINATE_ALL,
> @@ -130,6 +134,7 @@ enum dma_ctrl_cmd {
>        DMA_RESUME,
>        DMA_SLAVE_CONFIG,
>        FSLDMA_EXTERNAL_START,
> +       TI_DMA_STRIDE_CONFIG,
>  };
IMHO this isn't very correct.

1) Striding, in one form or other, is supported by other DMACs as well.
   The number will only increase in future.
   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?

2) As Dan noted, client drivers are going to have ifdef hackery in
order to be common
 to other SoCs.

3) TI may not have just one DMAC IP used in all the SoCs. So if you want
  vendor specific defines anyway, please atleast also add DMAC version to it.
  Something like
>        DMA_SLAVE_CONFIG,
>        FSLDMA_EXTERNAL_START,
> +       TI_DMA_v1_STRIDE_CONFIG,

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12  4:17   ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12  4:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Jul 10, 2011 at 8:33 PM, Sundaram Raju <sundaram@ti.com> wrote:
> Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
> controller specific configurations on how a buffer must be walked
> through and how data is picked for transfer based on a specified
> pattern over the channel.
>
> The configuration passed is specific to the TI DMA controller used.
>
> Signed-off-by: Sundaram Raju <sundaram@ti.com>
> ---
> ?include/linux/dmaengine.h | ? ?5 +++++
> ?1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index eee7add..51dadc4 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -123,6 +123,10 @@ enum dma_ctrl_flags {
> ?* command.
> ?* @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
> ?* into external start mode.
> + * @TI_DMA_STRIDE_CONFIG: this command is only implemented by TI DMA
> + * controllers that need to pass special configuration on how to walk through
> + * the buffer to pick up data in a specified pattern to be transferred in
> + * the channel.
> ?*/
> ?enum dma_ctrl_cmd {
> ? ? ? ?DMA_TERMINATE_ALL,
> @@ -130,6 +134,7 @@ enum dma_ctrl_cmd {
> ? ? ? ?DMA_RESUME,
> ? ? ? ?DMA_SLAVE_CONFIG,
> ? ? ? ?FSLDMA_EXTERNAL_START,
> + ? ? ? TI_DMA_STRIDE_CONFIG,
> ?};
IMHO this isn't very correct.

1) Striding, in one form or other, is supported by other DMACs as well.
   The number will only increase in future.
   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?

2) As Dan noted, client drivers are going to have ifdef hackery in
order to be common
 to other SoCs.

3) TI may not have just one DMAC IP used in all the SoCs. So if you want
  vendor specific defines anyway, please atleast also add DMAC version to it.
  Something like
>        DMA_SLAVE_CONFIG,
>        FSLDMA_EXTERNAL_START,
> +       TI_DMA_v1_STRIDE_CONFIG,

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-11 21:39     ` Dan Williams
@ 2011-07-12  9:58       ` Linus Walleij
  -1 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12  9:58 UTC (permalink / raw)
  To: Dan Williams
  Cc: Sundaram Raju, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, linux-omap

On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

> ...and I suspect the slave device drivers that use TI DMA are not
> expected to ever work with other dmaengines?  Likely the case, but
> just wondering out loud.

Typically the OMAP/TI drivers are one-to-one with this specific DMA
controller, but they *can* support controllers without stride options, and
notice that striding will only be used for the display driver IIRC,
pseudo-code:

ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
                        (unsigned long) &my_stride_config);
if (ret) {
   /*
    * OK no striding on this DMA engine, fall back to something else,
    * such as creating an SGlist which emulates the striding with one
    * sglist element per stride.
    */
}

By injecting an error in the stride config path this can even be
properly tested. So it will become an optional acceleration.

Thanks,
Linus Walleij

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12  9:58       ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12  9:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

> ...and I suspect the slave device drivers that use TI DMA are not
> expected to ever work with other dmaengines? ?Likely the case, but
> just wondering out loud.

Typically the OMAP/TI drivers are one-to-one with this specific DMA
controller, but they *can* support controllers without stride options, and
notice that striding will only be used for the display driver IIRC,
pseudo-code:

ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
                        (unsigned long) &my_stride_config);
if (ret) {
   /*
    * OK no striding on this DMA engine, fall back to something else,
    * such as creating an SGlist which emulates the striding with one
    * sglist element per stride.
    */
}

By injecting an error in the stride config path this can even be
properly tested. So it will become an optional acceleration.

Thanks,
Linus Walleij

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12  4:17   ` Jassi Brar
@ 2011-07-12 10:03     ` Linus Walleij
  -1 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12 10:03 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sundaram Raju, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:

> 1) Striding, in one form or other, is supported by other DMACs as well.
>   The number will only increase in future.
>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?

If we are sure about this and striding will work in a similar way on all
then let's have the enum named DMA_STRIDE_CONFIG and move the
passed-in struct to <linux/dmaengine.h) then?

Would that be:

struct dma_stride_config {
    u32 read_bytes;
    u32 skip_bytes;
};

Or something more complex?

> 2) As Dan noted, client drivers are going to have ifdef hackery in
> order to be common
>  to other SoCs.

Don't think so, why? This is a runtime config entirely, and I just illustrated
in mail to Dan how that can be handled by falling back to a sglist I believe?

We can *maybe* even put the fallback code into dmaengine, so that an
emulated sglist in place for the DMAengine is done automatically of the
DMA controller does not support striding.

> 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
>  vendor specific defines anyway, please atleast also add DMAC version to it.
>  Something like
>>        DMA_SLAVE_CONFIG,
>>        FSLDMA_EXTERNAL_START,
>> +       TI_DMA_v1_STRIDE_CONFIG,

Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
a lot of sense.

Linus Walleij

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 10:03     ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12 10:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:

> 1) Striding, in one form or other, is supported by other DMACs as well.
> ? The number will only increase in future.
> ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?

If we are sure about this and striding will work in a similar way on all
then let's have the enum named DMA_STRIDE_CONFIG and move the
passed-in struct to <linux/dmaengine.h) then?

Would that be:

struct dma_stride_config {
    u32 read_bytes;
    u32 skip_bytes;
};

Or something more complex?

> 2) As Dan noted, client drivers are going to have ifdef hackery in
> order to be common
> ?to other SoCs.

Don't think so, why? This is a runtime config entirely, and I just illustrated
in mail to Dan how that can be handled by falling back to a sglist I believe?

We can *maybe* even put the fallback code into dmaengine, so that an
emulated sglist in place for the DMAengine is done automatically of the
DMA controller does not support striding.

> 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
> ?vendor specific defines anyway, please atleast also add DMAC version to it.
> ?Something like
>> ? ? ? ?DMA_SLAVE_CONFIG,
>> ? ? ? ?FSLDMA_EXTERNAL_START,
>> + ? ? ? TI_DMA_v1_STRIDE_CONFIG,

Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
a lot of sense.

Linus Walleij

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12  9:58       ` Linus Walleij
  (?)
@ 2011-07-12 10:15         ` Raju, Sundaram
  -1 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:15 UTC (permalink / raw)
  To: Linus Walleij, Dan Williams
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	linux-omap

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: Tuesday, July 12, 2011 3:28 PM
> To: Dan Williams
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams <dan.j.williams@intel.com>
> wrote:
> > On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> 
> > ...and I suspect the slave device drivers that use TI DMA are not
> > expected to ever work with other dmaengines?  Likely the case, but
> > just wondering out loud.
> 
> Typically the OMAP/TI drivers are one-to-one with this specific DMA
> controller, but they *can* support controllers without stride options, and
> notice that striding will only be used for the display driver IIRC,
> pseudo-code:
> 
> ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
>                         (unsigned long) &my_stride_config);
> if (ret) {
>    /*
>     * OK no striding on this DMA engine, fall back to something else,
>     * such as creating an SGlist which emulates the striding with one
>     * sglist element per stride.
>     */
> }
> 
> By injecting an error in the stride config path this can even be
> properly tested. So it will become an optional acceleration.

Yes, this is exactly what I also wanted to say. :)

But if the client driver does not implement a fallback like this then that
driver will work only with TI DMA and not any other dmaengines.
(Mentioning this because, there are client drivers which are tightly 
coupled to this special configuration)

Keeping this in mind, I had started the original discussion with
the suggestion of modifying the existing prepare API and adding
an extra argument to it, which can be used to pass special configuration.
And I also wanted to generalize that configuration passed.

Anyways that design also will come down to this same path, and instead
of modifying the existing API signatures, I think this is the best way
we can go.

Regards,
Sundaram

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 10:15         ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:15 UTC (permalink / raw)
  To: Linus Walleij, Dan Williams
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	linux-omap

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: Tuesday, July 12, 2011 3:28 PM
> To: Dan Williams
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams <dan.j.williams@intel.com>
> wrote:
> > On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> 
> > ...and I suspect the slave device drivers that use TI DMA are not
> > expected to ever work with other dmaengines?  Likely the case, but
> > just wondering out loud.
> 
> Typically the OMAP/TI drivers are one-to-one with this specific DMA
> controller, but they *can* support controllers without stride options, and
> notice that striding will only be used for the display driver IIRC,
> pseudo-code:
> 
> ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
>                         (unsigned long) &my_stride_config);
> if (ret) {
>    /*
>     * OK no striding on this DMA engine, fall back to something else,
>     * such as creating an SGlist which emulates the striding with one
>     * sglist element per stride.
>     */
> }
> 
> By injecting an error in the stride config path this can even be
> properly tested. So it will become an optional acceleration.

Yes, this is exactly what I also wanted to say. :)

But if the client driver does not implement a fallback like this then that
driver will work only with TI DMA and not any other dmaengines.
(Mentioning this because, there are client drivers which are tightly 
coupled to this special configuration)

Keeping this in mind, I had started the original discussion with
the suggestion of modifying the existing prepare API and adding
an extra argument to it, which can be used to pass special configuration.
And I also wanted to generalize that configuration passed.

Anyways that design also will come down to this same path, and instead
of modifying the existing API signatures, I think this is the best way
we can go.

Regards,
Sundaram
--
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] 35+ messages in thread

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 10:15         ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:15 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Tuesday, July 12, 2011 3:28 PM
> To: Dan Williams
> Cc: Raju, Sundaram; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
> linux at arm.linux.org.uk; linux-omap at vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Mon, Jul 11, 2011 at 11:39 PM, Dan Williams <dan.j.williams@intel.com>
> wrote:
> > On Mon, Jul 11, 2011 at 2:28 AM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> 
> > ...and I suspect the slave device drivers that use TI DMA are not
> > expected to ever work with other dmaengines? ?Likely the case, but
> > just wondering out loud.
> 
> Typically the OMAP/TI drivers are one-to-one with this specific DMA
> controller, but they *can* support controllers without stride options, and
> notice that striding will only be used for the display driver IIRC,
> pseudo-code:
> 
> ret = dmaengine_device_control(chan, TI_DMA_STRIDE_CONFIG,
>                         (unsigned long) &my_stride_config);
> if (ret) {
>    /*
>     * OK no striding on this DMA engine, fall back to something else,
>     * such as creating an SGlist which emulates the striding with one
>     * sglist element per stride.
>     */
> }
> 
> By injecting an error in the stride config path this can even be
> properly tested. So it will become an optional acceleration.

Yes, this is exactly what I also wanted to say. :)

But if the client driver does not implement a fallback like this then that
driver will work only with TI DMA and not any other dmaengines.
(Mentioning this because, there are client drivers which are tightly 
coupled to this special configuration)

Keeping this in mind, I had started the original discussion with
the suggestion of modifying the existing prepare API and adding
an extra argument to it, which can be used to pass special configuration.
And I also wanted to generalize that configuration passed.

Anyways that design also will come down to this same path, and instead
of modifying the existing API signatures, I think this is the best way
we can go.

Regards,
Sundaram

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 10:03     ` Linus Walleij
  (?)
@ 2011-07-12 10:56       ` Raju, Sundaram
  -1 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:56 UTC (permalink / raw)
  To: Linus Walleij, Jassi Brar
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	dan.j.williams, linux-omap

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: Tuesday, July 12, 2011 3:33 PM
> To: Jassi Brar
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> 
> > 1) Striding, in one form or other, is supported by other DMACs as well.
> >   The number will only increase in future.
> >   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> 
> If we are sure about this and striding will work in a similar way on all
> then let's have the enum named DMA_STRIDE_CONFIG and move the
> passed-in struct to <linux/dmaengine.h) then?
> 
> Would that be:
> 
> struct dma_stride_config {
>     u32 read_bytes;
>     u32 skip_bytes;
> };
> 
> Or something more complex?
> 

When I started this discussion on stride config, I received comments like
this is too specific to TI DMAC, and there are not many DMACs which can
do this. I actually wanted to generalize the configuration passed and put
a comment on it similar to the one on top of dma_slave_config, which says

|
|/**
<snip>
| * The rationale for adding configuration information to this struct
| * is as follows: if it is likely that most DMA slave controllers in
| * the world will support the configuration option, then make it
| * generic. If not: if it is fixed so that it be sent in static from
| * the platform data, then prefer to do that. Else, if it is neither
| * fixed at runtime, nor generic enough (such as bus mastership on
| * some CPU family and whatnot) then create a custom slave config
| * struct and pass that, then make this config a member of that
| * struct, if applicable.
| */
|

If any other DMAC can do similar stride configuration,
then we can generalize it. 

Till we generalize this stride configuration I think a custom
configuration aligned between the client driver and
the offload engine driver can be used.

> > 2) As Dan noted, client drivers are going to have ifdef hackery in
> > order to be common
> >  to other SoCs.
> 
> Don't think so, why? This is a runtime config entirely, and I just illustrated
> in mail to Dan how that can be handled by falling back to a sglist I believe?
> 
> We can *maybe* even put the fallback code into dmaengine, so that an
> emulated sglist in place for the DMAengine is done automatically of the
> DMA controller does not support striding.
> 

Good Idea.

But the client might always have a better way to handle this fallback than
this suggested fallback code in dmaengine, which will be a common
implementation based on the received sg_list and the DMAC capabilities.
If this is done then preference should be provided to the client's fallback
implementation, if present.

> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
> >  vendor specific defines anyway, please atleast also add DMAC version to it.
> >  Something like
> >>        DMA_SLAVE_CONFIG,
> >>        FSLDMA_EXTERNAL_START,
> >> +       TI_DMA_v1_STRIDE_CONFIG,
> 
> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
> a lot of sense.
>

Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and 
one for SDMAC in OMAP series of SoCs.

Regards,
Sundaram

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 10:56       ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:56 UTC (permalink / raw)
  To: Linus Walleij, Jassi Brar
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	dan.j.williams, linux-omap

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: Tuesday, July 12, 2011 3:33 PM
> To: Jassi Brar
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> 
> > 1) Striding, in one form or other, is supported by other DMACs as well.
> >   The number will only increase in future.
> >   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> 
> If we are sure about this and striding will work in a similar way on all
> then let's have the enum named DMA_STRIDE_CONFIG and move the
> passed-in struct to <linux/dmaengine.h) then?
> 
> Would that be:
> 
> struct dma_stride_config {
>     u32 read_bytes;
>     u32 skip_bytes;
> };
> 
> Or something more complex?
> 

When I started this discussion on stride config, I received comments like
this is too specific to TI DMAC, and there are not many DMACs which can
do this. I actually wanted to generalize the configuration passed and put
a comment on it similar to the one on top of dma_slave_config, which says

|
|/**
<snip>
| * The rationale for adding configuration information to this struct
| * is as follows: if it is likely that most DMA slave controllers in
| * the world will support the configuration option, then make it
| * generic. If not: if it is fixed so that it be sent in static from
| * the platform data, then prefer to do that. Else, if it is neither
| * fixed at runtime, nor generic enough (such as bus mastership on
| * some CPU family and whatnot) then create a custom slave config
| * struct and pass that, then make this config a member of that
| * struct, if applicable.
| */
|

If any other DMAC can do similar stride configuration,
then we can generalize it. 

Till we generalize this stride configuration I think a custom
configuration aligned between the client driver and
the offload engine driver can be used.

> > 2) As Dan noted, client drivers are going to have ifdef hackery in
> > order to be common
> >  to other SoCs.
> 
> Don't think so, why? This is a runtime config entirely, and I just illustrated
> in mail to Dan how that can be handled by falling back to a sglist I believe?
> 
> We can *maybe* even put the fallback code into dmaengine, so that an
> emulated sglist in place for the DMAengine is done automatically of the
> DMA controller does not support striding.
> 

Good Idea.

But the client might always have a better way to handle this fallback than
this suggested fallback code in dmaengine, which will be a common
implementation based on the received sg_list and the DMAC capabilities.
If this is done then preference should be provided to the client's fallback
implementation, if present.

> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
> >  vendor specific defines anyway, please atleast also add DMAC version to it.
> >  Something like
> >>        DMA_SLAVE_CONFIG,
> >>        FSLDMA_EXTERNAL_START,
> >> +       TI_DMA_v1_STRIDE_CONFIG,
> 
> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
> a lot of sense.
>

Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and 
one for SDMAC in OMAP series of SoCs.

Regards,
Sundaram

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 10:56       ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 10:56 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij at linaro.org]
> Sent: Tuesday, July 12, 2011 3:33 PM
> To: Jassi Brar
> Cc: Raju, Sundaram; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
> linux at arm.linux.org.uk; dan.j.williams at intel.com; linux-omap at vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> 
> > 1) Striding, in one form or other, is supported by other DMACs as well.
> > ? The number will only increase in future.
> > ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?
> 
> If we are sure about this and striding will work in a similar way on all
> then let's have the enum named DMA_STRIDE_CONFIG and move the
> passed-in struct to <linux/dmaengine.h) then?
> 
> Would that be:
> 
> struct dma_stride_config {
>     u32 read_bytes;
>     u32 skip_bytes;
> };
> 
> Or something more complex?
> 

When I started this discussion on stride config, I received comments like
this is too specific to TI DMAC, and there are not many DMACs which can
do this. I actually wanted to generalize the configuration passed and put
a comment on it similar to the one on top of dma_slave_config, which says

|
|/**
<snip>
| * The rationale for adding configuration information to this struct
| * is as follows: if it is likely that most DMA slave controllers in
| * the world will support the configuration option, then make it
| * generic. If not: if it is fixed so that it be sent in static from
| * the platform data, then prefer to do that. Else, if it is neither
| * fixed at runtime, nor generic enough (such as bus mastership on
| * some CPU family and whatnot) then create a custom slave config
| * struct and pass that, then make this config a member of that
| * struct, if applicable.
| */
|

If any other DMAC can do similar stride configuration,
then we can generalize it. 

Till we generalize this stride configuration I think a custom
configuration aligned between the client driver and
the offload engine driver can be used.

> > 2) As Dan noted, client drivers are going to have ifdef hackery in
> > order to be common
> > ?to other SoCs.
> 
> Don't think so, why? This is a runtime config entirely, and I just illustrated
> in mail to Dan how that can be handled by falling back to a sglist I believe?
> 
> We can *maybe* even put the fallback code into dmaengine, so that an
> emulated sglist in place for the DMAengine is done automatically of the
> DMA controller does not support striding.
> 

Good Idea.

But the client might always have a better way to handle this fallback than
this suggested fallback code in dmaengine, which will be a common
implementation based on the received sg_list and the DMAC capabilities.
If this is done then preference should be provided to the client's fallback
implementation, if present.

> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
> > ?vendor specific defines anyway, please atleast also add DMAC version to it.
> > ?Something like
> >> ? ? ? ?DMA_SLAVE_CONFIG,
> >> ? ? ? ?FSLDMA_EXTERNAL_START,
> >> + ? ? ? TI_DMA_v1_STRIDE_CONFIG,
> 
> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
> a lot of sense.
>

Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and 
one for SDMAC in OMAP series of SoCs.

Regards,
Sundaram

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 10:56       ` Raju, Sundaram
  (?)
@ 2011-07-12 11:09         ` Linus Walleij
  -1 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12 11:09 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Jassi Brar, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 12:56 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> [Me]
>> [Jassi]
>> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
>> >  vendor specific defines anyway, please atleast also add DMAC version to it.
>> >  Something like
>> >>        DMA_SLAVE_CONFIG,
>> >>        FSLDMA_EXTERNAL_START,
>> >> +       TI_DMA_v1_STRIDE_CONFIG,
>>
>> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
>> a lot of sense.
>
> Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and
> one for SDMAC in OMAP series of SoCs.

Wait, that's two different silicon blocks right? Then you already have proof
that this spans more than one DMAC and then you can just go for a generic
DMA_STRIDE_CONFIG from day one.

That both are TI does not matter, if they are totally unrelated implementations.

Yours,
Linus Walleij

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 11:09         ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12 11:09 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Jassi Brar, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 12:56 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> [Me]
>> [Jassi]
>> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
>> >  vendor specific defines anyway, please atleast also add DMAC version to it.
>> >  Something like
>> >>        DMA_SLAVE_CONFIG,
>> >>        FSLDMA_EXTERNAL_START,
>> >> +       TI_DMA_v1_STRIDE_CONFIG,
>>
>> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
>> a lot of sense.
>
> Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and
> one for SDMAC in OMAP series of SoCs.

Wait, that's two different silicon blocks right? Then you already have proof
that this spans more than one DMAC and then you can just go for a generic
DMA_STRIDE_CONFIG from day one.

That both are TI does not matter, if they are totally unrelated implementations.

Yours,
Linus Walleij

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 11:09         ` Linus Walleij
  0 siblings, 0 replies; 35+ messages in thread
From: Linus Walleij @ 2011-07-12 11:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2011 at 12:56 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> [Me]
>> [Jassi]
>> > 3) TI may not have just one DMAC IP used in all the SoCs. So if you want
>> > ?vendor specific defines anyway, please atleast also add DMAC version to it.
>> > ?Something like
>> >> ? ? ? ?DMA_SLAVE_CONFIG,
>> >> ? ? ? ?FSLDMA_EXTERNAL_START,
>> >> + ? ? ? TI_DMA_v1_STRIDE_CONFIG,
>>
>> Yep unless we make it generic DMA_STRIDE_CONFIG simply, this makes
>> a lot of sense.
>
> Okay, I can add one cmd for the EDMAC in DaVinci series of SoCs and
> one for SDMAC in OMAP series of SoCs.

Wait, that's two different silicon blocks right? Then you already have proof
that this spans more than one DMAC and then you can just go for a generic
DMA_STRIDE_CONFIG from day one.

That both are TI does not matter, if they are totally unrelated implementations.

Yours,
Linus Walleij

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 10:03     ` Linus Walleij
@ 2011-07-12 11:20       ` Jassi Brar
  -1 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12 11:20 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Sundaram Raju, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
>> 1) Striding, in one form or other, is supported by other DMACs as well.
>>   The number will only increase in future.
>>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
>
> If we are sure about this and striding will work in a similar way on all
> then let's have the enum named DMA_STRIDE_CONFIG and move the
> passed-in struct to <linux/dmaengine.h) then?
>
> Would that be:
>
> struct dma_stride_config {
>    u32 read_bytes;
>    u32 skip_bytes;
> };
>
> Or something more complex?
Well, I am not sure if striding needs any special treatment at all.
Why not have client drivers prepare and submit sg-list.
Let the DMAC drivers interpret/parse the sg-list and program it
as strides if the h/w supports it.
If anything, we should make preparation and submission of sg-list
as efficient as possible.

>> 2) As Dan noted, client drivers are going to have ifdef hackery in
>> order to be common
>>  to other SoCs.
>
> Don't think so, why? This is a runtime config entirely, and I just illustrated
> in mail to Dan how that can be handled by falling back to a sglist I believe?
Runtime decision isn't neat either.
What if a client driver is common to 'N' SoCs each with different DMACs ?
We would need a switch construct !

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 11:20       ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12 11:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
>> 1) Striding, in one form or other, is supported by other DMACs as well.
>> ? The number will only increase in future.
>> ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?
>
> If we are sure about this and striding will work in a similar way on all
> then let's have the enum named DMA_STRIDE_CONFIG and move the
> passed-in struct to <linux/dmaengine.h) then?
>
> Would that be:
>
> struct dma_stride_config {
> ? ?u32 read_bytes;
> ? ?u32 skip_bytes;
> };
>
> Or something more complex?
Well, I am not sure if striding needs any special treatment at all.
Why not have client drivers prepare and submit sg-list.
Let the DMAC drivers interpret/parse the sg-list and program it
as strides if the h/w supports it.
If anything, we should make preparation and submission of sg-list
as efficient as possible.

>> 2) As Dan noted, client drivers are going to have ifdef hackery in
>> order to be common
>> ?to other SoCs.
>
> Don't think so, why? This is a runtime config entirely, and I just illustrated
> in mail to Dan how that can be handled by falling back to a sglist I believe?
Runtime decision isn't neat either.
What if a client driver is common to 'N' SoCs each with different DMACs ?
We would need a switch construct !

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 11:20       ` Jassi Brar
  (?)
@ 2011-07-12 11:31         ` Raju, Sundaram
  -1 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 11:31 UTC (permalink / raw)
  To: Jassi Brar, Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source, linux,
	dan.j.williams, linux-omap

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1926 bytes --]

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Tuesday, July 12, 2011 4:51 PM
> To: Linus Walleij
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> >
> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >>   The number will only increase in future.
> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >
> > If we are sure about this and striding will work in a similar way on all
> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> > passed-in struct to <linux/dmaengine.h) then?
> >
> > Would that be:
> >
> > struct dma_stride_config {
> >    u32 read_bytes;
> >    u32 skip_bytes;
> > };
> >
> > Or something more complex?
> Well, I am not sure if striding needs any special treatment at all.
> Why not have client drivers prepare and submit sg-list.
> Let the DMAC drivers interpret/parse the sg-list and program it
> as strides if the h/w supports it.
> If anything, we should make preparation and submission of sg-list
> as efficient as possible.
Jassi,

sg_lists describe only a bunch of disjoint buffers. But what if the
DMAC can skip and read the bytes within each of the buffers in
the sg_list? (like TI EDMAC and TI SDMAC)
How can that information be passed to the offload
engine driver from the client?

~Sundaram
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 11:31         ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 11:31 UTC (permalink / raw)
  To: Jassi Brar, Linus Walleij
  Cc: davinci-linux-open-source, linux, linux-kernel, dan.j.williams,
	linux-omap, linux-arm-kernel

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Tuesday, July 12, 2011 4:51 PM
> To: Linus Walleij
> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> >
> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >>   The number will only increase in future.
> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >
> > If we are sure about this and striding will work in a similar way on all
> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> > passed-in struct to <linux/dmaengine.h) then?
> >
> > Would that be:
> >
> > struct dma_stride_config {
> >    u32 read_bytes;
> >    u32 skip_bytes;
> > };
> >
> > Or something more complex?
> Well, I am not sure if striding needs any special treatment at all.
> Why not have client drivers prepare and submit sg-list.
> Let the DMAC drivers interpret/parse the sg-list and program it
> as strides if the h/w supports it.
> If anything, we should make preparation and submission of sg-list
> as efficient as possible.
Jassi,

sg_lists describe only a bunch of disjoint buffers. But what if the
DMAC can skip and read the bytes within each of the buffers in
the sg_list? (like TI EDMAC and TI SDMAC)
How can that information be passed to the offload
engine driver from the client?

~Sundaram
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 11:31         ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-12 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
> Sent: Tuesday, July 12, 2011 4:51 PM
> To: Linus Walleij
> Cc: Raju, Sundaram; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
> linux at arm.linux.org.uk; dan.j.williams at intel.com; linux-omap at vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> wrote:
> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> wrote:
> >
> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >> ? The number will only increase in future.
> >> ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >
> > If we are sure about this and striding will work in a similar way on all
> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> > passed-in struct to <linux/dmaengine.h) then?
> >
> > Would that be:
> >
> > struct dma_stride_config {
> > ? ?u32 read_bytes;
> > ? ?u32 skip_bytes;
> > };
> >
> > Or something more complex?
> Well, I am not sure if striding needs any special treatment at all.
> Why not have client drivers prepare and submit sg-list.
> Let the DMAC drivers interpret/parse the sg-list and program it
> as strides if the h/w supports it.
> If anything, we should make preparation and submission of sg-list
> as efficient as possible.
Jassi,

sg_lists describe only a bunch of disjoint buffers. But what if the
DMAC can skip and read the bytes within each of the buffers in
the sg_list? (like TI EDMAC and TI SDMAC)
How can that information be passed to the offload
engine driver from the client?

~Sundaram

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 11:31         ` Raju, Sundaram
  (?)
@ 2011-07-12 12:45           ` Jassi Brar
  -1 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12 12:45 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
>> Sent: Tuesday, July 12, 2011 4:51 PM
>> To: Linus Walleij
>> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
>> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
>> configuration
>>
>> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
>> wrote:
>> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
>> wrote:
>> >
>> >> 1) Striding, in one form or other, is supported by other DMACs as well.
>> >>   The number will only increase in future.
>> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
>> >
>> > If we are sure about this and striding will work in a similar way on all
>> > then let's have the enum named DMA_STRIDE_CONFIG and move the
>> > passed-in struct to <linux/dmaengine.h) then?
>> >
>> > Would that be:
>> >
>> > struct dma_stride_config {
>> >    u32 read_bytes;
>> >    u32 skip_bytes;
>> > };
>> >
>> > Or something more complex?
>> Well, I am not sure if striding needs any special treatment at all.
>> Why not have client drivers prepare and submit sg-list.
>> Let the DMAC drivers interpret/parse the sg-list and program it
>> as strides if the h/w supports it.
>> If anything, we should make preparation and submission of sg-list
>> as efficient as possible.
> Jassi,
>
> sg_lists describe only a bunch of disjoint buffers. But what if the
> DMAC can skip and read the bytes within each of the buffers in
> the sg_list? (like TI EDMAC and TI SDMAC)
> How can that information be passed to the offload
> engine driver from the client?
>
OK, I overlooked.
We do need something new to handle these ultra-fine-grained sg-lists.
But still we shouldn't add SoC specific API to the common sub-systems.

Maybe a new api to pass fixed-format variable-length encoded message
to the DMAC drivers?
Which could be interpreted by DMAC drivers to extract all the needed xfer
parameters from the 'header' section and instructions to program the xfers
in the DMAC from the variable length body of the 'message' buffer.
It might sound complicated but we can have helpers to make the job easy.
Btw, the regular single/sg-list xfers could also be expressed by this method.

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 12:45           ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12 12:45 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
>> Sent: Tuesday, July 12, 2011 4:51 PM
>> To: Linus Walleij
>> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
>> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
>> configuration
>>
>> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
>> wrote:
>> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
>> wrote:
>> >
>> >> 1) Striding, in one form or other, is supported by other DMACs as well.
>> >>   The number will only increase in future.
>> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
>> >
>> > If we are sure about this and striding will work in a similar way on all
>> > then let's have the enum named DMA_STRIDE_CONFIG and move the
>> > passed-in struct to <linux/dmaengine.h) then?
>> >
>> > Would that be:
>> >
>> > struct dma_stride_config {
>> >    u32 read_bytes;
>> >    u32 skip_bytes;
>> > };
>> >
>> > Or something more complex?
>> Well, I am not sure if striding needs any special treatment at all.
>> Why not have client drivers prepare and submit sg-list.
>> Let the DMAC drivers interpret/parse the sg-list and program it
>> as strides if the h/w supports it.
>> If anything, we should make preparation and submission of sg-list
>> as efficient as possible.
> Jassi,
>
> sg_lists describe only a bunch of disjoint buffers. But what if the
> DMAC can skip and read the bytes within each of the buffers in
> the sg_list? (like TI EDMAC and TI SDMAC)
> How can that information be passed to the offload
> engine driver from the client?
>
OK, I overlooked.
We do need something new to handle these ultra-fine-grained sg-lists.
But still we shouldn't add SoC specific API to the common sub-systems.

Maybe a new api to pass fixed-format variable-length encoded message
to the DMAC drivers?
Which could be interpreted by DMAC drivers to extract all the needed xfer
parameters from the 'header' section and instructions to program the xfers
in the DMAC from the variable length body of the 'message' buffer.
It might sound complicated but we can have helpers to make the job easy.
Btw, the regular single/sg-list xfers could also be expressed by this method.

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-12 12:45           ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-12 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
>> Sent: Tuesday, July 12, 2011 4:51 PM
>> To: Linus Walleij
>> Cc: Raju, Sundaram; linux-arm-kernel at lists.infradead.org; linux-
>> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
>> linux at arm.linux.org.uk; dan.j.williams at intel.com; linux-omap at vger.kernel.org
>> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
>> configuration
>>
>> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
>> wrote:
>> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
>> wrote:
>> >
>> >> 1) Striding, in one form or other, is supported by other DMACs as well.
>> >> ? The number will only increase in future.
>> >> ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?
>> >
>> > If we are sure about this and striding will work in a similar way on all
>> > then let's have the enum named DMA_STRIDE_CONFIG and move the
>> > passed-in struct to <linux/dmaengine.h) then?
>> >
>> > Would that be:
>> >
>> > struct dma_stride_config {
>> > ? ?u32 read_bytes;
>> > ? ?u32 skip_bytes;
>> > };
>> >
>> > Or something more complex?
>> Well, I am not sure if striding needs any special treatment at all.
>> Why not have client drivers prepare and submit sg-list.
>> Let the DMAC drivers interpret/parse the sg-list and program it
>> as strides if the h/w supports it.
>> If anything, we should make preparation and submission of sg-list
>> as efficient as possible.
> Jassi,
>
> sg_lists describe only a bunch of disjoint buffers. But what if the
> DMAC can skip and read the bytes within each of the buffers in
> the sg_list? (like TI EDMAC and TI SDMAC)
> How can that information be passed to the offload
> engine driver from the client?
>
OK, I overlooked.
We do need something new to handle these ultra-fine-grained sg-lists.
But still we shouldn't add SoC specific API to the common sub-systems.

Maybe a new api to pass fixed-format variable-length encoded message
to the DMAC drivers?
Which could be interpreted by DMAC drivers to extract all the needed xfer
parameters from the 'header' section and instructions to program the xfers
in the DMAC from the variable length body of the 'message' buffer.
It might sound complicated but we can have helpers to make the job easy.
Btw, the regular single/sg-list xfers could also be expressed by this method.

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-12 12:45           ` Jassi Brar
  (?)
@ 2011-07-18  7:51             ` Raju, Sundaram
  -1 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-18  7:51 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3672 bytes --]

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Tuesday, July 12, 2011 6:15 PM
> To: Raju, Sundaram
> Cc: Linus Walleij; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> >> -----Original Message-----
> >> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> >> Sent: Tuesday, July 12, 2011 4:51 PM
> >> To: Linus Walleij
> >> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> >> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> >> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-
> omap@vger.kernel.org
> >> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> >> configuration
> >>
> >> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> >> wrote:
> >> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> >> wrote:
> >> >
> >> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >> >>   The number will only increase in future.
> >> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >> >
> >> > If we are sure about this and striding will work in a similar way on all
> >> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> >> > passed-in struct to <linux/dmaengine.h) then?
> >> >
> >> > Would that be:
> >> >
> >> > struct dma_stride_config {
> >> >    u32 read_bytes;
> >> >    u32 skip_bytes;
> >> > };
> >> >
> >> > Or something more complex?
> >> Well, I am not sure if striding needs any special treatment at all.
> >> Why not have client drivers prepare and submit sg-list.
> >> Let the DMAC drivers interpret/parse the sg-list and program it
> >> as strides if the h/w supports it.
> >> If anything, we should make preparation and submission of sg-list
> >> as efficient as possible.
> > Jassi,
> >
> > sg_lists describe only a bunch of disjoint buffers. But what if the
> > DMAC can skip and read the bytes within each of the buffers in
> > the sg_list? (like TI EDMAC and TI SDMAC)
> > How can that information be passed to the offload
> > engine driver from the client?
> >
> OK, I overlooked.
> We do need something new to handle these ultra-fine-grained sg-lists.
> But still we shouldn't add SoC specific API to the common sub-systems.
> 
> Maybe a new api to pass fixed-format variable-length encoded message
> to the DMAC drivers?
> Which could be interpreted by DMAC drivers to extract all the needed xfer
> parameters from the 'header' section and instructions to program the xfers
> in the DMAC from the variable length body of the 'message' buffer.
> It might sound complicated but we can have helpers to make the job easy.
> Btw, the regular single/sg-list xfers could also be expressed by this method.

Do you expect this variable length body of the message to be DMAC
independent? I don't think so. In that case how is this different from what
we have here already?

If it can be DMAC independent, can you illustrate more on how this can
be done? But the point to note is, if this can be made DMAC independent
then the control command we have also can be made DMAC independent
by generalizing the configuration structure passed to it.

~ Sundaram
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* RE: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-18  7:51             ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-18  7:51 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Tuesday, July 12, 2011 6:15 PM
> To: Raju, Sundaram
> Cc: Linus Walleij; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> >> -----Original Message-----
> >> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> >> Sent: Tuesday, July 12, 2011 4:51 PM
> >> To: Linus Walleij
> >> Cc: Raju, Sundaram; linux-arm-kernel@lists.infradead.org; linux-
> >> kernel@vger.kernel.org; davinci-linux-open-source@linux.davincidsp.com;
> >> linux@arm.linux.org.uk; dan.j.williams@intel.com; linux-
> omap@vger.kernel.org
> >> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> >> configuration
> >>
> >> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> >> wrote:
> >> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> >> wrote:
> >> >
> >> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >> >>   The number will only increase in future.
> >> >>   Are we to add  <VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >> >
> >> > If we are sure about this and striding will work in a similar way on all
> >> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> >> > passed-in struct to <linux/dmaengine.h) then?
> >> >
> >> > Would that be:
> >> >
> >> > struct dma_stride_config {
> >> >    u32 read_bytes;
> >> >    u32 skip_bytes;
> >> > };
> >> >
> >> > Or something more complex?
> >> Well, I am not sure if striding needs any special treatment at all.
> >> Why not have client drivers prepare and submit sg-list.
> >> Let the DMAC drivers interpret/parse the sg-list and program it
> >> as strides if the h/w supports it.
> >> If anything, we should make preparation and submission of sg-list
> >> as efficient as possible.
> > Jassi,
> >
> > sg_lists describe only a bunch of disjoint buffers. But what if the
> > DMAC can skip and read the bytes within each of the buffers in
> > the sg_list? (like TI EDMAC and TI SDMAC)
> > How can that information be passed to the offload
> > engine driver from the client?
> >
> OK, I overlooked.
> We do need something new to handle these ultra-fine-grained sg-lists.
> But still we shouldn't add SoC specific API to the common sub-systems.
> 
> Maybe a new api to pass fixed-format variable-length encoded message
> to the DMAC drivers?
> Which could be interpreted by DMAC drivers to extract all the needed xfer
> parameters from the 'header' section and instructions to program the xfers
> in the DMAC from the variable length body of the 'message' buffer.
> It might sound complicated but we can have helpers to make the job easy.
> Btw, the regular single/sg-list xfers could also be expressed by this method.

Do you expect this variable length body of the message to be DMAC
independent? I don't think so. In that case how is this different from what
we have here already?

If it can be DMAC independent, can you illustrate more on how this can
be done? But the point to note is, if this can be made DMAC independent
then the control command we have also can be made DMAC independent
by generalizing the configuration structure passed to it.

~ Sundaram

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-18  7:51             ` Raju, Sundaram
  0 siblings, 0 replies; 35+ messages in thread
From: Raju, Sundaram @ 2011-07-18  7:51 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
> Sent: Tuesday, July 12, 2011 6:15 PM
> To: Raju, Sundaram
> Cc: Linus Walleij; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
> linux at arm.linux.org.uk; dan.j.williams at intel.com; linux-omap at vger.kernel.org
> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> configuration
> 
> On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram <sundaram@ti.com> wrote:
> >> -----Original Message-----
> >> From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
> >> Sent: Tuesday, July 12, 2011 4:51 PM
> >> To: Linus Walleij
> >> Cc: Raju, Sundaram; linux-arm-kernel at lists.infradead.org; linux-
> >> kernel at vger.kernel.org; davinci-linux-open-source at linux.davincidsp.com;
> >> linux at arm.linux.org.uk; dan.j.williams at intel.com; linux-
> omap at vger.kernel.org
> >> Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
> >> configuration
> >>
> >> On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij <linus.walleij@linaro.org>
> >> wrote:
> >> > On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar <jassisinghbrar@gmail.com>
> >> wrote:
> >> >
> >> >> 1) Striding, in one form or other, is supported by other DMACs as well.
> >> >> ? The number will only increase in future.
> >> >> ? Are we to add ?<VENDOR>_DMA_STRIDE_CONFIG for each case ?
> >> >
> >> > If we are sure about this and striding will work in a similar way on all
> >> > then let's have the enum named DMA_STRIDE_CONFIG and move the
> >> > passed-in struct to <linux/dmaengine.h) then?
> >> >
> >> > Would that be:
> >> >
> >> > struct dma_stride_config {
> >> > ? ?u32 read_bytes;
> >> > ? ?u32 skip_bytes;
> >> > };
> >> >
> >> > Or something more complex?
> >> Well, I am not sure if striding needs any special treatment at all.
> >> Why not have client drivers prepare and submit sg-list.
> >> Let the DMAC drivers interpret/parse the sg-list and program it
> >> as strides if the h/w supports it.
> >> If anything, we should make preparation and submission of sg-list
> >> as efficient as possible.
> > Jassi,
> >
> > sg_lists describe only a bunch of disjoint buffers. But what if the
> > DMAC can skip and read the bytes within each of the buffers in
> > the sg_list? (like TI EDMAC and TI SDMAC)
> > How can that information be passed to the offload
> > engine driver from the client?
> >
> OK, I overlooked.
> We do need something new to handle these ultra-fine-grained sg-lists.
> But still we shouldn't add SoC specific API to the common sub-systems.
> 
> Maybe a new api to pass fixed-format variable-length encoded message
> to the DMAC drivers?
> Which could be interpreted by DMAC drivers to extract all the needed xfer
> parameters from the 'header' section and instructions to program the xfers
> in the DMAC from the variable length body of the 'message' buffer.
> It might sound complicated but we can have helpers to make the job easy.
> Btw, the regular single/sg-list xfers could also be expressed by this method.

Do you expect this variable length body of the message to be DMAC
independent? I don't think so. In that case how is this different from what
we have here already?

If it can be DMAC independent, can you illustrate more on how this can
be done? But the point to note is, if this can be made DMAC independent
then the control command we have also can be made DMAC independent
by generalizing the configuration structure passed to it.

~ Sundaram

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
  2011-07-18  7:51             ` Raju, Sundaram
  (?)
@ 2011-07-23 20:35               ` Jassi Brar
  -1 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-23 20:35 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Mon, Jul 18, 2011 at 1:21 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>>
>> Maybe a new api to pass fixed-format variable-length encoded message
>> to the DMAC drivers?
>> Which could be interpreted by DMAC drivers to extract all the needed xfer
>> parameters from the 'header' section and instructions to program the xfers
>> in the DMAC from the variable length body of the 'message' buffer.
>> It might sound complicated but we can have helpers to make the job easy.
>> Btw, the regular single/sg-list xfers could also be expressed by this method.
>
> Do you expect this variable length body of the message to be DMAC
> independent? I don't think so. In that case how is this different from what
> we have here already?
Yes, this whould be DMAC independent.

> If it can be DMAC independent, can you illustrate more on how this can
> be done? But the point to note is, if this can be made DMAC independent
> then the control command we have also can be made DMAC independent
> by generalizing the configuration structure passed to it.
The 'header' I suggest, would in fact be a structure body, only an extra pointer
would point to the 'instructions' to convey actual location and sizes
of mico-xfers.
I don't think it is possible to have general definition of such transfers fully
within a structure. If I understand what you ask.

I have just posted an RFC. I kept the terms same so that it is easier
to understand.
Please have a look. You are CC'ed too.

Thanks,
Jassi

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

* Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-23 20:35               ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-23 20:35 UTC (permalink / raw)
  To: Raju, Sundaram
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	davinci-linux-open-source, linux, dan.j.williams, linux-omap

On Mon, Jul 18, 2011 at 1:21 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>>
>> Maybe a new api to pass fixed-format variable-length encoded message
>> to the DMAC drivers?
>> Which could be interpreted by DMAC drivers to extract all the needed xfer
>> parameters from the 'header' section and instructions to program the xfers
>> in the DMAC from the variable length body of the 'message' buffer.
>> It might sound complicated but we can have helpers to make the job easy.
>> Btw, the regular single/sg-list xfers could also be expressed by this method.
>
> Do you expect this variable length body of the message to be DMAC
> independent? I don't think so. In that case how is this different from what
> we have here already?
Yes, this whould be DMAC independent.

> If it can be DMAC independent, can you illustrate more on how this can
> be done? But the point to note is, if this can be made DMAC independent
> then the control command we have also can be made DMAC independent
> by generalizing the configuration structure passed to it.
The 'header' I suggest, would in fact be a structure body, only an extra pointer
would point to the 'instructions' to convey actual location and sizes
of mico-xfers.
I don't think it is possible to have general definition of such transfers fully
within a structure. If I understand what you ask.

I have just posted an RFC. I kept the terms same so that it is easier
to understand.
Please have a look. You are CC'ed too.

Thanks,
Jassi

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

* [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration
@ 2011-07-23 20:35               ` Jassi Brar
  0 siblings, 0 replies; 35+ messages in thread
From: Jassi Brar @ 2011-07-23 20:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 18, 2011 at 1:21 PM, Raju, Sundaram <sundaram@ti.com> wrote:
>>
>> Maybe a new api to pass fixed-format variable-length encoded message
>> to the DMAC drivers?
>> Which could be interpreted by DMAC drivers to extract all the needed xfer
>> parameters from the 'header' section and instructions to program the xfers
>> in the DMAC from the variable length body of the 'message' buffer.
>> It might sound complicated but we can have helpers to make the job easy.
>> Btw, the regular single/sg-list xfers could also be expressed by this method.
>
> Do you expect this variable length body of the message to be DMAC
> independent? I don't think so. In that case how is this different from what
> we have here already?
Yes, this whould be DMAC independent.

> If it can be DMAC independent, can you illustrate more on how this can
> be done? But the point to note is, if this can be made DMAC independent
> then the control command we have also can be made DMAC independent
> by generalizing the configuration structure passed to it.
The 'header' I suggest, would in fact be a structure body, only an extra pointer
would point to the 'instructions' to convey actual location and sizes
of mico-xfers.
I don't think it is possible to have general definition of such transfers fully
within a structure. If I understand what you ask.

I have just posted an RFC. I kept the terms same so that it is easier
to understand.
Please have a look. You are CC'ed too.

Thanks,
Jassi

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

end of thread, other threads:[~2011-07-23 20:35 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-10 15:03 [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration Sundaram Raju
2011-07-10 15:03 ` Sundaram Raju
2011-07-11  9:28 ` Linus Walleij
2011-07-11  9:28   ` Linus Walleij
2011-07-11 21:39   ` Dan Williams
2011-07-11 21:39     ` Dan Williams
2011-07-12  9:58     ` Linus Walleij
2011-07-12  9:58       ` Linus Walleij
2011-07-12 10:15       ` Raju, Sundaram
2011-07-12 10:15         ` Raju, Sundaram
2011-07-12 10:15         ` Raju, Sundaram
2011-07-12  4:17 ` Jassi Brar
2011-07-12  4:17   ` Jassi Brar
2011-07-12 10:03   ` Linus Walleij
2011-07-12 10:03     ` Linus Walleij
2011-07-12 10:56     ` Raju, Sundaram
2011-07-12 10:56       ` Raju, Sundaram
2011-07-12 10:56       ` Raju, Sundaram
2011-07-12 11:09       ` Linus Walleij
2011-07-12 11:09         ` Linus Walleij
2011-07-12 11:09         ` Linus Walleij
2011-07-12 11:20     ` Jassi Brar
2011-07-12 11:20       ` Jassi Brar
2011-07-12 11:31       ` Raju, Sundaram
2011-07-12 11:31         ` Raju, Sundaram
2011-07-12 11:31         ` Raju, Sundaram
2011-07-12 12:45         ` Jassi Brar
2011-07-12 12:45           ` Jassi Brar
2011-07-12 12:45           ` Jassi Brar
2011-07-18  7:51           ` Raju, Sundaram
2011-07-18  7:51             ` Raju, Sundaram
2011-07-18  7:51             ` Raju, Sundaram
2011-07-23 20:35             ` Jassi Brar
2011-07-23 20:35               ` Jassi Brar
2011-07-23 20:35               ` 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.