[PATCH 0/4] drm/rockchip: switch to drm_mm for support arm64 iommu

[PATCH 0/4] drm/rockchip: switch to drm_mm for support arm64 iommu

[Mark Yao]

Some iommu patches on the series[0] "iommu/rockchip: Fix bugs and
enable on ARM64" already landed, So drm/rockchip related patches [1] and [2]
ready to landed, this series just rebase them to lastest drm-next.

And fix some bugs for drm/rockchip drm_mm

[0]: http://www.spinics.net/lists/arm-kernel/msg513781.html
[1]: https://patchwork.kernel.org/patch/9196367
[2]: https://patchwork.kernel.org/patch/9196369

Mark Yao (2):
  drm/rockchip: gem: add mutex lock for drm mm
  drm/rockchip: gem: fixup iommu_map_sg error path

Shunqian Zheng (1):
  drm/rockchip: Use common IOMMU API to attach devices

Tomasz Figa (1):
  drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 101 ++++++------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   6 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 228 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 4 files changed, 286 insertions(+), 57 deletions(-)

-- 
1.9.1

[PATCH 0/4] drm/rockchip: switch to drm_mm for support arm64 iommu

[Mark Yao]

Some iommu patches on the series[0] "iommu/rockchip: Fix bugs and
enable on ARM64" already landed, So drm/rockchip related patches [1] and [2]
ready to landed, this series just rebase them to lastest drm-next.

And fix some bugs for drm/rockchip drm_mm

[0]: http://www.spinics.net/lists/arm-kernel/msg513781.html
[1]: https://patchwork.kernel.org/patch/9196367
[2]: https://patchwork.kernel.org/patch/9196369

Mark Yao (2):
  drm/rockchip: gem: add mutex lock for drm mm
  drm/rockchip: gem: fixup iommu_map_sg error path

Shunqian Zheng (1):
  drm/rockchip: Use common IOMMU API to attach devices

Tomasz Figa (1):
  drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 101 ++++++------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   6 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 228 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 4 files changed, 286 insertions(+), 57 deletions(-)

-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/4] drm/rockchip: switch to drm_mm for support arm64 iommu

[Mark Yao]

Some iommu patches on the series[0] "iommu/rockchip: Fix bugs and
enable on ARM64" already landed, So drm/rockchip related patches [1] and [2]
ready to landed, this series just rebase them to lastest drm-next.

And fix some bugs for drm/rockchip drm_mm

[0]: http://www.spinics.net/lists/arm-kernel/msg513781.html
[1]: https://patchwork.kernel.org/patch/9196367
[2]: https://patchwork.kernel.org/patch/9196369

Mark Yao (2):
  drm/rockchip: gem: add mutex lock for drm mm
  drm/rockchip: gem: fixup iommu_map_sg error path

Shunqian Zheng (1):
  drm/rockchip: Use common IOMMU API to attach devices

Tomasz Figa (1):
  drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 101 ++++++------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   6 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 228 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 4 files changed, 286 insertions(+), 57 deletions(-)

-- 
1.9.1

[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark Yao]

From: Tomasz Figa <tfiga@chromium.org>

The API is not suitable for subsystems consisting of multiple devices
and requires severe hacks to use it. To mitigate this, this patch
implements allocation and address space management locally by using
helpers provided by DRM framework, like other DRM drivers do, e.g.
Tegra.

This patch should not introduce any functional changes until the driver
is made to attach subdevices into an IOMMU domain with the generic IOMMU
API, which will happen in following patch. Based heavily on GEM
implementation of Tegra DRM driver.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 3 files changed, 219 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index fb6226c..7c123d9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -30,6 +30,7 @@
 
 struct drm_device;
 struct drm_connector;
+struct iommu_domain;
 
 /*
  * Rockchip drm private crtc funcs.
@@ -60,7 +61,8 @@ struct rockchip_drm_private {
 	struct drm_gem_object *fbdev_bo;
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
-
+	struct iommu_domain *domain;
+	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
 };
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index b70f942..5209392 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -16,11 +16,135 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_vma_manager.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
 
-static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+	int prot = IOMMU_READ | IOMMU_WRITE;
+	ssize_t ret;
+
+	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
+					 rk_obj->base.size, PAGE_SIZE,
+					 0, 0);
+	if (ret < 0) {
+		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
+		return ret;
+	}
+
+	rk_obj->dma_addr = rk_obj->mm.start;
+
+	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
+			   rk_obj->sgt->nents, prot);
+	if (ret < 0) {
+		DRM_ERROR("failed to map buffer: %zd\n", ret);
+		goto err_remove_node;
+	}
+
+	rk_obj->size = ret;
+
+	return 0;
+
+err_remove_node:
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return ret;
+}
+
+static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return 0;
+}
+
+static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	int ret, i;
+	struct scatterlist *s;
+
+	rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
+	if (IS_ERR(rk_obj->pages))
+		return PTR_ERR(rk_obj->pages);
+
+	rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
+
+	rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+	if (IS_ERR(rk_obj->sgt)) {
+		ret = PTR_ERR(rk_obj->sgt);
+		goto err_put_pages;
+	}
+
+	/*
+	 * Fake up the SG table so that dma_sync_sg_for_device() can be used
+	 * to flush the pages associated with it.
+	 *
+	 * TODO: Replace this by drm_clflush_sg() once it can be implemented
+	 * without relying on symbols that are not exported.
+	 */
+	for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+		sg_dma_address(s) = sg_phys(s);
+
+	dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
+			       DMA_TO_DEVICE);
+
+	return 0;
+
+err_put_pages:
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+	return ret;
+}
+
+static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
+{
+	sg_free_table(rk_obj->sgt);
+	kfree(rk_obj->sgt);
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+}
+
+static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
+				    bool alloc_kmap)
+{
+	int ret;
+
+	ret = rockchip_gem_get_pages(rk_obj);
+	if (ret < 0)
+		return ret;
+
+	ret = rockchip_gem_iommu_map(rk_obj);
+	if (ret < 0)
+		goto err_free;
+
+	if (alloc_kmap) {
+		rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+				      pgprot_writecombine(PAGE_KERNEL));
+		if (!rk_obj->kvaddr) {
+			DRM_ERROR("failed to vmap() buffer\n");
+			ret = -ENOMEM;
+			goto err_unmap;
+		}
+	}
+
+	return 0;
+
+err_unmap:
+	rockchip_gem_iommu_unmap(rk_obj);
+err_free:
+	rockchip_gem_put_pages(rk_obj);
+
+	return ret;
+}
+
+static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
 				  bool alloc_kmap)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
@@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
 	return 0;
 }
 
-static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+				  bool alloc_kmap)
+{
+	struct drm_gem_object *obj = &rk_obj->base;
+	struct drm_device *drm = obj->dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	if (private->domain)
+		return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
+	else
+		return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
+}
+
+static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
+{
+	vunmap(rk_obj->kvaddr);
+	rockchip_gem_iommu_unmap(rk_obj);
+	rockchip_gem_put_pages(rk_obj);
+}
+
+static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
 	struct drm_device *drm = obj->dev;
@@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
 		       rk_obj->dma_attrs);
 }
 
-static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
-					struct vm_area_struct *vma)
+static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+{
+	if (rk_obj->pages)
+		rockchip_gem_free_iommu(rk_obj);
+	else
+		rockchip_gem_free_dma(rk_obj);
+}
 
+static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
+					      struct vm_area_struct *vma)
 {
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+	unsigned int i, count = obj->size >> PAGE_SHIFT;
+	unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	unsigned long uaddr = vma->vm_start;
 	int ret;
+
+	if (user_count == 0 || user_count > count)
+		return -ENXIO;
+
+	for (i = 0; i < user_count; i++) {
+		ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
+		if (ret)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
+					    struct vm_area_struct *vma)
+{
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 	struct drm_device *drm = obj->dev;
 
+	return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
+			      obj->size, rk_obj->dma_attrs);
+}
+
+static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
+					struct vm_area_struct *vma)
+{
+	int ret;
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
 	/*
-	 * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
+	 * We allocated a struct page table for rk_obj, so clear
 	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
 	 */
 	vma->vm_flags &= ~VM_PFNMAP;
 	vma->vm_pgoff = 0;
 
-	ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
-			     obj->size, rk_obj->dma_attrs);
+	if (rk_obj->pages)
+		ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
+	else
+		ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
+
 	if (ret)
 		drm_gem_vm_close(vma);
 
@@ -117,7 +302,7 @@ struct rockchip_gem_object *
 
 	obj = &rk_obj->base;
 
-	drm_gem_private_object_init(drm, obj, size);
+	drm_gem_object_init(drm, obj, size);
 
 	ret = rockchip_gem_alloc_buf(rk_obj, alloc_kmap);
 	if (ret)
@@ -253,6 +438,9 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj)
 	struct sg_table *sgt;
 	int ret;
 
+	if (rk_obj->pages)
+		return drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+
 	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
 	if (!sgt)
 		return ERR_PTR(-ENOMEM);
@@ -273,6 +461,10 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 {
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 
+	if (rk_obj->pages)
+		return vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+			    pgprot_writecombine(PAGE_KERNEL));
+
 	if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
 		return NULL;
 
@@ -281,5 +473,12 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 
 void rockchip_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
 {
-	/* Nothing to do */
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
+	if (rk_obj->pages) {
+		vunmap(vaddr);
+		return;
+	}
+
+	/* Nothing to do if allocated by DMA mapping API. */
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
index 18b3488..3f6ea4d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
@@ -23,7 +23,15 @@ struct rockchip_gem_object {
 
 	void *kvaddr;
 	dma_addr_t dma_addr;
+	/* Used when IOMMU is disabled */
 	unsigned long dma_attrs;
+
+	/* Used when IOMMU is enabled */
+	struct drm_mm_node mm;
+	unsigned long num_pages;
+	struct page **pages;
+	struct sg_table *sgt;
+	size_t size;
 };
 
 struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj);
-- 
1.9.1

[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark Yao]

From: Tomasz Figa <tfiga@chromium.org>

The API is not suitable for subsystems consisting of multiple devices
and requires severe hacks to use it. To mitigate this, this patch
implements allocation and address space management locally by using
helpers provided by DRM framework, like other DRM drivers do, e.g.
Tegra.

This patch should not introduce any functional changes until the driver
is made to attach subdevices into an IOMMU domain with the generic IOMMU
API, which will happen in following patch. Based heavily on GEM
implementation of Tegra DRM driver.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 3 files changed, 219 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index fb6226c..7c123d9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -30,6 +30,7 @@
 
 struct drm_device;
 struct drm_connector;
+struct iommu_domain;
 
 /*
  * Rockchip drm private crtc funcs.
@@ -60,7 +61,8 @@ struct rockchip_drm_private {
 	struct drm_gem_object *fbdev_bo;
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
-
+	struct iommu_domain *domain;
+	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
 };
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index b70f942..5209392 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -16,11 +16,135 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_vma_manager.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
 
-static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+	int prot = IOMMU_READ | IOMMU_WRITE;
+	ssize_t ret;
+
+	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
+					 rk_obj->base.size, PAGE_SIZE,
+					 0, 0);
+	if (ret < 0) {
+		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
+		return ret;
+	}
+
+	rk_obj->dma_addr = rk_obj->mm.start;
+
+	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
+			   rk_obj->sgt->nents, prot);
+	if (ret < 0) {
+		DRM_ERROR("failed to map buffer: %zd\n", ret);
+		goto err_remove_node;
+	}
+
+	rk_obj->size = ret;
+
+	return 0;
+
+err_remove_node:
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return ret;
+}
+
+static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return 0;
+}
+
+static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	int ret, i;
+	struct scatterlist *s;
+
+	rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
+	if (IS_ERR(rk_obj->pages))
+		return PTR_ERR(rk_obj->pages);
+
+	rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
+
+	rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+	if (IS_ERR(rk_obj->sgt)) {
+		ret = PTR_ERR(rk_obj->sgt);
+		goto err_put_pages;
+	}
+
+	/*
+	 * Fake up the SG table so that dma_sync_sg_for_device() can be used
+	 * to flush the pages associated with it.
+	 *
+	 * TODO: Replace this by drm_clflush_sg() once it can be implemented
+	 * without relying on symbols that are not exported.
+	 */
+	for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+		sg_dma_address(s) = sg_phys(s);
+
+	dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
+			       DMA_TO_DEVICE);
+
+	return 0;
+
+err_put_pages:
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+	return ret;
+}
+
+static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
+{
+	sg_free_table(rk_obj->sgt);
+	kfree(rk_obj->sgt);
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+}
+
+static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
+				    bool alloc_kmap)
+{
+	int ret;
+
+	ret = rockchip_gem_get_pages(rk_obj);
+	if (ret < 0)
+		return ret;
+
+	ret = rockchip_gem_iommu_map(rk_obj);
+	if (ret < 0)
+		goto err_free;
+
+	if (alloc_kmap) {
+		rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+				      pgprot_writecombine(PAGE_KERNEL));
+		if (!rk_obj->kvaddr) {
+			DRM_ERROR("failed to vmap() buffer\n");
+			ret = -ENOMEM;
+			goto err_unmap;
+		}
+	}
+
+	return 0;
+
+err_unmap:
+	rockchip_gem_iommu_unmap(rk_obj);
+err_free:
+	rockchip_gem_put_pages(rk_obj);
+
+	return ret;
+}
+
+static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
 				  bool alloc_kmap)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
