[PATCH 0/9] drm/exynos: update codes related with gem

[PATCH 0/9] drm/exynos: update codes related with gem

[Joonyoung Shim]

Hi,

This patchset is about gem codes update of exynos-drm.

The first and second patches are cleanup to remove useless codes.
The rest is to support cachable gem allocation.

The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
doesn't give any effects.

This patchset introduces new buffer allocation to use
drm_gem_get/put_pages() instead of DMA mapping API. It will be used
for the rest except allocation of physically continuous buffer on
non-iommu, then exynos-drm can support cachable buffer of gem. Also it
can support physically non-continuous buffer on non-iommu.

EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
whether buffer is continuous physically if iommu is supported. This
patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
mean that the buffer is continuous for device.

There is no user API to control cache coherence for the cpu and device
about cachable buffer. This patchset introduces two ioctls for cpu
access of gem object from user. It will be synced properly in order for
the cpu and device if buffer of gem object is cachable.

Thanks.

Joonyoung Shim (9):
  drm/exynos: eliminate useless codes of exynos_drm_gem.h
  drm/exynos: use directly DMA mapping APIs on g2d
  drm/exynos: remove using non-consistent DMA attribute
  drm/exynos: split buffer allocation using DMA mapping API on non-iommu
  drm/exynos: introduce buffer allocation using drm_gem_get/put_pages()
  drm/exynos: switch to new buffer allocation
  drm/exynos: always use EXYNOS_BO_CONTIG flag on iommu
  drm/exynos: use DMA_ERROR_CODE
  drm/exynos: add ioctls for cpu access of gem object from user

 drivers/gpu/drm/exynos/exynos_drm_drv.c   |   4 +
 drivers/gpu/drm/exynos/exynos_drm_fb.c    |  32 +---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  14 +-
 drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  10 +-
 drivers/gpu/drm/exynos/exynos_drm_gem.c   | 288 ++++++++++++++++++------------
 drivers/gpu/drm/exynos/exynos_drm_gem.h   |  56 ++----
 include/uapi/drm/exynos_drm.h             |  23 ++-
 7 files changed, 225 insertions(+), 202 deletions(-)

-- 
1.9.1

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

[PATCH 1/9] drm/exynos: eliminate useless codes of exynos_drm_gem.h

[Joonyoung Shim]

This eliminates declaration of functions that is removed by the
commit 63540f01917c ("[media] drm/exynos: Convert
g2d_userptr_get_dma_addr() to use get_vaddr_frames()") and eliminate
vma_is_io() that isn't used anywhere now.

Also remove exynos_gem_get_pages() and exynos_drm_gem_userptr_ioctl(),
there is no body for them.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 0d0ab27b48fa..00223052b87b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -55,8 +55,6 @@ struct exynos_drm_gem {
 	struct sg_table		*sgt;
 };
 
-struct page **exynos_gem_get_pages(struct drm_gem_object *obj, gfp_t gfpmask);
-
 /* destroy a buffer with gem object */
 void exynos_drm_gem_destroy(struct exynos_drm_gem *exynos_gem);
 
@@ -95,10 +93,6 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
 					unsigned int gem_handle,
 					struct drm_file *filp);
 
-/* map user space allocated by malloc to pages. */
-int exynos_drm_gem_userptr_ioctl(struct drm_device *dev, void *data,
-				      struct drm_file *file_priv);
-
 /* get buffer information to memory region allocated by gem. */
 int exynos_drm_gem_get_ioctl(struct drm_device *dev, void *data,
 				      struct drm_file *file_priv);
@@ -127,28 +121,6 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
 /* set vm_flags and we can change the vm attribute to other one at here. */
 int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
