Patchwork [1/2] i.MX DMA: Add support for 2D transfers.

login
register
mail settings
Submitter Javier Martin
Date Feb. 3, 2012, 4:11 p.m.
Message ID <1328285474-26365-1-git-send-email-javier.martin@vista-silicon.com>
Download mbox | patch
Permalink /patch/139416/
State New
Headers show

Comments

Javier Martin - Feb. 3, 2012, 4:11 p.m.
DMAC present in i.MX2 and i.MX1 chips have two
2D configuration slots that any DMA channel can
use to make 2D DMA transfers.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 arch/arm/mach-imx/dma-v1.c              |   86 +++++++++++++++++++++++++++++++
 arch/arm/mach-imx/include/mach/dma-v1.h |    7 +++
 2 files changed, 93 insertions(+), 0 deletions(-)
Sascha Hauer - Feb. 9, 2012, 2:17 p.m.
On Fri, Feb 03, 2012 at 05:11:13PM +0100, Javier Martin wrote:
> DMAC present in i.MX2 and i.MX1 chips have two
> 2D configuration slots that any DMA channel can
> use to make 2D DMA transfers.
> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>

My problem with this patch is that it adds new code to
arch/arm/mach-imx/dma-v1.c.
The original intention of having a legacy driver in arch/arm/mach-imx/dma-v1.c
and a dmaengine driver in drivers/dma was that we move remove the
arch/arm parts to drivers/dma once all users have switched to the new
API.  I fixed the sound drivers and the mxcmmc driver already.  We
currently have the imxmmc driver and the i.MX1/2 camera drivers left
in the tree.  The imxmmc driver is broken for ages and I think we can
remove it when nobody takes care of putting it back to work. You seem to
work on the i.MX2 camera driver and thus you should be able to fix it
(do you use DMA here or the EMMA engine?). This leaves the i.MX1 driver
and I have no possibility to fix it. I once pinged Paulius about this
issue but appearently nothing has happened.
I think we should start cleaning this up even if we risk breaking
something. Otherwise we are stuck with the legacy driver forever.

Sascha
Javier Martin - Feb. 9, 2012, 3:45 p.m.
Hi Sascha,

On 9 February 2012 15:17, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Feb 03, 2012 at 05:11:13PM +0100, Javier Martin wrote:
>> DMAC present in i.MX2 and i.MX1 chips have two
>> 2D configuration slots that any DMA channel can
>> use to make 2D DMA transfers.
>>
>> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
>
> My problem with this patch is that it adds new code to
> arch/arm/mach-imx/dma-v1.c.
> The original intention of having a legacy driver in arch/arm/mach-imx/dma-v1.c
> and a dmaengine driver in drivers/dma was that we move remove the
> arch/arm parts to drivers/dma once all users have switched to the new
> API.

I wasn't aware of that. I couldn't find any warning comment in the
code or anything.

>I fixed the sound drivers and the mxcmmc driver already.  We
> currently have the imxmmc driver and the i.MX1/2 camera drivers left
> in the tree.

I know, I tested those changes myself and works properly. Thank you.

>The imxmmc driver is broken for ages and I think we can
> remove it when nobody takes care of putting it back to work. You seem to
> work on the i.MX2 camera driver and thus you should be able to fix it
> (do you use DMA here or the EMMA engine?).

Currently i.MX2 camera does not use DMAs. You removed support for it
recently (which I found very convenient) and I don't have plans to use
them. This means that there is no i.MX2 driver using legacy code out
there.

>This leaves the i.MX1 driver
> and I have no possibility to fix it. I once pinged Paulius about this
> issue but appearently nothing has happened.
> I think we should start cleaning this up even if we risk breaking
> something. Otherwise we are stuck with the legacy driver forever.

Let me explain the situation I have here (which is a summarize of what
I have already explained to Vinod):

We have a video processing chain which involves a tvp5150, mx2 camera
capture driver and and intermediate deinterlacing driver (mem2mem).

