[PATCH 01/14] drm/exynos: stop using sgtable in page fault handler

[PATCH 01/14] drm/exynos: stop using sgtable in page fault handler

[Joonyoung Shim]

Already struct exynos_drm_gem_buf has pages of the buffer when buffer is
created, so just can use pages in page fault handler, we don't have to
make sgtable of the buffer. But this needs to construct pages of the
buffer that is imported from dma-buf prime.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_buf.c    | 16 ----------------
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 18 ++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_gem.c    | 14 +-------------
 3 files changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 24994ba..9260dfb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -90,23 +90,12 @@ static int lowlevel_buffer_allocate(struct drm_device *dev,
 		}
 	}
 
-	buf->sgt = drm_prime_pages_to_sg(buf->pages, nr_pages);
-	if (IS_ERR(buf->sgt)) {
-		DRM_ERROR("failed to get sg table.\n");
-		ret = PTR_ERR(buf->sgt);
-		goto err_free_attrs;
-	}
-
 	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
 			(unsigned long)buf->dma_addr,
 			buf->size);
 
 	return ret;
 
-err_free_attrs:
-	dma_free_attrs(dev->dev, buf->size, buf->pages,
-			(dma_addr_t)buf->dma_addr, &buf->dma_attrs);
-	buf->dma_addr = (dma_addr_t)NULL;
 err_free:
 	if (!is_drm_iommu_supported(dev))
 		drm_free_large(buf->pages);
@@ -126,11 +115,6 @@ static void lowlevel_buffer_deallocate(struct drm_device *dev,
 			(unsigned long)buf->dma_addr,
 			buf->size);
 
-	sg_free_table(buf->sgt);
-
-	kfree(buf->sgt);
-	buf->sgt = NULL;
-
 	if (!is_drm_iommu_supported(dev)) {
 		dma_free_attrs(dev->dev, buf->size, buf->cookie,
 				(dma_addr_t)buf->dma_addr, &buf->dma_attrs);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cd485c0..d10f9b6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -203,6 +203,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
 	struct scatterlist *sgl;
 	struct exynos_drm_gem_obj *exynos_gem_obj;
 	struct exynos_drm_gem_buf *buffer;
+	int npages;
 	int ret;
 
 	/* is this one of own objects? */
@@ -251,6 +252,20 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
 	buffer->size = dma_buf->size;
 	buffer->dma_addr = sg_dma_address(sgl);
 
+	npages = dma_buf->size >> PAGE_SHIFT;
+	buffer->pages = drm_malloc_ab(npages, sizeof(struct page *));
+	if (!buffer->pages) {
+		ret = -ENOMEM;
+		goto err_free_gem;
+	}
+
+	ret = drm_prime_sg_to_page_addr_arrays(sgt, buffer->pages, NULL,
+			npages);
+	if (ret < 0) {
+		drm_free_large(buffer->pages);
+		goto err_free_gem;
+	}
+
 	if (sgt->nents == 1) {
 		/* always physically continuous memory if sgt->nents is 1. */
 		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
@@ -273,6 +288,9 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
 
 	return &exynos_gem_obj->base;
 
+err_free_gem:
+	drm_gem_object_release(&exynos_gem_obj->base);
+	kfree(exynos_gem_obj);
 err_free_buffer:
 	kfree(buffer);
 	buffer = NULL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 0d5b969..d320acd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -83,26 +83,14 @@ static int exynos_drm_gem_map_buf(struct drm_gem_object *obj,
 {
 	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
 	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
-	struct scatterlist *sgl;
 	unsigned long pfn;
-	int i;
-
-	if (!buf->sgt)
-		return -EINTR;
 
 	if (page_offset >= (buf->size >> PAGE_SHIFT)) {
 		DRM_ERROR("invalid page offset\n");
 		return -EINVAL;
 	}
 
-	sgl = buf->sgt->sgl;
-	for_each_sg(buf->sgt->sgl, sgl, buf->sgt->nents, i) {
-		if (page_offset < (sgl->length >> PAGE_SHIFT))
-			break;
-		page_offset -=	(sgl->length >> PAGE_SHIFT);
-	}
-
-	pfn = __phys_to_pfn(sg_phys(sgl)) + page_offset;
+	pfn = page_to_pfn(buf->pages[page_offset]);
 
 	return vm_insert_mixed(vma, f_vaddr, pfn);
 }
-- 
1.9.1

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

[PATCH 02/14] drm/exynos: remove function convert_to_vm_err_msg

[Joonyoung Shim]

The convert_to_vm_err_msg is called just once by exynos_drm_gem_fault,
so it's simple not to use the function.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index d320acd..752cb7c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,29 +20,6 @@
 #include "exynos_drm_buf.h"
 #include "exynos_drm_iommu.h"
 
-static unsigned int convert_to_vm_err_msg(int msg)
-{
-	unsigned int out_msg;
-
-	switch (msg) {
-	case 0:
-	case -ERESTARTSYS:
-	case -EINTR:
-		out_msg = VM_FAULT_NOPAGE;
-		break;
-
-	case -ENOMEM:
-		out_msg = VM_FAULT_OOM;
-		break;
-
-	default:
-		out_msg = VM_FAULT_SIGBUS;
-		break;
-	}
-
-	return out_msg;
-}
-
 static int check_gem_flags(unsigned int flags)
 {
 	if (flags & ~(EXYNOS_BO_MASK)) {
@@ -600,7 +577,15 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	mutex_unlock(&dev->struct_mutex);
 
-	return convert_to_vm_err_msg(ret);
+	switch (ret) {
+	case 0:
+	case -ERESTARTSYS:
+		return VM_FAULT_NOPAGE;
+	case -ENOMEM:
+		return VM_FAULT_OOM;
+	default:
+		return VM_FAULT_SIGBUS;
+	}
 }
 
 int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
-- 
1.9.1

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

Re: [PATCH 02/14] drm/exynos: remove function convert_to_vm_err_msg

[Inki Dae]

On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> The convert_to_vm_err_msg is called just once by exynos_drm_gem_fault,
> so it's simple not to use the function.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 33 +++++++++------------------------
>  1 file changed, 9 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index d320acd..752cb7c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -20,29 +20,6 @@
>  #include "exynos_drm_buf.h"
>  #include "exynos_drm_iommu.h"
>  
> -static unsigned int convert_to_vm_err_msg(int msg)
> -{
> -	unsigned int out_msg;
> -
> -	switch (msg) {
> -	case 0:
> -	case -ERESTARTSYS:
> -	case -EINTR:
> -		out_msg = VM_FAULT_NOPAGE;
> -		break;
> -
> -	case -ENOMEM:
> -		out_msg = VM_FAULT_OOM;
> -		break;
> -
> -	default:
> -		out_msg = VM_FAULT_SIGBUS;
> -		break;
> -	}
> -
> -	return out_msg;
> -}
> -
>  static int check_gem_flags(unsigned int flags)
>  {
>  	if (flags & ~(EXYNOS_BO_MASK)) {
> @@ -600,7 +577,15 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	return convert_to_vm_err_msg(ret);
> +	switch (ret) {
> +	case 0:
> +	case -ERESTARTSYS:

You missed -EINTR but I can modify it.

Thanks,
Inki Dae

> +		return VM_FAULT_NOPAGE;
> +	case -ENOMEM:
> +		return VM_FAULT_OOM;
> +	default:
> +		return VM_FAULT_SIGBUS;
> +	}
>  }
>  
>  int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> 

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

[PATCH 03/14] drm/exynos: remove mutex locking in pagefault handler

[Joonyoung Shim]

There is no reason to use mutex locking in pagefault handler.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 752cb7c..4741226 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -560,7 +560,6 @@ unlock:
 int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
-	struct drm_device *dev = obj->dev;
 	unsigned long f_vaddr;
 	pgoff_t page_offset;
 	int ret;
@@ -569,14 +568,10 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			vma->vm_start) >> PAGE_SHIFT;
 	f_vaddr = (unsigned long)vmf->virtual_address;
 
-	mutex_lock(&dev->struct_mutex);
-
 	ret = exynos_drm_gem_map_buf(obj, vma, f_vaddr, page_offset);
 	if (ret < 0)
 		DRM_ERROR("failed to map a buffer with user.\n");
 
-	mutex_unlock(&dev->struct_mutex);
-
 	switch (ret) {
 	case 0:
 	case -ERESTARTSYS:
-- 
1.9.1

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

[PATCH 04/14] drm/exynos: remove function exynos_drm_gem_map_buf

[Joonyoung Shim]

The exynos_drm_gem_map_buf can be merged in exynos_drm_gem_fault.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 36 +++++++++++----------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 4741226..cb5d9b6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -53,25 +53,6 @@ static unsigned long roundup_gem_size(unsigned long size, unsigned int flags)
 	return roundup(size, PAGE_SIZE);
 }
 
-static int exynos_drm_gem_map_buf(struct drm_gem_object *obj,
-					struct vm_area_struct *vma,
-					unsigned long f_vaddr,
-					pgoff_t page_offset)
-{
-	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
-	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
-	unsigned long pfn;
-
-	if (page_offset >= (buf->size >> PAGE_SHIFT)) {
-		DRM_ERROR("invalid page offset\n");
-		return -EINVAL;
-	}
-
-	pfn = page_to_pfn(buf->pages[page_offset]);
-
-	return vm_insert_mixed(vma, f_vaddr, pfn);
-}
-
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
 					struct drm_file *file_priv,
 					unsigned int *handle)
@@ -560,18 +541,25 @@ unlock:
 int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
-	unsigned long f_vaddr;
+	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
+	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
+	unsigned long pfn;
 	pgoff_t page_offset;
 	int ret;
 
 	page_offset = ((unsigned long)vmf->virtual_address -
 			vma->vm_start) >> PAGE_SHIFT;
-	f_vaddr = (unsigned long)vmf->virtual_address;
 
-	ret = exynos_drm_gem_map_buf(obj, vma, f_vaddr, page_offset);
-	if (ret < 0)
-		DRM_ERROR("failed to map a buffer with user.\n");
+	if (page_offset >= (buf->size >> PAGE_SHIFT)) {
+		DRM_ERROR("invalid page offset\n");
+		ret = -EINVAL;
+		goto out;
+	}
 
+	pfn = page_to_pfn(buf->pages[page_offset]);
+	ret = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn);
+
+out:
 	switch (ret) {
 	case 0:
 	case -ERESTARTSYS:
-- 
1.9.1

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

[PATCH 05/14] drm/exynos: stop copying sg table

[Joonyoung Shim]

Already struct exynos_drm_gem_buf has pages of the buffer, so we don't
need to copy from sg table of the buffer to sg table of dma-buf
attachment, just can make sg table from pages of the buffer.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 55 +++++++++++-------------------
 drivers/gpu/drm/exynos/exynos_drm_gem.c    |  3 --
 drivers/gpu/drm/exynos/exynos_drm_gem.h    |  2 --
 3 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index d10f9b6..619ecdd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -18,7 +18,7 @@
 #include <linux/dma-buf.h>
 
 struct exynos_drm_dmabuf_attachment {
-	struct sg_table sgt;
+	struct sg_table *sgt;
 	enum dma_data_direction dir;
 	bool is_mapped;
 };
@@ -53,13 +53,15 @@ static void exynos_gem_detach_dma_buf(struct dma_buf *dmabuf,
 	if (!exynos_attach)
 		return;
 
-	sgt = &exynos_attach->sgt;
-
-	if (exynos_attach->dir != DMA_NONE)
-		dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
-				exynos_attach->dir);
+	sgt = exynos_attach->sgt;
+	if (sgt) {
+		if (exynos_attach->dir != DMA_NONE)
+			dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
+					exynos_attach->dir);
+		sg_free_table(sgt);
+	}
 
-	sg_free_table(sgt);
+	kfree(sgt);
 	kfree(exynos_attach);
 	attach->priv = NULL;
 }
@@ -70,16 +72,13 @@ static struct sg_table *
 {
 	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
 	struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
-	struct drm_device *dev = gem_obj->base.dev;
 	struct exynos_drm_gem_buf *buf;
-	struct scatterlist *rd, *wr;
-	struct sg_table *sgt = NULL;
-	unsigned int i;
-	int nents, ret;
+	struct sg_table *sgt;
+	int npages;
 
 	/* just return current sgt if already requested. */
 	if (exynos_attach->dir == dir && exynos_attach->is_mapped)
-		return &exynos_attach->sgt;
+		return exynos_attach->sgt;
 
 	buf = gem_obj->buffer;
 	if (!buf) {
@@ -87,42 +86,29 @@ static struct sg_table *
 		return ERR_PTR(-ENOMEM);
 	}
 
-	sgt = &exynos_attach->sgt;
+	npages = buf->size >> PAGE_SHIFT;
 
-	ret = sg_alloc_table(sgt, buf->sgt->orig_nents, GFP_KERNEL);
-	if (ret) {
-		DRM_ERROR("failed to alloc sgt.\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	mutex_lock(&dev->struct_mutex);
-
-	rd = buf->sgt->sgl;
-	wr = sgt->sgl;
-	for (i = 0; i < sgt->orig_nents; ++i) {
-		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
-		rd = sg_next(rd);
-		wr = sg_next(wr);
-	}
+	sgt = drm_prime_pages_to_sg(buf->pages, npages);
+	if (IS_ERR(sgt))
+		goto err;
 
 	if (dir != DMA_NONE) {
-		nents = dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir);
-		if (!nents) {
+		if (!dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir)) {
 			DRM_ERROR("failed to map sgl with iommu.\n");
 			sg_free_table(sgt);
 			sgt = ERR_PTR(-EIO);
-			goto err_unlock;
+			goto err;
 		}
 	}
 
 	exynos_attach->is_mapped = true;
+	exynos_attach->sgt = sgt;
 	exynos_attach->dir = dir;
 	attach->priv = exynos_attach;
 
 	DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
 
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
+err:
 	return sgt;
 }
 
@@ -280,7 +266,6 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
 	}
 
 	exynos_gem_obj->buffer = buffer;
-	buffer->sgt = sgt;
 	exynos_gem_obj->base.import_attach = attach;
 
 	DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n", &buffer->dma_addr,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index cb5d9b6..8fffc4d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -455,9 +455,6 @@ void exynos_drm_gem_free_object(struct drm_gem_object *obj)
 	exynos_gem_obj = to_exynos_gem_obj(obj);
 	buf = exynos_gem_obj->buffer;
 
-	if (obj->import_attach)
-		drm_prime_gem_destroy(obj, buf->sgt);
-
 	exynos_drm_gem_destroy(to_exynos_gem_obj(obj));
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 6f42e22..5979f22 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -30,7 +30,6 @@
  *	device address with IOMMU.
  * @write: whether pages will be written to by the caller.
  * @pages: Array of backing pages.
- * @sgt: sg table to transfer page data.
  * @size: size of allocated memory region.
  * @pfnmap: indicate whether memory region from userptr is mmaped with
  *	VM_PFNMAP or not.
@@ -43,7 +42,6 @@ struct exynos_drm_gem_buf {
 	struct dma_attrs	dma_attrs;
 	unsigned int		write;
 	struct page		**pages;
-	struct sg_table		*sgt;
 	unsigned long		size;
 	bool			pfnmap;
 };
-- 
1.9.1

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

[PATCH 06/14] drm/exynos: remove unused fields of struct exynos_drm_gem_buf

[Joonyoung Shim]

The userptr, write and pfnmap of struct exynos_drm_gem_buf are not used
anywhere.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 5979f22..49b5ef1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -24,26 +24,19 @@
  *
  * @cookie: cookie returned by dma_alloc_attrs
  * @kvaddr: kernel virtual address to allocated memory region.
- * *userptr: user space address.
  * @dma_addr: bus address(accessed by dma) to allocated memory region.
  *	- this address could be physical address without IOMMU and
  *	device address with IOMMU.
- * @write: whether pages will be written to by the caller.
  * @pages: Array of backing pages.
  * @size: size of allocated memory region.
- * @pfnmap: indicate whether memory region from userptr is mmaped with
- *	VM_PFNMAP or not.
  */
 struct exynos_drm_gem_buf {
 	void 			*cookie;
 	void __iomem		*kvaddr;
-	unsigned long		userptr;
 	dma_addr_t		dma_addr;
 	struct dma_attrs	dma_attrs;
-	unsigned int		write;
 	struct page		**pages;
 	unsigned long		size;
-	bool			pfnmap;
 };
 
 /*
-- 
1.9.1

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

[PATCH 07/14] drm/exynos: use ERR_PTR instead of NULL in exynos_drm_gem_init

[Joonyoung Shim]

For more correct error information.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 8fffc4d..550d267 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -139,7 +139,7 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
 
 	exynos_gem_obj = kzalloc(sizeof(*exynos_gem_obj), GFP_KERNEL);
 	if (!exynos_gem_obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	exynos_gem_obj->size = size;
 	obj = &exynos_gem_obj->base;
@@ -148,7 +148,7 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
 	if (ret < 0) {
 		DRM_ERROR("failed to initialize gem object\n");
 		kfree(exynos_gem_obj);
-		return NULL;
+		return ERR_PTR(ret);
 	}
 
 	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
@@ -180,8 +180,8 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	exynos_gem_obj = exynos_drm_gem_init(dev, size);
-	if (!exynos_gem_obj) {
-		ret = -ENOMEM;
+	if (IS_ERR(exynos_gem_obj)) {
+		ret = PTR_ERR(exynos_gem_obj);
 		goto err_fini_buf;
 	}
 
-- 
1.9.1

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

[PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Joonyoung Shim]

Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
not, it will call drm_gem_create_mmap_offset whenever user requests
DRM_IOCTL_MODE_MAP_DUMB ioctl.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 550d267..c76aa8a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
 		return ERR_PTR(ret);
 	}
 
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret < 0) {
+		drm_gem_object_release(obj);
+		kfree(exynos_gem_obj);
+		return ERR_PTR(ret);
+	}
+
 	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
 
 	return exynos_gem_obj;
@@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
 		goto unlock;
 	}
 
-	ret = drm_gem_create_mmap_offset(obj);
-	if (ret)
-		goto out;
-
 	*offset = drm_vma_node_offset_addr(&obj->vma_node);
 	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
 
-out:
 	drm_gem_object_unreference(obj);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
-- 
1.9.1

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

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Inki Dae]

On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> not, it will call drm_gem_create_mmap_offset whenever user requests
> DRM_IOCTL_MODE_MAP_DUMB ioctl.

This patch makes drm_gem_create_mmap_offset to be called even in case of
not using dumb* interfaces. I.e.,
exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap

And drm_gem_create_mmap_offset checks if vma_node was already allocated
or not so this patch doesn't make sense.

Thanks,
Inki Dae

> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 550d267..c76aa8a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
>  		return ERR_PTR(ret);
>  	}
>  
> +	ret = drm_gem_create_mmap_offset(obj);
> +	if (ret < 0) {
> +		drm_gem_object_release(obj);
> +		kfree(exynos_gem_obj);
> +		return ERR_PTR(ret);
> +	}
> +
>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>  
>  	return exynos_gem_obj;
> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>  		goto unlock;
>  	}
>  
> -	ret = drm_gem_create_mmap_offset(obj);
> -	if (ret)
> -		goto out;
> -
>  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
>  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>  
> -out:
>  	drm_gem_object_unreference(obj);
>  unlock:
>  	mutex_unlock(&dev->struct_mutex);
> 

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

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Joonyoung Shim]

On 08/16/2015 01:50 PM, Inki Dae wrote:
> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>> not, it will call drm_gem_create_mmap_offset whenever user requests
>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
> 
> This patch makes drm_gem_create_mmap_offset to be called even in case of
> not using dumb* interfaces. I.e.,
> exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap
> 

I know mmap() of exynos-drm also needs mmap offset in drm_gem_mmap().

> And drm_gem_create_mmap_offset checks if vma_node was already allocated
> or not so this patch doesn't make sense.
> 

OK, but it calls drm_gem_create_mmap_offset still and will be returned
after checking node->allocated. It's not unnecessary to me.

> Thanks,
> Inki Dae
> 
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 550d267..c76aa8a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
>>  		return ERR_PTR(ret);
>>  	}
>>  
>> +	ret = drm_gem_create_mmap_offset(obj);
>> +	if (ret < 0) {
>> +		drm_gem_object_release(obj);
>> +		kfree(exynos_gem_obj);
>> +		return ERR_PTR(ret);
>> +	}
>> +
>>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>  
>>  	return exynos_gem_obj;
>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>>  		goto unlock;
>>  	}
>>  
>> -	ret = drm_gem_create_mmap_offset(obj);
>> -	if (ret)
>> -		goto out;
>> -
>>  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
>>  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>  
>> -out:
>>  	drm_gem_object_unreference(obj);
>>  unlock:
>>  	mutex_unlock(&dev->struct_mutex);
>>
> 
> 

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

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Inki Dae]

On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
> On 08/16/2015 01:50 PM, Inki Dae wrote:
>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>>> not, it will call drm_gem_create_mmap_offset whenever user requests
>>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>>
>> This patch makes drm_gem_create_mmap_offset to be called even in case of
>> not using dumb* interfaces. I.e.,
>> exynos_drm_gem_create_ioctl -> exynos_drm_gem_mmap
>>
> 
> I know mmap() of exynos-drm also needs mmap offset in drm_gem_mmap().

Ah, sorry. We already removed Exynos specific mmap ioctl. So we could
never use direct mapping ioctl anymore. I confused that.

> 
>> And drm_gem_create_mmap_offset checks if vma_node was already allocated
>> or not so this patch doesn't make sense.
>>
> 
> OK, but it calls drm_gem_create_mmap_offset still and will be returned
> after checking node->allocated. It's not unnecessary to me.
> 
>> Thanks,
>> Inki Dae
>>
>>>
>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index 550d267..c76aa8a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
>>>  		return ERR_PTR(ret);
>>>  	}
>>>  
>>> +	ret = drm_gem_create_mmap_offset(obj);
>>> +	if (ret < 0) {
>>> +		drm_gem_object_release(obj);
>>> +		kfree(exynos_gem_obj);
>>> +		return ERR_PTR(ret);
>>> +	}
>>> +
>>>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>>  
>>>  	return exynos_gem_obj;
>>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>>>  		goto unlock;
>>>  	}
>>>  
>>> -	ret = drm_gem_create_mmap_offset(obj);
>>> -	if (ret)
>>> -		goto out;
>>> -
>>>  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
>>>  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>>  
>>> -out:
>>>  	drm_gem_object_unreference(obj);
>>>  unlock:
>>>  	mutex_unlock(&dev->struct_mutex);
>>>
>>
>>
> 
> 

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

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Daniel Vetter]

On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> not, it will call drm_gem_create_mmap_offset whenever user requests
> DRM_IOCTL_MODE_MAP_DUMB ioctl.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 550d267..c76aa8a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
>  		return ERR_PTR(ret);
>  	}
>  
> +	ret = drm_gem_create_mmap_offset(obj);
> +	if (ret < 0) {
> +		drm_gem_object_release(obj);
> +		kfree(exynos_gem_obj);
> +		return ERR_PTR(ret);
> +	}
> +
>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>  
>  	return exynos_gem_obj;
> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>  		goto unlock;
>  	}
>  
> -	ret = drm_gem_create_mmap_offset(obj);

drm_gem_create_mmap_offset internally checks whether it's been already
(protected by locks), so this code is perfectly fine. I don't see any
justification for this change (but only noticed it because rockchip
cargo-culted this change).
-Daniel

> -	if (ret)
> -		goto out;
> -
>  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
>  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>  
> -out:
>  	drm_gem_object_unreference(obj);
>  unlock:
>  	mutex_unlock(&dev->struct_mutex);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Daniel Vetter]

On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> > Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> > not, it will call drm_gem_create_mmap_offset whenever user requests
> > DRM_IOCTL_MODE_MAP_DUMB ioctl.
> > 
> > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > index 550d267..c76aa8a 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> > @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
> >  		return ERR_PTR(ret);
> >  	}
> >  
> > +	ret = drm_gem_create_mmap_offset(obj);
> > +	if (ret < 0) {
> > +		drm_gem_object_release(obj);
> > +		kfree(exynos_gem_obj);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> >  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
> >  
> >  	return exynos_gem_obj;
> > @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
> >  		goto unlock;
> >  	}
> >  
> > -	ret = drm_gem_create_mmap_offset(obj);
> 
> drm_gem_create_mmap_offset internally checks whether it's been already
> (protected by locks), so this code is perfectly fine. I don't see any
> justification for this change (but only noticed it because rockchip
> cargo-culted this change).

I think it'd be good to revert this to stay consistent with cma helpers
and other drivers.
-Daniel

> -Daniel
> 
> > -	if (ret)
> > -		goto out;
> > -
> >  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
> >  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
> >  
> > -out:
> >  	drm_gem_object_unreference(obj);
> >  unlock:
> >  	mutex_unlock(&dev->struct_mutex);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Inki Dae]

2015년 11월 17일 01:23에 Daniel Vetter 이(가) 쓴 글:
> On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
>> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
>>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
>>> not, it will call drm_gem_create_mmap_offset whenever user requests
>>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
>>>
>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
>>>  1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index 550d267..c76aa8a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
>>>  		return ERR_PTR(ret);
>>>  	}
>>>  
>>> +	ret = drm_gem_create_mmap_offset(obj);
>>> +	if (ret < 0) {
>>> +		drm_gem_object_release(obj);
>>> +		kfree(exynos_gem_obj);
>>> +		return ERR_PTR(ret);
>>> +	}
>>> +
>>>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
>>>  
>>>  	return exynos_gem_obj;
>>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
>>>  		goto unlock;
>>>  	}
>>>  
>>> -	ret = drm_gem_create_mmap_offset(obj);
>>
>> drm_gem_create_mmap_offset internally checks whether it's been already
>> (protected by locks), so this code is perfectly fine. I don't see any
>> justification for this change (but only noticed it because rockchip
>> cargo-culted this change).
> 
> I think it'd be good to revert this to stay consistent with cma helpers
> and other drivers.

At least, seems cma halper doesn't also call drm_gem_create_mmap_offset function
at drm_gem_cma_dumb_map_offset function and calls it at cma creation instead.
So I think now Exynos drm keeps consistent with cma helper.

Thanks,
Inki Dae

> -Daniel
> 
>> -Daniel
>>
>>> -	if (ret)
>>> -		goto out;
>>> -
>>>  	*offset = drm_vma_node_offset_addr(&obj->vma_node);
>>>  	DRM_DEBUG_KMS("offset = 0x%lx\n", (unsigned long)*offset);
>>>  
>>> -out:
>>>  	drm_gem_object_unreference(obj);
>>>  unlock:
>>>  	mutex_unlock(&dev->struct_mutex);
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/14] drm/exynos: create a fake mmap offset with gem creation

[Daniel Vetter]

On Tue, Nov 17, 2015 at 11:53:28AM +0900, Inki Dae wrote:
> 
> 
> 2015년 11월 17일 01:23에 Daniel Vetter 이(가) 쓴 글:
> > On Mon, Nov 16, 2015 at 05:22:42PM +0100, Daniel Vetter wrote:
> >> On Tue, Jul 28, 2015 at 05:53:23PM +0900, Joonyoung Shim wrote:
> >>> Don't create a fake mmap offset in exynos_drm_gem_dumb_map_offset. If
> >>> not, it will call drm_gem_create_mmap_offset whenever user requests
> >>> DRM_IOCTL_MODE_MAP_DUMB ioctl.
> >>>
> >>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> >>> ---
> >>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 12 +++++++-----
> >>>  1 file changed, 7 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> index 550d267..c76aa8a 100644
> >>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> >>> @@ -151,6 +151,13 @@ struct exynos_drm_gem_obj *exynos_drm_gem_init(struct drm_device *dev,
> >>>  		return ERR_PTR(ret);
> >>>  	}
> >>>  
> >>> +	ret = drm_gem_create_mmap_offset(obj);
> >>> +	if (ret < 0) {
> >>> +		drm_gem_object_release(obj);
> >>> +		kfree(exynos_gem_obj);
> >>> +		return ERR_PTR(ret);
> >>> +	}
> >>> +
> >>>  	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
> >>>  
> >>>  	return exynos_gem_obj;
> >>> @@ -521,14 +528,9 @@ int exynos_drm_gem_dumb_map_offset(struct drm_file *file_priv,
> >>>  		goto unlock;
> >>>  	}
> >>>  
> >>> -	ret = drm_gem_create_mmap_offset(obj);
> >>
> >> drm_gem_create_mmap_offset internally checks whether it's been already
> >> (protected by locks), so this code is perfectly fine. I don't see any
> >> justification for this change (but only noticed it because rockchip
> >> cargo-culted this change).
> > 
> > I think it'd be good to revert this to stay consistent with cma helpers
> > and other drivers.
> 
> At least, seems cma halper doesn't also call drm_gem_create_mmap_offset function
> at drm_gem_cma_dumb_map_offset function and calls it at cma creation instead.
> So I think now Exynos drm keeps consistent with cma helper.

Indeed, common style is to create the offset at alloc time - I checked a
few other drivers too. Never mind my comment, sorry for the noise.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Joonyoung Shim]

The drm_gem_object_release() function already performs this cleanup,
so there is no reason to do it explicitly.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index c76aa8a..ab7d029 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -100,8 +100,6 @@ out:
 	exynos_drm_fini_buf(obj->dev, buf);
 	exynos_gem_obj->buffer = NULL;
 
-	drm_gem_free_mmap_offset(obj);
-
 	/* release file pointer to gem object. */
 	drm_gem_object_release(obj);
 
@@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 
 err_close_vm:
 	drm_gem_vm_close(vma);
-	drm_gem_free_mmap_offset(obj);
 
 	return ret;
 }