-static inline int vma_is_io(struct vm_area_struct *vma)
-{
-	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
-}
-
-/* get a copy of a virtual memory region. */
-struct vm_area_struct *exynos_gem_get_vma(struct vm_area_struct *vma);
-
-/* release a userspace virtual memory area. */
-void exynos_gem_put_vma(struct vm_area_struct *vma);
-
-/* get pages from user space. */
-int exynos_gem_get_pages_from_userptr(unsigned long start,
-						unsigned int npages,
-						struct page **pages,
-						struct vm_area_struct *vma);
-
-/* drop the reference to pages. */
-void exynos_gem_put_pages_to_userptr(struct page **pages,
-					unsigned int npages,
-					struct vm_area_struct *vma);
-
 /* map sgt with dma region. */
 int exynos_gem_map_sgt_with_dma(struct drm_device *drm_dev,
 				struct sg_table *sgt,
-- 
1.9.1

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

[PATCH 2/9] drm/exynos: use directly DMA mapping APIs on g2d

[Joonyoung Shim]

There is no reason to be wapper functions to use DMA mapping APIs. We
can use directly DMA mapping APIs without locking and remove the wapper
functions.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 10 +++++-----
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 26 --------------------------
 drivers/gpu/drm/exynos/exynos_drm_gem.h | 10 ----------
 3 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index c17efdb238a6..24d84a4298ac 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -365,6 +365,7 @@ static void g2d_userptr_put_dma_addr(struct drm_device *drm_dev,
 {
 	struct g2d_cmdlist_userptr *g2d_userptr =
 					(struct g2d_cmdlist_userptr *)obj;
+	struct sg_table *sgt;
 	struct page **pages;
 
 	if (!obj)
@@ -382,8 +383,8 @@ static void g2d_userptr_put_dma_addr(struct drm_device *drm_dev,
 		return;
 
 out:
-	exynos_gem_unmap_sgt_from_dma(drm_dev, g2d_userptr->sgt,
-					DMA_BIDIRECTIONAL);
+	sgt = g2d_userptr->sgt;
+	dma_unmap_sg(drm_dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
 
 	pages = frame_vector_pages(g2d_userptr->vec);
 	if (!IS_ERR(pages)) {
@@ -500,9 +501,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev,
 
 	g2d_userptr->sgt = sgt;
 
-	ret = exynos_gem_map_sgt_with_dma(drm_dev, g2d_userptr->sgt,
-						DMA_BIDIRECTIONAL);
-	if (ret < 0) {
+	if (!dma_map_sg(drm_dev->dev, sgt->sgl, sgt->nents,
+				DMA_BIDIRECTIONAL)) {
 		DRM_ERROR("failed to map sgt with dma region.\n");
 		goto err_sg_free_table;
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 29f48756e72f..17a52f89a690 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -373,32 +373,6 @@ int exynos_drm_gem_get_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
-int exynos_gem_map_sgt_with_dma(struct drm_device *drm_dev,
-				struct sg_table *sgt,
-				enum dma_data_direction dir)
-{
-	int nents;
-
-	mutex_lock(&drm_dev->struct_mutex);
-
-	nents = dma_map_sg(drm_dev->dev, sgt->sgl, sgt->nents, dir);
-	if (!nents) {
-		DRM_ERROR("failed to map sgl with dma.\n");
-		mutex_unlock(&drm_dev->struct_mutex);
-		return nents;
-	}
-
-	mutex_unlock(&drm_dev->struct_mutex);
-	return 0;
-}
-
-void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
-				struct sg_table *sgt,
-				enum dma_data_direction dir)
-{
-	dma_unmap_sg(drm_dev->dev, sgt->sgl, sgt->nents, dir);
-}
-
 void exynos_drm_gem_free_object(struct drm_gem_object *obj)
 {
 	exynos_drm_gem_destroy(to_exynos_gem(obj));
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 00223052b87b..c1df26877b76 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -121,16 +121,6 @@ int exynos_drm_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
 /* set vm_flags and we can change the vm attribute to other one at here. */
 int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
-/* map sgt with dma region. */
-int exynos_gem_map_sgt_with_dma(struct drm_device *drm_dev,
-				struct sg_table *sgt,
-				enum dma_data_direction dir);
-
-/* unmap sgt from dma region. */
-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 *
-- 
1.9.1

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

[PATCH 3/9] drm/exynos: remove using non-consistent DMA attribute

[Joonyoung Shim]

DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so
it doesn't give any effects to use non-consistent DMA attribute.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 17a52f89a690..d5951f75c774 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -41,15 +41,10 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 	if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
 		dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &exynos_gem->dma_attrs);
 
-	/*
-	 * if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping
-	 * else cachable mapping.
-	 */
+	/* if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping */
 	if (exynos_gem->flags & EXYNOS_BO_WC ||
 			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
 		attr = DMA_ATTR_WRITE_COMBINE;
-	else
-		attr = DMA_ATTR_NON_CONSISTENT;
 
 	dma_set_attr(attr, &exynos_gem->dma_attrs);
 	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
-- 
1.9.1

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

[PATCH 4/9] drm/exynos: split buffer allocation using DMA mapping API on non-iommu

[Joonyoung Shim]

This introduces new functions to allocate/free buffer using DMA mapping
API. Now already exynos-drm uses DMA mapping API to allocate/free buffer
but it is used on both iommu and non-iommu, so split it. It will be
added new buffer allocation not to use DMA mapping API later.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index d5951f75c774..88196edd4ade 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,6 +20,63 @@
 #include "exynos_drm_gem.h"
 #include "exynos_drm_iommu.h"
 
+static int exynos_drm_alloc_dma(struct exynos_drm_gem *exynos_gem)
+{
+	struct drm_device *dev = exynos_gem->base.dev;
+	unsigned int nr_pages;
+	unsigned int i;
+	dma_addr_t addr;
+
+	init_dma_attrs(&exynos_gem->dma_attrs);
+
+	if (exynos_gem->flags & EXYNOS_BO_WC ||
+			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
+		dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
+
+	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
+
+	nr_pages = exynos_gem->size >> PAGE_SHIFT;
+
+	exynos_gem->pages = drm_calloc_large(nr_pages, sizeof(struct page *));
+	if (!exynos_gem->pages) {
+		DRM_ERROR("failed to allocate pages\n");
+		return -ENOMEM;
+	}
+
+	exynos_gem->cookie = dma_alloc_attrs(dev->dev, exynos_gem->size,
+					     &exynos_gem->dma_addr, GFP_KERNEL,
+					     &exynos_gem->dma_attrs);
+	if (!exynos_gem->cookie) {
+		DRM_ERROR("failed to allocate buffer\n");
+		drm_free_large(exynos_gem->pages);
+		return -ENOMEM;
+	}
+
+	addr = exynos_gem->dma_addr;
+	for (i = 0; i < nr_pages; i++) {
+		exynos_gem->pages[i] =
+			pfn_to_page(dma_to_pfn(dev->dev, addr));
+		addr += PAGE_SIZE;
+	}
+
+	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
+			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
+
+	return 0;
+}
+
+static void exynos_drm_free_dma(struct exynos_drm_gem *exynos_gem)
+{
+	struct drm_device *dev = exynos_gem->base.dev;
+
+	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
+			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
+
+	dma_free_attrs(dev->dev, exynos_gem->size, exynos_gem->cookie,
+			exynos_gem->dma_addr, &exynos_gem->dma_attrs);
+	drm_free_large(exynos_gem->pages);
+}
+
 static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 {
 	struct drm_device *dev = exynos_gem->base.dev;
@@ -31,6 +88,9 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 		return 0;
 	}
 
+	if (!is_drm_iommu_supported(dev))
+		return exynos_drm_alloc_dma(exynos_gem);
+
 	init_dma_attrs(&exynos_gem->dma_attrs);
 
 	/*
@@ -51,15 +111,6 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 
 	nr_pages = exynos_gem->size >> PAGE_SHIFT;
 
-	if (!is_drm_iommu_supported(dev)) {
-		exynos_gem->pages = drm_calloc_large(nr_pages,
-						     sizeof(struct page *));
-		if (!exynos_gem->pages) {
-			DRM_ERROR("failed to allocate pages.\n");
-			return -ENOMEM;
-		}
-	}
-
 	exynos_gem->cookie = dma_alloc_attrs(dev->dev, exynos_gem->size,
 					     &exynos_gem->dma_addr, GFP_KERNEL,
 					     &exynos_gem->dma_attrs);
@@ -70,20 +121,7 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 		return -ENOMEM;
 	}
 
-	if (exynos_gem->pages) {
-		dma_addr_t start_addr;
-		unsigned int i = 0;
-
-		start_addr = exynos_gem->dma_addr;
-		while (i < nr_pages) {
-			exynos_gem->pages[i] =
-				pfn_to_page(dma_to_pfn(dev->dev, start_addr));
-			start_addr += PAGE_SIZE;
-			i++;
-		}
-	} else {
-		exynos_gem->pages = exynos_gem->cookie;
-	}
+	exynos_gem->pages = exynos_gem->cookie;
 
 	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
 			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
@@ -100,15 +138,15 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
 		return;
 	}
 
+	if (!is_drm_iommu_supported(dev))
+		return exynos_drm_free_dma(exynos_gem);
+
 	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
 			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
 
 	dma_free_attrs(dev->dev, exynos_gem->size, exynos_gem->cookie,
 			(dma_addr_t)exynos_gem->dma_addr,
 			&exynos_gem->dma_attrs);
-
-	if (!is_drm_iommu_supported(dev))
-		drm_free_large(exynos_gem->pages);
 }
 
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
-- 
1.9.1

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

[PATCH 5/9] drm/exynos: introduce buffer allocation using drm_gem_get/put_pages()

[Joonyoung Shim]

This introduces new functions to allocate/free buffer using
drm_gem_get/put_pages() instead of DMA mapping API. They also use sg
list to manage pages.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 48 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_gem.h |  2 +-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 88196edd4ade..d982d46b04da 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -20,6 +20,54 @@
 #include "exynos_drm_gem.h"
 #include "exynos_drm_iommu.h"
 
+static int exynos_drm_get_pages(struct exynos_drm_gem *exynos_gem)
+{
+	struct drm_device *dev = exynos_gem->base.dev;
+	struct page **pages;
+	struct sg_table *sgt;
+	int ret;
+
+	pages = drm_gem_get_pages(&exynos_gem->base);
+	if (IS_ERR(pages))
+		return PTR_ERR(pages);
+
+	sgt = drm_prime_pages_to_sg(pages, exynos_gem->size >> PAGE_SHIFT);
+	if (IS_ERR(sgt)) {
+		ret = PTR_ERR(sgt);
+		goto err;
+	}
+
+	if (!dma_map_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL)) {
+		sg_free_table(sgt);
+		kfree(sgt);
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
+	exynos_gem->sgt = sgt;
+	exynos_gem->pages = pages;
+
+	return 0;
+
+err:
+	drm_gem_put_pages(&exynos_gem->base, pages, false, false);
+	return ret;
+}
+
+static void exynos_drm_put_pages(struct exynos_drm_gem *exynos_gem)
+{
+	struct drm_device *dev = exynos_gem->base.dev;
+	struct sg_table *sgt = exynos_gem->sgt;
+
+	dma_unmap_sg(dev->dev, sgt->sgl, sgt->nents, DMA_BIDIRECTIONAL);
+
+	sg_free_table(sgt);
+	kfree(sgt);
+
+	drm_gem_put_pages(&exynos_gem->base, exynos_gem->pages, false, false);
+}
+
 static int exynos_drm_alloc_dma(struct exynos_drm_gem *exynos_gem)
 {
 	struct drm_device *dev = exynos_gem->base.dev;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index c1df26877b76..f47daec776e6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -38,7 +38,7 @@
  *	- this address could be physical address without IOMMU and
  *	device address with IOMMU.
  * @pages: Array of backing pages.
- * @sgt: Imported sg_table.
+ * @sgt: Converted sg_table of pages or imported sg_table.
  *
  * P.S. this object would be transferred to user as kms_bo.handle so
  *	user can access the buffer through kms_bo.handle.
-- 
1.9.1

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

[PATCH 6/9] drm/exynos: switch to new buffer allocation

[Joonyoung Shim]

The buffer allocation using DMA mapping API can't support non-continuous
buffer on non-iommu and cachable buffer, so switch to new buffer
allocation using drm_gem_get/put_pages() and doesn't use DMA mapping API
for mmap except allocation of physically continuous buffer on non-iommu.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index d982d46b04da..163d113df1ab 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -77,10 +77,7 @@ static int exynos_drm_alloc_dma(struct exynos_drm_gem *exynos_gem)
 
 	init_dma_attrs(&exynos_gem->dma_attrs);
 
-	if (exynos_gem->flags & EXYNOS_BO_WC ||
-			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
-		dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
-
+	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
 	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
 
 	nr_pages = exynos_gem->size >> PAGE_SHIFT;
@@ -128,51 +125,21 @@ static void exynos_drm_free_dma(struct exynos_drm_gem *exynos_gem)
 static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 {
 	struct drm_device *dev = exynos_gem->base.dev;
-	enum dma_attr attr;
-	unsigned int nr_pages;
+	int ret;
 
 	if (exynos_gem->dma_addr) {
 		DRM_DEBUG_KMS("already allocated.\n");
 		return 0;
 	}
 
-	if (!is_drm_iommu_supported(dev))
-		return exynos_drm_alloc_dma(exynos_gem);
-
-	init_dma_attrs(&exynos_gem->dma_attrs);
-
-	/*
-	 * if EXYNOS_BO_CONTIG, fully physically contiguous memory
-	 * region will be allocated else physically contiguous
-	 * as possible.
-	 */
-	if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
-		dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &exynos_gem->dma_attrs);
-
-	/* if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping */
-	if (exynos_gem->flags & EXYNOS_BO_WC ||
-			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
-		attr = DMA_ATTR_WRITE_COMBINE;
-
-	dma_set_attr(attr, &exynos_gem->dma_attrs);
-	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
-
-	nr_pages = exynos_gem->size >> PAGE_SHIFT;
-
-	exynos_gem->cookie = dma_alloc_attrs(dev->dev, exynos_gem->size,
-					     &exynos_gem->dma_addr, GFP_KERNEL,
-					     &exynos_gem->dma_attrs);
-	if (!exynos_gem->cookie) {
-		DRM_ERROR("failed to allocate buffer.\n");
-		if (exynos_gem->pages)
-			drm_free_large(exynos_gem->pages);
-		return -ENOMEM;
+	if (!is_drm_iommu_supported(dev)) {
+		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
+			return exynos_drm_alloc_dma(exynos_gem);
 	}
 
-	exynos_gem->pages = exynos_gem->cookie;
-
-	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
-			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
+	ret = exynos_drm_get_pages(exynos_gem);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }
@@ -186,15 +153,12 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
 		return;
 	}
 
-	if (!is_drm_iommu_supported(dev))
-		return exynos_drm_free_dma(exynos_gem);
-
-	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
-			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
+	if (!is_drm_iommu_supported(dev)) {
+		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
+			return exynos_drm_free_dma(exynos_gem);
+	}
 
-	dma_free_attrs(dev->dev, exynos_gem->size, exynos_gem->cookie,
-			(dma_addr_t)exynos_gem->dma_addr,
-			&exynos_gem->dma_attrs);
+	exynos_drm_put_pages(exynos_gem);
 }
 
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
@@ -400,8 +364,8 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
 	drm_gem_object_unreference_unlocked(obj);
 }
 
-static int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem *exynos_gem,
-				      struct vm_area_struct *vma)
+static int exynos_drm_gem_mmap_dma(struct exynos_drm_gem *exynos_gem,
+				   struct vm_area_struct *vma)
 {
 	struct drm_device *drm_dev = exynos_gem->base.dev;
 	unsigned long vm_size;
@@ -579,6 +543,19 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 
 	DRM_DEBUG_KMS("flags = 0x%x\n", exynos_gem->flags);
 
+	if (!is_drm_iommu_supported(obj->dev)) {
+		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG)) {
+			ret = exynos_drm_gem_mmap_dma(exynos_gem, vma);
+			if (ret < 0)
+				drm_gem_vm_close(vma);
+
+			return ret;
+		}
+	}
+
+	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_flags |= VM_MIXEDMAP;
+
 	/* non-cachable as default. */
 	if (exynos_gem->flags & EXYNOS_BO_CACHABLE)
 		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
@@ -589,16 +566,7 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 		vma->vm_page_prot =
 			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
 
-	ret = exynos_drm_gem_mmap_buffer(exynos_gem, vma);
-	if (ret)
-		goto err_close_vm;
-
-	return ret;
-
-err_close_vm:
-	drm_gem_vm_close(vma);
-
-	return ret;
+	return 0;
 }
 
 /* low-level interface prime helpers */
-- 
1.9.1

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

Re: [PATCH 6/9] drm/exynos: switch to new buffer allocation

[Inki Dae]

Hi,

How about combining patch 5 and 6?

Patch 5 just introduces new internal API but these API aren't used 
anywhere in patch 5.

Thanks,
Inki Dae

2015년 10월 13일 16:00에 Joonyoung Shim 이(가) 쓴 글:
> The buffer allocation using DMA mapping API can't support non-continuous
> buffer on non-iommu and cachable buffer, so switch to new buffer
> allocation using drm_gem_get/put_pages() and doesn't use DMA mapping API
> for mmap except allocation of physically continuous buffer on non-iommu.
>
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>   drivers/gpu/drm/exynos/exynos_drm_gem.c | 90 +++++++++++----------------------
>   1 file changed, 29 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index d982d46b04da..163d113df1ab 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -77,10 +77,7 @@ static int exynos_drm_alloc_dma(struct exynos_drm_gem *exynos_gem)
>
>   	init_dma_attrs(&exynos_gem->dma_attrs);
>
> -	if (exynos_gem->flags & EXYNOS_BO_WC ||
> -			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
> -		dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
> -
> +	dma_set_attr(DMA_ATTR_WRITE_COMBINE, &exynos_gem->dma_attrs);
>   	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
>
>   	nr_pages = exynos_gem->size >> PAGE_SHIFT;
> @@ -128,51 +125,21 @@ static void exynos_drm_free_dma(struct exynos_drm_gem *exynos_gem)
>   static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
>   {
>   	struct drm_device *dev = exynos_gem->base.dev;
> -	enum dma_attr attr;
> -	unsigned int nr_pages;
> +	int ret;
>
>   	if (exynos_gem->dma_addr) {
>   		DRM_DEBUG_KMS("already allocated.\n");
>   		return 0;
>   	}
>
> -	if (!is_drm_iommu_supported(dev))
> -		return exynos_drm_alloc_dma(exynos_gem);
> -
> -	init_dma_attrs(&exynos_gem->dma_attrs);
> -
> -	/*
> -	 * if EXYNOS_BO_CONTIG, fully physically contiguous memory
> -	 * region will be allocated else physically contiguous
> -	 * as possible.
> -	 */
> -	if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> -		dma_set_attr(DMA_ATTR_FORCE_CONTIGUOUS, &exynos_gem->dma_attrs);
> -
> -	/* if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping */
> -	if (exynos_gem->flags & EXYNOS_BO_WC ||
> -			!(exynos_gem->flags & EXYNOS_BO_CACHABLE))
> -		attr = DMA_ATTR_WRITE_COMBINE;
> -
> -	dma_set_attr(attr, &exynos_gem->dma_attrs);
> -	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &exynos_gem->dma_attrs);
> -
> -	nr_pages = exynos_gem->size >> PAGE_SHIFT;
> -
> -	exynos_gem->cookie = dma_alloc_attrs(dev->dev, exynos_gem->size,
> -					     &exynos_gem->dma_addr, GFP_KERNEL,
> -					     &exynos_gem->dma_attrs);
> -	if (!exynos_gem->cookie) {
> -		DRM_ERROR("failed to allocate buffer.\n");
> -		if (exynos_gem->pages)
> -			drm_free_large(exynos_gem->pages);
> -		return -ENOMEM;
> +	if (!is_drm_iommu_supported(dev)) {
> +		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> +			return exynos_drm_alloc_dma(exynos_gem);
>   	}
>
> -	exynos_gem->pages = exynos_gem->cookie;
> -
> -	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
> -			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
> +	ret = exynos_drm_get_pages(exynos_gem);
> +	if (ret < 0)
> +		return ret;
>
>   	return 0;
>   }
> @@ -186,15 +153,12 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
>   		return;
>   	}
>
> -	if (!is_drm_iommu_supported(dev))
> -		return exynos_drm_free_dma(exynos_gem);
> -
> -	DRM_DEBUG_KMS("dma_addr(0x%lx), size(0x%lx)\n",
> -			(unsigned long)exynos_gem->dma_addr, exynos_gem->size);
> +	if (!is_drm_iommu_supported(dev)) {
> +		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
> +			return exynos_drm_free_dma(exynos_gem);
> +	}
>
> -	dma_free_attrs(dev->dev, exynos_gem->size, exynos_gem->cookie,
> -			(dma_addr_t)exynos_gem->dma_addr,
> -			&exynos_gem->dma_attrs);
> +	exynos_drm_put_pages(exynos_gem);
>   }
>
>   static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
> @@ -400,8 +364,8 @@ void exynos_drm_gem_put_dma_addr(struct drm_device *dev,
>   	drm_gem_object_unreference_unlocked(obj);
>   }
>
> -static int exynos_drm_gem_mmap_buffer(struct exynos_drm_gem *exynos_gem,
> -				      struct vm_area_struct *vma)
> +static int exynos_drm_gem_mmap_dma(struct exynos_drm_gem *exynos_gem,
> +				   struct vm_area_struct *vma)
>   {
>   	struct drm_device *drm_dev = exynos_gem->base.dev;
>   	unsigned long vm_size;
> @@ -579,6 +543,19 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>
>   	DRM_DEBUG_KMS("flags = 0x%x\n", exynos_gem->flags);
>
> +	if (!is_drm_iommu_supported(obj->dev)) {
> +		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG)) {
> +			ret = exynos_drm_gem_mmap_dma(exynos_gem, vma);
> +			if (ret < 0)
> +				drm_gem_vm_close(vma);
> +
> +			return ret;
> +		}
> +	}
> +
> +	vma->vm_flags &= ~VM_PFNMAP;
> +	vma->vm_flags |= VM_MIXEDMAP;
> +
>   	/* non-cachable as default. */
>   	if (exynos_gem->flags & EXYNOS_BO_CACHABLE)
>   		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> @@ -589,16 +566,7 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>   		vma->vm_page_prot =
>   			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
>
> -	ret = exynos_drm_gem_mmap_buffer(exynos_gem, vma);
> -	if (ret)
> -		goto err_close_vm;
> -
> -	return ret;
> -
> -err_close_vm:
> -	drm_gem_vm_close(vma);
> -
> -	return ret;
> +	return 0;
>   }
>
>   /* low-level interface prime helpers */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/9] drm/exynos: switch to new buffer allocation