Deinterlacing driver requires a full working dmaengine API and
interleaved transfer support. As a consequence our development process
is organized as follows:

- Step 1 (already submitted):
[PATCH v3] dmaengine: Add support for MEMCPY for imx-dma.

This step is important for us since it gives us access to the dmatest
module and allows to develop a first prototype of the driver which
just copies the content of one buffer into another.

- Step 2 (also submitted):
[PATCH 1/2] i.MX DMA: Add support for 2D transfers.
[PATCH 2/2] dmaengine: i.MX: Add support for interleaved transfers.

With this step we have know a first working version of a driver which
actually does video deinterlacing. However, since current dmaengine
support for imx only regards a single descriptor, this code is quite
ugly and dirty.

- Step 3 (developing):
dmaengine: i.MX: Support multiple descriptors.

The problem here is that, after 6 days from my last patch, I have
Step3 nearly finished (I was just testing it when I received your
e-mail). According to your suggestion, I would have to rewrite all my
code in order to drop the arch/arm file. However, we are targeting for
merge window 3.4 and have already agreed with Guennadi some cleanup
patches for the mx2_camera driver.

I know that you want that file to be dropped and I understand, but my
schedule is so tight that I can't afford a complete rewriting right
now. I hope you can undesrtand too.

Regards.

Patch

