RE: [RFC 7/8] TILER-DMM: Main TILER driver implementation.

2010-07-24 Thread Shilimkar, Santosh
 -Original Message-
 From: Sin, David
 Sent: Saturday, July 24, 2010 4:52 AM
 To: linux-arm-ker...@lists.arm.linux.org.uk; linux-omap@vger.kernel.org;
 Tony Lindgren; Russell King
 Cc: Kanigeri, Hari; Ohad Ben-Cohen; Hiremath, Vaibhav; Shilimkar, Santosh;
 Molnar, Lajos; Sin, David
 Subject: [RFC 7/8] TILER-DMM: Main TILER driver implementation.

 From: Lajos Molnar mol...@ti.com

 This patch contains the TILER driver and implementation of the TILER
 block manipulation and mapping functions.

 It also contains the makefile and config file for the TILER driver.

 Signed-off-by: Lajos Molnar mol...@ti.com
 Signed-off-by: David Sin david...@ti.com
 ---
  drivers/media/video/tiler/Kconfig   |   65 +
  drivers/media/video/tiler/Makefile  |7 +
  drivers/media/video/tiler/tiler-iface.c |  106 
  drivers/media/video/tiler/tiler-main.c  |  426
 +++
  4 files changed, 604 insertions(+), 0 deletions(-)
  create mode 100644 drivers/media/video/tiler/Kconfig
  create mode 100644 drivers/media/video/tiler/Makefile
  create mode 100644 drivers/media/video/tiler/tiler-iface.c
  create mode 100644 drivers/media/video/tiler/tiler-main.c

 diff --git a/drivers/media/video/tiler/Kconfig
 b/drivers/media/video/tiler/Kconfig
 new file mode 100644
 index 000..2c61471
 --- /dev/null
 +++ b/drivers/media/video/tiler/Kconfig
 @@ -0,0 +1,65 @@
 +config HAVE_TI_TILER
 +bool
 +default y
 +depends on ARCH_OMAP4
 +
 +menuconfig TI_TILER
 +tristate TI TILER support
 +default y
 +depends on HAVE_TI_TILER
 +help
 +   TILER and TILER-DMM driver for TI chips.  The TI TILER device
 +   enables video rotation on certain TI chips such as OMAP4 or
 +   Netra.  Video rotation will be limited without TILER support.
Sorry but I don't understand the need of above two entries. Should one entry 
not sufficient ?
 +
 +config TILER_GRANULARITY
 +int Allocation granularity (2^n)
 +range 1 4096
 +default 128
 +depends on TI_TILER
 +help
 +   This option sets the default TILER allocation granularity.  It
 can
 +   be overriden by the tiler.grain boot argument.
 +
 +   The allocation granularity is the smallest TILER block size
 (in
 +   bytes) managed distinctly by the TILER driver.  TILER blocks
 of any
 +   size are managed in chunks of at least this size.
 +
 +   Must be a 2^n in the range of 1 to 4096; however, the TILER
 driver
 +   may use a larger supported granularity.
 +
 +   Supported values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512,
 1024,
 +   2048, 4096.
 +
 +config TILER_ALIGNMENT
 +int Allocation alignment (2^n)
 +range 1 4096
 +default 4096
 +depends on TI_TILER
 +help
 +   This option sets the default TILER allocation alignment.  It
 can
 +   be overriden by the tiler.align boot argument.