[Joonyoung Shim]

On 10/19/2015 09:20 PM, Inki Dae wrote:
> Hi,
> 
> How about combining patch 5 and 6?
> 
> Patch 5 just introduces new internal API but these API aren't used anywhere in patch 5.
> 

I split it to be easy to understand changes of codes on patch file. It's
no matter to me to combine them.

Anyway, because this patchset introduces new userspace interfaces and
there is no real userspace to use them yet, i'm not sure it's better
whether i keep going this patchset or without the patch to introduce new
userspace interfaces.

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

[PATCH 7/9] drm/exynos: always use EXYNOS_BO_CONTIG flag on iommu

[Joonyoung Shim]

It doesn't care whether memory is continuous physically if iommu is
supported but we will always use EXYNOS_BO_CONTIG flag on iommu, so it
can mean that the memory is continuous memory for device.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fb.c    | 32 ++++---------------------------
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 14 +-------------
 drivers/gpu/drm/exynos/exynos_drm_gem.c   | 31 ++++++++++++++----------------
 drivers/gpu/drm/exynos/exynos_drm_gem.h   |  2 --
 include/uapi/drm/exynos_drm.h             |  5 ++++-
 5 files changed, 23 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index fcea28bdbc42..8fbae903c7ed 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -39,32 +39,6 @@ struct exynos_drm_fb {
 	struct exynos_drm_gem	*exynos_gem[MAX_FB_BUFFER];
 };
 
