[PATCH 0/5] Add option to mmap GEM buffers cached, try 2

[PATCH 0/5] Add option to mmap GEM buffers cached, try 2

[Paul Cercueil]

Rework of my previous patchset which added support for GEM buffers
backed by non-coherent memory to the ingenic-drm driver.

For the record, the previous patchset was accepted for 5.10 then had
to be reverted, as it conflicted with some changes made to the DMA API.

This new patchset is pretty different as it adds the functionality to
the DRM core. The first three patches add variants to existing functions
but with the "non-coherent memory" twist, exported as GPL symbols. The
fourth patch adds a function to be used with the damage helpers.
Finally, the last patch adds support for non-coherent GEM buffers to the
ingenic-drm driver. The functionality is enabled through a module
parameter, and is disabled by default.

Cheers,
-Paul

Paul Cercueil (5):
  drm: Add and export function drm_gem_cma_create_noncoherent
  drm: Add and export function drm_gem_cma_dumb_create_noncoherent
  drm: Add and export function drm_gem_cma_mmap_noncoherent
  drm: Add and export function drm_gem_cma_sync_data
  drm/ingenic: Add option to alloc cached GEM buffers

 drivers/gpu/drm/drm_gem_cma_helper.c      | 190 +++++++++++++++++++---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c |  58 ++++++-
 drivers/gpu/drm/ingenic/ingenic-drm.h     |   4 +
 drivers/gpu/drm/ingenic/ingenic-ipu.c     |  12 +-
 include/drm/drm_gem_cma_helper.h          |  13 ++
 5 files changed, 251 insertions(+), 26 deletions(-)

-- 
2.28.0

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

[PATCH 1/5] drm: Add and export function drm_gem_cma_create_noncoherent

[Paul Cercueil]

This function can be used by drivers that need to create a GEM object
with non-coherent backing memory.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_gem_cma_helper.c | 71 +++++++++++++++++++++-------
 include/drm/drm_gem_cma_helper.h     |  2 +
 2 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 2165633c9b9e..717871d741fb 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -77,21 +77,10 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size)
 	return ERR_PTR(ret);
 }
 
-/**
- * drm_gem_cma_create - allocate an object with the given size
- * @drm: DRM device
- * @size: size of the object to allocate
- *
- * This function creates a CMA GEM object and allocates a contiguous chunk of
- * memory as backing store. The backing memory has the writecombine attribute
- * set.
- *
- * Returns:
- * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
- * error code on failure.
- */
-struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
-					      size_t size)
+static struct drm_gem_cma_object *
+drm_gem_cma_create_with_cache_param(struct drm_device *drm,
+				    size_t size,
+				    bool noncoherent)
 {
 	struct drm_gem_cma_object *cma_obj;
 	int ret;
@@ -102,8 +91,16 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 	if (IS_ERR(cma_obj))
 		return cma_obj;
 
-	cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr,
-				      GFP_KERNEL | __GFP_NOWARN);
+	if (noncoherent) {
+		cma_obj->vaddr = dma_alloc_noncoherent(drm->dev, size,
+						       &cma_obj->paddr,
+						       DMA_TO_DEVICE,
+						       GFP_KERNEL | __GFP_NOWARN);
+
+	} else {
+		cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr,
+					      GFP_KERNEL | __GFP_NOWARN);
+	}
 	if (!cma_obj->vaddr) {
 		drm_dbg(drm, "failed to allocate buffer with size %zu\n",
 			 size);
@@ -117,8 +114,48 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 	drm_gem_object_put(&cma_obj->base);
 	return ERR_PTR(ret);
 }
+
+/**
+ * drm_gem_cma_create - allocate an object with the given size
+ * @drm: DRM device
+ * @size: size of the object to allocate
+ *
+ * This function creates a CMA GEM object and allocates a contiguous chunk of
+ * memory as backing store. The backing memory has the writecombine attribute
+ * set.
+ *
+ * Returns:
+ * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
+ * error code on failure.
+ */
+struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
+					      size_t size)
+{
+	return drm_gem_cma_create_with_cache_param(drm, size, false);
+}
 EXPORT_SYMBOL_GPL(drm_gem_cma_create);
 
+/**
+ * drm_gem_cma_create_noncoherent - allocate an object with the given size
+ *     and non-coherent cache attribute
+ * @drm: DRM device
+ * @size: size of the object to allocate
+ *
+ * This function creates a CMA GEM object and allocates a contiguous chunk of
+ * memory as backing store. The backing memory has the noncoherent attribute
+ * set.
+ *
+ * Returns:
+ * A struct drm_gem_cma_object * on success or an ERR_PTR()-encoded negative
+ * error code on failure.
+ */
+struct drm_gem_cma_object *
+drm_gem_cma_create_noncoherent(struct drm_device *drm, size_t size)
+{
+	return drm_gem_cma_create_with_cache_param(drm, size, true);
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent);
+
 /**
  * drm_gem_cma_create_with_handle - allocate an object with the given size and
  *     return a GEM handle to it
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 2bfa2502607a..071c73ad7177 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -82,6 +82,8 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
 /* allocate physical memory */
 struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 					      size_t size);
+struct drm_gem_cma_object *
+drm_gem_cma_create_noncoherent(struct drm_device *drm, size_t size);
 
 extern const struct vm_operations_struct drm_gem_cma_vm_ops;
 
-- 
2.28.0

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

[PATCH 2/5] drm: Add and export function drm_gem_cma_dumb_create_noncoherent

[Paul Cercueil]

This function can be used by drivers to create dumb buffers with
non-coherent backing memory.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_gem_cma_helper.c | 37 +++++++++++++++++++++++++---
 include/drm/drm_gem_cma_helper.h     |  4 +++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 717871d741fb..3bdd67795e20 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -163,6 +163,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent);
  * @drm: DRM device
  * @size: size of the object to allocate
  * @handle: return location for the GEM handle
+ * @noncoherent: allocate object with non-coherent cache attribute
  *
  * This function creates a CMA GEM object, allocating a physically contiguous
  * chunk of memory as backing store. The GEM object is then added to the list
@@ -175,13 +176,13 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_create_noncoherent);
 static struct drm_gem_cma_object *
 drm_gem_cma_create_with_handle(struct drm_file *file_priv,
 			       struct drm_device *drm, size_t size,
-			       uint32_t *handle)
+			       uint32_t *handle, bool noncoherent)
 {
 	struct drm_gem_cma_object *cma_obj;
 	struct drm_gem_object *gem_obj;
 	int ret;
 
-	cma_obj = drm_gem_cma_create(drm, size);
+	cma_obj = drm_gem_cma_create_with_cache_param(drm, size, noncoherent);
 	if (IS_ERR(cma_obj))
 		return cma_obj;
 
@@ -258,7 +259,7 @@ int drm_gem_cma_dumb_create_internal(struct drm_file *file_priv,
 		args->size = args->pitch * args->height;
 
 	cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
-						 &args->handle);
+						 &args->handle, false);
 	return PTR_ERR_OR_ZERO(cma_obj);
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_internal);
@@ -291,11 +292,39 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
 	args->size = args->pitch * args->height;
 
 	cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