@@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
 	return 0;
 }
 
-static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+				  bool alloc_kmap)
+{
+	struct drm_gem_object *obj = &rk_obj->base;
+	struct drm_device *drm = obj->dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	if (private->domain)
+		return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
+	else
+		return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
+}
+
+static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
+{
+	vunmap(rk_obj->kvaddr);
+	rockchip_gem_iommu_unmap(rk_obj);
+	rockchip_gem_put_pages(rk_obj);
+}
+
+static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
 	struct drm_device *drm = obj->dev;
@@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
 		       rk_obj->dma_attrs);
 }
 
-static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
-					struct vm_area_struct *vma)
+static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+{
+	if (rk_obj->pages)
+		rockchip_gem_free_iommu(rk_obj);
+	else
+		rockchip_gem_free_dma(rk_obj);
+}
 
+static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
+					      struct vm_area_struct *vma)
 {
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+	unsigned int i, count = obj->size >> PAGE_SHIFT;
+	unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	unsigned long uaddr = vma->vm_start;
 	int ret;
+
+	if (user_count == 0 || user_count > count)
+		return -ENXIO;
+
+	for (i = 0; i < user_count; i++) {
+		ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
+		if (ret)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
+					    struct vm_area_struct *vma)
+{
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 	struct drm_device *drm = obj->dev;
 
+	return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
+			      obj->size, rk_obj->dma_attrs);
+}
+
+static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
+					struct vm_area_struct *vma)
+{
+	int ret;
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
 	/*
-	 * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
+	 * We allocated a struct page table for rk_obj, so clear
 	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
 	 */
 	vma->vm_flags &= ~VM_PFNMAP;
 	vma->vm_pgoff = 0;
 
-	ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
-			     obj->size, rk_obj->dma_attrs);
+	if (rk_obj->pages)
+		ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
+	else
+		ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
+
 	if (ret)
 		drm_gem_vm_close(vma);
 
@@ -117,7 +302,7 @@ struct rockchip_gem_object *
 
 	obj = &rk_obj->base;
 
-	drm_gem_private_object_init(drm, obj, size);
+	drm_gem_object_init(drm, obj, size);
 
 	ret = rockchip_gem_alloc_buf(rk_obj, alloc_kmap);
 	if (ret)
@@ -253,6 +438,9 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj)
 	struct sg_table *sgt;
 	int ret;
 
+	if (rk_obj->pages)
+		return drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+
 	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
 	if (!sgt)
 		return ERR_PTR(-ENOMEM);
@@ -273,6 +461,10 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 {
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 
+	if (rk_obj->pages)
+		return vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+			    pgprot_writecombine(PAGE_KERNEL));
+
 	if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
 		return NULL;
 
@@ -281,5 +473,12 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 
 void rockchip_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
 {
-	/* Nothing to do */
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
+	if (rk_obj->pages) {
+		vunmap(vaddr);
+		return;
+	}
+
+	/* Nothing to do if allocated by DMA mapping API. */
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
index 18b3488..3f6ea4d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
@@ -23,7 +23,15 @@ struct rockchip_gem_object {
 
 	void *kvaddr;
 	dma_addr_t dma_addr;
+	/* Used when IOMMU is disabled */
 	unsigned long dma_attrs;
+
+	/* Used when IOMMU is enabled */
+	struct drm_mm_node mm;
+	unsigned long num_pages;
+	struct page **pages;
+	struct sg_table *sgt;
+	size_t size;
 };
 
 struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj);
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark Yao]

From: Tomasz Figa <tfiga@chromium.org>

The API is not suitable for subsystems consisting of multiple devices
and requires severe hacks to use it. To mitigate this, this patch
implements allocation and address space management locally by using
helpers provided by DRM framework, like other DRM drivers do, e.g.
Tegra.

This patch should not introduce any functional changes until the driver
is made to attach subdevices into an IOMMU domain with the generic IOMMU
API, which will happen in following patch. Based heavily on GEM
implementation of Tegra DRM driver.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
 drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
 3 files changed, 219 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index fb6226c..7c123d9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -30,6 +30,7 @@
 
 struct drm_device;
 struct drm_connector;
+struct iommu_domain;
 
 /*
  * Rockchip drm private crtc funcs.
@@ -60,7 +61,8 @@ struct rockchip_drm_private {
 	struct drm_gem_object *fbdev_bo;
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
-
+	struct iommu_domain *domain;
+	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
 };
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index b70f942..5209392 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -16,11 +16,135 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_vma_manager.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
 
-static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+	int prot = IOMMU_READ | IOMMU_WRITE;
+	ssize_t ret;
+
+	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
+					 rk_obj->base.size, PAGE_SIZE,
+					 0, 0);
+	if (ret < 0) {
+		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
+		return ret;
+	}
+
+	rk_obj->dma_addr = rk_obj->mm.start;
+
+	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
+			   rk_obj->sgt->nents, prot);
+	if (ret < 0) {
+		DRM_ERROR("failed to map buffer: %zd\n", ret);
+		goto err_remove_node;
+	}
+
+	rk_obj->size = ret;
+
+	return 0;
+
+err_remove_node:
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return ret;
+}
+
+static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+	drm_mm_remove_node(&rk_obj->mm);
+
+	return 0;
+}
+
+static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
+{
+	struct drm_device *drm = rk_obj->base.dev;
+	int ret, i;
+	struct scatterlist *s;
+
+	rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
+	if (IS_ERR(rk_obj->pages))
+		return PTR_ERR(rk_obj->pages);
+
+	rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
+
+	rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+	if (IS_ERR(rk_obj->sgt)) {
+		ret = PTR_ERR(rk_obj->sgt);
+		goto err_put_pages;
+	}
+
+	/*
+	 * Fake up the SG table so that dma_sync_sg_for_device() can be used
+	 * to flush the pages associated with it.
+	 *
+	 * TODO: Replace this by drm_clflush_sg() once it can be implemented
+	 * without relying on symbols that are not exported.
+	 */
+	for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
+		sg_dma_address(s) = sg_phys(s);
+
+	dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
+			       DMA_TO_DEVICE);
+
+	return 0;
+
+err_put_pages:
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+	return ret;
+}
+
+static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
+{
+	sg_free_table(rk_obj->sgt);
+	kfree(rk_obj->sgt);
+	drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
+}
+
+static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
+				    bool alloc_kmap)
+{
+	int ret;
+
+	ret = rockchip_gem_get_pages(rk_obj);
+	if (ret < 0)
+		return ret;
+
+	ret = rockchip_gem_iommu_map(rk_obj);
+	if (ret < 0)
+		goto err_free;
+
+	if (alloc_kmap) {
+		rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+				      pgprot_writecombine(PAGE_KERNEL));
+		if (!rk_obj->kvaddr) {
+			DRM_ERROR("failed to vmap() buffer\n");
+			ret = -ENOMEM;
+			goto err_unmap;
+		}
+	}
+
+	return 0;
+
+err_unmap:
+	rockchip_gem_iommu_unmap(rk_obj);
+err_free:
+	rockchip_gem_put_pages(rk_obj);
+
+	return ret;
+}
+
+static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
 				  bool alloc_kmap)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
@@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
 	return 0;
 }
 
-static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
+				  bool alloc_kmap)
+{
+	struct drm_gem_object *obj = &rk_obj->base;
+	struct drm_device *drm = obj->dev;
+	struct rockchip_drm_private *private = drm->dev_private;
+
+	if (private->domain)
+		return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
+	else
+		return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
+}
+
+static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
+{
+	vunmap(rk_obj->kvaddr);
+	rockchip_gem_iommu_unmap(rk_obj);
+	rockchip_gem_put_pages(rk_obj);
+}
+
+static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
 {
 	struct drm_gem_object *obj = &rk_obj->base;
 	struct drm_device *drm = obj->dev;
@@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
 		       rk_obj->dma_attrs);
 }
 
-static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
-					struct vm_area_struct *vma)
+static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
+{
+	if (rk_obj->pages)
+		rockchip_gem_free_iommu(rk_obj);
+	else
+		rockchip_gem_free_dma(rk_obj);
+}
 
+static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
+					      struct vm_area_struct *vma)
 {
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+	unsigned int i, count = obj->size >> PAGE_SHIFT;
+	unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	unsigned long uaddr = vma->vm_start;
 	int ret;
+
+	if (user_count == 0 || user_count > count)
+		return -ENXIO;
+
+	for (i = 0; i < user_count; i++) {
+		ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
+		if (ret)
+			return ret;
+		uaddr += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
+					    struct vm_area_struct *vma)
+{
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 	struct drm_device *drm = obj->dev;
 
+	return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
+			      obj->size, rk_obj->dma_attrs);
+}
+
+static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
+					struct vm_area_struct *vma)
+{
+	int ret;
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
 	/*
-	 * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
+	 * We allocated a struct page table for rk_obj, so clear
 	 * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
 	 */
 	vma->vm_flags &= ~VM_PFNMAP;
 	vma->vm_pgoff = 0;
 
-	ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
-			     obj->size, rk_obj->dma_attrs);
+	if (rk_obj->pages)
+		ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
+	else
+		ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
+
 	if (ret)
 		drm_gem_vm_close(vma);
 
@@ -117,7 +302,7 @@ struct rockchip_gem_object *
 
 	obj = &rk_obj->base;
 
-	drm_gem_private_object_init(drm, obj, size);
+	drm_gem_object_init(drm, obj, size);
 
 	ret = rockchip_gem_alloc_buf(rk_obj, alloc_kmap);
 	if (ret)
@@ -253,6 +438,9 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj)
 	struct sg_table *sgt;
 	int ret;
 
+	if (rk_obj->pages)
+		return drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
+
 	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
 	if (!sgt)
 		return ERR_PTR(-ENOMEM);
@@ -273,6 +461,10 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 {
 	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
 
+	if (rk_obj->pages)
+		return vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
+			    pgprot_writecombine(PAGE_KERNEL));
+
 	if (rk_obj->dma_attrs & DMA_ATTR_NO_KERNEL_MAPPING)
 		return NULL;
 
@@ -281,5 +473,12 @@ void *rockchip_gem_prime_vmap(struct drm_gem_object *obj)
 
 void rockchip_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
 {
-	/* Nothing to do */
+	struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
+
+	if (rk_obj->pages) {
+		vunmap(vaddr);
+		return;
+	}
+
+	/* Nothing to do if allocated by DMA mapping API. */
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
index 18b3488..3f6ea4d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.h
@@ -23,7 +23,15 @@ struct rockchip_gem_object {
 
 	void *kvaddr;
 	dma_addr_t dma_addr;
+	/* Used when IOMMU is disabled */
 	unsigned long dma_attrs;
+
+	/* Used when IOMMU is enabled */
+	struct drm_mm_node mm;
+	unsigned long num_pages;
+	struct page **pages;
+	struct sg_table *sgt;
+	size_t size;
 };
 
 struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj);
-- 
1.9.1

[PATCH 2/4] drm/rockchip: Use common IOMMU API to attach devices

[Mark Yao]

From: Shunqian Zheng <zhengsq@rock-chips.com>

Rockchip DRM used the arm special API, arm_iommu_*(), to attach
iommu for ARM32 SoCs. This patch convert to common iommu API
so it would support ARM64 like RK3399.

Since previous patch added support for direct IOMMU address space
management, there is no need to use DMA API anymore and this patch wires
things to use the new method.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 +++++++++++++++-------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index c30d649..7a610e9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -14,19 +14,19 @@
  * GNU General Public License for more details.
  */
 