-static int check_fb_gem_memory_type(struct drm_device *drm_dev,
-				    struct exynos_drm_gem *exynos_gem)
-{
-	unsigned int flags;
-
-	/*
-	 * if exynos drm driver supports iommu then framebuffer can use
-	 * all the buffer types.
-	 */
-	if (is_drm_iommu_supported(drm_dev))
-		return 0;
-
-	flags = exynos_gem->flags;
-
-	/*
-	 * without iommu support, not support physically non-continuous memory
-	 * for framebuffer.
-	 */
-	if (IS_NONCONTIG_BUFFER(flags)) {
-		DRM_ERROR("cannot use this gem memory type for fb.\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static void exynos_drm_fb_destroy(struct drm_framebuffer *fb)
 {
 	struct exynos_drm_fb *exynos_fb = to_exynos_fb(fb);
@@ -130,9 +104,11 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	for (i = 0; i < count; i++) {
-		ret = check_fb_gem_memory_type(dev, exynos_gem[i]);
-		if (ret < 0)
+		/* Not support physically non-continuous memory for FB */
+		if (exynos_gem[i]->flags & EXYNOS_BO_NONCONTIG) {
+			ret = -EINVAL;
 			goto err;
+		}
 
 		exynos_fb->exynos_gem[i] = exynos_gem[i];
 	}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index f6118baa8e3e..fcbe43a580c1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -124,7 +124,6 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	struct exynos_drm_gem *exynos_gem;
 	struct drm_device *dev = helper->dev;
 	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
-	struct platform_device *pdev = dev->platformdev;
 	unsigned long size;
 	int ret;
 
@@ -142,18 +141,7 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 
-	exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);
-	/*
-	 * If physically contiguous memory allocation fails and if IOMMU is
-	 * supported then try to get buffer from non physically contiguous
-	 * memory area.
-	 */
-	if (IS_ERR(exynos_gem) && is_drm_iommu_supported(dev)) {
-		dev_warn(&pdev->dev, "contiguous FB allocation failed, falling back to non-contiguous\n");
-		exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG,
-						   size);
-	}
-
+	exynos_gem = exynos_drm_gem_create(dev, 0, size);
 	if (IS_ERR(exynos_gem)) {
 		ret = PTR_ERR(exynos_gem);
 		goto out;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 163d113df1ab..96a69468283b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -135,6 +135,8 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 	if (!is_drm_iommu_supported(dev)) {
 		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
 			return exynos_drm_alloc_dma(exynos_gem);
+	} else {
+		exynos_gem->flags &= ~EXYNOS_BO_NONCONTIG;
 	}
 
 	ret = exynos_drm_get_pages(exynos_gem);
@@ -440,10 +442,7 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
 	args->pitch = args->width * ((args->bpp + 7) / 8);
 	args->size = args->pitch * args->height;
 
-	if (is_drm_iommu_supported(dev))
-		flags = EXYNOS_BO_NONCONTIG | EXYNOS_BO_WC;
-	else
-		flags = EXYNOS_BO_CONTIG | EXYNOS_BO_WC;
+	flags = EXYNOS_BO_WC;
 
 	exynos_gem = exynos_drm_gem_create(dev, flags, args->size);
 	if (IS_ERR(exynos_gem)) {
@@ -597,6 +596,17 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
 
 	exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
 
+	/*
+	 * Always physically continuous memory if sgt->nents is 1. It
+	 * doesn't care if IOMMU is supported but EXYNOS_BO_NONCONTIG
+	 * flag will be cleared. It will mean the memory is continuous
+	 * for device. EXYNOS_BO_NONCONTIG flag will be set if not both.
+	 */
+	if (sgt->nents == 1 || is_drm_iommu_supported(dev))
+		exynos_gem->flags &= ~EXYNOS_BO_NONCONTIG;
+	else
+		exynos_gem->flags |= EXYNOS_BO_NONCONTIG;
+
 	npages = exynos_gem->size >> PAGE_SHIFT;
 	exynos_gem->pages = drm_malloc_ab(npages, sizeof(struct page *));
 	if (!exynos_gem->pages) {
@@ -611,19 +621,6 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
 
 	exynos_gem->sgt = sgt;
 
-	if (sgt->nents == 1) {
-		/* always physically continuous memory if sgt->nents is 1. */
-		exynos_gem->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->flags |= EXYNOS_BO_NONCONTIG;
-	}
-
 	return &exynos_gem->base;
 
 err_free_large:
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index f47daec776e6..e9eb5631f322 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -16,8 +16,6 @@
 
 #define to_exynos_gem(x)	container_of(x, struct exynos_drm_gem, base)
 
-#define IS_NONCONTIG_BUFFER(f)		(f & EXYNOS_BO_NONCONTIG)
-
 /*
  * exynos drm buffer structure.
  *
diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
index 18f0601f84d1..1d7c80734e43 100644
--- a/include/uapi/drm/exynos_drm.h
+++ b/include/uapi/drm/exynos_drm.h
@@ -76,7 +76,10 @@ struct drm_exynos_vidi_connection {
 
 /* memory type definitions. */
 enum e_drm_exynos_gem_mem_type {
-	/* Physically Continuous memory and used as default. */
+	/*
+	 * Physically Continuous memory or Continuous memory for device
+	 * on IOMMU. Used as default.
+	 */
 	EXYNOS_BO_CONTIG	= 0 << 0,
 	/* Physically Non-Continuous memory. */
 	EXYNOS_BO_NONCONTIG	= 1 << 0,
-- 
1.9.1

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

[PATCH 8/9] drm/exynos: use DMA_ERROR_CODE

[Joonyoung Shim]

The dma_addr of gem will be DMA_ERROR_CODE if gem is created and
will keep DMA_ERROR_CODE if gem has EXYNOS_BO_NONCONTIG flag on
non-iommu.

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

diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 96a69468283b..01c5e0854016 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -44,7 +44,9 @@ static int exynos_drm_get_pages(struct exynos_drm_gem *exynos_gem)
 		goto err;
 	}
 
-	exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
+	if (is_drm_iommu_supported(dev))
+		exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
+
 	exynos_gem->sgt = sgt;
 	exynos_gem->pages = pages;
 
@@ -127,11 +129,6 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
 	struct drm_device *dev = exynos_gem->base.dev;
 	int ret;
 
-	if (exynos_gem->dma_addr) {
-		DRM_DEBUG_KMS("already allocated.\n");
-		return 0;
-	}
-
 	if (!is_drm_iommu_supported(dev)) {
 		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
 			return exynos_drm_alloc_dma(exynos_gem);
@@ -150,11 +147,6 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
 {
 	struct drm_device *dev = exynos_gem->base.dev;
 
-	if (!exynos_gem->dma_addr) {
-		DRM_DEBUG_KMS("dma_addr is invalid.\n");
-		return;
-	}
-
 	if (!is_drm_iommu_supported(dev)) {
 		if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
 			return exynos_drm_free_dma(exynos_gem);
@@ -256,6 +248,8 @@ static struct exynos_drm_gem *exynos_drm_gem_init(struct drm_device *dev,
 		return ERR_PTR(ret);
 	}
 
+	exynos_gem->dma_addr = DMA_ERROR_CODE;
+
 	DRM_DEBUG_KMS("created file object = 0x%x\n", (unsigned int)obj->filp);
 
 	return exynos_gem;
@@ -594,18 +588,18 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
 		return ERR_PTR(ret);
 	}
 
-	exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
-
 	/*
 	 * Always physically continuous memory if sgt->nents is 1. It
 	 * doesn't care if IOMMU is supported but EXYNOS_BO_NONCONTIG
 	 * flag will be cleared. It will mean the memory is continuous
 	 * for device. EXYNOS_BO_NONCONTIG flag will be set if not both.
 	 */
-	if (sgt->nents == 1 || is_drm_iommu_supported(dev))
+	if (sgt->nents == 1 || is_drm_iommu_supported(dev)) {
 		exynos_gem->flags &= ~EXYNOS_BO_NONCONTIG;
-	else
+		exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
+	} else {
 		exynos_gem->flags |= EXYNOS_BO_NONCONTIG;
+	}
 
 	npages = exynos_gem->size >> PAGE_SHIFT;
 	exynos_gem->pages = drm_malloc_ab(npages, sizeof(struct page *));
-- 
1.9.1

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

[PATCH 9/9] drm/exynos: add ioctls for cpu access of gem object from user

[Joonyoung Shim]

This adds necessary two ioctls for cpu access of gem object from user.
It needs to be synced properly in order for the cpu and device if the
buffer of gem object is cachable.

- DRM_IOCTL_EXYNOS_GEM_CPU_PREP
Should be used explicitly before it will be cpu access of gem object
from user.

- DRM_IOCTL_EXYNOS_GEM_CPU_FINI
Should be used explicitly after it is finished cpu access of gem object
from user.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
This is based on patch of Daniel dropping DRM_UNLOCKED flag on each
drivers[1] and patch that is droped DRM_UNLOCKED flag from my posted
patch[2].

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-September/075368.html
[2] http://www.spinics.net/lists/dri-devel/msg91465.html

 drivers/gpu/drm/exynos/exynos_drm_drv.c |  4 +++
 drivers/gpu/drm/exynos/exynos_drm_gem.c | 48 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_gem.h | 14 ++++++++++
 include/uapi/drm/exynos_drm.h           | 18 ++++++++++++-
 4 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 74bf3afed519..f15945c1c01d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -408,6 +408,10 @@ static const struct drm_ioctl_desc exynos_ioctls[] = {
 			DRM_AUTH | DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MAP, exynos_drm_gem_map_ioctl,
 			DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CPU_PREP, exynos_drm_gem_cpu_prep_ioctl,
+			DRM_AUTH | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CPU_FINI, exynos_drm_gem_cpu_fini_ioctl,
+			DRM_AUTH | DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET, exynos_drm_gem_get_ioctl,
 			DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION, vidi_connection_ioctl,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 01c5e0854016..4907ad3994db 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -321,6 +321,54 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void *data,
 					      &args->offset);
 }
 
+int exynos_drm_gem_cpu_prep_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file)
+{
+	struct drm_exynos_gem_cpu_access *args = data;
+	struct drm_gem_object *obj;
+	struct exynos_drm_gem *exynos_gem;
+
+	obj = drm_gem_object_lookup(dev, file, args->handle);
+	if (!obj) {
+		DRM_ERROR("Failed to lookup gem object\n");
+		return -EINVAL;
+	}
+
+	exynos_gem = to_exynos_gem(obj);
+
+	if (exynos_gem->flags & EXYNOS_BO_CACHABLE)
+		dma_sync_sg_for_cpu(dev->dev, exynos_gem->sgt->sgl,
+				    exynos_gem->sgt->nents, DMA_FROM_DEVICE);
+
+	drm_gem_object_unreference_unlocked(obj);
+
+	return 0;
+}
+
+int exynos_drm_gem_cpu_fini_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file)
+{
+	struct drm_exynos_gem_cpu_access *args = data;
+	struct drm_gem_object *obj;
+	struct exynos_drm_gem *exynos_gem;
+
+	obj = drm_gem_object_lookup(dev, file, args->handle);
+	if (!obj) {
+		DRM_ERROR("Failed to lookup gem object\n");
+		return -EINVAL;
+	}
+
+	exynos_gem = to_exynos_gem(obj);
+
+	if (exynos_gem->flags & EXYNOS_BO_CACHABLE)
+		dma_sync_sg_for_device(dev->dev, exynos_gem->sgt->sgl,
+				       exynos_gem->sgt->nents, DMA_TO_DEVICE);
+
+	drm_gem_object_unreference_unlocked(obj);
+
+	return 0;
+}
+
 dma_addr_t *exynos_drm_gem_get_dma_addr(struct drm_device *dev,
 					unsigned int gem_handle,
 					struct drm_file *filp)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index e9eb5631f322..d5aac38f455a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -74,6 +74,20 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void *data,
 			     struct drm_file *file_priv);
 
 /*
+ * Should be used explicitly before it will be cpu access of gem object
+ * from user.
+ */
+int exynos_drm_gem_cpu_prep_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv);
+
+/*
+ * Should be used explicitly after it is finished cpu access of gem
+ * object from user.
+ */
+int exynos_drm_gem_cpu_fini_ioctl(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv);
+
+/*
  * get dma address from gem handle and this function could be used for
  * other drivers such as 2d/3d acceleration drivers.
  * with this function call, gem object reference count would be increased.
diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h
index 1d7c80734e43..b16a9a9ea34e 100644
--- a/include/uapi/drm/exynos_drm.h
+++ b/include/uapi/drm/exynos_drm.h
@@ -46,6 +46,16 @@ struct drm_exynos_gem_map {
 };
 
 /**
+ * A structure for cpu access of gem object from user.
+ *
+ * @handle: handle of gem object.
+ */
+struct drm_exynos_gem_cpu_access {
+	__u32 handle;
+	__u32 reserved;
+};
+
+/**
  * A structure to gem information.
  *
  * @handle: a handle to gem object created.
@@ -301,8 +311,10 @@ struct drm_exynos_ipp_cmd_ctrl {
 
 #define DRM_EXYNOS_GEM_CREATE		0x00
 #define DRM_EXYNOS_GEM_MAP		0x01
-/* Reserved 0x03 ~ 0x05 for exynos specific gem ioctl */
+#define DRM_EXYNOS_GEM_CPU_PREP		0x02
+#define DRM_EXYNOS_GEM_CPU_FINI		0x03
 #define DRM_EXYNOS_GEM_GET		0x04
+/* Reserved 0x05 ~ 0x06 for exynos specific gem ioctl */
 #define DRM_EXYNOS_VIDI_CONNECTION	0x07
 
 /* G2D */
@@ -320,6 +332,10 @@ struct drm_exynos_ipp_cmd_ctrl {
 		DRM_EXYNOS_GEM_CREATE, struct drm_exynos_gem_create)
 #define DRM_IOCTL_EXYNOS_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + \
 		DRM_EXYNOS_GEM_MAP, struct drm_exynos_gem_map)
+#define DRM_IOCTL_EXYNOS_GEM_CPU_PREP		DRM_IOWR(DRM_COMMAND_BASE + \
+		DRM_EXYNOS_GEM_CPU_PREP, struct drm_exynos_gem_cpu_access)
+#define DRM_IOCTL_EXYNOS_GEM_CPU_FINI		DRM_IOWR(DRM_COMMAND_BASE + \
+		DRM_EXYNOS_GEM_CPU_FINI, struct drm_exynos_gem_cpu_access)
 #define DRM_IOCTL_EXYNOS_GEM_GET	DRM_IOWR(DRM_COMMAND_BASE + \
 		DRM_EXYNOS_GEM_GET,	struct drm_exynos_gem_info)
 