-						 &args->handle);
+						 &args->handle, false);
 	return PTR_ERR_OR_ZERO(cma_obj);
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create);
 
+/**
+ * drm_gem_cma_dumb_create_noncoherent - create a dumb buffer object with
+ *     non-coherent cache attribute
+ * @file_priv: DRM file-private structure to create the dumb buffer for
+ * @drm: DRM device
+ * @args: IOCTL data
+ *
+ * Same as drm_gem_cma_dumb_create, but the dumb buffer object created has
+ * the non-coherent cache attribute set.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv,
+					struct drm_device *drm,
+					struct drm_mode_create_dumb *args)
+{
+	struct drm_gem_cma_object *cma_obj;
+
+	args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+	args->size = args->pitch * args->height;
+
+	cma_obj = drm_gem_cma_create_with_handle(file_priv, drm, args->size,
+						 &args->handle, true);
+	return PTR_ERR_OR_ZERO(cma_obj);
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_dumb_create_noncoherent);
+
 const struct vm_operations_struct drm_gem_cma_vm_ops = {
 	.open = drm_gem_vm_open,
 	.close = drm_gem_vm_close,
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 071c73ad7177..d0e6a1cd0950 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -76,6 +76,10 @@ int drm_gem_cma_dumb_create(struct drm_file *file_priv,
 			    struct drm_device *drm,
 			    struct drm_mode_create_dumb *args);
 
+int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv,
+					struct drm_device *drm,
+					struct drm_mode_create_dumb *args);
+
 /* set vm_flags and we can change the VM attribute to other one at here */
 int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
 
-- 
2.28.0

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

[PATCH 3/5] drm: Add and export function drm_gem_cma_mmap_noncoherent

[Paul Cercueil]

This function can be used by drivers that need to mmap dumb buffers
created with non-coherent backing memory.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_gem_cma_helper.c | 39 ++++++++++++++++++++++++++++
 include/drm/drm_gem_cma_helper.h     |  2 ++
 2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 3bdd67795e20..4ed63f4896bd 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -387,6 +387,45 @@ int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma)
 }
 EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
 
+/**
+ * drm_gem_cma_mmap_noncoherent - memory-map a CMA GEM object with
+ *     non-coherent cache attribute
+ * @filp: file object
+ * @vma: VMA for the area to be mapped
+ *
+ * Just like drm_gem_cma_mmap, but for a GEM object backed by non-coherent
+ * memory.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_cma_mmap_noncoherent(struct file *filp, struct vm_area_struct *vma)
+{
+	struct drm_gem_cma_object *cma_obj;
+	int ret;
+
+	ret = drm_gem_mmap(filp, vma);
+	if (ret)
+		return ret;
+
+	cma_obj = to_drm_gem_cma_obj(vma->vm_private_data);
+
+	/*
+	 * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
+	 * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
+	 * the whole buffer.
+	 */
+	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_pgoff = 0;
+	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+
+	return remap_pfn_range(vma, vma->vm_start,
+			       cma_obj->paddr >> PAGE_SHIFT,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot);
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_mmap_noncoherent);
+
 #ifndef CONFIG_MMU
 /**
  * drm_gem_cma_get_unmapped_area - propose address for mapping in noMMU cases
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index d0e6a1cd0950..6b01ad5581c3 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -83,6 +83,8 @@ int drm_gem_cma_dumb_create_noncoherent(struct drm_file *file_priv,
 /* set vm_flags and we can change the VM attribute to other one at here */
 int drm_gem_cma_mmap(struct file *filp, struct vm_area_struct *vma);
 
+int drm_gem_cma_mmap_noncoherent(struct file *filep, struct vm_area_struct *vma);
+
 /* allocate physical memory */
 struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
 					      size_t size);
-- 
2.28.0

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

Re: [PATCH 3/5] drm: Add and export function drm_gem_cma_mmap_noncoherent

[Paul Cercueil]

Hi Christoph,

Le mar. 3 nov. 2020 à 18:50, Christoph Hellwig <hch@infradead.org> a 
écrit :
> On Mon, Nov 02, 2020 at 10:06:49PM +0000, Paul Cercueil wrote:
>>  This function can be used by drivers that need to mmap dumb buffers
>>  created with non-coherent backing memory.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/gpu/drm/drm_gem_cma_helper.c | 39 
>> ++++++++++++++++++++++++++++
>>   include/drm/drm_gem_cma_helper.h     |  2 ++
>>   2 files changed, 41 insertions(+)
>> 
>>  diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
>> b/drivers/gpu/drm/drm_gem_cma_helper.c
>>  index 3bdd67795e20..4ed63f4896bd 100644
>>  --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>>  +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>>  @@ -387,6 +387,45 @@ int drm_gem_cma_mmap(struct file *filp, struct 
>> vm_area_struct *vma)
>>   }
>>   EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>> 
>>  +/**
>>  + * drm_gem_cma_mmap_noncoherent - memory-map a CMA GEM object with
>>  + *     non-coherent cache attribute
>>  + * @filp: file object
>>  + * @vma: VMA for the area to be mapped
>>  + *
>>  + * Just like drm_gem_cma_mmap, but for a GEM object backed by 
>> non-coherent
>>  + * memory.
>>  + *
>>  + * Returns:
>>  + * 0 on success or a negative error code on failure.
>>  + */
>>  +int drm_gem_cma_mmap_noncoherent(struct file *filp, struct 
>> vm_area_struct *vma)
>>  +{
>>  +	struct drm_gem_cma_object *cma_obj;
>>  +	int ret;
>>  +
>>  +	ret = drm_gem_mmap(filp, vma);
>>  +	if (ret)
>>  +		return ret;
>>  +
>>  +	cma_obj = to_drm_gem_cma_obj(vma->vm_private_data);
>>  +
>>  +	/*
>>  +	 * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and 
>> set the
>>  +	 * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want 
>> to map
>>  +	 * the whole buffer.
>>  +	 */
>>  +	vma->vm_flags &= ~VM_PFNMAP;
>>  +	vma->vm_pgoff = 0;
>>  +	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>>  +
>>  +	return remap_pfn_range(vma, vma->vm_start,
>>  +			       cma_obj->paddr >> PAGE_SHIFT,
>>  +			       vma->vm_end - vma->vm_start,
>>  +			       vma->vm_page_prot);
> 
> Per patch 1 cma_obj->paddr is the dma address, while remap_pfn_range
> expects a physical address.  This does not work.

Ok, what would be the correct way to mmap_noncoherent?

-Paul


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

Re: [PATCH 3/5] drm: Add and export function drm_gem_cma_mmap_noncoherent

[Paul Cercueil]

Hi Christoph,

Le mar. 3 nov. 2020 à 19:13, Paul Cercueil <paul@crapouillou.net> a 
écrit :
> Hi Christoph,
> 
> Le mar. 3 nov. 2020 à 18:50, Christoph Hellwig <hch@infradead.org> a 
> écrit :
>> On Mon, Nov 02, 2020 at 10:06:49PM +0000, Paul Cercueil wrote:
>>>  This function can be used by drivers that need to mmap dumb buffers
>>>  created with non-coherent backing memory.
>>> 
>>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>  ---
>>>   drivers/gpu/drm/drm_gem_cma_helper.c | 39 
>>> \x7f\x7f++++++++++++++++++++++++++++
>>>   include/drm/drm_gem_cma_helper.h     |  2 ++
>>>   2 files changed, 41 insertions(+)
>>> 
>>>  diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c 
>>> \x7f\x7fb/drivers/gpu/drm/drm_gem_cma_helper.c
>>>  index 3bdd67795e20..4ed63f4896bd 100644
>>>  --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>>>  +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>>>  @@ -387,6 +387,45 @@ int drm_gem_cma_mmap(struct file *filp, 
>>> struct \x7f\x7fvm_area_struct *vma)
>>>   }
>>>   EXPORT_SYMBOL_GPL(drm_gem_cma_mmap);
>>> 
>>>  +/**
>>>  + * drm_gem_cma_mmap_noncoherent - memory-map a CMA GEM object with
>>>  + *     non-coherent cache attribute
>>>  + * @filp: file object
>>>  + * @vma: VMA for the area to be mapped
>>>  + *
>>>  + * Just like drm_gem_cma_mmap, but for a GEM object backed by 
>>> \x7f\x7fnon-coherent
>>>  + * memory.
>>>  + *
>>>  + * Returns:
>>>  + * 0 on success or a negative error code on failure.
>>>  + */
>>>  +int drm_gem_cma_mmap_noncoherent(struct file *filp, struct 
>>> \x7f\x7fvm_area_struct *vma)
>>>  +{
>>>  +	struct drm_gem_cma_object *cma_obj;
>>>  +	int ret;
>>>  +
>>>  +	ret = drm_gem_mmap(filp, vma);
>>>  +	if (ret)
>>>  +		return ret;
>>>  +
>>>  +	cma_obj = to_drm_gem_cma_obj(vma->vm_private_data);
>>>  +
>>>  +	/*
>>>  +	 * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and 
>>> \x7f\x7fset the
>>>  +	 * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we 
>>> want \x7f\x7fto map
>>>  +	 * the whole buffer.
>>>  +	 */
>>>  +	vma->vm_flags &= ~VM_PFNMAP;
>>>  +	vma->vm_pgoff = 0;
>>>  +	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>>>  +
>>>  +	return remap_pfn_range(vma, vma->vm_start,
>>>  +			       cma_obj->paddr >> PAGE_SHIFT,
>>>  +			       vma->vm_end - vma->vm_start,
>>>  +			       vma->vm_page_prot);
>> 
>> Per patch 1 cma_obj->paddr is the dma address, while remap_pfn_range
>> expects a physical address.  This does not work.
> 
> Ok, what would be the correct way to mmap_noncoherent?

Waiting for your input here :)