-#include <asm/dma-iommu.h>
-
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
 #include <linux/console.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -50,28 +50,31 @@
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev)
 {
-	struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
+	struct rockchip_drm_private *private = drm_dev->dev_private;
 	int ret;
 
 	if (!is_support_iommu)
 		return 0;
 
-	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
-	if (ret)
+	ret = iommu_attach_device(private->domain, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach iommu device\n");
 		return ret;
+	}
 
-	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-	return arm_iommu_attach_device(dev, mapping);
+	return 0;
 }
 
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev)
 {
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
+
 	if (!is_support_iommu)
 		return;
 
-	arm_iommu_detach_device(dev);
+	iommu_detach_device(domain, dev);
 }
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
@@ -123,11 +126,45 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
 		priv->crtc_funcs[pipe]->disable_vblank(crtc);
 }
 
+static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain_geometry *geometry;
+	u64 start, end;
+
+	if (!is_support_iommu)
+		return 0;
+
+	private->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!private->domain)
+		return -ENOMEM;
+
+	geometry = &private->domain->geometry;
+	start = geometry->aperture_start;
+	end = geometry->aperture_end;
+
+	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
+		  start, end);
+	drm_mm_init(&private->mm, start, end - start + 1);
+
+	return 0;
+}
+
+static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+
+	if (!is_support_iommu)
+		return;
+
+	drm_mm_takedown(&private->mm);
+	iommu_domain_free(private->domain);
+}
+
 static int rockchip_drm_bind(struct device *dev)
 {
 	struct drm_device *drm_dev;
 	struct rockchip_drm_private *private;
-	struct dma_iommu_mapping *mapping = NULL;
 	int ret;
 
 	drm_dev = drm_dev_alloc(&rockchip_drm_driver, dev);
@@ -151,38 +188,14 @@ static int rockchip_drm_bind(struct device *dev)
 
 	rockchip_drm_mode_config_init(drm_dev);
 
-	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
-				      GFP_KERNEL);
-	if (!dev->dma_parms) {
-		ret = -ENOMEM;
+	ret = rockchip_drm_init_iommu(drm_dev);
+	if (ret)
 		goto err_config_cleanup;
-	}
-
-	if (is_support_iommu) {
-		/* TODO(djkurtz): fetch the mapping start/size from somewhere */
-		mapping = arm_iommu_create_mapping(&platform_bus_type,
-						   0x00000000,
-						   SZ_2G);
-		if (IS_ERR(mapping)) {
-			ret = PTR_ERR(mapping);
-			goto err_config_cleanup;
-		}
-
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret)
-			goto err_release_mapping;
-
-		dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-		ret = arm_iommu_attach_device(dev, mapping);
-		if (ret)
-			goto err_release_mapping;
-	}
 
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(dev, drm_dev);
 	if (ret)
-		goto err_detach_device;
+		goto err_iommu_cleanup;
 
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(drm_dev);
@@ -207,8 +220,6 @@ static int rockchip_drm_bind(struct device *dev)
 	if (ret)
 		goto err_fbdev_fini;
 
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
 	return 0;
 err_fbdev_fini:
 	rockchip_drm_fbdev_fini(drm_dev);
@@ -217,12 +228,8 @@ static int rockchip_drm_bind(struct device *dev)
 err_kms_helper_poll_fini:
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-err_detach_device:
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
-err_release_mapping:
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
+err_iommu_cleanup:
+	rockchip_iommu_cleanup(drm_dev);
 err_config_cleanup:
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
@@ -239,8 +246,7 @@ static void rockchip_drm_unbind(struct device *dev)
 	drm_vblank_cleanup(drm_dev);
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
+	rockchip_iommu_cleanup(drm_dev);
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
 	drm_dev_unregister(drm_dev);
-- 
1.9.1

[PATCH 2/4] drm/rockchip: Use common IOMMU API to attach devices

[Mark Yao]

From: Shunqian Zheng <zhengsq@rock-chips.com>

Rockchip DRM used the arm special API, arm_iommu_*(), to attach
iommu for ARM32 SoCs. This patch convert to common iommu API
so it would support ARM64 like RK3399.

Since previous patch added support for direct IOMMU address space
management, there is no need to use DMA API anymore and this patch wires
things to use the new method.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 +++++++++++++++-------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index c30d649..7a610e9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -14,19 +14,19 @@
  * GNU General Public License for more details.
  */
 
-#include <asm/dma-iommu.h>
-
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
 #include <linux/console.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -50,28 +50,31 @@
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev)
 {
-	struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
+	struct rockchip_drm_private *private = drm_dev->dev_private;
 	int ret;
 
 	if (!is_support_iommu)
 		return 0;
 
-	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
-	if (ret)
+	ret = iommu_attach_device(private->domain, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach iommu device\n");
 		return ret;
+	}
 
-	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-	return arm_iommu_attach_device(dev, mapping);
+	return 0;
 }
 
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev)
 {
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
+
 	if (!is_support_iommu)
 		return;
 
-	arm_iommu_detach_device(dev);
+	iommu_detach_device(domain, dev);
 }
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
@@ -123,11 +126,45 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
 		priv->crtc_funcs[pipe]->disable_vblank(crtc);
 }
 
+static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain_geometry *geometry;
+	u64 start, end;
+
+	if (!is_support_iommu)
+		return 0;
+
+	private->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!private->domain)
+		return -ENOMEM;
+
+	geometry = &private->domain->geometry;
+	start = geometry->aperture_start;
+	end = geometry->aperture_end;
+
+	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
+		  start, end);
+	drm_mm_init(&private->mm, start, end - start + 1);
+
+	return 0;
+}
+
+static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+
+	if (!is_support_iommu)
+		return;
+
+	drm_mm_takedown(&private->mm);
+	iommu_domain_free(private->domain);
+}
+
 static int rockchip_drm_bind(struct device *dev)
 {
 	struct drm_device *drm_dev;
 	struct rockchip_drm_private *private;
-	struct dma_iommu_mapping *mapping = NULL;
 	int ret;
 
 	drm_dev = drm_dev_alloc(&rockchip_drm_driver, dev);
@@ -151,38 +188,14 @@ static int rockchip_drm_bind(struct device *dev)
 
 	rockchip_drm_mode_config_init(drm_dev);
 
-	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
-				      GFP_KERNEL);
-	if (!dev->dma_parms) {
-		ret = -ENOMEM;
+	ret = rockchip_drm_init_iommu(drm_dev);
+	if (ret)
 		goto err_config_cleanup;
-	}
-
-	if (is_support_iommu) {
-		/* TODO(djkurtz): fetch the mapping start/size from somewhere */
-		mapping = arm_iommu_create_mapping(&platform_bus_type,
-						   0x00000000,
-						   SZ_2G);
-		if (IS_ERR(mapping)) {
-			ret = PTR_ERR(mapping);
-			goto err_config_cleanup;
-		}
-
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret)
-			goto err_release_mapping;
-
-		dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-		ret = arm_iommu_attach_device(dev, mapping);
-		if (ret)
-			goto err_release_mapping;
-	}
 
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(dev, drm_dev);
 	if (ret)
-		goto err_detach_device;
+		goto err_iommu_cleanup;
 
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(drm_dev);
@@ -207,8 +220,6 @@ static int rockchip_drm_bind(struct device *dev)
 	if (ret)
 		goto err_fbdev_fini;
 
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
 	return 0;
 err_fbdev_fini:
 	rockchip_drm_fbdev_fini(drm_dev);
@@ -217,12 +228,8 @@ static int rockchip_drm_bind(struct device *dev)
 err_kms_helper_poll_fini:
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-err_detach_device:
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
-err_release_mapping:
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
+err_iommu_cleanup:
+	rockchip_iommu_cleanup(drm_dev);
 err_config_cleanup:
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
@@ -239,8 +246,7 @@ static void rockchip_drm_unbind(struct device *dev)
 	drm_vblank_cleanup(drm_dev);
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
+	rockchip_iommu_cleanup(drm_dev);
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
 	drm_dev_unregister(drm_dev);
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/rockchip: Use common IOMMU API to attach devices

[Mark Yao]

From: Shunqian Zheng <zhengsq@rock-chips.com>

Rockchip DRM used the arm special API, arm_iommu_*(), to attach
iommu for ARM32 SoCs. This patch convert to common iommu API
so it would support ARM64 like RK3399.

Since previous patch added support for direct IOMMU address space
management, there is no need to use DMA API anymore and this patch wires
things to use the new method.

Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 100 +++++++++++++++-------------
 1 file changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index c30d649..7a610e9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -14,19 +14,19 @@
  * GNU General Public License for more details.
  */
 
-#include <asm/dma-iommu.h>
-
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_of.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-iommu.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/component.h>
 #include <linux/console.h>
+#include <linux/iommu.h>
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_fb.h"
@@ -50,28 +50,31 @@
 int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev)
 {
-	struct dma_iommu_mapping *mapping = drm_dev->dev->archdata.mapping;
+	struct rockchip_drm_private *private = drm_dev->dev_private;
 	int ret;
 
 	if (!is_support_iommu)
 		return 0;
 
-	ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
-	if (ret)
+	ret = iommu_attach_device(private->domain, dev);
+	if (ret) {
+		dev_err(dev, "Failed to attach iommu device\n");
 		return ret;
+	}
 
-	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-	return arm_iommu_attach_device(dev, mapping);
+	return 0;
 }
 
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev)
 {
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain *domain = private->domain;
+
 	if (!is_support_iommu)
 		return;
 
-	arm_iommu_detach_device(dev);
+	iommu_detach_device(domain, dev);
 }
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
@@ -123,11 +126,45 @@ static void rockchip_drm_crtc_disable_vblank(struct drm_device *dev,
 		priv->crtc_funcs[pipe]->disable_vblank(crtc);
 }
 
+static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+	struct iommu_domain_geometry *geometry;
+	u64 start, end;
+
+	if (!is_support_iommu)
+		return 0;
+
+	private->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!private->domain)
+		return -ENOMEM;
+
+	geometry = &private->domain->geometry;
+	start = geometry->aperture_start;
+	end = geometry->aperture_end;
+
+	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
+		  start, end);
+	drm_mm_init(&private->mm, start, end - start + 1);
+
+	return 0;
+}
+
+static void rockchip_iommu_cleanup(struct drm_device *drm_dev)
+{
+	struct rockchip_drm_private *private = drm_dev->dev_private;
+
+	if (!is_support_iommu)
+		return;
+
+	drm_mm_takedown(&private->mm);
+	iommu_domain_free(private->domain);
+}
+
 static int rockchip_drm_bind(struct device *dev)
 {
 	struct drm_device *drm_dev;
 	struct rockchip_drm_private *private;
-	struct dma_iommu_mapping *mapping = NULL;
 	int ret;
 
 	drm_dev = drm_dev_alloc(&rockchip_drm_driver, dev);
@@ -151,38 +188,14 @@ static int rockchip_drm_bind(struct device *dev)
 
 	rockchip_drm_mode_config_init(drm_dev);
 
-	dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
-				      GFP_KERNEL);
-	if (!dev->dma_parms) {
-		ret = -ENOMEM;
+	ret = rockchip_drm_init_iommu(drm_dev);
+	if (ret)
 		goto err_config_cleanup;
-	}
-
-	if (is_support_iommu) {
-		/* TODO(djkurtz): fetch the mapping start/size from somewhere */
-		mapping = arm_iommu_create_mapping(&platform_bus_type,
-						   0x00000000,
-						   SZ_2G);
-		if (IS_ERR(mapping)) {
-			ret = PTR_ERR(mapping);
-			goto err_config_cleanup;
-		}
-
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret)
-			goto err_release_mapping;
-
-		dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
-
-		ret = arm_iommu_attach_device(dev, mapping);
-		if (ret)
-			goto err_release_mapping;
-	}
 
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(dev, drm_dev);
 	if (ret)
-		goto err_detach_device;
+		goto err_iommu_cleanup;
 
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(drm_dev);
@@ -207,8 +220,6 @@ static int rockchip_drm_bind(struct device *dev)
 	if (ret)
 		goto err_fbdev_fini;
 
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
 	return 0;
 err_fbdev_fini:
 	rockchip_drm_fbdev_fini(drm_dev);
