[PATCH v15 00/19] drm/etnaviv: Add driver wrapper for vivante GPUs attached on PCI(e) device

[PATCH v15 00/19] drm/etnaviv: Add driver wrapper for vivante GPUs attached on PCI(e) device

[Sui Jingfeng]

drm/etnaviv use the component framework to bind multiple GPU cores to a
virtual master, the virtual master is manually create during driver load
time. This may works well for SoCs, yet there are some PCIe card has the
vivante GPU cores integrated. The driver lacks support for PCIe devices
currently.

Adds PCIe driver wrapper on the top of drm/etnaviv, the component
framework is still being used to bind subdevices, even though there is
only one GPU core. But the process is going to be reversed, we create
virtual platform device for each of the vivante GPU IP core that is
shipped by the PCIe card. Select the PCIe device as parent, generate a
virtual platform device as component master to take over the bind actions.

Sui Jingfeng (19):
  drm/etnaviv: Implement drm_gem_object_funcs::print_info()
  drm/etnaviv: Export drm_gem_print_info() and use it
  drm/etnaviv: Implement drm_gem_object_funcs::vunmap()
  drm/etnaviv: Make etnaviv_gem_prime_vmap() a static function
  drm/etnaviv: Add contructor and destructor for etnaviv_gem_get_mapping
    structure
  drm/etnaviv: Prefer drm_device based drm_WARN_ON() over regular
    WARN_ON()
  drm/etnaviv: Add a dedicated helper function to get various clocks
  drm/etnaviv: Fix wrong caching mode being used for non writecombine
    buffers
  drm/etnaviv: Add constructor and destructor for the
    etnaviv_drm_private structure
  drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private
  drm/etnaviv: Add etnaviv_gem_obj_remove() helper
  drm/etnaviv: Add support for cached coherent caching mode
  drm/etnaviv: Add support for vivante GPU cores attached via PCIe
    device
  drm/etnaviv: Add PCIe IP setup code
  drm/etnaviv: Make more use of the etnaviv_gem_new_private() function
  drm/etnaviv: Call etnaviv_gem_obj_add() in ernaviv_gem_new_private()
  drm/etnaviv: Support to manage dedicated VRAM base on drm_mm
  drm/etnaviv: Allow userspace specify the domain of etnaviv GEM buffer
    object
  drm/etnaviv: Expose basic sanity tests via debugfs

v10:
	* Add one more cleanup patch
	* Resolve the conflict with a patch from Rob
	* Make the dummy PCI stub inlined
	* Print only if the platform is dma-coherrent
V11:
	* Process reviews since V10.
	* Provide a side by side implement

V12:
	* Create a virtual platform device for the subcomponent GPU cores
	* Bind all subordinate GPU cores to the real PCI master via component.

V13:
	* Drop the non-component code path, always use the component framework
	  to bind subcomponent GPU core. Even though there is only one core.
	* Defer the irq handler register.
	* Rebase and improve the commit message

V14:
	* Rebase onto etnaviv-next and improve commit message.

V15:
	* Plug in a drm-mm based dedicated VRAM range allocator.

 drivers/gpu/drm/drm_gem.c                    |   1 +
 drivers/gpu/drm/etnaviv/Kconfig              |   9 +
 drivers/gpu/drm/etnaviv/Makefile             |   5 +
 drivers/gpu/drm/etnaviv/etnaviv_debugfs.c    | 118 +++++++++
 drivers/gpu/drm/etnaviv/etnaviv_debugfs.h    |  15 ++
 drivers/gpu/drm/etnaviv/etnaviv_drv.c        | 183 +++++++++----
 drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  40 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gem.c        | 224 ++++++++++++----
 drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  21 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  31 +--
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |   2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c   | 264 +++++++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h   |  14 +
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 136 +++++++---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h        |   4 +
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c        |  11 +-
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c    | 217 +++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h    |  63 +++++
 drivers/gpu/drm/etnaviv/pcie_ip_setup.c      | 109 ++++++++
 include/drm/drm_gem.h                        |   2 +
 include/uapi/drm/etnaviv_drm.h               |  13 +
 21 files changed, 1308 insertions(+), 174 deletions(-)
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_debugfs.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_debugfs.h
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
 create mode 100644 drivers/gpu/drm/etnaviv/pcie_ip_setup.c

-- 
2.43.0

[PATCH v15 01/19] drm/etnaviv: Implement drm_gem_object_funcs::print_info()

[Sui Jingfeng]

It will be called by drm_gem_print_info() if implemented, and it can
provide more information about the framebuffer objects.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++++++++++++++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 4247a10f8d4f..543d881585b3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -534,8 +534,40 @@ static const struct vm_operations_struct vm_ops = {
 	.close = drm_gem_vm_close,
 };
 
+static const char *etnaviv_gem_obj_caching_info(u32 flags)
+{
+	switch (flags & ETNA_BO_CACHE_MASK) {
+	case ETNA_BO_CACHED:
+		return "cached";
+	case ETNA_BO_UNCACHED:
+		return "uncached";
+	case ETNA_BO_WC:
+		return "write-combine";
+	default:
+		break;
+	}
+
+	return "unknown";
+}
+
+static void etnaviv_gem_object_info(struct drm_printer *p,
+				    unsigned int indent,
+				    const struct drm_gem_object *obj)
+{
+	const struct etnaviv_gem_object *etnaviv_obj;
+
+	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
+
+	drm_printf_indent(p, indent, "caching mode=%s\n",
+			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
+	drm_printf_indent(p, indent, "active=%s\n",
+			  str_yes_no(is_active(etnaviv_obj)));
+	drm_printf_indent(p, indent, "vaddr=%p\n", etnaviv_obj->vaddr);
+}
+
 static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
 	.free = etnaviv_gem_free_object,
+	.print_info = etnaviv_gem_object_info,
 	.pin = etnaviv_gem_prime_pin,
 	.unpin = etnaviv_gem_prime_unpin,
 	.get_sg_table = etnaviv_gem_prime_get_sg_table,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index a42d260cac2c..3f8fe19a77cc 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -68,7 +68,7 @@ struct etnaviv_gem_ops {
 	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
 };
 
-static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
+static inline bool is_active(const struct etnaviv_gem_object *etnaviv_obj)
 {
 	return atomic_read(&etnaviv_obj->gpu_active) != 0;
 }
-- 
2.43.0

[PATCH v15 02/19] drm/etnaviv: Export drm_gem_print_info() and use it

[Sui Jingfeng]

This will make the newly implemented etnaviv_gem_object_funcs::print_info
get in use, which improves code sharing and simplifies debugfs. Achieve
better humen readability for debug log.

Use container_of_const() if 'struct etnaviv_gem_object *etnaviv_obj' is a
constant pointer.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/drm_gem.c             |  1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 13 +++++--------
 include/drm/drm_gem.h                 |  2 ++
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index d4bbc5d109c8..9c5c971c1b23 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1160,6 +1160,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
 	if (obj->funcs->print_info)
 		obj->funcs->print_info(p, indent, obj);
 }
+EXPORT_SYMBOL(drm_gem_print_info);
 
 int drm_gem_pin_locked(struct drm_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 543d881585b3..6bdf72cd9e85 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2015-2018 Etnaviv Project
  */
 
+#include <drm/drm_gem.h>
 #include <drm/drm_prime.h>
 #include <linux/dma-mapping.h>
 #include <linux/shmem_fs.h>
@@ -432,15 +433,11 @@ int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
 #ifdef CONFIG_DEBUG_FS
 static void etnaviv_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
 {
-	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+	struct drm_printer p = drm_seq_file_printer(m);
 	struct dma_resv *robj = obj->resv;
-	unsigned long off = drm_vma_node_start(&obj->vma_node);
 	int r;
 
-	seq_printf(m, "%08x: %c %2d (%2d) %08lx %p %zd\n",
-			etnaviv_obj->flags, is_active(etnaviv_obj) ? 'A' : 'I',
-			obj->name, kref_read(&obj->refcount),
-			off, etnaviv_obj->vaddr, obj->size);
+	drm_gem_print_info(&p, 1, obj);
 
 	r = dma_resv_lock(robj, NULL);
 	if (r)
@@ -461,7 +458,7 @@ void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
 	list_for_each_entry(etnaviv_obj, &priv->gem_list, gem_node) {
 		struct drm_gem_object *obj = &etnaviv_obj->base;
 
-		seq_puts(m, "   ");
+		seq_printf(m, "obj[%d]:\n", count);
 		etnaviv_gem_describe(obj, m);
 		count++;
 		size += obj->size;
@@ -556,7 +553,7 @@ static void etnaviv_gem_object_info(struct drm_printer *p,
 {
 	const struct etnaviv_gem_object *etnaviv_obj;
 
-	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
+	etnaviv_obj = container_of_const(obj, struct etnaviv_gem_object, base);
 
 	drm_printf_indent(p, indent, "caching mode=%s\n",
 			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index bae4865b2101..0791566fab53 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -480,6 +480,8 @@ void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 		     struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
+			const struct drm_gem_object *obj);
 
 /**
  * drm_gem_object_get - acquire a GEM buffer object reference
-- 
2.43.0

[PATCH v15 03/19] drm/etnaviv: Implement drm_gem_object_funcs::vunmap()

[Sui Jingfeng]

The vunmap() can be used to release virtual mapping obtained by vmap(),
While the vmap() is used to make a long duration mapping of multiple
physical pages into a contiguous virtual space.

Make the implementation-specific vunmap() operation untangled with the
etnaviv_gem_xxx_release() function. As then, the etnaviv_gem_xxx_release()
only need to responsible for the release page works.

The etnaviv_gem_vunmap() is added for driver internal usa case, where no
DRM GEM framework is involved.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 38 ++++++++++++++++++++-
 drivers/gpu/drm/etnaviv/etnaviv_gem.h       |  1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 ++++---
 4 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index b3eb1662e90c..2eb2ff13f6e8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -61,6 +61,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
 void etnaviv_gem_prime_unpin(struct drm_gem_object *obj);
 void *etnaviv_gem_vmap(struct drm_gem_object *obj);
+void etnaviv_gem_vunmap(struct drm_gem_object *obj);
 int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
 		struct drm_etnaviv_timespec *timeout);
 int etnaviv_gem_cpu_fini(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 6bdf72cd9e85..fad23494d08e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -340,6 +340,25 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
 	return etnaviv_obj->vaddr;
 }
 
+void etnaviv_gem_vunmap(struct drm_gem_object *obj)
+{
+	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+
+	if (!etnaviv_obj->vaddr)
+		return;
+
+	mutex_lock(&etnaviv_obj->lock);
+	etnaviv_obj->ops->vunmap(etnaviv_obj);
+	etnaviv_obj->vaddr = NULL;
+	mutex_unlock(&etnaviv_obj->lock);
+}
+
+static void etnaviv_gem_object_vunmap(struct drm_gem_object *obj,
+				      struct iosys_map *map)
+{
+	etnaviv_gem_vunmap(obj);
+}
+
 static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
 {
 	struct page **pages;
@@ -471,14 +490,21 @@ void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
 
 static void etnaviv_gem_shmem_release(struct etnaviv_gem_object *etnaviv_obj)
 {
-	vunmap(etnaviv_obj->vaddr);
 	put_pages(etnaviv_obj);
 }
 
+static void etnaviv_gem_shmem_vunmap(struct etnaviv_gem_object *etnaviv_obj)
+{
+	lockdep_assert_held(&etnaviv_obj->lock);
+
+	vunmap(etnaviv_obj->vaddr);
+}
+
 static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
 	.get_pages = etnaviv_gem_shmem_get_pages,
 	.release = etnaviv_gem_shmem_release,
 	.vmap = etnaviv_gem_vmap_impl,
+	.vunmap = etnaviv_gem_shmem_vunmap,
 	.mmap = etnaviv_gem_mmap_obj,
 };
 
@@ -508,6 +534,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 		kfree(mapping);
 	}
 
+	etnaviv_obj->ops->vunmap(etnaviv_obj);
 	etnaviv_obj->ops->release(etnaviv_obj);
 	drm_gem_object_release(obj);
 
@@ -569,6 +596,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
 	.unpin = etnaviv_gem_prime_unpin,
 	.get_sg_table = etnaviv_gem_prime_get_sg_table,
 	.vmap = etnaviv_gem_prime_vmap,
+	.vunmap = etnaviv_gem_object_vunmap,
 	.mmap = etnaviv_gem_mmap,
 	.vm_ops = &vm_ops,
 };
@@ -723,6 +751,13 @@ static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj)
 	}
 }
 
+static void etnaviv_gem_userptr_vunmap(struct etnaviv_gem_object *etnaviv_obj)
+{
+	lockdep_assert_held(&etnaviv_obj->lock);
+
+	vunmap(etnaviv_obj->vaddr);
+}
+
 static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 		struct vm_area_struct *vma)
 {
@@ -733,6 +768,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_userptr_ops = {
 	.get_pages = etnaviv_gem_userptr_get_pages,
 	.release = etnaviv_gem_userptr_release,
 	.vmap = etnaviv_gem_vmap_impl,
+	.vunmap = etnaviv_gem_userptr_vunmap,
 	.mmap = etnaviv_gem_userptr_mmap_obj,
 };
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 3f8fe19a77cc..d4965de3007c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -65,6 +65,7 @@ struct etnaviv_gem_ops {
 	int (*get_pages)(struct etnaviv_gem_object *);
 	void (*release)(struct etnaviv_gem_object *);
 	void *(*vmap)(struct etnaviv_gem_object *);
+	void (*vunmap)(struct etnaviv_gem_object *);
 	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
 };
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 6b98200068e4..bea50d720450 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -62,11 +62,6 @@ void etnaviv_gem_prime_unpin(struct drm_gem_object *obj)
 
 static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)
 {
-	struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
-
-	if (etnaviv_obj->vaddr)
-		dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
-
 	/* Don't drop the pages for imported dmabuf, as they are not
 	 * ours, just free the array we allocated:
 	 */
@@ -88,6 +83,13 @@ static void *etnaviv_gem_prime_vmap_impl(struct etnaviv_gem_object *etnaviv_obj)
 	return map.vaddr;
 }
 
+static void etnaviv_gem_prime_vunmap(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
+
+	dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
+}
+
 static int etnaviv_gem_prime_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
 		struct vm_area_struct *vma)
 {
@@ -106,6 +108,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
 	/* .get_pages should never be called */
 	.release = etnaviv_gem_prime_release,
 	.vmap = etnaviv_gem_prime_vmap_impl,
+	.vunmap = etnaviv_gem_prime_vunmap,
 	.mmap = etnaviv_gem_prime_mmap_obj,
 };
 
-- 
2.43.0

[PATCH v15 04/19] drm/etnaviv: Make etnaviv_gem_prime_vmap() a static function

[Sui Jingfeng]

The etnaviv_gem_prime_vmap() function has no caller in the
etnaviv_gem_prime.c file, move it into etnaviv_gem.c file.
While at it, rename it as etnaviv_gem_object_vmap(), since
it is a intermidiate layer function, it has no direct relation
ship with the PRIME. The etnaviv_gem_prime.c file already has
etnaviv_gem_prime_vmap_impl() as the implementation to vmap
a imported GEM buffer object.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  1 -
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 16 +++++++++++++++-
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 12 ------------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 2eb2ff13f6e8..c217b54b214c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -55,7 +55,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 
 int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
 struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
-int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map);
 struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 	struct dma_buf_attachment *attach, struct sg_table *sg);
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index fad23494d08e..85d4e7c87a6a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -6,6 +6,7 @@
 #include <drm/drm_gem.h>
 #include <drm/drm_prime.h>
 #include <linux/dma-mapping.h>
+#include <linux/iosys-map.h>
 #include <linux/shmem_fs.h>
 #include <linux/spinlock.h>
 #include <linux/vmalloc.h>
@@ -340,6 +341,19 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
 	return etnaviv_obj->vaddr;
 }
 
+static int etnaviv_gem_object_vmap(struct drm_gem_object *obj,
+				   struct iosys_map *map)
+{
+	void *vaddr;
+
+	vaddr = etnaviv_gem_vmap(obj);
+	if (!vaddr)
+		return -ENOMEM;
+	iosys_map_set_vaddr(map, vaddr);
+
+	return 0;
+}
+
 void etnaviv_gem_vunmap(struct drm_gem_object *obj)
 {
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
@@ -595,7 +609,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
 	.pin = etnaviv_gem_prime_pin,
 	.unpin = etnaviv_gem_prime_unpin,
 	.get_sg_table = etnaviv_gem_prime_get_sg_table,
-	.vmap = etnaviv_gem_prime_vmap,
+	.vmap = etnaviv_gem_object_vmap,
 	.vunmap = etnaviv_gem_object_vunmap,
 	.mmap = etnaviv_gem_mmap,
 	.vm_ops = &vm_ops,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index bea50d720450..8f523cbee60a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -25,18 +25,6 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj)
 	return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages);
 }
 
