[PATCH v4 0/4] drm: Add shmem GEM library

[PATCH v4 0/4] drm: Add shmem GEM library

[Noralf Trønnes]

This patchset adds a library for shmem backed GEM objects and makes use
of it in tinydrm.

Thomas gave me some feedback that made me realise that the cachemodes
pattern I had picked up from some other drivers didn't apply to shmem
buffers in the way I though it did. So it's removed.

I recently did an RFC to see if it made sense to add a vtable to GEM
objects. Daniel welcomed that so I've included it here.

Noralf.

Changes since version 3:
- Drop cache modes (Thomas Hellstrom)
- Add a GEM object attached vtable

Changes since version 2:
- Grammar (Sam Ravnborg)
- s/drm_gem_shmem_put_pages_unlocked/drm_gem_shmem_put_pages_locked/
(Sam Ravnborg)
- Add debug ouput in error path (Sam Ravnborg)

Changes since version 1:
- Fix missing argument in docs (kbuild test robot)
- Fix: sparse: expression using sizeof(void) (kbuild test robot)
- Rebasing gave a new checkpatch warning, so I changed to bitfields:
CHECK: Avoid using bool structure members because of possible alignment
issues - see: https://lkml.org/lkml/2017/11/21/384
#834: FILE: include/drm/drm_gem_shmem_helper.h:84:
+       bool pages_mark_dirty_on_put;
#841: FILE: include/drm/drm_gem_shmem_helper.h:91:
+       bool pages_mark_accessed_on_put;

Noralf Trønnes (4):
  drm/driver: Add defaults for .gem_prime_export/import callbacks
  drm/gem: Add drm_gem_object_funcs
  drm: Add library for shmem backed GEM objects
  drm/tinydrm: Switch from CMA to shmem buffers

 Documentation/gpu/drm-kms-helpers.rst          |  12 +
 Documentation/gpu/todo.rst                     |   7 +
 drivers/gpu/drm/Kconfig                        |   6 +
 drivers/gpu/drm/Makefile                       |   1 +
 drivers/gpu/drm/drm_client.c                   |  12 +-
 drivers/gpu/drm/drm_fb_helper.c                |   8 +-
 drivers/gpu/drm/drm_gem.c                      | 109 ++++-
 drivers/gpu/drm/drm_gem_shmem_helper.c         | 586 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_prime.c                    |  50 ++-
 drivers/gpu/drm/tinydrm/Kconfig                |   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c    |  92 ++--
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |   5 +
 drivers/gpu/drm/tinydrm/ili9225.c              |  14 +-
 drivers/gpu/drm/tinydrm/ili9341.c              |   6 +-
 drivers/gpu/drm/tinydrm/mi0283qt.c             |   6 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c             |  38 +-
 drivers/gpu/drm/tinydrm/repaper.c              |  24 +-
 drivers/gpu/drm/tinydrm/st7586.c               |  15 +-
 drivers/gpu/drm/tinydrm/st7735r.c              |   6 +-
 include/drm/drm_drv.h                          |   4 +
 include/drm/drm_gem.h                          | 140 ++++++
 include/drm/drm_gem_shmem_helper.h             | 153 +++++++
 include/drm/tinydrm/tinydrm.h                  |  36 +-
 23 files changed, 1139 insertions(+), 193 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_gem_shmem_helper.c
 create mode 100644 include/drm/drm_gem_shmem_helper.h

-- 
2.15.1

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

[PATCH v4 1/4] drm/driver: Add defaults for .gem_prime_export/import callbacks

[Noralf Trønnes]

The majority of drivers use drm_gem_prime_export() and
drm_gem_prime_import() for these callbacks so let's make them the
default.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/gpu/todo.rst  |  7 +++++++
 drivers/gpu/drm/drm_prime.c | 10 ++++++++--
 include/drm/drm_drv.h       |  4 ++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 77c2b3c25565..39748e22dea8 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -234,6 +234,13 @@ efficient.
 
 Contact: Daniel Vetter
 
+Defaults for .gem_prime_import and export
+-----------------------------------------
+
+Most drivers don't need to set drm_driver->gem_prime_import and
+->gem_prime_export now that drm_gem_prime_import() and drm_gem_prime_export()
+are the default.
+
 Core refactorings
 =================
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 3f0205fc0a1a..6721571749ff 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -559,7 +559,10 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
 		return dmabuf;
 	}
 
-	dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
+	if (dev->driver->gem_prime_export)
+		dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
+	else
+		dmabuf = drm_gem_prime_export(dev, obj, flags);
 	if (IS_ERR(dmabuf)) {
 		/* normally the created dma-buf takes ownership of the ref,
 		 * but if that fails then drop the ref
@@ -792,7 +795,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	/* never seen this one, need to import */
 	mutex_lock(&dev->object_name_lock);
-	obj = dev->driver->gem_prime_import(dev, dma_buf);
+	if (dev->driver->gem_prime_import)
+		obj = dev->driver->gem_prime_import(dev, dma_buf);
+	else
+		obj = drm_gem_prime_import(dev, dma_buf);
 	if (IS_ERR(obj)) {
 		ret = PTR_ERR(obj);
 		goto out_unlock;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 8830e3de3a86..1162145f7f68 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -471,6 +471,8 @@ struct drm_driver {
 	 * @gem_prime_export:
 	 *
 	 * export GEM -> dmabuf
+	 *
+	 * This defaults to drm_gem_prime_export() if not set.
 	 */
 	struct dma_buf * (*gem_prime_export)(struct drm_device *dev,
 				struct drm_gem_object *obj, int flags);
@@ -478,6 +480,8 @@ struct drm_driver {
 	 * @gem_prime_import:
 	 *
 	 * import dmabuf -> GEM
+	 *
+	 * This defaults to drm_gem_prime_import() if not set.
 	 */
 	struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
 				struct dma_buf *dma_buf);
-- 
2.15.1

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

[PATCH v4 2/4] drm/gem: Add drm_gem_object_funcs

[Noralf Trønnes]

This adds an optional function table on GEM objects.
The main benefit is for drivers that support more than one type of
memory (shmem,vram,cma) for their buffers depending on the hardware it
runs on. With the callbacks attached to the GEM object itself, it is
easier to have core helpers for the the various buffer types. The driver
only has to make the decision about buffer type on GEM object creation
and all other callbacks can be handled by the chosen helper.

drm_driver->gem_prime_res_obj has not been added since there's a todo to
put a reservation_object into drm_gem_object.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---

Changes since RFC:
- drm_gem_object_funcs.map -> .prime_map let it only do PRIME mmap like
  the function it superseeds (Daniel Vetter)
- Flip around the if ladders and make obj->funcs the first choice
  highlighting the fact that this the new default way of doing it
  (Daniel Vetter)

 drivers/gpu/drm/drm_client.c    |  12 ++--
 drivers/gpu/drm/drm_fb_helper.c |   8 ++-
 drivers/gpu/drm/drm_gem.c       | 109 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/drm_prime.c     |  42 ++++++------
 include/drm/drm_gem.h           | 140 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 272 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 17d9a64e885e..eca7331762e4 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -80,8 +80,7 @@ int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
 {
 	int ret;
 
-	if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
-	    !dev->driver->dumb_create || !dev->driver->gem_prime_vmap)
+	if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create)
 		return -EOPNOTSUPP;
 
 	if (funcs && !try_module_get(funcs->owner))
@@ -212,8 +211,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
 {
 	struct drm_device *dev = buffer->client->dev;
 
-	if (buffer->vaddr && dev->driver->gem_prime_vunmap)
-		dev->driver->gem_prime_vunmap(buffer->gem, buffer->vaddr);
+	drm_gem_vunmap(buffer->gem, buffer->vaddr);
 
 	if (buffer->gem)
 		drm_gem_object_put_unlocked(buffer->gem);
@@ -266,9 +264,9 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
 	 * fd_install step out of the driver backend hooks, to make that
 	 * final step optional for internal users.
 	 */
-	vaddr = dev->driver->gem_prime_vmap(obj);
-	if (!vaddr) {
-		ret = -ENOMEM;
+	vaddr = drm_gem_vmap(obj);
+	if (IS_ERR(vaddr)) {
+		ret = PTR_ERR(vaddr);
 		goto err_delete;
 	}
 
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bf2190c5add5..0af405beceda 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -38,6 +38,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_gem.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -3010,9 +3011,12 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
 static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_gem_object *obj = fb_helper->buffer->gem;
 
-	if (fb_helper->dev->driver->gem_prime_mmap)
-		return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
+	if (obj->funcs && obj->funcs->prime_mmap)
+		return obj->funcs->prime_mmap(obj, vma);
+	else if (obj->dev->driver->gem_prime_mmap)
+		return obj->dev->driver->gem_prime_mmap(obj, vma);
 	else
 		return -ENODEV;
 }
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 512078ebd97b..dcea6f8229a4 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -257,7 +257,9 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_gem_object *obj = ptr;
 	struct drm_device *dev = obj->dev;
 
-	if (dev->driver->gem_close_object)
+	if (obj->funcs && obj->funcs->close)
+		obj->funcs->close(obj, file_priv);
+	else if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, file_priv);
 
 	if (drm_core_check_feature(dev, DRIVER_PRIME))
@@ -410,7 +412,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	if (ret)
 		goto err_remove;
 
-	if (dev->driver->gem_open_object) {
+	if (obj->funcs && obj->funcs->open) {
+		ret = obj->funcs->open(obj, file_priv);
+		if (ret)
+			goto err_revoke;
+	} else if (dev->driver->gem_open_object) {
 		ret = dev->driver->gem_open_object(obj, file_priv);
 		if (ret)
 			goto err_revoke;
@@ -835,7 +841,9 @@ drm_gem_object_free(struct kref *kref)
 		container_of(kref, struct drm_gem_object, refcount);
 	struct drm_device *dev = obj->dev;
 
-	if (dev->driver->gem_free_object_unlocked) {
+	if (obj->funcs) {
+		obj->funcs->free(obj);
+	} else if (dev->driver->gem_free_object_unlocked) {
 		dev->driver->gem_free_object_unlocked(obj);
 	} else if (dev->driver->gem_free_object) {
 		WARN_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -864,13 +872,13 @@ drm_gem_object_put_unlocked(struct drm_gem_object *obj)
 
 	dev = obj->dev;
 
-	if (dev->driver->gem_free_object_unlocked) {
-		kref_put(&obj->refcount, drm_gem_object_free);
-	} else {
+	if (dev->driver->gem_free_object) {
 		might_lock(&dev->struct_mutex);
 		if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
 				&dev->struct_mutex))
 			mutex_unlock(&dev->struct_mutex);
+	} else {
+		kref_put(&obj->refcount, drm_gem_object_free);
 	}
 }
 EXPORT_SYMBOL(drm_gem_object_put_unlocked);
@@ -960,11 +968,14 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 	if (obj_size < vma->vm_end - vma->vm_start)
 		return -EINVAL;
 
-	if (!dev->driver->gem_vm_ops)
+	if (obj->funcs && obj->funcs->vm_ops)
+		vma->vm_ops = obj->funcs->vm_ops;
+	else if (dev->driver->gem_vm_ops)
+		vma->vm_ops = dev->driver->gem_vm_ops;
+	else
 		return -EINVAL;
 
 	vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
-	vma->vm_ops = dev->driver->gem_vm_ops;
 	vma->vm_private_data = obj;
 	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
@@ -1066,6 +1077,86 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
 	drm_printf_indent(p, indent, "imported=%s\n",
 			  obj->import_attach ? "yes" : "no");
 
-	if (obj->dev->driver->gem_print_info)
+	if (obj->funcs && obj->funcs->print_info)
+		obj->funcs->print_info(p, indent, obj);
+	else if (obj->dev->driver->gem_print_info)
 		obj->dev->driver->gem_print_info(p, indent, obj);
 }
+
+/**
+ * drm_gem_pin - Pin backing buffer in memory
+ * @obj: GEM object
+ *
+ * Make sure the backing buffer is pinned in memory.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_pin(struct drm_gem_object *obj)
+{
+	if (obj->funcs && obj->funcs->pin)
+		return obj->funcs->pin(obj);
+	else if (obj->dev->driver->gem_prime_pin)
+		return obj->dev->driver->gem_prime_pin(obj);
+	else
+		return 0;
+}
+EXPORT_SYMBOL(drm_gem_pin);
+
+/**
+ * drm_gem_pin - Unpin backing buffer from memory
+ * @obj: GEM object
+ *
+ * Relax the requirement that the backing buffer is pinned in memory.
+ */
+void drm_gem_unpin(struct drm_gem_object *obj)
+{
+	if (obj->funcs && obj->funcs->unpin)
+		obj->funcs->unpin(obj);
+	else if (obj->dev->driver->gem_prime_unpin)
+		obj->dev->driver->gem_prime_unpin(obj);
+}
+EXPORT_SYMBOL(drm_gem_unpin);
+
+/**
+ * drm_gem_vmap - Map buffer into kernel virtual address space
+ * @obj: GEM object
+ *
+ * Returns:
+ * A virtual pointer to a newly created GEM object or an ERR_PTR-encoded negative
+ * error code on failure.
+ */
+void *drm_gem_vmap(struct drm_gem_object *obj)
+{
+	void *vaddr;
+
+	if (obj->funcs && obj->funcs->vmap)
+		vaddr = obj->funcs->vmap(obj);
+	else if (obj->dev->driver->gem_prime_vmap)
+		vaddr = obj->dev->driver->gem_prime_vmap(obj);
+	else
+		vaddr = ERR_PTR(-EOPNOTSUPP);
+
+	if (!vaddr)
+		vaddr = ERR_PTR(-ENOMEM);
+
+	return vaddr;
+}
+EXPORT_SYMBOL(drm_gem_vmap);
+
+/**
+ * drm_gem_vunmap - Remove buffer mapping from kernel virtual address space
+ * @obj: GEM object
+ * @vaddr: Virtual address (can be NULL)
+ */
+void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr)
+{
+	if (!vaddr)
+		return;
+
+	if (obj->funcs && obj->funcs->vunmap)
+		obj->funcs->vunmap(obj, vaddr);
+	else if (obj->dev->driver->gem_prime_vunmap)
+		obj->dev->driver->gem_prime_vunmap(obj, vaddr);
+}
+EXPORT_SYMBOL(drm_gem_vunmap);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 6721571749ff..f231c199d38f 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -199,7 +199,6 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
 {
 	struct drm_prime_attachment *prime_attach;
 	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
 
 	prime_attach = kzalloc(sizeof(*prime_attach), GFP_KERNEL);
 	if (!prime_attach)
@@ -208,10 +207,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
 	prime_attach->dir = DMA_NONE;
 	attach->priv = prime_attach;
 
-	if (!dev->driver->gem_prime_pin)
-		return 0;
-
-	return dev->driver->gem_prime_pin(obj);
+	return drm_gem_pin(obj);
 }
 EXPORT_SYMBOL(drm_gem_map_attach);
 
@@ -228,7 +224,6 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
 {
 	struct drm_prime_attachment *prime_attach = attach->priv;
 	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
 
 	if (prime_attach) {
 		struct sg_table *sgt = prime_attach->sgt;
@@ -247,8 +242,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
 		attach->priv = NULL;
 	}
 
-	if (dev->driver->gem_prime_unpin)
-		dev->driver->gem_prime_unpin(obj);
+	drm_gem_unpin(obj);
 }
 EXPORT_SYMBOL(drm_gem_map_detach);
 
@@ -310,7 +304,10 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
 	if (WARN_ON(prime_attach->dir != DMA_NONE))
 		return ERR_PTR(-EBUSY);
 
-	sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
+	if (obj->funcs)
+		sgt = obj->funcs->get_sg_table(obj);
+	else
+		sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
 
 	if (!IS_ERR(sgt)) {
 		if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
@@ -406,12 +403,13 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
 void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
+	void *vaddr;
 
-	if (dev->driver->gem_prime_vmap)
-		return dev->driver->gem_prime_vmap(obj);
-	else
-		return NULL;
+	vaddr = drm_gem_vmap(obj);
+	if (IS_ERR(vaddr))
+		vaddr = NULL;
+
+	return vaddr;
 }
 EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
 
@@ -426,10 +424,8 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
 void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
 
-	if (dev->driver->gem_prime_vunmap)
-		dev->driver->gem_prime_vunmap(obj, vaddr);
+	drm_gem_vunmap(obj, vaddr);
 }
 EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
 
@@ -476,10 +472,12 @@ int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
 	struct drm_gem_object *obj = dma_buf->priv;
 	struct drm_device *dev = obj->dev;
 
-	if (!dev->driver->gem_prime_mmap)
+	if (obj->funcs && obj->funcs->prime_mmap)
+		return obj->funcs->prime_mmap(obj, vma);
+	else if (dev->driver->gem_prime_mmap)
+		return dev->driver->gem_prime_mmap(obj, vma);
+	else
 		return -ENOSYS;
-
-	return dev->driver->gem_prime_mmap(obj, vma);
 }
 EXPORT_SYMBOL(drm_gem_dmabuf_mmap);
 