-- 
1.9.1

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Inki Dae]

On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> The drm_gem_object_release() function already performs this cleanup,
> so there is no reason to do it explicitly.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index c76aa8a..ab7d029 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -100,8 +100,6 @@ out:
>  	exynos_drm_fini_buf(obj->dev, buf);
>  	exynos_gem_obj->buffer = NULL;
>  
> -	drm_gem_free_mmap_offset(obj);
> -
>  	/* release file pointer to gem object. */
>  	drm_gem_object_release(obj);
>  
> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>  
>  err_close_vm:
>  	drm_gem_vm_close(vma);
> -	drm_gem_free_mmap_offset(obj);

Without previous patch, drm_gem_free_mmap_offset is required. I guess
the reason you removed above line is that you thought
drm_gem_object_release function would be called by drm_gem_vm_close
function which drops a reference of the gem object.

However, drm_gem_vm_close should be a pair with drm_gem_vm_open
function. These will be called whenever a process opens or closes the
VMA. So the reference count of the gem object had already been taken by
open operation when a new reference to the VMA had been created.

Thanks,
Inki Dae

>  
>  	return ret;
>  }
> 

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Joonyoung Shim]

On 08/16/2015 02:07 PM, Inki Dae wrote:
> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>> The drm_gem_object_release() function already performs this cleanup,
>> so there is no reason to do it explicitly.
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index c76aa8a..ab7d029 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -100,8 +100,6 @@ out:
>>  	exynos_drm_fini_buf(obj->dev, buf);
>>  	exynos_gem_obj->buffer = NULL;
>>  
>> -	drm_gem_free_mmap_offset(obj);
>> -
>>  	/* release file pointer to gem object. */
>>  	drm_gem_object_release(obj);
>>  
>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>  
>>  err_close_vm:
>>  	drm_gem_vm_close(vma);
>> -	drm_gem_free_mmap_offset(obj);
> 
> Without previous patch, drm_gem_free_mmap_offset is required. I guess
> the reason you removed above line is that you thought
> drm_gem_object_release function would be called by drm_gem_vm_close
> function which drops a reference of the gem object.
> 
> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
> function. These will be called whenever a process opens or closes the
> VMA. So the reference count of the gem object had already been taken by
> open operation when a new reference to the VMA had been created.
> 

This changes is not related with drm_gem_vm_close and prior patch. Why
should free mmap offset when mmap operation is failed? The mmap offset
can be used repeatedly.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Inki Dae]