@@ -217,12 +228,8 @@ static int rockchip_drm_bind(struct device *dev)
 err_kms_helper_poll_fini:
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-err_detach_device:
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
-err_release_mapping:
-	if (is_support_iommu)
-		arm_iommu_release_mapping(mapping);
+err_iommu_cleanup:
+	rockchip_iommu_cleanup(drm_dev);
 err_config_cleanup:
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
@@ -239,8 +246,7 @@ static void rockchip_drm_unbind(struct device *dev)
 	drm_vblank_cleanup(drm_dev);
 	drm_kms_helper_poll_fini(drm_dev);
 	component_unbind_all(dev, drm_dev);
-	if (is_support_iommu)
-		arm_iommu_detach_device(dev);
+	rockchip_iommu_cleanup(drm_dev);
 	drm_mode_config_cleanup(drm_dev);
 	drm_dev->dev_private = NULL;
 	drm_dev_unregister(drm_dev);
-- 
1.9.1

[PATCH 3/4] drm/rockchip: gem: add mutex lock for drm mm

[Mark Yao]

drm_mm_insert_node_generic and drm_mm_remove_node may access same
resource with list ops, it's not threads safe, so protect this context
with mutex lock.

Fix bug:
[49451.856244] ==================================================================
[49451.856350] BUG: KASAN: wild-memory-access on address dead000000000108
[49451.856379] Write of size 8 by task Binder:218_4/683
[49451.856417] CPU: 2 PID: 683 Comm: Binder:218_4 Not tainted 4.4.36 #62
[49451.856443] Hardware name: Rockchip RK3399 Excavator Board edp (Android) (DT)
[49451.856469] Call trace:
[49451.856519] [<ffffff900808a9d0>] dump_backtrace+0x0/0x230
[49451.856556] [<ffffff900808ac14>] show_stack+0x14/0x1c
[49451.856592] [<ffffff90084a4de0>] dump_stack+0xa0/0xc8
[49451.856633] [<ffffff900821b700>] kasan_report+0x110/0x4dc
[49451.856670] [<ffffff900821aa84>] __asan_store8+0x24/0x7c
[49451.856715] [<ffffff90086158c4>] drm_mm_insert_node_generic+0x2dc/0x464
[49451.856760] [<ffffff90086406a8>] rockchip_gem_iommu_map+0x60/0x158
[49451.856794] [<ffffff9008640bb4>] rockchip_gem_create_object+0x278/0x488
[49451.856827] [<ffffff9008641020>] rockchip_gem_create_with_handle+0x24/0x10c
[49451.856862] [<ffffff9008641364>] rockchip_gem_create_ioctl+0x3c/0x50
[49451.856896] [<ffffff900860aee4>] drm_ioctl+0x354/0x52c
[49451.856939] [<ffffff900823d948>] do_vfs_ioctl+0x670/0x78c
[49451.856976] [<ffffff900823dac4>] SyS_ioctl+0x60/0x88
[49451.857009] [<ffffff9008082ef0>] el0_svc_naked+0x24/0x28

Change-Id: I2ea377aa9ca24f70c59e2d86f2a6ad5ccb9c0891
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 ++
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 7a610e9..b360e62 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -146,6 +146,7 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
 		  start, end);
 	drm_mm_init(&private->mm, start, end - start + 1);
+	mutex_init(&private->mm_lock);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 7c123d9..adc3930 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -62,6 +62,8 @@ struct rockchip_drm_private {
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
 	struct iommu_domain *domain;
+	/* protect drm_mm on multi-threads */
+	struct mutex mm_lock;
 	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 5209392..8d27965 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -28,9 +28,13 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 	int prot = IOMMU_READ | IOMMU_WRITE;
 	ssize_t ret;
 
+	mutex_lock(&private->mm_lock);
+
 	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
 					 rk_obj->base.size, PAGE_SIZE,
 					 0, 0);
+
+	mutex_unlock(&private->mm_lock);
 	if (ret < 0) {
 		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
 		return ret;
@@ -61,8 +65,13 @@ static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
 	struct rockchip_drm_private *private = drm->dev_private;
 
 	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+
+	mutex_lock(&private->mm_lock);
+
 	drm_mm_remove_node(&rk_obj->mm);
 
+	mutex_unlock(&private->mm_lock);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH 3/4] drm/rockchip: gem: add mutex lock for drm mm

[Mark Yao]

drm_mm_insert_node_generic and drm_mm_remove_node may access same
resource with list ops, it's not threads safe, so protect this context
with mutex lock.

Fix bug:
[49451.856244] ==================================================================
[49451.856350] BUG: KASAN: wild-memory-access on address dead000000000108
[49451.856379] Write of size 8 by task Binder:218_4/683
[49451.856417] CPU: 2 PID: 683 Comm: Binder:218_4 Not tainted 4.4.36 #62
[49451.856443] Hardware name: Rockchip RK3399 Excavator Board edp (Android) (DT)
[49451.856469] Call trace:
[49451.856519] [<ffffff900808a9d0>] dump_backtrace+0x0/0x230
[49451.856556] [<ffffff900808ac14>] show_stack+0x14/0x1c
[49451.856592] [<ffffff90084a4de0>] dump_stack+0xa0/0xc8
[49451.856633] [<ffffff900821b700>] kasan_report+0x110/0x4dc
[49451.856670] [<ffffff900821aa84>] __asan_store8+0x24/0x7c
[49451.856715] [<ffffff90086158c4>] drm_mm_insert_node_generic+0x2dc/0x464
[49451.856760] [<ffffff90086406a8>] rockchip_gem_iommu_map+0x60/0x158
[49451.856794] [<ffffff9008640bb4>] rockchip_gem_create_object+0x278/0x488
[49451.856827] [<ffffff9008641020>] rockchip_gem_create_with_handle+0x24/0x10c
[49451.856862] [<ffffff9008641364>] rockchip_gem_create_ioctl+0x3c/0x50
[49451.856896] [<ffffff900860aee4>] drm_ioctl+0x354/0x52c
[49451.856939] [<ffffff900823d948>] do_vfs_ioctl+0x670/0x78c
[49451.856976] [<ffffff900823dac4>] SyS_ioctl+0x60/0x88
[49451.857009] [<ffffff9008082ef0>] el0_svc_naked+0x24/0x28

Change-Id: I2ea377aa9ca24f70c59e2d86f2a6ad5ccb9c0891
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 ++
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 7a610e9..b360e62 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -146,6 +146,7 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
 		  start, end);
 	drm_mm_init(&private->mm, start, end - start + 1);
+	mutex_init(&private->mm_lock);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 7c123d9..adc3930 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -62,6 +62,8 @@ struct rockchip_drm_private {
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
 	struct iommu_domain *domain;
+	/* protect drm_mm on multi-threads */
+	struct mutex mm_lock;
 	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 5209392..8d27965 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -28,9 +28,13 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 	int prot = IOMMU_READ | IOMMU_WRITE;
 	ssize_t ret;
 
+	mutex_lock(&private->mm_lock);
+
 	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
 					 rk_obj->base.size, PAGE_SIZE,
 					 0, 0);
+
+	mutex_unlock(&private->mm_lock);
 	if (ret < 0) {
 		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
 		return ret;
@@ -61,8 +65,13 @@ static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
 	struct rockchip_drm_private *private = drm->dev_private;
 
 	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+
+	mutex_lock(&private->mm_lock);
+
 	drm_mm_remove_node(&rk_obj->mm);
 
+	mutex_unlock(&private->mm_lock);
+
 	return 0;
 }
 
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/rockchip: gem: add mutex lock for drm mm

[Mark Yao]

drm_mm_insert_node_generic and drm_mm_remove_node may access same
resource with list ops, it's not threads safe, so protect this context
with mutex lock.

Fix bug:
[49451.856244] ==================================================================
[49451.856350] BUG: KASAN: wild-memory-access on address dead000000000108
[49451.856379] Write of size 8 by task Binder:218_4/683
[49451.856417] CPU: 2 PID: 683 Comm: Binder:218_4 Not tainted 4.4.36 #62
[49451.856443] Hardware name: Rockchip RK3399 Excavator Board edp (Android) (DT)
[49451.856469] Call trace:
[49451.856519] [<ffffff900808a9d0>] dump_backtrace+0x0/0x230
[49451.856556] [<ffffff900808ac14>] show_stack+0x14/0x1c
[49451.856592] [<ffffff90084a4de0>] dump_stack+0xa0/0xc8
[49451.856633] [<ffffff900821b700>] kasan_report+0x110/0x4dc
[49451.856670] [<ffffff900821aa84>] __asan_store8+0x24/0x7c
[49451.856715] [<ffffff90086158c4>] drm_mm_insert_node_generic+0x2dc/0x464
[49451.856760] [<ffffff90086406a8>] rockchip_gem_iommu_map+0x60/0x158
[49451.856794] [<ffffff9008640bb4>] rockchip_gem_create_object+0x278/0x488
[49451.856827] [<ffffff9008641020>] rockchip_gem_create_with_handle+0x24/0x10c
[49451.856862] [<ffffff9008641364>] rockchip_gem_create_ioctl+0x3c/0x50
[49451.856896] [<ffffff900860aee4>] drm_ioctl+0x354/0x52c
[49451.856939] [<ffffff900823d948>] do_vfs_ioctl+0x670/0x78c
[49451.856976] [<ffffff900823dac4>] SyS_ioctl+0x60/0x88
[49451.857009] [<ffffff9008082ef0>] el0_svc_naked+0x24/0x28

Change-Id: I2ea377aa9ca24f70c59e2d86f2a6ad5ccb9c0891
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 ++
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 7a610e9..b360e62 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -146,6 +146,7 @@ static int rockchip_drm_init_iommu(struct drm_device *drm_dev)
 	DRM_DEBUG("IOMMU context initialized (aperture: %#llx-%#llx)\n",
 		  start, end);
 	drm_mm_init(&private->mm, start, end - start + 1);
+	mutex_init(&private->mm_lock);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 7c123d9..adc3930 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -62,6 +62,8 @@ struct rockchip_drm_private {
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
 	struct iommu_domain *domain;
+	/* protect drm_mm on multi-threads */
+	struct mutex mm_lock;
 	struct drm_mm mm;
 	struct list_head psr_list;
 	spinlock_t psr_list_lock;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 5209392..8d27965 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -28,9 +28,13 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 	int prot = IOMMU_READ | IOMMU_WRITE;
 	ssize_t ret;
 
+	mutex_lock(&private->mm_lock);
+
 	ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
 					 rk_obj->base.size, PAGE_SIZE,
 					 0, 0);
+
+	mutex_unlock(&private->mm_lock);
 	if (ret < 0) {
 		DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
 		return ret;
@@ -61,8 +65,13 @@ static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
 	struct rockchip_drm_private *private = drm->dev_private;
 
 	iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
+
+	mutex_lock(&private->mm_lock);
+
 	drm_mm_remove_node(&rk_obj->mm);
 
+	mutex_unlock(&private->mm_lock);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH 4/4] drm/rockchip: gem: fixup iommu_map_sg error path

[Mark Yao]

The return value of iommu_map_sg is size_t, it's unsigned,
So check ret < 0 is wrong.

And if iommu_map_sg is error, it's return value is zero, but
rockchip_gem_iommu_map feel the zero return value is success,
bug happen:

[    5.227458] [drm:rockchip_gem_iommu_map] *ERROR* failed to map buffer: 0
[   12.291590] WARNING: at drivers/gpu/drm/drm_mm.c:369
[   12.291611] Modules linked in:
[   12.291634]
[   12.291658] CPU: 4 PID: 338 Comm: cameraserver Not tainted 4.4.41 #196
[   12.291680] Hardware name: rockchip,rk3399-mid (DT)
[   12.291703] task: ffffffc0e5a23100 ti: ffffffc0e5a64000 task.ti: ffffffc0e5a64000
[   12.291739] PC is at drm_mm_remove_node+0xc/0xf8
[   12.291766] LR is at rockchip_gem_iommu_unmap+0x3c/0x54
[   12.303799] [<ffffff80084526e0>] drm_mm_remove_node+0xc/0xf8
[   12.303827] [<ffffff8008475430>] rockchip_gem_free_object+0x98/0x168
[   12.303854] [<ffffff8008449e80>] drm_gem_object_free+0x2c/0x34
[   12.303878] [<ffffff80084626c4>] drm_gem_dmabuf_release+0x90/0xa4
[   12.303904] [<ffffff80084ee73c>] dma_buf_release+0x64/0x15c
[   12.303929] [<ffffff80081aa8dc>] __fput+0xe0/0x1a4
[   12.303950] [<ffffff80081aa9f8>] ____fput+0xc/0x14
[   12.303977] [<ffffff80080b65ec>] task_work_run+0xa0/0xc0
[   12.304004] [<ffffff8008087f18>] do_notify_resume+0x40/0x54
[   12.304026] [<ffffff80080825e4>] work_pending+0x10/0x14