@@ -559,7 +557,9 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
 		return dmabuf;
 	}
 
-	if (dev->driver->gem_prime_export)
+	if (obj->funcs && obj->funcs->export)
+		dmabuf = obj->funcs->export(obj, flags);
+	else if (dev->driver->gem_prime_export)
 		dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
 	else
 		dmabuf = drm_gem_prime_export(dev, obj, flags);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 3583b98a1718..ba5fe8af92ce 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -38,6 +38,130 @@
 
 #include <drm/drm_vma_manager.h>
 
+struct drm_gem_object;
+
+/**
+ * struct drm_gem_object_funcs - GEM object functions
+ */
+struct drm_gem_object_funcs {
+	/**
+	 * @free:
+	 *
+	 * Deconstructor for drm_gem_objects.
+	 *
+	 * This callback is mandatory.
+	 */
+	void (*free)(struct drm_gem_object *obj);
+
+	/**
+	 * @open:
+	 *
+	 * Called upon GEM handle creation.
+	 *
+	 * This callback is optional.
+	 */
+	int (*open)(struct drm_gem_object *obj, struct drm_file *file);
+
+	/**
+	 * @close:
+	 *
+	 * Called upon GEM handle release.
+	 *
+	 * This callback is optional.
+	 */
+	void (*close)(struct drm_gem_object *obj, struct drm_file *file);
+
+	/**
+	 * @print_info:
+	 *
+	 * If driver subclasses struct &drm_gem_object, it can implement this
+	 * optional hook for printing additional driver specific info.
+	 *
+	 * drm_printf_indent() should be used in the callback passing it the
+	 * indent argument.
+	 *
+	 * This callback is called from drm_gem_print_info().
+	 *
+	 * This callback is optional.
+	 */
+	void (*print_info)(struct drm_printer *p, unsigned int indent,
+			   const struct drm_gem_object *obj);
+
+	/**
+	 * @export:
+	 *
+	 * Export backing buffer as a &dma_buf.
+	 * If this is not set drm_gem_prime_export() is used.
+	 *
+	 * This callback is optional.
+	 */
+	struct dma_buf *(*export)(struct drm_gem_object *obj, int flags);
+
+	/**
+	 * @pin:
+	 *
+	 * Pin backing buffer in memory.
+	 *
+	 * This callback is optional.
+	 */
+	int (*pin)(struct drm_gem_object *obj);
+
+	/**
+	 * @unpin:
+	 *
+	 * Unpin backing buffer.
+	 *
+	 * This callback is optional.
+	 */
+	void (*unpin)(struct drm_gem_object *obj);
+
+	/**
+	 * @get_sg_table:
+	 *
+	 * Returns a Scatter-Gather table representation of the buffer.
+	 * Used when exporting a buffer.
+	 *
+	 * This callback is mandatory if buffer export is supported.
+	 */
+	struct sg_table *(*get_sg_table)(struct drm_gem_object *obj);
+
+	/**
+	 * @prime_mmap:
+	 *
+	 * Setup userspace mapping for a PRIME exported buffer.
+	 *
+	 * This callback is optional.
+	 */
+	int (*prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
+
+	/**
+	 * @vmap:
+	 *
+	 * Returns a virtual address for the buffer.
+	 *
+	 * This callback is optional.
+	 */
+	void *(*vmap)(struct drm_gem_object *obj);
+
+	/**
+	 * @vunmap:
+	 *
+	 * Releases the the address previously returned by @vmap.
+	 *
+	 * This callback is optional.
+	 */
+	void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
+
+	/**
+	 * @vm_ops:
+	 *
+	 * Virtual memory operations used with mmap.
+	 *
+	 * This is optional but necessary for mmap support.
+	 */
+	const struct vm_operations_struct *vm_ops;
+};
+
 /**
  * struct drm_gem_object - GEM buffer object
  *
@@ -146,6 +270,17 @@ struct drm_gem_object {
 	 * simply leave it as NULL.
 	 */
 	struct dma_buf_attachment *import_attach;
+
+	/**
+	 * @funcs:
+	 *
+	 * Optional GEM object functions. If this is set, it will be used instead of the
+	 * corresponding &drm_driver GEM callbacks.
+	 *
+	 * New drivers should use this.
+	 *
+	 */
+	const struct drm_gem_object_funcs *funcs;
 };
 
 /**
@@ -293,4 +428,9 @@ int drm_gem_dumb_destroy(struct drm_file *file,
 			 struct drm_device *dev,
 			 uint32_t handle);
 
+int drm_gem_pin(struct drm_gem_object *obj);
+void drm_gem_unpin(struct drm_gem_object *obj);
+void *drm_gem_vmap(struct drm_gem_object *obj);
+void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr);
+
 #endif /* __DRM_GEM_H__ */
-- 
2.15.1

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

Re: [PATCH v4 2/4] drm/gem: Add drm_gem_object_funcs

[Daniel Vetter]

On Wed, Sep 26, 2018 at 04:59:31PM +0200, Noralf Trønnes wrote:
> This adds an optional function table on GEM objects.
> The main benefit is for drivers that support more than one type of
> memory (shmem,vram,cma) for their buffers depending on the hardware it
> runs on. With the callbacks attached to the GEM object itself, it is
> easier to have core helpers for the the various buffer types. The driver
> only has to make the decision about buffer type on GEM object creation
> and all other callbacks can be handled by the chosen helper.
> 
> drm_driver->gem_prime_res_obj has not been added since there's a todo to
> put a reservation_object into drm_gem_object.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> 
> Changes since RFC:
> - drm_gem_object_funcs.map -> .prime_map let it only do PRIME mmap like
>   the function it superseeds (Daniel Vetter)
> - Flip around the if ladders and make obj->funcs the first choice
>   highlighting the fact that this the new default way of doing it
>   (Daniel Vetter)

I know every subsytem is different, but I like to keep the per-patch
changelog on the permanen record. It's already really hard to stitch
together the evolution of a patch idea after it's merged, this helps a
little bit if it's readily available.

On the first 2 patches:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

One nit below.

> 
>  drivers/gpu/drm/drm_client.c    |  12 ++--
>  drivers/gpu/drm/drm_fb_helper.c |   8 ++-
>  drivers/gpu/drm/drm_gem.c       | 109 ++++++++++++++++++++++++++++---
>  drivers/gpu/drm/drm_prime.c     |  42 ++++++------
>  include/drm/drm_gem.h           | 140 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 272 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 17d9a64e885e..eca7331762e4 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -80,8 +80,7 @@ int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
>  {
>  	int ret;
>  
> -	if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
> -	    !dev->driver->dumb_create || !dev->driver->gem_prime_vmap)
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create)
>  		return -EOPNOTSUPP;
>  
>  	if (funcs && !try_module_get(funcs->owner))
> @@ -212,8 +211,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>  {
>  	struct drm_device *dev = buffer->client->dev;
>  
> -	if (buffer->vaddr && dev->driver->gem_prime_vunmap)
> -		dev->driver->gem_prime_vunmap(buffer->gem, buffer->vaddr);
> +	drm_gem_vunmap(buffer->gem, buffer->vaddr);
>  
>  	if (buffer->gem)
>  		drm_gem_object_put_unlocked(buffer->gem);
> @@ -266,9 +264,9 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
>  	 * fd_install step out of the driver backend hooks, to make that
>  	 * final step optional for internal users.
>  	 */
> -	vaddr = dev->driver->gem_prime_vmap(obj);
> -	if (!vaddr) {
> -		ret = -ENOMEM;
> +	vaddr = drm_gem_vmap(obj);
> +	if (IS_ERR(vaddr)) {
> +		ret = PTR_ERR(vaddr);
>  		goto err_delete;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bf2190c5add5..0af405beceda 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -38,6 +38,7 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
> +#include <drm/drm_gem.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -3010,9 +3011,12 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
>  static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> +	struct drm_gem_object *obj = fb_helper->buffer->gem;
>  
> -	if (fb_helper->dev->driver->gem_prime_mmap)
> -		return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
> +	if (obj->funcs && obj->funcs->prime_mmap)
> +		return obj->funcs->prime_mmap(obj, vma);
> +	else if (obj->dev->driver->gem_prime_mmap)
> +		return obj->dev->driver->gem_prime_mmap(obj, vma);

This if ladder exists twice, would be good to extract into a little
helper.

Cheers, Daniel

>  	else
>  		return -ENODEV;
>  }
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 512078ebd97b..dcea6f8229a4 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -257,7 +257,9 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
>  	struct drm_gem_object *obj = ptr;
>  	struct drm_device *dev = obj->dev;
>  
> -	if (dev->driver->gem_close_object)
> +	if (obj->funcs && obj->funcs->close)
> +		obj->funcs->close(obj, file_priv);
> +	else if (dev->driver->gem_close_object)
>  		dev->driver->gem_close_object(obj, file_priv);
>  
>  	if (drm_core_check_feature(dev, DRIVER_PRIME))
> @@ -410,7 +412,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>  	if (ret)
>  		goto err_remove;
>  
> -	if (dev->driver->gem_open_object) {
> +	if (obj->funcs && obj->funcs->open) {
> +		ret = obj->funcs->open(obj, file_priv);
> +		if (ret)
> +			goto err_revoke;
> +	} else if (dev->driver->gem_open_object) {
>  		ret = dev->driver->gem_open_object(obj, file_priv);
>  		if (ret)
>  			goto err_revoke;
> @@ -835,7 +841,9 @@ drm_gem_object_free(struct kref *kref)
>  		container_of(kref, struct drm_gem_object, refcount);
>  	struct drm_device *dev = obj->dev;
>  
> -	if (dev->driver->gem_free_object_unlocked) {
> +	if (obj->funcs) {
> +		obj->funcs->free(obj);
> +	} else if (dev->driver->gem_free_object_unlocked) {
>  		dev->driver->gem_free_object_unlocked(obj);
>  	} else if (dev->driver->gem_free_object) {
>  		WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -864,13 +872,13 @@ drm_gem_object_put_unlocked(struct drm_gem_object *obj)
>  
>  	dev = obj->dev;
>  
> -	if (dev->driver->gem_free_object_unlocked) {
> -		kref_put(&obj->refcount, drm_gem_object_free);
> -	} else {
> +	if (dev->driver->gem_free_object) {
>  		might_lock(&dev->struct_mutex);
>  		if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
>  				&dev->struct_mutex))
>  			mutex_unlock(&dev->struct_mutex);
> +	} else {
> +		kref_put(&obj->refcount, drm_gem_object_free);
>  	}
>  }
>  EXPORT_SYMBOL(drm_gem_object_put_unlocked);
> @@ -960,11 +968,14 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  	if (obj_size < vma->vm_end - vma->vm_start)
>  		return -EINVAL;
>  
> -	if (!dev->driver->gem_vm_ops)
> +	if (obj->funcs && obj->funcs->vm_ops)
> +		vma->vm_ops = obj->funcs->vm_ops;
> +	else if (dev->driver->gem_vm_ops)
> +		vma->vm_ops = dev->driver->gem_vm_ops;
> +	else
>  		return -EINVAL;
>  
>  	vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> -	vma->vm_ops = dev->driver->gem_vm_ops;
>  	vma->vm_private_data = obj;
>  	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>  	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> @@ -1066,6 +1077,86 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>  	drm_printf_indent(p, indent, "imported=%s\n",
>  			  obj->import_attach ? "yes" : "no");
>  
> -	if (obj->dev->driver->gem_print_info)
> +	if (obj->funcs && obj->funcs->print_info)
> +		obj->funcs->print_info(p, indent, obj);
> +	else if (obj->dev->driver->gem_print_info)
>  		obj->dev->driver->gem_print_info(p, indent, obj);
>  }
> +
> +/**
> + * drm_gem_pin - Pin backing buffer in memory
> + * @obj: GEM object
> + *
> + * Make sure the backing buffer is pinned in memory.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_pin(struct drm_gem_object *obj)
> +{
> +	if (obj->funcs && obj->funcs->pin)
> +		return obj->funcs->pin(obj);
> +	else if (obj->dev->driver->gem_prime_pin)
> +		return obj->dev->driver->gem_prime_pin(obj);
> +	else
> +		return 0;
> +}
> +EXPORT_SYMBOL(drm_gem_pin);
> +
> +/**
> + * drm_gem_pin - Unpin backing buffer from memory
> + * @obj: GEM object
> + *
> + * Relax the requirement that the backing buffer is pinned in memory.
> + */
> +void drm_gem_unpin(struct drm_gem_object *obj)
> +{
> +	if (obj->funcs && obj->funcs->unpin)
> +		obj->funcs->unpin(obj);
> +	else if (obj->dev->driver->gem_prime_unpin)
> +		obj->dev->driver->gem_prime_unpin(obj);
> +}
> +EXPORT_SYMBOL(drm_gem_unpin);
> +
> +/**
> + * drm_gem_vmap - Map buffer into kernel virtual address space
> + * @obj: GEM object
> + *
> + * Returns:
> + * A virtual pointer to a newly created GEM object or an ERR_PTR-encoded negative
> + * error code on failure.
> + */
> +void *drm_gem_vmap(struct drm_gem_object *obj)
> +{
> +	void *vaddr;
> +
> +	if (obj->funcs && obj->funcs->vmap)
> +		vaddr = obj->funcs->vmap(obj);
> +	else if (obj->dev->driver->gem_prime_vmap)
> +		vaddr = obj->dev->driver->gem_prime_vmap(obj);
> +	else
> +		vaddr = ERR_PTR(-EOPNOTSUPP);
> +
> +	if (!vaddr)
> +		vaddr = ERR_PTR(-ENOMEM);
> +
> +	return vaddr;
> +}
> +EXPORT_SYMBOL(drm_gem_vmap);
> +
> +/**
> + * drm_gem_vunmap - Remove buffer mapping from kernel virtual address space
> + * @obj: GEM object
> + * @vaddr: Virtual address (can be NULL)
> + */
> +void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr)
> +{
> +	if (!vaddr)
> +		return;
> +
> +	if (obj->funcs && obj->funcs->vunmap)
> +		obj->funcs->vunmap(obj, vaddr);
> +	else if (obj->dev->driver->gem_prime_vunmap)
> +		obj->dev->driver->gem_prime_vunmap(obj, vaddr);
> +}
> +EXPORT_SYMBOL(drm_gem_vunmap);
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 6721571749ff..f231c199d38f 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -199,7 +199,6 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
>  {
>  	struct drm_prime_attachment *prime_attach;
>  	struct drm_gem_object *obj = dma_buf->priv;
> -	struct drm_device *dev = obj->dev;
>  
>  	prime_attach = kzalloc(sizeof(*prime_attach), GFP_KERNEL);
>  	if (!prime_attach)
> @@ -208,10 +207,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
>  	prime_attach->dir = DMA_NONE;
>  	attach->priv = prime_attach;
>  
> -	if (!dev->driver->gem_prime_pin)
> -		return 0;
> -
> -	return dev->driver->gem_prime_pin(obj);
> +	return drm_gem_pin(obj);
>  }
>  EXPORT_SYMBOL(drm_gem_map_attach);
>  
> @@ -228,7 +224,6 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
>  {
>  	struct drm_prime_attachment *prime_attach = attach->priv;
>  	struct drm_gem_object *obj = dma_buf->priv;
> -	struct drm_device *dev = obj->dev;
>  
>  	if (prime_attach) {
>  		struct sg_table *sgt = prime_attach->sgt;
> @@ -247,8 +242,7 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
>  		attach->priv = NULL;
>  	}
>  
> -	if (dev->driver->gem_prime_unpin)
> -		dev->driver->gem_prime_unpin(obj);
> +	drm_gem_unpin(obj);
>  }
>  EXPORT_SYMBOL(drm_gem_map_detach);
>  
> @@ -310,7 +304,10 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
>  	if (WARN_ON(prime_attach->dir != DMA_NONE))
>  		return ERR_PTR(-EBUSY);
>  
> -	sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
> +	if (obj->funcs)
> +		sgt = obj->funcs->get_sg_table(obj);
> +	else
> +		sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
>  
>  	if (!IS_ERR(sgt)) {
>  		if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
> @@ -406,12 +403,13 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
>  void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
>  {
>  	struct drm_gem_object *obj = dma_buf->priv;
> -	struct drm_device *dev = obj->dev;
> +	void *vaddr;
>  
> -	if (dev->driver->gem_prime_vmap)
> -		return dev->driver->gem_prime_vmap(obj);
> -	else
> -		return NULL;
> +	vaddr = drm_gem_vmap(obj);
> +	if (IS_ERR(vaddr))
> +		vaddr = NULL;
> +
> +	return vaddr;
>  }
>  EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
>  
> @@ -426,10 +424,8 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
>  void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
>  {
>  	struct drm_gem_object *obj = dma_buf->priv;
> -	struct drm_device *dev = obj->dev;
>  
> -	if (dev->driver->gem_prime_vunmap)
> -		dev->driver->gem_prime_vunmap(obj, vaddr);
> +	drm_gem_vunmap(obj, vaddr);
>  }
>  EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
>  
> @@ -476,10 +472,12 @@ int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
>  	struct drm_gem_object *obj = dma_buf->priv;
>  	struct drm_device *dev = obj->dev;
>  
> -	if (!dev->driver->gem_prime_mmap)
> +	if (obj->funcs && obj->funcs->prime_mmap)
> +		return obj->funcs->prime_mmap(obj, vma);
> +	else if (dev->driver->gem_prime_mmap)
> +		return dev->driver->gem_prime_mmap(obj, vma);
> +	else
>  		return -ENOSYS;
> -
> -	return dev->driver->gem_prime_mmap(obj, vma);
>  }
>  EXPORT_SYMBOL(drm_gem_dmabuf_mmap);
>  
> @@ -559,7 +557,9 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
>  		return dmabuf;
>  	}
>  
> -	if (dev->driver->gem_prime_export)
> +	if (obj->funcs && obj->funcs->export)
> +		dmabuf = obj->funcs->export(obj, flags);
> +	else if (dev->driver->gem_prime_export)
>  		dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
>  	else
>  		dmabuf = drm_gem_prime_export(dev, obj, flags);
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 3583b98a1718..ba5fe8af92ce 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -38,6 +38,130 @@
>  
>  #include <drm/drm_vma_manager.h>
>  
> +struct drm_gem_object;
> +
> +/**
> + * struct drm_gem_object_funcs - GEM object functions
> + */
> +struct drm_gem_object_funcs {
> +	/**
> +	 * @free:
> +	 *
> +	 * Deconstructor for drm_gem_objects.
> +	 *
> +	 * This callback is mandatory.
> +	 */
> +	void (*free)(struct drm_gem_object *obj);
> +
> +	/**
> +	 * @open:
> +	 *
> +	 * Called upon GEM handle creation.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	int (*open)(struct drm_gem_object *obj, struct drm_file *file);
> +
> +	/**
> +	 * @close:
> +	 *
> +	 * Called upon GEM handle release.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	void (*close)(struct drm_gem_object *obj, struct drm_file *file);
> +
> +	/**
> +	 * @print_info:
> +	 *
> +	 * If driver subclasses struct &drm_gem_object, it can implement this
> +	 * optional hook for printing additional driver specific info.
> +	 *
> +	 * drm_printf_indent() should be used in the callback passing it the
> +	 * indent argument.
> +	 *
> +	 * This callback is called from drm_gem_print_info().
> +	 *
> +	 * This callback is optional.
> +	 */
> +	void (*print_info)(struct drm_printer *p, unsigned int indent,
> +			   const struct drm_gem_object *obj);
> +
> +	/**
> +	 * @export:
> +	 *
> +	 * Export backing buffer as a &dma_buf.
> +	 * If this is not set drm_gem_prime_export() is used.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	struct dma_buf *(*export)(struct drm_gem_object *obj, int flags);
> +
> +	/**
> +	 * @pin:
> +	 *
> +	 * Pin backing buffer in memory.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	int (*pin)(struct drm_gem_object *obj);
> +
> +	/**
> +	 * @unpin:
> +	 *
> +	 * Unpin backing buffer.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	void (*unpin)(struct drm_gem_object *obj);
> +
> +	/**
> +	 * @get_sg_table:
> +	 *
> +	 * Returns a Scatter-Gather table representation of the buffer.
> +	 * Used when exporting a buffer.
> +	 *
> +	 * This callback is mandatory if buffer export is supported.
> +	 */
> +	struct sg_table *(*get_sg_table)(struct drm_gem_object *obj);
> +
> +	/**
> +	 * @prime_mmap:
> +	 *
> +	 * Setup userspace mapping for a PRIME exported buffer.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	int (*prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);
> +
> +	/**
> +	 * @vmap:
> +	 *
> +	 * Returns a virtual address for the buffer.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	void *(*vmap)(struct drm_gem_object *obj);
> +
> +	/**
> +	 * @vunmap:
> +	 *
> +	 * Releases the the address previously returned by @vmap.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	void (*vunmap)(struct drm_gem_object *obj, void *vaddr);
> +
> +	/**
> +	 * @vm_ops:
> +	 *
> +	 * Virtual memory operations used with mmap.
> +	 *
> +	 * This is optional but necessary for mmap support.
> +	 */
> +	const struct vm_operations_struct *vm_ops;
> +};
> +
>  /**
>   * struct drm_gem_object - GEM buffer object
>   *
> @@ -146,6 +270,17 @@ struct drm_gem_object {
>  	 * simply leave it as NULL.
>  	 */
>  	struct dma_buf_attachment *import_attach;
> +
> +	/**
> +	 * @funcs:
> +	 *
> +	 * Optional GEM object functions. If this is set, it will be used instead of the
> +	 * corresponding &drm_driver GEM callbacks.
> +	 *
> +	 * New drivers should use this.
> +	 *
> +	 */
> +	const struct drm_gem_object_funcs *funcs;
>  };
>  
>  /**
> @@ -293,4 +428,9 @@ int drm_gem_dumb_destroy(struct drm_file *file,
>  			 struct drm_device *dev,
>  			 uint32_t handle);
>  
> +int drm_gem_pin(struct drm_gem_object *obj);
> +void drm_gem_unpin(struct drm_gem_object *obj);
> +void *drm_gem_vmap(struct drm_gem_object *obj);
> +void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr);
> +
>  #endif /* __DRM_GEM_H__ */
> -- 
> 2.15.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH v4 3/4] drm: Add library for shmem backed GEM objects

