[1/3] dmaengine: add possibility for cyclic transfers
diff mbox series

Message ID 1281956870-12463-2-git-send-email-s.hauer@pengutronix.de
State New, archived
Headers show
Series
  • [1/3] dmaengine: add possibility for cyclic transfers
Related show

Commit Message

Sascha Hauer Aug. 16, 2010, 11:07 a.m. UTC
Cyclic transfers are useful for audio where a single buffer divided
in periods has to be transfered endlessly until stopped. After being
prepared the transfer is started using the dma_async_descriptor->tx_submit
function. dma_async_descriptor->callback is called after each period.
The transfer is stopped using the DMA_TERMINATE_ALL callback.
While being used for cyclic transfers the channel cannot be used
for other transfer types.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 drivers/dma/dmaengine.c   |    2 ++
 include/linux/dmaengine.h |    6 +++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

Comments

Lothar Waßmann Aug. 16, 2010, 11:56 a.m. UTC | #1
Hi,

Sascha Hauer writes:
> Cyclic transfers are useful for audio where a single buffer divided
> in periods has to be transfered endlessly until stopped. After being
> prepared the transfer is started using the dma_async_descriptor->tx_submit
> function. dma_async_descriptor->callback is called after each period.
> The transfer is stopped using the DMA_TERMINATE_ALL callback.
> While being used for cyclic transfers the channel cannot be used
> for other transfer types.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> ---
>  drivers/dma/dmaengine.c   |    2 ++
>  include/linux/dmaengine.h |    6 +++++-
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9d31d5e..e5e79ce 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -692,6 +692,8 @@ int dma_async_device_register(struct dma_device *device)
>  		!device->device_prep_dma_interrupt);
>  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>  		!device->device_prep_slave_sg);
> +	BUG_ON(dma_has_cap(DMA_CYCLIC, device->cap_mask) &&
> +		!device->device_prep_dma_cyclic);
>  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>  		!device->device_control);
>  
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index c61d4ca..0df7864 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -67,10 +67,11 @@ enum dma_transaction_type {
>  	DMA_PRIVATE,
>  	DMA_ASYNC_TX,
>  	DMA_SLAVE,
> +	DMA_CYCLIC,
>  };
>  
>  /* last transaction type for creation of the capabilities mask */
> -#define DMA_TX_TYPE_END (DMA_SLAVE + 1)
> +#define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
>  
>  
>  /**
> @@ -478,6 +479,9 @@ struct dma_device {
>  		struct dma_chan *chan, struct scatterlist *sgl,
>  		unsigned int sg_len, enum dma_data_direction direction,
>  		unsigned long flags);
> +	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);
>  	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  		unsigned long arg);
>  
> -- 
> 1.7.1
> 
Why not implement this feature using cyclic SG lists (created with
sg_chain())? This would give you endless DMA transfers without any
special DMA API extensions.


Lothar Waßmann
Linus Walleij Aug. 16, 2010, 12:22 p.m. UTC | #2
2010/8/16 Sascha Hauer <s.hauer@pengutronix.de>:

> Cyclic transfers are useful for audio where a single buffer divided
> in periods has to be transfered endlessly until stopped. After being
> prepared the transfer is started using the dma_async_descriptor->tx_submit
> function. dma_async_descriptor->callback is called after each period.
> The transfer is stopped using the DMA_TERMINATE_ALL callback.
> While being used for cyclic transfers the channel cannot be used
> for other transfer types.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

Looks good to me.
Acked-by: Linus Walleij <linus.walleij@stericsson.com>

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Linus Walleij Aug. 16, 2010, 12:27 p.m. UTC | #3
2010/8/16 Lothar Waßmann <LW@karo-electronics.de>:

> Why not implement this feature using cyclic SG lists (created with
> sg_chain())? This would give you endless DMA transfers without any
> special DMA API extensions.

That would be elegant...

The driver will have to detect that the sglist is chained like an
ouroboros to program the DMAC apropriately, is is easy to
detect if an sglist is chained onto itself?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Sascha Hauer Aug. 16, 2010, 12:32 p.m. UTC | #4
On Mon, Aug 16, 2010 at 01:56:34PM +0200, Lothar Waßmann wrote:
> Hi,
> 
> Sascha Hauer writes:
> > Cyclic transfers are useful for audio where a single buffer divided
> > in periods has to be transfered endlessly until stopped. After being
> > prepared the transfer is started using the dma_async_descriptor->tx_submit
> > function. dma_async_descriptor->callback is called after each period.
> > The transfer is stopped using the DMA_TERMINATE_ALL callback.
> > While being used for cyclic transfers the channel cannot be used
> > for other transfer types.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> > ---
> >  drivers/dma/dmaengine.c   |    2 ++
> >  include/linux/dmaengine.h |    6 +++++-
> >  2 files changed, 7 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index 9d31d5e..e5e79ce 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -692,6 +692,8 @@ int dma_async_device_register(struct dma_device *device)
> >  		!device->device_prep_dma_interrupt);
> >  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
> >  		!device->device_prep_slave_sg);
> > +	BUG_ON(dma_has_cap(DMA_CYCLIC, device->cap_mask) &&
> > +		!device->device_prep_dma_cyclic);
> >  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
> >  		!device->device_control);
> >  
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index c61d4ca..0df7864 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -67,10 +67,11 @@ enum dma_transaction_type {
> >  	DMA_PRIVATE,
> >  	DMA_ASYNC_TX,
> >  	DMA_SLAVE,
> > +	DMA_CYCLIC,
> >  };
> >  
> >  /* last transaction type for creation of the capabilities mask */
> > -#define DMA_TX_TYPE_END (DMA_SLAVE + 1)
> > +#define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
> >  
> >  
> >  /**
> > @@ -478,6 +479,9 @@ struct dma_device {
> >  		struct dma_chan *chan, struct scatterlist *sgl,
> >  		unsigned int sg_len, enum dma_data_direction direction,
> >  		unsigned long flags);
> > +	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);
> >  	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> >  		unsigned long arg);
> >  
> > -- 
> > 1.7.1
> > 
> Why not implement this feature using cyclic SG lists (created with
> sg_chain())? This would give you endless DMA transfers without any
> special DMA API extensions.

Been there, done that. In the end it just seemed like adding additional
overhead to create the sg list and using the sg for something for which
it is not really designed. Still you need extensions to the DMA API to
signal that you want to have a callback for every sg entry. Normally you
only need a callback on the end of the list.

Sascha
Sascha Hauer Sept. 20, 2010, 1:01 p.m. UTC | #5
Hi Dan,

Any comment to this patch?

Sascha

On Mon, Aug 16, 2010 at 01:07:48PM +0200, Sascha Hauer wrote:
> Cyclic transfers are useful for audio where a single buffer divided
> in periods has to be transfered endlessly until stopped. After being
> prepared the transfer is started using the dma_async_descriptor->tx_submit
> function. dma_async_descriptor->callback is called after each period.
> The transfer is stopped using the DMA_TERMINATE_ALL callback.
> While being used for cyclic transfers the channel cannot be used
> for other transfer types.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
> ---
>  drivers/dma/dmaengine.c   |    2 ++
>  include/linux/dmaengine.h |    6 +++++-
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9d31d5e..e5e79ce 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -692,6 +692,8 @@ int dma_async_device_register(struct dma_device *device)
>  		!device->device_prep_dma_interrupt);
>  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>  		!device->device_prep_slave_sg);
> +	BUG_ON(dma_has_cap(DMA_CYCLIC, device->cap_mask) &&
> +		!device->device_prep_dma_cyclic);
>  	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
>  		!device->device_control);
>  
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index c61d4ca..0df7864 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -67,10 +67,11 @@ enum dma_transaction_type {
>  	DMA_PRIVATE,
>  	DMA_ASYNC_TX,
>  	DMA_SLAVE,
> +	DMA_CYCLIC,
>  };
>  
>  /* last transaction type for creation of the capabilities mask */
> -#define DMA_TX_TYPE_END (DMA_SLAVE + 1)
> +#define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
>  
>  
>  /**
> @@ -478,6 +479,9 @@ struct dma_device {
>  		struct dma_chan *chan, struct scatterlist *sgl,
>  		unsigned int sg_len, enum dma_data_direction direction,
>  		unsigned long flags);
> +	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);
>  	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  		unsigned long arg);
>  
> -- 
> 1.7.1
> 
>
Dan Williams Sept. 23, 2010, 7:42 p.m. UTC | #6
On Mon, Sep 20, 2010 at 6:01 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> Hi Dan,
>
> Any comment to this patch?

Looks good to me especially given the unique implications for
callbacks.  Although, I'd like to put a description of the
prep_dma_cyclic semantics in the source code.  Can you respin this
with a description added to the kernel-doc for struct dma_device?

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Sascha Hauer Sept. 24, 2010, 7:25 a.m. UTC | #7
On Thu, Sep 23, 2010 at 12:42:20PM -0700, Dan Williams wrote:
> On Mon, Sep 20, 2010 at 6:01 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi Dan,
> >
> > Any comment to this patch?
> 
> Looks good to me especially given the unique implications for
> callbacks.  Although, I'd like to put a description of the
> prep_dma_cyclic semantics in the source code.  Can you respin this
> with a description added to the kernel-doc for struct dma_device?

Sure, will do.

Sascha

Patch
diff mbox series

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9d31d5e..e5e79ce 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -692,6 +692,8 @@  int dma_async_device_register(struct dma_device *device)
 		!device->device_prep_dma_interrupt);
 	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
 		!device->device_prep_slave_sg);
+	BUG_ON(dma_has_cap(DMA_CYCLIC, device->cap_mask) &&
+		!device->device_prep_dma_cyclic);
 	BUG_ON(dma_has_cap(DMA_SLAVE, device->cap_mask) &&
 		!device->device_control);
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index c61d4ca..0df7864 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -67,10 +67,11 @@  enum dma_transaction_type {
 	DMA_PRIVATE,
 	DMA_ASYNC_TX,
 	DMA_SLAVE,
+	DMA_CYCLIC,
 };
 
 /* last transaction type for creation of the capabilities mask */
-#define DMA_TX_TYPE_END (DMA_SLAVE + 1)
+#define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
 
 
 /**
@@ -478,6 +479,9 @@  struct dma_device {
 		struct dma_chan *chan, struct scatterlist *sgl,
 		unsigned int sg_len, enum dma_data_direction direction,
 		unsigned long flags);
+	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);
 	int (*device_control)(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 		unsigned long arg);