Change-Id: Id79c052691270553c1c60086f9926f39a5296354
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 8d27965..cc48673 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -44,8 +44,10 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 
 	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
 			   rk_obj->sgt->nents, prot);
-	if (ret < 0) {
-		DRM_ERROR("failed to map buffer: %zd\n", ret);
+	if (ret < rk_obj->base.size) {
+		DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
+			  ret, rk_obj->base.size);
+		ret = -ENOMEM;
 		goto err_remove_node;
 	}
 
-- 
1.9.1

[PATCH 4/4] drm/rockchip: gem: fixup iommu_map_sg error path

[Mark Yao]

The return value of iommu_map_sg is size_t, it's unsigned,
So check ret < 0 is wrong.

And if iommu_map_sg is error, it's return value is zero, but
rockchip_gem_iommu_map feel the zero return value is success,
bug happen:

[    5.227458] [drm:rockchip_gem_iommu_map] *ERROR* failed to map buffer: 0
[   12.291590] WARNING: at drivers/gpu/drm/drm_mm.c:369
[   12.291611] Modules linked in:
[   12.291634]
[   12.291658] CPU: 4 PID: 338 Comm: cameraserver Not tainted 4.4.41 #196
[   12.291680] Hardware name: rockchip,rk3399-mid (DT)
[   12.291703] task: ffffffc0e5a23100 ti: ffffffc0e5a64000 task.ti: ffffffc0e5a64000
[   12.291739] PC is at drm_mm_remove_node+0xc/0xf8
[   12.291766] LR is at rockchip_gem_iommu_unmap+0x3c/0x54
[   12.303799] [<ffffff80084526e0>] drm_mm_remove_node+0xc/0xf8
[   12.303827] [<ffffff8008475430>] rockchip_gem_free_object+0x98/0x168
[   12.303854] [<ffffff8008449e80>] drm_gem_object_free+0x2c/0x34
[   12.303878] [<ffffff80084626c4>] drm_gem_dmabuf_release+0x90/0xa4
[   12.303904] [<ffffff80084ee73c>] dma_buf_release+0x64/0x15c
[   12.303929] [<ffffff80081aa8dc>] __fput+0xe0/0x1a4
[   12.303950] [<ffffff80081aa9f8>] ____fput+0xc/0x14
[   12.303977] [<ffffff80080b65ec>] task_work_run+0xa0/0xc0
[   12.304004] [<ffffff8008087f18>] do_notify_resume+0x40/0x54
[   12.304026] [<ffffff80080825e4>] work_pending+0x10/0x14

Change-Id: Id79c052691270553c1c60086f9926f39a5296354
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 8d27965..cc48673 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -44,8 +44,10 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 
 	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
 			   rk_obj->sgt->nents, prot);
-	if (ret < 0) {
-		DRM_ERROR("failed to map buffer: %zd\n", ret);
+	if (ret < rk_obj->base.size) {
+		DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
+			  ret, rk_obj->base.size);
+		ret = -ENOMEM;
 		goto err_remove_node;
 	}
 
-- 
1.9.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/rockchip: gem: fixup iommu_map_sg error path

[Mark Yao]

The return value of iommu_map_sg is size_t, it's unsigned,
So check ret < 0 is wrong.

And if iommu_map_sg is error, it's return value is zero, but
rockchip_gem_iommu_map feel the zero return value is success,
bug happen:

[    5.227458] [drm:rockchip_gem_iommu_map] *ERROR* failed to map buffer: 0
[   12.291590] WARNING: at drivers/gpu/drm/drm_mm.c:369
[   12.291611] Modules linked in:
[   12.291634]
[   12.291658] CPU: 4 PID: 338 Comm: cameraserver Not tainted 4.4.41 #196
[   12.291680] Hardware name: rockchip,rk3399-mid (DT)
[   12.291703] task: ffffffc0e5a23100 ti: ffffffc0e5a64000 task.ti: ffffffc0e5a64000
[   12.291739] PC is at drm_mm_remove_node+0xc/0xf8
[   12.291766] LR is@rockchip_gem_iommu_unmap+0x3c/0x54
[   12.303799] [<ffffff80084526e0>] drm_mm_remove_node+0xc/0xf8
[   12.303827] [<ffffff8008475430>] rockchip_gem_free_object+0x98/0x168
[   12.303854] [<ffffff8008449e80>] drm_gem_object_free+0x2c/0x34
[   12.303878] [<ffffff80084626c4>] drm_gem_dmabuf_release+0x90/0xa4
[   12.303904] [<ffffff80084ee73c>] dma_buf_release+0x64/0x15c
[   12.303929] [<ffffff80081aa8dc>] __fput+0xe0/0x1a4
[   12.303950] [<ffffff80081aa9f8>] ____fput+0xc/0x14
[   12.303977] [<ffffff80080b65ec>] task_work_run+0xa0/0xc0
[   12.304004] [<ffffff8008087f18>] do_notify_resume+0x40/0x54
[   12.304026] [<ffffff80080825e4>] work_pending+0x10/0x14

Change-Id: Id79c052691270553c1c60086f9926f39a5296354
Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
index 8d27965..cc48673 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
@@ -44,8 +44,10 @@ static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
 
 	ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
 			   rk_obj->sgt->nents, prot);
-	if (ret < 0) {
-		DRM_ERROR("failed to map buffer: %zd\n", ret);
+	if (ret < rk_obj->base.size) {
+		DRM_ERROR("failed to map buffer: size=%zd request_size=%zd\n",
+			  ret, rk_obj->base.size);
+		ret = -ENOMEM;
 		goto err_remove_node;
 	}
 
-- 
1.9.1

Re: [PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Tomasz Figa]

Hi Mark,

Thanks for reviving this series and sorry for not taking care of it
myself. Please see some comments inline.

On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> The API is not suitable for subsystems consisting of multiple devices
> and requires severe hacks to use it. To mitigate this, this patch
> implements allocation and address space management locally by using
> helpers provided by DRM framework, like other DRM drivers do, e.g.
> Tegra.
>
> This patch should not introduce any functional changes until the driver
> is made to attach subdevices into an IOMMU domain with the generic IOMMU
> API, which will happen in following patch. Based heavily on GEM
> implementation of Tegra DRM driver.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Acked-by: Mark Yao <mark.yao@rock-chips.com>

I believe you need your Signed-off-by here.

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>  3 files changed, 219 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index fb6226c..7c123d9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -30,6 +30,7 @@
>
>  struct drm_device;
>  struct drm_connector;
> +struct iommu_domain;
>
>  /*
>   * Rockchip drm private crtc funcs.
> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>         struct drm_gem_object *fbdev_bo;
>         const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>         struct drm_atomic_state *state;
> -
> +       struct iommu_domain *domain;
> +       struct drm_mm mm;
>         struct list_head psr_list;
>         spinlock_t psr_list_lock;
>  };
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index b70f942..5209392 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -16,11 +16,135 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_vma_manager.h>
> +#include <linux/iommu.h>
>
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_gem.h"
>
> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +       int prot = IOMMU_READ | IOMMU_WRITE;
> +       ssize_t ret;
> +
> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
> +                                        rk_obj->base.size, PAGE_SIZE,
> +                                        0, 0);
> +       if (ret < 0) {
> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
> +               return ret;
> +       }
> +
> +       rk_obj->dma_addr = rk_obj->mm.start;
> +
> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
> +                          rk_obj->sgt->nents, prot);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
> +               goto err_remove_node;
> +       }
> +
> +       rk_obj->size = ret;
> +
> +       return 0;
> +
> +err_remove_node:
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return 0;
> +}
> +
> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       int ret, i;
> +       struct scatterlist *s;
> +
> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
> +       if (IS_ERR(rk_obj->pages))
> +               return PTR_ERR(rk_obj->pages);
> +
> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
> +
> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
> +       if (IS_ERR(rk_obj->sgt)) {
> +               ret = PTR_ERR(rk_obj->sgt);
> +               goto err_put_pages;
> +       }
> +
> +       /*
> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
> +        * to flush the pages associated with it.
> +        *
> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
> +        * without relying on symbols that are not exported.
> +        */
> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
> +               sg_dma_address(s) = sg_phys(s);
> +
> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
> +                              DMA_TO_DEVICE);
> +
> +       return 0;
> +
> +err_put_pages:
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> +       return ret;
> +}
> +
> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       sg_free_table(rk_obj->sgt);
> +       kfree(rk_obj->sgt);
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);

We need to call drm_gem_put_pages with two last arguments true, not
false, because we don't have any way of tracking the accesses, so we
need to assume the page was both accessed and dirty. See
https://chromium-review.googlesource.com/382934.

> +}
> +
> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
> +                                   bool alloc_kmap)
> +{
> +       int ret;
> +
> +       ret = rockchip_gem_get_pages(rk_obj);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = rockchip_gem_iommu_map(rk_obj);
> +       if (ret < 0)
> +               goto err_free;
> +
> +       if (alloc_kmap) {
> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
> +                                     pgprot_writecombine(PAGE_KERNEL));
> +               if (!rk_obj->kvaddr) {
> +                       DRM_ERROR("failed to vmap() buffer\n");
> +                       ret = -ENOMEM;
> +                       goto err_unmap;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_unmap:
> +       rockchip_gem_iommu_unmap(rk_obj);
> +err_free:
> +       rockchip_gem_put_pages(rk_obj);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>                                   bool alloc_kmap)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>         return 0;
>  }
>
> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +                                 bool alloc_kmap)
> +{
> +       struct drm_gem_object *obj = &rk_obj->base;
> +       struct drm_device *drm = obj->dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       if (private->domain)
> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
> +       else
> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
> +}
> +
> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
> +{
> +       vunmap(rk_obj->kvaddr);
> +       rockchip_gem_iommu_unmap(rk_obj);
> +       rockchip_gem_put_pages(rk_obj);
> +}
> +
> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
>         struct drm_device *drm = obj->dev;
> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>                        rk_obj->dma_attrs);
>  }
>
> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> -                                       struct vm_area_struct *vma)
> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +{
> +       if (rk_obj->pages)
> +               rockchip_gem_free_iommu(rk_obj);
> +       else
> +               rockchip_gem_free_dma(rk_obj);
> +}
>
> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
> +                                             struct vm_area_struct *vma)
>  {
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +       unsigned long uaddr = vma->vm_start;
>         int ret;
> +
> +       if (user_count == 0 || user_count > count)
> +               return -ENXIO;
> +
> +       for (i = 0; i < user_count; i++) {

Even though DRM mmap doesn't support page offset, we need to respect
it here for PRIME/DMA-buf mmap. See
https://chromium-review.googlesource.com/386477.

> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
> +               if (ret)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return 0;
> +}
> +
> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
> +                                           struct vm_area_struct *vma)
> +{
>         struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>         struct drm_device *drm = obj->dev;
>
> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> +                             obj->size, rk_obj->dma_attrs);
> +}
> +
> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> +                                       struct vm_area_struct *vma)
> +{
> +       int ret;
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +
>         /*
> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
> +        * We allocated a struct page table for rk_obj, so clear
>          * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>          */
>         vma->vm_flags &= ~VM_PFNMAP;
>         vma->vm_pgoff = 0;
>
> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> -                            obj->size, rk_obj->dma_attrs);
> +       if (rk_obj->pages)
> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
> +       else
> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
> +
>         if (ret)
>                 drm_gem_vm_close(vma);
>
> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>
>         obj = &rk_obj->base;
>
> -       drm_gem_private_object_init(drm, obj, size);
> +       drm_gem_object_init(drm, obj, size);

In error path of this function and in all functions that destroy the
GEM we need to call drm_gem_object_release(), as it's the counterpart
to drm_gem_object_init(). See
https://chromium-review.googlesource.com/385456.

Best regards,
Tomasz

Re: [PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Tomasz Figa]

Hi Mark,

Thanks for reviving this series and sorry for not taking care of it
myself. Please see some comments inline.