-int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
-{
-	void *vaddr;
-
-	vaddr = etnaviv_gem_vmap(obj);
-	if (!vaddr)
-		return -ENOMEM;
-	iosys_map_set_vaddr(map, vaddr);
-
-	return 0;
-}
-
 int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
 {
 	if (!obj->import_attach) {
-- 
2.43.0

[PATCH v15 05/19] drm/etnaviv: Add contructor and destructor for etnaviv_gem_get_mapping structure

[Sui Jingfeng]

Because this make the code more easier to understand, When GPU access the
VRAM, it will allocate a new mapping to use if there don't have one.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 40 +++++++++++++++++++--------
 drivers/gpu/drm/etnaviv/etnaviv_gem.h |  6 ++++
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 85d4e7c87a6a..55004fa9fabd 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -227,6 +227,30 @@ etnaviv_gem_get_vram_mapping(struct etnaviv_gem_object *obj,
 	return NULL;
 }
 
+static struct etnaviv_vram_mapping *
+etnaviv_gem_vram_mapping_new(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct etnaviv_vram_mapping *mapping;
+
+	mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
+	if (!mapping)
+		return NULL;
+
+	INIT_LIST_HEAD(&mapping->scan_node);
+	mapping->object = etnaviv_obj;
+	mapping->use = 1;
+
+	list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
+
+	return mapping;
+}
+
+static void etnaviv_gem_vram_mapping_destroy(struct etnaviv_vram_mapping *mapping)
+{
+	list_del(&mapping->obj_node);
+	kfree(mapping);
+}
+
 void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
 {
 	struct etnaviv_gem_object *etnaviv_obj = mapping->object;
@@ -289,27 +313,20 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
 	 */
 	mapping = etnaviv_gem_get_vram_mapping(etnaviv_obj, NULL);
 	if (!mapping) {
-		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
+		mapping = etnaviv_gem_vram_mapping_new(etnaviv_obj);
 		if (!mapping) {
 			ret = -ENOMEM;
 			goto out;
 		}
-
-		INIT_LIST_HEAD(&mapping->scan_node);
-		mapping->object = etnaviv_obj;
 	} else {
-		list_del(&mapping->obj_node);
+		mapping->use = 1;
 	}
 
-	mapping->use = 1;
-
 	ret = etnaviv_iommu_map_gem(mmu_context, etnaviv_obj,
 				    mmu_context->global->memory_base,
 				    mapping, va);
 	if (ret < 0)
-		kfree(mapping);
-	else
-		list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
+		etnaviv_gem_vram_mapping_destroy(mapping);
 
 out:
 	mutex_unlock(&etnaviv_obj->lock);
@@ -544,8 +561,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 		if (context)
 			etnaviv_iommu_unmap_gem(context, mapping);
 
-		list_del(&mapping->obj_node);
-		kfree(mapping);
+		etnaviv_gem_vram_mapping_destroy(mapping);
 	}
 
 	etnaviv_obj->ops->vunmap(etnaviv_obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index d4965de3007c..f2ac64d8e90b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -31,6 +31,12 @@ struct etnaviv_vram_mapping {
 	u32 iova;
 };
 
+static inline struct etnaviv_vram_mapping *
+to_etnaviv_vram_mapping(struct drm_mm_node *vram)
+{
+	return container_of(vram, struct etnaviv_vram_mapping, vram_node);
+}
+
 struct etnaviv_gem_object {
 	struct drm_gem_object base;
 	const struct etnaviv_gem_ops *ops;
-- 
2.43.0

[PATCH v15 06/19] drm/etnaviv: Prefer drm_device based drm_WARN_ON() over regular WARN_ON()

[Sui Jingfeng]

drm_WARN_ON() acts like WARN_ON(), but with the key difference of
using device specific information so that we know from which device
the warning is originating from.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 9 +++++----
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c       | 6 +++---
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c       | 7 +++++--
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 55004fa9fabd..1fd2cff20ef4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -256,7 +256,7 @@ void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
 	struct etnaviv_gem_object *etnaviv_obj = mapping->object;
 
 	mutex_lock(&etnaviv_obj->lock);
-	WARN_ON(mapping->use == 0);
+	drm_WARN_ON(etnaviv_obj->base.dev, mapping->use == 0);
 	mapping->use -= 1;
 	mutex_unlock(&etnaviv_obj->lock);
 
@@ -463,7 +463,7 @@ int etnaviv_gem_cpu_fini(struct drm_gem_object *obj)
 
 	if (etnaviv_obj->flags & ETNA_BO_CACHED) {
 		/* fini without a prep is almost certainly a userspace error */
-		WARN_ON(etnaviv_obj->last_cpu_prep_op == 0);
+		drm_WARN_ON(dev, etnaviv_obj->last_cpu_prep_op == 0);
 		dma_sync_sgtable_for_device(dev->dev, etnaviv_obj->sgt,
 			etnaviv_op_to_dma_dir(etnaviv_obj->last_cpu_prep_op));
 		etnaviv_obj->last_cpu_prep_op = 0;
@@ -541,12 +541,13 @@ static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
 
 void etnaviv_gem_free_object(struct drm_gem_object *obj)
 {
+	struct drm_device *drm = obj->dev;
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 	struct etnaviv_drm_private *priv = obj->dev->dev_private;
 	struct etnaviv_vram_mapping *mapping, *tmp;
 
 	/* object should not be active */
-	WARN_ON(is_active(etnaviv_obj));
+	drm_WARN_ON(drm, is_active(etnaviv_obj));
 
 	mutex_lock(&priv->gem_lock);
 	list_del(&etnaviv_obj->gem_node);
@@ -556,7 +557,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 				 obj_node) {
 		struct etnaviv_iommu_context *context = mapping->context;
 
-		WARN_ON(mapping->use);
+		drm_WARN_ON(drm, mapping->use);
 
 		if (context)
 			etnaviv_iommu_unmap_gem(context, mapping);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 8f523cbee60a..0062d808d6a9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -19,7 +19,7 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj)
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 	unsigned int npages = obj->size >> PAGE_SHIFT;
 
-	if (WARN_ON(!etnaviv_obj->pages))  /* should have already pinned! */
+	if (drm_WARN_ON(obj->dev, !etnaviv_obj->pages))  /* should have already pinned! */
 		return ERR_PTR(-EINVAL);
 
 	return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d0df5c53a829..4599dd995e11 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -686,7 +686,7 @@ static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
 	u16 prefetch;
 	u32 address;
 
-	WARN_ON(gpu->state != ETNA_GPU_STATE_INITIALIZED);
+	drm_WARN_ON(gpu->drm, gpu->state != ETNA_GPU_STATE_INITIALIZED);
 
 	/* setup the MMU */
 	etnaviv_iommu_restore(gpu, context);
@@ -734,8 +734,8 @@ static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
 
 static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 {
-	WARN_ON(!(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
-		  gpu->state == ETNA_GPU_STATE_RESET));
+	drm_WARN_ON(gpu->drm, !(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
+				gpu->state == ETNA_GPU_STATE_RESET));
 
 	if ((etnaviv_is_model_rev(gpu, 0x320, 0x5007) ||
 	     etnaviv_is_model_rev(gpu, 0x320, 0x5220)) &&
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index 02d9408d41bc..ed6c42384856 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -138,9 +138,10 @@ static void etnaviv_iommu_remove_mapping(struct etnaviv_iommu_context *context,
 void etnaviv_iommu_reap_mapping(struct etnaviv_vram_mapping *mapping)
 {
 	struct etnaviv_iommu_context *context = mapping->context;
+	struct drm_device *drm = dev_get_drvdata(context->global->dev);
 
 	lockdep_assert_held(&context->lock);
-	WARN_ON(mapping->use);
+	drm_WARN_ON(drm, mapping->use);
 
 	etnaviv_iommu_remove_mapping(context, mapping);
 	etnaviv_iommu_context_put(mapping->context);
@@ -333,7 +334,9 @@ int etnaviv_iommu_map_gem(struct etnaviv_iommu_context *context,
 void etnaviv_iommu_unmap_gem(struct etnaviv_iommu_context *context,
 	struct etnaviv_vram_mapping *mapping)
 {
-	WARN_ON(mapping->use);
+	struct drm_device *drm = dev_get_drvdata(context->global->dev);
+
+	drm_WARN_ON(drm, mapping->use);
 
 	mutex_lock(&context->lock);
 
-- 
2.43.0

[PATCH v15 07/19] drm/etnaviv: Add a dedicated helper function to get various clocks

[Sui Jingfeng]

Because the current implementation is DT-based, this only works when the
host platform has the DT support. The problem is that some host platforms
does not provide DT-based clocks drivers, as a result, the driver rage
quit.

PLL hardwares are typically provided by the host platform, which is part
of the entire clock tree. The PLL hardware provide clock pulse to the GPU
core, but it's not belong to the GPU corei itself. PLL registers can be
manipulated directly by the device driver. Hence, it may need dedicated
clock driver.

Add a the etnaviv_gpu_clk_get() function to group similar code blocks,
which make it easier to call this function on the platform where it works.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 53 ++++++++++++++++-----------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 4599dd995e11..eca6a06e9ade 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1609,6 +1609,35 @@ static irqreturn_t irq_handler(int irq, void *data)
 	return ret;
 }
 
+static int etnaviv_gpu_clk_get(struct etnaviv_gpu *gpu)
+{
+	struct device *dev = gpu->dev;
+
+	gpu->clk_reg = devm_clk_get_optional(dev, "reg");
+	DBG("clk_reg: %p", gpu->clk_reg);
+	if (IS_ERR(gpu->clk_reg))
+		return PTR_ERR(gpu->clk_reg);
+
+	gpu->clk_bus = devm_clk_get_optional(dev, "bus");
+	DBG("clk_bus: %p", gpu->clk_bus);
+	if (IS_ERR(gpu->clk_bus))
+		return PTR_ERR(gpu->clk_bus);
+
+	gpu->clk_core = devm_clk_get(dev, "core");
+	DBG("clk_core: %p", gpu->clk_core);
+	if (IS_ERR(gpu->clk_core))
+		return PTR_ERR(gpu->clk_core);
+	gpu->base_rate_core = clk_get_rate(gpu->clk_core);
+
+	gpu->clk_shader = devm_clk_get_optional(dev, "shader");
+	DBG("clk_shader: %p", gpu->clk_shader);
+	if (IS_ERR(gpu->clk_shader))
+		return PTR_ERR(gpu->clk_shader);
+	gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+
+	return 0;
+}
+
 static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
 {
 	int ret;
@@ -1884,27 +1913,9 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 	}
 
 	/* Get Clocks: */
-	gpu->clk_reg = devm_clk_get_optional(&pdev->dev, "reg");
-	DBG("clk_reg: %p", gpu->clk_reg);
-	if (IS_ERR(gpu->clk_reg))
-		return PTR_ERR(gpu->clk_reg);
-
-	gpu->clk_bus = devm_clk_get_optional(&pdev->dev, "bus");
-	DBG("clk_bus: %p", gpu->clk_bus);
-	if (IS_ERR(gpu->clk_bus))
-		return PTR_ERR(gpu->clk_bus);
-
-	gpu->clk_core = devm_clk_get(&pdev->dev, "core");
-	DBG("clk_core: %p", gpu->clk_core);
-	if (IS_ERR(gpu->clk_core))
-		return PTR_ERR(gpu->clk_core);
-	gpu->base_rate_core = clk_get_rate(gpu->clk_core);
-
-	gpu->clk_shader = devm_clk_get_optional(&pdev->dev, "shader");
-	DBG("clk_shader: %p", gpu->clk_shader);
-	if (IS_ERR(gpu->clk_shader))
-		return PTR_ERR(gpu->clk_shader);
-	gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+	err = etnaviv_gpu_clk_get(gpu);
+	if (err)
+		return err;
 
 	/* TODO: figure out max mapped size */
 	dev_set_drvdata(dev, gpu);
-- 
2.43.0

[PATCH v15 08/19] drm/etnaviv: Fix wrong caching mode being used for non writecombine buffers

[Sui Jingfeng]

In the etnaviv_gem_vmap_impl() function, the driver vmap whatever buffers
with write combine(WC) page property. This is incorrect, as some platforms
are cached coherent. Cached buffers should be mapped with cached page
property.

Fixes: a0a5ab3e99b8 ("drm/etnaviv: call correct function when trying to vmap a DMABUF")
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 1fd2cff20ef4..b899aea64e22 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -393,6 +393,7 @@ static void etnaviv_gem_object_vunmap(struct drm_gem_object *obj,
 static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
 {
 	struct page **pages;
+	pgprot_t prot;
 
 	lockdep_assert_held(&obj->lock);
 
@@ -400,8 +401,19 @@ static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
 	if (IS_ERR(pages))
 		return NULL;
 
-	return vmap(pages, obj->base.size >> PAGE_SHIFT,
-			VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+	switch (obj->flags) {
+	case ETNA_BO_CACHED:
+		prot = PAGE_KERNEL;
+		break;
+	case ETNA_BO_UNCACHED:
+		prot = pgprot_noncached(PAGE_KERNEL);
+		break;
+	case ETNA_BO_WC:
+	default:
+		prot = pgprot_writecombine(PAGE_KERNEL);
+	}
+
+	return vmap(pages, obj->base.size >> PAGE_SHIFT, VM_MAP, prot);
 }
 
 static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
-- 
2.43.0

[PATCH v15 09/19] drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure

[Sui Jingfeng]

Because there are a lot of data members in the struct etnaviv_drm_private,
which are intended to be shared by all GPU cores. It can be lengthy and
daunting on error handling, the 'gem_lock' of struct etnaviv_drm_private
just be forgeten to destroy on driver leave.

Switch to use the dedicated helpers introduced, etnaviv_bind() and
etnaviv_unbind() gets simplified. Another potential benefit is that
we could put the struct drm_device into struct etnaviv_drm_private
in the future, which made them share the same life time.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 73 +++++++++++++++++----------
 1 file changed, 46 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6591e420a051..809e5db85df4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,6 +41,45 @@ static struct device_node *etnaviv_of_first_available_node(void)
 	return NULL;
 }
 
+static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+{
+	struct etnaviv_drm_private *priv;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+
+	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
+
+	mutex_init(&priv->gem_lock);
+	INIT_LIST_HEAD(&priv->gem_list);
+	priv->num_gpus = 0;
+	priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
+
+	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
+	if (IS_ERR(priv->cmdbuf_suballoc)) {
+		kfree(priv);
+		dev_err(dev, "Failed to create cmdbuf suballocator\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return priv;
+}
+
+static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+{
+	if (!priv)
+		return;
+
+	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
+
+	xa_destroy(&priv->active_contexts);
+
+	mutex_destroy(&priv->gem_lock);
+
+	kfree(priv);
+}
+
 static void load_gpu(struct drm_device *dev)
 {
 	struct etnaviv_drm_private *priv = dev->dev_private;
@@ -521,35 +560,21 @@ static int etnaviv_bind(struct device *dev)
 	if (IS_ERR(drm))
 		return PTR_ERR(drm);
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		dev_err(dev, "failed to allocate private data\n");
-		ret = -ENOMEM;
+	priv = etnaviv_alloc_private(dev);
+	if (IS_ERR(priv)) {
+		ret = PTR_ERR(priv);
 		goto out_put;
 	}
+
 	drm->dev_private = priv;
 
 	dma_set_max_seg_size(dev, SZ_2G);
 
-	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
-
-	mutex_init(&priv->gem_lock);
-	INIT_LIST_HEAD(&priv->gem_list);
-	priv->num_gpus = 0;
-	priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
-
-	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
-	if (IS_ERR(priv->cmdbuf_suballoc)) {
-		dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
-		ret = PTR_ERR(priv->cmdbuf_suballoc);
-		goto out_free_priv;
-	}
-
 	dev_set_drvdata(dev, drm);
 
 	ret = component_bind_all(dev, drm);
 	if (ret < 0)
-		goto out_destroy_suballoc;
+		goto out_free_priv;
 
 	load_gpu(drm);
 
@@ -561,11 +586,8 @@ static int etnaviv_bind(struct device *dev)
 
 out_unbind:
 	component_unbind_all(dev, drm);
-out_destroy_suballoc:
-	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
 out_free_priv:
-	mutex_destroy(&priv->gem_lock);
-	kfree(priv);
+	etnaviv_free_private(priv);
 out_put:
 	drm_dev_put(drm);
 
@@ -581,12 +603,9 @@ static void etnaviv_unbind(struct device *dev)
 
 	component_unbind_all(dev, drm);
 
-	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
-
-	xa_destroy(&priv->active_contexts);
+	etnaviv_free_private(priv);
 
 	drm->dev_private = NULL;
-	kfree(priv);
 
 	drm_dev_put(drm);
 }
-- 
2.43.0

[PATCH v15 10/19] drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private

[Sui Jingfeng]

Both the instance of struct drm_device and the instance of struct
etnaviv_drm_private are intended to be shared by all GPU cores, both have
only one instance created across drm/etnaviv driver. After embedded in,
the whole structure can be allocated with devm_drm_dev_alloc(). And the
DRM device created is automatically put on driver detach, so we don't need
to call drm_dev_put() explicitly on driver leave. It's also eliminate the
need to use the .dev_private member, which is deprecated according to the
drm document. We can also use container_of() to retrieve pointer for the
containing structure.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c        | 65 ++++++++------------
 drivers/gpu/drm/etnaviv/etnaviv_drv.h        |  7 +++
 drivers/gpu/drm/etnaviv/etnaviv_gem.c        |  4 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c        |  6 +-
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c        |  4 +-
 6 files changed, 39 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 809e5db85df4..32ec1b5452ba 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,14 +41,9 @@ static struct device_node *etnaviv_of_first_available_node(void)
 	return NULL;
 }
 
-static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+static int etnaviv_private_init(struct device *dev,
+				struct etnaviv_drm_private *priv)
 {
-	struct etnaviv_drm_private *priv;
-
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return ERR_PTR(-ENOMEM);
-
 	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
 
 	mutex_init(&priv->gem_lock);
@@ -58,15 +53,14 @@ static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
 
 	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
 	if (IS_ERR(priv->cmdbuf_suballoc)) {
-		kfree(priv);
 		dev_err(dev, "Failed to create cmdbuf suballocator\n");
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	}
 
-	return priv;
+	return 0;
 }
 
-static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+static void etnaviv_private_fini(struct etnaviv_drm_private *priv)
 {
 	if (!priv)
 		return;
@@ -76,13 +70,11 @@ static void etnaviv_free_private(struct etnaviv_drm_private *priv)
 	xa_destroy(&priv->active_contexts);
 
 	mutex_destroy(&priv->gem_lock);
-
-	kfree(priv);
 }
 
 static void load_gpu(struct drm_device *dev)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	unsigned int i;
 
 	for (i = 0; i < ETNA_MAX_PIPES; i++) {
@@ -100,7 +92,7 @@ static void load_gpu(struct drm_device *dev)
 
 static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct etnaviv_file_private *ctx;
 	int ret, i;
 
@@ -143,7 +135,7 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 
 static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct etnaviv_file_private *ctx = file->driver_priv;
 	unsigned int i;
 
@@ -168,7 +160,7 @@ static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
 #ifdef CONFIG_DEBUG_FS
 static int etnaviv_gem_show(struct drm_device *dev, struct seq_file *m)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 
 	etnaviv_gem_describe_objects(priv, m);
 
@@ -262,7 +254,7 @@ static int show_each_gpu(struct seq_file *m, void *arg)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct etnaviv_gpu *gpu;
 	int (*show)(struct etnaviv_gpu *gpu, struct seq_file *m) =
 			node->info_ent->data;
@@ -305,7 +297,7 @@ static void etnaviv_debugfs_init(struct drm_minor *minor)
 static int etnaviv_ioctl_get_param(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_param *args = data;
 	struct etnaviv_gpu *gpu;
 
@@ -398,7 +390,7 @@ static int etnaviv_ioctl_wait_fence(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
 	struct drm_etnaviv_wait_fence *args = data;
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_timespec *timeout = &args->timeout;
 	struct etnaviv_gpu *gpu;
 
@@ -446,7 +438,7 @@ static int etnaviv_ioctl_gem_userptr(struct drm_device *dev, void *data,
 static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
 	struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_gem_wait *args = data;
 	struct drm_etnaviv_timespec *timeout = &args->timeout;
 	struct drm_gem_object *obj;
@@ -480,7 +472,7 @@ static int etnaviv_ioctl_gem_wait(struct drm_device *dev, void *data,
 static int etnaviv_ioctl_pm_query_dom(struct drm_device *dev, void *data,
 	struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_pm_domain *args = data;
 	struct etnaviv_gpu *gpu;
 
@@ -497,7 +489,7 @@ static int etnaviv_ioctl_pm_query_dom(struct drm_device *dev, void *data,
 static int etnaviv_ioctl_pm_query_sig(struct drm_device *dev, void *data,
 	struct drm_file *file)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_pm_signal *args = data;
 	struct etnaviv_gpu *gpu;
 
@@ -556,17 +548,14 @@ static int etnaviv_bind(struct device *dev)
 	struct drm_device *drm;
 	int ret;
 
-	drm = drm_dev_alloc(&etnaviv_drm_driver, dev);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
+	priv = devm_drm_dev_alloc(dev, &etnaviv_drm_driver,
+				  struct etnaviv_drm_private, drm);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
 
-	priv = etnaviv_alloc_private(dev);
-	if (IS_ERR(priv)) {
-		ret = PTR_ERR(priv);
-		goto out_put;
-	}
+	etnaviv_private_init(dev, priv);
 
-	drm->dev_private = priv;
+	drm = &priv->drm;
 
 	dma_set_max_seg_size(dev, SZ_2G);
 
@@ -587,9 +576,7 @@ static int etnaviv_bind(struct device *dev)
 out_unbind:
 	component_unbind_all(dev, drm);
 out_free_priv:
-	etnaviv_free_private(priv);
-out_put:
-	drm_dev_put(drm);
+	etnaviv_private_fini(priv);
 
 	return ret;
 }
@@ -597,17 +584,13 @@ static int etnaviv_bind(struct device *dev)
 static void etnaviv_unbind(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct etnaviv_drm_private *priv = drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(drm);
 
 	drm_dev_unregister(drm);
 
 	component_unbind_all(dev, drm);
 
-	etnaviv_free_private(priv);
-
-	drm->dev_private = NULL;
-
-	drm_dev_put(drm);
+	etnaviv_private_fini(priv);
 }
 
 static const struct component_master_ops etnaviv_master_ops = {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index c217b54b214c..e2a991160cb3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -35,6 +35,7 @@ struct etnaviv_file_private {
 };
 
 struct etnaviv_drm_private {
+	struct drm_device drm;
 	int num_gpus;
 	struct etnaviv_gpu *gpu[ETNA_MAX_PIPES];
 	gfp_t shm_gfp_mask;
@@ -50,6 +51,12 @@ struct etnaviv_drm_private {
 	struct list_head gem_list;
 };
 
+static inline struct etnaviv_drm_private *
+to_etnaviv_priv(struct drm_device *ddev)
+{
+	return container_of(ddev, struct etnaviv_drm_private, drm);
+}
+
 int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index b899aea64e22..39cfece67b90 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -587,7 +587,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 
 void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
 
 	mutex_lock(&priv->gem_lock);
@@ -687,7 +687,7 @@ static int etnaviv_gem_new_impl(struct drm_device *dev, u32 flags,
 int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 	u32 size, u32 flags, u32 *handle)
 {
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_gem_object *obj = NULL;
 	int ret;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 3c0a5c3e0e3d..b0ac0b5bd675 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -412,7 +412,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
 	struct etnaviv_file_private *ctx = file->driver_priv;
-	struct etnaviv_drm_private *priv = dev->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct drm_etnaviv_gem_submit *args = data;
 	struct drm_etnaviv_gem_submit_reloc *relocs;
 	struct drm_etnaviv_gem_submit_pmr *pmrs;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index eca6a06e9ade..0c8e394b569c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -38,7 +38,7 @@ static const struct platform_device_id gpu_ids[] = {
 
 int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
 {
-	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
 
 	switch (param) {
 	case ETNAVIV_PARAM_GPU_MODEL:
@@ -789,7 +789,7 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 
 int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 {
-	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
 	dma_addr_t cmdbuf_paddr;
 	int ret, i;
 
@@ -1783,7 +1783,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
 	void *data)
 {
 	struct drm_device *drm = data;
-	struct etnaviv_drm_private *priv = drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(drm);
 	struct etnaviv_gpu *gpu = dev_get_drvdata(dev);
 	int ret;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index ed6c42384856..5be0bb6a3f02 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -493,7 +493,7 @@ void etnaviv_iommu_dump(struct etnaviv_iommu_context *context, void *buf)
 int etnaviv_iommu_global_init(struct etnaviv_gpu *gpu)
 {
 	enum etnaviv_iommu_version version = ETNAVIV_IOMMU_V1;
-	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
 	struct etnaviv_iommu_global *global;
 	struct device *dev = gpu->drm->dev;
 
@@ -553,7 +553,7 @@ int etnaviv_iommu_global_init(struct etnaviv_gpu *gpu)
 
 void etnaviv_iommu_global_fini(struct etnaviv_gpu *gpu)
 {
-	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(gpu->drm);
 	struct etnaviv_iommu_global *global = priv->mmu_global;
 
 	if (!global)
-- 
2.43.0

[PATCH v15 11/19] drm/etnaviv: Add etnaviv_gem_obj_remove() helper

[Sui Jingfeng]

Which is corresonding to the etnaviv_gem_obj_add()

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 39cfece67b90..3732288ff530 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -19,6 +19,8 @@
 static struct lock_class_key etnaviv_shm_lock_class;
 static struct lock_class_key etnaviv_userptr_lock_class;
 
+static void etnaviv_gem_obj_remove(struct drm_gem_object *obj);
+
 static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj)
 {
 	struct drm_device *dev = etnaviv_obj->base.dev;
@@ -555,15 +557,12 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 {
 	struct drm_device *drm = obj->dev;
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
-	struct etnaviv_drm_private *priv = obj->dev->dev_private;
 	struct etnaviv_vram_mapping *mapping, *tmp;
 
 	/* object should not be active */
 	drm_WARN_ON(drm, is_active(etnaviv_obj));
 
-	mutex_lock(&priv->gem_lock);
-	list_del(&etnaviv_obj->gem_node);
-	mutex_unlock(&priv->gem_lock);
+	etnaviv_gem_obj_remove(obj);
 
 	list_for_each_entry_safe(mapping, tmp, &etnaviv_obj->vram_list,
 				 obj_node) {
@@ -595,6 +594,16 @@ void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
 	mutex_unlock(&priv->gem_lock);
 }
 
+static void etnaviv_gem_obj_remove(struct drm_gem_object *obj)
+{
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
+	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
+
+	mutex_lock(&priv->gem_lock);
+	list_del(&etnaviv_obj->gem_node);
+	mutex_unlock(&priv->gem_lock);
+}
+
 static const struct vm_operations_struct vm_ops = {
 	.fault = etnaviv_gem_fault,
 	.open = drm_gem_vm_open,
-- 
2.43.0

[PATCH v15 12/19] drm/etnaviv: Add support for cached coherent caching mode

[Sui Jingfeng]

Many modern CPUs and/or platforms choose to define their peripheral devices
as cached coherent by default, to be specific, the PCH is capable of
snooping CPU's cache. When hit, the peripheral devices will access data
directly from CPU's cache. This means that device drivers do not need to
maintain the coherency issue between a processor and peripheral I/O for
the cached buffers. Hence, it dosen't need us to sync manually on the
software side, which is useful to avoid some overheads, especially for
userspace, but userspace is not known this yet.

Probe the hardware maintained cached coherent support with the
dev_is_dma_coherent() function, and store the result in the
etnaviv_drm_private structure. Since this is a host and/or platform
implementation-defined hardware feature, and is again meant to be shared
by all GPU cores. The probe function can be extended in the future if it
not reflect the hardware perfectly.

Expose it via etnaviv parameter mechanism to let userspace know.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9 +++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4 ++++
 include/uapi/drm/etnaviv_drm.h        | 1 +
 4 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 32ec1b5452ba..18686b573d77 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -5,6 +5,7 @@
 
 #include <linux/component.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-map-ops.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -57,6 +58,8 @@ static int etnaviv_private_init(struct device *dev,
 		return -ENOMEM;
 	}
 
+	priv->cached_coherent = dev_is_dma_coherent(dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index e2a991160cb3..02d706b34752 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -46,6 +46,15 @@ struct etnaviv_drm_private {
 	struct xarray active_contexts;
 	u32 next_context_id;
 
+	/*
+	 * If true, the cached mapping is consistent for all CPU cores and
+	 * peripheral bus masters in the system. It means that both of the
+	 * CPU and GPU will see the same data if the buffer being accessed
+	 * is cached. And the coherency is guaranteed by the host platform
+	 * specific hardwares.
+	 */
+	bool cached_coherent;
+
 	/* list of GEM objects: */
 	struct mutex gem_lock;
 	struct list_head gem_list;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 0c8e394b569c..89fb36aee779 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -164,6 +164,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value)
 		*value = gpu->identity.eco_id;
 		break;
 
+	case ETNAVIV_PARAM_CACHED_COHERENT:
+		*value = priv->cached_coherent;
+		break;
+
 	default:
 		DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
 		return -EINVAL;
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index af024d90453d..61eaa8cd0f5e 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -77,6 +77,7 @@ struct drm_etnaviv_timespec {
 #define ETNAVIV_PARAM_GPU_PRODUCT_ID                0x1c
 #define ETNAVIV_PARAM_GPU_CUSTOMER_ID               0x1d
 #define ETNAVIV_PARAM_GPU_ECO_ID                    0x1e
+#define ETNAVIV_PARAM_CACHED_COHERENT               0x1f
 
 #define ETNA_MAX_PIPES 4
 
-- 
2.43.0

[PATCH v15 13/19] drm/etnaviv: Add support for vivante GPU cores attached via PCIe device

[Sui Jingfeng]

Previously, component framework is being used to bind multiple platform
(DT) devices to a virtual master. The virtual master is manually created
during probe time. This is fine and works well for embedded SoCs, yet
there have some hardware venders that integrate Vivante GPU IP cores
into their PCIe card. This driver lacks support for PCI(e) devices.

Creating platform device as logical denotation for each GPU IP core, the
manually created platform devices are functional as subcomponen. To
reflects the actual hardware layout, make all of them be child of the PCIe
master device. The master is real for PCIe devices, which is working at
background and spread the device seeds out.

Creating yet an another platform device as component master, offload the
component binding/unbinding affairs to it. The virtual component master
works at foreground, the PCIe device behind it works at background.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/Kconfig           |   9 ++
 drivers/gpu/drm/etnaviv/Makefile          |   2 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c     |  45 +++++-
 drivers/gpu/drm/etnaviv/etnaviv_drv.h     |  11 ++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c     |  73 +++++++--
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h     |   4 +
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 185 ++++++++++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h |  48 ++++++
 8 files changed, 355 insertions(+), 22 deletions(-)
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h

diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index faa7fc68b009..7cb44f72d512 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -15,6 +15,15 @@ config DRM_ETNAVIV
 	help
 	  DRM driver for Vivante GPUs.
 
+config DRM_ETNAVIV_PCI_DRIVER
+	bool "enable ETNAVIV PCI driver support"
+	depends on DRM_ETNAVIV
+	depends on PCI
+	default n
+	help
+	  Compile in support for Vivante GPUs attached via PCIe card.
+	  Say Y if you have such hardwares.
+
 config DRM_ETNAVIV_THERMAL
 	bool "enable ETNAVIV thermal throttling"
 	depends on DRM_ETNAVIV
diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 46e5ffad69a6..6829e1ebf2db 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -16,4 +16,6 @@ etnaviv-y := \
 	etnaviv_perfmon.o \
 	etnaviv_sched.o
 
+etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
+
 obj-$(CONFIG_DRM_ETNAVIV)	+= etnaviv.o
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 18686b573d77..7fc654f051a3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -24,6 +24,7 @@
 #include "etnaviv_gpu.h"
 #include "etnaviv_gem.h"
 #include "etnaviv_mmu.h"
+#include "etnaviv_pci_drv.h"
 #include "etnaviv_perfmon.h"
 
 /*
@@ -568,6 +569,10 @@ static int etnaviv_bind(struct device *dev)
 	if (ret < 0)
 		goto out_free_priv;
 
+	ret = etnaviv_register_irq_handler(dev, priv);
+	if (ret)
+		goto out_unbind;
+
 	load_gpu(drm);
 
 	ret = drm_dev_register(drm, 0);
@@ -670,16 +675,32 @@ static struct platform_driver etnaviv_platform_driver = {
 	},
 };
 
-static int etnaviv_create_platform_device(const char *name,
-					  struct platform_device **ppdev)
+int etnaviv_create_platform_device(struct device *parent,
+				   const char *name, int id,
+				   struct resource *pres,
+				   void *pdata, unsigned int ndata,
+				   struct platform_device **ppdev)
 {
 	struct platform_device *pdev;
 	int ret;
 
-	pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+	pdev = platform_device_alloc(name, id);
 	if (!pdev)
 		return -ENOMEM;
 
+	pdev->dev.parent = parent;
+
+	if (pres) {
+		ret = platform_device_add_resources(pdev, pres, 1);
+		if (ret) {
+			platform_device_put(pdev);
+			return ret;
+		}
+	}
+
+	if (pdata && ndata)
+		platform_device_add_data(pdev, pdata, ndata);
+
 	ret = platform_device_add(pdev);
 	if (ret) {
 		platform_device_put(pdev);
@@ -691,7 +712,7 @@ static int etnaviv_create_platform_device(const char *name,
 	return 0;
 }
 
-static void etnaviv_destroy_platform_device(struct platform_device **ppdev)
+void etnaviv_destroy_platform_device(struct platform_device **ppdev)
 {
 	struct platform_device *pdev = *ppdev;
 
@@ -703,7 +724,7 @@ static void etnaviv_destroy_platform_device(struct platform_device **ppdev)
 	*ppdev = NULL;
 }
 
-static struct platform_device *etnaviv_drm;
+struct platform_device *etnaviv_drm;
 
 static int __init etnaviv_init(void)
 {
@@ -720,6 +741,10 @@ static int __init etnaviv_init(void)
 	if (ret != 0)
 		goto unregister_gpu_driver;
 
+	ret = etnaviv_register_pci_driver();
+	if (ret)
+		goto unregister_platform_driver;
+
 	/*
 	 * If the DT contains at least one available GPU device, instantiate
 	 * the DRM platform device.
@@ -728,13 +753,18 @@ static int __init etnaviv_init(void)
 	if (np) {
 		of_node_put(np);
 
-		ret = etnaviv_create_platform_device("etnaviv", &etnaviv_drm);
+		ret = etnaviv_create_platform_device(NULL, "etnaviv",
+						     PLATFORM_DEVID_NONE,
+						     NULL, NULL, 0,
+						     &etnaviv_drm);
 		if (ret)
-			goto unregister_platform_driver;
+			goto unregister_pci_driver;
 	}
 
 	return 0;
 
+unregister_pci_driver:
+	etnaviv_unregister_pci_driver();
 unregister_platform_driver:
 	platform_driver_unregister(&etnaviv_platform_driver);
 unregister_gpu_driver:
@@ -746,6 +776,7 @@ module_init(etnaviv_init);
 static void __exit etnaviv_exit(void)
 {
 	etnaviv_destroy_platform_device(&etnaviv_drm);
+	etnaviv_unregister_pci_driver();
 	platform_driver_unregister(&etnaviv_platform_driver);
 	platform_driver_unregister(&etnaviv_gpu_driver);
 }
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 02d706b34752..84f2e79f0a53 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -9,6 +9,7 @@
 #include <linux/io.h>
 #include <linux/list.h>
 #include <linux/mm_types.h>
+#include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/time64.h>
 #include <linux/types.h>
@@ -26,6 +27,8 @@ struct etnaviv_gem_object;
 struct etnaviv_gem_submit;
 struct etnaviv_iommu_global;
 
+extern struct platform_device *etnaviv_drm;
+
 #define ETNAVIV_SOFTPIN_START_ADDRESS	SZ_4M /* must be >= SUBALLOC_SIZE */
 
 struct etnaviv_file_private {
@@ -98,6 +101,14 @@ bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu,
 	u32 *stream, unsigned int size,
 	struct drm_etnaviv_gem_submit_reloc *relocs, unsigned int reloc_size);
 
+int etnaviv_create_platform_device(struct device *parent,
+				   const char *name, int id,
+				   struct resource *pres,
+				   void *data, unsigned int ndata,
+				   struct platform_device **ppdev);
+
+void etnaviv_destroy_platform_device(struct platform_device **ppdev);
+
 #ifdef CONFIG_DEBUG_FS
 void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
 	struct seq_file *m);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 89fb36aee779..3cfeacc49489 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -10,6 +10,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
@@ -29,6 +30,7 @@
 
 static const struct platform_device_id gpu_ids[] = {
 	{ .name = "etnaviv-gpu,2d" },
+	{ .name = "etnaviv-gpu,3d" },
 	{ },
 };
 
@@ -1546,15 +1548,23 @@ static void dump_mmu_fault(struct etnaviv_gpu *gpu)
 
 static irqreturn_t irq_handler(int irq, void *data)
 {
-	struct etnaviv_gpu *gpu = data;
+	struct etnaviv_drm_private *priv = data;
 	irqreturn_t ret = IRQ_NONE;
+	int i;
 
-	u32 intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
-
-	if (intr != 0) {
+	for (i = 0; i < priv->num_gpus; i++) {
+		struct etnaviv_gpu *gpu = priv->gpu[i];
 		ktime_t now = ktime_get();
+		u32 intr;
 		int event;
 
+		if (!gpu)
+			continue;
+
+		intr = gpu_read(gpu, VIVS_HI_INTR_ACKNOWLEDGE);
+		if (!intr)
+			continue;
+
 		pm_runtime_mark_last_busy(gpu->dev);
 
 		dev_dbg(gpu->dev, "intr 0x%08x\n", intr);
@@ -1885,10 +1895,42 @@ static const struct of_device_id etnaviv_gpu_match[] = {
 };
 MODULE_DEVICE_TABLE(of, etnaviv_gpu_match);
 
+/*
+ * dev point to the component master.
+ */
+int etnaviv_register_irq_handler(struct device *dev,
+				 struct etnaviv_drm_private *priv)
+{
+	int ret = 0;
+
+	if (dev->parent && dev_is_pci(dev->parent)) {
+		struct pci_dev *pdev = to_pci_dev(dev->parent);
+
+		ret = request_irq(pdev->irq, irq_handler, IRQF_SHARED,
+				  dev_name(dev->parent), priv);
+	} else {
+		int i;
+
+		for (i = 0; i < priv->num_gpus; i++) {
+			struct etnaviv_gpu *gpu = priv->gpu[i];
+
+			ret = devm_request_irq(gpu->dev, gpu->irq, irq_handler,
+					       0, dev_name(dev), priv);
+			if (ret) {
+				dev_err(dev, "failed to request IRQ handler: %d\n", ret);
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
 static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct etnaviv_gpu *gpu;
+	bool is_pci;
 	int err;
 
 	gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
@@ -1904,22 +1946,23 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 	if (IS_ERR(gpu->mmio))
 		return PTR_ERR(gpu->mmio);
 
+	is_pci = dev->parent ? dev_is_pci(dev->parent) : false;
+
 	/* Get Interrupt: */
-	gpu->irq = platform_get_irq(pdev, 0);
+	if (is_pci)
+		gpu->irq = to_pci_dev(dev->parent)->irq;
+	else
+		gpu->irq = platform_get_irq(pdev, 0);
+
 	if (gpu->irq < 0)
 		return gpu->irq;
 
-	err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
-			       dev_name(dev), gpu);
-	if (err) {
-		dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
-		return err;
-	}
-
 	/* Get Clocks: */
-	err = etnaviv_gpu_clk_get(gpu);
-	if (err)
-		return err;
+	if (!is_pci) {
+		err = etnaviv_gpu_clk_get(gpu);
+		if (err)
+			return err;
+	}
 
 	/* TODO: figure out max mapped size */
 	dev_set_drvdata(dev, gpu);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 31322195b9e4..1162572843d9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -204,6 +204,10 @@ static inline u32 gpu_read_power(struct etnaviv_gpu *gpu, u32 reg)
 int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32 param, u64 *value);
 
 int etnaviv_gpu_init(struct etnaviv_gpu *gpu);
+
+int etnaviv_register_irq_handler(struct device *dev,
+				 struct etnaviv_drm_private *priv);
+
 bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu);
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
new file mode 100644
index 000000000000..f13f3208120f
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pci.h>
+
+#include "etnaviv_drv.h"
+#include "etnaviv_pci_drv.h"
+
+static const struct etnaviv_pci_gpu_data
+gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
+	{
+		.chip_id = GCCORE_PCI_CHIP_ID_UNKNOWN,
+	},
+	{
+		.chip_id = JM9100,
+		.num_core = 1,
+		.num_vram = 2,
+		.vram_bars = {0, 2},
+		.mmio_bar = 1,
+		.ip_block = {{0, 0x00900000, 0x00004000, "etnaviv-gpu,3d"},},
+		.has_dedicated_vram = true,
+		.has_display = true,
+		.market_name = "JingJia Micro JM9100",
+	},
+	{
+		.chip_id = JD9230P,
+		.num_core = 2,
+		.num_vram = 2,
+		.vram_bars = {0, 2},
+		.mmio_bar = 1,
+		.ip_block = {{0, 0x00900000, 0x00004000, "etnaviv-gpu,3d"},
+			     {1, 0x00910000, 0x00004000, "etnaviv-gpu,3d"},},
+		.has_dedicated_vram = true,
+		.has_display = true,
+		.market_name = "JingJia Micro JD9230P",
+	},
+	{
+		.chip_id = GP102,
+		.num_core = 2,
+		.num_vram = 1,
+		.vram_bars = {0,},
+		.mmio_bar = 2,
+		.ip_block = {{0, 0x00040000, 0x00004000, "etnaviv-gpu,3d"},
+			     {0, 0x000C0000, 0x00004000, "etnaviv-gpu,2d"},},
+		.has_dedicated_vram = true,
+		.has_display = true,
+		.market_name = "LingJiu GP102",
+	},
+	{
+		.chip_id = GC1000_IN_LS7A1000,
+		.num_core = 1,
+		.num_vram = 1,
+		.vram_bars = {2,},
+		.mmio_bar = 0,
+		.ip_block = {{0, 0, 0x00004000, "etnaviv-gpu,3d"},},
+		.has_dedicated_vram = true,
+		.has_display = false,
+		.market_name = "GC1000 in LS7A1000",
+	},
+};
+
+static const char *match_names[ETNA_MAX_IP_BLOCK] = {0};
+
+static const struct etnaviv_pci_gpu_data *
+etnaviv_pci_get_platform_data(const struct pci_device_id *entity)
+{
+	enum etnaviv_pci_chip_id chip_id = entity->driver_data;
+
+	if (chip_id == GCCORE_PCI_CHIP_ID_UNKNOWN ||
+	    chip_id >= GCCORE_PCI_CHIP_ID_LAST)
+		return NULL;
+
+	return &gccore_platform_data[chip_id];
+}
+
+static void platform_device_remove_callback(void *data)
+{
+	struct platform_device *pdev = (struct platform_device *)data;
+
+	etnaviv_destroy_platform_device(&pdev);
+}
+
+static int etnaviv_pci_probe(struct pci_dev *pdev,
+			     const struct pci_device_id *ent)
+{
+	const struct etnaviv_pci_gpu_data *pdata;
+	struct device *dev = &pdev->dev;
+	unsigned int i;
+	unsigned int num_core;
+	int ret;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
+	pdata = etnaviv_pci_get_platform_data(ent);
+	if (!pdata)
+		return -ENODEV;
+
+	num_core = pdata->num_core;
+
+	dev_info(dev, "%s has %u GPU cores\n", pdata->market_name, num_core);
+
+	for (i = 0; i < num_core; i++) {
+		const struct vivante_gc_ip_block *pblock = &pdata->ip_block[i];
+		struct platform_device *virtual_child;
+		resource_size_t start;
+		struct resource res;
+
+		start = pci_resource_start(pdev, pdata->mmio_bar);
+		memset(&res, 0, sizeof(res));
+		res.flags = IORESOURCE_MEM;
+		res.name = "registers";
+		res.start = start + pblock->offset;
+		res.end = start + pblock->offset + pblock->size - 1;
+
+		ret = etnaviv_create_platform_device(dev,
+						     pblock->compatible,
+						     pblock->id,
+						     &res,
+						     (void *)pdata,
+						     sizeof(*pdata),
+						     &virtual_child);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev,
+					       platform_device_remove_callback,
+					       virtual_child);
+		if (ret)
+			return ret;
+
+		match_names[i] = dev_name(&virtual_child->dev);
+	}
+
+	for (i = 0; i < num_core; i++)
+		dev_info(dev, "ip block %u: %s\n", i, match_names[i]);
+
+	ret = etnaviv_create_platform_device(dev, "etnaviv",
+					     PLATFORM_DEVID_NONE,
+					     NULL, match_names,
+					     sizeof(match_names),
+					     &etnaviv_drm);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void etnaviv_pci_remove(struct pci_dev *pdev)
+{
+
+}
+
+static const struct pci_device_id etnaviv_pci_id_list[] = {
+	{0x0731, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JM9100},
+	{0x0731, 0x9230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, JD9230P},
+	{0x0709, 0x0001, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GP102},
+	{0x0014, 0x7A15, PCI_ANY_ID, PCI_ANY_ID, 0, 0, GC1000_IN_LS7A1000},
+	{ }
+};
+
+static struct pci_driver etnaviv_pci_driver = {
+	.name = "etnaviv",
+	.id_table = etnaviv_pci_id_list,
+	.probe = etnaviv_pci_probe,
+	.remove = etnaviv_pci_remove,
+};
+
+int etnaviv_register_pci_driver(void)
+{
+	return pci_register_driver(&etnaviv_pci_driver);
+}
+
+void etnaviv_unregister_pci_driver(void)
+{
+	pci_unregister_driver(&etnaviv_pci_driver);
+}
+
+MODULE_DEVICE_TABLE(pci, etnaviv_pci_id_list);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
new file mode 100644
index 000000000000..eae8cdea5674
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ETNAVIV_PCI_DRV_H__
+#define __ETNAVIV_PCI_DRV_H__
+
+#ifdef CONFIG_DRM_ETNAVIV_PCI_DRIVER
+
+#define ETNA_MAX_IP_BLOCK          4
+
+enum etnaviv_pci_chip_id {
+	GCCORE_PCI_CHIP_ID_UNKNOWN = 0,
+	JM9100 = 1,
+	JD9230P = 2,
+	GP102 = 3,
+	GC1000_IN_LS7A1000 = 4,
+	GCCORE_PCI_CHIP_ID_LAST,
+};
+
+struct vivante_gc_ip_block {
+	u32 id;
+	u32 offset;
+	u32 size;
+	char compatible[20];
+};
+
+struct etnaviv_pci_gpu_data {
+	enum etnaviv_pci_chip_id chip_id;
+	u32 num_core;
+	u32 num_vram;
+	u32 vram_bars[2];
+	u32 mmio_bar;
+	struct vivante_gc_ip_block ip_block[ETNA_MAX_IP_BLOCK];
+	bool has_dedicated_vram;
+	bool has_display;
+	char market_name[24];
+};
+
+int etnaviv_register_pci_driver(void);
+void etnaviv_unregister_pci_driver(void);
+
+#else
+
+static inline int etnaviv_register_pci_driver(void) { return 0; }
+static inline void etnaviv_unregister_pci_driver(void) { }
+
+#endif
+
+#endif
-- 
2.43.0

[PATCH v15 14/19] drm/etnaviv: Add PCIe IP setup code

[Sui Jingfeng]

Because some PCIe IP need special setup before its VRAM bar can be usable,
do this with instance specific object function.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/Makefile          |   3 +-
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c |  19 ++++
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h |   9 ++
 drivers/gpu/drm/etnaviv/pcie_ip_setup.c   | 109 ++++++++++++++++++++++
 4 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/etnaviv/pcie_ip_setup.c

diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 6829e1ebf2db..383f181bfc4c 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -16,6 +16,7 @@ etnaviv-y := \
 	etnaviv_perfmon.o \
 	etnaviv_sched.o
 
-etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
+etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o \
+					    pcie_ip_setup.o
 
 obj-$(CONFIG_DRM_ETNAVIV)	+= etnaviv.o
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
index f13f3208120f..9911bfdc41a9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
@@ -5,6 +5,11 @@
 #include "etnaviv_drv.h"
 #include "etnaviv_pci_drv.h"
 
+static const struct etnaviv_pcie_ip_funcs jemo_9xxxx_gpu_pcie_ip_funcs = {
+	.init = jemo_pcie_init,
+	.fini = NULL,
+};
+
 static const struct etnaviv_pci_gpu_data
 gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
 	{
@@ -18,7 +23,9 @@ gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
 		.mmio_bar = 1,
 		.ip_block = {{0, 0x00900000, 0x00004000, "etnaviv-gpu,3d"},},
 		.has_dedicated_vram = true,
+		.has_iatu = true,
 		.has_display = true,
+		.pcie_ip_funcs = &jemo_9xxxx_gpu_pcie_ip_funcs,
 		.market_name = "JingJia Micro JM9100",
 	},
 	{
@@ -30,7 +37,9 @@ gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
 		.ip_block = {{0, 0x00900000, 0x00004000, "etnaviv-gpu,3d"},
 			     {1, 0x00910000, 0x00004000, "etnaviv-gpu,3d"},},
 		.has_dedicated_vram = true,
+		.has_iatu = true,
 		.has_display = true,
+		.pcie_ip_funcs = &jemo_9xxxx_gpu_pcie_ip_funcs,
 		.market_name = "JingJia Micro JD9230P",
 	},
 	{
@@ -42,6 +51,7 @@ gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
 		.ip_block = {{0, 0x00040000, 0x00004000, "etnaviv-gpu,3d"},
 			     {0, 0x000C0000, 0x00004000, "etnaviv-gpu,2d"},},
 		.has_dedicated_vram = true,
+		.has_iatu = false,
 		.has_display = true,
 		.market_name = "LingJiu GP102",
 	},
@@ -53,6 +63,7 @@ gccore_platform_data[GCCORE_PCI_CHIP_ID_LAST] = {
 		.mmio_bar = 0,
 		.ip_block = {{0, 0, 0x00004000, "etnaviv-gpu,3d"},},
 		.has_dedicated_vram = true,
+		.has_iatu = false,
 		.has_display = false,
 		.market_name = "GC1000 in LS7A1000",
 	},
@@ -83,6 +94,7 @@ static int etnaviv_pci_probe(struct pci_dev *pdev,
 			     const struct pci_device_id *ent)
 {
 	const struct etnaviv_pci_gpu_data *pdata;
+	const struct etnaviv_pcie_ip_funcs *pcie_ip_funcs;
 	struct device *dev = &pdev->dev;
 	unsigned int i;
 	unsigned int num_core;
@@ -102,6 +114,13 @@ static int etnaviv_pci_probe(struct pci_dev *pdev,
 	if (!pdata)
 		return -ENODEV;
 
+	pcie_ip_funcs = pdata->pcie_ip_funcs;
+	if (pcie_ip_funcs) {
+		ret = pcie_ip_funcs->init(pdev);
+		if (ret)
+			return ret;
+	}
+
 	num_core = pdata->num_core;
 
 	dev_info(dev, "%s has %u GPU cores\n", pdata->market_name, num_core);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
index eae8cdea5674..39eb2851355a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
@@ -23,6 +23,11 @@ struct vivante_gc_ip_block {
 	char compatible[20];
 };
 
+struct etnaviv_pcie_ip_funcs {
+	int (*init)(struct pci_dev *pdev);
+	void (*fini)(struct pci_dev *pdev);
+};
+
 struct etnaviv_pci_gpu_data {
 	enum etnaviv_pci_chip_id chip_id;
 	u32 num_core;
@@ -31,13 +36,17 @@ struct etnaviv_pci_gpu_data {
 	u32 mmio_bar;
 	struct vivante_gc_ip_block ip_block[ETNA_MAX_IP_BLOCK];
 	bool has_dedicated_vram;
+	bool has_iatu;
 	bool has_display;
+	const struct etnaviv_pcie_ip_funcs *pcie_ip_funcs;
 	char market_name[24];
 };
 
 int etnaviv_register_pci_driver(void);
 void etnaviv_unregister_pci_driver(void);
 
+int jemo_pcie_init(struct pci_dev *pdev);
+
 #else
 
 static inline int etnaviv_register_pci_driver(void) { return 0; }
diff --git a/drivers/gpu/drm/etnaviv/pcie_ip_setup.c b/drivers/gpu/drm/etnaviv/pcie_ip_setup.c
new file mode 100644
index 000000000000..f90db8260c35
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/pcie_ip_setup.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pci.h>
+
+#include "etnaviv_drv.h"
+#include "etnaviv_pci_drv.h"
+
+#define PCIE_IATU_BASE_ADDR               0x10000
+#define PCIE_IATU_BAR_ADDR_INC            0x200
+
+#define PCIE_REGION_INBOUND               1
+#define PCIE_REGION_OUTBOUND              0
+#define PCIE_REGION_DIRECT_BIT            31
+#define PCIE_REGION_DIRECT_BITMASK        0x80000000
+#define PCIE_REGION_INDEX_BITMASK         0x7FFFFFFF
+
+#define PCIE_REGION_TYPE_MEM              0x00
+#define PCIE_REGION_TYPE_IO               0x10
+
+#define PCIE_REGION_MATCH_BAR             1
+#define PCIE_REGION_MATCH_ADDR            0
+
+#define PCIE_REGION_ENABLE_BITMASK        BIT(31)
+#define PCIE_REGION_ENABLE_BIT            BIT(31)
+#define PCIE_REGION_MODE_BITMASK          BIT(30)
+#define PCIE_REGION_MODE_BIT              BIT(30)
+
+#define PCIE_REGION_BAR_NUM_BITMASK       GENMASK(10, 8)
+#define PCIE_REGION_BAR_NUM_SHIFT         8
+
+#define PCIE_REGION_INBOUND_TYPE          0x100
+#define PCIE_REGION_INBOUND_CTRL          0x104
+#define PCIE_REGION_INBOUND_ADDR_LO       0x114
+#define PCIE_REGION_INBOUND_ADDR_HI       0x118
+
+static void iatu_write(void __iomem *iatu, u32 bar, u32 offset, u32 value)
+{
+	u32 bar_base = PCIE_IATU_BASE_ADDR + bar * PCIE_IATU_BAR_ADDR_INC;
+
+	writel(value, iatu + bar_base + offset);
+}
+
+static u32 iatu_read(void __iomem *iatu, u32 bar, u32 offset)
+{
+	u32 bar_base = PCIE_IATU_BASE_ADDR + bar * PCIE_IATU_BAR_ADDR_INC;
+
+	return readl(iatu + bar_base + offset);
+}
+
+static int iatu_map_bar(void __iomem *iatu, u32 bar, u64 axi_addr)
+{
+	u32 addr_hi = axi_addr >> 32;
+	u32 addr_lo = axi_addr & 0xffffffff;
+	u32 val;
+
+	iatu_write(iatu, bar + 9, PCIE_REGION_INBOUND_ADDR_LO, addr_lo);
+	iatu_write(iatu, bar + 9, PCIE_REGION_INBOUND_ADDR_HI, addr_hi);
+	iatu_write(iatu, bar + 9, PCIE_REGION_INBOUND_TYPE,
+				  PCIE_REGION_TYPE_MEM);
+
+	val = PCIE_REGION_ENABLE_BIT |
+	      PCIE_REGION_MODE_BIT |
+	      bar << PCIE_REGION_BAR_NUM_SHIFT;
+	iatu_write(iatu, bar + 9, PCIE_REGION_INBOUND_CTRL, val);
+
+	/* sanity check */
+	val = iatu_read(iatu, bar + 9, PCIE_REGION_INBOUND_ADDR_LO);
+	if (val != addr_lo) {
+		pr_err("%s : %u\n", __func__, __LINE__);
+		return -EINVAL;
+	}
+
+	val = iatu_read(iatu, bar + 9, PCIE_REGION_INBOUND_ADDR_HI);
+	if (val != addr_hi) {
+		pr_err("%s : %u\n", __func__, __LINE__);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+int jemo_pcie_init(struct pci_dev *pdev)
+{
+	void __iomem *iatu;
+	int ret;
+
+	/* Bar 4 is PCIe iATU */
+	iatu = pci_iomap(pdev, 4, 0);
+	if (!iatu)
+		return -ENOMEM;
+
+	ret = iatu_map_bar(iatu, 0, 0x10000000);
+	if (ret)
+		return ret;
+
+	ret = iatu_map_bar(iatu, 1, 0x00000000);
+	if (ret)
+		return ret;
+
+	ret = iatu_map_bar(iatu, 2, 0x10000000);
+	if (ret)
+		return ret;
+
+	pci_iounmap(pdev, iatu);
+
+	dev_info(&pdev->dev, "PCIe iATU init done\n");
+
+	return 0;
+}
-- 
2.43.0

[PATCH v15 15/19] drm/etnaviv: Make more use of the etnaviv_gem_new_private() function

[Sui Jingfeng]

Add the 'bool shmem' as the 4th argument of etnaviv_gem_new_private(),
then call etnaviv_gem_new_handle() to allocate the etnaviv_gem_object
instance for us.

A small benefit is to reduce code duplication across different etnaviv
GEM buffer objects. This allow us to reuse etnaviv_gem_new_private()
everywhere, increasing code reusage.

We also should call drm_gem_private_object_fini() to uninitialize an
already allocated GEM object when it initialized failed. Now
etnaviv_gem_new_private() handle this trouble for us, the upper caller
can just use it, no need to worry about error handling anymore.

if true, the drm_gem_object_init() will allocate backing storage for us,
then this is a shmem buffer object. if false, we have to implement driver
specific backing storage.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 28 +++++++++++++--------
 drivers/gpu/drm/etnaviv/etnaviv_gem.h       |  4 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c |  2 +-
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 3732288ff530..27e4a93c981c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -697,21 +697,20 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 	u32 size, u32 flags, u32 *handle)
 {
 	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
-	struct drm_gem_object *obj = NULL;
+	struct etnaviv_gem_object *etnaviv_obj;
+	struct drm_gem_object *obj;
 	int ret;
 
 	size = PAGE_ALIGN(size);
 
-	ret = etnaviv_gem_new_impl(dev, flags, &etnaviv_gem_shmem_ops, &obj);
+	ret = etnaviv_gem_new_private(dev, size, flags, true,
+				      &etnaviv_gem_shmem_ops, &etnaviv_obj);
 	if (ret)
 		goto fail;
 
-	lockdep_set_class(&to_etnaviv_bo(obj)->lock, &etnaviv_shm_lock_class);
-
-	ret = drm_gem_object_init(dev, obj, size);
-	if (ret)
-		goto fail;
+	lockdep_set_class(&etnaviv_obj->lock, &etnaviv_shm_lock_class);
 
+	obj = &etnaviv_obj->base;
 	/*
 	 * Our buffers are kept pinned, so allocating them from the MOVABLE
 	 * zone is a really bad idea, and conflicts with CMA. See comments
@@ -732,7 +731,8 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 }
 
 int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
-	const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res)
+			    bool shmem, const struct etnaviv_gem_ops *ops,
+			    struct etnaviv_gem_object **res)
 {
 	struct drm_gem_object *obj;
 	int ret;
@@ -741,7 +741,15 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
 	if (ret)
 		return ret;
 
-	drm_gem_private_object_init(dev, obj, size);
+	if (shmem) {
+		ret = drm_gem_object_init(dev, obj, size);
+		if (ret) {
+			drm_gem_private_object_fini(obj);
+			return ret;
+		}
+	} else {
+		drm_gem_private_object_init(dev, obj, size);
+	}
 
 	*res = to_etnaviv_bo(obj);
 
@@ -830,7 +838,7 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
 	struct etnaviv_gem_object *etnaviv_obj;
 	int ret;
 
-	ret = etnaviv_gem_new_private(dev, size, ETNA_BO_CACHED,
+	ret = etnaviv_gem_new_private(dev, size, ETNA_BO_CACHED, false,
 				      &etnaviv_gem_userptr_ops, &etnaviv_obj);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index f2ac64d8e90b..b174a9e4cc48 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -118,7 +118,9 @@ void etnaviv_submit_put(struct etnaviv_gem_submit * submit);
 int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
 	struct drm_etnaviv_timespec *timeout);
 int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
-	const struct etnaviv_gem_ops *ops, struct etnaviv_gem_object **res);
+			    bool shmem, const struct etnaviv_gem_ops *ops,
+			    struct etnaviv_gem_object **res);
+
 void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
 struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj);
 void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 0062d808d6a9..64a858a0b0cf 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -107,7 +107,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 	size_t size = PAGE_ALIGN(attach->dmabuf->size);
 	int ret, npages;
 
-	ret = etnaviv_gem_new_private(dev, size, ETNA_BO_WC,
+	ret = etnaviv_gem_new_private(dev, size, ETNA_BO_WC, false,
 				      &etnaviv_gem_prime_ops, &etnaviv_obj);
 	if (ret < 0)
 		return ERR_PTR(ret);
-- 
2.43.0

[PATCH v15 16/19] drm/etnaviv: Call etnaviv_gem_obj_add() in ernaviv_gem_new_private()

[Sui Jingfeng]

The etnaviv_gem_obj_add() a common operation, the 'etnaviv_drm_private::
gem_list' is being used to record(track) all of the etnaviv GEM buffer
object created in this driver.

Once a etnaviv GEM buffer object has been allocated successfully, we
should add it into the global etnaviv_drm_private::gem_list'. Because
we need to free it and remove it free the list if the invoke of the
subsequent functions fail.

The only way that destroy etnaviv GEM buffer object is the implementation
of etnaviv_gem_free_object() function. The etnaviv_gem_free_object() first
remove the etnaviv GEM object from the list, then destroy its mapping and
finally free its memory footprint. Therefore, in order to corresponding
this, we add the freshly created etnaviv GEM buffer object immediately
after it was successfully created.

A benifit is that we only need to call etnaviv_gem_obj_add() once now,
since the ernaviv_gem_new_private() has been unified. Make the
etnaviv_gem_obj_add() static is a next nature thing.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 8 +++-----
 drivers/gpu/drm/etnaviv/etnaviv_gem.h       | 1 -
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 --
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index 27e4a93c981c..ee799c02d0aa 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -584,7 +584,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 	kfree(etnaviv_obj);
 }
 
-void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
+static void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
 {
 	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
 	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
@@ -719,8 +719,6 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 	 */
 	mapping_set_gfp_mask(obj->filp->f_mapping, priv->shm_gfp_mask);
 
-	etnaviv_gem_obj_add(dev, obj);
-
 	ret = drm_gem_handle_create(file, obj, handle);
 
 	/* drop reference from allocate - handle holds it now */
@@ -751,6 +749,8 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
 		drm_gem_private_object_init(dev, obj, size);
 	}
 
+	etnaviv_gem_obj_add(dev, obj);
+
 	*res = to_etnaviv_bo(obj);
 
 	return 0;
@@ -849,8 +849,6 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
 	etnaviv_obj->userptr.mm = current->mm;
 	etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE);
 
-	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
-
 	ret = drm_gem_handle_create(file, &etnaviv_obj->base, handle);
 
 	/* drop reference from allocate - handle holds it now */
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index b174a9e4cc48..8d8fc5b3a541 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -121,7 +121,6 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
 			    bool shmem, const struct etnaviv_gem_ops *ops,
 			    struct etnaviv_gem_object **res);
 
-void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
 struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj);
 void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 64a858a0b0cf..04ee31461b8c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -127,8 +127,6 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
-
 	return &etnaviv_obj->base;
 
 fail:
-- 
2.43.0

[PATCH v15 17/19] drm/etnaviv: Support to manage dedicated VRAM base on drm_mm

[Sui Jingfeng]

The dedicated VRAM is seen as a kind of device memory (IOMEM) at present,
we should use ioremap() to make device memory CPU accessible, we should
also need to implement mmap() driver for userspace. Therefore, we add a
simple drm-mm based range allocator for the upper (GEM object function)
layer.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/Makefile           |   1 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  12 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.h      |  11 +
 drivers/gpu/drm/etnaviv/etnaviv_gem.h      |   5 +
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c | 258 +++++++++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h |  12 +
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c  |  13 ++
 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h  |   6 +
 8 files changed, 318 insertions(+)
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h

diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 383f181bfc4c..aba2578966ff 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -6,6 +6,7 @@ etnaviv-y := \
 	etnaviv_drv.o \
 	etnaviv_dump.o \
 	etnaviv_gem_prime.o \
+	etnaviv_gem_vram.o \
 	etnaviv_gem_submit.o \
 	etnaviv_gem.o \
 	etnaviv_gpu.o \
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 7fc654f051a3..f10661fe079f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -23,6 +23,7 @@
 #include "etnaviv_drv.h"
 #include "etnaviv_gpu.h"
 #include "etnaviv_gem.h"
+#include "etnaviv_gem_vram.h"
 #include "etnaviv_mmu.h"
 #include "etnaviv_pci_drv.h"
 #include "etnaviv_perfmon.h"
@@ -46,6 +47,8 @@ static struct device_node *etnaviv_of_first_available_node(void)
 static int etnaviv_private_init(struct device *dev,
 				struct etnaviv_drm_private *priv)
 {
+	int ret;
+
 	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
 
 	mutex_init(&priv->gem_lock);
@@ -61,6 +64,10 @@ static int etnaviv_private_init(struct device *dev,
 
 	priv->cached_coherent = dev_is_dma_coherent(dev);
 
+	ret = etnaviv_init_dedicated_vram(dev, priv);
+	if (ret)
+		dev_err(dev, "Failed to init dedicated vram\n");
+
 	return 0;
 }
 
@@ -74,6 +81,11 @@ static void etnaviv_private_fini(struct etnaviv_drm_private *priv)
 	xa_destroy(&priv->active_contexts);
 
 	mutex_destroy(&priv->gem_lock);
+
+	if (priv->dedicated_vram) {
+		drm_mm_takedown(&priv->vram.mm);
+		priv->dedicated_vram = false;
+	}
 }
 
 static void load_gpu(struct drm_device *dev)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 84f2e79f0a53..b093f832599f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -58,6 +58,17 @@ struct etnaviv_drm_private {
 	 */
 	bool cached_coherent;
 
+	/*
+	 * dedicated VRAM (device memory) resource
+	 */
+	struct {
+		struct drm_mm mm;
+		u64 gpu_base;
+		u64 cpu_base;
+		u64 size;
+	} vram;
+	bool dedicated_vram;
+
 	/* list of GEM objects: */
 	struct mutex gem_lock;
 	struct list_head gem_list;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 8d8fc5b3a541..60bbbbc2dd19 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -59,6 +59,9 @@ struct etnaviv_gem_object {
 	u32 last_cpu_prep_op;
 
 	struct etnaviv_gem_userptr userptr;
+
+	/* dedicated vram, which is physically contiguous */
+	struct drm_mm_node *vram_np;
 };
 
 static inline
@@ -129,4 +132,6 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
 	u64 va);
 void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping);
 
+u64 etnaviv_obj_gpu_phys_addr(struct etnaviv_gem_object *etnaviv_obj);
+
 #endif /* __ETNAVIV_GEM_H__ */
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
new file mode 100644
index 000000000000..c2942317a64e
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/pci.h>
+
+#include "etnaviv_drv.h"
+#include "etnaviv_gem.h"
+#include "etnaviv_pci_drv.h"
+
+static struct lock_class_key etnaviv_vram_lock_class;
+
+static u64 etnaviv_obj_cpu_phys_addr(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct drm_gem_object *obj = &etnaviv_obj->base;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
+
+	if (!etnaviv_obj->vram_np) {
+		drm_err(obj->dev, "No backing vram node, please pin it\n");
+		return 0;
+	}
+
+	return priv->vram.cpu_base + etnaviv_obj->vram_np->start;
+}
+
+u64 etnaviv_obj_gpu_phys_addr(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct drm_gem_object *obj = &etnaviv_obj->base;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
+
+	if (!etnaviv_obj->vram_np) {
+		drm_err(obj->dev, "No backing vram node, no gpu offset\n");
+		return 0;
+	}
+
+	return priv->vram.gpu_base + etnaviv_obj->vram_np->start;
+}
+
+/* Called with etnaviv_obj->lock held, allocate pages from dedicated VRAM */
+static int etnaviv_gem_vram_get_pages(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct drm_gem_object *obj = &etnaviv_obj->base;
+	struct drm_device *drm = obj->dev;
+	struct etnaviv_drm_private *priv = to_etnaviv_priv(drm);
+	unsigned int npages = obj->size >> PAGE_SHIFT;
+	struct drm_mm_node *vram_np;
+	struct page **ppages;
+	u64 gpu_paddr;
+	unsigned int i;
+	int ret;
+
+	lockdep_assert_held(&etnaviv_obj->lock);
+
+	vram_np = kzalloc(sizeof(*vram_np), GFP_KERNEL);
+	if (!vram_np)
+		return -ENOMEM;
+
+	ret = drm_mm_insert_node(&priv->vram.mm, vram_np, obj->size);
+	if (ret) {
+		drm_err(drm, "Failed to insert %zu KiB\n", obj->size >> 10);
+		goto fail_allocate_range;
+	}
+
+	ppages = kvmalloc_array(npages, sizeof(*ppages), GFP_KERNEL);
+	if (!ppages) {
+		ret = -ENOMEM;
+		goto fail_allocate_pages;
+	}
+
+	etnaviv_obj->vram_np = vram_np;
+
+	gpu_paddr = etnaviv_obj_gpu_phys_addr(etnaviv_obj);
+	for (i = 0; i < npages; ++i) {
+		ppages[i] = pfn_to_page(__phys_to_pfn(gpu_paddr));
+		gpu_paddr += PAGE_SIZE;
+	}
+
+	etnaviv_obj->pages = ppages;
+
+	return 0;
+
+fail_allocate_pages:
+	drm_mm_remove_node(vram_np);
+fail_allocate_range:
+	kfree(vram_np);
+
+	return ret;
+}
+
+static void *etnaviv_gem_vram_vmap(struct etnaviv_gem_object *etnaviv_obj)
+{
+	struct drm_device *drm = etnaviv_obj->base.dev;
+	u64 offset;
+	u64 size;
+	int ret;
+
+	lockdep_assert_held(&etnaviv_obj->lock);
+
+	/*
+	 * This is equivalent to pin, we need the dedicated vram node
+	 * inserted into the whole drm_mm, to get a valid offset.
+	 */
+	if (!etnaviv_obj->pages) {
+		ret = etnaviv_obj->ops->get_pages(etnaviv_obj);
+		if (ret) {
+			drm_err(drm, "Failed to pin %p\n", etnaviv_obj);
+			return NULL;
+		}
+	}
+
+	offset = etnaviv_obj_cpu_phys_addr(etnaviv_obj);
+	size = etnaviv_obj->base.size;
+
+	drm_dbg(drm, "offset : 0x%llx, %llu\n", offset, size >> 10);
+
+	if (etnaviv_obj->flags & ETNA_BO_WC)
+		return ioremap_wc(offset, size);
+
+	return ioremap(offset, size);
+}
+
+static void etnaviv_gem_vram_vunmap(struct etnaviv_gem_object *etnaviv_obj)
+{
+	lockdep_assert_held(&etnaviv_obj->lock);
+
+	iounmap(etnaviv_obj->vaddr);
+}
+
+static void etnaviv_gem_vram_release(struct etnaviv_gem_object *etnaviv_obj)
+{
+	if (etnaviv_obj->sgt) {
+		sg_free_table(etnaviv_obj->sgt);
+		kfree(etnaviv_obj->sgt);
+		etnaviv_obj->sgt = NULL;
+	}
+
+	if (etnaviv_obj->pages) {
+		kvfree(etnaviv_obj->pages);
+		etnaviv_obj->pages = NULL;
+	}
+
+	if (etnaviv_obj->vram_np) {
+		drm_mm_remove_node(etnaviv_obj->vram_np);
+		kfree(etnaviv_obj->vram_np);
+		etnaviv_obj->vram_np = NULL;
+	}
+}
+
+static int etnaviv_gem_vram_mmap(struct etnaviv_gem_object *etnaviv_obj,
+				 struct vm_area_struct *vma)
+{
+	pgprot_t vm_page_prot;
+
+	vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
+
+	vm_page_prot = vm_get_page_prot(vma->vm_flags);
+
+	if (etnaviv_obj->flags & ETNA_BO_WC)
+		vma->vm_page_prot = pgprot_writecombine(vm_page_prot);
+	else if (etnaviv_obj->flags & ETNA_BO_UNCACHED)
+		vma->vm_page_prot = pgprot_noncached(vm_page_prot);
+	else
+		vma->vm_page_prot = vm_page_prot;
+
+	return 0;
+}
+
+static const struct etnaviv_gem_ops etnaviv_gem_vram_ops = {
+	.get_pages = etnaviv_gem_vram_get_pages,
+	.release = etnaviv_gem_vram_release,
+	.vmap = etnaviv_gem_vram_vmap,
+	.vunmap = etnaviv_gem_vram_vunmap,
+	.mmap = etnaviv_gem_vram_mmap,
+};
+
+int etnaviv_gem_new_vram(struct drm_device *dev, struct drm_file *file,
+			 u32 size, u32 flags, u32 *handle)
+{
+	struct etnaviv_gem_object *etnaviv_obj;
+	struct drm_gem_object *obj;
+	int ret;
+
+	size = PAGE_ALIGN(size);
+
+	ret = etnaviv_gem_new_private(dev, size, flags, false,
+				      &etnaviv_gem_vram_ops, &etnaviv_obj);
+	if (ret)
+		return ret;
+
+	lockdep_set_class(&etnaviv_obj->lock, &etnaviv_vram_lock_class);
+
+	obj = &etnaviv_obj->base;
+
+	ret = drm_gem_handle_create(file, obj, handle);
+
+	/* drop reference from allocate - handle holds it now */
+	drm_gem_object_put(obj);
+
+	return ret;
+}
+
+static int etnaviv_pci_init_dedicated_vram(struct device *dev,
+					   struct etnaviv_drm_private *priv)
+{
+	struct pci_dev *gpu = to_pci_dev(dev);
+	static const struct etnaviv_pci_gpu_data *pdata;
+	resource_size_t base, size;
+	u32 bar;
+
+	pdata = etnaviv_pci_get_match_data(dev);
+	if (!pdata)
+		return -ENOENT;
+
+	if (pdata->num_vram <= 0) {
+		dev_err(dev, "Don't has a dedicated VRAM\n");
+		return -ENODEV;
+	}
+
+	/* Using the first vram bar */
+	bar = pdata->vram_bars[0];
+
+	base = pci_resource_start(gpu, bar);
+	size = pci_resource_len(gpu, bar);
+	if (!base || !size)
+		return -ENOSPC;
+
+	priv->vram.gpu_base = pci_bus_address(gpu, bar);
+	priv->vram.cpu_base = base;
+	priv->vram.size = size;
+	priv->dedicated_vram = true;
+
+	dev_info(dev, "GPU Bar %u contains dedicated VRAM\n", bar);
+
+	return 0;
+}
+
+int etnaviv_init_dedicated_vram(struct device *dev,
+				struct etnaviv_drm_private *priv)
+{
+	int ret;
+
+	if (dev->parent && dev_is_pci(dev->parent)) {
+		ret = etnaviv_pci_init_dedicated_vram(dev->parent, priv);
+		if (ret)
+			return ret;
+	}
+
+	if (!priv->vram.size)
+		return 0;
+
+	/* CPU physical address */
+	drm_mm_init(&priv->vram.mm, 0, priv->vram.size);
+
+	dev_info(dev, "VRAM device address range: %08llx-%08llx\n",
+		 priv->vram.gpu_base, priv->vram.gpu_base + priv->vram.size);
+	dev_info(dev, "VRAM CPU physical address range: %08llx~%08llx\n",
+		 priv->vram.cpu_base, priv->vram.cpu_base + priv->vram.size);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
new file mode 100644
index 000000000000..bbce93f118a2
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ETNAVIV_GEM_VRAM_H__
+#define __ETNAVIV_GEM_VRAM_H__
+
+int etnaviv_gem_new_vram(struct drm_device *dev, struct drm_file *file,
+			 u32 size, u32 flags, u32 *handle);
+
+int etnaviv_init_dedicated_vram(struct device *dev,
+				struct etnaviv_drm_private *priv);
+
+#endif
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
index 9911bfdc41a9..cbb7bc0947a7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
@@ -191,6 +191,19 @@ static struct pci_driver etnaviv_pci_driver = {
 	.remove = etnaviv_pci_remove,
 };
 
+const struct etnaviv_pci_gpu_data *
+etnaviv_pci_get_match_data(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	const struct pci_device_id *entity;
+
+	entity = pci_match_id(etnaviv_pci_id_list, pdev);
+	if (!entity)
+		return NULL;
+
+	return etnaviv_pci_get_platform_data(entity);
+}
+
 int etnaviv_register_pci_driver(void)
 {
 	return pci_register_driver(&etnaviv_pci_driver);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
index 39eb2851355a..f5184fb35f50 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
@@ -42,6 +42,9 @@ struct etnaviv_pci_gpu_data {
 	char market_name[24];
 };
 
+const struct etnaviv_pci_gpu_data *
+etnaviv_pci_get_match_data(struct device *dev);
+
 int etnaviv_register_pci_driver(void);
 void etnaviv_unregister_pci_driver(void);
 
@@ -52,6 +55,9 @@ int jemo_pcie_init(struct pci_dev *pdev);
 static inline int etnaviv_register_pci_driver(void) { return 0; }
 static inline void etnaviv_unregister_pci_driver(void) { }
 
+static inline struct etnaviv_pci_gpu_data *
+etnaviv_pci_get_match_data(struct device *dev) { return NULL; }
+
 #endif
 
 #endif
-- 
2.43.0

[PATCH v15 18/19] drm/etnaviv: Allow userspace specify the domain of etnaviv GEM buffer object

[Sui Jingfeng]

Otherwise we don't know where a etnaviv GEM buffer object should put when
we create it at userspace.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c |  9 +++++++++
 include/uapi/drm/etnaviv_drm.h        | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index f10661fe079f..cdc62f64b200 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -331,11 +331,20 @@ static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
 	struct drm_etnaviv_gem_new *args = data;
+	u32 domain;
+
+	domain = args->flags & ETNA_BO_DOMAIN_MASK;
+
+	args->flags &= ~ETNA_BO_DOMAIN_MASK;
 
 	if (args->flags & ~(ETNA_BO_CACHED | ETNA_BO_WC | ETNA_BO_UNCACHED |
 			    ETNA_BO_FORCE_MMU))
 		return -EINVAL;
 
+	if (domain == ETNA_BO_PL_VRAM)
+		return etnaviv_gem_new_vram(dev, file, args->size,
+					    args->flags, &args->handle);
+
 	return etnaviv_gem_new_handle(dev, file, args->size,
 			args->flags, &args->handle);
 }
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index 61eaa8cd0f5e..00e778c9d312 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -99,6 +99,18 @@ struct drm_etnaviv_param {
 /* map flags */
 #define ETNA_BO_FORCE_MMU    0x00100000
 
+/* domain (placement) flags */
+#define ETNA_BO_DOMAIN_MASK  0x00f00000
+
+/* CPU accessible, GPU accessible pages in dedicated VRAM */
+#define ETNA_BO_PL_VRAM      0x01000000
+/* CPU accessible, GPU accessible pages in SHMEM */
+#define ETNA_BO_PL_GTT       0x02000000
+/* Userspace allocated memory, at least CPU accessible */
+#define ETNA_BO_PL_USERPTR   0x08000000
+/* GPU accessible but CPU not accessible private VRAM pages */
+#define ETNA_BO_PL_PRIV      0x04000000
+
 struct drm_etnaviv_gem_new {
 	__u64 size;           /* in */
 	__u32 flags;          /* in, mask of ETNA_BO_x */
-- 
2.43.0

[PATCH v15 19/19] drm/etnaviv: Expose basic sanity tests via debugfs

[Sui Jingfeng]

To test the correctess of the implemented vmap() and vunmap() operations,
to see if is works correctly on dedicated VRAM.

Usage:

cd /sys/kernel/debug/dri/etnaviv
cat sanity

My test is able to pass on x86-64 with a JM9230P card, see below log:

Test Write to VRAM 8294400 bytes
Write to VRAM Passed: 8294400 bytes
Test Write to SHMEM 8294400 bytes
Write to SHMEM Passed: 8294400 bytes

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/Makefile           |   1 +
 drivers/gpu/drm/etnaviv/etnaviv_debugfs.c  | 118 +++++++++++++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_debugfs.h  |  15 +++
 drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  12 +++
 drivers/gpu/drm/etnaviv/etnaviv_gem.c      |   5 +
 drivers/gpu/drm/etnaviv/etnaviv_gem.h      |   2 +
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c |   6 ++
 drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h |   2 +
 8 files changed, 161 insertions(+)
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_debugfs.c
 create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_debugfs.h

diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index aba2578966ff..f278e75ee7cd 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -17,6 +17,7 @@ etnaviv-y := \
 	etnaviv_perfmon.o \
 	etnaviv_sched.o
 
+etnaviv-$(CONFIG_DEBUG_FS) += etnaviv_debugfs.o
 etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o \
 					    pcie_ip_setup.o
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_debugfs.c b/drivers/gpu/drm/etnaviv/etnaviv_debugfs.c
new file mode 100644
index 000000000000..0cfedbc6574c
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_debugfs.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <drm/drm_debugfs.h>
+
+#include "etnaviv_debugfs.h"
+#include "etnaviv_drv.h"
+#include "etnaviv_gem.h"
+#include "etnaviv_gem_vram.h"
+
+static void bo_test_write_to_vram_by_cpu(void *addr, unsigned int num)
+{
+	u32 val = 0;
+
+	while (num--) {
+		writel(val, addr);
+		++val;
+		addr += 4;
+	}
+}
+
+static unsigned int bo_test_read_from_vram_by_cpu(void *addr, unsigned int num)
+{
+	unsigned int i = 0;
+
+	while (i < num) {
+		u32 val = readl(addr);
+
+		if (val != i)
+			return i;
+
+		addr += 4;
+		++i;
+	}
+
+	return 0;
+}
+
+void etnaviv_sanity_test_vram_impl(struct drm_device *drm, struct drm_printer *p)
+{
+	struct etnaviv_gem_object *etnaviv_obj;
+	unsigned int size = 1920 * 1080 * 4;
+	void *addr;
+	int ret;
+
+	size = ALIGN(size, PAGE_SIZE);
+
+	drm_printf(p, "Test Write to VRAM %u bytes\n", size);
+
+	ret = etnaviv_gem_new_private(drm, size, ETNA_BO_UNCACHED, false,
+				      etnaviv_gem_get_vram_ops(),
+				      &etnaviv_obj);
+	if (ret) {
+		drm_printf(p, "create dst bo failed\n");
+		return;
+	}
+
+	addr = etnaviv_gem_vmap(&etnaviv_obj->base);
+	if (!addr) {
+		drm_printf(p, "write to vram by cpu failed: vmap\n");
+		goto out;
+	}
+
+	etnaviv_gem_vunmap(&etnaviv_obj->base);
+
+	addr = etnaviv_gem_vmap(&etnaviv_obj->base);
+
+	bo_test_write_to_vram_by_cpu(addr, size / 4);
+
+	ret = bo_test_read_from_vram_by_cpu(addr, size / 4);
+
+	drm_printf(p, "Write to VRAM %s: %u bytes\n",
+		   ret ? "not pass" : "Passed", ret ? ret : size);
+
+	etnaviv_gem_vunmap(&etnaviv_obj->base);
+out:
+	drm_gem_object_put(&etnaviv_obj->base);
+}
+
+void etnaviv_sanity_test_shmem_impl(struct drm_device *drm, struct drm_printer *p)
+{
+	struct etnaviv_gem_object *etnaviv_obj;
+	unsigned int size = 1920 * 1080 * 4;
+	void *addr;
+	int ret;
+
+	size = ALIGN(size, PAGE_SIZE);
+
+	drm_printf(p, "Test Write to SHMEM %u bytes\n", size);
+
+	ret = etnaviv_gem_new_private(drm, size, ETNA_BO_CACHED, true,
+				      etnaviv_gem_get_shmem_ops(),
+				      &etnaviv_obj);
+	if (ret) {
+		drm_printf(p, "create dst bo failed\n");
+		return;
+	}
+
+	addr = etnaviv_gem_vmap(&etnaviv_obj->base);
+	if (!addr) {
+		drm_printf(p, "write to shmem by cpu failed: vmap\n");
+		goto out;
+	}
+
+	etnaviv_gem_vunmap(&etnaviv_obj->base);
+
+	addr = etnaviv_gem_vmap(&etnaviv_obj->base);
+
+	bo_test_write_to_vram_by_cpu(addr, size / 4);
+
+	ret = bo_test_read_from_vram_by_cpu(addr, size / 4);
+
+	drm_printf(p, "Write to SHMEM %s: %u bytes\n",
+		   ret ? "not pass" : "Passed", ret ? ret : size);
+
+	etnaviv_gem_vunmap(&etnaviv_obj->base);
+out:
+	drm_gem_object_put(&etnaviv_obj->base);
+}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_debugfs.h b/drivers/gpu/drm/etnaviv/etnaviv_debugfs.h
new file mode 100644
index 000000000000..5214349534ef
--- /dev/null
+++ b/drivers/gpu/drm/etnaviv/etnaviv_debugfs.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ETNAVIV_DEBUGFS_H__
+#define __ETNAVIV_DEBUGFS_H__
+
+#include "etnaviv_drv.h"
+#include "etnaviv_gem.h"
+
+void etnaviv_sanity_test_vram_impl(struct drm_device *ddev,
+				   struct drm_printer *p);
+
+void etnaviv_sanity_test_shmem_impl(struct drm_device *ddev,
+				    struct drm_printer *p);
+
+#endif
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index cdc62f64b200..e500b8caf138 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -20,6 +20,7 @@
 #include <drm/drm_prime.h>
 
 #include "etnaviv_cmdbuf.h"
+#include "etnaviv_debugfs.h"
 #include "etnaviv_drv.h"
 #include "etnaviv_gpu.h"
 #include "etnaviv_gem.h"
@@ -183,6 +184,16 @@ static int etnaviv_gem_show(struct drm_device *dev, struct seq_file *m)
 	return 0;
 }
 
+static int etnaviv_sanity_test_show(struct drm_device *dev, struct seq_file *m)
+{
+	struct drm_printer printer = drm_seq_file_printer(m);
+
+	etnaviv_sanity_test_vram_impl(dev, &printer);
+	etnaviv_sanity_test_shmem_impl(dev, &printer);
+
+	return 0;
+}
+
 static int etnaviv_mm_show(struct drm_device *dev, struct seq_file *m)
 {
 	struct drm_printer p = drm_seq_file_printer(m);
@@ -296,6 +307,7 @@ static struct drm_info_list etnaviv_debugfs_list[] = {
 	{ "mm", show_unlocked, 0, etnaviv_mm_show },
 	{"mmu", show_each_gpu, 0, etnaviv_mmu_show},
 	{"ring", show_each_gpu, 0, etnaviv_ring_show},
+	{"sanity", show_unlocked, 0, etnaviv_sanity_test_show },
 };
 
 static void etnaviv_debugfs_init(struct drm_minor *minor)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index ee799c02d0aa..31bcd80770b3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -553,6 +553,11 @@ static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
 	.mmap = etnaviv_gem_mmap_obj,
 };
 
+const struct etnaviv_gem_ops *etnaviv_gem_get_shmem_ops(void)
+{
+	return &etnaviv_gem_shmem_ops;
+}
+
 void etnaviv_gem_free_object(struct drm_gem_object *obj)
 {
 	struct drm_device *drm = obj->dev;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index 60bbbbc2dd19..1836e1d82df2 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -134,4 +134,6 @@ void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping);
 
 u64 etnaviv_obj_gpu_phys_addr(struct etnaviv_gem_object *etnaviv_obj);
 
+const struct etnaviv_gem_ops *etnaviv_gem_get_shmem_ops(void);
+
 #endif /* __ETNAVIV_GEM_H__ */
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
index c2942317a64e..1ca558429387 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.c
@@ -4,6 +4,7 @@
 
 #include "etnaviv_drv.h"
 #include "etnaviv_gem.h"
+#include "etnaviv_gem_vram.h"
 #include "etnaviv_pci_drv.h"
 
 static struct lock_class_key etnaviv_vram_lock_class;
@@ -171,6 +172,11 @@ static const struct etnaviv_gem_ops etnaviv_gem_vram_ops = {
 	.mmap = etnaviv_gem_vram_mmap,
 };
 
+const struct etnaviv_gem_ops *etnaviv_gem_get_vram_ops(void)
+{
+	return &etnaviv_gem_vram_ops;
+}
+
 int etnaviv_gem_new_vram(struct drm_device *dev, struct drm_file *file,
 			 u32 size, u32 flags, u32 *handle)
 {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
index bbce93f118a2..54f98936ddb4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_vram.h
@@ -3,6 +3,8 @@
 #ifndef __ETNAVIV_GEM_VRAM_H__
 #define __ETNAVIV_GEM_VRAM_H__
 
+const struct etnaviv_gem_ops *etnaviv_gem_get_vram_ops(void);
+
 int etnaviv_gem_new_vram(struct drm_device *dev, struct drm_file *file,
 			 u32 size, u32 flags, u32 *handle);
 
-- 
2.43.0

Re: [PATCH v15 01/19] drm/etnaviv: Implement drm_gem_object_funcs::print_info()

[Lucas Stach]

Hi Sui,

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> It will be called by drm_gem_print_info() if implemented, and it can
> provide more information about the framebuffer objects.

Etnaviv GEM BOs are not framebuffer objects.

> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++++++++++++++++++++++++++
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 4247a10f8d4f..543d881585b3 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -534,8 +534,40 @@ static const struct vm_operations_struct vm_ops = {
>  	.close = drm_gem_vm_close,
>  };
>  
> +static const char *etnaviv_gem_obj_caching_info(u32 flags)
> +{
> +	switch (flags & ETNA_BO_CACHE_MASK) {
> +	case ETNA_BO_CACHED:
> +		return "cached";
> +	case ETNA_BO_UNCACHED:
> +		return "uncached";
> +	case ETNA_BO_WC:
> +		return "write-combine";

"write-combined" to be consistent with the other two.

> +	default:
> +		break;
> +	}
> +
> +	return "unknown";
> +}
> +
> +static void etnaviv_gem_object_info(struct drm_printer *p,
> +				    unsigned int indent,
> +				    const struct drm_gem_object *obj)
> +{
> +	const struct etnaviv_gem_object *etnaviv_obj;
> +
> +	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
> +
> +	drm_printf_indent(p, indent, "caching mode=%s\n",
> +			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
> +	drm_printf_indent(p, indent, "active=%s\n",
> +			  str_yes_no(is_active(etnaviv_obj)));
> +	drm_printf_indent(p, indent, "vaddr=%p\n", etnaviv_obj->vaddr);

Why should we expose the vaddr to userspace? I don't see why this would
be relevant even as debug info and it leaks the kernel vmap area
address, which could be abused to facilitate kernel exploits.

Regards,
Lucas

> +}
> +
>  static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>  	.free = etnaviv_gem_free_object,
> +	.print_info = etnaviv_gem_object_info,
>  	.pin = etnaviv_gem_prime_pin,
>  	.unpin = etnaviv_gem_prime_unpin,
>  	.get_sg_table = etnaviv_gem_prime_get_sg_table,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index a42d260cac2c..3f8fe19a77cc 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -68,7 +68,7 @@ struct etnaviv_gem_ops {
>  	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
>  };
>  
> -static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
> +static inline bool is_active(const struct etnaviv_gem_object *etnaviv_obj)
>  {
>  	return atomic_read(&etnaviv_obj->gpu_active) != 0;
>  }

Re: [PATCH v15 02/19] drm/etnaviv: Export drm_gem_print_info() and use it

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> This will make the newly implemented etnaviv_gem_object_funcs::print_info
> get in use, which improves code sharing and simplifies debugfs. Achieve
> better humen readability for debug log.
> 
> Use container_of_const() if 'struct etnaviv_gem_object *etnaviv_obj' is a
> constant pointer.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/drm_gem.c             |  1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 13 +++++--------
>  include/drm/drm_gem.h                 |  2 ++
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index d4bbc5d109c8..9c5c971c1b23 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1160,6 +1160,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>  	if (obj->funcs->print_info)
>  		obj->funcs->print_info(p, indent, obj);
>  }
> +EXPORT_SYMBOL(drm_gem_print_info);

This needs to be a separate patch. I don't think I can take such a
change intermingled with etnaviv changes in the same patch. This needs
some acks from DRM core.

Regards,
Lucas

>  
>  int drm_gem_pin_locked(struct drm_gem_object *obj)
>  {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 543d881585b3..6bdf72cd9e85 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2015-2018 Etnaviv Project
>   */
>  
> +#include <drm/drm_gem.h>
>  #include <drm/drm_prime.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/shmem_fs.h>
> @@ -432,15 +433,11 @@ int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
>  #ifdef CONFIG_DEBUG_FS
>  static void etnaviv_gem_describe(struct drm_gem_object *obj, struct seq_file *m)
>  {
> -	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> +	struct drm_printer p = drm_seq_file_printer(m);
>  	struct dma_resv *robj = obj->resv;
> -	unsigned long off = drm_vma_node_start(&obj->vma_node);
>  	int r;
>  
> -	seq_printf(m, "%08x: %c %2d (%2d) %08lx %p %zd\n",
> -			etnaviv_obj->flags, is_active(etnaviv_obj) ? 'A' : 'I',
> -			obj->name, kref_read(&obj->refcount),
> -			off, etnaviv_obj->vaddr, obj->size);
> +	drm_gem_print_info(&p, 1, obj);
>  
>  	r = dma_resv_lock(robj, NULL);
>  	if (r)
> @@ -461,7 +458,7 @@ void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
>  	list_for_each_entry(etnaviv_obj, &priv->gem_list, gem_node) {
>  		struct drm_gem_object *obj = &etnaviv_obj->base;
>  
> -		seq_puts(m, "   ");
> +		seq_printf(m, "obj[%d]:\n", count);
>  		etnaviv_gem_describe(obj, m);
>  		count++;
>  		size += obj->size;
> @@ -556,7 +553,7 @@ static void etnaviv_gem_object_info(struct drm_printer *p,
>  {
>  	const struct etnaviv_gem_object *etnaviv_obj;
>  
> -	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
> +	etnaviv_obj = container_of_const(obj, struct etnaviv_gem_object, base);
>  
>  	drm_printf_indent(p, indent, "caching mode=%s\n",
>  			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index bae4865b2101..0791566fab53 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -480,6 +480,8 @@ void drm_gem_vm_close(struct vm_area_struct *vma);
>  int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  		     struct vm_area_struct *vma);
>  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
> +void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> +			const struct drm_gem_object *obj);
>  
>  /**
>   * drm_gem_object_get - acquire a GEM buffer object reference

Re: [PATCH v15 03/19] drm/etnaviv: Implement drm_gem_object_funcs::vunmap()

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> The vunmap() can be used to release virtual mapping obtained by vmap(),
> While the vmap() is used to make a long duration mapping of multiple
> physical pages into a contiguous virtual space.
> 
> Make the implementation-specific vunmap() operation untangled with the
> etnaviv_gem_xxx_release() function. As then, the etnaviv_gem_xxx_release()
> only need to responsible for the release page works.
> 
> The etnaviv_gem_vunmap() is added for driver internal usa case, where no
> DRM GEM framework is involved.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 38 ++++++++++++++++++++-
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h       |  1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 13 ++++---
>  4 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index b3eb1662e90c..2eb2ff13f6e8 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -61,6 +61,7 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>  void etnaviv_gem_prime_unpin(struct drm_gem_object *obj);
>  void *etnaviv_gem_vmap(struct drm_gem_object *obj);
> +void etnaviv_gem_vunmap(struct drm_gem_object *obj);
>  int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op,
>  		struct drm_etnaviv_timespec *timeout);
>  int etnaviv_gem_cpu_fini(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 6bdf72cd9e85..fad23494d08e 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -340,6 +340,25 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
>  	return etnaviv_obj->vaddr;
>  }
>  
> +void etnaviv_gem_vunmap(struct drm_gem_object *obj)
> +{
> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> +
> +	if (!etnaviv_obj->vaddr)
> +		return;
> +
> +	mutex_lock(&etnaviv_obj->lock);

Either the mutex must extend across the above vaddr check, or the check
need to be repeated within the mutex section, similar to the
implementation in etnaviv_gem_vmap.

Regards,
Lucas

> +	etnaviv_obj->ops->vunmap(etnaviv_obj);
> +	etnaviv_obj->vaddr = NULL;
> +	mutex_unlock(&etnaviv_obj->lock);
> +}
> +
> +static void etnaviv_gem_object_vunmap(struct drm_gem_object *obj,
> +				      struct iosys_map *map)
> +{
> +	etnaviv_gem_vunmap(obj);
> +}
> +
>  static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
>  {
>  	struct page **pages;
> @@ -471,14 +490,21 @@ void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
>  
>  static void etnaviv_gem_shmem_release(struct etnaviv_gem_object *etnaviv_obj)
>  {
> -	vunmap(etnaviv_obj->vaddr);
>  	put_pages(etnaviv_obj);
>  }
>  
> +static void etnaviv_gem_shmem_vunmap(struct etnaviv_gem_object *etnaviv_obj)
> +{
> +	lockdep_assert_held(&etnaviv_obj->lock);
> +
> +	vunmap(etnaviv_obj->vaddr);
> +}
> +
>  static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = {
>  	.get_pages = etnaviv_gem_shmem_get_pages,
>  	.release = etnaviv_gem_shmem_release,
>  	.vmap = etnaviv_gem_vmap_impl,
> +	.vunmap = etnaviv_gem_shmem_vunmap,
>  	.mmap = etnaviv_gem_mmap_obj,
>  };
>  
> @@ -508,6 +534,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  		kfree(mapping);
>  	}
>  
> +	etnaviv_obj->ops->vunmap(etnaviv_obj);
>  	etnaviv_obj->ops->release(etnaviv_obj);
>  	drm_gem_object_release(obj);
>  
> @@ -569,6 +596,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>  	.unpin = etnaviv_gem_prime_unpin,
>  	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>  	.vmap = etnaviv_gem_prime_vmap,
> +	.vunmap = etnaviv_gem_object_vunmap,
>  	.mmap = etnaviv_gem_mmap,
>  	.vm_ops = &vm_ops,
>  };
> @@ -723,6 +751,13 @@ static void etnaviv_gem_userptr_release(struct etnaviv_gem_object *etnaviv_obj)
>  	}
>  }
>  
> +static void etnaviv_gem_userptr_vunmap(struct etnaviv_gem_object *etnaviv_obj)
> +{
> +	lockdep_assert_held(&etnaviv_obj->lock);
> +
> +	vunmap(etnaviv_obj->vaddr);
> +}
> +
>  static int etnaviv_gem_userptr_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  		struct vm_area_struct *vma)
>  {
> @@ -733,6 +768,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_userptr_ops = {
>  	.get_pages = etnaviv_gem_userptr_get_pages,
>  	.release = etnaviv_gem_userptr_release,
>  	.vmap = etnaviv_gem_vmap_impl,
> +	.vunmap = etnaviv_gem_userptr_vunmap,
>  	.mmap = etnaviv_gem_userptr_mmap_obj,
>  };
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index 3f8fe19a77cc..d4965de3007c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -65,6 +65,7 @@ struct etnaviv_gem_ops {
>  	int (*get_pages)(struct etnaviv_gem_object *);
>  	void (*release)(struct etnaviv_gem_object *);
>  	void *(*vmap)(struct etnaviv_gem_object *);
> +	void (*vunmap)(struct etnaviv_gem_object *);
>  	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
>  };
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index 6b98200068e4..bea50d720450 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -62,11 +62,6 @@ void etnaviv_gem_prime_unpin(struct drm_gem_object *obj)
>  
>  static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)
>  {
> -	struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
> -
> -	if (etnaviv_obj->vaddr)
> -		dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
> -
>  	/* Don't drop the pages for imported dmabuf, as they are not
>  	 * ours, just free the array we allocated:
>  	 */
> @@ -88,6 +83,13 @@ static void *etnaviv_gem_prime_vmap_impl(struct etnaviv_gem_object *etnaviv_obj)
>  	return map.vaddr;
>  }
>  
> +static void etnaviv_gem_prime_vunmap(struct etnaviv_gem_object *etnaviv_obj)
> +{
> +	struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
> +
> +	dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
> +}
> +
>  static int etnaviv_gem_prime_mmap_obj(struct etnaviv_gem_object *etnaviv_obj,
>  		struct vm_area_struct *vma)
>  {
> @@ -106,6 +108,7 @@ static const struct etnaviv_gem_ops etnaviv_gem_prime_ops = {
>  	/* .get_pages should never be called */
>  	.release = etnaviv_gem_prime_release,
>  	.vmap = etnaviv_gem_prime_vmap_impl,
> +	.vunmap = etnaviv_gem_prime_vunmap,
>  	.mmap = etnaviv_gem_prime_mmap_obj,
>  };
>  

Re: [PATCH v15 04/19] drm/etnaviv: Make etnaviv_gem_prime_vmap() a static function

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> The etnaviv_gem_prime_vmap() function has no caller in the
> etnaviv_gem_prime.c file, move it into etnaviv_gem.c file.
> While at it, rename it as etnaviv_gem_object_vmap(), since
> it is a intermidiate layer function, it has no direct relation
> ship with the PRIME. The etnaviv_gem_prime.c file already has
> etnaviv_gem_prime_vmap_impl() as the implementation to vmap
> a imported GEM buffer object.
> 
I don't agree with the premise with this patch. This function is
clearly prime specific, so I don't think it should move.

Regards,
Lucas

> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  1 -
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 16 +++++++++++++++-
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 12 ------------
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 2eb2ff13f6e8..c217b54b214c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -55,7 +55,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>  
>  int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>  struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
> -int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map);
>  struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  	struct dma_buf_attachment *attach, struct sg_table *sg);
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index fad23494d08e..85d4e7c87a6a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -6,6 +6,7 @@
>  #include <drm/drm_gem.h>
>  #include <drm/drm_prime.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/iosys-map.h>
>  #include <linux/shmem_fs.h>
>  #include <linux/spinlock.h>
>  #include <linux/vmalloc.h>
> @@ -340,6 +341,19 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
>  	return etnaviv_obj->vaddr;
>  }
>  
> +static int etnaviv_gem_object_vmap(struct drm_gem_object *obj,
> +				   struct iosys_map *map)
> +{
> +	void *vaddr;
> +
> +	vaddr = etnaviv_gem_vmap(obj);
> +	if (!vaddr)
> +		return -ENOMEM;
> +	iosys_map_set_vaddr(map, vaddr);
> +
> +	return 0;
> +}
> +
>  void etnaviv_gem_vunmap(struct drm_gem_object *obj)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> @@ -595,7 +609,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>  	.pin = etnaviv_gem_prime_pin,
>  	.unpin = etnaviv_gem_prime_unpin,
>  	.get_sg_table = etnaviv_gem_prime_get_sg_table,
> -	.vmap = etnaviv_gem_prime_vmap,
> +	.vmap = etnaviv_gem_object_vmap,
>  	.vunmap = etnaviv_gem_object_vunmap,
>  	.mmap = etnaviv_gem_mmap,
>  	.vm_ops = &vm_ops,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index bea50d720450..8f523cbee60a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -25,18 +25,6 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj)
>  	return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages);
>  }
>  
> -int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
> -{
> -	void *vaddr;
> -
> -	vaddr = etnaviv_gem_vmap(obj);
> -	if (!vaddr)
> -		return -ENOMEM;
> -	iosys_map_set_vaddr(map, vaddr);
> -
> -	return 0;
> -}
> -
>  int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
>  {
>  	if (!obj->import_attach) {

Re: [PATCH v15 05/19] drm/etnaviv: Add contructor and destructor for etnaviv_gem_get_mapping structure

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> Because this make the code more easier to understand, When GPU access the
> VRAM, it will allocate a new mapping to use if there don't have one.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 40 +++++++++++++++++++--------
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  6 ++++
>  2 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 85d4e7c87a6a..55004fa9fabd 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -227,6 +227,30 @@ etnaviv_gem_get_vram_mapping(struct etnaviv_gem_object *obj,
>  	return NULL;
>  }
>  
> +static struct etnaviv_vram_mapping *
> +etnaviv_gem_vram_mapping_new(struct etnaviv_gem_object *etnaviv_obj)
> +{
> +	struct etnaviv_vram_mapping *mapping;
> +
> +	mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> +	if (!mapping)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&mapping->scan_node);
> +	mapping->object = etnaviv_obj;
> +	mapping->use = 1;
> +
> +	list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
> +
> +	return mapping;
> +}
> +
> +static void etnaviv_gem_vram_mapping_destroy(struct etnaviv_vram_mapping *mapping)
> +{
> +	list_del(&mapping->obj_node);
> +	kfree(mapping);
> +}
> +
>  void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
>  {
>  	struct etnaviv_gem_object *etnaviv_obj = mapping->object;
> @@ -289,27 +313,20 @@ struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
>  	 */
>  	mapping = etnaviv_gem_get_vram_mapping(etnaviv_obj, NULL);
>  	if (!mapping) {
> -		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> +		mapping = etnaviv_gem_vram_mapping_new(etnaviv_obj);
>  		if (!mapping) {
>  			ret = -ENOMEM;
>  			goto out;
>  		}
> -
> -		INIT_LIST_HEAD(&mapping->scan_node);
> -		mapping->object = etnaviv_obj;
>  	} else {
> -		list_del(&mapping->obj_node);
> +		mapping->use = 1;
>  	}
>  
> -	mapping->use = 1;
> -
>  	ret = etnaviv_iommu_map_gem(mmu_context, etnaviv_obj,
>  				    mmu_context->global->memory_base,
>  				    mapping, va);
>  	if (ret < 0)
> -		kfree(mapping);
> -	else
> -		list_add_tail(&mapping->obj_node, &etnaviv_obj->vram_list);
> +		etnaviv_gem_vram_mapping_destroy(mapping);
>  
>  out:
>  	mutex_unlock(&etnaviv_obj->lock);
> @@ -544,8 +561,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  		if (context)
>  			etnaviv_iommu_unmap_gem(context, mapping);
>  
> -		list_del(&mapping->obj_node);
> -		kfree(mapping);
> +		etnaviv_gem_vram_mapping_destroy(mapping);
>  	}
>  
>  	etnaviv_obj->ops->vunmap(etnaviv_obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index d4965de3007c..f2ac64d8e90b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -31,6 +31,12 @@ struct etnaviv_vram_mapping {
>  	u32 iova;
>  };
>  
> +static inline struct etnaviv_vram_mapping *
> +to_etnaviv_vram_mapping(struct drm_mm_node *vram)
> +{
> +	return container_of(vram, struct etnaviv_vram_mapping, vram_node);
> +}
> +
This hunk looks unrelated to this patch. Otherwise patch looks good.

Regards,
Lucas

>  struct etnaviv_gem_object {
>  	struct drm_gem_object base;
>  	const struct etnaviv_gem_ops *ops;

Re: [PATCH v15 08/19] drm/etnaviv: Fix wrong caching mode being used for non writecombine buffers

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> In the etnaviv_gem_vmap_impl() function, the driver vmap whatever buffers
> with write combine(WC) page property. This is incorrect, as some platforms
> are cached coherent. Cached buffers should be mapped with cached page
> property.
> 
> Fixes: a0a5ab3e99b8 ("drm/etnaviv: call correct function when trying to vmap a DMABUF")
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 1fd2cff20ef4..b899aea64e22 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -393,6 +393,7 @@ static void etnaviv_gem_object_vunmap(struct drm_gem_object *obj,
>  static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
>  {
>  	struct page **pages;
> +	pgprot_t prot;
>  
>  	lockdep_assert_held(&obj->lock);
>  
> @@ -400,8 +401,19 @@ static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
>  	if (IS_ERR(pages))
>  		return NULL;
>  
> -	return vmap(pages, obj->base.size >> PAGE_SHIFT,
> -			VM_MAP, pgprot_writecombine(PAGE_KERNEL));
> +	switch (obj->flags) {

obj->flags & ETNA_BO_CACHE_MASK

> +	case ETNA_BO_CACHED:
> +		prot = PAGE_KERNEL;
> +		break;
> +	case ETNA_BO_UNCACHED:
> +		prot = pgprot_noncached(PAGE_KERNEL);
> +		break;
> +	case ETNA_BO_WC:
> +	default:
> +		prot = pgprot_writecombine(PAGE_KERNEL);
> +	}
> +
> +	return vmap(pages, obj->base.size >> PAGE_SHIFT, VM_MAP, prot);
>  }
>  
>  static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)

Re: [PATCH v15 04/19] drm/etnaviv: Make etnaviv_gem_prime_vmap() a static function

[Sui Jingfeng]

Hi,

On 2024/10/1 21:40, Lucas Stach wrote:
> Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
>> The etnaviv_gem_prime_vmap() function has no caller in the
>> etnaviv_gem_prime.c file, move it into etnaviv_gem.c file.
>> While at it, rename it as etnaviv_gem_object_vmap(), since
>> it is a intermidiate layer function, it has no direct relation
>> ship with the PRIME. The etnaviv_gem_prime.c file already has
>> etnaviv_gem_prime_vmap_impl() as the implementation to vmap
>> a imported GEM buffer object.
>>
> I don't agree with the premise with this patch.

I think it is a fact, not a premise.

> This function is
> clearly prime specific,

Because the drm_gem_object_funcs::vmap() will be called by the drm_gem_vmap().
Therefore, all etnaviv GEM buffer object has to response to the invoke of the
drm_gem_object_funcs::vmap() interface.

- Dedicated VRAM buffer object
- SHMEM buffer object
- PRIME buffer object
- userptr (I'm not sure if this one has the need)

> so I don't think it should move.

The name of the etnaviv_gem_prime_vmap() sounds like that
a PRIME buffer object is being vmapped. But dedicated VRAM
backed BO, SHMEM backed BO can also be vmapped as well. So
I think the etnaviv_gem_object_vmap() should be universal.


> Regards,
> Lucas
>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.h       |  1 -
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 16 +++++++++++++++-
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 12 ------------
>>   3 files changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> index 2eb2ff13f6e8..c217b54b214c 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
>> @@ -55,7 +55,6 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>>   
>>   int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
>>   struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
>> -int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map);
>>   struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>>   	struct dma_buf_attachment *attach, struct sg_table *sg);
>>   int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index fad23494d08e..85d4e7c87a6a 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -6,6 +6,7 @@
>>   #include <drm/drm_gem.h>
>>   #include <drm/drm_prime.h>
>>   #include <linux/dma-mapping.h>
>> +#include <linux/iosys-map.h>
>>   #include <linux/shmem_fs.h>
>>   #include <linux/spinlock.h>
>>   #include <linux/vmalloc.h>
>> @@ -340,6 +341,19 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
>>   	return etnaviv_obj->vaddr;
>>   }
>>   
>> +static int etnaviv_gem_object_vmap(struct drm_gem_object *obj,
>> +				   struct iosys_map *map)
>> +{
>> +	void *vaddr;
>> +
>> +	vaddr = etnaviv_gem_vmap(obj);
>> +	if (!vaddr)
>> +		return -ENOMEM;
>> +	iosys_map_set_vaddr(map, vaddr);
>> +
>> +	return 0;
>> +}
>> +
>>   void etnaviv_gem_vunmap(struct drm_gem_object *obj)
>>   {
>>   	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>> @@ -595,7 +609,7 @@ static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>>   	.pin = etnaviv_gem_prime_pin,
>>   	.unpin = etnaviv_gem_prime_unpin,
>>   	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>> -	.vmap = etnaviv_gem_prime_vmap,
>> +	.vmap = etnaviv_gem_object_vmap,
>>   	.vunmap = etnaviv_gem_object_vunmap,
>>   	.mmap = etnaviv_gem_mmap,
>>   	.vm_ops = &vm_ops,
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> index bea50d720450..8f523cbee60a 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> @@ -25,18 +25,6 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj)
>>   	return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages);
>>   }
>>   
>> -int etnaviv_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map)
>> -{
>> -	void *vaddr;
>> -
>> -	vaddr = etnaviv_gem_vmap(obj);
>> -	if (!vaddr)
>> -		return -ENOMEM;
>> -	iosys_map_set_vaddr(map, vaddr);
>> -
>> -	return 0;
>> -}
>> -
>>   int etnaviv_gem_prime_pin(struct drm_gem_object *obj)
>>   {
>>   	if (!obj->import_attach) {

-- 
Best regards,
Sui

Re: [PATCH v15 09/19] drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> Because there are a lot of data members in the struct etnaviv_drm_private,
> which are intended to be shared by all GPU cores. It can be lengthy and
> daunting on error handling, the 'gem_lock' of struct etnaviv_drm_private
> just be forgeten to destroy on driver leave.
> 
As you seem to have based this patch on top of "drm/etnaviv: Fix
missing mutex_destroy()", the last part of the above sentence doesn't
match the code. Please drop.

> Switch to use the dedicated helpers introduced, etnaviv_bind() and
> etnaviv_unbind() gets simplified. Another potential benefit is that
> we could put the struct drm_device into struct etnaviv_drm_private
> in the future, which made them share the same life time.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 73 +++++++++++++++++----------
>  1 file changed, 46 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6591e420a051..809e5db85df4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -41,6 +41,45 @@ static struct device_node *etnaviv_of_first_available_node(void)
>  	return NULL;
>  }
>  
> +static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
> +{
> +	struct etnaviv_drm_private *priv;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return ERR_PTR(-ENOMEM);
> +
> +	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
> +
> +	mutex_init(&priv->gem_lock);
> +	INIT_LIST_HEAD(&priv->gem_list);
> +	priv->num_gpus = 0;
> +	priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> +
> +	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
> +	if (IS_ERR(priv->cmdbuf_suballoc)) {

If this is supposed to do everything by the books, we should also
destroy the gem_lock mutex and active_contexts xarray here.

Regards,
Lucas

> +		kfree(priv);
> +		dev_err(dev, "Failed to create cmdbuf suballocator\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	return priv;
> +}
> +
> +static void etnaviv_free_private(struct etnaviv_drm_private *priv)
> +{
> +	if (!priv)
> +		return;
> +
> +	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
> +
> +	xa_destroy(&priv->active_contexts);
> +
> +	mutex_destroy(&priv->gem_lock);
> +
> +	kfree(priv);
> +}
> +
>  static void load_gpu(struct drm_device *dev)
>  {
>  	struct etnaviv_drm_private *priv = dev->dev_private;
> @@ -521,35 +560,21 @@ static int etnaviv_bind(struct device *dev)
>  	if (IS_ERR(drm))
>  		return PTR_ERR(drm);
>  
> -	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> -	if (!priv) {
> -		dev_err(dev, "failed to allocate private data\n");
> -		ret = -ENOMEM;
> +	priv = etnaviv_alloc_private(dev);
> +	if (IS_ERR(priv)) {
> +		ret = PTR_ERR(priv);
>  		goto out_put;
>  	}
> +
>  	drm->dev_private = priv;
>  
>  	dma_set_max_seg_size(dev, SZ_2G);
>  
> -	xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC);
> -
> -	mutex_init(&priv->gem_lock);
> -	INIT_LIST_HEAD(&priv->gem_list);
> -	priv->num_gpus = 0;
> -	priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> -
> -	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
> -	if (IS_ERR(priv->cmdbuf_suballoc)) {
> -		dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
> -		ret = PTR_ERR(priv->cmdbuf_suballoc);
> -		goto out_free_priv;
> -	}
> -
>  	dev_set_drvdata(dev, drm);
>  
>  	ret = component_bind_all(dev, drm);
>  	if (ret < 0)
> -		goto out_destroy_suballoc;
> +		goto out_free_priv;
>  
>  	load_gpu(drm);
>  
> @@ -561,11 +586,8 @@ static int etnaviv_bind(struct device *dev)
>  
>  out_unbind:
>  	component_unbind_all(dev, drm);
> -out_destroy_suballoc:
> -	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
>  out_free_priv:
> -	mutex_destroy(&priv->gem_lock);
> -	kfree(priv);
> +	etnaviv_free_private(priv);
>  out_put:
>  	drm_dev_put(drm);
>  
> @@ -581,12 +603,9 @@ static void etnaviv_unbind(struct device *dev)
>  
>  	component_unbind_all(dev, drm);
>  
> -	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
> -
> -	xa_destroy(&priv->active_contexts);
> +	etnaviv_free_private(priv);
>  
>  	drm->dev_private = NULL;
> -	kfree(priv);
>  
>  	drm_dev_put(drm);
>  }

Re: [PATCH v15 11/19] drm/etnaviv: Add etnaviv_gem_obj_remove() helper

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> Which is corresonding to the etnaviv_gem_obj_add()
> 
While symmetry is nice, it's still not really symmetric, as this
function isn't exported into the PRIME parts of the driver like
etnaviv_gem_obj_add(). Given that I don't really see how this patch
improves code legibility.

Regards,
Lucas

> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 39cfece67b90..3732288ff530 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -19,6 +19,8 @@
>  static struct lock_class_key etnaviv_shm_lock_class;
>  static struct lock_class_key etnaviv_userptr_lock_class;
>  
> +static void etnaviv_gem_obj_remove(struct drm_gem_object *obj);
> +
>  static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj)
>  {
>  	struct drm_device *dev = etnaviv_obj->base.dev;
> @@ -555,15 +557,12 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  {
>  	struct drm_device *drm = obj->dev;
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> -	struct etnaviv_drm_private *priv = obj->dev->dev_private;
>  	struct etnaviv_vram_mapping *mapping, *tmp;
>  
>  	/* object should not be active */
>  	drm_WARN_ON(drm, is_active(etnaviv_obj));
>  
> -	mutex_lock(&priv->gem_lock);
> -	list_del(&etnaviv_obj->gem_node);
> -	mutex_unlock(&priv->gem_lock);
> +	etnaviv_gem_obj_remove(obj);
>  
>  	list_for_each_entry_safe(mapping, tmp, &etnaviv_obj->vram_list,
>  				 obj_node) {
> @@ -595,6 +594,16 @@ void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>  	mutex_unlock(&priv->gem_lock);
>  }
>  
> +static void etnaviv_gem_obj_remove(struct drm_gem_object *obj)
> +{
> +	struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> +
> +	mutex_lock(&priv->gem_lock);
> +	list_del(&etnaviv_obj->gem_node);
> +	mutex_unlock(&priv->gem_lock);
> +}
> +
>  static const struct vm_operations_struct vm_ops = {
>  	.fault = etnaviv_gem_fault,
>  	.open = drm_gem_vm_open,

Re: [PATCH v15 16/19] drm/etnaviv: Call etnaviv_gem_obj_add() in ernaviv_gem_new_private()

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> The etnaviv_gem_obj_add() a common operation, the 'etnaviv_drm_private::
> gem_list' is being used to record(track) all of the etnaviv GEM buffer
> object created in this driver.
> 
> Once a etnaviv GEM buffer object has been allocated successfully, we
> should add it into the global etnaviv_drm_private::gem_list'. Because
> we need to free it and remove it free the list if the invoke of the
> subsequent functions fail.
> 
> The only way that destroy etnaviv GEM buffer object is the implementation
> of etnaviv_gem_free_object() function. The etnaviv_gem_free_object() first
> remove the etnaviv GEM object from the list, then destroy its mapping and
> finally free its memory footprint. Therefore, in order to corresponding
> this, we add the freshly created etnaviv GEM buffer object immediately
> after it was successfully created.
> 
> A benifit is that we only need to call etnaviv_gem_obj_add() once now,
> since the ernaviv_gem_new_private() has been unified. Make the
> etnaviv_gem_obj_add() static is a next nature thing.
> 
Seeing this patch, I would really ask you to drop patch 11 from this
series and go the other way around: remove etnaviv_gem_obj_add() here
altogether and simply inline the few lines of code into
etnaviv_gem_new_private().

Regards,
Lucas

> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 8 +++-----
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h       | 1 -
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 --
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 27e4a93c981c..ee799c02d0aa 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -584,7 +584,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  	kfree(etnaviv_obj);
>  }
>  
> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
> +static void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>  {
>  	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
>  	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> @@ -719,8 +719,6 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
>  	 */
>  	mapping_set_gfp_mask(obj->filp->f_mapping, priv->shm_gfp_mask);
>  
> -	etnaviv_gem_obj_add(dev, obj);
> -
>  	ret = drm_gem_handle_create(file, obj, handle);
>  
>  	/* drop reference from allocate - handle holds it now */
> @@ -751,6 +749,8 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>  		drm_gem_private_object_init(dev, obj, size);
>  	}
>  
> +	etnaviv_gem_obj_add(dev, obj);
> +
>  	*res = to_etnaviv_bo(obj);
>  
>  	return 0;
> @@ -849,8 +849,6 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
>  	etnaviv_obj->userptr.mm = current->mm;
>  	etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE);
>  
> -	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
> -
>  	ret = drm_gem_handle_create(file, &etnaviv_obj->base, handle);
>  
>  	/* drop reference from allocate - handle holds it now */
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index b174a9e4cc48..8d8fc5b3a541 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -121,7 +121,6 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>  			    bool shmem, const struct etnaviv_gem_ops *ops,
>  			    struct etnaviv_gem_object **res);
>  
> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
>  struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj);
>  void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index 64a858a0b0cf..04ee31461b8c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -127,8 +127,6 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>  	if (ret)
>  		goto fail;
>  
> -	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
> -
>  	return &etnaviv_obj->base;
>  
>  fail:

Re: [PATCH v15 18/19] drm/etnaviv: Allow userspace specify the domain of etnaviv GEM buffer object

[Lucas Stach]

Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
> Otherwise we don't know where a etnaviv GEM buffer object should put when
> we create it at userspace.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c |  9 +++++++++
>  include/uapi/drm/etnaviv_drm.h        | 12 ++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index f10661fe079f..cdc62f64b200 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -331,11 +331,20 @@ static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
>  		struct drm_file *file)
>  {
>  	struct drm_etnaviv_gem_new *args = data;
> +	u32 domain;
> +
> +	domain = args->flags & ETNA_BO_DOMAIN_MASK;
> +
> +	args->flags &= ~ETNA_BO_DOMAIN_MASK;

This is not a proper input validation, as it would accept data in the
domain mask range that doesn't correspond to valid flags. You need to
add your new valid flag bits to the check below.

>  
>  	if (args->flags & ~(ETNA_BO_CACHED | ETNA_BO_WC | ETNA_BO_UNCACHED |
>  			    ETNA_BO_FORCE_MMU))
>  		return -EINVAL;
>  
> +	if (domain == ETNA_BO_PL_VRAM)
> +		return etnaviv_gem_new_vram(dev, file, args->size,
> +					    args->flags, &args->handle);
> +
>  	return etnaviv_gem_new_handle(dev, file, args->size,
>  			args->flags, &args->handle);
>  }
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 61eaa8cd0f5e..00e778c9d312 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -99,6 +99,18 @@ struct drm_etnaviv_param {
>  /* map flags */
>  #define ETNA_BO_FORCE_MMU    0x00100000
>  
> +/* domain (placement) flags */
> +#define ETNA_BO_DOMAIN_MASK  0x00f00000

How does this work? Has this been tested? This define masks different
bits than the placement flags defined below.
> 
> +
> +/* CPU accessible, GPU accessible pages in dedicated VRAM */
> +#define ETNA_BO_PL_VRAM      0x01000000

Other drivers call this the visible VRAM range.

> +/* CPU accessible, GPU accessible pages in SHMEM */
> +#define ETNA_BO_PL_GTT       0x02000000
> +/* Userspace allocated memory, at least CPU accessible */
> +#define ETNA_BO_PL_USERPTR   0x08000000

How is this a valid placement? If it's userspace allocated memory, the
driver has no influence on placement. All it can do is to pin the pages
and set up a GART mapping.

> +/* GPU accessible but CPU not accessible private VRAM pages */
> +#define ETNA_BO_PL_PRIV      0x04000000
> +

VRAM_INVISIBLE would be a more descriptive name for the flag than PRIV.

However I'm not sure if we can make the distinction between visible and
invisible VRAM at allocation time. What needs to be CPU visible may
change over the runtime of the workload, which is why real TTM drivers
can migrate BOs in and out of the visible region.

Regards,
Lucas

>  struct drm_etnaviv_gem_new {
>  	__u64 size;           /* in */
>  	__u32 flags;          /* in, mask of ETNA_BO_x */

Re: [PATCH v15 11/19] drm/etnaviv: Add etnaviv_gem_obj_remove() helper

[Sui Jingfeng]

Hi,

On 2024/10/1 22:21, Lucas Stach wrote:
> Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
>> Which is corresonding to the etnaviv_gem_obj_add()
>>
> While symmetry is nice,


Thanks a lot for understanding and review my patch.


> it's still not really symmetric,

patch 0016 will try try to make it symmetric.
It will do this uniformly for all etnaviv GEM buffer objects.


> as this
> function isn't exported into the PRIME parts of the driver like
> etnaviv_gem_obj_add().

Yes, exactly.

> Given that I don't really see how this patch
> improves code legibility.

When the reference counter of a GEM buffer object decrease to 0,
the drm_gem_object_free() will be get called. which in turn,
etnaviv_gem_free_object() will get called.

The etnaviv_gem_free_object() will remove the GEM BO node
from the 'priv->gem_list' without checking if it has been
added into the list.

The data field of the struct etnaviv_gem_object::gem_node
will be all ZERO under such a case.

When drm/etnaviv import a shared buffer from an another driver.
etnaviv_gem_prime_import_sg_table() will be get called. But it
could fails before the "etnaviv_gem_obj_add(dev, &etnaviv_obj->base)"
get executed. The fails might either due to out of memory or
drm_prime_sg_to_page_array() failed.


Those fails will lead to NULL pointer de-reference, as we will
use uninitialized data member(say the 'gem_node') of an GEM
buffer object.

This is also the reason why we want to add it into the
etnaviv_drm_private::gem_list immediately after an etnaviv
GEM buffer object is successfully created.

> Regards,
> Lucas
>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c | 17 +++++++++++++----
>>   1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index 39cfece67b90..3732288ff530 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -19,6 +19,8 @@
>>   static struct lock_class_key etnaviv_shm_lock_class;
>>   static struct lock_class_key etnaviv_userptr_lock_class;
>>   
>> +static void etnaviv_gem_obj_remove(struct drm_gem_object *obj);
>> +
>>   static void etnaviv_gem_scatter_map(struct etnaviv_gem_object *etnaviv_obj)
>>   {
>>   	struct drm_device *dev = etnaviv_obj->base.dev;
>> @@ -555,15 +557,12 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>>   {
>>   	struct drm_device *drm = obj->dev;
>>   	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>> -	struct etnaviv_drm_private *priv = obj->dev->dev_private;
>>   	struct etnaviv_vram_mapping *mapping, *tmp;
>>   
>>   	/* object should not be active */
>>   	drm_WARN_ON(drm, is_active(etnaviv_obj));
>>   
>> -	mutex_lock(&priv->gem_lock);
>> -	list_del(&etnaviv_obj->gem_node);
>> -	mutex_unlock(&priv->gem_lock);
>> +	etnaviv_gem_obj_remove(obj);
>>   
>>   	list_for_each_entry_safe(mapping, tmp, &etnaviv_obj->vram_list,
>>   				 obj_node) {
>> @@ -595,6 +594,16 @@ void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>>   	mutex_unlock(&priv->gem_lock);
>>   }
>>   
>> +static void etnaviv_gem_obj_remove(struct drm_gem_object *obj)
>> +{
>> +	struct etnaviv_drm_private *priv = to_etnaviv_priv(obj->dev);
>> +	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>> +
>> +	mutex_lock(&priv->gem_lock);
>> +	list_del(&etnaviv_obj->gem_node);
>> +	mutex_unlock(&priv->gem_lock);
>> +}
>> +
>>   static const struct vm_operations_struct vm_ops = {
>>   	.fault = etnaviv_gem_fault,
>>   	.open = drm_gem_vm_open,

-- 
Best regards,
Sui

Re: [PATCH v15 16/19] drm/etnaviv: Call etnaviv_gem_obj_add() in ernaviv_gem_new_private()

[Sui Jingfeng]

Hi,

On 2024/10/1 22:39, Lucas Stach wrote:
> Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
>> The etnaviv_gem_obj_add() a common operation, the 'etnaviv_drm_private::
>> gem_list' is being used to record(track) all of the etnaviv GEM buffer
>> object created in this driver.
>>
>> Once a etnaviv GEM buffer object has been allocated successfully, we
>> should add it into the global etnaviv_drm_private::gem_list'. Because
>> we need to free it and remove it free the list if the invoke of the
>> subsequent functions fail.
>>
>> The only way that destroy etnaviv GEM buffer object is the implementation
>> of etnaviv_gem_free_object() function. The etnaviv_gem_free_object() first
>> remove the etnaviv GEM object from the list, then destroy its mapping and
>> finally free its memory footprint. Therefore, in order to corresponding
>> this, we add the freshly created etnaviv GEM buffer object immediately
>> after it was successfully created.
>>
>> A benifit is that we only need to call etnaviv_gem_obj_add() once now,
>> since the ernaviv_gem_new_private() has been unified. Make the
>> etnaviv_gem_obj_add() static is a next nature thing.
>>
> Seeing this patch, I would really ask you to drop patch 11 from this
> series and go the other way around: remove etnaviv_gem_obj_add() here
> altogether and simply inline the few lines of code into
> etnaviv_gem_new_private().


The 'etnaviv_drm_private::gem_list' is being used to manage the buffer
object itself, not the backing memory of a specific buffer object.

Therefore, once a etnaviv GEM buffer object is successfully created,
we want to add it into the global etnaviv_drm_private::gem_list'.
To prevent the cases which removing an entry that is not in the list.

Both etnaviv_gem_obj_add() and etnaviv_gem_obj_remove() touches driver
private but shared data members. Which has nothing to do with the GEM
core functionality itself.

The unction pair is for tracking, recording, listing and showing
purpose. Which is clearly separated functionality. Split them from
the BO construction is a untangled fashion and is a good thing.


> Regards,
> Lucas
>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c       | 8 +++-----
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.h       | 1 -
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 --
>>   3 files changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index 27e4a93c981c..ee799c02d0aa 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -584,7 +584,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>>   	kfree(etnaviv_obj);
>>   }
>>   
>> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>> +static void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj)
>>   {
>>   	struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
>>   	struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
>> @@ -719,8 +719,6 @@ int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file,
>>   	 */
>>   	mapping_set_gfp_mask(obj->filp->f_mapping, priv->shm_gfp_mask);
>>   
>> -	etnaviv_gem_obj_add(dev, obj);
>> -
>>   	ret = drm_gem_handle_create(file, obj, handle);
>>   
>>   	/* drop reference from allocate - handle holds it now */
>> @@ -751,6 +749,8 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>>   		drm_gem_private_object_init(dev, obj, size);
>>   	}
>>   
>> +	etnaviv_gem_obj_add(dev, obj);
>> +
>>   	*res = to_etnaviv_bo(obj);
>>   
>>   	return 0;
>> @@ -849,8 +849,6 @@ int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file,
>>   	etnaviv_obj->userptr.mm = current->mm;
>>   	etnaviv_obj->userptr.ro = !(flags & ETNA_USERPTR_WRITE);
>>   
>> -	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
>> -
>>   	ret = drm_gem_handle_create(file, &etnaviv_obj->base, handle);
>>   
>>   	/* drop reference from allocate - handle holds it now */
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> index b174a9e4cc48..8d8fc5b3a541 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> @@ -121,7 +121,6 @@ int etnaviv_gem_new_private(struct drm_device *dev, size_t size, u32 flags,
>>   			    bool shmem, const struct etnaviv_gem_ops *ops,
>>   			    struct etnaviv_gem_object **res);
>>   
>> -void etnaviv_gem_obj_add(struct drm_device *dev, struct drm_gem_object *obj);
>>   struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *obj);
>>   void etnaviv_gem_put_pages(struct etnaviv_gem_object *obj);
>>   
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> index 64a858a0b0cf..04ee31461b8c 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
>> @@ -127,8 +127,6 @@ struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device *dev,
>>   	if (ret)
>>   		goto fail;
>>   
>> -	etnaviv_gem_obj_add(dev, &etnaviv_obj->base);
>> -
>>   	return &etnaviv_obj->base;
>>   
>>   fail:

-- 
Best regards,
Sui

Re: [PATCH v15 01/19] drm/etnaviv: Implement drm_gem_object_funcs::print_info()

[Sui Jingfeng]

Hi,

On 2024/10/1 21:04, Lucas Stach wrote:
> Hi Sui,
>
> Am Sonntag, dem 08.09.2024 um 17:43 +0800 schrieb Sui Jingfeng:
>> It will be called by drm_gem_print_info() if implemented, and it can
>> provide more information about the framebuffer objects.
> Etnaviv GEM BOs are not framebuffer objects.
>
>> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++++++++++++++++++++++++++
>>   drivers/gpu/drm/etnaviv/etnaviv_gem.h |  2 +-
>>   2 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> index 4247a10f8d4f..543d881585b3 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
>> @@ -534,8 +534,40 @@ static const struct vm_operations_struct vm_ops = {
>>   	.close = drm_gem_vm_close,
>>   };
>>   
>> +static const char *etnaviv_gem_obj_caching_info(u32 flags)
>> +{
>> +	switch (flags & ETNA_BO_CACHE_MASK) {
>> +	case ETNA_BO_CACHED:
>> +		return "cached";
>> +	case ETNA_BO_UNCACHED:
>> +		return "uncached";
>> +	case ETNA_BO_WC:
>> +		return "write-combine";
> "write-combined" to be consistent with the other two.


OK,


>> +	default:
>> +		break;
>> +	}
>> +
>> +	return "unknown";
>> +}
>> +
>> +static void etnaviv_gem_object_info(struct drm_printer *p,
>> +				    unsigned int indent,
>> +				    const struct drm_gem_object *obj)
>> +{
>> +	const struct etnaviv_gem_object *etnaviv_obj;
>> +
>> +	etnaviv_obj = container_of(obj, struct etnaviv_gem_object, base);
>> +
>> +	drm_printf_indent(p, indent, "caching mode=%s\n",
>> +			  etnaviv_gem_obj_caching_info(etnaviv_obj->flags));
>> +	drm_printf_indent(p, indent, "active=%s\n",
>> +			  str_yes_no(is_active(etnaviv_obj)));
>> +	drm_printf_indent(p, indent, "vaddr=%p\n", etnaviv_obj->vaddr);
> Why should we expose the vaddr to userspace? I don't see why this would
> be relevant even as debug info and it leaks the kernel vmap area
> address, which could be abused to facilitate kernel exploits.


This is nearly a re-implement for the etnaviv_gem_describe(),

It's not intend to leak, but to give us a *hint* that
if a specific buffer object have been VMAP-ed and
we will know how many BOs have been vmap-ed.


> Regards,
> Lucas
>
>> +}
>> +
>>   static const struct drm_gem_object_funcs etnaviv_gem_object_funcs = {
>>   	.free = etnaviv_gem_free_object,
>> +	.print_info = etnaviv_gem_object_info,
>>   	.pin = etnaviv_gem_prime_pin,
>>   	.unpin = etnaviv_gem_prime_unpin,
>>   	.get_sg_table = etnaviv_gem_prime_get_sg_table,
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> index a42d260cac2c..3f8fe19a77cc 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>> @@ -68,7 +68,7 @@ struct etnaviv_gem_ops {
>>   	int (*mmap)(struct etnaviv_gem_object *, struct vm_area_struct *);
>>   };
>>   
>> -static inline bool is_active(struct etnaviv_gem_object *etnaviv_obj)
>> +static inline bool is_active(const struct etnaviv_gem_object *etnaviv_obj)
>>   {
>>   	return atomic_read(&etnaviv_obj->gpu_active) != 0;
>>   }

-- 
Best regards,
Sui