diff --git a/arch/arm/mach-imx/dma-v1.c b/arch/arm/mach-imx/dma-v1.c
index 42afc29..7401138 100644
--- a/arch/arm/mach-imx/dma-v1.c
+++ b/arch/arm/mach-imx/dma-v1.c
@@ -121,6 +121,9 @@  struct imx_dma_channel {
 
 	int in_use;
 
+	bool enabled_2d;
+	int slot_2d;
+
 	u32 ccr_from_device;
 	u32 ccr_to_device;
 
@@ -129,6 +132,13 @@  struct imx_dma_channel {
 	int hw_chaining;
 };
 
+struct imx_dma_2d_config {
+	u16		xsr;
+	u16		ysr;
+	u16		wsr;
+	int		count;
+};
+
 static void __iomem *imx_dmav1_baseaddr;
 
 static void imx_dmav1_writel(unsigned val, unsigned offset)
@@ -143,6 +153,9 @@  static unsigned imx_dmav1_readl(unsigned offset)
 
 static struct imx_dma_channel imx_dma_channels[IMX_DMA_CHANNELS];
 
+static struct imx_dma_2d_config imx_dma_2d_slots[IMX_DMA_2D_SLOTS];
+static spinlock_t lock_2d;
+
 static struct clk *dma_clk;
 
 static int imx_dma_hw_chain(struct imx_dma_channel *imxdma)
@@ -369,6 +382,11 @@  imx_dma_config_channel(int channel, unsigned int config_port,
 	imxdma->ccr_from_device = config_port | (config_mem << 2) | dreq;
 	imxdma->ccr_to_device = config_mem | (config_port << 2) | dreq;
 
+	if (imxdma->enabled_2d && (imxdma->slot_2d == IMX_DMA_2D_SLOT_B)) {
+		imxdma->ccr_from_device |= CCR_MSEL_B;
+		imxdma->ccr_to_device |= CCR_MSEL_B;
+	}
+
 	imx_dmav1_writel(dmareq, DMA_RSSR(channel));
 
 	return 0;
@@ -382,6 +400,63 @@  void imx_dma_config_burstlen(int channel, unsigned int burstlen)
 EXPORT_SYMBOL(imx_dma_config_burstlen);
 
 /**
+ * imx_dma_config_2d - prepare i.MX DMA channel for a 2D transfer.
+ * @channel: i.MX DMA channel number
+ * @x: x-size of the 2D window.
+ * @y: number of rows that make up the 2D window.
+ * @w: display size of the 2D window
+ */
+int imx_dma_config_2d(int channel, unsigned int x, unsigned int y,
+		      unsigned int w)
+{
+	struct imx_dma_channel *imxdma = &imx_dma_channels[channel];
+	int slot = -1;
+	int i;
+
+	spin_lock(&lock_2d);
+	/* If the channel already owns a slot, free it first */
+	if (imxdma->enabled_2d) {
+		imx_dma_2d_slots[imxdma->slot_2d].count--;
+		imxdma->enabled_2d = false;
+	}
+	/* Try to get free 2D slot */
+	for (i = 0; i < IMX_DMA_2D_SLOTS; i++) {
+		if ((imx_dma_2d_slots[i].count > 0) &&
+		    ((imx_dma_2d_slots[i].xsr != x) ||
+		     (imx_dma_2d_slots[i].ysr != y) ||
+		     (imx_dma_2d_slots[i].wsr != w)))
+			continue;
+		slot = i;
+		break;
+	}
+	if (slot < 0)
+		return -EBUSY;
+
+	imx_dma_2d_slots[slot].xsr = x;
+	imx_dma_2d_slots[slot].ysr = y;
+	imx_dma_2d_slots[slot].wsr = w;
+	imx_dma_2d_slots[slot].count++;
+
+	spin_unlock(&lock_2d);
+
+	imxdma->slot_2d = slot;
+	imxdma->enabled_2d = true;
+
+	if (slot == IMX_DMA_2D_SLOT_A) {
+		imx_dmav1_writel(x, DMA_XSRA);
+		imx_dmav1_writel(y, DMA_YSRA);
+		imx_dmav1_writel(w, DMA_WSRA);
+	} else {
+		imx_dmav1_writel(x, DMA_XSRB);
+		imx_dmav1_writel(y, DMA_YSRB);
+		imx_dmav1_writel(w, DMA_WSRB);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(imx_dma_config_2d);
+
+/**
  * imx_dma_setup_handlers - setup i.MX DMA channel end and error notification
  * handlers
  * @channel: i.MX DMA channel number
@@ -732,6 +807,13 @@  void imx_dma_free(int channel)
 		return;
 	}
 
+	spin_lock(&lock_2d);
+	if (imxdma->enabled_2d) {
+		imx_dma_2d_slots[imxdma->slot_2d].count--;
+		imxdma->enabled_2d = false;
+	}
+	spin_unlock(&lock_2d);
+
 	local_irq_save(flags);
 	/* Disable interrupts */
 	imx_dma_disable(channel);
@@ -840,6 +922,10 @@  static int __init imx_dma_init(void)
 		imx_dma_channels[i].dma_num = i;
 	}
 
+	for (i = 0; i < IMX_DMA_2D_SLOTS; i++)
+		imx_dma_2d_slots[i].count = 0;
+	spin_lock_init(&lock_2d);
+
 	return ret;
 }
 
diff --git a/arch/arm/mach-imx/include/mach/dma-v1.h b/arch/arm/mach-imx/include/mach/dma-v1.h
index ac6fd71..bab9183 100644
--- a/arch/arm/mach-imx/include/mach/dma-v1.h
+++ b/arch/arm/mach-imx/include/mach/dma-v1.h
@@ -30,6 +30,10 @@ 
 #include <mach/dma.h>
 
 #define IMX_DMA_CHANNELS  16
+#define IMX_DMA_2D_SLOTS   2
+
+#define IMX_DMA_2D_SLOT_A  0
+#define IMX_DMA_2D_SLOT_B  1
 
 #define DMA_MODE_READ		0
 #define DMA_MODE_WRITE		1
@@ -64,6 +68,9 @@  void
 imx_dma_config_burstlen(int channel, unsigned int burstlen);
 
 int
+imx_dma_config_2d(int channel, unsigned int x, unsigned int y, unsigned int w);
+
+int
 imx_dma_setup_single(int channel, dma_addr_t dma_address,
 		unsigned int dma_length, unsigned int dev_addr,
 		unsigned int dmamode);