[Noralf Trønnes]

This adds a library for shmem backed GEM objects.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---

Changes since version 3:
- Drop cache modes (Thomas Hellstrom)
- Add a GEM attached vtable

 Documentation/gpu/drm-kms-helpers.rst  |  12 +
 drivers/gpu/drm/Kconfig                |   6 +
 drivers/gpu/drm/Makefile               |   1 +
 drivers/gpu/drm/drm_gem_shmem_helper.c | 586 +++++++++++++++++++++++++++++++++
 include/drm/drm_gem_shmem_helper.h     | 153 +++++++++
 5 files changed, 758 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_gem_shmem_helper.c
 create mode 100644 include/drm/drm_gem_shmem_helper.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index f9cfcdcdf024..bc24b1b5216a 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -326,3 +326,15 @@ Legacy CRTC/Modeset Helper Functions Reference
 
 .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
    :export:
+
+SHMEM GEM Helper Reference
+==========================
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
+   :doc: overview
+
+.. kernel-doc:: include/drm/drm_gem_shmem_helper.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
+   :export:
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cb88528e7b10..db588ae44bcc 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -157,6 +157,12 @@ config DRM_KMS_CMA_HELPER
 	help
 	  Choose this if you need the KMS CMA helper functions
 
+config DRM_GEM_SHMEM_HELPER
+	bool
+	depends on DRM
+	help
+	  Choose this if you need the GEM shmem helper functions
+
 config DRM_VM
 	bool
 	depends on DRM && MMU
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index bc6a16a3c36e..13777f22dc25 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -25,6 +25,7 @@ drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
+drm-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_gem_shmem_helper.o
 drm-$(CONFIG_PCI) += ati_pcigart.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 drm-$(CONFIG_OF) += drm_of.o
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
new file mode 100644
index 000000000000..7471856d34ac
--- /dev/null
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -0,0 +1,586 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2018 Noralf Trønnes
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/export.h>
+#include <linux/mutex.h>
+#include <linux/shmem_fs.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_prime.h>
+#include <drm/drm_print.h>
+
+/**
+ * DOC: overview
+ *
+ * This library provides helpers for GEM objects backed by shmem buffers
+ * allocated using anonymous pageable memory.
+ */
+
+static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
+	.free = drm_gem_shmem_free_object,
+	.print_info = drm_gem_shmem_print_info,
+	.pin = drm_gem_shmem_pin,
+	.unpin = drm_gem_shmem_unpin,
+	.get_sg_table = drm_gem_shmem_get_sg_table,
+	.vmap = drm_gem_shmem_vmap,
+	.vunmap = drm_gem_shmem_vunmap,
+	.prime_mmap = drm_gem_shmem_prime_mmap,
+	.vm_ops = &drm_gem_shmem_vm_ops,
+};
+
+/**
+ * drm_gem_shmem_create - Allocate an object with the given size
+ * @dev: DRM device
+ * @size: Size of the object to allocate
+ *
+ * This function creates a shmem GEM object.
+ *
+ * Returns:
+ * A struct drm_gem_shmem_object * on success or an ERR_PTR()-encoded negative
+ * error code on failure.
+ */
+struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size)
+{
+	struct drm_gem_shmem_object *shmem;
+	struct drm_gem_object *obj;
+	int ret;
+
+	size = PAGE_ALIGN(size);
+
+	if (dev->driver->gem_create_object)
+		obj = dev->driver->gem_create_object(dev, size);
+	else
+		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	if (!obj->funcs)
+		obj->funcs = &drm_gem_shmem_funcs;
+
+	ret = drm_gem_object_init(dev, obj, size);
+	if (ret)
+		goto err_free;
+
+	ret = drm_gem_create_mmap_offset(obj);
+	if (ret)
+		goto err_release;
+
+	shmem = to_drm_gem_shmem_obj(obj);
+	mutex_init(&shmem->pages_lock);
+	mutex_init(&shmem->vmap_lock);
+
+	return shmem;
+
+err_release:
+	drm_gem_object_release(obj);
+err_free:
+	kfree(shmem);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+
+/**
+ * drm_gem_shmem_free_object - Free resources associated with a shmem GEM object
+ * @obj: GEM object to free
+ *
+ * This function cleans up the GEM object state and frees the memory used to
+ * store the object itself.
+ */
+void drm_gem_shmem_free_object(struct drm_gem_object *obj)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	WARN_ON(shmem->vmap_use_count);
+
+	if (obj->import_attach) {
+		shmem->pages_use_count--;
+		drm_prime_gem_destroy(obj, shmem->sgt);
+		kvfree(shmem->pages);
+	}
+
+	WARN_ON(shmem->pages_use_count);
+
+	drm_gem_object_release(obj);
+	mutex_destroy(&shmem->pages_lock);
+	mutex_destroy(&shmem->vmap_lock);
+	kfree(shmem);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_free_object);
+
+static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
+{
+	struct drm_gem_object *obj = &shmem->base;
+	struct page **pages;
+
+	if (shmem->pages_use_count++ > 0)
+		return 0;
+
+	pages = drm_gem_get_pages(obj);
+	if (IS_ERR(pages)) {
+		DRM_DEBUG_KMS("Failed to get pages (%ld)\n", PTR_ERR(pages));
+		shmem->pages_use_count = 0;
+		return PTR_ERR(pages);
+	}
+
+	shmem->pages = pages;
+
+	return 0;
+}
+
+/*
+ * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
+ * @shmem: shmem GEM object
+ *
+ * This function makes sure that backing pages exists for the shmem GEM object
+ * and increases the use count.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+{
+	int ret;
+
+	ret = mutex_lock_interruptible(&shmem->pages_lock);
+	if (ret)
+		return ret;
+	ret = drm_gem_shmem_get_pages_locked(shmem);
+	mutex_unlock(&shmem->pages_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_shmem_get_pages);
+
+static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
+{
+	struct drm_gem_object *obj = &shmem->base;
+
+	if (WARN_ON_ONCE(!shmem->pages_use_count))
+		return;
+
+	if (--shmem->pages_use_count > 0)
+		return;
+
+	drm_gem_put_pages(obj, shmem->pages,
+			  shmem->pages_mark_dirty_on_put,
+			  shmem->pages_mark_accessed_on_put);
+	shmem->pages = NULL;
+}
+
+/*
+ * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a shmem GEM object
+ * @shmem: shmem GEM object
+ *
+ * This function decreases the use count and puts the backing pages when use drops to zero.
+ */
+void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
+{
+	mutex_lock(&shmem->pages_lock);
+	drm_gem_shmem_put_pages_locked(shmem);
+	mutex_unlock(&shmem->pages_lock);
+}
+EXPORT_SYMBOL(drm_gem_shmem_put_pages);
+
+/**
+ * drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
+ * @obj: GEM object
+ *
+ * This function makes sure the backing pages are pinned in memory while the
+ * buffer is exported.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_pin(struct drm_gem_object *obj)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	return drm_gem_shmem_get_pages(shmem);
+}
+EXPORT_SYMBOL(drm_gem_shmem_pin);
+
+/**
+ * drm_gem_shmem_unpin - Unpin backing pages for a shmem GEM object
+ * @obj: GEM object
+ *
+ * This function removes the requirement that the backing pages are pinned in
+ * memory.
+ */
+void drm_gem_shmem_unpin(struct drm_gem_object *obj)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	drm_gem_shmem_put_pages(shmem);
+}
+EXPORT_SYMBOL(drm_gem_shmem_unpin);
+
+static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem)
+{
+	struct drm_gem_object *obj = &shmem->base;
+	int ret;
+
+	if (shmem->vmap_use_count++ > 0)
+		return shmem->vaddr;
+
+	ret = drm_gem_shmem_get_pages(shmem);
+	if (ret)
+		goto err_zero_use;
+
+	if (obj->import_attach)
+		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
+	else
+		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, VM_MAP, PAGE_KERNEL);
+
+	if (!shmem->vaddr) {
+		DRM_DEBUG_KMS("Failed to vmap pages\n");
+		ret = -ENOMEM;
+		goto err_put_pages;
+	}
+
+	return shmem->vaddr;
+
+err_put_pages:
+	drm_gem_shmem_put_pages(shmem);
+err_zero_use:
+	shmem->vmap_use_count = 0;
+
+	return ERR_PTR(ret);
+}
+
+/*
+ * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
+ * @shmem: shmem GEM object
+ *
+ * This function makes sure that a virtual address exists for the buffer backing
+ * the shmem GEM object.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+void *drm_gem_shmem_vmap(struct drm_gem_object *obj)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+	void *vaddr;
+	int ret;
+
+	ret = mutex_lock_interruptible(&shmem->vmap_lock);
+	if (ret)
+		return ERR_PTR(ret);
+	vaddr = drm_gem_shmem_vmap_locked(shmem);
+	mutex_unlock(&shmem->vmap_lock);
+
+	return vaddr;
+}
+EXPORT_SYMBOL(drm_gem_shmem_vmap);
+
+static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem)
+{
+	struct drm_gem_object *obj = &shmem->base;
+
+	if (WARN_ON_ONCE(!shmem->vmap_use_count))
+		return;
+
+	if (--shmem->vmap_use_count > 0)
+		return;
+
+	if (obj->import_attach)
+		dma_buf_vunmap(obj->import_attach->dmabuf, shmem->vaddr);
+	else
+		vunmap(shmem->vaddr);
+
+	shmem->vaddr = NULL;
+	drm_gem_shmem_put_pages(shmem);
+}
+
+/*
+ * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object
+ * @shmem: shmem GEM object
+ *
+ * This function removes the virtual address when use count drops to zero.
+ */
+void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	mutex_lock(&shmem->vmap_lock);
+	drm_gem_shmem_vunmap_locked(shmem);
+	mutex_unlock(&shmem->vmap_lock);
+}
+EXPORT_SYMBOL(drm_gem_shmem_vunmap);
+
+static struct drm_gem_shmem_object *
+drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
+				 struct drm_device *dev, size_t size,
+				 uint32_t *handle)
+{
+	struct drm_gem_shmem_object *shmem;
+	int ret;
+
+	shmem = drm_gem_shmem_create(dev, size);
+	if (IS_ERR(shmem))
+		return shmem;
+
+	/*
+	 * Allocate an id of idr table where the obj is registered
+	 * and handle has the id what user can see.
+	 */
+	ret = drm_gem_handle_create(file_priv, &shmem->base, handle);
+	/* drop reference from allocate - handle holds it now. */
+	drm_gem_object_put_unlocked(&shmem->base);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return shmem;
+}
+
+/**
+ * drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
+ * @file: DRM file structure to create the dumb buffer for
+ * @dev: DRM device
+ * @args: IOCTL data
+ *
+ * This function computes the pitch of the dumb buffer and rounds it up to an
+ * integer number of bytes per pixel. Drivers for hardware that doesn't have
+ * any additional restrictions on the pitch can directly use this function as
+ * their &drm_driver.dumb_create callback.
+ *
+ * For hardware with additional restrictions, drivers can adjust the fields
+ * set up by userspace before calling into this function.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
+			      struct drm_mode_create_dumb *args)
+{
+	u32 min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+	struct drm_gem_shmem_object *shmem;
+
+	if (!args->pitch || !args->size) {
+		args->pitch = min_pitch;
+		args->size = args->pitch * args->height;
+	} else {
+		/* ensure sane minimum values */
+		if (args->pitch < min_pitch)
+			args->pitch = min_pitch;
+		if (args->size < args->pitch * args->height)
+			args->size = args->pitch * args->height;
+	}
+
+	shmem = drm_gem_shmem_create_with_handle(file, dev, args->size, &args->handle);
+
+	return PTR_ERR_OR_ZERO(shmem);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
+
+static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
+{
+	struct vm_area_struct *vma = vmf->vma;
+	struct drm_gem_object *obj = vma->vm_private_data;
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+	/* We don't use vmf->pgoff since that has the fake offset: */
+	pgoff_t pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
+	loff_t num_pages = obj->size >> PAGE_SHIFT;
+	struct page *page;
+
+	if (pgoff > num_pages || WARN_ON_ONCE(!shmem->pages))
+		return VM_FAULT_SIGBUS;
+
+	page = shmem->pages[pgoff];
+
+	return vmf_insert_page(vma, vmf->address, page);
+}
+
+static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
+{
+	struct drm_gem_object *obj = vma->vm_private_data;
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	drm_gem_shmem_put_pages(shmem);
+	drm_gem_vm_close(vma);
+}
+
+const struct vm_operations_struct drm_gem_shmem_vm_ops = {
+	.fault = drm_gem_shmem_fault,
+	.open = drm_gem_vm_open,
+	.close = drm_gem_shmem_vm_close,
+};
+EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
+
+static int drm_gem_shmem_mmap_obj(struct drm_gem_shmem_object *shmem,
+				  struct vm_area_struct *vma)
+{
+	int ret;
+
+	ret = drm_gem_shmem_get_pages(shmem);
+	if (ret) {
+		drm_gem_vm_close(vma);
+		return ret;
+	}
+
+	/* VM_PFNMAP was set by drm_gem_mmap() */
+	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_flags |= VM_MIXEDMAP;
+	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+
+	fput(vma->vm_file);
+	get_file(shmem->base.filp);
+	vma->vm_pgoff = 0;
+	vma->vm_file  = shmem->base.filp;
+
+	return 0;
+}
+
+/**
+ * drm_gem_shmem_mmap - Memory-map a shmem GEM object
+ * @filp: File object
+ * @vma: VMA for the area to be mapped
+ *
+ * This function implements an augmented version of the GEM DRM file mmap
+ * operation for shmem objects. Drivers which employ the shmem helpers should
+ * use this function as their &file_operations.mmap handler in the DRM device file's
+ * file_operations structure.
+ *
+ * Instead of directly referencing this function, drivers should use the
+ * DEFINE_DRM_GEM_SHMEM_FOPS() macro.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct drm_gem_shmem_object *shmem;
+	int ret;
+
+	ret = drm_gem_mmap(filp, vma);
+	if (ret)
+		return ret;
+
+	shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
+
+	return drm_gem_shmem_mmap_obj(shmem, vma);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
+
+/**
+ * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
+ * @p: DRM printer
+ * @indent: Tab indentation level
+ * @obj: GEM object
+ */
+void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
+			      const struct drm_gem_object *obj)
+{
+	const struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	drm_printf_indent(p, indent, "pages_use_count=%u\n", shmem->pages_use_count);
+	drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
+	drm_printf_indent(p, indent, "vmap_use_count=%u\n", shmem->vmap_use_count);
+}
+EXPORT_SYMBOL(drm_gem_shmem_print_info);
+
+/**
+ * drm_gem_shmem_get_sg_table - Provide a scatter/gather table of pinned
+ *                              pages for a shmem GEM object
+ * @obj: GEM object
+ *
+ * This function exports a scatter/gather table suitable for PRIME usage by
+ * calling the standard DMA mapping API.
+ *
+ * Returns:
+ * A pointer to the scatter/gather table of pinned pages or NULL on failure.
+ */
+struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+
+	return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
+
+/**
+ * drm_gem_shmem_prime_import_sg_table - Produce a shmem GEM object from
+ *                 another driver's scatter/gather table of pinned pages
+ * @dev: Device to import into
+ * @attach: DMA-BUF attachment
+ * @sgt: Scatter/gather table of pinned pages
+ *
+ * This function imports a scatter/gather table exported via DMA-BUF by
+ * another driver. Drivers that use the shmem helpers should set this as their
+ * &drm_driver.gem_prime_import_sg_table callback.
+ *
+ * Returns:
+ * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
+ * error code on failure.
+ */
+struct drm_gem_object *
+drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
+				    struct dma_buf_attachment *attach,
+				    struct sg_table *sgt)
+{
+	size_t size = PAGE_ALIGN(attach->dmabuf->size);
+	size_t npages = size >> PAGE_SHIFT;
+	struct drm_gem_shmem_object *shmem;
+	int ret;
+
+	shmem = drm_gem_shmem_create(dev, size);
+	if (IS_ERR(shmem))
+		return ERR_CAST(shmem);
+
+	shmem->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
+	if (!shmem->pages) {
+		ret = -ENOMEM;
+		goto err_free_gem;
+	}
+
+	ret = drm_prime_sg_to_page_addr_arrays(sgt, shmem->pages, NULL, npages);
+	if (ret < 0)
+		goto err_free_array;
+
+	shmem->sgt = sgt;
+	shmem->pages_use_count = 1; /* Permanently pinned from our point of view */
+
+	DRM_DEBUG_PRIME("size = %zu\n", size);
+
+	return &shmem->base;
+
+err_free_array:
+	kvfree(shmem->pages);
+err_free_gem:
+	drm_gem_object_put_unlocked(&shmem->base);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
+
+/**
+ * drm_gem_shmem_prime_mmap - Memory-map an exported shmem GEM object
+ * @obj: GEM object
+ * @vma: VMA for the area to be mapped
+ *
+ * This function maps a buffer imported via DRM PRIME into a userspace
+ * process's address space.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
+{
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+	int ret;
+
+	ret = drm_gem_mmap_obj(obj, obj->size, vma);
+	if (ret)
+		return ret;
+
+	return drm_gem_shmem_mmap_obj(shmem, vma);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_mmap);
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
new file mode 100644
index 000000000000..94ec99fdcffc
--- /dev/null
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -0,0 +1,153 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __DRM_GEM_SHMEM_HELPER_H__
+#define __DRM_GEM_SHMEM_HELPER_H__
+
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/mutex.h>
+
+#include <drm/drm_file.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_prime.h>
+
+struct dma_buf_attachment;
+struct drm_mode_create_dumb;
+struct drm_printer;
+struct sg_table;
+
+/**
+ * struct drm_gem_shmem_object - GEM object backed by shmem
+ */
+struct drm_gem_shmem_object {
+	/**
+	 * @base: Base GEM object
+	 */
+	struct drm_gem_object base;
+
+	/**
+	 * @pages_lock: Protects the page table and use count
+	 */
+	struct mutex pages_lock;
+
+	/**
+	 * @pages: Page table
+	 */
+	struct page **pages;
+
+	/**
+	 * @pages_use_count:
+	 *
+	 * Reference count on the pages table.
+	 * The pages are put when the count reaches zero.
+	 */
+	unsigned int pages_use_count;
+
+	/**
+	 * @pages_mark_dirty_on_put:
+	 *
+	 * Mark pages as dirty when they are put.
+	 */
+	unsigned int pages_mark_dirty_on_put    : 1;
+
+	/**
+	 * @pages_mark_accessed_on_put:
+	 *
+	 * Mark pages as accessed when they are put.
+	 */
+	unsigned int pages_mark_accessed_on_put : 1;
+
+	/**
+	 * @sgt: Scatter/gather table for imported PRIME buffers
+	 */
+	struct sg_table *sgt;
+
+	/**
+	 * @vmap_lock: Protects the vmap address and use count
+	 */
+	struct mutex vmap_lock;
+
+	/**
+	 * @vaddr: Kernel virtual address of the backing memory
+	 */
+	void *vaddr;
+
+	/**
+	 * @vmap_use_count:
+	 *
+	 * Reference count on the virtual address.
+	 * The address are un-mapped when the count reaches zero.
+	 */
+	unsigned int vmap_use_count;
+};
+
+#define to_drm_gem_shmem_obj(obj) \
+	container_of(obj, struct drm_gem_shmem_object, base)
+
+/**
+ * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem drivers
+ * @name: name for the generated structure
+ *
+ * This macro autogenerates a suitable &struct file_operations for shmem based
+ * drivers, which can be assigned to &drm_driver.fops. Note that this structure
+ * cannot be shared between drivers, because it contains a reference to the
+ * current module using THIS_MODULE.
+ *
+ * Note that the declaration is already marked as static - if you need a
+ * non-static version of this you're probably doing it wrong and will break the
+ * THIS_MODULE reference by accident.
+ */
+#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \
+	static const struct file_operations name = {\
+		.owner		= THIS_MODULE,\
+		.open		= drm_open,\
+		.release	= drm_release,\
+		.unlocked_ioctl	= drm_ioctl,\
+		.compat_ioctl	= drm_compat_ioctl,\
+		.poll		= drm_poll,\
+		.read		= drm_read,\
+		.llseek		= noop_llseek,\
+		.mmap		= drm_gem_shmem_mmap, \
+	}
+
+struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
+void drm_gem_shmem_free_object(struct drm_gem_object *obj);
+
+int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem);
+void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
+int drm_gem_shmem_pin(struct drm_gem_object *obj);
+void drm_gem_shmem_unpin(struct drm_gem_object *obj);
+void *drm_gem_shmem_vmap(struct drm_gem_object *obj);
+void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr);
+
+int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
+			      struct drm_mode_create_dumb *args);
+
+int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma);
+
+extern const struct vm_operations_struct drm_gem_shmem_vm_ops;
+
+void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
+			      const struct drm_gem_object *obj);
+
+struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj);
+struct drm_gem_object *
+drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
+				    struct dma_buf_attachment *attach,
+				    struct sg_table *sgt);
+int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+
+/**
+ * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
+ *
+ * This macro provides a shortcut for setting the shmem GEM operations in
+ * the &drm_driver structure.
+ */
+#define DRM_GEM_SHMEM_DRIVER_OPS \
+	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
+	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
+	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
+	.dumb_create		= drm_gem_shmem_dumb_create
+
+#endif /* __DRM_GEM_SHMEM_HELPER_H__ */
-- 
2.15.1

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