-- 
1.9.1

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

Re: [PATCH 0/9] drm/exynos: update codes related with gem

[Daniel Vetter]

On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
> Hi,
> 
> This patchset is about gem codes update of exynos-drm.
> 
> The first and second patches are cleanup to remove useless codes.
> The rest is to support cachable gem allocation.
> 
> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
> doesn't give any effects.
> 
> This patchset introduces new buffer allocation to use
> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
> for the rest except allocation of physically continuous buffer on
> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
> can support physically non-continuous buffer on non-iommu.
> 
> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
> whether buffer is continuous physically if iommu is supported. This
> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
> mean that the buffer is continuous for device.
> 
> There is no user API to control cache coherence for the cpu and device
> about cachable buffer. This patchset introduces two ioctls for cpu
> access of gem object from user. It will be synced properly in order for
> the cpu and device if buffer of gem object is cachable.

Out of curiosity, where's the userspace part for this work? Usually kernel
abi extensions come with a link to the relevant branch for easier review.
-Daniel

> 
> Thanks.
> 
> Joonyoung Shim (9):
>   drm/exynos: eliminate useless codes of exynos_drm_gem.h
>   drm/exynos: use directly DMA mapping APIs on g2d
>   drm/exynos: remove using non-consistent DMA attribute
>   drm/exynos: split buffer allocation using DMA mapping API on non-iommu
>   drm/exynos: introduce buffer allocation using drm_gem_get/put_pages()
>   drm/exynos: switch to new buffer allocation
>   drm/exynos: always use EXYNOS_BO_CONTIG flag on iommu
>   drm/exynos: use DMA_ERROR_CODE
>   drm/exynos: add ioctls for cpu access of gem object from user
> 
>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |   4 +
>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  32 +---
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  14 +-
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  10 +-
>  drivers/gpu/drm/exynos/exynos_drm_gem.c   | 288 ++++++++++++++++++------------
>  drivers/gpu/drm/exynos/exynos_drm_gem.h   |  56 ++----
>  include/uapi/drm/exynos_drm.h             |  23 ++-
>  7 files changed, 225 insertions(+), 202 deletions(-)
> 
> -- 
> 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 0/9] drm/exynos: update codes related with gem

