[PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Prathyush K]

The dma-mapping framework needs a IOMMU mapping to be created for the
device which allocates/maps/frees the non-contig buffer. In the DRM
framework, a gem buffer is created by the DRM virtual device and not
directly by any of the physical devices (FIMD, HDMI etc). Each gem object
can be set as a framebuffer to one or many of the drm devices. So a gem
object cannot be allocated for any one device. All the DRM devices should
be able to access this buffer.

The proposed method is to create a common IOMMU mapping during drm init. This
mapping is then attached to all of the drm devices including the drm device.
[PATCH 1/7] drm/exynos: create common IOMMU mapping for DRM

During the probe of drm fimd, the driver retrieves a 'sysmmu' field
in the device node for fimd. If such a field exists, the driver retrieves the
platform device of the sysmmu device. This sysmmu is set as the sysmmu
for fimd. The common mapping created is then attached to fimd.
This needs to be done for all the other devices (hdmi, vidi etc).
[PATCH 2/7] ARM: EXYNOS5: add sysmmu field to fimd device node
[PATCH 3/7] drm/exynos: add IOMMU support to drm fimd

During DRM's probe which happens last, the common mapping is set to its archdata
and iommu ops are set as its dma ops. This requires a modification in the
dma-mapping framework so that the iommu ops can be visible to all drivers.
[PATCH 4/7] ARM: dma-mapping: rename and export iommu_ops
[PATCH 5/7] drm/exynos: attach drm device with common drm mapping

Currently allocation and free use the iommu framework by calling
dma_alloc_writecombine and dma_free_writecombine respectively.
For mapping the buffers to user space, the mmap functions assume that
the buffer is contiguous. This is modified by calling dma_mmap_writecombine.
[PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function
[PATCH 7/7] Add IOMMU support for mapping gem object

The device tree based patches are based on Leela's patch which was posted
last week for adding DT support to DRM FIMD. The patch to add sysmmu
field is for reference only and will be posted to the device tree
mailing list. Same with the rename and export iommu_ops patch.

These patches are tested on Exynos5250 SMDK board and tested with modetest
from libdrm tests.

Prathyush K (7):
  drm/exynos: create common IOMMU mapping for DRM
  ARM: EXYNOS5: add sysmmu field to fimd device node
  drm/exynos: add IOMMU support to drm fimd
  ARM: dma-mapping: rename and export iommu_ops
  drm/exynos: attach drm device with common drm mapping
  drm/exynos: Add exynos drm specific fb_mmap function
  drm/exynos: Add IOMMU support for mapping gem object

 arch/arm/boot/dts/exynos5250.dtsi         |    1 +
 arch/arm/include/asm/dma-mapping.h        |    1 +
 arch/arm/mm/dma-mapping.c                 |    5 ++-
 drivers/gpu/drm/exynos/exynos_drm_core.c  |    3 ++
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |   30 ++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |   10 +++++
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++
 drivers/gpu/drm/exynos/exynos_drm_fimd.c  |   54 ++++++++++++++++++++++++++++-
 drivers/gpu/drm/exynos/exynos_drm_gem.c   |   35 ++++++++----------
 9 files changed, 133 insertions(+), 22 deletions(-)

[PATCH 1/7] drm/exynos: create common IOMMU mapping for DRM

[Prathyush K]

This patch creates an IOMMU mapping during drm init. This is used by
all the drm devices including the exynos drm virtual device. This
ensures that when drm creates a buffer using the dma-mapping framework,
this buffer is accessible by all the drm devices like hdmi/fimd.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_core.c |    3 +++
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |   21 +++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |   10 ++++++++++
 3 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c
index eaf630d..13ecca6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_core.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_core.c
@@ -34,6 +34,9 @@
 
 static LIST_HEAD(exynos_drm_subdrv_list);
 static struct drm_device *drm_dev;
+#ifdef CONFIG_EXYNOS_IOMMU
+struct dma_iommu_mapping *exynos_drm_common_mapping;
+#endif
 
 static int exynos_drm_subdrv_probe(struct drm_device *dev,
 					struct exynos_drm_subdrv *subdrv)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index d6de2e0..c3ad87e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -307,6 +307,18 @@ static int __init exynos_drm_init(void)
 
 	DRM_DEBUG_DRIVER("%s\n", __FILE__);
 
+#ifdef CONFIG_EXYNOS_IOMMU
+	exynos_drm_common_mapping = arm_iommu_create_mapping(&platform_bus_type,
+					EXYNOS_DRM_DMA_ADDR,
+					EXYNOS_DRM_IOMMU_SIZE,
+					EXYNOS_DRM_IOMMU_ORDER);
+	if (IS_ERR_OR_NULL(exynos_drm_common_mapping)) {
+		DRM_ERROR("failed to created IOMMU mapping\n");
+		ret = PTR_ERR(exynos_drm_common_mapping);
+		goto out_iommu;
+	}
+#endif
+
 #ifdef CONFIG_DRM_EXYNOS_FIMD
 	ret = platform_driver_register(&fimd_driver);
 	if (ret < 0)
@@ -367,6 +379,11 @@ out_hdmi:
 	platform_driver_unregister(&fimd_driver);
 out_fimd:
 #endif
+#ifdef CONFIG_EXYNOS_IOMMU
+	arm_iommu_release_mapping(exynos_drm_common_mapping);
+	exynos_drm_common_mapping = NULL;
+out_iommu:
+#endif
 	return ret;
 }
 
@@ -393,6 +410,10 @@ static void __exit exynos_drm_exit(void)
 #ifdef CONFIG_DRM_EXYNOS_FIMD
 	platform_driver_unregister(&fimd_driver);
 #endif
+#ifdef CONFIG_EXYNOS_IOMMU
+	arm_iommu_release_mapping(exynos_drm_common_mapping);
+	exynos_drm_common_mapping = NULL;
+#endif
 }
 
 module_init(exynos_drm_init);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index c82c90c..bd12ee2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -31,11 +31,18 @@
 
 #include <linux/module.h>
 #include "drm.h"
+#ifdef CONFIG_EXYNOS_IOMMU
+#include <asm/dma-iommu.h>
+#include <mach/sysmmu.h>
+#endif
 
 #define MAX_CRTC	3
 #define MAX_PLANE	5
 #define MAX_FB_BUFFER	4
 #define DEFAULT_ZPOS	-1
+#define EXYNOS_DRM_DMA_ADDR	0x20000000
+#define EXYNOS_DRM_IOMMU_SIZE	SZ_128M
+#define EXYNOS_DRM_IOMMU_ORDER	4
 
 struct drm_device;
 struct exynos_drm_overlay;