On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
> On 08/16/2015 02:07 PM, Inki Dae wrote:
>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>> The drm_gem_object_release() function already performs this cleanup,
>>> so there is no reason to do it explicitly.
>>>
>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> index c76aa8a..ab7d029 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>> @@ -100,8 +100,6 @@ out:
>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>  	exynos_gem_obj->buffer = NULL;
>>>  
>>> -	drm_gem_free_mmap_offset(obj);
>>> -
>>>  	/* release file pointer to gem object. */
>>>  	drm_gem_object_release(obj);
>>>  
>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>  
>>>  err_close_vm:
>>>  	drm_gem_vm_close(vma);
>>> -	drm_gem_free_mmap_offset(obj);
>>
>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>> the reason you removed above line is that you thought
>> drm_gem_object_release function would be called by drm_gem_vm_close
>> function which drops a reference of the gem object.
>>
>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>> function. These will be called whenever a process opens or closes the
>> VMA. So the reference count of the gem object had already been taken by
>> open operation when a new reference to the VMA had been created.
>>
> 
> This changes is not related with drm_gem_vm_close and prior patch. Why
> should free mmap offset when mmap operation is failed? The mmap offset
> can be used repeatedly.

Isn't vm space of vm manager still used even if any user-space process
doesn't use the region? And if mmap is failed, then the user-space
process will be terminated. Do you think it can be re-tried? However,
mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
understand how the mmap offset can be re-used. So can you show me some
example?

> 

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Joonyoung Shim]

On 08/17/2015 04:52 PM, Inki Dae wrote:
> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>> The drm_gem_object_release() function already performs this cleanup,
>>>> so there is no reason to do it explicitly.
>>>>
>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>  1 file changed, 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>> index c76aa8a..ab7d029 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>> @@ -100,8 +100,6 @@ out:
>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>  	exynos_gem_obj->buffer = NULL;
>>>>  
>>>> -	drm_gem_free_mmap_offset(obj);
>>>> -
>>>>  	/* release file pointer to gem object. */
>>>>  	drm_gem_object_release(obj);
>>>>  
>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>  
>>>>  err_close_vm:
>>>>  	drm_gem_vm_close(vma);
>>>> -	drm_gem_free_mmap_offset(obj);
>>>
>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>> the reason you removed above line is that you thought
>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>> function which drops a reference of the gem object.
>>>
>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>> function. These will be called whenever a process opens or closes the
>>> VMA. So the reference count of the gem object had already been taken by
>>> open operation when a new reference to the VMA had been created.
>>>
>>
>> This changes is not related with drm_gem_vm_close and prior patch. Why
>> should free mmap offset when mmap operation is failed? The mmap offset
>> can be used repeatedly.
> 
> Isn't vm space of vm manager still used even if any user-space process
> doesn't use the region? And if mmap is failed, then the user-space
> process will be terminated. Do you think it can be re-tried? However,
> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
> understand how the mmap offset can be re-used. So can you show me some
> example?
> 

Currently, mmap offset of exynos-drm gem is made by                 
DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
offset. User will use same mmap offset about same gem. It's why mmap
offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
enough to make mmap offset from when gem is create. You can get a
reference from drm_gem_cma_helper.c file.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Inki Dae]

On 2015년 08월 17일 17:17, Joonyoung Shim wrote:
> On 08/17/2015 04:52 PM, Inki Dae wrote:
>> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>>> The drm_gem_object_release() function already performs this cleanup,
>>>>> so there is no reason to do it explicitly.
>>>>>
>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>>  1 file changed, 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> index c76aa8a..ab7d029 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> @@ -100,8 +100,6 @@ out:
>>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>>  	exynos_gem_obj->buffer = NULL;
>>>>>  
>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>> -
>>>>>  	/* release file pointer to gem object. */
>>>>>  	drm_gem_object_release(obj);
>>>>>  
>>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>>  
>>>>>  err_close_vm:
>>>>>  	drm_gem_vm_close(vma);
>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>
>>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>>> the reason you removed above line is that you thought
>>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>>> function which drops a reference of the gem object.
>>>>
>>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>>> function. These will be called whenever a process opens or closes the
>>>> VMA. So the reference count of the gem object had already been taken by
>>>> open operation when a new reference to the VMA had been created.
>>>>
>>>
>>> This changes is not related with drm_gem_vm_close and prior patch. Why
>>> should free mmap offset when mmap operation is failed? The mmap offset
>>> can be used repeatedly.
>>
>> Isn't vm space of vm manager still used even if any user-space process
>> doesn't use the region? And if mmap is failed, then the user-space
>> process will be terminated. Do you think it can be re-tried? However,
>> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
>> understand how the mmap offset can be re-used. So can you show me some
>> example?
>>
> 
> Currently, mmap offset of exynos-drm gem is made by                 
> DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
> offset. User will use same mmap offset about same gem. It's why mmap
> offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
> enough to make mmap offset from when gem is create. You can get a
> reference from drm_gem_cma_helper.c file.

Hmm... It's not that the mmap offset can be re-used or not. It's that
the mmap offset should be released or not when mmap failed. As your
original comment, the call of drm_gem_free_mmap_offset() is unnecessary
if mmap offset is created when gem creation because the mmap offset is
removed by drm_gem_object_release() when gem is destroyed - gem should
also be released when mmap failed.

Ok, let's create mmap offset at gem creation and remove it gem
releasing. Will merge these two patches.

Thanks,
Inki Dae

> 

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Joonyoung Shim]

Hi Inki,

On 08/17/2015 06:03 PM, Inki Dae wrote:
> On 2015년 08월 17일 17:17, Joonyoung Shim wrote:
>> On 08/17/2015 04:52 PM, Inki Dae wrote:
>>> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>>>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>>>> The drm_gem_object_release() function already performs this cleanup,
>>>>>> so there is no reason to do it explicitly.
>>>>>>
>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>>>  1 file changed, 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>> index c76aa8a..ab7d029 100644
>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>> @@ -100,8 +100,6 @@ out:
>>>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>>>  	exynos_gem_obj->buffer = NULL;
>>>>>>  
>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>> -
>>>>>>  	/* release file pointer to gem object. */
>>>>>>  	drm_gem_object_release(obj);
>>>>>>  
>>>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>>>  
>>>>>>  err_close_vm:
>>>>>>  	drm_gem_vm_close(vma);
>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>
>>>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>>>> the reason you removed above line is that you thought
>>>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>>>> function which drops a reference of the gem object.
>>>>>
>>>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>>>> function. These will be called whenever a process opens or closes the
>>>>> VMA. So the reference count of the gem object had already been taken by
>>>>> open operation when a new reference to the VMA had been created.
>>>>>
>>>>
>>>> This changes is not related with drm_gem_vm_close and prior patch. Why
>>>> should free mmap offset when mmap operation is failed? The mmap offset
>>>> can be used repeatedly.
>>>
>>> Isn't vm space of vm manager still used even if any user-space process
>>> doesn't use the region? And if mmap is failed, then the user-space
>>> process will be terminated. Do you think it can be re-tried? However,
>>> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
>>> understand how the mmap offset can be re-used. So can you show me some
>>> example?
>>>
>>
>> Currently, mmap offset of exynos-drm gem is made by                 
>> DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
>> offset. User will use same mmap offset about same gem. It's why mmap
>> offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
>> enough to make mmap offset from when gem is create. You can get a
>> reference from drm_gem_cma_helper.c file.
> 
> Hmm... It's not that the mmap offset can be re-used or not. It's that
> the mmap offset should be released or not when mmap failed. As your
> original comment, the call of drm_gem_free_mmap_offset() is unnecessary
> if mmap offset is created when gem creation because the mmap offset is
> removed by drm_gem_object_release() when gem is destroyed - gem should
> also be released when mmap failed.
> 
> Ok, let's create mmap offset at gem creation and remove it gem
> releasing. Will merge these two patches.
> 

I can't find them from your git. Could you merge them?

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Inki Dae]

On 2015년 09월 24일 10:01, Joonyoung Shim wrote:
> Hi Inki,
> 
> On 08/17/2015 06:03 PM, Inki Dae wrote:
>> On 2015년 08월 17일 17:17, Joonyoung Shim wrote:
>>> On 08/17/2015 04:52 PM, Inki Dae wrote:
>>>> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>>>>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>>>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>>>>> The drm_gem_object_release() function already performs this cleanup,
>>>>>>> so there is no reason to do it explicitly.
>>>>>>>
>>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>>>>  1 file changed, 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>> index c76aa8a..ab7d029 100644
>>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>> @@ -100,8 +100,6 @@ out:
>>>>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>>>>  	exynos_gem_obj->buffer = NULL;
>>>>>>>  
>>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>>> -
>>>>>>>  	/* release file pointer to gem object. */
>>>>>>>  	drm_gem_object_release(obj);
>>>>>>>  
>>>>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>>>>  
>>>>>>>  err_close_vm:
>>>>>>>  	drm_gem_vm_close(vma);
>>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>>
>>>>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>>>>> the reason you removed above line is that you thought
>>>>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>>>>> function which drops a reference of the gem object.
>>>>>>
>>>>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>>>>> function. These will be called whenever a process opens or closes the
>>>>>> VMA. So the reference count of the gem object had already been taken by
>>>>>> open operation when a new reference to the VMA had been created.
>>>>>>
>>>>>
>>>>> This changes is not related with drm_gem_vm_close and prior patch. Why
>>>>> should free mmap offset when mmap operation is failed? The mmap offset
>>>>> can be used repeatedly.
>>>>
>>>> Isn't vm space of vm manager still used even if any user-space process
>>>> doesn't use the region? And if mmap is failed, then the user-space
>>>> process will be terminated. Do you think it can be re-tried? However,
>>>> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
>>>> understand how the mmap offset can be re-used. So can you show me some
>>>> example?
>>>>
>>>
>>> Currently, mmap offset of exynos-drm gem is made by                 
>>> DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
>>> offset. User will use same mmap offset about same gem. It's why mmap
>>> offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
>>> enough to make mmap offset from when gem is create. You can get a
>>> reference from drm_gem_cma_helper.c file.
>>
>> Hmm... It's not that the mmap offset can be re-used or not. It's that
>> the mmap offset should be released or not when mmap failed. As your
>> original comment, the call of drm_gem_free_mmap_offset() is unnecessary
>> if mmap offset is created when gem creation because the mmap offset is
>> removed by drm_gem_object_release() when gem is destroyed - gem should
>> also be released when mmap failed.
>>
>> Ok, let's create mmap offset at gem creation and remove it gem
>> releasing. Will merge these two patches.
>>
> 
> I can't find them from your git. Could you merge them?

Oops, sorry. Merged.

Thanks,
Inki Dae

> 
> Thanks.
> 

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