[Joonyoung Shim]

On 10/13/2015 05:27 PM, Daniel Vetter wrote:
> On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
>> Hi,
>>
>> This patchset is about gem codes update of exynos-drm.
>>
>> The first and second patches are cleanup to remove useless codes.
>> The rest is to support cachable gem allocation.
>>
>> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
>> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
>> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
>> doesn't give any effects.
>>
>> This patchset introduces new buffer allocation to use
>> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
>> for the rest except allocation of physically continuous buffer on
>> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
>> can support physically non-continuous buffer on non-iommu.
>>
>> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
>> whether buffer is continuous physically if iommu is supported. This
>> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
>> mean that the buffer is continuous for device.
>>
>> There is no user API to control cache coherence for the cpu and device
>> about cachable buffer. This patchset introduces two ioctls for cpu
>> access of gem object from user. It will be synced properly in order for
>> the cpu and device if buffer of gem object is cachable.
> 
> Out of curiosity, where's the userspace part for this work? Usually kernel
> abi extensions come with a link to the relevant branch for easier review.
> -Daniel
> 

Right, thanks for point.

I just modified a little bit exynos_fimg2d_test and exynos parts of
libdrm to test them as workaround. I will expose it.

Thanks.

>>
>> Thanks.
>>
>> Joonyoung Shim (9):
>>   drm/exynos: eliminate useless codes of exynos_drm_gem.h
>>   drm/exynos: use directly DMA mapping APIs on g2d
>>   drm/exynos: remove using non-consistent DMA attribute
>>   drm/exynos: split buffer allocation using DMA mapping API on non-iommu
>>   drm/exynos: introduce buffer allocation using drm_gem_get/put_pages()
>>   drm/exynos: switch to new buffer allocation
>>   drm/exynos: always use EXYNOS_BO_CONTIG flag on iommu
>>   drm/exynos: use DMA_ERROR_CODE
>>   drm/exynos: add ioctls for cpu access of gem object from user
>>
>>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |   4 +
>>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  32 +---
>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  14 +-
>>  drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  10 +-
>>  drivers/gpu/drm/exynos/exynos_drm_gem.c   | 288 ++++++++++++++++++------------
>>  drivers/gpu/drm/exynos/exynos_drm_gem.h   |  56 ++----
>>  include/uapi/drm/exynos_drm.h             |  23 ++-
>>  7 files changed, 225 insertions(+), 202 deletions(-)
>>
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