On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> The API is not suitable for subsystems consisting of multiple devices
> and requires severe hacks to use it. To mitigate this, this patch
> implements allocation and address space management locally by using
> helpers provided by DRM framework, like other DRM drivers do, e.g.
> Tegra.
>
> This patch should not introduce any functional changes until the driver
> is made to attach subdevices into an IOMMU domain with the generic IOMMU
> API, which will happen in following patch. Based heavily on GEM
> implementation of Tegra DRM driver.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Acked-by: Mark Yao <mark.yao@rock-chips.com>

I believe you need your Signed-off-by here.

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>  3 files changed, 219 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index fb6226c..7c123d9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -30,6 +30,7 @@
>
>  struct drm_device;
>  struct drm_connector;
> +struct iommu_domain;
>
>  /*
>   * Rockchip drm private crtc funcs.
> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>         struct drm_gem_object *fbdev_bo;
>         const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>         struct drm_atomic_state *state;
> -
> +       struct iommu_domain *domain;
> +       struct drm_mm mm;
>         struct list_head psr_list;
>         spinlock_t psr_list_lock;
>  };
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index b70f942..5209392 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -16,11 +16,135 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_vma_manager.h>
> +#include <linux/iommu.h>
>
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_gem.h"
>
> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +       int prot = IOMMU_READ | IOMMU_WRITE;
> +       ssize_t ret;
> +
> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
> +                                        rk_obj->base.size, PAGE_SIZE,
> +                                        0, 0);
> +       if (ret < 0) {
> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
> +               return ret;
> +       }
> +
> +       rk_obj->dma_addr = rk_obj->mm.start;
> +
> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
> +                          rk_obj->sgt->nents, prot);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
> +               goto err_remove_node;
> +       }
> +
> +       rk_obj->size = ret;
> +
> +       return 0;
> +
> +err_remove_node:
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return 0;
> +}
> +
> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       int ret, i;
> +       struct scatterlist *s;
> +
> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
> +       if (IS_ERR(rk_obj->pages))
> +               return PTR_ERR(rk_obj->pages);
> +
> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
> +
> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
> +       if (IS_ERR(rk_obj->sgt)) {
> +               ret = PTR_ERR(rk_obj->sgt);
> +               goto err_put_pages;
> +       }
> +
> +       /*
> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
> +        * to flush the pages associated with it.
> +        *
> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
> +        * without relying on symbols that are not exported.
> +        */
> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
> +               sg_dma_address(s) = sg_phys(s);
> +
> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
> +                              DMA_TO_DEVICE);
> +
> +       return 0;
> +
> +err_put_pages:
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> +       return ret;
> +}
> +
> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       sg_free_table(rk_obj->sgt);
> +       kfree(rk_obj->sgt);
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);

We need to call drm_gem_put_pages with two last arguments true, not
false, because we don't have any way of tracking the accesses, so we
need to assume the page was both accessed and dirty. See
https://chromium-review.googlesource.com/382934.

> +}
> +
> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
> +                                   bool alloc_kmap)
> +{
> +       int ret;
> +
> +       ret = rockchip_gem_get_pages(rk_obj);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = rockchip_gem_iommu_map(rk_obj);
> +       if (ret < 0)
> +               goto err_free;
> +
> +       if (alloc_kmap) {
> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
> +                                     pgprot_writecombine(PAGE_KERNEL));
> +               if (!rk_obj->kvaddr) {
> +                       DRM_ERROR("failed to vmap() buffer\n");
> +                       ret = -ENOMEM;
> +                       goto err_unmap;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_unmap:
> +       rockchip_gem_iommu_unmap(rk_obj);
> +err_free:
> +       rockchip_gem_put_pages(rk_obj);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>                                   bool alloc_kmap)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>         return 0;
>  }
>
> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +                                 bool alloc_kmap)
> +{
> +       struct drm_gem_object *obj = &rk_obj->base;
> +       struct drm_device *drm = obj->dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       if (private->domain)
> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
> +       else
> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
> +}
> +
> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
> +{
> +       vunmap(rk_obj->kvaddr);
> +       rockchip_gem_iommu_unmap(rk_obj);
> +       rockchip_gem_put_pages(rk_obj);
> +}
> +
> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
>         struct drm_device *drm = obj->dev;
> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>                        rk_obj->dma_attrs);
>  }
>
> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> -                                       struct vm_area_struct *vma)
> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +{
> +       if (rk_obj->pages)
> +               rockchip_gem_free_iommu(rk_obj);
> +       else
> +               rockchip_gem_free_dma(rk_obj);
> +}
>
> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
> +                                             struct vm_area_struct *vma)
>  {
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +       unsigned long uaddr = vma->vm_start;
>         int ret;
> +
> +       if (user_count == 0 || user_count > count)
> +               return -ENXIO;
> +
> +       for (i = 0; i < user_count; i++) {

Even though DRM mmap doesn't support page offset, we need to respect
it here for PRIME/DMA-buf mmap. See
https://chromium-review.googlesource.com/386477.

> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
> +               if (ret)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return 0;
> +}
> +
> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
> +                                           struct vm_area_struct *vma)
> +{
>         struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>         struct drm_device *drm = obj->dev;
>
> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> +                             obj->size, rk_obj->dma_attrs);
> +}
> +
> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> +                                       struct vm_area_struct *vma)
> +{
> +       int ret;
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +
>         /*
> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
> +        * We allocated a struct page table for rk_obj, so clear
>          * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>          */
>         vma->vm_flags &= ~VM_PFNMAP;
>         vma->vm_pgoff = 0;
>
> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> -                            obj->size, rk_obj->dma_attrs);
> +       if (rk_obj->pages)
> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
> +       else
> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
> +
>         if (ret)
>                 drm_gem_vm_close(vma);
>
> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>
>         obj = &rk_obj->base;
>
> -       drm_gem_private_object_init(drm, obj, size);
> +       drm_gem_object_init(drm, obj, size);

In error path of this function and in all functions that destroy the
GEM we need to call drm_gem_object_release(), as it's the counterpart
to drm_gem_object_init(). See
https://chromium-review.googlesource.com/385456.

Best regards,
Tomasz
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Tomasz Figa]

Hi Mark,

Thanks for reviving this series and sorry for not taking care of it
myself. Please see some comments inline.

On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> The API is not suitable for subsystems consisting of multiple devices
> and requires severe hacks to use it. To mitigate this, this patch
> implements allocation and address space management locally by using
> helpers provided by DRM framework, like other DRM drivers do, e.g.
> Tegra.
>
> This patch should not introduce any functional changes until the driver
> is made to attach subdevices into an IOMMU domain with the generic IOMMU
> API, which will happen in following patch. Based heavily on GEM
> implementation of Tegra DRM driver.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
> Acked-by: Mark Yao <mark.yao@rock-chips.com>

I believe you need your Signed-off-by here.

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>  drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>  3 files changed, 219 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index fb6226c..7c123d9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -30,6 +30,7 @@
>
>  struct drm_device;
>  struct drm_connector;
> +struct iommu_domain;
>
>  /*
>   * Rockchip drm private crtc funcs.
> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>         struct drm_gem_object *fbdev_bo;
>         const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>         struct drm_atomic_state *state;
> -
> +       struct iommu_domain *domain;
> +       struct drm_mm mm;
>         struct list_head psr_list;
>         spinlock_t psr_list_lock;
>  };
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index b70f942..5209392 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -16,11 +16,135 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_gem.h>
>  #include <drm/drm_vma_manager.h>
> +#include <linux/iommu.h>
>
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_gem.h"
>
> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +       int prot = IOMMU_READ | IOMMU_WRITE;
> +       ssize_t ret;
> +
> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
> +                                        rk_obj->base.size, PAGE_SIZE,
> +                                        0, 0);
> +       if (ret < 0) {
> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
> +               return ret;
> +       }
> +
> +       rk_obj->dma_addr = rk_obj->mm.start;
> +
> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
> +                          rk_obj->sgt->nents, prot);
> +       if (ret < 0) {
> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
> +               goto err_remove_node;
> +       }
> +
> +       rk_obj->size = ret;
> +
> +       return 0;
> +
> +err_remove_node:
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
> +       drm_mm_remove_node(&rk_obj->mm);
> +
> +       return 0;
> +}
> +
> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       struct drm_device *drm = rk_obj->base.dev;
> +       int ret, i;
> +       struct scatterlist *s;
> +
> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
> +       if (IS_ERR(rk_obj->pages))
> +               return PTR_ERR(rk_obj->pages);
> +
> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
> +
> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
> +       if (IS_ERR(rk_obj->sgt)) {
> +               ret = PTR_ERR(rk_obj->sgt);
> +               goto err_put_pages;
> +       }
> +
> +       /*
> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
> +        * to flush the pages associated with it.
> +        *
> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
> +        * without relying on symbols that are not exported.
> +        */
> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
> +               sg_dma_address(s) = sg_phys(s);
> +
> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
> +                              DMA_TO_DEVICE);
> +
> +       return 0;
> +
> +err_put_pages:
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> +       return ret;
> +}
> +
> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
> +{
> +       sg_free_table(rk_obj->sgt);
> +       kfree(rk_obj->sgt);
> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);

We need to call drm_gem_put_pages with two last arguments true, not
false, because we don't have any way of tracking the accesses, so we
need to assume the page was both accessed and dirty. See
https://chromium-review.googlesource.com/382934.

> +}
> +
> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
> +                                   bool alloc_kmap)
> +{
> +       int ret;
> +
> +       ret = rockchip_gem_get_pages(rk_obj);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = rockchip_gem_iommu_map(rk_obj);
> +       if (ret < 0)
> +               goto err_free;
> +
> +       if (alloc_kmap) {
> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
> +                                     pgprot_writecombine(PAGE_KERNEL));
> +               if (!rk_obj->kvaddr) {
> +                       DRM_ERROR("failed to vmap() buffer\n");
> +                       ret = -ENOMEM;
> +                       goto err_unmap;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_unmap:
> +       rockchip_gem_iommu_unmap(rk_obj);
> +err_free:
> +       rockchip_gem_put_pages(rk_obj);
> +
> +       return ret;
> +}
> +
> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>                                   bool alloc_kmap)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>         return 0;
>  }
>
> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
> +                                 bool alloc_kmap)
> +{
> +       struct drm_gem_object *obj = &rk_obj->base;
> +       struct drm_device *drm = obj->dev;
> +       struct rockchip_drm_private *private = drm->dev_private;
> +
> +       if (private->domain)
> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
> +       else
> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
> +}
> +
> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
> +{
> +       vunmap(rk_obj->kvaddr);
> +       rockchip_gem_iommu_unmap(rk_obj);
> +       rockchip_gem_put_pages(rk_obj);
> +}
> +
> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>  {
>         struct drm_gem_object *obj = &rk_obj->base;
>         struct drm_device *drm = obj->dev;
> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>                        rk_obj->dma_attrs);
>  }
>
> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> -                                       struct vm_area_struct *vma)
> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
> +{
> +       if (rk_obj->pages)
> +               rockchip_gem_free_iommu(rk_obj);
> +       else
> +               rockchip_gem_free_dma(rk_obj);
> +}
>
> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
> +                                             struct vm_area_struct *vma)
>  {
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +       unsigned long uaddr = vma->vm_start;
>         int ret;
> +
> +       if (user_count == 0 || user_count > count)
> +               return -ENXIO;
> +
> +       for (i = 0; i < user_count; i++) {

Even though DRM mmap doesn't support page offset, we need to respect
it here for PRIME/DMA-buf mmap. See
https://chromium-review.googlesource.com/386477.

> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
> +               if (ret)
> +                       return ret;
> +               uaddr += PAGE_SIZE;
> +       }
> +
> +       return 0;
> +}
> +
> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
> +                                           struct vm_area_struct *vma)
> +{
>         struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>         struct drm_device *drm = obj->dev;
>
> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> +                             obj->size, rk_obj->dma_attrs);
> +}
> +
> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
> +                                       struct vm_area_struct *vma)
> +{
> +       int ret;
> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
> +
>         /*
> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
> +        * We allocated a struct page table for rk_obj, so clear
>          * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>          */
>         vma->vm_flags &= ~VM_PFNMAP;
>         vma->vm_pgoff = 0;
>
> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
> -                            obj->size, rk_obj->dma_attrs);
> +       if (rk_obj->pages)
> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
> +       else
> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
> +
>         if (ret)
>                 drm_gem_vm_close(vma);
>
> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>
>         obj = &rk_obj->base;
>
> -       drm_gem_private_object_init(drm, obj, size);
> +       drm_gem_object_init(drm, obj, size);