Re: [PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

[Joonyoung Shim]

On 09/25/2015 06:15 PM, Inki Dae wrote:
> On 2015년 09월 24일 10:01, Joonyoung Shim wrote:
>> Hi Inki,
>>
>> On 08/17/2015 06:03 PM, Inki Dae wrote:
>>> On 2015년 08월 17일 17:17, Joonyoung Shim wrote:
>>>> On 08/17/2015 04:52 PM, Inki Dae wrote:
>>>>> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>>>>>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>>>>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>>>>>> The drm_gem_object_release() function already performs this cleanup,
>>>>>>>> so there is no reason to do it explicitly.
>>>>>>>>
>>>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>>>> ---
>>>>>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>>>>>  1 file changed, 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>>> index c76aa8a..ab7d029 100644
>>>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>>>>> @@ -100,8 +100,6 @@ out:
>>>>>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>>>>>  	exynos_gem_obj->buffer = NULL;
>>>>>>>>  
>>>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>>>> -
>>>>>>>>  	/* release file pointer to gem object. */
>>>>>>>>  	drm_gem_object_release(obj);
>>>>>>>>  
>>>>>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>>>>>  
>>>>>>>>  err_close_vm:
>>>>>>>>  	drm_gem_vm_close(vma);
>>>>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>>>>
>>>>>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>>>>>> the reason you removed above line is that you thought
>>>>>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>>>>>> function which drops a reference of the gem object.
>>>>>>>
>>>>>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>>>>>> function. These will be called whenever a process opens or closes the
>>>>>>> VMA. So the reference count of the gem object had already been taken by
>>>>>>> open operation when a new reference to the VMA had been created.
>>>>>>>
>>>>>>
>>>>>> This changes is not related with drm_gem_vm_close and prior patch. Why
>>>>>> should free mmap offset when mmap operation is failed? The mmap offset
>>>>>> can be used repeatedly.
>>>>>
>>>>> Isn't vm space of vm manager still used even if any user-space process
>>>>> doesn't use the region? And if mmap is failed, then the user-space
>>>>> process will be terminated. Do you think it can be re-tried? However,
>>>>> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
>>>>> understand how the mmap offset can be re-used. So can you show me some
>>>>> example?
>>>>>
>>>>
>>>> Currently, mmap offset of exynos-drm gem is made by                 
>>>> DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
>>>> offset. User will use same mmap offset about same gem. It's why mmap
>>>> offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
>>>> enough to make mmap offset from when gem is create. You can get a
>>>> reference from drm_gem_cma_helper.c file.
>>>
>>> Hmm... It's not that the mmap offset can be re-used or not. It's that
>>> the mmap offset should be released or not when mmap failed. As your
>>> original comment, the call of drm_gem_free_mmap_offset() is unnecessary
>>> if mmap offset is created when gem creation because the mmap offset is
>>> removed by drm_gem_object_release() when gem is destroyed - gem should
>>> also be released when mmap failed.
>>>
>>> Ok, let's create mmap offset at gem creation and remove it gem
>>> releasing. Will merge these two patches.
>>>
>>
>> I can't find them from your git. Could you merge them?
> 
> Oops, sorry. Merged.
> 

Thanks for merge, but you are missing the first patch still,
http://www.spinics.net/lists/dri-devel/msg86891.html

Thanks.

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

[PATCH 10/14] drm/exynos: remove function check_gem_flags

[Joonyoung Shim]

The function check_gem_flags is too simple, so it's better to move codes
in each consumer functions.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index ab7d029..03e85d8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,16 +20,6 @@
 #include "exynos_drm_buf.h"
 #include "exynos_drm_iommu.h"
 
-static int check_gem_flags(unsigned int flags)
-{
-	if (flags & ~(EXYNOS_BO_MASK)) {
-		DRM_ERROR("invalid flags.\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
 					struct vm_area_struct *vma)
 {
@@ -169,6 +159,11 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 	struct exynos_drm_gem_buf *buf;
 	int ret;
 
+	if (flags & ~(EXYNOS_BO_MASK)) {
+		DRM_ERROR("invalid flags.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
 	if (!size) {
 		DRM_ERROR("invalid size.\n");
 		return ERR_PTR(-EINVAL);
@@ -176,10 +171,6 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 
 	size = roundup_gem_size(size, flags);
 
-	ret = check_gem_flags(flags);
-	if (ret)
-		return ERR_PTR(ret);
-
 	buf = exynos_drm_init_buf(dev, size);
 	if (!buf)
 		return ERR_PTR(-ENOMEM);
@@ -584,9 +575,10 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	obj = vma->vm_private_data;
 	exynos_gem_obj = to_exynos_gem_obj(obj);
 
-	ret = check_gem_flags(exynos_gem_obj->flags);
-	if (ret)
+	if (exynos_gem_obj->flags & ~(EXYNOS_BO_MASK)) {
+		DRM_ERROR("invalid flags.\n");
 		goto err_close_vm;
+	}
 
 	update_vm_cache_attr(exynos_gem_obj, vma);
 
-- 
1.9.1

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

Re: [PATCH 10/14] drm/exynos: remove function check_gem_flags

[Daniel Stone]

Hi Joonyoung,

On 28 July 2015 at 09:53, Joonyoung Shim <jy0922.shim@samsung.com> wrote:
>  static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
>                                         struct vm_area_struct *vma)
>  {
> @@ -169,6 +159,11 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>         struct exynos_drm_gem_buf *buf;
>         int ret;
>
> +       if (flags & ~(EXYNOS_BO_MASK)) {
> +               DRM_ERROR("invalid flags.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
>         if (!size) {
>                 DRM_ERROR("invalid size.\n");
>                 return ERR_PTR(-EINVAL);
> @@ -176,10 +171,6 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>
>         size = roundup_gem_size(size, flags);
>
> -       ret = check_gem_flags(flags);
> -       if (ret)
> -               return ERR_PTR(ret);
> -
>         buf = exynos_drm_init_buf(dev, size);
>         if (!buf)
>                 return ERR_PTR(-ENOMEM);
> @@ -584,9 +575,10 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>         obj = vma->vm_private_data;
>         exynos_gem_obj = to_exynos_gem_obj(obj);
>
> -       ret = check_gem_flags(exynos_gem_obj->flags);
> -       if (ret)
> +       if (exynos_gem_obj->flags & ~(EXYNOS_BO_MASK)) {
> +               DRM_ERROR("invalid flags.\n");
>                 goto err_close_vm;
> +       }

This check should be removed, or turned into a WARN_ON: there is no
way for the user to control the GEM object flags except through the
create ioctl, no?

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 10/14] drm/exynos: remove function check_gem_flags

[Joonyoung Shim]

On 08/06/2015 07:00 PM, Daniel Stone wrote:
> Hi Joonyoung,
> 
> On 28 July 2015 at 09:53, Joonyoung Shim <jy0922.shim@samsung.com> wrote:
>>  static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
>>                                         struct vm_area_struct *vma)
>>  {
>> @@ -169,6 +159,11 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>>         struct exynos_drm_gem_buf *buf;
>>         int ret;
>>
>> +       if (flags & ~(EXYNOS_BO_MASK)) {
>> +               DRM_ERROR("invalid flags.\n");
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +
>>         if (!size) {
>>                 DRM_ERROR("invalid size.\n");
>>                 return ERR_PTR(-EINVAL);
>> @@ -176,10 +171,6 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>>
>>         size = roundup_gem_size(size, flags);
>>
>> -       ret = check_gem_flags(flags);
>> -       if (ret)
>> -               return ERR_PTR(ret);
>> -
>>         buf = exynos_drm_init_buf(dev, size);
>>         if (!buf)
>>                 return ERR_PTR(-ENOMEM);
>> @@ -584,9 +575,10 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>         obj = vma->vm_private_data;
>>         exynos_gem_obj = to_exynos_gem_obj(obj);
>>
>> -       ret = check_gem_flags(exynos_gem_obj->flags);
>> -       if (ret)
>> +       if (exynos_gem_obj->flags & ~(EXYNOS_BO_MASK)) {
>> +               DRM_ERROR("invalid flags.\n");
>>                 goto err_close_vm;
>> +       }
> 
> This check should be removed, or turned into a WARN_ON: there is no
> way for the user to control the GEM object flags except through the
> create ioctl, no?
> 

Right, your point is reasonable to me.

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

Re: [PATCH 10/14] drm/exynos: remove function check_gem_flags

[Inki Dae]

On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> The function check_gem_flags is too simple, so it's better to move codes
> in each consumer functions.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index ab7d029..03e85d8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -20,16 +20,6 @@
>  #include "exynos_drm_buf.h"
>  #include "exynos_drm_iommu.h"
>  
> -static int check_gem_flags(unsigned int flags)
> -{
> -	if (flags & ~(EXYNOS_BO_MASK)) {
> -		DRM_ERROR("invalid flags.\n");
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>  static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
>  					struct vm_area_struct *vma)
>  {
> @@ -169,6 +159,11 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>  	struct exynos_drm_gem_buf *buf;
>  	int ret;
>  
> +	if (flags & ~(EXYNOS_BO_MASK)) {
> +		DRM_ERROR("invalid flags.\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
>  	if (!size) {
>  		DRM_ERROR("invalid size.\n");
>  		return ERR_PTR(-EINVAL);
> @@ -176,10 +171,6 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
>  
>  	size = roundup_gem_size(size, flags);
>  
> -	ret = check_gem_flags(flags);
> -	if (ret)
> -		return ERR_PTR(ret);
> -
>  	buf = exynos_drm_init_buf(dev, size);
>  	if (!buf)
>  		return ERR_PTR(-ENOMEM);
> @@ -584,9 +575,10 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>  	obj = vma->vm_private_data;
>  	exynos_gem_obj = to_exynos_gem_obj(obj);
>  
> -	ret = check_gem_flags(exynos_gem_obj->flags);
> -	if (ret)
> +	if (exynos_gem_obj->flags & ~(EXYNOS_BO_MASK)) {
> +		DRM_ERROR("invalid flags.\n");
>  		goto err_close_vm;
> +	}

I will remove above checking as Daniel Stone commented because user
space cannot control the flag.

Thanks,
Inki Dae

>  
>  	update_vm_cache_attr(exynos_gem_obj, vma);
>  
> 

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

[PATCH 11/14] drm/exynos: remove function update_vm_cache_attr

[Joonyoung Shim]

The function update_vm_cache_attr can be merged in exynos_drm_gem_mmap.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 03e85d8..55c64cf 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,22 +20,6 @@
 #include "exynos_drm_buf.h"
 #include "exynos_drm_iommu.h"
 
-static void update_vm_cache_attr(struct exynos_drm_gem_obj *obj,
-					struct vm_area_struct *vma)
-{
-	DRM_DEBUG_KMS("flags = 0x%x\n", obj->flags);
-
-	/* non-cachable as default. */
-	if (obj->flags & EXYNOS_BO_CACHABLE)
-		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
-	else if (obj->flags & EXYNOS_BO_WC)
-		vma->vm_page_prot =
-			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
-	else
-		vma->vm_page_prot =
-			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
-}
-
 static unsigned long roundup_gem_size(unsigned long size, unsigned int flags)
 {
 	/* TODO */
@@ -580,7 +564,17 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 		goto err_close_vm;
 	}
 
-	update_vm_cache_attr(exynos_gem_obj, vma);
+	DRM_DEBUG_KMS("flags = 0x%x\n", exynos_gem_obj->flags);
+
+	/* non-cachable as default. */
+	if (exynos_gem_obj->flags & EXYNOS_BO_CACHABLE)
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+	else if (exynos_gem_obj->flags & EXYNOS_BO_WC)
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+	else
+		vma->vm_page_prot =
+			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
 
 	ret = exynos_drm_gem_mmap_buffer(exynos_gem_obj, vma);
 	if (ret)
-- 
1.9.1

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

[PATCH 12/14] drm/exynos: remove function roundup_gem_size

[Joonyoung Shim]

The function roundup_gem_size can be merged in exynos_drm_gem_create.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 55c64cf..9df100f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,13 +20,6 @@
 #include "exynos_drm_buf.h"
 #include "exynos_drm_iommu.h"
 
-static unsigned long roundup_gem_size(unsigned long size, unsigned int flags)
-{
-	/* TODO */
-
-	return roundup(size, PAGE_SIZE);
-}
-
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
 					struct drm_file *file_priv,
 					unsigned int *handle)
@@ -153,7 +146,7 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
-	size = roundup_gem_size(size, flags);
+	size = roundup(size, PAGE_SIZE);
 
 	buf = exynos_drm_init_buf(dev, size);
 	if (!buf)
-- 
1.9.1

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

[PATCH 13/14] drm/exynos: use prime helpers

[Joonyoung Shim]

The dma-buf codes of exynos drm is almost same with prime helpers. A
difference is that consider DMA_NONE when import dma-buf, but it's wrong
and we don't consider it any more, so we can use prime interface.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/Makefile            |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 289 -----------------------------
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.h |  20 --
 drivers/gpu/drm/exynos/exynos_drm_drv.c    |   9 +-
 drivers/gpu/drm/exynos/exynos_drm_gem.c    |  79 ++++++++
 drivers/gpu/drm/exynos/exynos_drm_gem.h    |   9 +
 6 files changed, 95 insertions(+), 313 deletions(-)
 delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
 delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.h

diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
index 7de0b10..95d0306 100644
--- a/drivers/gpu/drm/exynos/Makefile
+++ b/drivers/gpu/drm/exynos/Makefile
@@ -6,7 +6,7 @@ ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/exynos
 exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \
 		exynos_drm_crtc.o exynos_drm_fbdev.o exynos_drm_fb.o \
 		exynos_drm_buf.o exynos_drm_gem.o exynos_drm_core.o \
-		exynos_drm_plane.o exynos_drm_dmabuf.o
+		exynos_drm_plane.o
 
 exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o
 exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)	+= exynos_drm_fimd.o
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
deleted file mode 100644
index 619ecdd..0000000
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ /dev/null
@@ -1,289 +0,0 @@
-/* exynos_drm_dmabuf.c
- *
- * Copyright (c) 2012 Samsung Electronics Co., Ltd.
- * Author: Inki Dae <inki.dae@samsung.com>
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#include <drm/drmP.h>
-#include <drm/exynos_drm.h>
-#include "exynos_drm_dmabuf.h"
-#include "exynos_drm_drv.h"
-#include "exynos_drm_gem.h"
-
-#include <linux/dma-buf.h>
-
-struct exynos_drm_dmabuf_attachment {
-	struct sg_table *sgt;
-	enum dma_data_direction dir;
-	bool is_mapped;
-};
-
-static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
-{
-	return to_exynos_gem_obj(buf->priv);
-}
-
-static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
-					struct device *dev,
-					struct dma_buf_attachment *attach)
-{
-	struct exynos_drm_dmabuf_attachment *exynos_attach;
-
-	exynos_attach = kzalloc(sizeof(*exynos_attach), GFP_KERNEL);
-	if (!exynos_attach)
-		return -ENOMEM;
-
-	exynos_attach->dir = DMA_NONE;
-	attach->priv = exynos_attach;
-
-	return 0;
-}
-
-static void exynos_gem_detach_dma_buf(struct dma_buf *dmabuf,
-					struct dma_buf_attachment *attach)
-{
-	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
-	struct sg_table *sgt;
-
-	if (!exynos_attach)
-		return;
-
-	sgt = exynos_attach->sgt;
-	if (sgt) {
-		if (exynos_attach->dir != DMA_NONE)
-			dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
-					exynos_attach->dir);
-		sg_free_table(sgt);
-	}
-
-	kfree(sgt);
-	kfree(exynos_attach);
-	attach->priv = NULL;
-}
-
-static struct sg_table *
-		exynos_gem_map_dma_buf(struct dma_buf_attachment *attach,
-					enum dma_data_direction dir)
-{
-	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
-	struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
-	struct exynos_drm_gem_buf *buf;
-	struct sg_table *sgt;
-	int npages;
-
-	/* just return current sgt if already requested. */
-	if (exynos_attach->dir == dir && exynos_attach->is_mapped)
-		return exynos_attach->sgt;
-
-	buf = gem_obj->buffer;
-	if (!buf) {
-		DRM_ERROR("buffer is null.\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	npages = buf->size >> PAGE_SHIFT;
-
-	sgt = drm_prime_pages_to_sg(buf->pages, npages);
-	if (IS_ERR(sgt))
-		goto err;
-
-	if (dir != DMA_NONE) {
-		if (!dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir)) {
-			DRM_ERROR("failed to map sgl with iommu.\n");
-			sg_free_table(sgt);
-			sgt = ERR_PTR(-EIO);
-			goto err;
-		}
-	}
-
-	exynos_attach->is_mapped = true;
-	exynos_attach->sgt = sgt;
-	exynos_attach->dir = dir;
-	attach->priv = exynos_attach;
-
-	DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
-
-err:
-	return sgt;
-}
-
-static void exynos_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
-						struct sg_table *sgt,
-						enum dma_data_direction dir)
-{
-	/* Nothing to do. */
-}
-
-static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
-						unsigned long page_num)
-{
-	/* TODO */
-
-	return NULL;
-}
-
-static void exynos_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
-						unsigned long page_num,
-						void *addr)
-{
-	/* TODO */
-}
-
-static void *exynos_gem_dmabuf_kmap(struct dma_buf *dma_buf,
-					unsigned long page_num)
-{
-	/* TODO */
-
-	return NULL;
-}
-
-static void exynos_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
-					unsigned long page_num, void *addr)
-{
-	/* TODO */
-}
-
-static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
-	struct vm_area_struct *vma)
-{
-	return -ENOTTY;
-}
-
-static struct dma_buf_ops exynos_dmabuf_ops = {
-	.attach			= exynos_gem_attach_dma_buf,
-	.detach			= exynos_gem_detach_dma_buf,
-	.map_dma_buf		= exynos_gem_map_dma_buf,
-	.unmap_dma_buf		= exynos_gem_unmap_dma_buf,
-	.kmap			= exynos_gem_dmabuf_kmap,
-	.kmap_atomic		= exynos_gem_dmabuf_kmap_atomic,
-	.kunmap			= exynos_gem_dmabuf_kunmap,
-	.kunmap_atomic		= exynos_gem_dmabuf_kunmap_atomic,
-	.mmap			= exynos_gem_dmabuf_mmap,
-	.release		= drm_gem_dmabuf_release,
-};
-
-struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
-				struct drm_gem_object *obj, int flags)
-{
-	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
-	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
-
-	exp_info.ops = &exynos_dmabuf_ops;
-	exp_info.size = exynos_gem_obj->base.size;
-	exp_info.flags = flags;
-	exp_info.priv = obj;
-
-	return dma_buf_export(&exp_info);
-}
-
-struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
-				struct dma_buf *dma_buf)
-{
-	struct dma_buf_attachment *attach;
-	struct sg_table *sgt;
-	struct scatterlist *sgl;
-	struct exynos_drm_gem_obj *exynos_gem_obj;
-	struct exynos_drm_gem_buf *buffer;
-	int npages;
-	int ret;
-
-	/* is this one of own objects? */
-	if (dma_buf->ops == &exynos_dmabuf_ops) {
-		struct drm_gem_object *obj;
-
-		obj = dma_buf->priv;
-
-		/* is it from our device? */
-		if (obj->dev == drm_dev) {
-			/*
-			 * Importing dmabuf exported from out own gem increases
-			 * refcount on gem itself instead of f_count of dmabuf.
-			 */
-			drm_gem_object_reference(obj);
-			return obj;
-		}
-	}
-
-	attach = dma_buf_attach(dma_buf, drm_dev->dev);
-	if (IS_ERR(attach))
-		return ERR_PTR(-EINVAL);
-
-	get_dma_buf(dma_buf);
-
-	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
-	if (IS_ERR(sgt)) {
-		ret = PTR_ERR(sgt);
-		goto err_buf_detach;
-	}
-
-	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
-	if (!buffer) {
-		ret = -ENOMEM;
-		goto err_unmap_attach;
-	}
-
-	exynos_gem_obj = exynos_drm_gem_init(drm_dev, dma_buf->size);
-	if (!exynos_gem_obj) {
-		ret = -ENOMEM;
-		goto err_free_buffer;
-	}
-
-	sgl = sgt->sgl;
-
-	buffer->size = dma_buf->size;
-	buffer->dma_addr = sg_dma_address(sgl);
-
-	npages = dma_buf->size >> PAGE_SHIFT;
-	buffer->pages = drm_malloc_ab(npages, sizeof(struct page *));
-	if (!buffer->pages) {
-		ret = -ENOMEM;
-		goto err_free_gem;
-	}
-
-	ret = drm_prime_sg_to_page_addr_arrays(sgt, buffer->pages, NULL,
-			npages);
-	if (ret < 0) {
-		drm_free_large(buffer->pages);
-		goto err_free_gem;
-	}
-
-	if (sgt->nents == 1) {
-		/* always physically continuous memory if sgt->nents is 1. */
-		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
-	} else {
-		/*
-		 * this case could be CONTIG or NONCONTIG type but for now
-		 * sets NONCONTIG.
-		 * TODO. we have to find a way that exporter can notify
-		 * the type of its own buffer to importer.
-		 */
-		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
-	}
-
-	exynos_gem_obj->buffer = buffer;
-	exynos_gem_obj->base.import_attach = attach;
-
-	DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n", &buffer->dma_addr,
-								buffer->size);
-
-	return &exynos_gem_obj->base;
-
-err_free_gem:
-	drm_gem_object_release(&exynos_gem_obj->base);
-	kfree(exynos_gem_obj);
-err_free_buffer:
-	kfree(buffer);
-	buffer = NULL;
-err_unmap_attach:
-	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
-err_buf_detach:
-	dma_buf_detach(dma_buf, attach);
-	dma_buf_put(dma_buf);
-
-	return ERR_PTR(ret);
-}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
deleted file mode 100644
index 886de9f..0000000
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* exynos_drm_dmabuf.h
- *
- * Copyright (c) 2012 Samsung Electronics Co., Ltd.
- * Author: Inki Dae <inki.dae@samsung.com>
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#ifndef _EXYNOS_DRM_DMABUF_H_
-#define _EXYNOS_DRM_DMABUF_H_
-
-struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
-				struct drm_gem_object *obj, int flags);
-
-struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
-						struct dma_buf *dma_buf);
-#endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index f1d6966..e9ddaa2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -27,7 +27,6 @@
 #include "exynos_drm_gem.h"
 #include "exynos_drm_plane.h"
 #include "exynos_drm_vidi.h"
-#include "exynos_drm_dmabuf.h"
 #include "exynos_drm_g2d.h"
 #include "exynos_drm_ipp.h"
 #include "exynos_drm_iommu.h"
@@ -297,8 +296,12 @@ static struct drm_driver exynos_drm_driver = {
 	.dumb_destroy		= drm_gem_dumb_destroy,
 	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
-	.gem_prime_export	= exynos_dmabuf_prime_export,
-	.gem_prime_import	= exynos_dmabuf_prime_import,
+	.gem_prime_export	= drm_gem_prime_export,
+	.gem_prime_import	= drm_gem_prime_import,
+	.gem_prime_get_sg_table	= exynos_drm_gem_prime_get_sg_table,
+	.gem_prime_import_sg_table	= exynos_drm_gem_prime_import_sg_table,
+	.gem_prime_vmap		= exynos_drm_gem_prime_vmap,
+	.gem_prime_vunmap	= exynos_drm_gem_prime_vunmap,
 	.ioctls			= exynos_ioctls,
 	.num_ioctls		= ARRAY_SIZE(exynos_ioctls),
 	.fops			= &exynos_drm_driver_fops,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 9df100f..7d4bf9d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -13,6 +13,7 @@
 #include <drm/drm_vma_manager.h>
 
 #include <linux/shmem_fs.h>
+#include <linux/dma-buf.h>
 #include <drm/exynos_drm.h>
 
 #include "exynos_drm_drv.h"
@@ -580,3 +581,81 @@ err_close_vm:
 
 	return ret;
 }
+
+/* low-level interface prime helpers */
+struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
+{
+	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
+	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
+	int npages;
+
+	npages = buf->size >> PAGE_SHIFT;
+
+	return drm_prime_pages_to_sg(buf->pages, npages);
+}
+
+struct drm_gem_object *
+exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
+				     struct dma_buf_attachment *attach,
+				     struct sg_table *sgt)
+{
+	struct exynos_drm_gem_obj *exynos_gem_obj;
+	struct exynos_drm_gem_buf *buf;
+	int npages;
+	int ret;
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	buf->size = attach->dmabuf->size;
+	buf->dma_addr = sg_dma_address(sgt->sgl);
+
+	npages = buf->size >> PAGE_SHIFT;
+	buf->pages = drm_malloc_ab(npages, sizeof(struct page *));
+	if (!buf->pages) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = drm_prime_sg_to_page_addr_arrays(sgt, buf->pages, NULL, npages);
+	if (ret < 0)
+		goto err_free_large;
+
+	exynos_gem_obj = exynos_drm_gem_init(dev, buf->size);
+	if (IS_ERR(exynos_gem_obj)) {
+		ret = PTR_ERR(exynos_gem_obj);
+		goto err;
+	}
+
+	if (sgt->nents == 1) {
+		/* always physically continuous memory if sgt->nents is 1. */
+		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
+	} else {
+		/*
+		 * this case could be CONTIG or NONCONTIG type but for now
+		 * sets NONCONTIG.
+		 * TODO. we have to find a way that exporter can notify
+		 * the type of its own buffer to importer.
+		 */
+		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
+	}
+
+	return &exynos_gem_obj->base;
+
+err_free_large:
+	drm_free_large(buf->pages);
+err:
+	kfree(buf);
+	return ERR_PTR(ret);
+}
+
+void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj)
+{
+	return NULL;
+}
+
+void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
+{
+	/* Nothing to do */
+}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 49b5ef1..5e20da6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -168,4 +168,13 @@ void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
 				struct sg_table *sgt,
 				enum dma_data_direction dir);
 
+/* low-level interface prime helpers */
+struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj);
+struct drm_gem_object *
+exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
+				     struct dma_buf_attachment *attach,
+				     struct sg_table *sgt);
+void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj);
+void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
+
 #endif