Re: [PATCH v4 3/4] drm: Add library for shmem backed GEM objects

[Daniel Vetter]

On Wed, Sep 26, 2018 at 04:59:32PM +0200, Noralf Trønnes wrote:
> This adds a library for shmem backed GEM objects.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> 
> Changes since version 3:
> - Drop cache modes (Thomas Hellstrom)
> - Add a GEM attached vtable
> 
>  Documentation/gpu/drm-kms-helpers.rst  |  12 +
>  drivers/gpu/drm/Kconfig                |   6 +
>  drivers/gpu/drm/Makefile               |   1 +
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 586 +++++++++++++++++++++++++++++++++
>  include/drm/drm_gem_shmem_helper.h     | 153 +++++++++
>  5 files changed, 758 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_gem_shmem_helper.c
>  create mode 100644 include/drm/drm_gem_shmem_helper.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index f9cfcdcdf024..bc24b1b5216a 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -326,3 +326,15 @@ Legacy CRTC/Modeset Helper Functions Reference
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
>     :export:
> +
> +SHMEM GEM Helper Reference
> +==========================
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
> +   :doc: overview
> +
> +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
> +   :export:
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index cb88528e7b10..db588ae44bcc 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -157,6 +157,12 @@ config DRM_KMS_CMA_HELPER
>  	help
>  	  Choose this if you need the KMS CMA helper functions
>  
> +config DRM_GEM_SHMEM_HELPER
> +	bool
> +	depends on DRM
> +	help
> +	  Choose this if you need the GEM shmem helper functions
> +
>  config DRM_VM
>  	bool
>  	depends on DRM && MMU
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index bc6a16a3c36e..13777f22dc25 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -25,6 +25,7 @@ drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>  drm-$(CONFIG_DRM_VM) += drm_vm.o
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> +drm-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_gem_shmem_helper.o
>  drm-$(CONFIG_PCI) += ati_pcigart.o
>  drm-$(CONFIG_DRM_PANEL) += drm_panel.o
>  drm-$(CONFIG_OF) += drm_of.o
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> new file mode 100644
> index 000000000000..7471856d34ac
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2018 Noralf Trønnes
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/export.h>
> +#include <linux/mutex.h>
> +#include <linux/shmem_fs.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_prime.h>
> +#include <drm/drm_print.h>
> +
> +/**
> + * DOC: overview
> + *
> + * This library provides helpers for GEM objects backed by shmem buffers
> + * allocated using anonymous pageable memory.
> + */
> +
> +static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
> +	.free = drm_gem_shmem_free_object,
> +	.print_info = drm_gem_shmem_print_info,
> +	.pin = drm_gem_shmem_pin,
> +	.unpin = drm_gem_shmem_unpin,
> +	.get_sg_table = drm_gem_shmem_get_sg_table,
> +	.vmap = drm_gem_shmem_vmap,
> +	.vunmap = drm_gem_shmem_vunmap,
> +	.prime_mmap = drm_gem_shmem_prime_mmap,
> +	.vm_ops = &drm_gem_shmem_vm_ops,
> +};
> +
> +/**
> + * drm_gem_shmem_create - Allocate an object with the given size
> + * @dev: DRM device
> + * @size: Size of the object to allocate
> + *
> + * This function creates a shmem GEM object.
> + *
> + * Returns:
> + * A struct drm_gem_shmem_object * on success or an ERR_PTR()-encoded negative
> + * error code on failure.
> + */
> +struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size)
> +{
> +	struct drm_gem_shmem_object *shmem;
> +	struct drm_gem_object *obj;
> +	int ret;
> +
> +	size = PAGE_ALIGN(size);
> +
> +	if (dev->driver->gem_create_object)
> +		obj = dev->driver->gem_create_object(dev, size);
> +	else
> +		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
> +	if (!obj)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (!obj->funcs)
> +		obj->funcs = &drm_gem_shmem_funcs;
> +
> +	ret = drm_gem_object_init(dev, obj, size);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = drm_gem_create_mmap_offset(obj);
> +	if (ret)
> +		goto err_release;
> +
> +	shmem = to_drm_gem_shmem_obj(obj);
> +	mutex_init(&shmem->pages_lock);
> +	mutex_init(&shmem->vmap_lock);
> +
> +	return shmem;
> +
> +err_release:
> +	drm_gem_object_release(obj);
> +err_free:
> +	kfree(shmem);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
> +
> +/**
> + * drm_gem_shmem_free_object - Free resources associated with a shmem GEM object
> + * @obj: GEM object to free
> + *
> + * This function cleans up the GEM object state and frees the memory used to
> + * store the object itself.
> + */
> +void drm_gem_shmem_free_object(struct drm_gem_object *obj)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	WARN_ON(shmem->vmap_use_count);
> +
> +	if (obj->import_attach) {
> +		shmem->pages_use_count--;
> +		drm_prime_gem_destroy(obj, shmem->sgt);
> +		kvfree(shmem->pages);
> +	}
> +
> +	WARN_ON(shmem->pages_use_count);
> +
> +	drm_gem_object_release(obj);
> +	mutex_destroy(&shmem->pages_lock);
> +	mutex_destroy(&shmem->vmap_lock);
> +	kfree(shmem);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_free_object);
> +
> +static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +	struct page **pages;
> +
> +	if (shmem->pages_use_count++ > 0)
> +		return 0;
> +
> +	pages = drm_gem_get_pages(obj);
> +	if (IS_ERR(pages)) {
> +		DRM_DEBUG_KMS("Failed to get pages (%ld)\n", PTR_ERR(pages));
> +		shmem->pages_use_count = 0;
> +		return PTR_ERR(pages);
> +	}
> +
> +	shmem->pages = pages;
> +
> +	return 0;
> +}
> +
> +/*
> + * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
> + * @shmem: shmem GEM object
> + *
> + * This function makes sure that backing pages exists for the shmem GEM object
> + * and increases the use count.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> +{
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&shmem->pages_lock);
> +	if (ret)
> +		return ret;
> +	ret = drm_gem_shmem_get_pages_locked(shmem);
> +	mutex_unlock(&shmem->pages_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_get_pages);
> +
> +static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +
> +	if (WARN_ON_ONCE(!shmem->pages_use_count))
> +		return;
> +
> +	if (--shmem->pages_use_count > 0)
> +		return;
> +
> +	drm_gem_put_pages(obj, shmem->pages,
> +			  shmem->pages_mark_dirty_on_put,
> +			  shmem->pages_mark_accessed_on_put);
> +	shmem->pages = NULL;
> +}
> +
> +/*
> + * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a shmem GEM object
> + * @shmem: shmem GEM object
> + *
> + * This function decreases the use count and puts the backing pages when use drops to zero.
> + */
> +void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
> +{
> +	mutex_lock(&shmem->pages_lock);
> +	drm_gem_shmem_put_pages_locked(shmem);
> +	mutex_unlock(&shmem->pages_lock);
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_put_pages);
> +
> +/**
> + * drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
> + * @obj: GEM object
> + *
> + * This function makes sure the backing pages are pinned in memory while the
> + * buffer is exported.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_pin(struct drm_gem_object *obj)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	return drm_gem_shmem_get_pages(shmem);
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_pin);
> +
> +/**
> + * drm_gem_shmem_unpin - Unpin backing pages for a shmem GEM object
> + * @obj: GEM object
> + *
> + * This function removes the requirement that the backing pages are pinned in
> + * memory.
> + */
> +void drm_gem_shmem_unpin(struct drm_gem_object *obj)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	drm_gem_shmem_put_pages(shmem);
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_unpin);
> +
> +static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +	int ret;
> +
> +	if (shmem->vmap_use_count++ > 0)
> +		return shmem->vaddr;
> +
> +	ret = drm_gem_shmem_get_pages(shmem);
> +	if (ret)
> +		goto err_zero_use;
> +
> +	if (obj->import_attach)
> +		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
> +	else
> +		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, VM_MAP, PAGE_KERNEL);
> +
> +	if (!shmem->vaddr) {
> +		DRM_DEBUG_KMS("Failed to vmap pages\n");
> +		ret = -ENOMEM;
> +		goto err_put_pages;
> +	}
> +
> +	return shmem->vaddr;
> +
> +err_put_pages:
> +	drm_gem_shmem_put_pages(shmem);
> +err_zero_use:
> +	shmem->vmap_use_count = 0;
> +
> +	return ERR_PTR(ret);
> +}
> +
> +/*
> + * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
> + * @shmem: shmem GEM object
> + *
> + * This function makes sure that a virtual address exists for the buffer backing
> + * the shmem GEM object.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +void *drm_gem_shmem_vmap(struct drm_gem_object *obj)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +	void *vaddr;
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&shmem->vmap_lock);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	vaddr = drm_gem_shmem_vmap_locked(shmem);
> +	mutex_unlock(&shmem->vmap_lock);
> +
> +	return vaddr;
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_vmap);
> +
> +static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem)
> +{
> +	struct drm_gem_object *obj = &shmem->base;
> +
> +	if (WARN_ON_ONCE(!shmem->vmap_use_count))
> +		return;
> +
> +	if (--shmem->vmap_use_count > 0)
> +		return;
> +
> +	if (obj->import_attach)
> +		dma_buf_vunmap(obj->import_attach->dmabuf, shmem->vaddr);
> +	else
> +		vunmap(shmem->vaddr);
> +
> +	shmem->vaddr = NULL;
> +	drm_gem_shmem_put_pages(shmem);
> +}
> +
> +/*
> + * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object
> + * @shmem: shmem GEM object
> + *
> + * This function removes the virtual address when use count drops to zero.
> + */
> +void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	mutex_lock(&shmem->vmap_lock);
> +	drm_gem_shmem_vunmap_locked(shmem);
> +	mutex_unlock(&shmem->vmap_lock);
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_vunmap);
> +
> +static struct drm_gem_shmem_object *
> +drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
> +				 struct drm_device *dev, size_t size,
> +				 uint32_t *handle)
> +{
> +	struct drm_gem_shmem_object *shmem;
> +	int ret;
> +
> +	shmem = drm_gem_shmem_create(dev, size);
> +	if (IS_ERR(shmem))
> +		return shmem;
> +
> +	/*
> +	 * Allocate an id of idr table where the obj is registered
> +	 * and handle has the id what user can see.
> +	 */
> +	ret = drm_gem_handle_create(file_priv, &shmem->base, handle);
> +	/* drop reference from allocate - handle holds it now. */
> +	drm_gem_object_put_unlocked(&shmem->base);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return shmem;
> +}
> +
> +/**
> + * drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
> + * @file: DRM file structure to create the dumb buffer for
> + * @dev: DRM device
> + * @args: IOCTL data
> + *
> + * This function computes the pitch of the dumb buffer and rounds it up to an
> + * integer number of bytes per pixel. Drivers for hardware that doesn't have
> + * any additional restrictions on the pitch can directly use this function as
> + * their &drm_driver.dumb_create callback.
> + *
> + * For hardware with additional restrictions, drivers can adjust the fields
> + * set up by userspace before calling into this function.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> +			      struct drm_mode_create_dumb *args)
> +{
> +	u32 min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> +	struct drm_gem_shmem_object *shmem;
> +
> +	if (!args->pitch || !args->size) {
> +		args->pitch = min_pitch;
> +		args->size = args->pitch * args->height;
> +	} else {
> +		/* ensure sane minimum values */
> +		if (args->pitch < min_pitch)
> +			args->pitch = min_pitch;
> +		if (args->size < args->pitch * args->height)
> +			args->size = args->pitch * args->height;
> +	}
> +
> +	shmem = drm_gem_shmem_create_with_handle(file, dev, args->size, &args->handle);
> +
> +	return PTR_ERR_OR_ZERO(shmem);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
> +
> +static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> +{
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct drm_gem_object *obj = vma->vm_private_data;
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +	/* We don't use vmf->pgoff since that has the fake offset: */
> +	pgoff_t pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
> +	loff_t num_pages = obj->size >> PAGE_SHIFT;
> +	struct page *page;
> +
> +	if (pgoff > num_pages || WARN_ON_ONCE(!shmem->pages))
> +		return VM_FAULT_SIGBUS;
> +
> +	page = shmem->pages[pgoff];
> +
> +	return vmf_insert_page(vma, vmf->address, page);
> +}
> +
> +static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
> +{
> +	struct drm_gem_object *obj = vma->vm_private_data;
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	drm_gem_shmem_put_pages(shmem);
> +	drm_gem_vm_close(vma);
> +}
> +
> +const struct vm_operations_struct drm_gem_shmem_vm_ops = {
> +	.fault = drm_gem_shmem_fault,
> +	.open = drm_gem_vm_open,
> +	.close = drm_gem_shmem_vm_close,
> +};
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
> +
> +static int drm_gem_shmem_mmap_obj(struct drm_gem_shmem_object *shmem,
> +				  struct vm_area_struct *vma)
> +{
> +	int ret;
> +
> +	ret = drm_gem_shmem_get_pages(shmem);
> +	if (ret) {
> +		drm_gem_vm_close(vma);
> +		return ret;
> +	}
> +
> +	/* VM_PFNMAP was set by drm_gem_mmap() */
> +	vma->vm_flags &= ~VM_PFNMAP;
> +	vma->vm_flags |= VM_MIXEDMAP;
> +	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
> +
> +	fput(vma->vm_file);
> +	get_file(shmem->base.filp);
> +	vma->vm_pgoff = 0;
> +	vma->vm_file  = shmem->base.filp;
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_gem_shmem_mmap - Memory-map a shmem GEM object
> + * @filp: File object
> + * @vma: VMA for the area to be mapped
> + *
> + * This function implements an augmented version of the GEM DRM file mmap
> + * operation for shmem objects. Drivers which employ the shmem helpers should
> + * use this function as their &file_operations.mmap handler in the DRM device file's
> + * file_operations structure.
> + *
> + * Instead of directly referencing this function, drivers should use the
> + * DEFINE_DRM_GEM_SHMEM_FOPS() macro.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> +	struct drm_gem_shmem_object *shmem;
> +	int ret;
> +
> +	ret = drm_gem_mmap(filp, vma);
> +	if (ret)
> +		return ret;
> +
> +	shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
> +
> +	return drm_gem_shmem_mmap_obj(shmem, vma);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
> +
> +/**
> + * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
> + * @p: DRM printer
> + * @indent: Tab indentation level
> + * @obj: GEM object
> + */
> +void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
> +			      const struct drm_gem_object *obj)
> +{
> +	const struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	drm_printf_indent(p, indent, "pages_use_count=%u\n", shmem->pages_use_count);
> +	drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
> +	drm_printf_indent(p, indent, "vmap_use_count=%u\n", shmem->vmap_use_count);
> +}
> +EXPORT_SYMBOL(drm_gem_shmem_print_info);
> +
> +/**
> + * drm_gem_shmem_get_sg_table - Provide a scatter/gather table of pinned
> + *                              pages for a shmem GEM object
> + * @obj: GEM object
> + *
> + * This function exports a scatter/gather table suitable for PRIME usage by
> + * calling the standard DMA mapping API.
> + *
> + * Returns:
> + * A pointer to the scatter/gather table of pinned pages or NULL on failure.
> + */
> +struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +
> +	return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
> +
> +/**
> + * drm_gem_shmem_prime_import_sg_table - Produce a shmem GEM object from
> + *                 another driver's scatter/gather table of pinned pages
> + * @dev: Device to import into
> + * @attach: DMA-BUF attachment
> + * @sgt: Scatter/gather table of pinned pages
> + *
> + * This function imports a scatter/gather table exported via DMA-BUF by
> + * another driver. Drivers that use the shmem helpers should set this as their
> + * &drm_driver.gem_prime_import_sg_table callback.
> + *
> + * Returns:
> + * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
> + * error code on failure.
> + */
> +struct drm_gem_object *
> +drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> +				    struct dma_buf_attachment *attach,
> +				    struct sg_table *sgt)
> +{
> +	size_t size = PAGE_ALIGN(attach->dmabuf->size);
> +	size_t npages = size >> PAGE_SHIFT;
> +	struct drm_gem_shmem_object *shmem;
> +	int ret;
> +
> +	shmem = drm_gem_shmem_create(dev, size);
> +	if (IS_ERR(shmem))
> +		return ERR_CAST(shmem);
> +
> +	shmem->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> +	if (!shmem->pages) {
> +		ret = -ENOMEM;
> +		goto err_free_gem;
> +	}
> +
> +	ret = drm_prime_sg_to_page_addr_arrays(sgt, shmem->pages, NULL, npages);
> +	if (ret < 0)
> +		goto err_free_array;
> +
> +	shmem->sgt = sgt;
> +	shmem->pages_use_count = 1; /* Permanently pinned from our point of view */
> +
> +	DRM_DEBUG_PRIME("size = %zu\n", size);
> +
> +	return &shmem->base;
> +
> +err_free_array:
> +	kvfree(shmem->pages);
> +err_free_gem:
> +	drm_gem_object_put_unlocked(&shmem->base);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
> +
> +/**
> + * drm_gem_shmem_prime_mmap - Memory-map an exported shmem GEM object
> + * @obj: GEM object
> + * @vma: VMA for the area to be mapped
> + *
> + * This function maps a buffer imported via DRM PRIME into a userspace
> + * process's address space.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> +{
> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> +	int ret;
> +
> +	ret = drm_gem_mmap_obj(obj, obj->size, vma);
> +	if (ret)
> +		return ret;
> +
> +	return drm_gem_shmem_mmap_obj(shmem, vma);
> +}

If we'd use the gem vma offset manager and adjust offsets and then call
into the gem mmap stuff I think we could make this 100% generic. That
would also avoid the need to add the gem_ops->prime_mmap callback, since
we'll have a default that should work for everyone.
-Daniel

> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_mmap);
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> new file mode 100644
> index 000000000000..94ec99fdcffc
> --- /dev/null
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -0,0 +1,153 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __DRM_GEM_SHMEM_HELPER_H__
> +#define __DRM_GEM_SHMEM_HELPER_H__
> +
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +
> +#include <drm/drm_file.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_ioctl.h>
> +#include <drm/drm_prime.h>
> +
> +struct dma_buf_attachment;
> +struct drm_mode_create_dumb;
> +struct drm_printer;
> +struct sg_table;
> +
> +/**
> + * struct drm_gem_shmem_object - GEM object backed by shmem
> + */
> +struct drm_gem_shmem_object {
> +	/**
> +	 * @base: Base GEM object
> +	 */
> +	struct drm_gem_object base;
> +
> +	/**
> +	 * @pages_lock: Protects the page table and use count
> +	 */
> +	struct mutex pages_lock;
> +
> +	/**
> +	 * @pages: Page table
> +	 */
> +	struct page **pages;
> +
> +	/**
> +	 * @pages_use_count:
> +	 *
> +	 * Reference count on the pages table.
> +	 * The pages are put when the count reaches zero.
> +	 */
> +	unsigned int pages_use_count;
> +
> +	/**
> +	 * @pages_mark_dirty_on_put:
> +	 *
> +	 * Mark pages as dirty when they are put.
> +	 */
> +	unsigned int pages_mark_dirty_on_put    : 1;
> +
> +	/**
> +	 * @pages_mark_accessed_on_put:
> +	 *
> +	 * Mark pages as accessed when they are put.
> +	 */
> +	unsigned int pages_mark_accessed_on_put : 1;
> +
> +	/**
> +	 * @sgt: Scatter/gather table for imported PRIME buffers
> +	 */
> +	struct sg_table *sgt;
> +
> +	/**
> +	 * @vmap_lock: Protects the vmap address and use count
> +	 */
> +	struct mutex vmap_lock;
> +
> +	/**
> +	 * @vaddr: Kernel virtual address of the backing memory
> +	 */
> +	void *vaddr;
> +
> +	/**
> +	 * @vmap_use_count:
> +	 *
> +	 * Reference count on the virtual address.
> +	 * The address are un-mapped when the count reaches zero.
> +	 */
> +	unsigned int vmap_use_count;
> +};
> +
> +#define to_drm_gem_shmem_obj(obj) \
> +	container_of(obj, struct drm_gem_shmem_object, base)
> +
> +/**
> + * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem drivers
> + * @name: name for the generated structure
> + *
> + * This macro autogenerates a suitable &struct file_operations for shmem based
> + * drivers, which can be assigned to &drm_driver.fops. Note that this structure
> + * cannot be shared between drivers, because it contains a reference to the
> + * current module using THIS_MODULE.
> + *
> + * Note that the declaration is already marked as static - if you need a
> + * non-static version of this you're probably doing it wrong and will break the
> + * THIS_MODULE reference by accident.
> + */
> +#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \
> +	static const struct file_operations name = {\
> +		.owner		= THIS_MODULE,\
> +		.open		= drm_open,\
> +		.release	= drm_release,\
> +		.unlocked_ioctl	= drm_ioctl,\
> +		.compat_ioctl	= drm_compat_ioctl,\
> +		.poll		= drm_poll,\
> +		.read		= drm_read,\
> +		.llseek		= noop_llseek,\
> +		.mmap		= drm_gem_shmem_mmap, \
> +	}
> +
> +struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
> +void drm_gem_shmem_free_object(struct drm_gem_object *obj);
> +
> +int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem);
> +void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
> +int drm_gem_shmem_pin(struct drm_gem_object *obj);
> +void drm_gem_shmem_unpin(struct drm_gem_object *obj);
> +void *drm_gem_shmem_vmap(struct drm_gem_object *obj);
> +void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr);
> +
> +int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
> +			      struct drm_mode_create_dumb *args);
> +
> +int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma);
> +
> +extern const struct vm_operations_struct drm_gem_shmem_vm_ops;
> +
> +void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
> +			      const struct drm_gem_object *obj);
> +
> +struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj);
> +struct drm_gem_object *
> +drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
> +				    struct dma_buf_attachment *attach,
> +				    struct sg_table *sgt);
> +int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
> +
> +/**
> + * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
> + *
> + * This macro provides a shortcut for setting the shmem GEM operations in
> + * the &drm_driver structure.
> + */
> +#define DRM_GEM_SHMEM_DRIVER_OPS \
> +	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
> +	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
> +	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
> +	.dumb_create		= drm_gem_shmem_dumb_create
> +
> +#endif /* __DRM_GEM_SHMEM_HELPER_H__ */
> -- 
> 2.15.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v4 3/4] drm: Add library for shmem backed GEM objects