Cheers,
-Paul


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

[PATCH 4/5] drm: Add and export function drm_gem_cma_sync_data

[Paul Cercueil]

This function can be used by drivers that use damage clips and have
CMA GEM objects backed by non-coherent memory. Calling this function
in a plane's .atomic_update ensures that all the data in the backing
memory have been written to RAM.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/drm_gem_cma_helper.c | 43 ++++++++++++++++++++++++++++
 include/drm/drm_gem_cma_helper.h     |  5 ++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 4ed63f4896bd..74bff6d45d4e 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -17,9 +17,14 @@
 #include <linux/slab.h>
 
 #include <drm/drm.h>
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_plane.h>
 #include <drm/drm_vma_manager.h>
 
 /**
@@ -742,3 +747,41 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev,
 	return obj;
 }
 EXPORT_SYMBOL(drm_gem_cma_prime_import_sg_table_vmap);
+
+/**
+ * drm_gem_cma_sync_data - Sync GEM object to non-coherent backing memory
+ * @dev: DRM device
+ * @old_state: Old plane state
+ * @state: New plane state
+ *
+ * This function can be used by drivers that use damage clips and have
+ * CMA GEM objects backed by non-coherent memory. Calling this function
+ * in a plane's .atomic_update ensures that all the data in the backing
+ * memory have been written to RAM.
+ */
+void drm_gem_cma_sync_data(struct device *dev,
+			   struct drm_plane_state *old_state,
+			   struct drm_plane_state *state)
+{
+	const struct drm_format_info *finfo = state->fb->format;
+	struct drm_atomic_helper_damage_iter iter;
+	unsigned int offset, i;
+	struct drm_rect clip;
+	dma_addr_t daddr;
+
+	drm_atomic_helper_damage_iter_init(&iter, old_state, state);
+
+	drm_atomic_for_each_plane_damage(&iter, &clip) {
+		for (i = 0; i < finfo->num_planes; i++) {
+			daddr = drm_fb_cma_get_gem_addr(state->fb, state, i);
+
+			/* Ignore x1/x2 values, invalidate complete lines */
+			offset = clip.y1 * state->fb->pitches[i];
+
+			dma_sync_single_for_device(dev, daddr + offset,
+				       (clip.y2 - clip.y1) * state->fb->pitches[i],
+				       DMA_TO_DEVICE);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(drm_gem_cma_sync_data);
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 6b01ad5581c3..9bdba4ce65ba 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -7,6 +7,7 @@
 #include <drm/drm_gem.h>
 
 struct drm_mode_create_dumb;
+struct drm_plane_state;
 
 /**
  * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
@@ -200,4 +201,8 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm,
 				       struct dma_buf_attachment *attach,
 				       struct sg_table *sgt);
 
+void drm_gem_cma_sync_data(struct device *dev,
+			   struct drm_plane_state *old_state,
+			   struct drm_plane_state *state);
+
 #endif /* __DRM_GEM_CMA_HELPER_H__ */
-- 
2.28.0

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

[PATCH 5/5] drm/ingenic: Add option to alloc cached GEM buffers

[Paul Cercueil]

With the module parameter ingenic-drm.cached_gem_buffers, it is possible
to specify that we want GEM buffers backed by non-coherent memory.

This dramatically speeds up software rendering on Ingenic SoCs, even for
tasks where write-combine memory should in theory be faster (e.g. simple
blits).

Leave it disabled by default, since it is specific to one use-case
(software rendering).

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++--
 drivers/gpu/drm/ingenic/ingenic-drm.h     |  4 ++
 drivers/gpu/drm/ingenic/ingenic-ipu.c     | 12 ++++-
 3 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index b9c156e13156..1ec2ec2faa04 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -9,6 +9,7 @@
 #include <linux/component.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
@@ -22,6 +23,7 @@
 #include <drm/drm_color_mgmt.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -97,6 +99,11 @@ struct ingenic_drm {
 	struct notifier_block clock_nb;
 };
 
+static bool ingenic_drm_cached_gem_buf;
+module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400);
+MODULE_PARM_DESC(cached_gem_buffers,
+		 "Enable fully cached GEM buffers [default=false]");
+
 static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
 	     plane->state->fb->format->format != state->fb->format->format))
 		crtc_state->mode_changed = true;
 
+	drm_atomic_helper_check_plane_damage(state->state, state);
+
 	return 0;
 }
 
@@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv,
 	}
 }
 