Do you want this entry since you already support boot-argument ?
 +
 +   Must be a 2^n in the range of 1 to 4096; however, it is
 naturally
 +   aligned to the TILER granularity.
 +
 +   Supported values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512,
 1024,
 +   2048, 4096.
 +
 +config TILER_CACHE_LIMIT
 +int Memory limit to cache free pages in MBytes
 +range 0 128
 +default 40
 +depends on TI_TILER
 +help
 +   This option sets the minimum memory that TILER retains even if
 +   there is less TILER allocated memory is use.  The unused
 memory is
 +   instead stored in a cache to speed up allocation and freeing
 of
 +   physical pages.
 +
 +   This option can be overriden by the tiler.cache boot argument.
 +
 +   While initially TILER will use less memory than this limit
 (0), it
 +   will not release any memory used until it reaches this limit.
 +   Thereafter, TILER will release any unused memory immediately
 as
 +   long as there it is above this threshold.
 diff --git a/drivers/media/video/tiler/Makefile
 b/drivers/media/video/tiler/Makefile
 new file mode 100644
 index 000..4a6495e
 --- /dev/null
 +++ b/drivers/media/video/tiler/Makefile
 @@ -0,0 +1,7 @@
 +obj-$(CONFIG_TI_TILER) += tcm/
 +
 +obj-$(CONFIG_TI_TILER) += tiler.o
 +tiler-objs = tiler-geom.o tiler-main.o tiler-iface.o tmm-pat.o
 +
 +obj-$(CONFIG_TI_TILER) += tiler_dmm.o
 +tiler_dmm-objs = dmm.o
 diff --git a/drivers/media/video/tiler/tiler-iface.c
 b/drivers/media/video/tiler/tiler-iface.c
 new file mode 100644
 index 000..0b10fae
 --- /dev/null
 +++ b/drivers/media/video/tiler/tiler-iface.c
 @@ -0,0 +1,106 @@
 +/*
 + * tiler-iface.c
 + *
 + * TILER driver interace functions for TI TILER hardware block.
 + *
 + * Authors: Lajos Molnar mol...@ti.com
 + *  David Sin david...@ti.com
 + *
 + * Copyright (C) 2009-2010 Texas

Re: [RFC 7/8] TILER-DMM: Main TILER driver implementation.

2010-07-24 Thread Russell King - ARM Linux
On Fri, Jul 23, 2010 at 06:22:27PM -0500, David Sin wrote:
 +struct platform_driver tiler_driver_ldm = {
 + .driver = {
 + .owner = THIS_MODULE,
 + .name = tiler,
 + },
 + .probe = NULL,
 + .shutdown = NULL,
 + .remove = NULL,
 +};

What's the purpose of this apparantly stub driver?

 +static s32 refill_pat(struct tmm *tmm, struct tcm_area *area, u32 *ptr)
 +{
 + s32 res = 0;
 + struct pat_area p_area = {0};
 +
 + p_area.x0 = area-p0.x;
 + p_area.y0 = area-p0.y;
 + p_area.x1 = area-p1.x;
 + p_area.y1 = area-p1.y;
 +
 + memcpy(dmac_va, ptr, sizeof(*ptr) * tcm_sizeof(*area));
 +
 + if (tmm_map(tmm, p_area, dmac_pa))
 + res = -EFAULT;

What's wrong with making tmm_map return an error code and propagating it?
This can be a simple:

return tmm_map(tmm, p_area, dmac_pa);

In any case, I'm not sure I like all this layered code - it makes it much
harder to follow what's going on when you have to look at multiple files
to find out what X does.

If tmm_map() is only used here, move the contents of tmm_map here.

 +/* verify input params and calculate tiler container params for a block */
 +static s32 __analize_area(enum tiler_fmt fmt, u32 width, u32 height,
 +   u16 *x_area, u16 *y_area, u16 *align, u16 *offs)

Analyze is spelt like that, not with an i.

 +/* driver initialization */
 +static s32 __init tiler_init(void)
 +{
 + s32 r = -1;
 + struct tcm *sita = NULL;
 + struct tmm *tmm_pat = NULL;
 +
 + tiler_geom_init(tiler);
 +
 + /* check module parameters for correctness */
 + if (default_align  PAGE_SIZE ||
 + default_align  (default_align - 1) ||
 + granularity  1 || granularity  PAGE_SIZE ||
 + granularity  (granularity - 1))
 + return -EINVAL;
 +
 + /*
 +  * Array of physical pages for PAT programming, which must be a 16-byte
 +  * aligned physical address.
 +  */
 + dmac_va = dma_alloc_coherent(NULL, tiler.width * tiler.height *
 + sizeof(*dmac_va), dmac_pa, GFP_ATOMIC);

Why GFP_ATOMIC?  Surely GFP_KERNEL will work here?

What if there ends up being more than one tiler at some point in the future
and you need two of these?  Shouldn't this be in some driver probe function
so you have a struct device to use with it?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 7/8] TILER-DMM: Main TILER driver implementation.