[Noralf Trønnes]

Den 01.10.2018 09.46, skrev Daniel Vetter:
> On Wed, Sep 26, 2018 at 04:59:32PM +0200, Noralf Trønnes wrote:
>> This adds a library for shmem backed GEM objects.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>
>> Changes since version 3:
>> - Drop cache modes (Thomas Hellstrom)
>> - Add a GEM attached vtable
>>
>>   Documentation/gpu/drm-kms-helpers.rst  |  12 +
>>   drivers/gpu/drm/Kconfig                |   6 +
>>   drivers/gpu/drm/Makefile               |   1 +
>>   drivers/gpu/drm/drm_gem_shmem_helper.c | 586 +++++++++++++++++++++++++++++++++
>>   include/drm/drm_gem_shmem_helper.h     | 153 +++++++++
>>   5 files changed, 758 insertions(+)
>>   create mode 100644 drivers/gpu/drm/drm_gem_shmem_helper.c
>>   create mode 100644 include/drm/drm_gem_shmem_helper.h
>>
>> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
>> index f9cfcdcdf024..bc24b1b5216a 100644
>> --- a/Documentation/gpu/drm-kms-helpers.rst
>> +++ b/Documentation/gpu/drm-kms-helpers.rst
>> @@ -326,3 +326,15 @@ Legacy CRTC/Modeset Helper Functions Reference
>>   
>>   .. kernel-doc:: drivers/gpu/drm/drm_crtc_helper.c
>>      :export:
>> +
>> +SHMEM GEM Helper Reference
>> +==========================
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
>> +   :doc: overview
>> +
>> +.. kernel-doc:: include/drm/drm_gem_shmem_helper.h
>> +   :internal:
>> +
>> +.. kernel-doc:: drivers/gpu/drm/drm_gem_shmem_helper.c
>> +   :export:
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index cb88528e7b10..db588ae44bcc 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -157,6 +157,12 @@ config DRM_KMS_CMA_HELPER
>>   	help
>>   	  Choose this if you need the KMS CMA helper functions
>>   
>> +config DRM_GEM_SHMEM_HELPER
>> +	bool
>> +	depends on DRM
>> +	help
>> +	  Choose this if you need the GEM shmem helper functions
>> +
>>   config DRM_VM
>>   	bool
>>   	depends on DRM && MMU
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index bc6a16a3c36e..13777f22dc25 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -25,6 +25,7 @@ drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>>   drm-$(CONFIG_DRM_VM) += drm_vm.o
>>   drm-$(CONFIG_COMPAT) += drm_ioc32.o
>>   drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> +drm-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_gem_shmem_helper.o
>>   drm-$(CONFIG_PCI) += ati_pcigart.o
>>   drm-$(CONFIG_DRM_PANEL) += drm_panel.o
>>   drm-$(CONFIG_OF) += drm_of.o
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> new file mode 100644
>> index 000000000000..7471856d34ac
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -0,0 +1,586 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2018 Noralf Trønnes
>> + */
>> +
>> +#include <linux/dma-buf.h>
>> +#include <linux/export.h>
>> +#include <linux/mutex.h>
>> +#include <linux/shmem_fs.h>
>> +#include <linux/slab.h>
>> +#include <linux/vmalloc.h>
>> +
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_drv.h>
>> +#include <drm/drm_gem_shmem_helper.h>
>> +#include <drm/drm_prime.h>
>> +#include <drm/drm_print.h>
>> +
>> +/**
>> + * DOC: overview
>> + *
>> + * This library provides helpers for GEM objects backed by shmem buffers
>> + * allocated using anonymous pageable memory.
>> + */
>> +
>> +static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
>> +	.free = drm_gem_shmem_free_object,
>> +	.print_info = drm_gem_shmem_print_info,
>> +	.pin = drm_gem_shmem_pin,
>> +	.unpin = drm_gem_shmem_unpin,
>> +	.get_sg_table = drm_gem_shmem_get_sg_table,
>> +	.vmap = drm_gem_shmem_vmap,
>> +	.vunmap = drm_gem_shmem_vunmap,
>> +	.prime_mmap = drm_gem_shmem_prime_mmap,
>> +	.vm_ops = &drm_gem_shmem_vm_ops,
>> +};
>> +
>> +/**
>> + * drm_gem_shmem_create - Allocate an object with the given size
>> + * @dev: DRM device
>> + * @size: Size of the object to allocate
>> + *
>> + * This function creates a shmem GEM object.
>> + *
>> + * Returns:
>> + * A struct drm_gem_shmem_object * on success or an ERR_PTR()-encoded negative
>> + * error code on failure.
>> + */
>> +struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size)
>> +{
>> +	struct drm_gem_shmem_object *shmem;
>> +	struct drm_gem_object *obj;
>> +	int ret;
>> +
>> +	size = PAGE_ALIGN(size);
>> +
>> +	if (dev->driver->gem_create_object)
>> +		obj = dev->driver->gem_create_object(dev, size);
>> +	else
>> +		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
>> +	if (!obj)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	if (!obj->funcs)
>> +		obj->funcs = &drm_gem_shmem_funcs;
>> +
>> +	ret = drm_gem_object_init(dev, obj, size);
>> +	if (ret)
>> +		goto err_free;
>> +
>> +	ret = drm_gem_create_mmap_offset(obj);
>> +	if (ret)
>> +		goto err_release;
>> +
>> +	shmem = to_drm_gem_shmem_obj(obj);
>> +	mutex_init(&shmem->pages_lock);
>> +	mutex_init(&shmem->vmap_lock);
>> +
>> +	return shmem;
>> +
>> +err_release:
>> +	drm_gem_object_release(obj);
>> +err_free:
>> +	kfree(shmem);
>> +
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
>> +
>> +/**
>> + * drm_gem_shmem_free_object - Free resources associated with a shmem GEM object
>> + * @obj: GEM object to free
>> + *
>> + * This function cleans up the GEM object state and frees the memory used to
>> + * store the object itself.
>> + */
>> +void drm_gem_shmem_free_object(struct drm_gem_object *obj)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	WARN_ON(shmem->vmap_use_count);
>> +
>> +	if (obj->import_attach) {
>> +		shmem->pages_use_count--;
>> +		drm_prime_gem_destroy(obj, shmem->sgt);
>> +		kvfree(shmem->pages);
>> +	}
>> +
>> +	WARN_ON(shmem->pages_use_count);
>> +
>> +	drm_gem_object_release(obj);
>> +	mutex_destroy(&shmem->pages_lock);
>> +	mutex_destroy(&shmem->vmap_lock);
>> +	kfree(shmem);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_free_object);
>> +
>> +static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
>> +{
>> +	struct drm_gem_object *obj = &shmem->base;
>> +	struct page **pages;
>> +
>> +	if (shmem->pages_use_count++ > 0)
>> +		return 0;
>> +
>> +	pages = drm_gem_get_pages(obj);
>> +	if (IS_ERR(pages)) {
>> +		DRM_DEBUG_KMS("Failed to get pages (%ld)\n", PTR_ERR(pages));
>> +		shmem->pages_use_count = 0;
>> +		return PTR_ERR(pages);
>> +	}
>> +
>> +	shmem->pages = pages;
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
>> + * @shmem: shmem GEM object
>> + *
>> + * This function makes sure that backing pages exists for the shmem GEM object
>> + * and increases the use count.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
>> +{
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&shmem->pages_lock);
>> +	if (ret)
>> +		return ret;
>> +	ret = drm_gem_shmem_get_pages_locked(shmem);
>> +	mutex_unlock(&shmem->pages_lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_get_pages);
>> +
>> +static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
>> +{
>> +	struct drm_gem_object *obj = &shmem->base;
>> +
>> +	if (WARN_ON_ONCE(!shmem->pages_use_count))
>> +		return;
>> +
>> +	if (--shmem->pages_use_count > 0)
>> +		return;
>> +
>> +	drm_gem_put_pages(obj, shmem->pages,
>> +			  shmem->pages_mark_dirty_on_put,
>> +			  shmem->pages_mark_accessed_on_put);
>> +	shmem->pages = NULL;
>> +}
>> +
>> +/*
>> + * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a shmem GEM object
>> + * @shmem: shmem GEM object
>> + *
>> + * This function decreases the use count and puts the backing pages when use drops to zero.
>> + */
>> +void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
>> +{
>> +	mutex_lock(&shmem->pages_lock);
>> +	drm_gem_shmem_put_pages_locked(shmem);
>> +	mutex_unlock(&shmem->pages_lock);
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_put_pages);
>> +
>> +/**
>> + * drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
>> + * @obj: GEM object
>> + *
>> + * This function makes sure the backing pages are pinned in memory while the
>> + * buffer is exported.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_shmem_pin(struct drm_gem_object *obj)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	return drm_gem_shmem_get_pages(shmem);
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_pin);
>> +
>> +/**
>> + * drm_gem_shmem_unpin - Unpin backing pages for a shmem GEM object
>> + * @obj: GEM object
>> + *
>> + * This function removes the requirement that the backing pages are pinned in
>> + * memory.
>> + */
>> +void drm_gem_shmem_unpin(struct drm_gem_object *obj)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	drm_gem_shmem_put_pages(shmem);
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_unpin);
>> +
>> +static void *drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem)
>> +{
>> +	struct drm_gem_object *obj = &shmem->base;
>> +	int ret;
>> +
>> +	if (shmem->vmap_use_count++ > 0)
>> +		return shmem->vaddr;
>> +
>> +	ret = drm_gem_shmem_get_pages(shmem);
>> +	if (ret)
>> +		goto err_zero_use;
>> +
>> +	if (obj->import_attach)
>> +		shmem->vaddr = dma_buf_vmap(obj->import_attach->dmabuf);
>> +	else
>> +		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT, VM_MAP, PAGE_KERNEL);
>> +
>> +	if (!shmem->vaddr) {
>> +		DRM_DEBUG_KMS("Failed to vmap pages\n");
>> +		ret = -ENOMEM;
>> +		goto err_put_pages;
>> +	}
>> +
>> +	return shmem->vaddr;
>> +
>> +err_put_pages:
>> +	drm_gem_shmem_put_pages(shmem);
>> +err_zero_use:
>> +	shmem->vmap_use_count = 0;
>> +
>> +	return ERR_PTR(ret);
>> +}
>> +
>> +/*
>> + * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
>> + * @shmem: shmem GEM object
>> + *
>> + * This function makes sure that a virtual address exists for the buffer backing
>> + * the shmem GEM object.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +void *drm_gem_shmem_vmap(struct drm_gem_object *obj)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +	void *vaddr;
>> +	int ret;
>> +
>> +	ret = mutex_lock_interruptible(&shmem->vmap_lock);
>> +	if (ret)
>> +		return ERR_PTR(ret);
>> +	vaddr = drm_gem_shmem_vmap_locked(shmem);
>> +	mutex_unlock(&shmem->vmap_lock);
>> +
>> +	return vaddr;
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_vmap);
>> +
>> +static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem)
>> +{
>> +	struct drm_gem_object *obj = &shmem->base;
>> +
>> +	if (WARN_ON_ONCE(!shmem->vmap_use_count))
>> +		return;
>> +
>> +	if (--shmem->vmap_use_count > 0)
>> +		return;
>> +
>> +	if (obj->import_attach)
>> +		dma_buf_vunmap(obj->import_attach->dmabuf, shmem->vaddr);
>> +	else
>> +		vunmap(shmem->vaddr);
>> +
>> +	shmem->vaddr = NULL;
>> +	drm_gem_shmem_put_pages(shmem);
>> +}
>> +
>> +/*
>> + * drm_gem_shmem_vunmap - Unmap a virtual mapping fo a shmem GEM object
>> + * @shmem: shmem GEM object
>> + *
>> + * This function removes the virtual address when use count drops to zero.
>> + */
>> +void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	mutex_lock(&shmem->vmap_lock);
>> +	drm_gem_shmem_vunmap_locked(shmem);
>> +	mutex_unlock(&shmem->vmap_lock);
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_vunmap);
>> +
>> +static struct drm_gem_shmem_object *
>> +drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
>> +				 struct drm_device *dev, size_t size,
>> +				 uint32_t *handle)
>> +{
>> +	struct drm_gem_shmem_object *shmem;
>> +	int ret;
>> +
>> +	shmem = drm_gem_shmem_create(dev, size);
>> +	if (IS_ERR(shmem))
>> +		return shmem;
>> +
>> +	/*
>> +	 * Allocate an id of idr table where the obj is registered
>> +	 * and handle has the id what user can see.
>> +	 */
>> +	ret = drm_gem_handle_create(file_priv, &shmem->base, handle);
>> +	/* drop reference from allocate - handle holds it now. */
>> +	drm_gem_object_put_unlocked(&shmem->base);
>> +	if (ret)
>> +		return ERR_PTR(ret);
>> +
>> +	return shmem;
>> +}
>> +
>> +/**
>> + * drm_gem_shmem_dumb_create - Create a dumb shmem buffer object
>> + * @file: DRM file structure to create the dumb buffer for
>> + * @dev: DRM device
>> + * @args: IOCTL data
>> + *
>> + * This function computes the pitch of the dumb buffer and rounds it up to an
>> + * integer number of bytes per pixel. Drivers for hardware that doesn't have
>> + * any additional restrictions on the pitch can directly use this function as
>> + * their &drm_driver.dumb_create callback.
>> + *
>> + * For hardware with additional restrictions, drivers can adjust the fields
>> + * set up by userspace before calling into this function.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
>> +			      struct drm_mode_create_dumb *args)
>> +{
>> +	u32 min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>> +	struct drm_gem_shmem_object *shmem;
>> +
>> +	if (!args->pitch || !args->size) {
>> +		args->pitch = min_pitch;
>> +		args->size = args->pitch * args->height;
>> +	} else {
>> +		/* ensure sane minimum values */
>> +		if (args->pitch < min_pitch)
>> +			args->pitch = min_pitch;
>> +		if (args->size < args->pitch * args->height)
>> +			args->size = args->pitch * args->height;
>> +	}
>> +
>> +	shmem = drm_gem_shmem_create_with_handle(file, dev, args->size, &args->handle);
>> +
>> +	return PTR_ERR_OR_ZERO(shmem);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
>> +
>> +static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
>> +{
>> +	struct vm_area_struct *vma = vmf->vma;
>> +	struct drm_gem_object *obj = vma->vm_private_data;
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +	/* We don't use vmf->pgoff since that has the fake offset: */
>> +	pgoff_t pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>> +	loff_t num_pages = obj->size >> PAGE_SHIFT;
>> +	struct page *page;
>> +
>> +	if (pgoff > num_pages || WARN_ON_ONCE(!shmem->pages))
>> +		return VM_FAULT_SIGBUS;
>> +
>> +	page = shmem->pages[pgoff];
>> +
>> +	return vmf_insert_page(vma, vmf->address, page);
>> +}
>> +
>> +static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
>> +{
>> +	struct drm_gem_object *obj = vma->vm_private_data;
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	drm_gem_shmem_put_pages(shmem);
>> +	drm_gem_vm_close(vma);
>> +}
>> +
>> +const struct vm_operations_struct drm_gem_shmem_vm_ops = {
>> +	.fault = drm_gem_shmem_fault,
>> +	.open = drm_gem_vm_open,
>> +	.close = drm_gem_shmem_vm_close,
>> +};
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>> +
>> +static int drm_gem_shmem_mmap_obj(struct drm_gem_shmem_object *shmem,
>> +				  struct vm_area_struct *vma)
>> +{
>> +	int ret;
>> +
>> +	ret = drm_gem_shmem_get_pages(shmem);
>> +	if (ret) {
>> +		drm_gem_vm_close(vma);
>> +		return ret;
>> +	}
>> +
>> +	/* VM_PFNMAP was set by drm_gem_mmap() */
>> +	vma->vm_flags &= ~VM_PFNMAP;
>> +	vma->vm_flags |= VM_MIXEDMAP;
>> +	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>> +
>> +	fput(vma->vm_file);
>> +	get_file(shmem->base.filp);
>> +	vma->vm_pgoff = 0;
>> +	vma->vm_file  = shmem->base.filp;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * drm_gem_shmem_mmap - Memory-map a shmem GEM object
>> + * @filp: File object
>> + * @vma: VMA for the area to be mapped
>> + *
>> + * This function implements an augmented version of the GEM DRM file mmap
>> + * operation for shmem objects. Drivers which employ the shmem helpers should
>> + * use this function as their &file_operations.mmap handler in the DRM device file's
>> + * file_operations structure.
>> + *
>> + * Instead of directly referencing this function, drivers should use the
>> + * DEFINE_DRM_GEM_SHMEM_FOPS() macro.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma)
>> +{
>> +	struct drm_gem_shmem_object *shmem;
>> +	int ret;
>> +
>> +	ret = drm_gem_mmap(filp, vma);
>> +	if (ret)
>> +		return ret;
>> +
>> +	shmem = to_drm_gem_shmem_obj(vma->vm_private_data);
>> +
>> +	return drm_gem_shmem_mmap_obj(shmem, vma);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
>> +
>> +/**
>> + * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
>> + * @p: DRM printer
>> + * @indent: Tab indentation level
>> + * @obj: GEM object
>> + */
>> +void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
>> +			      const struct drm_gem_object *obj)
>> +{
>> +	const struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	drm_printf_indent(p, indent, "pages_use_count=%u\n", shmem->pages_use_count);
>> +	drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
>> +	drm_printf_indent(p, indent, "vmap_use_count=%u\n", shmem->vmap_use_count);
>> +}
>> +EXPORT_SYMBOL(drm_gem_shmem_print_info);
>> +
>> +/**
>> + * drm_gem_shmem_get_sg_table - Provide a scatter/gather table of pinned
>> + *                              pages for a shmem GEM object
>> + * @obj: GEM object
>> + *
>> + * This function exports a scatter/gather table suitable for PRIME usage by
>> + * calling the standard DMA mapping API.
>> + *
>> + * Returns:
>> + * A pointer to the scatter/gather table of pinned pages or NULL on failure.
>> + */
>> +struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +
>> +	return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
>> +
>> +/**
>> + * drm_gem_shmem_prime_import_sg_table - Produce a shmem GEM object from
>> + *                 another driver's scatter/gather table of pinned pages
>> + * @dev: Device to import into
>> + * @attach: DMA-BUF attachment
>> + * @sgt: Scatter/gather table of pinned pages
>> + *
>> + * This function imports a scatter/gather table exported via DMA-BUF by
>> + * another driver. Drivers that use the shmem helpers should set this as their
>> + * &drm_driver.gem_prime_import_sg_table callback.
>> + *
>> + * Returns:
>> + * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
>> + * error code on failure.
>> + */
>> +struct drm_gem_object *
>> +drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
>> +				    struct dma_buf_attachment *attach,
>> +				    struct sg_table *sgt)
>> +{
>> +	size_t size = PAGE_ALIGN(attach->dmabuf->size);
>> +	size_t npages = size >> PAGE_SHIFT;
>> +	struct drm_gem_shmem_object *shmem;
>> +	int ret;
>> +
>> +	shmem = drm_gem_shmem_create(dev, size);
>> +	if (IS_ERR(shmem))
>> +		return ERR_CAST(shmem);
>> +
>> +	shmem->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
>> +	if (!shmem->pages) {
>> +		ret = -ENOMEM;
>> +		goto err_free_gem;
>> +	}
>> +
>> +	ret = drm_prime_sg_to_page_addr_arrays(sgt, shmem->pages, NULL, npages);
>> +	if (ret < 0)
>> +		goto err_free_array;
>> +
>> +	shmem->sgt = sgt;
>> +	shmem->pages_use_count = 1; /* Permanently pinned from our point of view */
>> +
>> +	DRM_DEBUG_PRIME("size = %zu\n", size);
>> +
>> +	return &shmem->base;
>> +
>> +err_free_array:
>> +	kvfree(shmem->pages);
>> +err_free_gem:
>> +	drm_gem_object_put_unlocked(&shmem->base);
>> +
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_sg_table);
>> +
>> +/**
>> + * drm_gem_shmem_prime_mmap - Memory-map an exported shmem GEM object
>> + * @obj: GEM object
>> + * @vma: VMA for the area to be mapped
>> + *
>> + * This function maps a buffer imported via DRM PRIME into a userspace
>> + * process's address space.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
>> +{
>> +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>> +	int ret;
>> +
>> +	ret = drm_gem_mmap_obj(obj, obj->size, vma);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return drm_gem_shmem_mmap_obj(shmem, vma);
>> +}
> If we'd use the gem vma offset manager and adjust offsets and then call
> into the gem mmap stuff I think we could make this 100% generic. That
> would also avoid the need to add the gem_ops->prime_mmap callback, since
> we'll have a default that should work for everyone.

Is this what you had in mind?

int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct 
*vma)
{
     /* Used by drm_gem_mmap() to lookup the GEM object */
     struct drm_file priv = {
         .minor = obj->dev->primary,
     };
     struct file fil = {
         .private_data = &priv,
     };
     int ret;

     ret = drm_vma_node_allow(&obj->vma_node, &priv);
     if (ret)
         return ret;

     vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);

     ret = obj->dev->driver->fops->mmap(&fil, vma);

     drm_vma_node_revoke(&obj->vma_node, &priv);

     return ret;
}