@@ -304,4 +311,7 @@ extern struct platform_driver mixer_driver;
 extern struct platform_driver exynos_drm_common_hdmi_driver;
 extern struct platform_driver vidi_driver;
 extern struct platform_driver g2d_driver;
+#ifdef CONFIG_EXYNOS_IOMMU
+extern struct dma_iommu_mapping *exynos_drm_common_mapping;
+#endif
 #endif
-- 
1.7.0.4

[PATCH 2/7] ARM: EXYNOS5: add sysmmu field to fimd device node

[Prathyush K]

This patch adds the sysmmu field to the fimd device node in the
exynos5250 device tree. This field is used to link the parent device
and its sysmmu device in the device tree.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 3e85b2b..2c7ce61 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -461,6 +461,7 @@
 		interrupt-parent = <&combiner>;
 		reg = <0x14400000 0x40000>;
 		interrupts = <18 5>, <18 4>, <18 6>;
+		sysmmu = <&sysmmu_11>;
 	};
 
 	mipi {
-- 
1.7.0.4

[PATCH 3/7] drm/exynos: add IOMMU support to drm fimd

[Prathyush K]

This patch adds device tree based IOMMU support to DRM FIMD. During
probe, the driver searches for a 'sysmmu' field in the device node. The
sysmmu field points to the corresponding sysmmu device of fimd.
This sysmmu device is retrieved and set as fimd's sysmmu. The common
IOMMU mapping created during DRM init is then attached to drm fimd.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |   54 +++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 15b5286..6d4048a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -19,7 +19,7 @@
 #include <linux/clk.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
-
+#include <linux/of_platform.h>
 #include <drm/exynos_drm.h>
 #include <plat/regs-fb-v4.h>
 
@@ -790,12 +790,56 @@ static int fimd_power_on(struct fimd_context *ctx, bool enable)
 }
 
 #ifdef CONFIG_OF
+
+#ifdef CONFIG_EXYNOS_IOMMU
+static int iommu_init(struct device *dev)
+{
+	struct platform_device *pds;
+	struct device_node *dn, *dns;
+	const __be32 *parp;
+	int ret;
+
+	dn = dev->of_node;
+	parp = of_get_property(dn, "sysmmu", NULL);
+	if (parp == NULL) {
+		dev_err(dev, "failed to find sysmmu property\n");
+		return -EINVAL;
+	}
+	dns = of_find_node_by_phandle(be32_to_cpup(parp));
+	if (dns == NULL) {
+		dev_err(dev, "failed to find sysmmu node\n");
+		return -EINVAL;
+	}
+	pds = of_find_device_by_node(dns);
+	if (pds == NULL) {
+		dev_err(dev, "failed to find sysmmu platform device\n");
+		return -EINVAL;
+	}
+
+	platform_set_sysmmu(&pds->dev, dev);
+	dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
+	if (!dev->dma_parms) {
+		dev_err(dev, "failed to allocate dma parms\n");
+		return -ENOMEM;
+	}
+	dma_set_max_seg_size(dev, 0xffffffffu);
+
+	ret = arm_iommu_attach_device(dev, exynos_drm_common_mapping);
+	if (ret) {
+		dev_err(dev, "failed to attach device\n");
+		return ret;
+	}
+	return 0;
+}
+#endif
+
 static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct device_node *disp_np;
 	struct exynos_drm_fimd_pdata *pd;
 	u32 data[4];
+	int ret;
 
 	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
 	if (!pd) {
@@ -803,6 +847,14 @@ static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device *dev)
 		return ERR_PTR(-ENOMEM);
 	}
 
+#ifdef CONFIG_EXYNOS_IOMMU
+	ret = iommu_init(dev);
+	if (ret) {
+		dev_err(dev, "failed to initialize iommu\n");
+		return ERR_PTR(ret);
+	}
+#endif
+
 	if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
 		pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
 	if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
-- 
1.7.0.4

[PATCH 4/7] ARM: dma-mapping: rename and export iommu_ops

[Prathyush K]

This patch renames the dma_ops structure for arm from iommu_ops
to arm_iommu_ops. This structure is also exported and declared
extern so that it can be set as any device's dma ops directly.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 arch/arm/include/asm/dma-mapping.h |    1 +
 arch/arm/mm/dma-mapping.c          |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index bbef15d..5957357 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -13,6 +13,7 @@
 
 #define DMA_ERROR_CODE	(~0)
 extern struct dma_map_ops arm_dma_ops;