+void ingenic_drm_sync_data(struct device *dev,
+			   struct drm_plane_state *old_state,
+			   struct drm_plane_state *state)
+{
+	if (ingenic_drm_cached_gem_buf)
+		drm_gem_cma_sync_data(dev, old_state, state);
+}
+
 static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 					    struct drm_plane_state *oldstate)
 {
@@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 	u32 fourcc;
 
 	if (state && state->fb) {
+		ingenic_drm_sync_data(priv->dev, oldstate, state);
+
 		crtc_state = state->crtc->state;
 
 		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
@@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc)
 	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0);
 }
 
-DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
+static struct drm_framebuffer *
+ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
+			  const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	if (ingenic_drm_cached_gem_buf)
+		return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);
+
+	return drm_gem_fb_create(dev, file, mode_cmd);
+}
+
+static int ingenic_drm_gem_cma_mmap(struct file *filp,
+				    struct vm_area_struct *vma)
+{
+	if (ingenic_drm_cached_gem_buf)
+		return drm_gem_cma_mmap_noncoherent(filp, vma);
+
+	return drm_gem_cma_mmap(filp, vma);
+}
+
+static const struct file_operations ingenic_drm_fops = {
+	.owner		= THIS_MODULE,
+	.open		= drm_open,
+	.release	= drm_release,
+	.unlocked_ioctl	= drm_ioctl,
+	.compat_ioctl	= drm_compat_ioctl,
+	.poll		= drm_poll,
+	.read		= drm_read,
+	.llseek		= noop_llseek,
+	.mmap		= ingenic_drm_gem_cma_mmap,
+};
 
 static struct drm_driver ingenic_drm_driver_data = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
@@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = {
 	.patchlevel		= 0,
 
 	.fops			= &ingenic_drm_fops,
-	DRM_GEM_CMA_DRIVER_OPS,
+	DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
 
 	.irq_handler		= ingenic_drm_irq_handler,
 };
@@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs =
 };
 
 static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
-	.fb_create		= drm_gem_fb_create,
+	.fb_create		= ingenic_drm_gem_fb_create,
 	.output_poll_changed	= drm_fb_helper_output_poll_changed,
 	.atomic_check		= drm_atomic_helper_check,
 	.atomic_commit		= drm_atomic_helper_commit,
@@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 		return ret;
 	}
 
+	drm_plane_enable_fb_damage_clips(&priv->f1);
+
 	drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
 
 	ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
@@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 			return ret;
 		}
 
+		drm_plane_enable_fb_damage_clips(&priv->f0);
+
 		if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) {
 			ret = component_bind_all(dev, drm);
 			if (ret) {
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index 9b48ce02803d..ee3a892c0383 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev,
 			      struct drm_plane *plane, u32 fourcc);
 void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
 
+void ingenic_drm_sync_data(struct device *dev,
+			   struct drm_plane_state *old_state,
+			   struct drm_plane_state *state);
+
 extern struct platform_driver *ingenic_ipu_driver_ptr;
 
 #endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index fc8c6e970ee3..38c83e8cc6a5 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -20,6 +20,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fourcc.h>
@@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
 				JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL);
 	}
 
+	ingenic_drm_sync_data(ipu->master, oldstate, state);
+
 	/* New addresses will be committed in vblank handler... */
 	ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0);
 	if (finfo->num_planes > 1)
@@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
 
 	if (!state->crtc ||
 	    !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
-		return 0;
+		goto out_check_damage;
 
 	/* Plane must be fully visible */
 	if (state->crtc_x < 0 || state->crtc_y < 0 ||
@@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 
 	if (!osd_changed(state, plane->state))
-		return 0;
+		goto out_check_damage;
 
 	crtc_state->mode_changed = true;
 
@@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
 	ipu->denom_w = denom_w;
 	ipu->denom_h = denom_h;
 
+out_check_damage:
+	drm_atomic_helper_check_plane_damage(state->state, state);
+
 	return 0;
 }
 
@@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
 		return err;
 	}
 