-- 
1.9.1

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

Re: [PATCH 13/14] drm/exynos: use prime helpers

[Inki Dae]

On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> The dma-buf codes of exynos drm is almost same with prime helpers. A
> difference is that consider DMA_NONE when import dma-buf, but it's wrong
> and we don't consider it any more, so we can use prime interface.

I think it's good idea to use prime helpers. However, I'm not sure of
there really is no any corner case the prime helper doesn't consider for
Vendor SoC yet. So let's have more reviews about this.

Thanks,
Inki Dae

> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/gpu/drm/exynos/Makefile            |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 289 -----------------------------
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.h |  20 --
>  drivers/gpu/drm/exynos/exynos_drm_drv.c    |   9 +-
>  drivers/gpu/drm/exynos/exynos_drm_gem.c    |  79 ++++++++
>  drivers/gpu/drm/exynos/exynos_drm_gem.h    |   9 +
>  6 files changed, 95 insertions(+), 313 deletions(-)
>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> 
> diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
> index 7de0b10..95d0306 100644
> --- a/drivers/gpu/drm/exynos/Makefile
> +++ b/drivers/gpu/drm/exynos/Makefile
> @@ -6,7 +6,7 @@ ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/exynos
>  exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \
>  		exynos_drm_crtc.o exynos_drm_fbdev.o exynos_drm_fb.o \
>  		exynos_drm_buf.o exynos_drm_gem.o exynos_drm_core.o \
> -		exynos_drm_plane.o exynos_drm_dmabuf.o
> +		exynos_drm_plane.o
>  
>  exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o
>  exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)	+= exynos_drm_fimd.o
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> deleted file mode 100644
> index 619ecdd..0000000
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ /dev/null
> @@ -1,289 +0,0 @@
> -/* exynos_drm_dmabuf.c
> - *
> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> - * Author: Inki Dae <inki.dae@samsung.com>
> - *
> - * This program is free software; you can redistribute  it and/or modify it
> - * under  the terms of  the GNU General  Public License as published by the
> - * Free Software Foundation;  either version 2 of the  License, or (at your
> - * option) any later version.
> - */
> -
> -#include <drm/drmP.h>
> -#include <drm/exynos_drm.h>
> -#include "exynos_drm_dmabuf.h"
> -#include "exynos_drm_drv.h"
> -#include "exynos_drm_gem.h"
> -
> -#include <linux/dma-buf.h>
> -
> -struct exynos_drm_dmabuf_attachment {
> -	struct sg_table *sgt;
> -	enum dma_data_direction dir;
> -	bool is_mapped;
> -};
> -
> -static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
> -{
> -	return to_exynos_gem_obj(buf->priv);
> -}
> -
> -static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
> -					struct device *dev,
> -					struct dma_buf_attachment *attach)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach;
> -
> -	exynos_attach = kzalloc(sizeof(*exynos_attach), GFP_KERNEL);
> -	if (!exynos_attach)
> -		return -ENOMEM;
> -
> -	exynos_attach->dir = DMA_NONE;
> -	attach->priv = exynos_attach;
> -
> -	return 0;
> -}
> -
> -static void exynos_gem_detach_dma_buf(struct dma_buf *dmabuf,
> -					struct dma_buf_attachment *attach)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
> -	struct sg_table *sgt;
> -
> -	if (!exynos_attach)
> -		return;
> -
> -	sgt = exynos_attach->sgt;
> -	if (sgt) {
> -		if (exynos_attach->dir != DMA_NONE)
> -			dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
> -					exynos_attach->dir);
> -		sg_free_table(sgt);
> -	}
> -
> -	kfree(sgt);
> -	kfree(exynos_attach);
> -	attach->priv = NULL;
> -}
> -
> -static struct sg_table *
> -		exynos_gem_map_dma_buf(struct dma_buf_attachment *attach,
> -					enum dma_data_direction dir)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
> -	struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
> -	struct exynos_drm_gem_buf *buf;
> -	struct sg_table *sgt;
> -	int npages;
> -
> -	/* just return current sgt if already requested. */
> -	if (exynos_attach->dir == dir && exynos_attach->is_mapped)
> -		return exynos_attach->sgt;
> -
> -	buf = gem_obj->buffer;
> -	if (!buf) {
> -		DRM_ERROR("buffer is null.\n");
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	npages = buf->size >> PAGE_SHIFT;
> -
> -	sgt = drm_prime_pages_to_sg(buf->pages, npages);
> -	if (IS_ERR(sgt))
> -		goto err;
> -
> -	if (dir != DMA_NONE) {
> -		if (!dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir)) {
> -			DRM_ERROR("failed to map sgl with iommu.\n");
> -			sg_free_table(sgt);
> -			sgt = ERR_PTR(-EIO);
> -			goto err;
> -		}
> -	}
> -
> -	exynos_attach->is_mapped = true;
> -	exynos_attach->sgt = sgt;
> -	exynos_attach->dir = dir;
> -	attach->priv = exynos_attach;
> -
> -	DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
> -
> -err:
> -	return sgt;
> -}
> -
> -static void exynos_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
> -						struct sg_table *sgt,
> -						enum dma_data_direction dir)
> -{
> -	/* Nothing to do. */
> -}
> -
> -static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
> -						unsigned long page_num)
> -{
> -	/* TODO */
> -
> -	return NULL;
> -}
> -
> -static void exynos_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
> -						unsigned long page_num,
> -						void *addr)
> -{
> -	/* TODO */
> -}
> -
> -static void *exynos_gem_dmabuf_kmap(struct dma_buf *dma_buf,
> -					unsigned long page_num)
> -{
> -	/* TODO */
> -
> -	return NULL;
> -}
> -
> -static void exynos_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
> -					unsigned long page_num, void *addr)
> -{
> -	/* TODO */
> -}
> -
> -static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
> -	struct vm_area_struct *vma)
> -{
> -	return -ENOTTY;
> -}
> -
> -static struct dma_buf_ops exynos_dmabuf_ops = {
> -	.attach			= exynos_gem_attach_dma_buf,
> -	.detach			= exynos_gem_detach_dma_buf,
> -	.map_dma_buf		= exynos_gem_map_dma_buf,
> -	.unmap_dma_buf		= exynos_gem_unmap_dma_buf,
> -	.kmap			= exynos_gem_dmabuf_kmap,
> -	.kmap_atomic		= exynos_gem_dmabuf_kmap_atomic,
> -	.kunmap			= exynos_gem_dmabuf_kunmap,
> -	.kunmap_atomic		= exynos_gem_dmabuf_kunmap_atomic,
> -	.mmap			= exynos_gem_dmabuf_mmap,
> -	.release		= drm_gem_dmabuf_release,
> -};
> -
> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
> -				struct drm_gem_object *obj, int flags)
> -{
> -	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
> -	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> -
> -	exp_info.ops = &exynos_dmabuf_ops;
> -	exp_info.size = exynos_gem_obj->base.size;
> -	exp_info.flags = flags;
> -	exp_info.priv = obj;
> -
> -	return dma_buf_export(&exp_info);
> -}
> -
> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> -				struct dma_buf *dma_buf)
> -{
> -	struct dma_buf_attachment *attach;
> -	struct sg_table *sgt;
> -	struct scatterlist *sgl;
> -	struct exynos_drm_gem_obj *exynos_gem_obj;
> -	struct exynos_drm_gem_buf *buffer;
> -	int npages;
> -	int ret;
> -
> -	/* is this one of own objects? */
> -	if (dma_buf->ops == &exynos_dmabuf_ops) {
> -		struct drm_gem_object *obj;
> -
> -		obj = dma_buf->priv;
> -
> -		/* is it from our device? */
> -		if (obj->dev == drm_dev) {
> -			/*
> -			 * Importing dmabuf exported from out own gem increases
> -			 * refcount on gem itself instead of f_count of dmabuf.
> -			 */
> -			drm_gem_object_reference(obj);
> -			return obj;
> -		}
> -	}
> -
> -	attach = dma_buf_attach(dma_buf, drm_dev->dev);
> -	if (IS_ERR(attach))
> -		return ERR_PTR(-EINVAL);
> -
> -	get_dma_buf(dma_buf);
> -
> -	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> -	if (IS_ERR(sgt)) {
> -		ret = PTR_ERR(sgt);
> -		goto err_buf_detach;
> -	}
> -
> -	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> -	if (!buffer) {
> -		ret = -ENOMEM;
> -		goto err_unmap_attach;
> -	}
> -
> -	exynos_gem_obj = exynos_drm_gem_init(drm_dev, dma_buf->size);
> -	if (!exynos_gem_obj) {
> -		ret = -ENOMEM;
> -		goto err_free_buffer;
> -	}
> -
> -	sgl = sgt->sgl;
> -
> -	buffer->size = dma_buf->size;
> -	buffer->dma_addr = sg_dma_address(sgl);
> -
> -	npages = dma_buf->size >> PAGE_SHIFT;
> -	buffer->pages = drm_malloc_ab(npages, sizeof(struct page *));
> -	if (!buffer->pages) {
> -		ret = -ENOMEM;
> -		goto err_free_gem;
> -	}
> -
> -	ret = drm_prime_sg_to_page_addr_arrays(sgt, buffer->pages, NULL,
> -			npages);
> -	if (ret < 0) {
> -		drm_free_large(buffer->pages);
> -		goto err_free_gem;
> -	}
> -
> -	if (sgt->nents == 1) {
> -		/* always physically continuous memory if sgt->nents is 1. */
> -		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
> -	} else {
> -		/*
> -		 * this case could be CONTIG or NONCONTIG type but for now
> -		 * sets NONCONTIG.
> -		 * TODO. we have to find a way that exporter can notify
> -		 * the type of its own buffer to importer.
> -		 */
> -		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
> -	}
> -
> -	exynos_gem_obj->buffer = buffer;
> -	exynos_gem_obj->base.import_attach = attach;
> -
> -	DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n", &buffer->dma_addr,
> -								buffer->size);
> -
> -	return &exynos_gem_obj->base;
> -
> -err_free_gem:
> -	drm_gem_object_release(&exynos_gem_obj->base);
> -	kfree(exynos_gem_obj);
> -err_free_buffer:
> -	kfree(buffer);
> -	buffer = NULL;
> -err_unmap_attach:
> -	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> -err_buf_detach:
> -	dma_buf_detach(dma_buf, attach);
> -	dma_buf_put(dma_buf);
> -
> -	return ERR_PTR(ret);
> -}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> deleted file mode 100644
> index 886de9f..0000000
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* exynos_drm_dmabuf.h
> - *
> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> - * Author: Inki Dae <inki.dae@samsung.com>
> - *
> - * This program is free software; you can redistribute  it and/or modify it
> - * under  the terms of  the GNU General  Public License as published by the
> - * Free Software Foundation;  either version 2 of the  License, or (at your
> - * option) any later version.
> - */
> -
> -#ifndef _EXYNOS_DRM_DMABUF_H_
> -#define _EXYNOS_DRM_DMABUF_H_
> -
> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
> -				struct drm_gem_object *obj, int flags);
> -
> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> -						struct dma_buf *dma_buf);
> -#endif
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index f1d6966..e9ddaa2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -27,7 +27,6 @@
>  #include "exynos_drm_gem.h"
>  #include "exynos_drm_plane.h"
>  #include "exynos_drm_vidi.h"
> -#include "exynos_drm_dmabuf.h"
>  #include "exynos_drm_g2d.h"
>  #include "exynos_drm_ipp.h"
>  #include "exynos_drm_iommu.h"
> @@ -297,8 +296,12 @@ static struct drm_driver exynos_drm_driver = {
>  	.dumb_destroy		= drm_gem_dumb_destroy,
>  	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>  	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> -	.gem_prime_export	= exynos_dmabuf_prime_export,
> -	.gem_prime_import	= exynos_dmabuf_prime_import,
> +	.gem_prime_export	= drm_gem_prime_export,
> +	.gem_prime_import	= drm_gem_prime_import,
> +	.gem_prime_get_sg_table	= exynos_drm_gem_prime_get_sg_table,
> +	.gem_prime_import_sg_table	= exynos_drm_gem_prime_import_sg_table,
> +	.gem_prime_vmap		= exynos_drm_gem_prime_vmap,
> +	.gem_prime_vunmap	= exynos_drm_gem_prime_vunmap,
>  	.ioctls			= exynos_ioctls,
>  	.num_ioctls		= ARRAY_SIZE(exynos_ioctls),
>  	.fops			= &exynos_drm_driver_fops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 9df100f..7d4bf9d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -13,6 +13,7 @@
>  #include <drm/drm_vma_manager.h>
>  
>  #include <linux/shmem_fs.h>
> +#include <linux/dma-buf.h>
>  #include <drm/exynos_drm.h>
>  
>  #include "exynos_drm_drv.h"
> @@ -580,3 +581,81 @@ err_close_vm:
>  
>  	return ret;
>  }
> +
> +/* low-level interface prime helpers */
> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
> +{
> +	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
> +	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
> +	int npages;
> +
> +	npages = buf->size >> PAGE_SHIFT;
> +
> +	return drm_prime_pages_to_sg(buf->pages, npages);
> +}
> +
> +struct drm_gem_object *
> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> +				     struct dma_buf_attachment *attach,
> +				     struct sg_table *sgt)
> +{
> +	struct exynos_drm_gem_obj *exynos_gem_obj;
> +	struct exynos_drm_gem_buf *buf;
> +	int npages;
> +	int ret;
> +
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->size = attach->dmabuf->size;
> +	buf->dma_addr = sg_dma_address(sgt->sgl);
> +
> +	npages = buf->size >> PAGE_SHIFT;
> +	buf->pages = drm_malloc_ab(npages, sizeof(struct page *));
> +	if (!buf->pages) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	ret = drm_prime_sg_to_page_addr_arrays(sgt, buf->pages, NULL, npages);
> +	if (ret < 0)
> +		goto err_free_large;
> +
> +	exynos_gem_obj = exynos_drm_gem_init(dev, buf->size);
> +	if (IS_ERR(exynos_gem_obj)) {
> +		ret = PTR_ERR(exynos_gem_obj);
> +		goto err;
> +	}
> +
> +	if (sgt->nents == 1) {
> +		/* always physically continuous memory if sgt->nents is 1. */
> +		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
> +	} else {
> +		/*
> +		 * this case could be CONTIG or NONCONTIG type but for now
> +		 * sets NONCONTIG.
> +		 * TODO. we have to find a way that exporter can notify
> +		 * the type of its own buffer to importer.
> +		 */
> +		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
> +	}
> +
> +	return &exynos_gem_obj->base;
> +
> +err_free_large:
> +	drm_free_large(buf->pages);
> +err:
> +	kfree(buf);
> +	return ERR_PTR(ret);
> +}
> +
> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj)
> +{
> +	return NULL;
> +}
> +
> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
> +{
> +	/* Nothing to do */
> +}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index 49b5ef1..5e20da6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -168,4 +168,13 @@ void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
>  				struct sg_table *sgt,
>  				enum dma_data_direction dir);
>  
> +/* low-level interface prime helpers */
> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj);
> +struct drm_gem_object *
> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> +				     struct dma_buf_attachment *attach,
> +				     struct sg_table *sgt);
> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj);
> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> +
>  #endif
> 

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