In error path of this function and in all functions that destroy the
GEM we need to call drm_gem_object_release(), as it's the counterpart
to drm_gem_object_init(). See
https://chromium-review.googlesource.com/385456.

Best regards,
Tomasz

Re: [PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark yao]

On 2017年02月07日 14:53, Tomasz Figa wrote:
> Hi Mark,
>
> Thanks for reviving this series and sorry for not taking care of it
> myself. Please see some comments inline.

Hi Tomasz

Thanks for review,

I will add the patches you mentioned  into v2 version.

>
> On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
>> From: Tomasz Figa <tfiga@chromium.org>
>>
>> The API is not suitable for subsystems consisting of multiple devices
>> and requires severe hacks to use it. To mitigate this, this patch
>> implements allocation and address space management locally by using
>> helpers provided by DRM framework, like other DRM drivers do, e.g.
>> Tegra.
>>
>> This patch should not introduce any functional changes until the driver
>> is made to attach subdevices into an IOMMU domain with the generic IOMMU
>> API, which will happen in following patch. Based heavily on GEM
>> implementation of Tegra DRM driver.
>>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Acked-by: Mark Yao <mark.yao@rock-chips.com>
> I believe you need your Signed-off-by here.
>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>>   3 files changed, 219 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index fb6226c..7c123d9 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -30,6 +30,7 @@
>>
>>   struct drm_device;
>>   struct drm_connector;
>> +struct iommu_domain;
>>
>>   /*
>>    * Rockchip drm private crtc funcs.
>> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>>          struct drm_gem_object *fbdev_bo;
>>          const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>>          struct drm_atomic_state *state;
>> -
>> +       struct iommu_domain *domain;
>> +       struct drm_mm mm;
>>          struct list_head psr_list;
>>          spinlock_t psr_list_lock;
>>   };
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> index b70f942..5209392 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> @@ -16,11 +16,135 @@
>>   #include <drm/drmP.h>
>>   #include <drm/drm_gem.h>
>>   #include <drm/drm_vma_manager.h>
>> +#include <linux/iommu.h>
>>
>>   #include "rockchip_drm_drv.h"
>>   #include "rockchip_drm_gem.h"
>>
>> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +       int prot = IOMMU_READ | IOMMU_WRITE;
>> +       ssize_t ret;
>> +
>> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
>> +                                        rk_obj->base.size, PAGE_SIZE,
>> +                                        0, 0);
>> +       if (ret < 0) {
>> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       rk_obj->dma_addr = rk_obj->mm.start;
>> +
>> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
>> +                          rk_obj->sgt->nents, prot);
>> +       if (ret < 0) {
>> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
>> +               goto err_remove_node;
>> +       }
>> +
>> +       rk_obj->size = ret;
>> +
>> +       return 0;
>> +
>> +err_remove_node:
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       int ret, i;
>> +       struct scatterlist *s;
>> +
>> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
>> +       if (IS_ERR(rk_obj->pages))
>> +               return PTR_ERR(rk_obj->pages);
>> +
>> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
>> +
>> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
>> +       if (IS_ERR(rk_obj->sgt)) {
>> +               ret = PTR_ERR(rk_obj->sgt);
>> +               goto err_put_pages;
>> +       }
>> +
>> +       /*
>> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
>> +        * to flush the pages associated with it.
>> +        *
>> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
>> +        * without relying on symbols that are not exported.
>> +        */
>> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
>> +               sg_dma_address(s) = sg_phys(s);
>> +
>> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
>> +                              DMA_TO_DEVICE);
>> +
>> +       return 0;
>> +
>> +err_put_pages:
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
>> +       return ret;
>> +}
>> +
>> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       sg_free_table(rk_obj->sgt);
>> +       kfree(rk_obj->sgt);
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> We need to call drm_gem_put_pages with two last arguments true, not
> false, because we don't have any way of tracking the accesses, so we
> need to assume the page was both accessed and dirty. See
> https://chromium-review.googlesource.com/382934.
>
>> +}
>> +
>> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
>> +                                   bool alloc_kmap)
>> +{
>> +       int ret;
>> +
>> +       ret = rockchip_gem_get_pages(rk_obj);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = rockchip_gem_iommu_map(rk_obj);
>> +       if (ret < 0)
>> +               goto err_free;
>> +
>> +       if (alloc_kmap) {
>> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
>> +                                     pgprot_writecombine(PAGE_KERNEL));
>> +               if (!rk_obj->kvaddr) {
>> +                       DRM_ERROR("failed to vmap() buffer\n");
>> +                       ret = -ENOMEM;
>> +                       goto err_unmap;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +
>> +err_unmap:
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +err_free:
>> +       rockchip_gem_put_pages(rk_obj);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>>                                    bool alloc_kmap)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>>          return 0;
>>   }
>>
>> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +                                 bool alloc_kmap)
>> +{
>> +       struct drm_gem_object *obj = &rk_obj->base;
>> +       struct drm_device *drm = obj->dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       if (private->domain)
>> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
>> +       else
>> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
>> +}
>> +
>> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
>> +{
>> +       vunmap(rk_obj->kvaddr);
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +       rockchip_gem_put_pages(rk_obj);
>> +}
>> +
>> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>>          struct drm_device *drm = obj->dev;
>> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>>                         rk_obj->dma_attrs);
>>   }
>>
>> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> -                                       struct vm_area_struct *vma)
>> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +{
>> +       if (rk_obj->pages)
>> +               rockchip_gem_free_iommu(rk_obj);
>> +       else
>> +               rockchip_gem_free_dma(rk_obj);
>> +}
>>
>> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
>> +                                             struct vm_area_struct *vma)
>>   {
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
>> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> +       unsigned long uaddr = vma->vm_start;
>>          int ret;
>> +
>> +       if (user_count == 0 || user_count > count)
>> +               return -ENXIO;
>> +
>> +       for (i = 0; i < user_count; i++) {
> Even though DRM mmap doesn't support page offset, we need to respect
> it here for PRIME/DMA-buf mmap. See
> https://chromium-review.googlesource.com/386477.
>
>> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
>> +               if (ret)
>> +                       return ret;
>> +               uaddr += PAGE_SIZE;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
>> +                                           struct vm_area_struct *vma)
>> +{
>>          struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>>          struct drm_device *drm = obj->dev;
>>
>> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> +                             obj->size, rk_obj->dma_attrs);
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> +                                       struct vm_area_struct *vma)
>> +{
>> +       int ret;
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +
>>          /*
>> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
>> +        * We allocated a struct page table for rk_obj, so clear
>>           * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>>           */
>>          vma->vm_flags &= ~VM_PFNMAP;
>>          vma->vm_pgoff = 0;
>>
>> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> -                            obj->size, rk_obj->dma_attrs);
>> +       if (rk_obj->pages)
>> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
>> +       else
>> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
>> +
>>          if (ret)
>>                  drm_gem_vm_close(vma);
>>
>> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>>
>>          obj = &rk_obj->base;
>>
>> -       drm_gem_private_object_init(drm, obj, size);
>> +       drm_gem_object_init(drm, obj, size);
> In error path of this function and in all functions that destroy the
> GEM we need to call drm_gem_object_release(), as it's the counterpart
> to drm_gem_object_init(). See
> https://chromium-review.googlesource.com/385456.
>
> Best regards,
> Tomasz
>
>
>


-- 
Ｍark Yao

Re: [PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark yao]

On 2017年02月07日 14:53, Tomasz Figa wrote:
> Hi Mark,
>
> Thanks for reviving this series and sorry for not taking care of it
> myself. Please see some comments inline.

Hi Tomasz

Thanks for review,

I will add the patches you mentioned  into v2 version.

>
> On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
>> From: Tomasz Figa <tfiga@chromium.org>
>>
>> The API is not suitable for subsystems consisting of multiple devices
>> and requires severe hacks to use it. To mitigate this, this patch
>> implements allocation and address space management locally by using
>> helpers provided by DRM framework, like other DRM drivers do, e.g.
>> Tegra.
>>
>> This patch should not introduce any functional changes until the driver
>> is made to attach subdevices into an IOMMU domain with the generic IOMMU
>> API, which will happen in following patch. Based heavily on GEM
>> implementation of Tegra DRM driver.
>>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Acked-by: Mark Yao <mark.yao@rock-chips.com>
> I believe you need your Signed-off-by here.
>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>>   3 files changed, 219 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index fb6226c..7c123d9 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -30,6 +30,7 @@
>>
>>   struct drm_device;
>>   struct drm_connector;
>> +struct iommu_domain;
>>
>>   /*
>>    * Rockchip drm private crtc funcs.
>> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>>          struct drm_gem_object *fbdev_bo;
>>          const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>>          struct drm_atomic_state *state;
>> -
>> +       struct iommu_domain *domain;
>> +       struct drm_mm mm;
>>          struct list_head psr_list;
>>          spinlock_t psr_list_lock;
>>   };
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> index b70f942..5209392 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> @@ -16,11 +16,135 @@
>>   #include <drm/drmP.h>
>>   #include <drm/drm_gem.h>
>>   #include <drm/drm_vma_manager.h>
>> +#include <linux/iommu.h>
>>
>>   #include "rockchip_drm_drv.h"
>>   #include "rockchip_drm_gem.h"
>>
>> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +       int prot = IOMMU_READ | IOMMU_WRITE;
>> +       ssize_t ret;
>> +
>> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
>> +                                        rk_obj->base.size, PAGE_SIZE,
>> +                                        0, 0);
>> +       if (ret < 0) {
>> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       rk_obj->dma_addr = rk_obj->mm.start;
>> +
>> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
>> +                          rk_obj->sgt->nents, prot);
>> +       if (ret < 0) {
>> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
>> +               goto err_remove_node;
>> +       }
>> +
>> +       rk_obj->size = ret;
>> +
>> +       return 0;
>> +
>> +err_remove_node:
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       int ret, i;
>> +       struct scatterlist *s;
>> +
>> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
>> +       if (IS_ERR(rk_obj->pages))
>> +               return PTR_ERR(rk_obj->pages);
>> +
>> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
>> +
>> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
>> +       if (IS_ERR(rk_obj->sgt)) {
>> +               ret = PTR_ERR(rk_obj->sgt);
>> +               goto err_put_pages;
>> +       }
>> +
>> +       /*
>> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
>> +        * to flush the pages associated with it.
>> +        *
>> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
>> +        * without relying on symbols that are not exported.
>> +        */
>> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
>> +               sg_dma_address(s) = sg_phys(s);
>> +
>> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
>> +                              DMA_TO_DEVICE);
>> +
>> +       return 0;
>> +
>> +err_put_pages:
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
>> +       return ret;
>> +}
>> +
>> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       sg_free_table(rk_obj->sgt);
>> +       kfree(rk_obj->sgt);
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> We need to call drm_gem_put_pages with two last arguments true, not
> false, because we don't have any way of tracking the accesses, so we
> need to assume the page was both accessed and dirty. See
> https://chromium-review.googlesource.com/382934.
>
>> +}
>> +
>> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
>> +                                   bool alloc_kmap)
>> +{
>> +       int ret;
>> +
>> +       ret = rockchip_gem_get_pages(rk_obj);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = rockchip_gem_iommu_map(rk_obj);
>> +       if (ret < 0)
>> +               goto err_free;
>> +
>> +       if (alloc_kmap) {
>> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
>> +                                     pgprot_writecombine(PAGE_KERNEL));
>> +               if (!rk_obj->kvaddr) {
>> +                       DRM_ERROR("failed to vmap() buffer\n");
>> +                       ret = -ENOMEM;
>> +                       goto err_unmap;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +
>> +err_unmap:
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +err_free:
>> +       rockchip_gem_put_pages(rk_obj);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>>                                    bool alloc_kmap)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>>          return 0;
>>   }
>>
>> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +                                 bool alloc_kmap)
>> +{
>> +       struct drm_gem_object *obj = &rk_obj->base;
>> +       struct drm_device *drm = obj->dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       if (private->domain)
>> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
>> +       else
>> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
>> +}
>> +
>> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
>> +{
>> +       vunmap(rk_obj->kvaddr);
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +       rockchip_gem_put_pages(rk_obj);
>> +}
>> +
>> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>>          struct drm_device *drm = obj->dev;
>> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>>                         rk_obj->dma_attrs);
>>   }
>>
>> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> -                                       struct vm_area_struct *vma)
>> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +{
>> +       if (rk_obj->pages)
>> +               rockchip_gem_free_iommu(rk_obj);
>> +       else
>> +               rockchip_gem_free_dma(rk_obj);
>> +}
>>
>> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
>> +                                             struct vm_area_struct *vma)
>>   {
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
>> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> +       unsigned long uaddr = vma->vm_start;
>>          int ret;
>> +
>> +       if (user_count == 0 || user_count > count)
>> +               return -ENXIO;
>> +
>> +       for (i = 0; i < user_count; i++) {
> Even though DRM mmap doesn't support page offset, we need to respect
> it here for PRIME/DMA-buf mmap. See
> https://chromium-review.googlesource.com/386477.
>
>> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
>> +               if (ret)
>> +                       return ret;
>> +               uaddr += PAGE_SIZE;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
>> +                                           struct vm_area_struct *vma)
>> +{
>>          struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>>          struct drm_device *drm = obj->dev;
>>
>> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> +                             obj->size, rk_obj->dma_attrs);
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> +                                       struct vm_area_struct *vma)
>> +{
>> +       int ret;
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +
>>          /*
>> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
>> +        * We allocated a struct page table for rk_obj, so clear
>>           * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>>           */
>>          vma->vm_flags &= ~VM_PFNMAP;
>>          vma->vm_pgoff = 0;
>>
>> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> -                            obj->size, rk_obj->dma_attrs);
>> +       if (rk_obj->pages)
>> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
>> +       else
>> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
>> +
>>          if (ret)
>>                  drm_gem_vm_close(vma);
>>
>> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>>
>>          obj = &rk_obj->base;
>>
>> -       drm_gem_private_object_init(drm, obj, size);
>> +       drm_gem_object_init(drm, obj, size);
> In error path of this function and in all functions that destroy the
> GEM we need to call drm_gem_object_release(), as it's the counterpart
> to drm_gem_object_init(). See
> https://chromium-review.googlesource.com/385456.
>
> Best regards,
> Tomasz
>
>
>