+extern struct dma_map_ops arm_iommu_ops;
 
 static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 {
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 0f9358b..cff13ac 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1567,7 +1567,7 @@ static void arm_iommu_sync_single_for_device(struct device *dev,
 	__dma_page_cpu_to_dev(page, offset, size, dir);
 }
 
-struct dma_map_ops iommu_ops = {
+struct dma_map_ops arm_iommu_ops = {
 	.alloc		= arm_iommu_alloc_attrs,
 	.free		= arm_iommu_free_attrs,
 	.mmap		= arm_iommu_mmap_attrs,
@@ -1582,6 +1582,7 @@ struct dma_map_ops iommu_ops = {
 	.sync_sg_for_cpu	= arm_iommu_sync_sg_for_cpu,
 	.sync_sg_for_device	= arm_iommu_sync_sg_for_device,
 };
+EXPORT_SYMBOL(arm_iommu_ops);
 
 /**
  * arm_iommu_create_mapping
@@ -1674,7 +1675,7 @@ int arm_iommu_attach_device(struct device *dev,
 
 	kref_get(&mapping->kref);
 	dev->archdata.mapping = mapping;
-	set_dma_ops(dev, &iommu_ops);
+	set_dma_ops(dev, &arm_iommu_ops);
 
 	pr_info("Attached IOMMU controller to %s device.\n", dev_name(dev));
 	return 0;
-- 
1.7.0.4

[PATCH 5/7] drm/exynos: attach drm device with common drm mapping

[Prathyush K]

This patch sets the common mapping created during drm init, to the
drm device's archdata. The dma_ops of drm device is set as arm_iommu_ops.
The common mapping is shared across all the drm devices which ensures
that any buffer allocated with drm is accessible by drm-fimd or drm-hdmi
or both.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index c3ad87e..2e40ca8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -276,6 +276,15 @@ static struct drm_driver exynos_drm_driver = {
 
 static int exynos_drm_platform_probe(struct platform_device *pdev)
 {
+#ifdef CONFIG_EXYNOS_IOMMU
+	struct device *dev = &pdev->dev;
+
+	kref_get(&exynos_drm_common_mapping->kref);
+	dev->archdata.mapping = exynos_drm_common_mapping;
+	set_dma_ops(dev, &arm_iommu_ops);
+
+	DRM_INFO("drm common mapping set to drm device.\n");
+#endif
 	DRM_DEBUG_DRIVER("%s\n", __FILE__);
 
 	exynos_drm_driver.num_ioctls = DRM_ARRAY_SIZE(exynos_ioctls);
-- 
1.7.0.4

[PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function

[Prathyush K]

This patch adds a exynos drm specific implementation of fb_mmap
which supports mapping a non-contiguous buffer to user space.
This new function does not assume that the frame buffer is contiguous
and calls dma_mmap_writecombine for mapping the buffer to user space.
dma_mmap_writecombine will be able to map a contiguous buffer as well
as non-contig buffer depending on whether an IOMMU mapping is created
for drm or not.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index d5586cc..b53e638 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -46,8 +46,24 @@ struct exynos_drm_fbdev {
 	struct exynos_drm_gem_obj	*exynos_gem_obj;
 };
 
+static int exynos_drm_fb_mmap(struct fb_info *info,
+			      struct vm_area_struct *vma)
+{
+	if ((vma->vm_end - vma->vm_start) > info->fix.smem_len)
+		return -EINVAL;
+
+	vma->vm_pgoff = 0;
+	vma->vm_flags |= VM_IO | VM_RESERVED;
+	if (dma_mmap_writecombine(info->device, vma, info->screen_base,
+			info->fix.smem_start, vma->vm_end - vma->vm_start))
+		return -EAGAIN;
+
+	return 0;
+}
+
 static struct fb_ops exynos_drm_fb_ops = {
 	.owner		= THIS_MODULE,
+	.fb_mmap	= exynos_drm_fb_mmap,
 	.fb_fillrect	= cfb_fillrect,
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
-- 
1.7.0.4

[PATCH 7/7] drm/exynos: Add IOMMU support for mapping gem object

[Prathyush K]

A gem object is created using dma_alloc_writecombine. Currently, this
buffer is assumed to be contiguous. If a IOMMU mapping is created for
DRM, this buffer would be non-contig so the map functions are modified
to call dma_mmap_writecombine. This works for both contig and non-contig
buffers.

Signed-off-by: Prathyush K <prathyush.k@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c |   35 ++++++++++++++----------------
 1 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 5c8b683..59240f7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -162,17 +162,22 @@ static int exynos_drm_gem_map_pages(struct drm_gem_object *obj,
 {
 	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
 	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
-	unsigned long pfn;
 
 	if (exynos_gem_obj->flags & EXYNOS_BO_NONCONTIG) {
+		unsigned long pfn;
 		if (!buf->pages)
 			return -EINTR;
 
 		pfn = page_to_pfn(buf->pages[page_offset++]);
-	} else
-		pfn = (buf->dma_addr >> PAGE_SHIFT) + page_offset;
-
-	return vm_insert_mixed(vma, f_vaddr, pfn);
+		return vm_insert_mixed(vma, f_vaddr, pfn);
+	} else {
+		int ret;
+		ret = dma_mmap_writecombine(obj->dev->dev, vma, buf->kvaddr,
+					buf->dma_addr, buf->size);
+		if (ret)
+			DRM_ERROR("dma_mmap_writecombine failed\n");
+		return ret;
+	}
 }
 
 static int exynos_drm_gem_get_pages(struct drm_gem_object *obj)
@@ -503,7 +508,7 @@ static int exynos_drm_gem_mmap_buffer(struct file *filp,
 	struct drm_gem_object *obj = filp->private_data;
 	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
 	struct exynos_drm_gem_buf *buffer;
-	unsigned long pfn, vm_size, usize, uaddr = vma->vm_start;
+	unsigned long vm_size, usize, uaddr = vma->vm_start;
 	int ret;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
@@ -543,19 +548,11 @@ static int exynos_drm_gem_mmap_buffer(struct file *filp,
 			usize -= PAGE_SIZE;
 		} while (usize > 0);
 	} else {
-		/*
-		 * get page frame number to physical memory to be mapped
-		 * to user space.
-		 */
-		pfn = ((unsigned long)exynos_gem_obj->buffer->dma_addr) >>
-								PAGE_SHIFT;
-
-		DRM_DEBUG_KMS("pfn = 0x%lx\n", pfn);
-
-		if (remap_pfn_range(vma, vma->vm_start, pfn, vm_size,
-					vma->vm_page_prot)) {
-			DRM_ERROR("failed to remap pfn range.\n");
-			return -EAGAIN;
+		ret = dma_mmap_writecombine(obj->dev->dev, vma, buffer->kvaddr,
+					buffer->dma_addr, buffer->size);
+		if (ret) {
+			DRM_ERROR("dma_mmap_writecombine failed\n");
+			return ret;
 		}
 	}
 
-- 
1.7.0.4

RE: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Inki Dae]

> -----Original Message-----
> From: Prathyush K [mailto:prathyush.k@samsung.com]
> Sent: Wednesday, July 11, 2012 6:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
inki.dae@samsung.com;
> subash.ramaswamy@linaro.org
> Subject: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> 
> The dma-mapping framework needs a IOMMU mapping to be created for the
> device which allocates/maps/frees the non-contig buffer. In the DRM
> framework, a gem buffer is created by the DRM virtual device and not
> directly by any of the physical devices (FIMD, HDMI etc). Each gem object
> can be set as a framebuffer to one or many of the drm devices. So a gem
> object cannot be allocated for any one device. All the DRM devices should
> be able to access this buffer.
>

It's good to use unified iommu table so I agree to your opinion but we don't
decide whether we use dma mapping api or not. now dma mapping api has one
issue.
in case of using iommu with dma mapping api, we couldn't use physically
contiguous memory region with iommu. for this, there is a case that we
should use physically contiguous memory region with iommu. it is because we
sometime may use mfc(hw video codec) with secure zone such as ARM TrustZone.
Then, it needs physically contiguous memory region.

Thanks,
Inki Dae

> The proposed method is to create a common IOMMU mapping during drm init.
> This
> mapping is then attached to all of the drm devices including the drm
> device.
> [PATCH 1/7] drm/exynos: create common IOMMU mapping for DRM
> 
> During the probe of drm fimd, the driver retrieves a 'sysmmu' field
> in the device node for fimd. If such a field exists, the driver retrieves
> the
> platform device of the sysmmu device. This sysmmu is set as the sysmmu
> for fimd. The common mapping created is then attached to fimd.
> This needs to be done for all the other devices (hdmi, vidi etc).
> [PATCH 2/7] ARM: EXYNOS5: add sysmmu field to fimd device node
> [PATCH 3/7] drm/exynos: add IOMMU support to drm fimd
> 
> During DRM's probe which happens last, the common mapping is set to its
> archdata
> and iommu ops are set as its dma ops. This requires a modification in the
> dma-mapping framework so that the iommu ops can be visible to all drivers.
> [PATCH 4/7] ARM: dma-mapping: rename and export iommu_ops
> [PATCH 5/7] drm/exynos: attach drm device with common drm mapping
> 
> Currently allocation and free use the iommu framework by calling
> dma_alloc_writecombine and dma_free_writecombine respectively.
> For mapping the buffers to user space, the mmap functions assume that
> the buffer is contiguous. This is modified by calling
> dma_mmap_writecombine.
> [PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function
> [PATCH 7/7] Add IOMMU support for mapping gem object
> 
> The device tree based patches are based on Leela's patch which was posted
> last week for adding DT support to DRM FIMD. The patch to add sysmmu
> field is for reference only and will be posted to the device tree
> mailing list. Same with the rename and export iommu_ops patch.
> 
> These patches are tested on Exynos5250 SMDK board and tested with modetest
> from libdrm tests.
> 
> Prathyush K (7):
>   drm/exynos: create common IOMMU mapping for DRM
>   ARM: EXYNOS5: add sysmmu field to fimd device node
>   drm/exynos: add IOMMU support to drm fimd
>   ARM: dma-mapping: rename and export iommu_ops
>   drm/exynos: attach drm device with common drm mapping
>   drm/exynos: Add exynos drm specific fb_mmap function
>   drm/exynos: Add IOMMU support for mapping gem object
> 
>  arch/arm/boot/dts/exynos5250.dtsi         |    1 +
>  arch/arm/include/asm/dma-mapping.h        |    1 +
>  arch/arm/mm/dma-mapping.c                 |    5 ++-
>  drivers/gpu/drm/exynos/exynos_drm_core.c  |    3 ++
>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |   30 ++++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_drv.h   |   10 +++++
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c  |   54
> ++++++++++++++++++++++++++++-
>  drivers/gpu/drm/exynos/exynos_drm_gem.c   |   35 ++++++++----------
>  9 files changed, 133 insertions(+), 22 deletions(-)

RE: [PATCH 3/7] drm/exynos: add IOMMU support to drm fimd

[Inki Dae]

> -----Original Message-----
> From: Prathyush K [mailto:prathyush.k@samsung.com]
> Sent: Wednesday, July 11, 2012 6:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
inki.dae@samsung.com;
> subash.ramaswamy@linaro.org
> Subject: [PATCH 3/7] drm/exynos: add IOMMU support to drm fimd
> 
> This patch adds device tree based IOMMU support to DRM FIMD. During
> probe, the driver searches for a 'sysmmu' field in the device node. The
> sysmmu field points to the corresponding sysmmu device of fimd.
> This sysmmu device is retrieved and set as fimd's sysmmu. The common
> IOMMU mapping created during DRM init is then attached to drm fimd.
> 
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   54
> +++++++++++++++++++++++++++++-
>  1 files changed, 53 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 15b5286..6d4048a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -19,7 +19,7 @@
>  #include <linux/clk.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/of.h>
> -
> +#include <linux/of_platform.h>
>  #include <drm/exynos_drm.h>
>  #include <plat/regs-fb-v4.h>
> 
> @@ -790,12 +790,56 @@ static int fimd_power_on(struct fimd_context *ctx,
> bool enable)
>  }
> 
>  #ifdef CONFIG_OF
> +
> +#ifdef CONFIG_EXYNOS_IOMMU
> +static int iommu_init(struct device *dev)
> +{
> +	struct platform_device *pds;
> +	struct device_node *dn, *dns;
> +	const __be32 *parp;
> +	int ret;
> +
> +	dn = dev->of_node;
> +	parp = of_get_property(dn, "sysmmu", NULL);
> +	if (parp == NULL) {
> +		dev_err(dev, "failed to find sysmmu property\n");
> +		return -EINVAL;
> +	}
> +	dns = of_find_node_by_phandle(be32_to_cpup(parp));
> +	if (dns == NULL) {
> +		dev_err(dev, "failed to find sysmmu node\n");
> +		return -EINVAL;
> +	}
> +	pds = of_find_device_by_node(dns);
> +	if (pds == NULL) {
> +		dev_err(dev, "failed to find sysmmu platform device\n");
> +		return -EINVAL;
> +	}
> +
> +	platform_set_sysmmu(&pds->dev, dev);
> +	dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> +	if (!dev->dma_parms) {
> +		dev_err(dev, "failed to allocate dma parms\n");
> +		return -ENOMEM;
> +	}
> +	dma_set_max_seg_size(dev, 0xffffffffu);
> +
> +	ret = arm_iommu_attach_device(dev, exynos_drm_common_mapping);

where is exynos_drm_common_mapping declared? you can get this point using
exynos_drm_private structure.


> +	if (ret) {
> +		dev_err(dev, "failed to attach device\n");
> +		return ret;
> +	}
> +	return 0;
> +}
> +#endif
> +

with your patch, we can use iommu feature only with device tree. I think
iommu feature should be used commonly.


>  static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct
> device *dev)
>  {
>  	struct device_node *np = dev->of_node;
>  	struct device_node *disp_np;
>  	struct exynos_drm_fimd_pdata *pd;
>  	u32 data[4];
> +	int ret;
> 
>  	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>  	if (!pd) {
> @@ -803,6 +847,14 @@ static struct exynos_drm_fimd_pdata
> *drm_fimd_dt_parse_pdata(struct device *dev)
>  		return ERR_PTR(-ENOMEM);
>  	}
> 
> +#ifdef CONFIG_EXYNOS_IOMMU

and please avoid such #ifdef in device driver.

> +	ret = iommu_init(dev);
> +	if (ret) {
> +		dev_err(dev, "failed to initialize iommu\n");
> +		return ERR_PTR(ret);
> +	}
> +#endif
> +
>  	if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
>  		pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
>  	if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
> --
> 1.7.0.4

RE: [PATCH 5/7] drm/exynos: attach drm device with common drm mapping

[Inki Dae]

> -----Original Message-----
> From: Prathyush K [mailto:prathyush.k@samsung.com]
> Sent: Wednesday, July 11, 2012 6:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
inki.dae@samsung.com;
> subash.ramaswamy@linaro.org
> Subject: [PATCH 5/7] drm/exynos: attach drm device with common drm mapping
> 
> This patch sets the common mapping created during drm init, to the
> drm device's archdata. The dma_ops of drm device is set as arm_iommu_ops.
> The common mapping is shared across all the drm devices which ensures
> that any buffer allocated with drm is accessible by drm-fimd or drm-hdmi
> or both.
> 
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index c3ad87e..2e40ca8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -276,6 +276,15 @@ static struct drm_driver exynos_drm_driver = {
> 
>  static int exynos_drm_platform_probe(struct platform_device *pdev)
>  {
> +#ifdef CONFIG_EXYNOS_IOMMU
> +	struct device *dev = &pdev->dev;
> +
> +	kref_get(&exynos_drm_common_mapping->kref);
> +	dev->archdata.mapping = exynos_drm_common_mapping;

Ok, exynos_drm_common_mapping is shared with drivers using
dev->archdata.mapping

> +	set_dma_ops(dev, &arm_iommu_ops);
> +
> +	DRM_INFO("drm common mapping set to drm device.\n");
> +#endif
>  	DRM_DEBUG_DRIVER("%s\n", __FILE__);
> 
>  	exynos_drm_driver.num_ioctls = DRM_ARRAY_SIZE(exynos_ioctls);
> --
> 1.7.0.4

Re: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Subash Patel]

On 07/13/2012 12:09 PM, Inki Dae wrote:
>
>> -----Original Message-----
>> From: Prathyush K [mailto:prathyush.k@samsung.com]
>> Sent: Wednesday, July 11, 2012 6:40 PM
>> To: dri-devel@lists.freedesktop.org
>> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
> inki.dae@samsung.com;
>> subash.ramaswamy@linaro.org
>> Subject: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
>>
>> The dma-mapping framework needs a IOMMU mapping to be created for the
>> device which allocates/maps/frees the non-contig buffer. In the DRM
>> framework, a gem buffer is created by the DRM virtual device and not
>> directly by any of the physical devices (FIMD, HDMI etc). Each gem object
>> can be set as a framebuffer to one or many of the drm devices. So a gem
>> object cannot be allocated for any one device. All the DRM devices should
>> be able to access this buffer.
>>
> It's good to use unified iommu table so I agree to your opinion but we don't
> decide whether we use dma mapping api or not. now dma mapping api has one
> issue.
> in case of using iommu with dma mapping api, we couldn't use physically
> contiguous memory region with iommu. for this, there is a case that we
> should use physically contiguous memory region with iommu. it is because we
> sometime may use mfc(hw video codec) with secure zone such as ARM TrustZone.
> Then, it needs physically contiguous memory region.
>
> Thanks,
> Inki Dae
I agree. In the mainline code, as of now only the arm_dma_ops has the 
support allocating
from the CMA. But in the function arm_iommu_alloc_attrs(), there is no 
way to know if the
device had declared a contiguous memory range. The reason, we don't 
store that cookie
into the device during the dma_declare_contiguous(). So is it advisable 
to store such information
like mapping(in the iommu operations) in the device.archdata?

Regards,
Subash
>
>> The proposed method is to create a common IOMMU mapping during drm init.
>> This
>> mapping is then attached to all of the drm devices including the drm
>> device.
>> [PATCH 1/7] drm/exynos: create common IOMMU mapping for DRM
>>
>> During the probe of drm fimd, the driver retrieves a 'sysmmu' field
>> in the device node for fimd. If such a field exists, the driver retrieves
>> the
>> platform device of the sysmmu device. This sysmmu is set as the sysmmu
>> for fimd. The common mapping created is then attached to fimd.
>> This needs to be done for all the other devices (hdmi, vidi etc).
>> [PATCH 2/7] ARM: EXYNOS5: add sysmmu field to fimd device node
>> [PATCH 3/7] drm/exynos: add IOMMU support to drm fimd
>>
>> During DRM's probe which happens last, the common mapping is set to its
>> archdata
>> and iommu ops are set as its dma ops. This requires a modification in the
>> dma-mapping framework so that the iommu ops can be visible to all drivers.
>> [PATCH 4/7] ARM: dma-mapping: rename and export iommu_ops
>> [PATCH 5/7] drm/exynos: attach drm device with common drm mapping
>>
>> Currently allocation and free use the iommu framework by calling
>> dma_alloc_writecombine and dma_free_writecombine respectively.
>> For mapping the buffers to user space, the mmap functions assume that
>> the buffer is contiguous. This is modified by calling
>> dma_mmap_writecombine.
>> [PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function
>> [PATCH 7/7] Add IOMMU support for mapping gem object
>>
>> The device tree based patches are based on Leela's patch which was posted
>> last week for adding DT support to DRM FIMD. The patch to add sysmmu
>> field is for reference only and will be posted to the device tree
>> mailing list. Same with the rename and export iommu_ops patch.
>>
>> These patches are tested on Exynos5250 SMDK board and tested with modetest
>> from libdrm tests.
>>
>> Prathyush K (7):
>>    drm/exynos: create common IOMMU mapping for DRM
>>    ARM: EXYNOS5: add sysmmu field to fimd device node
>>    drm/exynos: add IOMMU support to drm fimd
>>    ARM: dma-mapping: rename and export iommu_ops
>>    drm/exynos: attach drm device with common drm mapping
>>    drm/exynos: Add exynos drm specific fb_mmap function
>>    drm/exynos: Add IOMMU support for mapping gem object
>>
>>   arch/arm/boot/dts/exynos5250.dtsi         |    1 +
>>   arch/arm/include/asm/dma-mapping.h        |    1 +
>>   arch/arm/mm/dma-mapping.c                 |    5 ++-
>>   drivers/gpu/drm/exynos/exynos_drm_core.c  |    3 ++
>>   drivers/gpu/drm/exynos/exynos_drm_drv.c   |   30 ++++++++++++++++
>>   drivers/gpu/drm/exynos/exynos_drm_drv.h   |   10 +++++
>>   drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c  |   54
>> ++++++++++++++++++++++++++++-
>>   drivers/gpu/drm/exynos/exynos_drm_gem.c   |   35 ++++++++----------
>>   9 files changed, 133 insertions(+), 22 deletions(-)

RE: [PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function

[Inki Dae]

> -----Original Message-----
> From: Prathyush K [mailto:prathyush.k@samsung.com]
> Sent: Wednesday, July 11, 2012 6:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
inki.dae@samsung.com;
> subash.ramaswamy@linaro.org
> Subject: [PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function
> 
> This patch adds a exynos drm specific implementation of fb_mmap
> which supports mapping a non-contiguous buffer to user space.
> This new function does not assume that the frame buffer is contiguous
> and calls dma_mmap_writecombine for mapping the buffer to user space.
> dma_mmap_writecombine will be able to map a contiguous buffer as well
> as non-contig buffer depending on whether an IOMMU mapping is created
> for drm or not.
> 
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index d5586cc..b53e638 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -46,8 +46,24 @@ struct exynos_drm_fbdev {
>  	struct exynos_drm_gem_obj	*exynos_gem_obj;
>  };
> 
> +static int exynos_drm_fb_mmap(struct fb_info *info,
> +			      struct vm_area_struct *vma)
> +{
> +	if ((vma->vm_end - vma->vm_start) > info->fix.smem_len)
> +		return -EINVAL;
> +
> +	vma->vm_pgoff = 0;
> +	vma->vm_flags |= VM_IO | VM_RESERVED;
> +	if (dma_mmap_writecombine(info->device, vma, info->screen_base,
> +			info->fix.smem_start, vma->vm_end - vma->vm_start))
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +

Ok, it's good feature. actually the physically non-contiguous gem buffer
allocated for console framebuffer has to be mapped with user space.

Thanks.

>  static struct fb_ops exynos_drm_fb_ops = {
>  	.owner		= THIS_MODULE,
> +	.fb_mmap	= exynos_drm_fb_mmap,
>  	.fb_fillrect	= cfb_fillrect,
>  	.fb_copyarea	= cfb_copyarea,
>  	.fb_imageblit	= cfb_imageblit,
> --
> 1.7.0.4

RE: [PATCH 7/7] drm/exynos: Add IOMMU support for mapping gem object

[Inki Dae]

> -----Original Message-----
> From: Prathyush K [mailto:prathyush.k@samsung.com]
> Sent: Wednesday, July 11, 2012 6:40 PM
> To: dri-devel@lists.freedesktop.org
> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
inki.dae@samsung.com;
> subash.ramaswamy@linaro.org
> Subject: [PATCH 7/7] drm/exynos: Add IOMMU support for mapping gem object
> 
> A gem object is created using dma_alloc_writecombine. Currently, this
> buffer is assumed to be contiguous. If a IOMMU mapping is created for
> DRM, this buffer would be non-contig so the map functions are modified
> to call dma_mmap_writecombine. This works for both contig and non-contig
> buffers.
> 
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c |   35
++++++++++++++-------------
> ---
>  1 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 5c8b683..59240f7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -162,17 +162,22 @@ static int exynos_drm_gem_map_pages(struct
> drm_gem_object *obj,
>  {
>  	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>  	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
> -	unsigned long pfn;
> 
>  	if (exynos_gem_obj->flags & EXYNOS_BO_NONCONTIG) {
> +		unsigned long pfn;
>  		if (!buf->pages)
>  			return -EINTR;
> 
>  		pfn = page_to_pfn(buf->pages[page_offset++]);
> -	} else
> -		pfn = (buf->dma_addr >> PAGE_SHIFT) + page_offset;
> -
> -	return vm_insert_mixed(vma, f_vaddr, pfn);
> +		return vm_insert_mixed(vma, f_vaddr, pfn);
> +	} else {

It's not good. EXYNOS_BO_NONCONTIG means physically non-contiguous otherwise
physically contiguous memory but with your patch, in case of using iommu,
memory type of the gem object may have no any meaning. in this case, the
memory type is EXYNOS_BO_CONTIG and has physically non-contiguous memory.

> +		int ret;
> +		ret = dma_mmap_writecombine(obj->dev->dev, vma, buf->kvaddr,
> +					buf->dma_addr, buf->size);
> +		if (ret)
> +			DRM_ERROR("dma_mmap_writecombine failed\n");
> +		return ret;
> +	}
>  }
> 
>  static int exynos_drm_gem_get_pages(struct drm_gem_object *obj)
> @@ -503,7 +508,7 @@ static int exynos_drm_gem_mmap_buffer(struct file
> *filp,
>  	struct drm_gem_object *obj = filp->private_data;
>  	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>  	struct exynos_drm_gem_buf *buffer;
> -	unsigned long pfn, vm_size, usize, uaddr = vma->vm_start;
> +	unsigned long vm_size, usize, uaddr = vma->vm_start;
>  	int ret;
> 
>  	DRM_DEBUG_KMS("%s\n", __FILE__);
> @@ -543,19 +548,11 @@ static int exynos_drm_gem_mmap_buffer(struct file
> *filp,
>  			usize -= PAGE_SIZE;
>  		} while (usize > 0);
>  	} else {
> -		/*
> -		 * get page frame number to physical memory to be mapped
> -		 * to user space.
> -		 */
> -		pfn = ((unsigned long)exynos_gem_obj->buffer->dma_addr) >>
> -								PAGE_SHIFT;
> -
> -		DRM_DEBUG_KMS("pfn = 0x%lx\n", pfn);
> -
> -		if (remap_pfn_range(vma, vma->vm_start, pfn, vm_size,
> -					vma->vm_page_prot)) {
> -			DRM_ERROR("failed to remap pfn range.\n");
> -			return -EAGAIN;
> +		ret = dma_mmap_writecombine(obj->dev->dev, vma, buffer-
> >kvaddr,

What if we don't use iommu and memory type of this buffer is non-contiguous?

> +					buffer->dma_addr, buffer->size);
> +		if (ret) {
> +			DRM_ERROR("dma_mmap_writecombine failed\n");
> +			return ret;
>  		}
>  	}
> 
> --
> 1.7.0.4

RE: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Inki Dae]

> -----Original Message-----
> From: Subash Patel [mailto:subash.ramaswamy@linaro.org]
> Sent: Friday, July 13, 2012 3:58 PM
> To: Inki Dae
> Cc: 'Prathyush K'; dri-devel@lists.freedesktop.org;
prathyush@chromium.org;
> m.szyprowski@samsung.com
> Subject: Re: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> 
> On 07/13/2012 12:09 PM, Inki Dae wrote:
> >
> >> -----Original Message-----
> >> From: Prathyush K [mailto:prathyush.k@samsung.com]
> >> Sent: Wednesday, July 11, 2012 6:40 PM
> >> To: dri-devel@lists.freedesktop.org
> >> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
> > inki.dae@samsung.com;
> >> subash.ramaswamy@linaro.org
> >> Subject: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> >>
> >> The dma-mapping framework needs a IOMMU mapping to be created for the
> >> device which allocates/maps/frees the non-contig buffer. In the DRM
> >> framework, a gem buffer is created by the DRM virtual device and not
> >> directly by any of the physical devices (FIMD, HDMI etc). Each gem
> object
> >> can be set as a framebuffer to one or many of the drm devices. So a gem
> >> object cannot be allocated for any one device. All the DRM devices
> should
> >> be able to access this buffer.
> >>
> > It's good to use unified iommu table so I agree to your opinion but we
> don't
> > decide whether we use dma mapping api or not. now dma mapping api has
> one
> > issue.
> > in case of using iommu with dma mapping api, we couldn't use physically
> > contiguous memory region with iommu. for this, there is a case that we
> > should use physically contiguous memory region with iommu. it is because
> we
> > sometime may use mfc(hw video codec) with secure zone such as ARM
> TrustZone.
> > Then, it needs physically contiguous memory region.
> >
> > Thanks,
> > Inki Dae
> I agree. In the mainline code, as of now only the arm_dma_ops has the
> support allocating
> from the CMA. But in the function arm_iommu_alloc_attrs(), there is no
> way to know if the
> device had declared a contiguous memory range. The reason, we don't
> store that cookie
> into the device during the dma_declare_contiguous(). So is it advisable
> to store such information
> like mapping(in the iommu operations) in the device.archdata?
> 
> Regards,
> Subash

There was my missing point. dma mapping api with iommu tries to allocate
pages contiguously if possible. as HW Video codec above, there is the case
that physically contiguous memory region is need necessarily but it seems
like that now dma mapping api doesn't guarantee fully physically contiguous
memory region(for example, if physically contiguous memory allocation is
failed then it should return an error so that we can check it)

Marek, for this, could you please give us comments?

Thanks,
Inki Dae

> >
> >> The proposed method is to create a common IOMMU mapping during drm
init.
> >> This
> >> mapping is then attached to all of the drm devices including the drm
> >> device.
> >> [PATCH 1/7] drm/exynos: create common IOMMU mapping for DRM
> >>
> >> During the probe of drm fimd, the driver retrieves a 'sysmmu' field
> >> in the device node for fimd. If such a field exists, the driver
> retrieves
> >> the
> >> platform device of the sysmmu device. This sysmmu is set as the sysmmu
> >> for fimd. The common mapping created is then attached to fimd.
> >> This needs to be done for all the other devices (hdmi, vidi etc).
> >> [PATCH 2/7] ARM: EXYNOS5: add sysmmu field to fimd device node
> >> [PATCH 3/7] drm/exynos: add IOMMU support to drm fimd
> >>
> >> During DRM's probe which happens last, the common mapping is set to its
> >> archdata
> >> and iommu ops are set as its dma ops. This requires a modification in
> the
> >> dma-mapping framework so that the iommu ops can be visible to all
> drivers.
> >> [PATCH 4/7] ARM: dma-mapping: rename and export iommu_ops
> >> [PATCH 5/7] drm/exynos: attach drm device with common drm mapping
> >>
> >> Currently allocation and free use the iommu framework by calling
> >> dma_alloc_writecombine and dma_free_writecombine respectively.
> >> For mapping the buffers to user space, the mmap functions assume that
> >> the buffer is contiguous. This is modified by calling
> >> dma_mmap_writecombine.
> >> [PATCH 6/7] drm/exynos: Add exynos drm specific fb_mmap function
> >> [PATCH 7/7] Add IOMMU support for mapping gem object
> >>
> >> The device tree based patches are based on Leela's patch which was
> posted
> >> last week for adding DT support to DRM FIMD. The patch to add sysmmu
> >> field is for reference only and will be posted to the device tree
> >> mailing list. Same with the rename and export iommu_ops patch.
> >>
> >> These patches are tested on Exynos5250 SMDK board and tested with
> modetest
> >> from libdrm tests.
> >>
> >> Prathyush K (7):
> >>    drm/exynos: create common IOMMU mapping for DRM
> >>    ARM: EXYNOS5: add sysmmu field to fimd device node
> >>    drm/exynos: add IOMMU support to drm fimd
> >>    ARM: dma-mapping: rename and export iommu_ops
> >>    drm/exynos: attach drm device with common drm mapping
> >>    drm/exynos: Add exynos drm specific fb_mmap function
> >>    drm/exynos: Add IOMMU support for mapping gem object
> >>
> >>   arch/arm/boot/dts/exynos5250.dtsi         |    1 +
> >>   arch/arm/include/asm/dma-mapping.h        |    1 +
> >>   arch/arm/mm/dma-mapping.c                 |    5 ++-
> >>   drivers/gpu/drm/exynos/exynos_drm_core.c  |    3 ++
> >>   drivers/gpu/drm/exynos/exynos_drm_drv.c   |   30 ++++++++++++++++
> >>   drivers/gpu/drm/exynos/exynos_drm_drv.h   |   10 +++++
> >>   drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   16 ++++++++
> >>   drivers/gpu/drm/exynos/exynos_drm_fimd.c  |   54
> >> ++++++++++++++++++++++++++++-
> >>   drivers/gpu/drm/exynos/exynos_drm_gem.c   |   35 ++++++++----------
> >>   9 files changed, 133 insertions(+), 22 deletions(-)

RE: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Marek Szyprowski]

Hello,

On Monday, July 16, 2012 3:47 PM Inki Dae wrote:

> > -----Original Message-----
> > From: Subash Patel [mailto:subash.ramaswamy@linaro.org]
> > Sent: Friday, July 13, 2012 3:58 PM
> > To: Inki Dae
> > Cc: 'Prathyush K'; dri-devel@lists.freedesktop.org;
> prathyush@chromium.org;
> > m.szyprowski@samsung.com
> > Subject: Re: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> >
> > On 07/13/2012 12:09 PM, Inki Dae wrote:
> > >
> > >> -----Original Message-----
> > >> From: Prathyush K [mailto:prathyush.k@samsung.com]
> > >> Sent: Wednesday, July 11, 2012 6:40 PM
> > >> To: dri-devel@lists.freedesktop.org
> > >> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
> > > inki.dae@samsung.com;
> > >> subash.ramaswamy@linaro.org
> > >> Subject: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> > >>
> > >> The dma-mapping framework needs a IOMMU mapping to be created for the
> > >> device which allocates/maps/frees the non-contig buffer. In the DRM
> > >> framework, a gem buffer is created by the DRM virtual device and not
> > >> directly by any of the physical devices (FIMD, HDMI etc). Each gem
> > object
> > >> can be set as a framebuffer to one or many of the drm devices. So a gem
> > >> object cannot be allocated for any one device. All the DRM devices
> > should
> > >> be able to access this buffer.
> > >>
> > > It's good to use unified iommu table so I agree to your opinion but we
> > don't
> > > decide whether we use dma mapping api or not. now dma mapping api has
> > one
> > > issue.
> > > in case of using iommu with dma mapping api, we couldn't use physically
> > > contiguous memory region with iommu. for this, there is a case that we
> > > should use physically contiguous memory region with iommu. it is because
> > we
> > > sometime may use mfc(hw video codec) with secure zone such as ARM
> > TrustZone.
> > > Then, it needs physically contiguous memory region.
> > >
> > > Thanks,
> > > Inki Dae
> > I agree. In the mainline code, as of now only the arm_dma_ops has the
> > support allocating
> > from the CMA. But in the function arm_iommu_alloc_attrs(), there is no
> > way to know if the
> > device had declared a contiguous memory range. The reason, we don't
> > store that cookie
> > into the device during the dma_declare_contiguous(). So is it advisable
> > to store such information
> > like mapping(in the iommu operations) in the device.archdata?
> >
> > Regards,
> > Subash
> 
> There was my missing point. dma mapping api with iommu tries to allocate
> pages contiguously if possible. as HW Video codec above, there is the case
> that physically contiguous memory region is need necessarily but it seems
> like that now dma mapping api doesn't guarantee fully physically contiguous
> memory region(for example, if physically contiguous memory allocation is
> failed then it should return an error so that we can check it)
> 
> Marek, for this, could you please give us comments?

If I understand right, you want to be able to allocate both physically contiguous
and physically discontiguous memory buffers for the same device with IOMMU. This 
can be achieved by adding a new attribute to dma-mapping subsystem, let's call it 
DMA_ATTR_FORCE_CONTIGUOUS, which forces dma-mapping to allocate the buffer from 
the CMA area or reserved memory (depending on what is available) instead of 
assembling it from individual pages.

Would it solve your problem?

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

RE: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM

[Inki Dae]

Hello Marek,


> -----Original Message-----
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> Sent: Tuesday, July 17, 2012 5:04 PM
> To: 'Inki Dae'; 'Subash Patel'
> Cc: 'Prathyush K'; dri-devel@lists.freedesktop.org; prathyush@chromium.org
> Subject: RE: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> 
> Hello,
> 
> On Monday, July 16, 2012 3:47 PM Inki Dae wrote:
> 
> > > -----Original Message-----
> > > From: Subash Patel [mailto:subash.ramaswamy@linaro.org]
> > > Sent: Friday, July 13, 2012 3:58 PM
> > > To: Inki Dae
> > > Cc: 'Prathyush K'; dri-devel@lists.freedesktop.org;
> > prathyush@chromium.org;
> > > m.szyprowski@samsung.com
> > > Subject: Re: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> > >
> > > On 07/13/2012 12:09 PM, Inki Dae wrote:
> > > >
> > > >> -----Original Message-----
> > > >> From: Prathyush K [mailto:prathyush.k@samsung.com]
> > > >> Sent: Wednesday, July 11, 2012 6:40 PM
> > > >> To: dri-devel@lists.freedesktop.org
> > > >> Cc: prathyush@chromium.org; m.szyprowski@samsung.com;
> > > > inki.dae@samsung.com;
> > > >> subash.ramaswamy@linaro.org
> > > >> Subject: [PATCH 0/7] [RFC] drm/exynos: Add IOMMU support to DRM
> > > >>
> > > >> The dma-mapping framework needs a IOMMU mapping to be created for
> the
> > > >> device which allocates/maps/frees the non-contig buffer. In the DRM
> > > >> framework, a gem buffer is created by the DRM virtual device and
> not
> > > >> directly by any of the physical devices (FIMD, HDMI etc). Each gem
> > > object
> > > >> can be set as a framebuffer to one or many of the drm devices. So a
> gem
> > > >> object cannot be allocated for any one device. All the DRM devices
> > > should
> > > >> be able to access this buffer.
> > > >>
> > > > It's good to use unified iommu table so I agree to your opinion but
> we
> > > don't
> > > > decide whether we use dma mapping api or not. now dma mapping api
> has
> > > one
> > > > issue.
> > > > in case of using iommu with dma mapping api, we couldn't use
> physically
> > > > contiguous memory region with iommu. for this, there is a case that
> we
> > > > should use physically contiguous memory region with iommu. it is
> because
> > > we
> > > > sometime may use mfc(hw video codec) with secure zone such as ARM
> > > TrustZone.
> > > > Then, it needs physically contiguous memory region.
> > > >
> > > > Thanks,
> > > > Inki Dae
> > > I agree. In the mainline code, as of now only the arm_dma_ops has the
> > > support allocating
> > > from the CMA. But in the function arm_iommu_alloc_attrs(), there is no
> > > way to know if the
> > > device had declared a contiguous memory range. The reason, we don't
> > > store that cookie
> > > into the device during the dma_declare_contiguous(). So is it
> advisable
> > > to store such information
> > > like mapping(in the iommu operations) in the device.archdata?
> > >
> > > Regards,
> > > Subash
> >
> > There was my missing point. dma mapping api with iommu tries to allocate
> > pages contiguously if possible. as HW Video codec above, there is the
> case
> > that physically contiguous memory region is need necessarily but it
> seems
> > like that now dma mapping api doesn't guarantee fully physically
> contiguous
> > memory region(for example, if physically contiguous memory allocation is
> > failed then it should return an error so that we can check it)
> >
> > Marek, for this, could you please give us comments?
> 
> If I understand right, you want to be able to allocate both physically
> contiguous
> and physically discontiguous memory buffers for the same device with
IOMMU.
> This
> can be achieved by adding a new attribute to dma-mapping subsystem, let's
> call it
> DMA_ATTR_FORCE_CONTIGUOUS, which forces dma-mapping to allocate the buffer
> from
> the CMA area or reserved memory (depending on what is available) instead
> of
> assembling it from individual pages.
> 
> Would it solve your problem?
> 

Yes, it does. we need DMA_ATTR_FORCE_CONGIGUOUS feature so that the CMA can
guarantee fully physically contiguous memory allocation in some case.

Thanks,
Inki Dae

> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center