Re: [PATCH 13/14] drm/exynos: use prime helpers

[Inki Dae]

On 2015년 08월 16일 14:26, Inki Dae wrote:
> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>> The dma-buf codes of exynos drm is almost same with prime helpers. A
>> difference is that consider DMA_NONE when import dma-buf, but it's wrong
>> and we don't consider it any more, so we can use prime interface.
> 
> I think it's good idea to use prime helpers. However, I'm not sure of
> there really is no any corner case the prime helper doesn't consider for
> Vendor SoC yet. So let's have more reviews about this.

Just merged. We could consider something specific to Vendor SoC later
when that is clarified.

Thanks,
Inki Dae

> 
> Thanks,
> Inki Dae
> 
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/Makefile            |   2 +-
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 289 -----------------------------
>>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.h |  20 --
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c    |   9 +-
>>  drivers/gpu/drm/exynos/exynos_drm_gem.c    |  79 ++++++++
>>  drivers/gpu/drm/exynos/exynos_drm_gem.h    |   9 +
>>  6 files changed, 95 insertions(+), 313 deletions(-)
>>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
>>
>> diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
>> index 7de0b10..95d0306 100644
>> --- a/drivers/gpu/drm/exynos/Makefile
>> +++ b/drivers/gpu/drm/exynos/Makefile
>> @@ -6,7 +6,7 @@ ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/exynos
>>  exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \
>>  		exynos_drm_crtc.o exynos_drm_fbdev.o exynos_drm_fb.o \
>>  		exynos_drm_buf.o exynos_drm_gem.o exynos_drm_core.o \
>> -		exynos_drm_plane.o exynos_drm_dmabuf.o
>> +		exynos_drm_plane.o
>>  
>>  exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o
>>  exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)	+= exynos_drm_fimd.o
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>> deleted file mode 100644
>> index 619ecdd..0000000
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>> +++ /dev/null
>> @@ -1,289 +0,0 @@
>> -/* exynos_drm_dmabuf.c
>> - *
>> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>> - * Author: Inki Dae <inki.dae@samsung.com>
>> - *
>> - * This program is free software; you can redistribute  it and/or modify it
>> - * under  the terms of  the GNU General  Public License as published by the
>> - * Free Software Foundation;  either version 2 of the  License, or (at your
>> - * option) any later version.
>> - */
>> -
>> -#include <drm/drmP.h>
>> -#include <drm/exynos_drm.h>
>> -#include "exynos_drm_dmabuf.h"
>> -#include "exynos_drm_drv.h"
>> -#include "exynos_drm_gem.h"
>> -
>> -#include <linux/dma-buf.h>
>> -
>> -struct exynos_drm_dmabuf_attachment {
>> -	struct sg_table *sgt;
>> -	enum dma_data_direction dir;
>> -	bool is_mapped;
>> -};
>> -
>> -static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
>> -{
>> -	return to_exynos_gem_obj(buf->priv);
>> -}
>> -
>> -static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
>> -					struct device *dev,
>> -					struct dma_buf_attachment *attach)
>> -{
>> -	struct exynos_drm_dmabuf_attachment *exynos_attach;
>> -
>> -	exynos_attach = kzalloc(sizeof(*exynos_attach), GFP_KERNEL);
>> -	if (!exynos_attach)
>> -		return -ENOMEM;
>> -
>> -	exynos_attach->dir = DMA_NONE;
>> -	attach->priv = exynos_attach;
>> -
>> -	return 0;
>> -}
>> -
>> -static void exynos_gem_detach_dma_buf(struct dma_buf *dmabuf,
>> -					struct dma_buf_attachment *attach)
>> -{
>> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
>> -	struct sg_table *sgt;
>> -
>> -	if (!exynos_attach)
>> -		return;
>> -
>> -	sgt = exynos_attach->sgt;
>> -	if (sgt) {
>> -		if (exynos_attach->dir != DMA_NONE)
>> -			dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
>> -					exynos_attach->dir);
>> -		sg_free_table(sgt);
>> -	}
>> -
>> -	kfree(sgt);
>> -	kfree(exynos_attach);
>> -	attach->priv = NULL;
>> -}
>> -
>> -static struct sg_table *
>> -		exynos_gem_map_dma_buf(struct dma_buf_attachment *attach,
>> -					enum dma_data_direction dir)
>> -{
>> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
>> -	struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
>> -	struct exynos_drm_gem_buf *buf;
>> -	struct sg_table *sgt;
>> -	int npages;
>> -
>> -	/* just return current sgt if already requested. */
>> -	if (exynos_attach->dir == dir && exynos_attach->is_mapped)
>> -		return exynos_attach->sgt;
>> -
>> -	buf = gem_obj->buffer;
>> -	if (!buf) {
>> -		DRM_ERROR("buffer is null.\n");
>> -		return ERR_PTR(-ENOMEM);
>> -	}
>> -
>> -	npages = buf->size >> PAGE_SHIFT;
>> -
>> -	sgt = drm_prime_pages_to_sg(buf->pages, npages);
>> -	if (IS_ERR(sgt))
>> -		goto err;
>> -
>> -	if (dir != DMA_NONE) {
>> -		if (!dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir)) {
>> -			DRM_ERROR("failed to map sgl with iommu.\n");
>> -			sg_free_table(sgt);
>> -			sgt = ERR_PTR(-EIO);
>> -			goto err;
>> -		}
>> -	}
>> -
>> -	exynos_attach->is_mapped = true;
>> -	exynos_attach->sgt = sgt;
>> -	exynos_attach->dir = dir;
>> -	attach->priv = exynos_attach;
>> -
>> -	DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
>> -
>> -err:
>> -	return sgt;
>> -}
>> -
>> -static void exynos_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
>> -						struct sg_table *sgt,
>> -						enum dma_data_direction dir)
>> -{
>> -	/* Nothing to do. */
>> -}
>> -
>> -static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
>> -						unsigned long page_num)
>> -{
>> -	/* TODO */
>> -
>> -	return NULL;
>> -}
>> -
>> -static void exynos_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
>> -						unsigned long page_num,
>> -						void *addr)
>> -{
>> -	/* TODO */
>> -}
>> -
>> -static void *exynos_gem_dmabuf_kmap(struct dma_buf *dma_buf,
>> -					unsigned long page_num)
>> -{
>> -	/* TODO */
>> -
>> -	return NULL;
>> -}
>> -
>> -static void exynos_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
>> -					unsigned long page_num, void *addr)
>> -{
>> -	/* TODO */
>> -}
>> -
>> -static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
>> -	struct vm_area_struct *vma)
>> -{
>> -	return -ENOTTY;
>> -}
>> -
>> -static struct dma_buf_ops exynos_dmabuf_ops = {
>> -	.attach			= exynos_gem_attach_dma_buf,
>> -	.detach			= exynos_gem_detach_dma_buf,
>> -	.map_dma_buf		= exynos_gem_map_dma_buf,
>> -	.unmap_dma_buf		= exynos_gem_unmap_dma_buf,
>> -	.kmap			= exynos_gem_dmabuf_kmap,
>> -	.kmap_atomic		= exynos_gem_dmabuf_kmap_atomic,
>> -	.kunmap			= exynos_gem_dmabuf_kunmap,
>> -	.kunmap_atomic		= exynos_gem_dmabuf_kunmap_atomic,
>> -	.mmap			= exynos_gem_dmabuf_mmap,
>> -	.release		= drm_gem_dmabuf_release,
>> -};
>> -
>> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
>> -				struct drm_gem_object *obj, int flags)
>> -{
>> -	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>> -	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>> -
>> -	exp_info.ops = &exynos_dmabuf_ops;
>> -	exp_info.size = exynos_gem_obj->base.size;
>> -	exp_info.flags = flags;
>> -	exp_info.priv = obj;
>> -
>> -	return dma_buf_export(&exp_info);
>> -}
>> -
>> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
>> -				struct dma_buf *dma_buf)
>> -{
>> -	struct dma_buf_attachment *attach;
>> -	struct sg_table *sgt;
>> -	struct scatterlist *sgl;
>> -	struct exynos_drm_gem_obj *exynos_gem_obj;
>> -	struct exynos_drm_gem_buf *buffer;
>> -	int npages;
>> -	int ret;
>> -
>> -	/* is this one of own objects? */
>> -	if (dma_buf->ops == &exynos_dmabuf_ops) {
>> -		struct drm_gem_object *obj;
>> -
>> -		obj = dma_buf->priv;
>> -
>> -		/* is it from our device? */
>> -		if (obj->dev == drm_dev) {
>> -			/*
>> -			 * Importing dmabuf exported from out own gem increases
>> -			 * refcount on gem itself instead of f_count of dmabuf.
>> -			 */
>> -			drm_gem_object_reference(obj);
>> -			return obj;
>> -		}
>> -	}
>> -
>> -	attach = dma_buf_attach(dma_buf, drm_dev->dev);
>> -	if (IS_ERR(attach))
>> -		return ERR_PTR(-EINVAL);
>> -
>> -	get_dma_buf(dma_buf);
>> -
>> -	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
>> -	if (IS_ERR(sgt)) {
>> -		ret = PTR_ERR(sgt);
>> -		goto err_buf_detach;
>> -	}
>> -
>> -	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
>> -	if (!buffer) {
>> -		ret = -ENOMEM;
>> -		goto err_unmap_attach;
>> -	}
>> -
>> -	exynos_gem_obj = exynos_drm_gem_init(drm_dev, dma_buf->size);
>> -	if (!exynos_gem_obj) {
>> -		ret = -ENOMEM;
>> -		goto err_free_buffer;
>> -	}
>> -
>> -	sgl = sgt->sgl;
>> -
>> -	buffer->size = dma_buf->size;
>> -	buffer->dma_addr = sg_dma_address(sgl);
>> -
>> -	npages = dma_buf->size >> PAGE_SHIFT;
>> -	buffer->pages = drm_malloc_ab(npages, sizeof(struct page *));
>> -	if (!buffer->pages) {
>> -		ret = -ENOMEM;
>> -		goto err_free_gem;
>> -	}
>> -
>> -	ret = drm_prime_sg_to_page_addr_arrays(sgt, buffer->pages, NULL,
>> -			npages);
>> -	if (ret < 0) {
>> -		drm_free_large(buffer->pages);
>> -		goto err_free_gem;
>> -	}
>> -
>> -	if (sgt->nents == 1) {
>> -		/* always physically continuous memory if sgt->nents is 1. */
>> -		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
>> -	} else {
>> -		/*
>> -		 * this case could be CONTIG or NONCONTIG type but for now
>> -		 * sets NONCONTIG.
>> -		 * TODO. we have to find a way that exporter can notify
>> -		 * the type of its own buffer to importer.
>> -		 */
>> -		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
>> -	}
>> -
>> -	exynos_gem_obj->buffer = buffer;
>> -	exynos_gem_obj->base.import_attach = attach;
>> -
>> -	DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n", &buffer->dma_addr,
>> -								buffer->size);
>> -
>> -	return &exynos_gem_obj->base;
>> -
>> -err_free_gem:
>> -	drm_gem_object_release(&exynos_gem_obj->base);
>> -	kfree(exynos_gem_obj);
>> -err_free_buffer:
>> -	kfree(buffer);
>> -	buffer = NULL;
>> -err_unmap_attach:
>> -	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
>> -err_buf_detach:
>> -	dma_buf_detach(dma_buf, attach);
>> -	dma_buf_put(dma_buf);
>> -
>> -	return ERR_PTR(ret);
>> -}
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
>> deleted file mode 100644
>> index 886de9f..0000000
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
>> +++ /dev/null
>> @@ -1,20 +0,0 @@
>> -/* exynos_drm_dmabuf.h
>> - *
>> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
>> - * Author: Inki Dae <inki.dae@samsung.com>
>> - *
>> - * This program is free software; you can redistribute  it and/or modify it
>> - * under  the terms of  the GNU General  Public License as published by the
>> - * Free Software Foundation;  either version 2 of the  License, or (at your
>> - * option) any later version.
>> - */
>> -
>> -#ifndef _EXYNOS_DRM_DMABUF_H_
>> -#define _EXYNOS_DRM_DMABUF_H_
>> -
>> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
>> -				struct drm_gem_object *obj, int flags);
>> -
>> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
>> -						struct dma_buf *dma_buf);
>> -#endif
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index f1d6966..e9ddaa2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -27,7 +27,6 @@
>>  #include "exynos_drm_gem.h"
>>  #include "exynos_drm_plane.h"
>>  #include "exynos_drm_vidi.h"
>> -#include "exynos_drm_dmabuf.h"
>>  #include "exynos_drm_g2d.h"
>>  #include "exynos_drm_ipp.h"
>>  #include "exynos_drm_iommu.h"
>> @@ -297,8 +296,12 @@ static struct drm_driver exynos_drm_driver = {
>>  	.dumb_destroy		= drm_gem_dumb_destroy,
>>  	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>>  	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
>> -	.gem_prime_export	= exynos_dmabuf_prime_export,
>> -	.gem_prime_import	= exynos_dmabuf_prime_import,
>> +	.gem_prime_export	= drm_gem_prime_export,
>> +	.gem_prime_import	= drm_gem_prime_import,
>> +	.gem_prime_get_sg_table	= exynos_drm_gem_prime_get_sg_table,
>> +	.gem_prime_import_sg_table	= exynos_drm_gem_prime_import_sg_table,
>> +	.gem_prime_vmap		= exynos_drm_gem_prime_vmap,
>> +	.gem_prime_vunmap	= exynos_drm_gem_prime_vunmap,
>>  	.ioctls			= exynos_ioctls,
>>  	.num_ioctls		= ARRAY_SIZE(exynos_ioctls),
>>  	.fops			= &exynos_drm_driver_fops,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 9df100f..7d4bf9d 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> @@ -13,6 +13,7 @@
>>  #include <drm/drm_vma_manager.h>
>>  
>>  #include <linux/shmem_fs.h>
>> +#include <linux/dma-buf.h>
>>  #include <drm/exynos_drm.h>
>>  
>>  #include "exynos_drm_drv.h"
>> @@ -580,3 +581,81 @@ err_close_vm:
>>  
>>  	return ret;
>>  }
>> +
>> +/* low-level interface prime helpers */
>> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
>> +{
>> +	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
>> +	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
>> +	int npages;
>> +
>> +	npages = buf->size >> PAGE_SHIFT;
>> +
>> +	return drm_prime_pages_to_sg(buf->pages, npages);
>> +}
>> +
>> +struct drm_gem_object *
>> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>> +				     struct dma_buf_attachment *attach,
>> +				     struct sg_table *sgt)
>> +{
>> +	struct exynos_drm_gem_obj *exynos_gem_obj;
>> +	struct exynos_drm_gem_buf *buf;
>> +	int npages;
>> +	int ret;
>> +
>> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>> +	if (!buf)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	buf->size = attach->dmabuf->size;
>> +	buf->dma_addr = sg_dma_address(sgt->sgl);
>> +
>> +	npages = buf->size >> PAGE_SHIFT;
>> +	buf->pages = drm_malloc_ab(npages, sizeof(struct page *));
>> +	if (!buf->pages) {
>> +		ret = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	ret = drm_prime_sg_to_page_addr_arrays(sgt, buf->pages, NULL, npages);
>> +	if (ret < 0)
>> +		goto err_free_large;
>> +
>> +	exynos_gem_obj = exynos_drm_gem_init(dev, buf->size);
>> +	if (IS_ERR(exynos_gem_obj)) {
>> +		ret = PTR_ERR(exynos_gem_obj);
>> +		goto err;
>> +	}
>> +
>> +	if (sgt->nents == 1) {
>> +		/* always physically continuous memory if sgt->nents is 1. */
>> +		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
>> +	} else {
>> +		/*
>> +		 * this case could be CONTIG or NONCONTIG type but for now
>> +		 * sets NONCONTIG.
>> +		 * TODO. we have to find a way that exporter can notify
>> +		 * the type of its own buffer to importer.
>> +		 */
>> +		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
>> +	}
>> +
>> +	return &exynos_gem_obj->base;
>> +
>> +err_free_large:
>> +	drm_free_large(buf->pages);
>> +err:
>> +	kfree(buf);
>> +	return ERR_PTR(ret);
>> +}
>> +
>> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj)
>> +{
>> +	return NULL;
>> +}
>> +
>> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
>> +{
>> +	/* Nothing to do */
>> +}
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> index 49b5ef1..5e20da6 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
>> @@ -168,4 +168,13 @@ void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
>>  				struct sg_table *sgt,
>>  				enum dma_data_direction dir);
>>  
>> +/* low-level interface prime helpers */
>> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj);
>> +struct drm_gem_object *
>> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
>> +				     struct dma_buf_attachment *attach,
>> +				     struct sg_table *sgt);
>> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj);
>> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
>> +
>>  #endif
>>
> 

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