Noralf.

> -Daniel
>
>> +EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_mmap);
>> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
>> new file mode 100644
>> index 000000000000..94ec99fdcffc
>> --- /dev/null
>> +++ b/include/drm/drm_gem_shmem_helper.h
>> @@ -0,0 +1,153 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __DRM_GEM_SHMEM_HELPER_H__
>> +#define __DRM_GEM_SHMEM_HELPER_H__
>> +
>> +#include <linux/fs.h>
>> +#include <linux/mm.h>
>> +#include <linux/mutex.h>
>> +
>> +#include <drm/drm_file.h>
>> +#include <drm/drm_gem.h>
>> +#include <drm/drm_ioctl.h>
>> +#include <drm/drm_prime.h>
>> +
>> +struct dma_buf_attachment;
>> +struct drm_mode_create_dumb;
>> +struct drm_printer;
>> +struct sg_table;
>> +
>> +/**
>> + * struct drm_gem_shmem_object - GEM object backed by shmem
>> + */
>> +struct drm_gem_shmem_object {
>> +	/**
>> +	 * @base: Base GEM object
>> +	 */
>> +	struct drm_gem_object base;
>> +
>> +	/**
>> +	 * @pages_lock: Protects the page table and use count
>> +	 */
>> +	struct mutex pages_lock;
>> +
>> +	/**
>> +	 * @pages: Page table
>> +	 */
>> +	struct page **pages;
>> +
>> +	/**
>> +	 * @pages_use_count:
>> +	 *
>> +	 * Reference count on the pages table.
>> +	 * The pages are put when the count reaches zero.
>> +	 */
>> +	unsigned int pages_use_count;
>> +
>> +	/**
>> +	 * @pages_mark_dirty_on_put:
>> +	 *
>> +	 * Mark pages as dirty when they are put.
>> +	 */
>> +	unsigned int pages_mark_dirty_on_put    : 1;
>> +
>> +	/**
>> +	 * @pages_mark_accessed_on_put:
>> +	 *
>> +	 * Mark pages as accessed when they are put.
>> +	 */
>> +	unsigned int pages_mark_accessed_on_put : 1;
>> +
>> +	/**
>> +	 * @sgt: Scatter/gather table for imported PRIME buffers
>> +	 */
>> +	struct sg_table *sgt;
>> +
>> +	/**
>> +	 * @vmap_lock: Protects the vmap address and use count
>> +	 */
>> +	struct mutex vmap_lock;
>> +
>> +	/**
>> +	 * @vaddr: Kernel virtual address of the backing memory
>> +	 */
>> +	void *vaddr;
>> +
>> +	/**
>> +	 * @vmap_use_count:
>> +	 *
>> +	 * Reference count on the virtual address.
>> +	 * The address are un-mapped when the count reaches zero.
>> +	 */
>> +	unsigned int vmap_use_count;
>> +};
>> +
>> +#define to_drm_gem_shmem_obj(obj) \
>> +	container_of(obj, struct drm_gem_shmem_object, base)
>> +
>> +/**
>> + * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem drivers
>> + * @name: name for the generated structure
>> + *
>> + * This macro autogenerates a suitable &struct file_operations for shmem based
>> + * drivers, which can be assigned to &drm_driver.fops. Note that this structure
>> + * cannot be shared between drivers, because it contains a reference to the
>> + * current module using THIS_MODULE.
>> + *
>> + * Note that the declaration is already marked as static - if you need a
>> + * non-static version of this you're probably doing it wrong and will break the
>> + * THIS_MODULE reference by accident.
>> + */
>> +#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \
>> +	static const struct file_operations name = {\
>> +		.owner		= THIS_MODULE,\
>> +		.open		= drm_open,\
>> +		.release	= drm_release,\
>> +		.unlocked_ioctl	= drm_ioctl,\
>> +		.compat_ioctl	= drm_compat_ioctl,\
>> +		.poll		= drm_poll,\
>> +		.read		= drm_read,\
>> +		.llseek		= noop_llseek,\
>> +		.mmap		= drm_gem_shmem_mmap, \
>> +	}
>> +
>> +struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
>> +void drm_gem_shmem_free_object(struct drm_gem_object *obj);
>> +
>> +int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem);
>> +void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
>> +int drm_gem_shmem_pin(struct drm_gem_object *obj);
>> +void drm_gem_shmem_unpin(struct drm_gem_object *obj);
>> +void *drm_gem_shmem_vmap(struct drm_gem_object *obj);
>> +void drm_gem_shmem_vunmap(struct drm_gem_object *obj, void *vaddr);
>> +
>> +int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
>> +			      struct drm_mode_create_dumb *args);
>> +
>> +int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma);
>> +
>> +extern const struct vm_operations_struct drm_gem_shmem_vm_ops;
>> +
>> +void drm_gem_shmem_print_info(struct drm_printer *p, unsigned int indent,
>> +			      const struct drm_gem_object *obj);
>> +
>> +struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj);
>> +struct drm_gem_object *
>> +drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
>> +				    struct dma_buf_attachment *attach,
>> +				    struct sg_table *sgt);
>> +int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
>> +
>> +/**
>> + * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations
>> + *
>> + * This macro provides a shortcut for setting the shmem GEM operations in
>> + * the &drm_driver structure.
>> + */
>> +#define DRM_GEM_SHMEM_DRIVER_OPS \
>> +	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
>> +	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
>> +	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
>> +	.dumb_create		= drm_gem_shmem_dumb_create
>> +
>> +#endif /* __DRM_GEM_SHMEM_HELPER_H__ */
>> -- 
>> 2.15.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH v4 3/4] drm: Add library for shmem backed GEM objects