2010-07-24 Thread Hari Kanigeri
 +s32 tiler_mmap_blk(struct tiler_block_t *blk, u32 offs, u32 size,
 +                             struct vm_area_struct *vma, u32 voffs)
 +{
 +     u32 v, p, len;
 +
 +     /* don't allow mremap */
 +     vma-vm_flags |= VM_DONTEXPAND | VM_RESERVED;
 Should we add  VM_LOCKED as well considering the page swapping  ?

-- VM_RESERVED serves this purpose right ?

 +
 +     /* mapping must fit into vma */
 +     BUG_ON(vma-vm_start  vma-vm_start + voffs ||
 +             vma-vm_start + voffs  vma-vm_start + voffs + size ||
 +             vma-vm_start + voffs + size  vma-vm_end);

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


RE: [RFC 7/8] TILER-DMM: Main TILER driver implementation.

2010-07-24 Thread Shilimkar, Santosh
 -Original Message-
 From: Hari Kanigeri [mailto:hari.kanig...@gmail.com]
 Sent: Saturday, July 24, 2010 7:12 PM
 To: Shilimkar, Santosh
 Cc: Sin, David; linux-arm-ker...@lists.arm.linux.org.uk; linux-
 o...@vger.kernel.org; Tony Lindgren; Russell King; Kanigeri, Hari; Ohad
 Ben-Cohen; Hiremath, Vaibhav; Molnar, Lajos
 Subject: Re: [RFC 7/8] TILER-DMM: Main TILER driver implementation.
 
  +s32 tiler_mmap_blk(struct tiler_block_t *blk, u32 offs, u32 size,
  +                             struct vm_area_struct *vma, u32 voffs)
  +{
  +     u32 v, p, len;
  +
  +     /* don't allow mremap */
  +     vma-vm_flags |= VM_DONTEXPAND | VM_RESERVED;
  Should we add  VM_LOCKED as well considering the page swapping  ?
 
 -- VM_RESERVED serves this purpose right ?

I think you are right.

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


[RFC 7/8] TILER-DMM: Main TILER driver implementation.

2010-07-23 Thread David Sin
From: Lajos Molnar mol...@ti.com

This patch contains the TILER driver and implementation of the TILER
block manipulation and mapping functions.

It also contains the makefile and config file for the TILER driver.

Signed-off-by: Lajos Molnar mol...@ti.com
Signed-off-by: David Sin david...@ti.com
---
 drivers/media/video/tiler/Kconfig   |   65 +
 drivers/media/video/tiler/Makefile  |7 +
 drivers/media/video/tiler/tiler-iface.c |  106 
 drivers/media/video/tiler/tiler-main.c  |  426 +++
 4 files changed, 604 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/tiler/Kconfig
 create mode 100644 drivers/media/video/tiler/Makefile
 create mode 100644 drivers/media/video/tiler/tiler-iface.c
 create mode 100644 drivers/media/video/tiler/tiler-main.c

diff --git a/drivers/media/video/tiler/Kconfig 
b/drivers/media/video/tiler/Kconfig
new file mode 100644
index 000..2c61471
--- /dev/null
+++ b/drivers/media/video/tiler/Kconfig
@@ -0,0 +1,65 @@
+config HAVE_TI_TILER
+bool
+default y
+depends on ARCH_OMAP4
+
+menuconfig TI_TILER
+tristate TI TILER support
+default y
+depends on HAVE_TI_TILER
+help
+   TILER and TILER-DMM driver for TI chips.  The TI TILER device
+   enables video rotation on certain TI chips such as OMAP4 or
+   Netra.  Video rotation will be limited without TILER support.
+
+config TILER_GRANULARITY
+int Allocation granularity (2^n)
+range 1 4096
+default 128
+depends on TI_TILER
+help
+   This option sets the default TILER allocation granularity.  It can
+   be overriden by the tiler.grain boot argument.
+
+   The allocation granularity is the smallest TILER block size (in
+   bytes) managed distinctly by the TILER driver.  TILER blocks of any
+   size are managed in chunks of at least this size.
+
+   Must be a 2^n in the range of 1 to 4096; however, the TILER driver
+   may use a larger supported granularity.
+
+   Supported values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
+   2048, 4096.
+
+config TILER_ALIGNMENT
+int Allocation alignment (2^n)
+range 1 4096
+default 4096
+depends on TI_TILER
+help
+   This option sets the default TILER allocation alignment.  It can
+   be overriden by the tiler.align boot argument.
+
+   Must be a 2^n in the range of 1 to 4096; however, it is naturally
+   aligned to the TILER granularity.
+
+   Supported values are: 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024,
+   2048, 4096.
+
+config TILER_CACHE_LIMIT
+int Memory limit to cache free pages in MBytes
+range 0 128
+default 40
+depends on TI_TILER
+help
+   This option sets the minimum memory that TILER retains even if
+   there is less TILER allocated memory is use.  The unused memory is
+   instead stored in a cache to speed up allocation and freeing of
+   physical pages.
+
+   This option can be overriden by the tiler.cache boot argument.
+
+   While initially TILER will use less memory than this limit (0), it
+   will not release any memory used until it reaches this limit.
+   Thereafter, TILER will release any unused memory immediately as
+   long as there it is above this threshold.
diff --git a/drivers/media/video/tiler/Makefile 
b/drivers/media/video/tiler/Makefile
new file mode 100644
index 000..4a6495e
--- /dev/null
+++ b/drivers/media/video/tiler/Makefile
@@ -0,0 +1,7 @@
+obj-$(CONFIG_TI_TILER) += tcm/
+
+obj-$(CONFIG_TI_TILER) += tiler.o
+tiler-objs = tiler-geom.o tiler-main.o tiler-iface.o tmm-pat.o
+
+obj-$(CONFIG_TI_TILER) += tiler_dmm.o
+tiler_dmm-objs = dmm.o
diff --git a/drivers/media/video/tiler/tiler-iface.c 
b/drivers/media/video/tiler/tiler-iface.c
new file mode 100644
index 000..0b10fae
--- /dev/null
+++ b/drivers/media/video/tiler/tiler-iface.c
@@ -0,0 +1,106 @@
+/*
+ * tiler-iface.c
+ *
+ * TILER driver interace functions for TI TILER hardware block.
+ *
+ * Authors: Lajos Molnar mol...@ti.com
+ *  David Sin david...@ti.com
+ *
+ * Copyright (C) 2009-2010 Texas Instruments, Inc.
+ *
+ * This package is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#include linux/module.h
+#include linux/slab.h/* kmalloc */
+#include linux/mm.h
+#include linux/mm_types.h
+#include asm/mach/map.h  /* for ioremap_page */
+
+#include _tiler.h
+
+/*
+ *  Memory-Map Kernel