Re: [PATCH 0/9] drm/exynos: update codes related with gem

[Joonyoung Shim]

On 10/13/2015 05:37 PM, Joonyoung Shim wrote:
> On 10/13/2015 05:27 PM, Daniel Vetter wrote:
>> On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
>>> Hi,
>>>
>>> This patchset is about gem codes update of exynos-drm.
>>>
>>> The first and second patches are cleanup to remove useless codes.
>>> The rest is to support cachable gem allocation.
>>>
>>> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
>>> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
>>> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
>>> doesn't give any effects.
>>>
>>> This patchset introduces new buffer allocation to use
>>> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
>>> for the rest except allocation of physically continuous buffer on
>>> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
>>> can support physically non-continuous buffer on non-iommu.
>>>
>>> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
>>> whether buffer is continuous physically if iommu is supported. This
>>> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
>>> mean that the buffer is continuous for device.
>>>
>>> There is no user API to control cache coherence for the cpu and device
>>> about cachable buffer. This patchset introduces two ioctls for cpu
>>> access of gem object from user. It will be synced properly in order for
>>> the cpu and device if buffer of gem object is cachable.
>>
>> Out of curiosity, where's the userspace part for this work? Usually kernel
>> abi extensions come with a link to the relevant branch for easier review.
>> -Daniel
>>
> 
> Right, thanks for point.
> 
> I just modified a little bit exynos_fimg2d_test and exynos parts of
> libdrm to test them as workaround. I will expose it.
> 

Please refer follows.
https://github.com/dofmind/libdrm/commits/exynos

If fimd2d test program uses cachable gem, it will show cache coherency
problem, so it adds to use new ioctls to keep cache coherency.

Thanks.

> Thanks.
> 
>>>
>>> Thanks.
>>>
>>> Joonyoung Shim (9):
>>>   drm/exynos: eliminate useless codes of exynos_drm_gem.h
>>>   drm/exynos: use directly DMA mapping APIs on g2d
>>>   drm/exynos: remove using non-consistent DMA attribute
>>>   drm/exynos: split buffer allocation using DMA mapping API on non-iommu
>>>   drm/exynos: introduce buffer allocation using drm_gem_get/put_pages()
>>>   drm/exynos: switch to new buffer allocation
>>>   drm/exynos: always use EXYNOS_BO_CONTIG flag on iommu
>>>   drm/exynos: use DMA_ERROR_CODE
>>>   drm/exynos: add ioctls for cpu access of gem object from user
>>>
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c   |   4 +
>>>  drivers/gpu/drm/exynos/exynos_drm_fb.c    |  32 +---
>>>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |  14 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_g2d.c   |  10 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c   | 288 ++++++++++++++++++------------
>>>  drivers/gpu/drm/exynos/exynos_drm_gem.h   |  56 ++----
>>>  include/uapi/drm/exynos_drm.h             |  23 ++-
>>>  7 files changed, 225 insertions(+), 202 deletions(-)
>>>
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

Re: [PATCH 0/9] drm/exynos: update codes related with gem

[Daniel Vetter]