[PATCH 14/14] drm/exynos: merge exynos_drm_buf.c to exynos_drm_gem.c

[Joonyoung Shim]

The struct exynos_drm_gem_obj can have fields of the struct
exynos_drm_gem_buf then don't need to use exynos_drm_buf.c file.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/Makefile           |   3 +-
 drivers/gpu/drm/exynos/exynos_drm_buf.c   | 170 ------------------------
 drivers/gpu/drm/exynos/exynos_drm_buf.h   |  33 -----
 drivers/gpu/drm/exynos/exynos_drm_fb.c    |  14 +-
 drivers/gpu/drm/exynos/exynos_drm_fb.h    |   4 +-
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  30 ++---
 drivers/gpu/drm/exynos/exynos_drm_gem.c   | 209 +++++++++++++++++++-----------
 drivers/gpu/drm/exynos/exynos_drm_gem.h   |  40 +++---
 drivers/gpu/drm/exynos/exynos_drm_plane.c |  10 +-
 9 files changed, 179 insertions(+), 334 deletions(-)
 delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_buf.c
 delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_buf.h

diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
index 95d0306..9c72535 100644
--- a/drivers/gpu/drm/exynos/Makefile
+++ b/drivers/gpu/drm/exynos/Makefile
@@ -5,8 +5,7 @@
 ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/exynos
 exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \
 		exynos_drm_crtc.o exynos_drm_fbdev.o exynos_drm_fb.o \
-		exynos_drm_buf.o exynos_drm_gem.o exynos_drm_core.o \
-		exynos_drm_plane.o
+		exynos_drm_gem.o exynos_drm_core.o exynos_drm_plane.o
 
 exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o
 exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)	+= exynos_drm_fimd.o
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c b/drivers/gpu/drm/exynos/exynos_drm_buf.c
deleted file mode 100644
index 9260dfb..0000000
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ /dev/null
@@ -1,170 +0,0 @@
-/* exynos_drm_buf.c
- *
- * Copyright (c) 2011 Samsung Electronics Co., Ltd.
- * Author: Inki Dae <inki.dae@samsung.com>
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#include <drm/drmP.h>
-#include <drm/exynos_drm.h>
-
-#include "exynos_drm_drv.h"
-#include "exynos_drm_gem.h"
-#include "exynos_drm_buf.h"
-#include "exynos_drm_iommu.h"
-
-static int lowlevel_buffer_allocate(struct drm_device *dev,
-		unsigned int flags, struct exynos_drm_gem_buf *buf)
-{
-	int ret = 0;
-	enum dma_attr attr;
-	unsigned int nr_pages;
-
-	if (buf->dma_addr) {
-		DRM_DEBUG_KMS("already allocated.\n");
-		return 0;
-	}
-
-	init_dma_attrs(&buf->dma_attrs);
-
-	/*
-	 * if EXYNOS_BO_CONTIG, fully physically contiguous memory
-	 * region will be allocated else physically contiguous
-	 * as possible.
-	 */
-	if (!(flags & EXYNOS_BO_NONCONTIG))
-		dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &buf->dma_attrs);
-
-	/*
-	 * if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping
-	 * else cachable mapping.
-	 */
-	if (flags & EXYNOS_BO_WC || !(flags & EXYNOS_BO_CACHABLE))
-		attr = DMA_ATTR_WRITE_COMBINE;
-	else
-		attr = DMA_ATTR_NON_CONSISTENT;
-
-	dma_set_attr(attr, &buf->dma_attrs);
-	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &buf->dma_attrs);
-
-	nr_pages = buf->size >> PAGE_SHIFT;
-
-	if (!is_drm_iommu_supported(dev)) {
-		dma_addr_t start_addr;
-		unsigned int i = 0;
-
-		buf->pages = drm_calloc_large(nr_pages, sizeof(struct page *));
-		if (!buf->pages) {
-			DRM_ERROR("failed to allocate pages.\n");
-			return -ENOMEM;
-		}
-
-		buf->cookie = dma_alloc_attrs(dev->dev,
-					buf->size,
-					&buf->dma_addr, GFP_KERNEL,
-					&buf->dma_attrs);
-		if (!buf->cookie) {
-			DRM_ERROR("failed to allocate buffer.\n");
-			ret = -ENOMEM;
-			goto err_free;
-		}
-
-		start_addr = buf->dma_addr;
-		while (i < nr_pages) {
-			buf->pages[i] = phys_to_page(start_addr);
-			start_addr += PAGE_SIZE;
-			i++;
-		}
-	} else {
-
-		buf->pages = dma_alloc_attrs(dev->dev, buf->size,
-					&buf->dma_addr, GFP_KERNEL,
-					&buf->dma_attrs);
-		if (!buf->pages) {
-			DRM_ERROR("failed to allocate buffer.\n");
-			return -ENOMEM;
-		}
-	}
-
-	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
-			(unsigned long)buf->dma_addr,
-			buf->size);
-
-	return ret;
-
-err_free:
-	if (!is_drm_iommu_supported(dev))
-		drm_free_large(buf->pages);
-
-	return ret;
-}
-
-static void lowlevel_buffer_deallocate(struct drm_device *dev,
-		unsigned int flags, struct exynos_drm_gem_buf *buf)
-{
-	if (!buf->dma_addr) {
-		DRM_DEBUG_KMS("dma_addr is invalid.\n");
-		return;
-	}
-
-	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
-			(unsigned long)buf->dma_addr,
-			buf->size);
-
-	if (!is_drm_iommu_supported(dev)) {
-		dma_free_attrs(dev->dev, buf->size, buf->cookie,
-				(dma_addr_t)buf->dma_addr, &buf->dma_attrs);
-		drm_free_large(buf->pages);
-	} else
-		dma_free_attrs(dev->dev, buf->size, buf->pages,
-				(dma_addr_t)buf->dma_addr, &buf->dma_attrs);
-
-	buf->dma_addr = (dma_addr_t)NULL;
-}
-
-struct exynos_drm_gem_buf *exynos_drm_init_buf(struct drm_device *dev,
-						unsigned int size)
-{
-	struct exynos_drm_gem_buf *buffer;
-
-	DRM_DEBUG_KMS("desired size = 0x%x\n", size);
-
-	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
-	if (!buffer)
-		return NULL;
-
-	buffer->size = size;
-	return buffer;
-}
-
-void exynos_drm_fini_buf(struct drm_device *dev,
-				struct exynos_drm_gem_buf *buffer)
-{
-	kfree(buffer);
-	buffer = NULL;
-}
-
-int exynos_drm_alloc_buf(struct drm_device *dev,
-		struct exynos_drm_gem_buf *buf, unsigned int flags)
-{
-
-	/*
-	 * allocate memory region and set the memory information
-	 * to vaddr and dma_addr of a buffer object.
-	 */
-	if (lowlevel_buffer_allocate(dev, flags, buf) < 0)
-		return -ENOMEM;
-
-	return 0;
-}
-
-void exynos_drm_free_buf(struct drm_device *dev,
-		unsigned int flags, struct exynos_drm_gem_buf *buffer)
-{
-
-	lowlevel_buffer_deallocate(dev, flags, buffer);
-}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h b/drivers/gpu/drm/exynos/exynos_drm_buf.h
deleted file mode 100644
index a6412f1..0000000
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* exynos_drm_buf.h
- *
- * Copyright (c) 2011 Samsung Electronics Co., Ltd.
- * Author: Inki Dae <inki.dae@samsung.com>
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#ifndef _EXYNOS_DRM_BUF_H_
-#define _EXYNOS_DRM_BUF_H_
-
-/* create and initialize buffer object. */
-struct exynos_drm_gem_buf *exynos_drm_init_buf(struct drm_device *dev,
-						unsigned int size);
-
-/* destroy buffer object. */
-void exynos_drm_fini_buf(struct drm_device *dev,
-				struct exynos_drm_gem_buf *buffer);
-
-/* allocate physical memory region and setup sgt. */
-int exynos_drm_alloc_buf(struct drm_device *dev,
-				struct exynos_drm_gem_buf *buf,
-				unsigned int flags);
-
-/* release physical memory region, and sgt. */
-void exynos_drm_free_buf(struct drm_device *dev,
-				unsigned int flags,
-				struct exynos_drm_gem_buf *buffer);
-
-#endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index 2b6320e..9738f4e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -238,22 +238,22 @@ err_free:
 	return ERR_PTR(ret);
 }
 
-struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer *fb,
-						int index)
+struct exynos_drm_gem_obj *exynos_drm_fb_gem_obj(struct drm_framebuffer *fb,
+						 int index)
 {
 	struct exynos_drm_fb *exynos_fb = to_exynos_fb(fb);
-	struct exynos_drm_gem_buf *buffer;
+	struct exynos_drm_gem_obj *obj;
 
 	if (index >= MAX_FB_BUFFER)
 		return NULL;
 
-	buffer = exynos_fb->exynos_gem_obj[index]->buffer;
-	if (!buffer)
+	obj = exynos_fb->exynos_gem_obj[index];
+	if (!obj)
 		return NULL;
 
-	DRM_DEBUG_KMS("dma_addr = 0x%lx\n", (unsigned long)buffer->dma_addr);
+	DRM_DEBUG_KMS("dma_addr = 0x%lx\n", (unsigned long)obj->dma_addr);
 
-	return buffer;
+	return obj;
 }
 
 static void exynos_drm_output_poll_changed(struct drm_device *dev)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.h b/drivers/gpu/drm/exynos/exynos_drm_fb.h
index 517471b..1c9e27c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.h
@@ -19,8 +19,8 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
 			    struct drm_mode_fb_cmd2 *mode_cmd,
 			    struct drm_gem_object *obj);
 