[Daniel Vetter]

On Wed, Oct 10, 2018 at 09:14:50PM +0200, Noralf Trønnes wrote:
> Den 01.10.2018 09.46, skrev Daniel Vetter:
> > On Wed, Sep 26, 2018 at 04:59:32PM +0200, Noralf Trønnes wrote:
> > > +int drm_gem_shmem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > > +{
> > > +	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
> > > +	int ret;
> > > +
> > > +	ret = drm_gem_mmap_obj(obj, obj->size, vma);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return drm_gem_shmem_mmap_obj(shmem, vma);
> > > +}
> > If we'd use the gem vma offset manager and adjust offsets and then call
> > into the gem mmap stuff I think we could make this 100% generic. That
> > would also avoid the need to add the gem_ops->prime_mmap callback, since
> > we'll have a default that should work for everyone.
> 
> Is this what you had in mind?
> 
> int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct
> *vma)
> {
>     /* Used by drm_gem_mmap() to lookup the GEM object */
>     struct drm_file priv = {
>         .minor = obj->dev->primary,
>     };
>     struct file fil = {
>         .private_data = &priv,
>     };
>     int ret;
> 
>     ret = drm_vma_node_allow(&obj->vma_node, &priv);
>     if (ret)
>         return ret;
> 
>     vma->vm_pgoff += drm_vma_node_start(&obj->vma_node);

Yup, this offset adjusting is what I had in mind.
-Daniel

>     ret = obj->dev->driver->fops->mmap(&fil, vma);
> 
>     drm_vma_node_revoke(&obj->vma_node, &priv);
> 
>     return ret;
> }
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 4/4] drm/tinydrm: Switch from CMA to shmem buffers

[Noralf Trønnes]

This move makes tinydrm useful for more drivers. tinydrm doesn't need
continuous memory, but at the time it was convenient to use the CMA
library. The spi core can do dma on is_vmalloc() addresses making this
possible.

Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: David Lechner <david@lechnology.com>
Tested-by: David Lechner <david@lechnology.com>
---

Changes since version 3:
- Use the new drm_gem_vmap() and drm_gem_vunmap() instead of directly
  calling the shmem variants.

 drivers/gpu/drm/tinydrm/Kconfig                |  2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c    | 92 +++++++-------------------
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |  5 ++
 drivers/gpu/drm/tinydrm/ili9225.c              | 14 ++--
 drivers/gpu/drm/tinydrm/ili9341.c              |  6 +-
 drivers/gpu/drm/tinydrm/mi0283qt.c             |  6 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c             | 38 ++++-------
 drivers/gpu/drm/tinydrm/repaper.c              | 24 +++----
 drivers/gpu/drm/tinydrm/st7586.c               | 15 +++--
 drivers/gpu/drm/tinydrm/st7735r.c              |  6 +-
 include/drm/tinydrm/tinydrm.h                  | 36 +++-------
 11 files changed, 91 insertions(+), 153 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 16f4b5c91f1b..aa0cabba5ace 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -2,7 +2,7 @@ menuconfig DRM_TINYDRM
 	tristate "Support for simple displays"
 	depends on DRM
 	select DRM_KMS_HELPER
-	select DRM_KMS_CMA_HELPER
+	select DRM_GEM_SHMEM_HELPER
 	help
 	  Choose this option if you have a tinydrm supported display.
 	  If M is selected the module will be called tinydrm.
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 255341ee4eb9..38ba361d1af2 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -12,6 +12,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/tinydrm.h>
 #include <linux/device.h>
 #include <linux/dma-buf.h>
@@ -23,7 +24,7 @@
  *
  * It is based on &drm_simple_display_pipe coupled with a &drm_connector which
  * has only one fixed &drm_display_mode. The framebuffers are backed by the
- * cma helper and have support for framebuffer flushing (dirty).
+ * shmem buffers and have support for framebuffer flushing (dirty).
  * fbdev support is also included.
  *
  */
@@ -37,84 +38,41 @@
  */
 
 /**
- * tinydrm_gem_cma_prime_import_sg_table - Produce a CMA GEM object from
- *     another driver's scatter/gather table of pinned pages
- * @drm: DRM device to import into
- * @attach: DMA-BUF attachment
- * @sgt: Scatter/gather table of pinned pages
+ * tinydrm_fb_destroy - Destroy framebuffer
+ * @fb: Framebuffer
  *
- * This function imports a scatter/gather table exported via DMA-BUF by
- * another driver using drm_gem_cma_prime_import_sg_table(). It sets the
- * kernel virtual address on the CMA object. Drivers should use this as their
- * &drm_driver->gem_prime_import_sg_table callback if they need the virtual
- * address. tinydrm_gem_cma_free_object() should be used in combination with
- * this function.
- *
- * Returns:
- * A pointer to a newly created GEM object or an ERR_PTR-encoded negative
- * error code on failure.
+ * This function unmaps the virtual address on the backing buffer and destroys the framebuffer.
+ * Drivers should use this as their &drm_framebuffer_funcs->destroy callback.
  */