+	drm_plane_enable_fb_damage_clips(plane);
+
 	/*
 	 * Sharpness settings range is [0,32]
 	 * 0       : nearest-neighbor
-- 
2.28.0

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

Re: [PATCH 5/5] drm/ingenic: Add option to alloc cached GEM buffers

[Daniel Vetter]

On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
> With the module parameter ingenic-drm.cached_gem_buffers, it is possible
> to specify that we want GEM buffers backed by non-coherent memory.
> 
> This dramatically speeds up software rendering on Ingenic SoCs, even for
> tasks where write-combine memory should in theory be faster (e.g. simple
> blits).
> 
> Leave it disabled by default, since it is specific to one use-case
> (software rendering).
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Hm so maybe I'm making myself supremely unpopular here again with being
very late, but we have dev->mode_config.prefer_shadow for this. Drivers
should set this if software rendering is slow, and userspace should follow
this.

Now unfortunately it looks like most kms drivers don't bother setting
this, and we're a lot worse.

So if "slow sw rendering" is the only reason, setting that flag is the
right option.

Now the other thing is fbdev, since fbdev doesn't have this hint at all.
But we already have full generic fbdev shadow support (for defio), so for
that I think the best option is adding a force_shadow option to fbdev.

If we otoh do this here, and some drivers get a in-kernel shadow support
for kms, while others don't, then we have a pretty supreme mess. I'd like
to avoid that.

So maybe the right solution here is that we make sure that when you have
cma helpers set up that mode_config.prefer_shadow is set. Ideally only
when coherent dma memory is uncached/write-combine and hence slow for sw
rendering. This might need a slight dma-api layering violation.
-Daniel

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 +++++++++++++++++++++--
>  drivers/gpu/drm/ingenic/ingenic-drm.h     |  4 ++
>  drivers/gpu/drm/ingenic/ingenic-ipu.c     | 12 ++++-
>  3 files changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index b9c156e13156..1ec2ec2faa04 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -9,6 +9,7 @@
>  #include <linux/component.h>
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of_device.h>
> @@ -22,6 +23,7 @@
>  #include <drm/drm_color_mgmt.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_damage_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
> @@ -97,6 +99,11 @@ struct ingenic_drm {
>  	struct notifier_block clock_nb;
>  };
>  
> +static bool ingenic_drm_cached_gem_buf;
> +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, bool, 0400);
> +MODULE_PARM_DESC(cached_gem_buffers,
> +		 "Enable fully cached GEM buffers [default=false]");
> +
>  static bool ingenic_drm_writeable_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
> @@ -400,6 +407,8 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>  	     plane->state->fb->format->format != state->fb->format->format))
>  		crtc_state->mode_changed = true;
>  
> +	drm_atomic_helper_check_plane_damage(state->state, state);
> +
>  	return 0;
>  }
>  
> @@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct ingenic_drm *priv,
>  	}
>  }
>  
> +void ingenic_drm_sync_data(struct device *dev,
> +			   struct drm_plane_state *old_state,
> +			   struct drm_plane_state *state)
> +{
> +	if (ingenic_drm_cached_gem_buf)
> +		drm_gem_cma_sync_data(dev, old_state, state);
> +}
> +
>  static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>  					    struct drm_plane_state *oldstate)
>  {
> @@ -543,6 +560,8 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>  	u32 fourcc;
>  
>  	if (state && state->fb) {
> +		ingenic_drm_sync_data(priv->dev, oldstate, state);
> +
>  		crtc_state = state->crtc->state;
>  
>  		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> @@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct drm_crtc *crtc)
>  	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, JZ_LCD_CTRL_EOF_IRQ, 0);
>  }
>  
> -DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
> +static struct drm_framebuffer *
> +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> +			  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	if (ingenic_drm_cached_gem_buf)
> +		return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);
> +
> +	return drm_gem_fb_create(dev, file, mode_cmd);
> +}
> +
> +static int ingenic_drm_gem_cma_mmap(struct file *filp,
> +				    struct vm_area_struct *vma)
> +{
> +	if (ingenic_drm_cached_gem_buf)
> +		return drm_gem_cma_mmap_noncoherent(filp, vma);
> +
> +	return drm_gem_cma_mmap(filp, vma);
> +}
> +
> +static const struct file_operations ingenic_drm_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= drm_open,
> +	.release	= drm_release,
> +	.unlocked_ioctl	= drm_ioctl,
> +	.compat_ioctl	= drm_compat_ioctl,
> +	.poll		= drm_poll,
> +	.read		= drm_read,
> +	.llseek		= noop_llseek,
> +	.mmap		= ingenic_drm_gem_cma_mmap,
> +};
>  
>  static struct drm_driver ingenic_drm_driver_data = {
>  	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> @@ -726,7 +774,7 @@ static struct drm_driver ingenic_drm_driver_data = {
>  	.patchlevel		= 0,
>  
>  	.fops			= &ingenic_drm_fops,
> -	DRM_GEM_CMA_DRIVER_OPS,
> +	DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
>  
>  	.irq_handler		= ingenic_drm_irq_handler,
>  };
> @@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs ingenic_drm_encoder_helper_funcs =
>  };
>  
>  static const struct drm_mode_config_funcs ingenic_drm_mode_config_funcs = {
> -	.fb_create		= drm_gem_fb_create,
> +	.fb_create		= ingenic_drm_gem_fb_create,
>  	.output_poll_changed	= drm_fb_helper_output_poll_changed,
>  	.atomic_check		= drm_atomic_helper_check,
>  	.atomic_commit		= drm_atomic_helper_commit,
> @@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>  		return ret;
>  	}
>  
> +	drm_plane_enable_fb_damage_clips(&priv->f1);
> +
>  	drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
>  
>  	ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
> @@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>  			return ret;
>  		}
>  
> +		drm_plane_enable_fb_damage_clips(&priv->f0);
> +
>  		if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) {
>  			ret = component_bind_all(dev, drm);
>  			if (ret) {
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
> index 9b48ce02803d..ee3a892c0383 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
> @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device *dev,
>  			      struct drm_plane *plane, u32 fourcc);
>  void ingenic_drm_plane_disable(struct device *dev, struct drm_plane *plane);
>  
> +void ingenic_drm_sync_data(struct device *dev,
> +			   struct drm_plane_state *old_state,
> +			   struct drm_plane_state *state);
> +
>  extern struct platform_driver *ingenic_ipu_driver_ptr;
>  
>  #endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
> diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> index fc8c6e970ee3..38c83e8cc6a5 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> @@ -20,6 +20,7 @@
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_fourcc.h>
> @@ -316,6 +317,8 @@ static void ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
>  				JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL);
>  	}
>  
> +	ingenic_drm_sync_data(ipu->master, oldstate, state);
> +
>  	/* New addresses will be committed in vblank handler... */
>  	ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0);
>  	if (finfo->num_planes > 1)
> @@ -534,7 +537,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>  
>  	if (!state->crtc ||
>  	    !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
> -		return 0;
> +		goto out_check_damage;
>  
>  	/* Plane must be fully visible */
>  	if (state->crtc_x < 0 || state->crtc_y < 0 ||
> @@ -551,7 +554,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>  		return -EINVAL;
>  
>  	if (!osd_changed(state, plane->state))
> -		return 0;
> +		goto out_check_damage;
>  
>  	crtc_state->mode_changed = true;
>  
> @@ -578,6 +581,9 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>  	ipu->denom_w = denom_w;
>  	ipu->denom_h = denom_h;
>  
> +out_check_damage:
> +	drm_atomic_helper_check_plane_damage(state->state, state);
> +
>  	return 0;
>  }
>  
> @@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, struct device *master, void *d)
>  		return err;
>  	}
>  
> +	drm_plane_enable_fb_damage_clips(plane);
> +
>  	/*
>  	 * Sharpness settings range is [0,32]
>  	 * 0       : nearest-neighbor
> -- 
> 2.28.0
> 

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

Re: [PATCH 5/5] drm/ingenic: Add option to alloc cached GEM buffers

[Paul Cercueil]

Hi Daniel,

Le mar. 3 nov. 2020 à 11:17, Daniel Vetter <daniel@ffwll.ch> a écrit :
> On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
>>  With the module parameter ingenic-drm.cached_gem_buffers, it is 
>> possible
>>  to specify that we want GEM buffers backed by non-coherent memory.
>> 
>>  This dramatically speeds up software rendering on Ingenic SoCs, 
>> even for
>>  tasks where write-combine memory should in theory be faster (e.g. 
>> simple
>>  blits).
>> 
>>  Leave it disabled by default, since it is specific to one use-case
>>  (software rendering).
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> 
> Hm so maybe I'm making myself supremely unpopular here again with 
> being
> very late, but we have dev->mode_config.prefer_shadow for this. 
> Drivers
> should set this if software rendering is slow, and userspace should 
> follow
> this.
> 
> Now unfortunately it looks like most kms drivers don't bother setting
> this, and we're a lot worse.
> 
> So if "slow sw rendering" is the only reason, setting that flag is the
> right option.

The "prefer_shadow" is based on the assumption that rendering to a 
shadow buffer, then memcpy that buffer to the write-combine destination 
buffer, is faster than rendering to a non-coherent destination buffer 
and sync the data cache. This might be true on some hardware, but is 
not the case on Ingenic SoCs, where even simple blits (e.g. memcpy) are 
about three times faster using the second method.

-Paul

> Now the other thing is fbdev, since fbdev doesn't have this hint at 
> all.
> But we already have full generic fbdev shadow support (for defio), so 
> for
> that I think the best option is adding a force_shadow option to fbdev.
> 
> If we otoh do this here, and some drivers get a in-kernel shadow 
> support
> for kms, while others don't, then we have a pretty supreme mess. I'd 
> like
> to avoid that.
> 
> So maybe the right solution here is that we make sure that when you 
> have
> cma helpers set up that mode_config.prefer_shadow is set. Ideally only
> when coherent dma memory is uncached/write-combine and hence slow for 
> sw
> rendering. This might need a slight dma-api layering violation.
> 
> -Daniel
> 
>>  ---
>>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58 
>> +++++++++++++++++++++--
>>   drivers/gpu/drm/ingenic/ingenic-drm.h     |  4 ++
>>   drivers/gpu/drm/ingenic/ingenic-ipu.c     | 12 ++++-
>>   3 files changed, 69 insertions(+), 5 deletions(-)
>> 
>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>  index b9c156e13156..1ec2ec2faa04 100644
>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>  @@ -9,6 +9,7 @@
>>   #include <linux/component.h>
>>   #include <linux/clk.h>
>>   #include <linux/dma-mapping.h>
>>  +#include <linux/io.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>>   #include <linux/of_device.h>
>>  @@ -22,6 +23,7 @@
>>   #include <drm/drm_color_mgmt.h>
>>   #include <drm/drm_crtc.h>
>>   #include <drm/drm_crtc_helper.h>
>>  +#include <drm/drm_damage_helper.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_gem_cma_helper.h>
>>   #include <drm/drm_fb_cma_helper.h>
>>  @@ -97,6 +99,11 @@ struct ingenic_drm {
>>   	struct notifier_block clock_nb;
>>   };
>> 
>>  +static bool ingenic_drm_cached_gem_buf;
>>  +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf, 
>> bool, 0400);
>>  +MODULE_PARM_DESC(cached_gem_buffers,
>>  +		 "Enable fully cached GEM buffers [default=false]");
>>  +
>>   static bool ingenic_drm_writeable_reg(struct device *dev, unsigned 
>> int reg)
>>   {
>>   	switch (reg) {
>>  @@ -400,6 +407,8 @@ static int 
>> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>   	     plane->state->fb->format->format != 
>> state->fb->format->format))
>>   		crtc_state->mode_changed = true;
>> 
>>  +	drm_atomic_helper_check_plane_damage(state->state, state);
>>  +
>>   	return 0;
>>   }
>> 
>>  @@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct 
>> ingenic_drm *priv,
>>   	}
>>   }
>> 
>>  +void ingenic_drm_sync_data(struct device *dev,
>>  +			   struct drm_plane_state *old_state,
>>  +			   struct drm_plane_state *state)
>>  +{
>>  +	if (ingenic_drm_cached_gem_buf)
>>  +		drm_gem_cma_sync_data(dev, old_state, state);
>>  +}
>>  +
>>   static void ingenic_drm_plane_atomic_update(struct drm_plane 
>> *plane,
>>   					    struct drm_plane_state *oldstate)
>>   {
>>  @@ -543,6 +560,8 @@ static void 
>> ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>>   	u32 fourcc;
>> 
>>   	if (state && state->fb) {
>>  +		ingenic_drm_sync_data(priv->dev, oldstate, state);
>>  +
>>   		crtc_state = state->crtc->state;
>> 
>>   		addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
>>  @@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct 
>> drm_crtc *crtc)
>>   	regmap_update_bits(priv->map, JZ_REG_LCD_CTRL, 
>> JZ_LCD_CTRL_EOF_IRQ, 0);
>>   }
>> 
>>  -DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
>>  +static struct drm_framebuffer *
>>  +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file 
>> *file,
>>  +			  const struct drm_mode_fb_cmd2 *mode_cmd)
>>  +{
>>  +	if (ingenic_drm_cached_gem_buf)
>>  +		return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);
>>  +
>>  +	return drm_gem_fb_create(dev, file, mode_cmd);
>>  +}
>>  +
>>  +static int ingenic_drm_gem_cma_mmap(struct file *filp,
>>  +				    struct vm_area_struct *vma)
>>  +{
>>  +	if (ingenic_drm_cached_gem_buf)
>>  +		return drm_gem_cma_mmap_noncoherent(filp, vma);
>>  +
>>  +	return drm_gem_cma_mmap(filp, vma);
>>  +}
>>  +
>>  +static const struct file_operations ingenic_drm_fops = {
>>  +	.owner		= THIS_MODULE,
>>  +	.open		= drm_open,
>>  +	.release	= drm_release,
>>  +	.unlocked_ioctl	= drm_ioctl,
>>  +	.compat_ioctl	= drm_compat_ioctl,
>>  +	.poll		= drm_poll,
>>  +	.read		= drm_read,
>>  +	.llseek		= noop_llseek,
>>  +	.mmap		= ingenic_drm_gem_cma_mmap,
>>  +};
>> 
>>   static struct drm_driver ingenic_drm_driver_data = {
>>   	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
>>  @@ -726,7 +774,7 @@ static struct drm_driver 
>> ingenic_drm_driver_data = {
>>   	.patchlevel		= 0,
>> 
>>   	.fops			= &ingenic_drm_fops,
>>  -	DRM_GEM_CMA_DRIVER_OPS,
>>  
>> +	DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
>> 
>>   	.irq_handler		= ingenic_drm_irq_handler,
>>   };
>>  @@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs 
>> ingenic_drm_encoder_helper_funcs =
>>   };
>> 
>>   static const struct drm_mode_config_funcs 
>> ingenic_drm_mode_config_funcs = {
>>  -	.fb_create		= drm_gem_fb_create,
>>  +	.fb_create		= ingenic_drm_gem_fb_create,
>>   	.output_poll_changed	= drm_fb_helper_output_poll_changed,
>>   	.atomic_check		= drm_atomic_helper_check,
>>   	.atomic_commit		= drm_atomic_helper_commit,
>>  @@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev, 
>> bool has_components)
>>   		return ret;
>>   	}
>> 
>>  +	drm_plane_enable_fb_damage_clips(&priv->f1);
>>  +
>>   	drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
>> 
>>   	ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
>>  @@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device 
>> *dev, bool has_components)
>>   			return ret;
>>   		}
>> 
>>  +		drm_plane_enable_fb_damage_clips(&priv->f0);
>>  +
>>   		if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) {
>>   			ret = component_bind_all(dev, drm);
>>   			if (ret) {
>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h 
>> b/drivers/gpu/drm/ingenic/ingenic-drm.h
>>  index 9b48ce02803d..ee3a892c0383 100644
>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
>>  @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device 
>> *dev,
>>   			      struct drm_plane *plane, u32 fourcc);
>>   void ingenic_drm_plane_disable(struct device *dev, struct 
>> drm_plane *plane);
>> 
>>  +void ingenic_drm_sync_data(struct device *dev,
>>  +			   struct drm_plane_state *old_state,
>>  +			   struct drm_plane_state *state);
>>  +
>>   extern struct platform_driver *ingenic_ipu_driver_ptr;
>> 
>>   #endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c 
>> b/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  index fc8c6e970ee3..38c83e8cc6a5 100644
>>  --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
>>  @@ -20,6 +20,7 @@
>> 
>>   #include <drm/drm_atomic.h>
>>   #include <drm/drm_atomic_helper.h>
>>  +#include <drm/drm_damage_helper.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_fb_cma_helper.h>
>>   #include <drm/drm_fourcc.h>
>>  @@ -316,6 +317,8 @@ static void 
>> ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
>>   				JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL);
>>   	}
>> 
>>  +	ingenic_drm_sync_data(ipu->master, oldstate, state);
>>  +
>>   	/* New addresses will be committed in vblank handler... */
>>   	ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0);
>>   	if (finfo->num_planes > 1)
>>  @@ -534,7 +537,7 @@ static int 
>> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>> 
>>   	if (!state->crtc ||
>>   	    !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
>>  -		return 0;
>>  +		goto out_check_damage;
>> 
>>   	/* Plane must be fully visible */
>>   	if (state->crtc_x < 0 || state->crtc_y < 0 ||
>>  @@ -551,7 +554,7 @@ static int 
>> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>>   		return -EINVAL;
>> 
>>   	if (!osd_changed(state, plane->state))
>>  -		return 0;
>>  +		goto out_check_damage;
>> 
>>   	crtc_state->mode_changed = true;
>> 
>>  @@ -578,6 +581,9 @@ static int 
>> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
>>   	ipu->denom_w = denom_w;
>>   	ipu->denom_h = denom_h;
>> 
>>  +out_check_damage:
>>  +	drm_atomic_helper_check_plane_damage(state->state, state);
>>  +
>>   	return 0;
>>   }
>> 
>>  @@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev, 
>> struct device *master, void *d)
>>   		return err;
>>   	}
>> 
>>  +	drm_plane_enable_fb_damage_clips(plane);
>>  +
>>   	/*
>>   	 * Sharpness settings range is [0,32]
>>   	 * 0       : nearest-neighbor
>>  --
>>  2.28.0
>> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


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

Re: [PATCH 5/5] drm/ingenic: Add option to alloc cached GEM buffers

[Daniel Vetter]

On Tue, Nov 3, 2020 at 12:57 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi Daniel,
>
> Le mar. 3 nov. 2020 à 11:17, Daniel Vetter <daniel@ffwll.ch> a écrit :
> > On Mon, Nov 02, 2020 at 10:06:51PM +0000, Paul Cercueil wrote:
> >>  With the module parameter ingenic-drm.cached_gem_buffers, it is
> >> possible
> >>  to specify that we want GEM buffers backed by non-coherent memory.
> >>
> >>  This dramatically speeds up software rendering on Ingenic SoCs,
> >> even for
> >>  tasks where write-combine memory should in theory be faster (e.g.
> >> simple
> >>  blits).
> >>
> >>  Leave it disabled by default, since it is specific to one use-case
> >>  (software rendering).
> >>
> >>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> >
> > Hm so maybe I'm making myself supremely unpopular here again with
> > being
> > very late, but we have dev->mode_config.prefer_shadow for this.
> > Drivers
> > should set this if software rendering is slow, and userspace should
> > follow
> > this.
> >
> > Now unfortunately it looks like most kms drivers don't bother setting
> > this, and we're a lot worse.
> >
> > So if "slow sw rendering" is the only reason, setting that flag is the
> > right option.
>
> The "prefer_shadow" is based on the assumption that rendering to a
> shadow buffer, then memcpy that buffer to the write-combine destination
> buffer, is faster than rendering to a non-coherent destination buffer
> and sync the data cache. This might be true on some hardware, but is
> not the case on Ingenic SoCs, where even simple blits (e.g. memcpy) are
> about three times faster using the second method.

Ah right, vague memories coming back, pretty sure I asked this before.
Please include this in your commit message (and ideally also in some
code comment somewhere), since otherwise I'll keep asking the same
question every single time you submit this or I stumble over it again
:-)

Cheers, Daniel

>
> -Paul
>
> > Now the other thing is fbdev, since fbdev doesn't have this hint at
> > all.
> > But we already have full generic fbdev shadow support (for defio), so
> > for
> > that I think the best option is adding a force_shadow option to fbdev.
> >
> > If we otoh do this here, and some drivers get a in-kernel shadow
> > support
> > for kms, while others don't, then we have a pretty supreme mess. I'd
> > like
> > to avoid that.
> >
> > So maybe the right solution here is that we make sure that when you
> > have
> > cma helpers set up that mode_config.prefer_shadow is set. Ideally only
> > when coherent dma memory is uncached/write-combine and hence slow for
> > sw
> > rendering. This might need a slight dma-api layering violation.
> >
> > -Daniel
> >
> >>  ---
> >>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 58
> >> +++++++++++++++++++++--
> >>   drivers/gpu/drm/ingenic/ingenic-drm.h     |  4 ++
> >>   drivers/gpu/drm/ingenic/ingenic-ipu.c     | 12 ++++-
> >>   3 files changed, 69 insertions(+), 5 deletions(-)
> >>
> >>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> >> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> >>  index b9c156e13156..1ec2ec2faa04 100644
> >>  --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> >>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> >>  @@ -9,6 +9,7 @@
> >>   #include <linux/component.h>
> >>   #include <linux/clk.h>
> >>   #include <linux/dma-mapping.h>
> >>  +#include <linux/io.h>
> >>   #include <linux/module.h>
> >>   #include <linux/mutex.h>
> >>   #include <linux/of_device.h>
> >>  @@ -22,6 +23,7 @@
> >>   #include <drm/drm_color_mgmt.h>
> >>   #include <drm/drm_crtc.h>
> >>   #include <drm/drm_crtc_helper.h>
> >>  +#include <drm/drm_damage_helper.h>
> >>   #include <drm/drm_drv.h>
> >>   #include <drm/drm_gem_cma_helper.h>
> >>   #include <drm/drm_fb_cma_helper.h>
> >>  @@ -97,6 +99,11 @@ struct ingenic_drm {
> >>      struct notifier_block clock_nb;
> >>   };
> >>
> >>  +static bool ingenic_drm_cached_gem_buf;
> >>  +module_param_named(cached_gem_buffers, ingenic_drm_cached_gem_buf,
> >> bool, 0400);
> >>  +MODULE_PARM_DESC(cached_gem_buffers,
> >>  +            "Enable fully cached GEM buffers [default=false]");
> >>  +
> >>   static bool ingenic_drm_writeable_reg(struct device *dev, unsigned
> >> int reg)
> >>   {
> >>      switch (reg) {
> >>  @@ -400,6 +407,8 @@ static int
> >> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
> >>           plane->state->fb->format->format !=
> >> state->fb->format->format))
> >>              crtc_state->mode_changed = true;
> >>
> >>  +   drm_atomic_helper_check_plane_damage(state->state, state);
> >>  +
> >>      return 0;
> >>   }
> >>
> >>  @@ -531,6 +540,14 @@ static void ingenic_drm_update_palette(struct
> >> ingenic_drm *priv,
> >>      }
> >>   }
> >>
> >>  +void ingenic_drm_sync_data(struct device *dev,
> >>  +                      struct drm_plane_state *old_state,
> >>  +                      struct drm_plane_state *state)
> >>  +{
> >>  +   if (ingenic_drm_cached_gem_buf)
> >>  +           drm_gem_cma_sync_data(dev, old_state, state);
> >>  +}
> >>  +
> >>   static void ingenic_drm_plane_atomic_update(struct drm_plane
> >> *plane,
> >>                                          struct drm_plane_state *oldstate)
> >>   {
> >>  @@ -543,6 +560,8 @@ static void
> >> ingenic_drm_plane_atomic_update(struct drm_plane *plane,
> >>      u32 fourcc;
> >>
> >>      if (state && state->fb) {
> >>  +           ingenic_drm_sync_data(priv->dev, oldstate, state);
> >>  +
> >>              crtc_state = state->crtc->state;
> >>
> >>              addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> >>  @@ -714,7 +733,36 @@ static void ingenic_drm_disable_vblank(struct
> >> drm_crtc *crtc)
> >>      regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
> >> JZ_LCD_CTRL_EOF_IRQ, 0);
> >>   }
> >>
> >>  -DEFINE_DRM_GEM_CMA_FOPS(ingenic_drm_fops);
> >>  +static struct drm_framebuffer *
> >>  +ingenic_drm_gem_fb_create(struct drm_device *dev, struct drm_file
> >> *file,
> >>  +                     const struct drm_mode_fb_cmd2 *mode_cmd)
> >>  +{
> >>  +   if (ingenic_drm_cached_gem_buf)
> >>  +           return drm_gem_fb_create_with_dirty(dev, file, mode_cmd);
> >>  +
> >>  +   return drm_gem_fb_create(dev, file, mode_cmd);
> >>  +}
> >>  +
> >>  +static int ingenic_drm_gem_cma_mmap(struct file *filp,
> >>  +                               struct vm_area_struct *vma)
> >>  +{
> >>  +   if (ingenic_drm_cached_gem_buf)
> >>  +           return drm_gem_cma_mmap_noncoherent(filp, vma);
> >>  +
> >>  +   return drm_gem_cma_mmap(filp, vma);
> >>  +}
> >>  +
> >>  +static const struct file_operations ingenic_drm_fops = {
> >>  +   .owner          = THIS_MODULE,
> >>  +   .open           = drm_open,
> >>  +   .release        = drm_release,
> >>  +   .unlocked_ioctl = drm_ioctl,
> >>  +   .compat_ioctl   = drm_compat_ioctl,
> >>  +   .poll           = drm_poll,
> >>  +   .read           = drm_read,
> >>  +   .llseek         = noop_llseek,
> >>  +   .mmap           = ingenic_drm_gem_cma_mmap,
> >>  +};
> >>
> >>   static struct drm_driver ingenic_drm_driver_data = {
> >>      .driver_features        = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> >>  @@ -726,7 +774,7 @@ static struct drm_driver
> >> ingenic_drm_driver_data = {
> >>      .patchlevel             = 0,
> >>
> >>      .fops                   = &ingenic_drm_fops,
> >>  -   DRM_GEM_CMA_DRIVER_OPS,
> >>
> >> +    DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE(drm_gem_cma_dumb_create_noncoherent),
> >>
> >>      .irq_handler            = ingenic_drm_irq_handler,
> >>   };
> >>  @@ -778,7 +826,7 @@ static const struct drm_encoder_helper_funcs
> >> ingenic_drm_encoder_helper_funcs =
> >>   };
> >>
> >>   static const struct drm_mode_config_funcs
> >> ingenic_drm_mode_config_funcs = {
> >>  -   .fb_create              = drm_gem_fb_create,
> >>  +   .fb_create              = ingenic_drm_gem_fb_create,
> >>      .output_poll_changed    = drm_fb_helper_output_poll_changed,
> >>      .atomic_check           = drm_atomic_helper_check,
> >>      .atomic_commit          = drm_atomic_helper_commit,
> >>  @@ -932,6 +980,8 @@ static int ingenic_drm_bind(struct device *dev,
> >> bool has_components)
> >>              return ret;
> >>      }
> >>
> >>  +   drm_plane_enable_fb_damage_clips(&priv->f1);
> >>  +
> >>      drm_crtc_helper_add(&priv->crtc, &ingenic_drm_crtc_helper_funcs);
> >>
> >>      ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->f1,
> >>  @@ -960,6 +1010,8 @@ static int ingenic_drm_bind(struct device
> >> *dev, bool has_components)
> >>                      return ret;
> >>              }
> >>
> >>  +           drm_plane_enable_fb_damage_clips(&priv->f0);
> >>  +
> >>              if (IS_ENABLED(CONFIG_DRM_INGENIC_IPU) && has_components) {
> >>                      ret = component_bind_all(dev, drm);
> >>                      if (ret) {
> >>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h
> >> b/drivers/gpu/drm/ingenic/ingenic-drm.h
> >>  index 9b48ce02803d..ee3a892c0383 100644
> >>  --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
> >>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
> >>  @@ -171,6 +171,10 @@ void ingenic_drm_plane_config(struct device
> >> *dev,
> >>                            struct drm_plane *plane, u32 fourcc);
> >>   void ingenic_drm_plane_disable(struct device *dev, struct
> >> drm_plane *plane);
> >>
> >>  +void ingenic_drm_sync_data(struct device *dev,
> >>  +                      struct drm_plane_state *old_state,
> >>  +                      struct drm_plane_state *state);
> >>  +
> >>   extern struct platform_driver *ingenic_ipu_driver_ptr;
> >>
> >>   #endif /* DRIVERS_GPU_DRM_INGENIC_INGENIC_DRM_H */
> >>  diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> >> b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> >>  index fc8c6e970ee3..38c83e8cc6a5 100644
> >>  --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> >>  +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> >>  @@ -20,6 +20,7 @@
> >>
> >>   #include <drm/drm_atomic.h>
> >>   #include <drm/drm_atomic_helper.h>
> >>  +#include <drm/drm_damage_helper.h>
> >>   #include <drm/drm_drv.h>
> >>   #include <drm/drm_fb_cma_helper.h>
> >>   #include <drm/drm_fourcc.h>
> >>  @@ -316,6 +317,8 @@ static void
> >> ingenic_ipu_plane_atomic_update(struct drm_plane *plane,
> >>                              JZ_IPU_CTRL_CHIP_EN | JZ_IPU_CTRL_LCDC_SEL);
> >>      }
> >>
> >>  +   ingenic_drm_sync_data(ipu->master, oldstate, state);
> >>  +
> >>      /* New addresses will be committed in vblank handler... */
> >>      ipu->addr_y = drm_fb_cma_get_gem_addr(state->fb, state, 0);
> >>      if (finfo->num_planes > 1)
> >>  @@ -534,7 +537,7 @@ static int
> >> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
> >>
> >>      if (!state->crtc ||
> >>          !crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
> >>  -           return 0;
> >>  +           goto out_check_damage;
> >>
> >>      /* Plane must be fully visible */
> >>      if (state->crtc_x < 0 || state->crtc_y < 0 ||
> >>  @@ -551,7 +554,7 @@ static int
> >> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
> >>              return -EINVAL;
> >>
> >>      if (!osd_changed(state, plane->state))
> >>  -           return 0;
> >>  +           goto out_check_damage;
> >>
> >>      crtc_state->mode_changed = true;
> >>
> >>  @@ -578,6 +581,9 @@ static int
> >> ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
> >>      ipu->denom_w = denom_w;
> >>      ipu->denom_h = denom_h;
> >>
> >>  +out_check_damage:
> >>  +   drm_atomic_helper_check_plane_damage(state->state, state);
> >>  +
> >>      return 0;
> >>   }
> >>
> >>  @@ -759,6 +765,8 @@ static int ingenic_ipu_bind(struct device *dev,
> >> struct device *master, void *d)
> >>              return err;
> >>      }
> >>
> >>  +   drm_plane_enable_fb_damage_clips(plane);
> >>  +
> >>      /*
> >>       * Sharpness settings range is [0,32]
> >>       * 0       : nearest-neighbor
> >>  --
> >>  2.28.0
> >>
> >
> > --
> > 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
https://lists.freedesktop.org/mailman/listinfo/dri-devel