-/* get memory information of a drm framebuffer */
-struct exynos_drm_gem_buf *exynos_drm_fb_buffer(struct drm_framebuffer *fb,
+/* get gem object of a drm framebuffer */
+struct exynos_drm_gem_obj *exynos_drm_fb_gem_obj(struct drm_framebuffer *fb,
 						 int index);
 
 void exynos_drm_mode_config_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index e0b085b..cd4ad83 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -40,8 +40,7 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
 {
 	struct drm_fb_helper *helper = info->par;
 	struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
-	struct exynos_drm_gem_obj *exynos_gem_obj = exynos_fbd->exynos_gem_obj;
-	struct exynos_drm_gem_buf *buffer = exynos_gem_obj->buffer;
+	struct exynos_drm_gem_obj *obj = exynos_fbd->exynos_gem_obj;
 	unsigned long vm_size;
 	int ret;
 
@@ -49,11 +48,11 @@ static int exynos_drm_fb_mmap(struct fb_info *info,
 
 	vm_size = vma->vm_end - vma->vm_start;
 
-	if (vm_size > buffer->size)
+	if (vm_size > obj->size)
 		return -EINVAL;
 
-	ret = dma_mmap_attrs(helper->dev->dev, vma, buffer->pages,
-		buffer->dma_addr, buffer->size, &buffer->dma_attrs);
+	ret = dma_mmap_attrs(helper->dev->dev, vma, obj->pages, obj->dma_addr,
+			     obj->size, &obj->dma_attrs);
 	if (ret < 0) {
 		DRM_ERROR("failed to mmap.\n");
 		return ret;
@@ -80,7 +79,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 				     struct drm_framebuffer *fb)
 {
 	struct fb_info *fbi = helper->fbdev;
-	struct exynos_drm_gem_buf *buffer;
+	struct exynos_drm_gem_obj *obj;
 	unsigned int size = fb->width * fb->height * (fb->bits_per_pixel >> 3);
 	unsigned int nr_pages;
 	unsigned long offset;
@@ -89,18 +88,17 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 	drm_fb_helper_fill_var(fbi, helper, sizes->fb_width, sizes->fb_height);
 
 	/* RGB formats use only one buffer */
-	buffer = exynos_drm_fb_buffer(fb, 0);
-	if (!buffer) {
-		DRM_DEBUG_KMS("buffer is null.\n");
+	obj = exynos_drm_fb_gem_obj(fb, 0);
+	if (!obj) {
+		DRM_DEBUG_KMS("gem object is null.\n");
 		return -EFAULT;
 	}
 
-	nr_pages = buffer->size >> PAGE_SHIFT;
+	nr_pages = obj->size >> PAGE_SHIFT;
 
-	buffer->kvaddr = (void __iomem *) vmap(buffer->pages,
-			nr_pages, VM_MAP,
+	obj->kvaddr = (void __iomem *) vmap(obj->pages, nr_pages, VM_MAP,
 			pgprot_writecombine(PAGE_KERNEL));
-	if (!buffer->kvaddr) {
+	if (!obj->kvaddr) {
 		DRM_ERROR("failed to map pages to kernel space.\n");
 		return -EIO;
 	}
@@ -111,7 +109,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 	offset = fbi->var.xoffset * (fb->bits_per_pixel >> 3);
 	offset += fbi->var.yoffset * fb->pitches[0];
 
-	fbi->screen_base = buffer->kvaddr + offset;
+	fbi->screen_base = obj->kvaddr + offset;
 	fbi->screen_size = size;
 	fbi->fix.smem_len = size;
 
@@ -300,8 +298,8 @@ static void exynos_drm_fbdev_destroy(struct drm_device *dev,
 	struct exynos_drm_gem_obj *exynos_gem_obj = exynos_fbd->exynos_gem_obj;
 	struct drm_framebuffer *fb;
 
-	if (exynos_gem_obj->buffer->kvaddr)
-		vunmap(exynos_gem_obj->buffer->kvaddr);
+	if (exynos_gem_obj->kvaddr)
+		vunmap(exynos_gem_obj->kvaddr);
 
 	/* release drm framebuffer and real buffer */
 	if (fb_helper->fb && fb_helper->fb->funcs) {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 7d4bf9d..4ea12a7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -18,9 +18,109 @@
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_gem.h"
-#include "exynos_drm_buf.h"
 #include "exynos_drm_iommu.h"
 
+static int exynos_drm_alloc_buf(struct exynos_drm_gem_obj *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	enum dma_attr attr;
+	unsigned int nr_pages;
+
+	if (obj->dma_addr) {
+		DRM_DEBUG_KMS("already allocated.\n");
+		return 0;
+	}
+
+	init_dma_attrs(&obj->dma_attrs);
+
+	/*
+	 * if EXYNOS_BO_CONTIG, fully physically contiguous memory
+	 * region will be allocated else physically contiguous
+	 * as possible.
+	 */
+	if (!(obj->flags & EXYNOS_BO_NONCONTIG))
+		dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &obj->dma_attrs);
+
+	/*
+	 * if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping
+	 * else cachable mapping.
+	 */
+	if (obj->flags & EXYNOS_BO_WC || !(obj->flags & EXYNOS_BO_CACHABLE))
+		attr = DMA_ATTR_WRITE_COMBINE;
+	else
+		attr = DMA_ATTR_NON_CONSISTENT;
+
+	dma_set_attr(attr, &obj->dma_attrs);
+	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &obj->dma_attrs);
+
+	nr_pages = obj->size >> PAGE_SHIFT;
+
+	if (!is_drm_iommu_supported(dev)) {
+		dma_addr_t start_addr;
+		unsigned int i = 0;
+
+		obj->pages = drm_calloc_large(nr_pages, sizeof(struct page *));
+		if (!obj->pages) {
+			DRM_ERROR("failed to allocate pages.\n");
+			return -ENOMEM;
+		}
+
+		obj->cookie = dma_alloc_attrs(dev->dev,
+					obj->size,
+					&obj->dma_addr, GFP_KERNEL,
+					&obj->dma_attrs);
+		if (!obj->cookie) {
+			DRM_ERROR("failed to allocate buffer.\n");
+			drm_free_large(obj->pages);
+			return -ENOMEM;
+		}
+
+		start_addr = obj->dma_addr;
+		while (i < nr_pages) {
+			obj->pages[i] = phys_to_page(start_addr);
+			start_addr += PAGE_SIZE;
+			i++;
+		}
+	} else {
+		obj->pages = dma_alloc_attrs(dev->dev, obj->size,
+					&obj->dma_addr, GFP_KERNEL,
+					&obj->dma_attrs);
+		if (!obj->pages) {
+			DRM_ERROR("failed to allocate buffer.\n");
+			return -ENOMEM;
+		}
+	}
+
+	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
+			(unsigned long)obj->dma_addr,
+			obj->size);
+
+	return 0;
+}
+
+static void exynos_drm_free_buf(struct exynos_drm_gem_obj *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+
+	if (!obj->dma_addr) {
+		DRM_DEBUG_KMS("dma_addr is invalid.\n");
+		return;
+	}
+
+	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
+			(unsigned long)obj->dma_addr, obj->size);
+
+	if (!is_drm_iommu_supported(dev)) {
+		dma_free_attrs(dev->dev, obj->size, obj->cookie,
+				(dma_addr_t)obj->dma_addr, &obj->dma_attrs);
+		drm_free_large(obj->pages);
+	} else
+		dma_free_attrs(dev->dev, obj->size, obj->pages,
+				(dma_addr_t)obj->dma_addr, &obj->dma_attrs);
+
+	obj->dma_addr = (dma_addr_t)NULL;
+}
+
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
 					struct drm_file *file_priv,
 					unsigned int *handle)
@@ -45,11 +145,7 @@ static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
 
 void exynos_drm_gem_destroy(struct exynos_drm_gem_obj *exynos_gem_obj)
 {
-	struct drm_gem_object *obj;
-	struct exynos_drm_gem_buf *buf;
-
-	obj = &exynos_gem_obj->base;
-	buf = exynos_gem_obj->buffer;
+	struct drm_gem_object *obj = &exynos_gem_obj->base;
 
 	DRM_DEBUG_KMS("handle count = %d\n", obj->handle_count);
 
@@ -62,12 +158,9 @@ void exynos_drm_gem_destroy(struct exynos_drm_gem_obj *exynos_gem_obj)
 	if (obj->import_attach)
 		goto out;
 
-	exynos_drm_free_buf(obj->dev, exynos_gem_obj->flags, buf);
+	exynos_drm_free_buf(exynos_gem_obj);
 
 out:
-	exynos_drm_fini_buf(obj->dev, buf);
-	exynos_gem_obj->buffer = NULL;
-
 	/* release file pointer to gem object. */
 	drm_gem_object_release(obj);
 
@@ -92,7 +185,7 @@ unsigned long exynos_drm_gem_get_size(struct drm_device *dev,
 
 	drm_gem_object_unreference_unlocked(obj);
 
-	return exynos_gem_obj->buffer->size;
+	return exynos_gem_obj->size;
 }
 
 
@@ -134,7 +227,6 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 						unsigned long size)
 {
 	struct exynos_drm_gem_obj *exynos_gem_obj;
-	struct exynos_drm_gem_buf *buf;
 	int ret;
 
 	if (flags & ~(EXYNOS_BO_MASK)) {
@@ -149,33 +241,21 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
 
 	size = roundup(size, PAGE_SIZE);
 
-	buf = exynos_drm_init_buf(dev, size);
-	if (!buf)
-		return ERR_PTR(-ENOMEM);
-
 	exynos_gem_obj = exynos_drm_gem_init(dev, size);
-	if (IS_ERR(exynos_gem_obj)) {
-		ret = PTR_ERR(exynos_gem_obj);
-		goto err_fini_buf;
-	}
-
-	exynos_gem_obj->buffer = buf;
+	if (IS_ERR(exynos_gem_obj))
+		return exynos_gem_obj;
 
 	/* set memory type and cache attribute from user side. */
 	exynos_gem_obj->flags = flags;
 
-	ret = exynos_drm_alloc_buf(dev, buf, flags);
-	if (ret < 0)
-		goto err_gem_fini;
+	ret = exynos_drm_alloc_buf(exynos_gem_obj);
+	if (ret < 0) {
+		drm_gem_object_release(&exynos_gem_obj->base);
+		kfree(exynos_gem_obj);
+		return ERR_PTR(ret);
+	}
 
 	return exynos_gem_obj;
-
-err_gem_fini:
-	drm_gem_object_release(&exynos_gem_obj->base);
-	kfree(exynos_gem_obj);
-err_fini_buf:
-	exynos_drm_fini_buf(dev, buf);
-	return ERR_PTR(ret);
 }
 
 int exynos_drm_gem_create_ioctl(struct drm_device *dev, void *data,
@@ -214,7 +294,7 @@ dma_addr_t *exynos_drm_gem_get_dma_addr(struct drm_device *dev,
 
 	exynos_gem_obj = to_exynos_gem_obj(obj);
 
-	return &exynos_gem_obj->buffer->dma_addr;
+	return &exynos_gem_obj->dma_addr;
 }
 
 void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
@@ -242,7 +322,6 @@ int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem_obj *exynos_gem_obj,
 				      struct vm_area_struct *vma)
 {
 	struct drm_device *drm_dev = exynos_gem_obj->base.dev;
-	struct exynos_drm_gem_buf *buffer;
 	unsigned long vm_size;
 	int ret;
 
@@ -251,19 +330,13 @@ int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem_obj *exynos_gem_obj,
 
 	vm_size = vma->vm_end - vma->vm_start;
 
-	/*
-	 * a buffer contains information to physically continuous memory
-	 * allocated by user request or at framebuffer creation.
-	 */
-	buffer = exynos_gem_obj->buffer;
-
 	/* check if user-requested size is valid. */
-	if (vm_size > buffer->size)
+	if (vm_size > exynos_gem_obj->size)
 		return -EINVAL;
 
-	ret = dma_mmap_attrs(drm_dev->dev, vma, buffer->pages,
-				buffer->dma_addr, buffer->size,
-				&buffer->dma_attrs);
+	ret = dma_mmap_attrs(drm_dev->dev, vma, exynos_gem_obj->pages,
+				exynos_gem_obj->dma_addr, exynos_gem_obj->size,
+				&exynos_gem_obj->dma_attrs);
 	if (ret < 0) {
 		DRM_ERROR("failed to mmap.\n");
 		return ret;
@@ -423,12 +496,6 @@ void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
 
 void exynos_drm_gem_free_object(struct drm_gem_object *obj)
 {
-	struct exynos_drm_gem_obj *exynos_gem_obj;
-	struct exynos_drm_gem_buf *buf;
-
-	exynos_gem_obj = to_exynos_gem_obj(obj);
-	buf = exynos_gem_obj->buffer;
-
 	exynos_drm_gem_destroy(to_exynos_gem_obj(obj));
 }
 
@@ -508,7 +575,6 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct drm_gem_object *obj = vma->vm_private_data;
 	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
-	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
 	unsigned long pfn;
 	pgoff_t page_offset;
 	int ret;
@@ -516,13 +582,13 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	page_offset = ((unsigned long)vmf->virtual_address -
 			vma->vm_start) >> PAGE_SHIFT;
 
-	if (page_offset >= (buf->size >> PAGE_SHIFT)) {
+	if (page_offset >= (exynos_gem_obj->size >> PAGE_SHIFT)) {
 		DRM_ERROR("invalid page offset\n");
 		ret = -EINVAL;
 		goto out;
 	}
 
-	pfn = page_to_pfn(buf->pages[page_offset]);
+	pfn = page_to_pfn(exynos_gem_obj->pages[page_offset]);
 	ret = vm_insert_mixed(vma, (unsigned long)vmf->virtual_address, pfn);
 
 out:
@@ -586,12 +652,11 @@ err_close_vm:
 struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
 {
 	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
-	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
 	int npages;
 
-	npages = buf->size >> PAGE_SHIFT;
+	npages = exynos_gem_obj->size >> PAGE_SHIFT;
 
-	return drm_prime_pages_to_sg(buf->pages, npages);
+	return drm_prime_pages_to_sg(exynos_gem_obj->pages, npages);
 }
 
 struct drm_gem_object *
@@ -600,34 +665,29 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
 				     struct sg_table *sgt)
 {
 	struct exynos_drm_gem_obj *exynos_gem_obj;
-	struct exynos_drm_gem_buf *buf;
 	int npages;
 	int ret;
 
-	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
-	if (!buf)
-		return ERR_PTR(-ENOMEM);
+	exynos_gem_obj = exynos_drm_gem_init(dev, attach->dmabuf->size);
+	if (IS_ERR(exynos_gem_obj)) {
+		ret = PTR_ERR(exynos_gem_obj);
+		goto err;
+	}
 
-	buf->size = attach->dmabuf->size;
-	buf->dma_addr = sg_dma_address(sgt->sgl);
+	exynos_gem_obj->dma_addr = sg_dma_address(sgt->sgl);
 
-	npages = buf->size >> PAGE_SHIFT;
-	buf->pages = drm_malloc_ab(npages, sizeof(struct page *));
-	if (!buf->pages) {
+	npages = exynos_gem_obj->size >> PAGE_SHIFT;
+	exynos_gem_obj->pages = drm_malloc_ab(npages, sizeof(struct page *));
+	if (!exynos_gem_obj->pages) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
-	ret = drm_prime_sg_to_page_addr_arrays(sgt, buf->pages, NULL, npages);
+	ret = drm_prime_sg_to_page_addr_arrays(sgt, exynos_gem_obj->pages, NULL,
+			npages);
 	if (ret < 0)
 		goto err_free_large;
 
-	exynos_gem_obj = exynos_drm_gem_init(dev, buf->size);
-	if (IS_ERR(exynos_gem_obj)) {
-		ret = PTR_ERR(exynos_gem_obj);
-		goto err;
-	}
-
 	if (sgt->nents == 1) {
 		/* always physically continuous memory if sgt->nents is 1. */
 		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
@@ -644,9 +704,10 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
 	return &exynos_gem_obj->base;
 
 err_free_large:
-	drm_free_large(buf->pages);
+	drm_free_large(exynos_gem_obj->pages);
 err:
-	kfree(buf);
+	drm_gem_object_release(&exynos_gem_obj->base);
+	kfree(exynos_gem_obj);
 	return ERR_PTR(ret);
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 5e20da6..cd62f84 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -20,26 +20,6 @@
 #define IS_NONCONTIG_BUFFER(f)		(f & EXYNOS_BO_NONCONTIG)
 
 /*
- * exynos drm gem buffer structure.
- *
- * @cookie: cookie returned by dma_alloc_attrs
- * @kvaddr: kernel virtual address to allocated memory region.
- * @dma_addr: bus address(accessed by dma) to allocated memory region.
- *	- this address could be physical address without IOMMU and
- *	device address with IOMMU.
- * @pages: Array of backing pages.
- * @size: size of allocated memory region.
- */
-struct exynos_drm_gem_buf {
-	void 			*cookie;
-	void __iomem		*kvaddr;
-	dma_addr_t		dma_addr;
-	struct dma_attrs	dma_attrs;
-	struct page		**pages;
-	unsigned long		size;
-};
-
-/*
  * exynos drm buffer structure.
  *
  * @base: a gem object.
@@ -50,18 +30,28 @@ struct exynos_drm_gem_buf {
  *	by user request or at framebuffer creation.
  *	continuous memory region allocated by user request
  *	or at framebuffer creation.
+ * @flags: indicate memory type to allocated buffer and cache attruibute.
  * @size: size requested from user, in bytes and this size is aligned
  *	in page unit.
- * @flags: indicate memory type to allocated buffer and cache attruibute.
+ * @cookie: cookie returned by dma_alloc_attrs
+ * @kvaddr: kernel virtual address to allocated memory region.
+ * @dma_addr: bus address(accessed by dma) to allocated memory region.
+ *	- this address could be physical address without IOMMU and
+ *	device address with IOMMU.
+ * @pages: Array of backing pages.
  *
  * P.S. this object would be transferred to user as kms_bo.handle so
  *	user can access the buffer through kms_bo.handle.
  */
 struct exynos_drm_gem_obj {
-	struct drm_gem_object		base;
-	struct exynos_drm_gem_buf	*buffer;
-	unsigned long			size;
-	unsigned int			flags;
+	struct drm_gem_object	base;
+	unsigned int		flags;
+	unsigned long		size;
+	void			*cookie;
+	void __iomem		*kvaddr;
+	dma_addr_t		dma_addr;
+	struct dma_attrs	dma_attrs;
+	struct page		**pages;
 };
 
 struct page **exynos_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index a729980..0b657a8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -145,15 +145,15 @@ static int exynos_plane_atomic_check(struct drm_plane *plane,
 
 	nr = exynos_drm_fb_get_buf_cnt(state->fb);
 	for (i = 0; i < nr; i++) {
-		struct exynos_drm_gem_buf *buffer =
-					exynos_drm_fb_buffer(state->fb, i);
+		struct exynos_drm_gem_obj *obj =
+					exynos_drm_fb_gem_obj(state->fb, i);
 
-		if (!buffer) {
-			DRM_DEBUG_KMS("buffer is null\n");
+		if (!obj) {
+			DRM_DEBUG_KMS("gem object is null\n");
 			return -EFAULT;
 		}
 
-		exynos_plane->dma_addr[i] = buffer->dma_addr +
+		exynos_plane->dma_addr[i] = obj->dma_addr +
 					    state->fb->offsets[i];
 
 		DRM_DEBUG_KMS("buffer: %d, dma_addr = 0x%lx\n",
-- 
1.9.1

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