-struct drm_gem_object *
-tinydrm_gem_cma_prime_import_sg_table(struct drm_device *drm,
-				      struct dma_buf_attachment *attach,
-				      struct sg_table *sgt)
+void tinydrm_fb_destroy(struct drm_framebuffer *fb)
 {
-	struct drm_gem_cma_object *cma_obj;
-	struct drm_gem_object *obj;
-	void *vaddr;
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
 
-	vaddr = dma_buf_vmap(attach->dmabuf);
-	if (!vaddr) {
-		DRM_ERROR("Failed to vmap PRIME buffer\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	obj = drm_gem_cma_prime_import_sg_table(drm, attach, sgt);
-	if (IS_ERR(obj)) {
-		dma_buf_vunmap(attach->dmabuf, vaddr);
-		return obj;
-	}
-
-	cma_obj = to_drm_gem_cma_obj(obj);
-	cma_obj->vaddr = vaddr;
-
-	return obj;
+	drm_gem_vunmap(gem, shmem->vaddr);
+	drm_gem_fb_destroy(fb);
 }
-EXPORT_SYMBOL(tinydrm_gem_cma_prime_import_sg_table);
-
-/**
- * tinydrm_gem_cma_free_object - Free resources associated with a CMA GEM
- *                               object
- * @gem_obj: GEM object to free
- *
- * This function frees the backing memory of the CMA GEM object, cleans up the
- * GEM object state and frees the memory used to store the object itself using
- * drm_gem_cma_free_object(). It also handles PRIME buffers which has the kernel
- * virtual address set by tinydrm_gem_cma_prime_import_sg_table(). Drivers
- * can use this as their &drm_driver->gem_free_object_unlocked callback.
- */
-void tinydrm_gem_cma_free_object(struct drm_gem_object *gem_obj)
-{
-	if (gem_obj->import_attach) {
-		struct drm_gem_cma_object *cma_obj;
-
-		cma_obj = to_drm_gem_cma_obj(gem_obj);
-		dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr);
-		cma_obj->vaddr = NULL;
-	}
-
-	drm_gem_cma_free_object(gem_obj);
-}
-EXPORT_SYMBOL_GPL(tinydrm_gem_cma_free_object);
+EXPORT_SYMBOL(tinydrm_fb_destroy);
 
 static struct drm_framebuffer *
 tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv,
 		  const struct drm_mode_fb_cmd2 *mode_cmd)
 {
 	struct tinydrm_device *tdev = drm->dev_private;
+	struct drm_framebuffer *fb;
+	void *vaddr;
 
-	return drm_gem_fb_create_with_funcs(drm, file_priv, mode_cmd,
-					    tdev->fb_funcs);
+	fb = drm_gem_fb_create_with_funcs(drm, file_priv, mode_cmd, tdev->fb_funcs);
+	if (IS_ERR(fb))
+		return fb;
+
+	vaddr = drm_gem_vmap(drm_gem_fb_get_obj(fb, 0));
+	if (IS_ERR(vaddr)) {
+		drm_gem_fb_destroy(fb);
+		return ERR_CAST(vaddr);
+	}
+
+	return fb;
 }
 
 static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index dcd390163a4a..f9d35acff23d 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -9,12 +9,17 @@
 
 #include <linux/backlight.h>
 #include <linux/dma-buf.h>
+#include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/spi/spi.h>
 #include <linux/swab.h>
 
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
 #include <drm/tinydrm/tinydrm.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
+#include <drm/drm_print.h>
 
 static unsigned int spi_max;
 module_param(spi_max, uint, 0400);
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 455fefe012f5..c24d6e4c834b 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -20,8 +20,11 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
@@ -77,7 +80,8 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 			    unsigned int color, struct drm_clip_rect *clips,
 			    unsigned int num_clips)
 {
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
 	struct tinydrm_device *tdev = fb->dev->dev_private;
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	bool swap = mipi->swap_bytes;
@@ -104,7 +108,7 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 		if (ret)
 			return ret;
 	} else {
-		tr = cma_obj->vaddr;
+		tr = shmem->vaddr;
 	}
 
 	switch (mipi->rotation) {
@@ -157,7 +161,7 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
+	.destroy	= tinydrm_fb_destroy,
 	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= tinydrm_fb_dirty,
 };
@@ -361,13 +365,13 @@ static const struct drm_display_mode ili9225_mode = {
 	TINYDRM_MODE(176, 220, 35, 44),
 };
 
-DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(ili9225_fops);
 
 static struct drm_driver ili9225_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &ili9225_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.name			= "ili9225",
 	.desc			= "Ilitek ILI9225",
 	.date			= "20171106",
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 6701037749a7..7a80f8e6d6f3 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -15,8 +15,10 @@
 #include <linux/property.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_modeset_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
@@ -139,12 +141,12 @@ static const struct drm_display_mode yx240qv29_mode = {
 	TINYDRM_MODE(240, 320, 37, 49),
 };
 
-DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(ili9341_fops);
 
 static struct drm_driver ili9341_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
 	.fops			= &ili9341_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "ili9341",
 	.desc			= "Ilitek ILI9341",
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index d7bb4c5e6657..b5837a6b3633 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -17,9 +17,11 @@
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 #include <video/mipi_display.h>
@@ -147,13 +149,13 @@ static const struct drm_display_mode mi0283qt_mode = {
 	TINYDRM_MODE(320, 240, 58, 43),
 };
 
-DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(mi0283qt_fops);
 
 static struct drm_driver mi0283qt_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &mi0283qt_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "mi0283qt",
 	.desc			= "Multi-Inno MI0283QT",
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index cb3441e51d5f..e0f59177c2ef 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -9,10 +9,15 @@
  * (at your option) any later version.
  */
 
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 #include <linux/debugfs.h>
+#include <linux/delay.h>
 #include <linux/dma-buf.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
@@ -167,10 +172,11 @@ EXPORT_SYMBOL(mipi_dbi_command_buf);
 int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 		      struct drm_clip_rect *clip, bool swap)
 {
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
+	struct dma_buf_attachment *import_attach = gem->import_attach;
 	struct drm_format_name_buf format_name;
-	void *src = cma_obj->vaddr;
+	void *src = shmem->vaddr;
 	int ret = 0;
 
 	if (import_attach) {
@@ -210,7 +216,8 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 			     struct drm_clip_rect *clips,
 			     unsigned int num_clips)
 {
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
 	struct tinydrm_device *tdev = fb->dev->dev_private;
 	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
 	bool swap = mipi->swap_bytes;
@@ -235,7 +242,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 		if (ret)
 			return ret;
 	} else {
-		tr = cma_obj->vaddr;
+		tr = shmem->vaddr;
 	}
 
 	mipi_dbi_command(mipi, MIPI_DCS_SET_COLUMN_ADDRESS,
@@ -252,7 +259,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
+	.destroy	= tinydrm_fb_destroy,
 	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= tinydrm_fb_dirty,
 };
@@ -882,31 +889,12 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 {
 	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
 	struct device *dev = &spi->dev;
-	int ret;
 
 	if (tx_size < 16) {
 		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
 		return -EINVAL;
 	}
 
-	/*
-	 * Even though it's not the SPI device that does DMA (the master does),
-	 * the dma mask is necessary for the dma_alloc_wc() in
-	 * drm_gem_cma_create(). The dma_addr returned will be a physical
-	 * adddress which might be different from the bus address, but this is
-	 * not a problem since the address will not be used.
-	 * The virtual address is used in the transfer and the SPI core
-	 * re-maps it on the SPI master device using the DMA streaming API
-	 * (spi_map_buf()).
-	 */
-	if (!dev->coherent_dma_mask) {
-		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret) {
-			dev_warn(dev, "Failed to set dma mask %d\n", ret);
-			return ret;
-		}
-	}
-
 	mipi->spi = spi;
 	mipi->read_commands = mipi_dbi_dcs_read_commands;
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 50a1d4216ce7..82d8c6bf0c91 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -26,7 +26,9 @@
 #include <linux/spi/spi.h>
 #include <linux/thermal.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/tinydrm.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
@@ -526,8 +528,9 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 			    struct drm_clip_rect *clips,
 			    unsigned int num_clips)
 {
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
+	struct dma_buf_attachment *import_attach = gem->import_attach;
 	struct tinydrm_device *tdev = fb->dev->dev_private;
 	struct repaper_epd *epd = epd_from_tinydrm(tdev);
 	struct drm_clip_rect clip;
@@ -559,7 +562,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 			goto out_free;
 	}
 
-	tinydrm_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip);
+	tinydrm_xrgb8888_to_gray8(buf, shmem->vaddr, fb, &clip);
 
 	if (import_attach) {
 		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
@@ -624,7 +627,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs repaper_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
+	.destroy	= tinydrm_fb_destroy,
 	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= tinydrm_fb_dirty,
 };
@@ -876,13 +879,13 @@ static const struct drm_display_mode repaper_e2271cs021_mode = {
 static const u8 repaper_e2271cs021_cs[] = { 0x00, 0x00, 0x00, 0x7f,
 					    0xff, 0xfe, 0x00, 0x00 };
 
-DEFINE_DRM_GEM_CMA_FOPS(repaper_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(repaper_fops);
 
 static struct drm_driver repaper_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &repaper_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.name			= "repaper",
 	.desc			= "Pervasive Displays RePaper e-ink panels",
 	.date			= "20170405",
@@ -929,15 +932,6 @@ static int repaper_probe(struct spi_device *spi)
 		model = spi_id->driver_data;
 	}
 
-	/* The SPI device is used to allocate dma memory */
-	if (!dev->coherent_dma_mask) {
-		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
-		if (ret) {
-			dev_warn(dev, "Failed to set dma mask %d\n", ret);
-			return ret;
-		}
-	}
-
 	epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
 	if (!epd)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 2fcbc3067d71..340c9be2613b 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -17,8 +17,10 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
@@ -88,9 +90,10 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
 			   struct drm_clip_rect *clip)
 {
-	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
-	void *src = cma_obj->vaddr;
+	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
+	struct dma_buf_attachment *import_attach = gem->import_attach;
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(gem);
+	void *src = shmem->vaddr;
 	int ret = 0;
 
 	if (import_attach) {
@@ -156,7 +159,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs st7586_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
+	.destroy	= tinydrm_fb_destroy,
 	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= tinydrm_fb_dirty,
 };
@@ -297,13 +300,13 @@ static const struct drm_display_mode st7586_mode = {
 	TINYDRM_MODE(178, 128, 37, 27),
 };
 
-DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(st7586_fops);
 
 static struct drm_driver st7586_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &st7586_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "st7586",
 	.desc			= "Sitronix ST7586",
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 3081bc57c116..1108088a1a03 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -14,8 +14,10 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
@@ -113,13 +115,13 @@ static const struct drm_display_mode jd_t18003_t01_mode = {
 	TINYDRM_MODE(128, 160, 28, 35),
 };
 
-DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
+DEFINE_DRM_GEM_SHMEM_FOPS(st7735r_fops);
 
 static struct drm_driver st7735r_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &st7735r_fops,
-	TINYDRM_GEM_DRIVER_OPS,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "st7735r",
 	.desc			= "Sitronix ST7735R",
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index fe9827d0ca8a..e2cb254125c5 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -10,10 +10,15 @@
 #ifndef __LINUX_TINYDRM_H
 #define __LINUX_TINYDRM_H
 
-#include <drm/drm_gem_cma_helper.h>
-#include <drm/drm_fb_cma_helper.h>
+#include <linux/mutex.h>
 #include <drm/drm_simple_kms_helper.h>
 
+struct drm_driver;
+struct drm_framebuffer;
+struct drm_file;
+struct drm_clip_rect;
+struct drm_framebuffer_funcs;
+
 /**
  * struct tinydrm_device - tinydrm device
  */
@@ -53,27 +58,6 @@ pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 	return container_of(pipe, struct tinydrm_device, pipe);
 }
 
-/**
- * TINYDRM_GEM_DRIVER_OPS - default tinydrm gem operations
- *
- * This macro provides a shortcut for setting the tinydrm GEM operations in
- * the &drm_driver structure.
- */
-#define TINYDRM_GEM_DRIVER_OPS \
-	.gem_free_object_unlocked = tinydrm_gem_cma_free_object, \
-	.gem_print_info		= drm_gem_cma_print_info, \
-	.gem_vm_ops		= &drm_gem_cma_vm_ops, \
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
-	.gem_prime_import	= drm_gem_prime_import, \
-	.gem_prime_export	= drm_gem_prime_export, \
-	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table, \
-	.gem_prime_import_sg_table = tinydrm_gem_cma_prime_import_sg_table, \
-	.gem_prime_vmap		= drm_gem_cma_prime_vmap, \
-	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap, \
-	.gem_prime_mmap		= drm_gem_cma_prime_mmap, \
-	.dumb_create		= drm_gem_cma_dumb_create
-
 /**
  * TINYDRM_MODE - tinydrm display mode
  * @hd: Horizontal resolution, width
@@ -97,11 +81,7 @@ pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 	.type = DRM_MODE_TYPE_DRIVER, \
 	.clock = 1 /* pass validation */
 
-void tinydrm_gem_cma_free_object(struct drm_gem_object *gem_obj);
-struct drm_gem_object *
-tinydrm_gem_cma_prime_import_sg_table(struct drm_device *drm,
-				      struct dma_buf_attachment *attach,
-				      struct sg_table *sgt);
+void tinydrm_fb_destroy(struct drm_framebuffer *fb);
 int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 		      const struct drm_framebuffer_funcs *fb_funcs,
 		      struct drm_driver *driver);
-- 
2.15.1

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

Re: [PATCH v4 0/4] drm: Add shmem GEM library

[Daniel Vetter]

On Wed, Sep 26, 2018 at 04:59:29PM +0200, Noralf Trønnes wrote:
> This patchset adds a library for shmem backed GEM objects and makes use
> of it in tinydrm.
> 
> Thomas gave me some feedback that made me realise that the cachemodes
> pattern I had picked up from some other drivers didn't apply to shmem
> buffers in the way I though it did. So it's removed.
> 
> I recently did an RFC to see if it made sense to add a vtable to GEM
> objects. Daniel welcomed that so I've included it here.

I like this. Made some tiny comments, but proof's in the pudding so I'll
leave review to someone who knows tinydrm better. Or other driver
maintainers, if you extend the scope a bit. From a high level I think it's
ready, looks all reasonable (except maybe the prime_mmap stuff, that seems
like it can be improved a bit more still).
-Daniel

> 
> Noralf.
> 
> Changes since version 3:
> - Drop cache modes (Thomas Hellstrom)
> - Add a GEM object attached vtable
> 
> Changes since version 2:
> - Grammar (Sam Ravnborg)
> - s/drm_gem_shmem_put_pages_unlocked/drm_gem_shmem_put_pages_locked/
> (Sam Ravnborg)
> - Add debug ouput in error path (Sam Ravnborg)
> 
> Changes since version 1:
> - Fix missing argument in docs (kbuild test robot)
> - Fix: sparse: expression using sizeof(void) (kbuild test robot)
> - Rebasing gave a new checkpatch warning, so I changed to bitfields:
> CHECK: Avoid using bool structure members because of possible alignment
> issues - see: https://lkml.org/lkml/2017/11/21/384
> #834: FILE: include/drm/drm_gem_shmem_helper.h:84:
> +       bool pages_mark_dirty_on_put;
> #841: FILE: include/drm/drm_gem_shmem_helper.h:91:
> +       bool pages_mark_accessed_on_put;
> 
> Noralf Trønnes (4):
>   drm/driver: Add defaults for .gem_prime_export/import callbacks
>   drm/gem: Add drm_gem_object_funcs
>   drm: Add library for shmem backed GEM objects
>   drm/tinydrm: Switch from CMA to shmem buffers
> 
>  Documentation/gpu/drm-kms-helpers.rst          |  12 +
>  Documentation/gpu/todo.rst                     |   7 +
>  drivers/gpu/drm/Kconfig                        |   6 +
>  drivers/gpu/drm/Makefile                       |   1 +
>  drivers/gpu/drm/drm_client.c                   |  12 +-
>  drivers/gpu/drm/drm_fb_helper.c                |   8 +-
>  drivers/gpu/drm/drm_gem.c                      | 109 ++++-
>  drivers/gpu/drm/drm_gem_shmem_helper.c         | 586 +++++++++++++++++++++++++
>  drivers/gpu/drm/drm_prime.c                    |  50 ++-
>  drivers/gpu/drm/tinydrm/Kconfig                |   2 +-
>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c    |  92 ++--
>  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c |   5 +
>  drivers/gpu/drm/tinydrm/ili9225.c              |  14 +-
>  drivers/gpu/drm/tinydrm/ili9341.c              |   6 +-
>  drivers/gpu/drm/tinydrm/mi0283qt.c             |   6 +-
>  drivers/gpu/drm/tinydrm/mipi-dbi.c             |  38 +-
>  drivers/gpu/drm/tinydrm/repaper.c              |  24 +-
>  drivers/gpu/drm/tinydrm/st7586.c               |  15 +-
>  drivers/gpu/drm/tinydrm/st7735r.c              |   6 +-
>  include/drm/drm_drv.h                          |   4 +
>  include/drm/drm_gem.h                          | 140 ++++++
>  include/drm/drm_gem_shmem_helper.h             | 153 +++++++
>  include/drm/tinydrm/tinydrm.h                  |  36 +-
>  23 files changed, 1139 insertions(+), 193 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_gem_shmem_helper.c
>  create mode 100644 include/drm/drm_gem_shmem_helper.h
> 
> -- 
> 2.15.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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