[PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[Javier Martin]

MEMCPY transfers allow DMA copies from memory to
memory. This patch has been tested with dmatest
device driver.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/dma/imx-dma.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index d99f71c..222cd7b 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -196,7 +196,8 @@ static int imxdma_alloc_chan_resources(struct dma_chan *chan)
 	struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
 	struct imx_dma_data *data = chan->private;
 
-	imxdmac->dma_request = data->dma_request;
+	if (data != NULL)
+		imxdmac->dma_request = data->dma_request;
 
 	dma_async_tx_descriptor_init(&imxdmac->desc, chan);
 	imxdmac->desc.tx_submit = imxdma_tx_submit;
@@ -328,6 +329,37 @@ static struct dma_async_tx_descriptor *imxdma_prep_dma_cyclic(
 	return &imxdmac->desc;
 }
 
+static struct dma_async_tx_descriptor *imxdma_prep_dma_memcpy(
+	struct dma_chan *chan, dma_addr_t dest,
+	dma_addr_t src, size_t len, unsigned long flags)
+{
+	struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
+	struct imxdma_engine *imxdma = imxdmac->imxdma;
+	int ret;
+
+	dev_dbg(imxdma->dev, "%s channel: %d src=0x%x dst=0x%x len=%d\n",
+			__func__, imxdmac->channel, src, dest, len);
+
+	if (imxdmac->status == DMA_IN_PROGRESS)
+		return NULL;
+
+	imxdmac->status = DMA_IN_PROGRESS;
+
+	ret = imx_dma_config_channel(imxdmac->imxdma_channel,
+			       IMX_DMA_MEMSIZE_32 | IMX_DMA_TYPE_LINEAR,
+			       IMX_DMA_MEMSIZE_32 | IMX_DMA_TYPE_LINEAR,
+			       0, 0);
+	if (ret)
+		return NULL;
+
+	ret = imx_dma_setup_single(imxdmac->imxdma_channel, src, len,
+				   dest, DMA_MODE_WRITE);
+	if (ret)
+		return NULL;
+
+	return &imxdmac->desc;
+}
+
 static void imxdma_issue_pending(struct dma_chan *chan)
 {
 	/*
@@ -348,6 +380,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
 
 	dma_cap_set(DMA_SLAVE, imxdma->dma_device.cap_mask);
 	dma_cap_set(DMA_CYCLIC, imxdma->dma_device.cap_mask);
+	dma_cap_set(DMA_MEMCPY, imxdma->dma_device.cap_mask);
 
 	/* Initialize channel parameters */
 	for (i = 0; i < MAX_DMA_CHANNELS; i++) {
@@ -381,11 +414,13 @@ static int __init imxdma_probe(struct platform_device *pdev)
 	imxdma->dma_device.device_tx_status = imxdma_tx_status;
 	imxdma->dma_device.device_prep_slave_sg = imxdma_prep_slave_sg;
 	imxdma->dma_device.device_prep_dma_cyclic = imxdma_prep_dma_cyclic;
+	imxdma->dma_device.device_prep_dma_memcpy = imxdma_prep_dma_memcpy;
 	imxdma->dma_device.device_control = imxdma_control;
 	imxdma->dma_device.device_issue_pending = imxdma_issue_pending;
 
 	platform_set_drvdata(pdev, imxdma);
 
+	imxdma->dma_device.copy_align = 2; /* 2^2 = 4 bytes alignment */
 	imxdma->dma_device.dev->dma_parms = &imxdma->dma_parms;
 	dma_set_max_seg_size(imxdma->dma_device.dev, 0xffffff);
 
-- 
1.7.0.4

Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[javier Martin]

Any comments on this patch?
I don't know exactly which tree this belongs to. Sascha have you
applied it to your tree?

Thank you.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[Vinod Koul]

On Wed, 2011-12-21 at 12:41 +0100, javier Martin wrote:
> Any comments on this patch?
> I don't know exactly which tree this belongs to. Sascha have you
> applied it to your tree?

I was on vacation last week, so I am bit back logged, would get cleared
before next week hopefully :)


-- 
~Vinod

Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[Vinod Koul]

On Fri, 2011-12-16 at 16:59 +0100, Javier Martin wrote:
> MEMCPY transfers allow DMA copies from memory to
> memory. This patch has been tested with dmatest
> device driver.
> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/dma/imx-dma.c |   37 ++++++++++++++++++++++++++++++++++++-
>  1 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index d99f71c..222cd7b 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
> @@ -196,7 +196,8 @@ static int imxdma_alloc_chan_resources(struct dma_chan *chan)
>  	struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
>  	struct imx_dma_data *data = chan->private;
>  
> -	imxdmac->dma_request = data->dma_request;
> +	if (data != NULL)
> +		imxdmac->dma_request = data->dma_request;
>  
>  	dma_async_tx_descriptor_init(&imxdmac->desc, chan);
>  	imxdmac->desc.tx_submit = imxdma_tx_submit;
> @@ -328,6 +329,37 @@ static struct dma_async_tx_descriptor *imxdma_prep_dma_cyclic(
>  	return &imxdmac->desc;
>  }
>  
> +static struct dma_async_tx_descriptor *imxdma_prep_dma_memcpy(
> +	struct dma_chan *chan, dma_addr_t dest,
> +	dma_addr_t src, size_t len, unsigned long flags)
> +{
> +	struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
> +	struct imxdma_engine *imxdma = imxdmac->imxdma;
> +	int ret;
> +
> +	dev_dbg(imxdma->dev, "%s channel: %d src=0x%x dst=0x%x len=%d\n",
> +			__func__, imxdmac->channel, src, dest, len);
> +
> +	if (imxdmac->status == DMA_IN_PROGRESS)
> +		return NULL;
> +
> +	imxdmac->status = DMA_IN_PROGRESS;
why? prepare doesn't mean DMA is in progress. You can get multiple
descriptors for same channel by calling prepare multiple times.
Even one descriptor can be memcpy whereas other is slave...
In issue_pending this should be moved to DMA_IN_PROGRESS
> +
> +	ret = imx_dma_config_channel(imxdmac->imxdma_channel,
> +			       IMX_DMA_MEMSIZE_32 | IMX_DMA_TYPE_LINEAR,
> +			       IMX_DMA_MEMSIZE_32 | IMX_DMA_TYPE_LINEAR,
> +			       0, 0);
> +	if (ret)
> +		return NULL;
> +
> +	ret = imx_dma_setup_single(imxdmac->imxdma_channel, src, len,
> +				   dest, DMA_MODE_WRITE);
> +	if (ret)
> +		return NULL;
> +
> +	return &imxdmac->desc;
> +}
> +
>  static void imxdma_issue_pending(struct dma_chan *chan)
>  {
>  	/*
> @@ -348,6 +380,7 @@ static int __init imxdma_probe(struct platform_device *pdev)
>  
>  	dma_cap_set(DMA_SLAVE, imxdma->dma_device.cap_mask);
>  	dma_cap_set(DMA_CYCLIC, imxdma->dma_device.cap_mask);
> +	dma_cap_set(DMA_MEMCPY, imxdma->dma_device.cap_mask);
>  
>  	/* Initialize channel parameters */
>  	for (i = 0; i < MAX_DMA_CHANNELS; i++) {
> @@ -381,11 +414,13 @@ static int __init imxdma_probe(struct platform_device *pdev)
>  	imxdma->dma_device.device_tx_status = imxdma_tx_status;
>  	imxdma->dma_device.device_prep_slave_sg = imxdma_prep_slave_sg;
>  	imxdma->dma_device.device_prep_dma_cyclic = imxdma_prep_dma_cyclic;
> +	imxdma->dma_device.device_prep_dma_memcpy = imxdma_prep_dma_memcpy;
>  	imxdma->dma_device.device_control = imxdma_control;
>  	imxdma->dma_device.device_issue_pending = imxdma_issue_pending;
>  
>  	platform_set_drvdata(pdev, imxdma);
>  
> +	imxdma->dma_device.copy_align = 2; /* 2^2 = 4 bytes alignment */
>  	imxdma->dma_device.dev->dma_parms = &imxdma->dma_parms;
>  	dma_set_max_seg_size(imxdma->dma_device.dev, 0xffffff);
>  


-- 
~Vinod

Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[javier Martin]

Hi Vinod,
thank you for your review.

On 23 December 2011 18:38, Vinod Koul <vinod.koul@intel.com> wrote:
>> @@ -328,6 +329,37 @@ static struct dma_async_tx_descriptor *imxdma_prep_dma_cyclic(
>>       return &imxdmac->desc;
>>  }
>>
>> +static struct dma_async_tx_descriptor *imxdma_prep_dma_memcpy(
>> +     struct dma_chan *chan, dma_addr_t dest,
>> +     dma_addr_t src, size_t len, unsigned long flags)
>> +{
>> +     struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
>> +     struct imxdma_engine *imxdma = imxdmac->imxdma;
>> +     int ret;
>> +
>> +     dev_dbg(imxdma->dev, "%s channel: %d src=0x%x dst=0x%x len=%d\n",
>> +                     __func__, imxdmac->channel, src, dest, len);
>> +
>> +     if (imxdmac->status == DMA_IN_PROGRESS)
>> +             return NULL;
>> +
>> +     imxdmac->status = DMA_IN_PROGRESS;
> why? prepare doesn't mean DMA is in progress. You can get multiple
> descriptors for same channel by calling prepare multiple times.
> Even one descriptor can be memcpy whereas other is slave...
> In issue_pending this should be moved to DMA_IN_PROGRESS

According to the original developer of this file it only supports a
single descriptor. Please, take a look at the following function:

static void imxdma_issue_pending(struct dma_chan *chan)
{
	/*
	 * Nothing to do. We only have a single descriptor
	 */
}


Also, the same behavior is present in the other prepare functions:
"imxdma_prep_slave_sg" and "imxdma_prep_dma_cyclic".

I think the less painful approach here is merging my patch so that
multiple descriptors support can be added later.

--
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH] dmaengine: Add support for MEMCPY for imx-dma.

[Vinod Koul]

On Mon, 2012-01-02 at 11:08 +0100, javier Martin wrote:
> Hi Vinod,
> thank you for your review.
> 
> On 23 December 2011 18:38, Vinod Koul <vinod.koul@intel.com> wrote:
> >> @@ -328,6 +329,37 @@ static struct dma_async_tx_descriptor *imxdma_prep_dma_cyclic(
> >>       return &imxdmac->desc;
> >>  }
> >>
> >> +static struct dma_async_tx_descriptor *imxdma_prep_dma_memcpy(
> >> +     struct dma_chan *chan, dma_addr_t dest,
> >> +     dma_addr_t src, size_t len, unsigned long flags)
> >> +{
> >> +     struct imxdma_channel *imxdmac = to_imxdma_chan(chan);
> >> +     struct imxdma_engine *imxdma = imxdmac->imxdma;
> >> +     int ret;
> >> +
> >> +     dev_dbg(imxdma->dev, "%s channel: %d src=0x%x dst=0x%x len=%d\n",
> >> +                     __func__, imxdmac->channel, src, dest, len);
> >> +
> >> +     if (imxdmac->status == DMA_IN_PROGRESS)
> >> +             return NULL;
> >> +
> >> +     imxdmac->status = DMA_IN_PROGRESS;
> > why? prepare doesn't mean DMA is in progress. You can get multiple
> > descriptors for same channel by calling prepare multiple times.
> > Even one descriptor can be memcpy whereas other is slave...
> > In issue_pending this should be moved to DMA_IN_PROGRESS
> 
> According to the original developer of this file it only supports a
> single descriptor. Please, take a look at the following function:
> 
> static void imxdma_issue_pending(struct dma_chan *chan)
> {
> 	/*
> 	 * Nothing to do. We only have a single descriptor
> 	 */
> }
> 
> 
> Also, the same behavior is present in the other prepare functions:
> "imxdma_prep_slave_sg" and "imxdma_prep_dma_cyclic".
> 
> I think the less painful approach here is merging my patch so that
> multiple descriptors support can be added later.
This behavior has nothing to do with single/multiple descriptors.
Prepare means to prepare the transfer and not to submit. There are other
APIs given to do the job. Please go thru the Documentation/dmaengine.txt
as well.

I know some drivers are broken in this respect, but would be great if
you can fix this behavior here as well.

-- 
~Vinod