On Tue, Oct 13, 2015 at 06:32:53PM +0900, Joonyoung Shim wrote:
> On 10/13/2015 05:37 PM, Joonyoung Shim wrote:
> > On 10/13/2015 05:27 PM, Daniel Vetter wrote:
> >> On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
> >>> Hi,
> >>>
> >>> This patchset is about gem codes update of exynos-drm.
> >>>
> >>> The first and second patches are cleanup to remove useless codes.
> >>> The rest is to support cachable gem allocation.
> >>>
> >>> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
> >>> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
> >>> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
> >>> doesn't give any effects.
> >>>
> >>> This patchset introduces new buffer allocation to use
> >>> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
> >>> for the rest except allocation of physically continuous buffer on
> >>> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
> >>> can support physically non-continuous buffer on non-iommu.
> >>>
> >>> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
> >>> whether buffer is continuous physically if iommu is supported. This
> >>> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
> >>> mean that the buffer is continuous for device.
> >>>
> >>> There is no user API to control cache coherence for the cpu and device
> >>> about cachable buffer. This patchset introduces two ioctls for cpu
> >>> access of gem object from user. It will be synced properly in order for
> >>> the cpu and device if buffer of gem object is cachable.
> >>
> >> Out of curiosity, where's the userspace part for this work? Usually kernel
> >> abi extensions come with a link to the relevant branch for easier review.
> >> -Daniel
> >>
> > 
> > Right, thanks for point.
> > 
> > I just modified a little bit exynos_fimg2d_test and exynos parts of
> > libdrm to test them as workaround. I will expose it.
> > 
> 
> Please refer follows.
> https://github.com/dofmind/libdrm/commits/exynos
> 
> If fimd2d test program uses cachable gem, it will show cache coherency
> problem, so it adds to use new ioctls to keep cache coherency.

Is there some real userspace too, not just a test/demo app?
-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

Re: [PATCH 0/9] drm/exynos: update codes related with gem

[Joonyoung Shim]

On 10/13/2015 09:29 PM, Daniel Vetter wrote:
> On Tue, Oct 13, 2015 at 06:32:53PM +0900, Joonyoung Shim wrote:
>> On 10/13/2015 05:37 PM, Joonyoung Shim wrote:
>>> On 10/13/2015 05:27 PM, Daniel Vetter wrote:
>>>> On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
>>>>> Hi,
>>>>>
>>>>> This patchset is about gem codes update of exynos-drm.
>>>>>
>>>>> The first and second patches are cleanup to remove useless codes.
>>>>> The rest is to support cachable gem allocation.
>>>>>
>>>>> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
>>>>> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
>>>>> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
>>>>> doesn't give any effects.
>>>>>
>>>>> This patchset introduces new buffer allocation to use
>>>>> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
>>>>> for the rest except allocation of physically continuous buffer on
>>>>> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
>>>>> can support physically non-continuous buffer on non-iommu.
>>>>>
>>>>> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
>>>>> whether buffer is continuous physically if iommu is supported. This
>>>>> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
>>>>> mean that the buffer is continuous for device.
>>>>>
>>>>> There is no user API to control cache coherence for the cpu and device
>>>>> about cachable buffer. This patchset introduces two ioctls for cpu
>>>>> access of gem object from user. It will be synced properly in order for
>>>>> the cpu and device if buffer of gem object is cachable.
>>>>
>>>> Out of curiosity, where's the userspace part for this work? Usually kernel
>>>> abi extensions come with a link to the relevant branch for easier review.
>>>> -Daniel
>>>>
>>>
>>> Right, thanks for point.
>>>
>>> I just modified a little bit exynos_fimg2d_test and exynos parts of
>>> libdrm to test them as workaround. I will expose it.
>>>
>>
>> Please refer follows.
>> https://github.com/dofmind/libdrm/commits/exynos
>>
>> If fimd2d test program uses cachable gem, it will show cache coherency
>> problem, so it adds to use new ioctls to keep cache coherency.
> 
> Is there some real userspace too, not just a test/demo app?

Unfortunately not yet, i hope it can be used on real userspace.

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

Re: [PATCH 0/9] drm/exynos: update codes related with gem

[Daniel Vetter]

On Wed, Oct 14, 2015 at 09:33:32AM +0900, Joonyoung Shim wrote:
> On 10/13/2015 09:29 PM, Daniel Vetter wrote:
> > On Tue, Oct 13, 2015 at 06:32:53PM +0900, Joonyoung Shim wrote:
> >> On 10/13/2015 05:37 PM, Joonyoung Shim wrote:
> >>> On 10/13/2015 05:27 PM, Daniel Vetter wrote:
> >>>> On Tue, Oct 13, 2015 at 04:00:45PM +0900, Joonyoung Shim wrote:
> >>>>> Hi,
> >>>>>
> >>>>> This patchset is about gem codes update of exynos-drm.
> >>>>>
> >>>>> The first and second patches are cleanup to remove useless codes.
> >>>>> The rest is to support cachable gem allocation.
> >>>>>
> >>>>> The exynos-drm uses DMA mapping API to allocate/mmap buffer of gem. If
> >>>>> it is cachable, does it with DMA_ATTR_NON_CONSISTENT attribute, but
> >>>>> DMA_ATTR_NON_CONSISTENT isn't supported in DMA mapping API of ARM, so it
> >>>>> doesn't give any effects.
> >>>>>
> >>>>> This patchset introduces new buffer allocation to use
> >>>>> drm_gem_get/put_pages() instead of DMA mapping API. It will be used
> >>>>> for the rest except allocation of physically continuous buffer on
> >>>>> non-iommu, then exynos-drm can support cachable buffer of gem. Also it
> >>>>> can support physically non-continuous buffer on non-iommu.
> >>>>>
> >>>>> EXYNOS_BO_CONTIG flag on iommu is ambiguous because it doesn't care
> >>>>> whether buffer is continuous physically if iommu is supported. This
> >>>>> patchset always will use EXYNOS_BO_CONTIG flag on iommu and then can
> >>>>> mean that the buffer is continuous for device.
> >>>>>
> >>>>> There is no user API to control cache coherence for the cpu and device
> >>>>> about cachable buffer. This patchset introduces two ioctls for cpu
> >>>>> access of gem object from user. It will be synced properly in order for
> >>>>> the cpu and device if buffer of gem object is cachable.
> >>>>
> >>>> Out of curiosity, where's the userspace part for this work? Usually kernel
> >>>> abi extensions come with a link to the relevant branch for easier review.
> >>>> -Daniel
> >>>>
> >>>
> >>> Right, thanks for point.
> >>>
> >>> I just modified a little bit exynos_fimg2d_test and exynos parts of
> >>> libdrm to test them as workaround. I will expose it.
> >>>
> >>
> >> Please refer follows.
> >> https://github.com/dofmind/libdrm/commits/exynos
> >>
> >> If fimd2d test program uses cachable gem, it will show cache coherency
> >> problem, so it adds to use new ioctls to keep cache coherency.
> > 
> > Is there some real userspace too, not just a test/demo app?
> 
> Unfortunately not yet, i hope it can be used on real userspace.

In general we wait with merging new userspace ABI until that's really
ready, since very often there's some corner cases that you just didn't
think about in your toy demo compared to the real thing. Like how to
manage buffer caches/reuse for apps that essentially run forever (like X
server or a compositor).
-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