-- 
Ｍark Yao


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/rockchip: Do not use DMA mapping API if attached to IOMMU domain

[Mark yao]

On 2017?02?07? 14:53, Tomasz Figa wrote:
> Hi Mark,
>
> Thanks for reviving this series and sorry for not taking care of it
> myself. Please see some comments inline.

Hi Tomasz

Thanks for review,

I will add the patches you mentioned  into v2 version.

>
> On Tue, Feb 7, 2017 at 3:09 PM, Mark Yao <mark.yao@rock-chips.com> wrote:
>> From: Tomasz Figa <tfiga@chromium.org>
>>
>> The API is not suitable for subsystems consisting of multiple devices
>> and requires severe hacks to use it. To mitigate this, this patch
>> implements allocation and address space management locally by using
>> helpers provided by DRM framework, like other DRM drivers do, e.g.
>> Tegra.
>>
>> This patch should not introduce any functional changes until the driver
>> is made to attach subdevices into an IOMMU domain with the generic IOMMU
>> API, which will happen in following patch. Based heavily on GEM
>> implementation of Tegra DRM driver.
>>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com>
>> Acked-by: Mark Yao <mark.yao@rock-chips.com>
> I believe you need your Signed-off-by here.
>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   4 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 217 ++++++++++++++++++++++++++--
>>   drivers/gpu/drm/rockchip/rockchip_drm_gem.h |   8 +
>>   3 files changed, 219 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index fb6226c..7c123d9 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -30,6 +30,7 @@
>>
>>   struct drm_device;
>>   struct drm_connector;
>> +struct iommu_domain;
>>
>>   /*
>>    * Rockchip drm private crtc funcs.
>> @@ -60,7 +61,8 @@ struct rockchip_drm_private {
>>          struct drm_gem_object *fbdev_bo;
>>          const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>>          struct drm_atomic_state *state;
>> -
>> +       struct iommu_domain *domain;
>> +       struct drm_mm mm;
>>          struct list_head psr_list;
>>          spinlock_t psr_list_lock;
>>   };
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> index b70f942..5209392 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
>> @@ -16,11 +16,135 @@
>>   #include <drm/drmP.h>
>>   #include <drm/drm_gem.h>
>>   #include <drm/drm_vma_manager.h>
>> +#include <linux/iommu.h>
>>
>>   #include "rockchip_drm_drv.h"
>>   #include "rockchip_drm_gem.h"
>>
>> -static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +static int rockchip_gem_iommu_map(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +       int prot = IOMMU_READ | IOMMU_WRITE;
>> +       ssize_t ret;
>> +
>> +       ret = drm_mm_insert_node_generic(&private->mm, &rk_obj->mm,
>> +                                        rk_obj->base.size, PAGE_SIZE,
>> +                                        0, 0);
>> +       if (ret < 0) {
>> +               DRM_ERROR("out of I/O virtual memory: %zd\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       rk_obj->dma_addr = rk_obj->mm.start;
>> +
>> +       ret = iommu_map_sg(private->domain, rk_obj->dma_addr, rk_obj->sgt->sgl,
>> +                          rk_obj->sgt->nents, prot);
>> +       if (ret < 0) {
>> +               DRM_ERROR("failed to map buffer: %zd\n", ret);
>> +               goto err_remove_node;
>> +       }
>> +
>> +       rk_obj->size = ret;
>> +
>> +       return 0;
>> +
>> +err_remove_node:
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_iommu_unmap(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       iommu_unmap(private->domain, rk_obj->dma_addr, rk_obj->size);
>> +       drm_mm_remove_node(&rk_obj->mm);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       struct drm_device *drm = rk_obj->base.dev;
>> +       int ret, i;
>> +       struct scatterlist *s;
>> +
>> +       rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
>> +       if (IS_ERR(rk_obj->pages))
>> +               return PTR_ERR(rk_obj->pages);
>> +
>> +       rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT;
>> +
>> +       rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages);
>> +       if (IS_ERR(rk_obj->sgt)) {
>> +               ret = PTR_ERR(rk_obj->sgt);
>> +               goto err_put_pages;
>> +       }
>> +
>> +       /*
>> +        * Fake up the SG table so that dma_sync_sg_for_device() can be used
>> +        * to flush the pages associated with it.
>> +        *
>> +        * TODO: Replace this by drm_clflush_sg() once it can be implemented
>> +        * without relying on symbols that are not exported.
>> +        */
>> +       for_each_sg(rk_obj->sgt->sgl, s, rk_obj->sgt->nents, i)
>> +               sg_dma_address(s) = sg_phys(s);
>> +
>> +       dma_sync_sg_for_device(drm->dev, rk_obj->sgt->sgl, rk_obj->sgt->nents,
>> +                              DMA_TO_DEVICE);
>> +
>> +       return 0;
>> +
>> +err_put_pages:
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
>> +       return ret;
>> +}
>> +
>> +static void rockchip_gem_put_pages(struct rockchip_gem_object *rk_obj)
>> +{
>> +       sg_free_table(rk_obj->sgt);
>> +       kfree(rk_obj->sgt);
>> +       drm_gem_put_pages(&rk_obj->base, rk_obj->pages, false, false);
> We need to call drm_gem_put_pages with two last arguments true, not
> false, because we don't have any way of tracking the accesses, so we
> need to assume the page was both accessed and dirty. See
> https://chromium-review.googlesource.com/382934.
>
>> +}
>> +
>> +static int rockchip_gem_alloc_iommu(struct rockchip_gem_object *rk_obj,
>> +                                   bool alloc_kmap)
>> +{
>> +       int ret;
>> +
>> +       ret = rockchip_gem_get_pages(rk_obj);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = rockchip_gem_iommu_map(rk_obj);
>> +       if (ret < 0)
>> +               goto err_free;
>> +
>> +       if (alloc_kmap) {
>> +               rk_obj->kvaddr = vmap(rk_obj->pages, rk_obj->num_pages, VM_MAP,
>> +                                     pgprot_writecombine(PAGE_KERNEL));
>> +               if (!rk_obj->kvaddr) {
>> +                       DRM_ERROR("failed to vmap() buffer\n");
>> +                       ret = -ENOMEM;
>> +                       goto err_unmap;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +
>> +err_unmap:
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +err_free:
>> +       rockchip_gem_put_pages(rk_obj);
>> +
>> +       return ret;
>> +}
>> +
>> +static int rockchip_gem_alloc_dma(struct rockchip_gem_object *rk_obj,
>>                                    bool alloc_kmap)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>> @@ -42,7 +166,27 @@ static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>>          return 0;
>>   }
>>
>> -static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +static int rockchip_gem_alloc_buf(struct rockchip_gem_object *rk_obj,
>> +                                 bool alloc_kmap)
>> +{
>> +       struct drm_gem_object *obj = &rk_obj->base;
>> +       struct drm_device *drm = obj->dev;
>> +       struct rockchip_drm_private *private = drm->dev_private;
>> +
>> +       if (private->domain)
>> +               return rockchip_gem_alloc_iommu(rk_obj, alloc_kmap);
>> +       else
>> +               return rockchip_gem_alloc_dma(rk_obj, alloc_kmap);
>> +}
>> +
>> +static void rockchip_gem_free_iommu(struct rockchip_gem_object *rk_obj)
>> +{
>> +       vunmap(rk_obj->kvaddr);
>> +       rockchip_gem_iommu_unmap(rk_obj);
>> +       rockchip_gem_put_pages(rk_obj);
>> +}
>> +
>> +static void rockchip_gem_free_dma(struct rockchip_gem_object *rk_obj)
>>   {
>>          struct drm_gem_object *obj = &rk_obj->base;
>>          struct drm_device *drm = obj->dev;
>> @@ -51,23 +195,64 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>>                         rk_obj->dma_attrs);
>>   }
>>
>> -static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> -                                       struct vm_area_struct *vma)
>> +static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj)
>> +{
>> +       if (rk_obj->pages)
>> +               rockchip_gem_free_iommu(rk_obj);
>> +       else
>> +               rockchip_gem_free_dma(rk_obj);
>> +}
>>
>> +static int rockchip_drm_gem_object_mmap_iommu(struct drm_gem_object *obj,
>> +                                             struct vm_area_struct *vma)
>>   {
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +       unsigned int i, count = obj->size >> PAGE_SHIFT;
>> +       unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> +       unsigned long uaddr = vma->vm_start;
>>          int ret;
>> +
>> +       if (user_count == 0 || user_count > count)
>> +               return -ENXIO;
>> +
>> +       for (i = 0; i < user_count; i++) {
> Even though DRM mmap doesn't support page offset, we need to respect
> it here for PRIME/DMA-buf mmap. See
> https://chromium-review.googlesource.com/386477.
>
>> +               ret = vm_insert_page(vma, uaddr, rk_obj->pages[i]);
>> +               if (ret)
>> +                       return ret;
>> +               uaddr += PAGE_SIZE;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap_dma(struct drm_gem_object *obj,
>> +                                           struct vm_area_struct *vma)
>> +{
>>          struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>>          struct drm_device *drm = obj->dev;
>>
>> +       return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> +                             obj->size, rk_obj->dma_attrs);
>> +}
>> +
>> +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj,
>> +                                       struct vm_area_struct *vma)
>> +{
>> +       int ret;
>> +       struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj);
>> +
>>          /*
>> -        * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear
>> +        * We allocated a struct page table for rk_obj, so clear
>>           * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap().
>>           */
>>          vma->vm_flags &= ~VM_PFNMAP;
>>          vma->vm_pgoff = 0;
>>
>> -       ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr,
>> -                            obj->size, rk_obj->dma_attrs);
>> +       if (rk_obj->pages)
>> +               ret = rockchip_drm_gem_object_mmap_iommu(obj, vma);
>> +       else
>> +               ret = rockchip_drm_gem_object_mmap_dma(obj, vma);
>> +
>>          if (ret)
>>                  drm_gem_vm_close(vma);
>>
>> @@ -117,7 +302,7 @@ struct rockchip_gem_object *
>>
>>          obj = &rk_obj->base;
>>
>> -       drm_gem_private_object_init(drm, obj, size);
>> +       drm_gem_object_init(drm, obj, size);
> In error path of this function and in all functions that destroy the
> GEM we need to call drm_gem_object_release(), as it's the counterpart
> to drm_gem_object_init(). See
> https://chromium-review.googlesource.com/385456.
>
> Best regards,
> Tomasz
>
>
>


-- 
?ark Yao