Prime helpers

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into GTT
gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export
gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures.
- Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

drivers/gpu/drm/drm_prime.c +4 -0

+static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
+		enum dma_data_direction dir)
+{
+	struct drm_gem_object *obj = attach->dmabuf->priv;

drivers/gpu/drm/drm_prime.c +27 -0

+	return sgt;
+}
+
+static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
+		struct sg_table *sgt, enum dma_data_direction dir)
+{
+	dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+	sg_free_table(sgt);
+	kfree(sgt);
+}
+
+static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+
+	if (obj->export_dma_buf == dma_buf) {
+		/* drop the reference on the export fd holds */
+		obj->export_dma_buf = NULL;
+		drm_gem_object_unreference_unlocked(obj);
+	}
+}
+
+static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+

drivers/gpu/drm/drm_prime.c +132 -0

+
+static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
+		unsigned long page_num)
+{
+	return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
+		unsigned long page_num, void *addr)
+{
+
+}
+static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,
+		unsigned long page_num)
+{
+	return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
+		unsigned long page_num, void *addr)
+{
+
+}
+
+static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
+		struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+
+static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.map_dma_buf = drm_gem_map_dma_buf,
+	.unmap_dma_buf = drm_gem_unmap_dma_buf,
+	.release = drm_gem_dmabuf_release,
+	.kmap = drm_gem_dmabuf_kmap,
+	.kmap_atomic = drm_gem_dmabuf_kmap_atomic,
+	.kunmap = drm_gem_dmabuf_kunmap,
+	.kunmap_atomic = drm_gem_dmabuf_kunmap_atomic,
+	.mmap = drm_gem_dmabuf_mmap,
+	.vmap = drm_gem_dmabuf_vmap,
+	.vunmap = drm_gem_dmabuf_vunmap,
+};
+
+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
+ * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
+ * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ *  - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ *  - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages
+ *
+ *  - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ *  - @gem_prime_import_sg_table (import): produce a GEM object from another
+ *    driver's scatter/gather table
+ */
+
+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+				     struct drm_gem_object *obj, int flags)
+{
+	if (dev->driver->gem_prime_pin) {
+		int ret = dev->driver->gem_prime_pin(obj);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			      0600);
+}
+EXPORT_SYMBOL(drm_gem_prime_export);
+
 int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 		struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 		int *prime_fd)

@@ -117,6 +249,58 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+					    struct dma_buf *dma_buf)
+{
+	struct dma_buf_attachment *attach;
+	struct sg_table *sgt;
+	struct drm_gem_object *obj;
+	int ret;
+
+	if (!dev->driver->gem_prime_import_sg_table)
+		return ERR_PTR(-EINVAL);
+
+	if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {
+		obj = dma_buf->priv;
+		if (obj->dev == dev) {
+			/*
+			 * Importing dmabuf exported from out own gem increases
+			 * refcount on gem itself instead of f_count of dmabuf.
+			 */
+			drm_gem_object_reference(obj);
+			dma_buf_put(dma_buf);
+			return obj;
+		}
+	}
+
+	attach = dma_buf_attach(dma_buf, dev->dev);
+	if (IS_ERR(attach))
+		return ERR_PTR(PTR_ERR(attach));
+
+	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+	if (IS_ERR_OR_NULL(sgt)) {
+		ret = PTR_ERR(sgt);
+		goto fail_detach;
+	}
+
+	obj = dev->driver->gem_prime_import_sg_table(dev, dma_buf->size, sgt);
+	if (IS_ERR(obj)) {
+		ret = PTR_ERR(obj);
+		goto fail_unmap;
+	}
+
+	obj->import_attach = attach;
+
+	return obj;
+
+fail_unmap:
+	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
+fail_detach:
+	dma_buf_detach(dma_buf, attach);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(drm_gem_prime_import);
+
 int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 		struct drm_file *file_priv, int prime_fd, uint32_t *handle)
 {

include/drm/drmP.h +12 -0

index fad21c9..2d4ca6f 100644
@@ -919,6 +919,14 @@ struct drm_driver {
 	/* import dmabuf -> GEM */
 	struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
 				struct dma_buf *dma_buf);
+	/* low-level interface used by drm_gem_prime_{import,export} */
+	int (*gem_prime_pin)(struct drm_gem_object *obj);
+	struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
+	struct drm_gem_object *(*gem_prime_import_sg_table)(
+				struct drm_device *dev, size_t size,
+				struct sg_table *sgt);
+	void *(*gem_prime_vmap)(struct drm_gem_object *obj);
+	void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr);
 
 	/* vga arb irq handler */
 	void (*vgaarb_irq)(struct drm_device *dev, bool state);

@@ -1540,9 +1548,13 @@ extern int drm_clients_info(struct seq_file *m, void* data);
 extern int drm_gem_name_info(struct seq_file *m, void *data);
 
 
+extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+		struct drm_gem_object *obj, int flags);
 extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 		struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 		int *prime_fd);
+extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+		struct dma_buf *dma_buf);
 extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 		struct drm_file *file_priv, int prime_fd, uint32_t *handle);
 

Signature · 1 line

1.8.1.1

Quoted 33 lines · Aaron Plattner

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into GTT
gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export
gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures.
- Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Quoted 3 lines

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

Two minor things, but since you're not touching drm/i915 I don't care that much ...

Quoted 11 lines

(quoted diff) +11 -0

+static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
+             enum dma_data_direction dir)
+{
+     struct drm_gem_object *obj = attach->dmabuf->priv;
+     struct sg_table *sgt;
+
+     mutex_lock(&obj->dev->struct_mutex);
+
+     sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
+
+     if (!IS_ERR_OR_NULL(sgt))

Review

Quoted 3 lines

(quoted diff) +3 -0

+             dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+
+     mutex_unlock(&obj->dev->struct_mutex);

The locking here looks superflous and not required to protect anything drm_gem_map_dma_buf does. If drivers need this (and ttm based ones shouldn't really), they can grab it in their ->gem_prime_get_sg_table callback. So I think a follow-up patch to ditch this would be good.

Quoted 35 lines

(quoted diff) +35 -0

+     return sgt;
+}
+
+static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
+             struct sg_table *sgt, enum dma_data_direction dir)
+{
+     dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+     sg_free_table(sgt);
+     kfree(sgt);
+}
+
+static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+{
+     struct drm_gem_object *obj = dma_buf->priv;
+
+     if (obj->export_dma_buf == dma_buf) {
+             /* drop the reference on the export fd holds */
+             obj->export_dma_buf = NULL;
+             drm_gem_object_unreference_unlocked(obj);
+     }
+}
+
+static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
+{
+     struct drm_gem_object *obj = dma_buf->priv;
+     struct drm_device *dev = obj->dev;
+
+     return dev->driver->gem_prime_vmap(obj);
+}
+
+static 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;
+

Review

Quoted 2 lines

(quoted diff) +2 -0

+     dev->driver->gem_prime_vunmap(obj, vaddr);
+}

Imo these two don't add value, simpler to add a typechecking drm_dmabuf_to_gem_obj static inline somewhere. But then you'd need to pass in the dmabuf fops struct from drivers and export the helpers. More flexible, but also a bit work to redo. Like I've said, I don't care too much ...

Can I consider this a Reviewed-by?

Quoted 1 line

Can I consider this a Reviewed-by?

Essentially it was just a drive-by bikeshed ;-) I think it'd be good if Maarten takes a look at this and checks whether it complies with his massive prime/dma_buf rework to use fences and ticketing reservations ...
-Daniel

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into GTT
gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export
gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures.
- Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---
 Documentation/DocBook/drm.tmpl |   4 +
 drivers/gpu/drm/drm_prime.c    | 186 ++++++++++++++++++++++++++++++++++++++++-
 include/drm/drmP.h             |  12 +++
 3 files changed, 201 insertions(+), 1 deletion(-)

Documentation/DocBook/drm.tmpl +4 -0

index 4ee2304..ed40576 100644
@@ -743,6 +743,10 @@ char *date;</synopsis>
           These two operations are mandatory for GEM drivers that support DRM
           PRIME.
         </para>
+        <sect4>
+          <title>DRM PRIME Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_prime.c PRIME Helpers
+        </sect4>
       </sect3>
       <sect3 id="drm-gem-objects-mapping">
         <title>GEM Objects Mapping</title>

drivers/gpu/drm/drm_prime.c +135 -1

index 7f12573..366910d 100644
@@ -53,7 +53,8 @@
  * Self-importing: if userspace is using PRIME as a replacement for flink
  * then it will get a fd->handle request for a GEM object that it created.
  * Drivers should detect this situation and return back the gem object
- * from the dma-buf private.
+ * from the dma-buf private.  Prime will do this automatically for drivers that
+ * use the drm_gem_prime_{import,export} helpers.
  */
 
 struct drm_prime_member {

@@ -62,6 +63,137 @@ struct drm_prime_member {
 	uint32_t handle;
 };
 
+static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
+		enum dma_data_direction dir)
+{
+	struct drm_gem_object *obj = attach->dmabuf->priv;
+	struct sg_table *sgt;
+
+	mutex_lock(&obj->dev->struct_mutex);
+
+	sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
+
+	if (!IS_ERR_OR_NULL(sgt))
+		dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+
+	mutex_unlock(&obj->dev->struct_mutex);
+	return sgt;
+}
+
+static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
+		struct sg_table *sgt, enum dma_data_direction dir)
+{
+	dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+	sg_free_table(sgt);
+	kfree(sgt);
+}
+
+static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+
+	if (obj->export_dma_buf == dma_buf) {
+		/* drop the reference on the export fd holds */
+		obj->export_dma_buf = NULL;
+		drm_gem_object_unreference_unlocked(obj);
+	}
+}
+
+static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+
+	return dev->driver->gem_prime_vmap(obj);
+}
+
+static 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;
+
+	dev->driver->gem_prime_vunmap(obj, vaddr);
+}
+
+static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
+		unsigned long page_num)
+{
+	return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
+		unsigned long page_num, void *addr)
+{
+
+}
+static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,
+		unsigned long page_num)
+{
+	return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
+		unsigned long page_num, void *addr)
+{
+
+}
+
+static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
+		struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+
+static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+	.map_dma_buf = drm_gem_map_dma_buf,
+	.unmap_dma_buf = drm_gem_unmap_dma_buf,
+	.release = drm_gem_dmabuf_release,
+	.kmap = drm_gem_dmabuf_kmap,
+	.kmap_atomic = drm_gem_dmabuf_kmap_atomic,
+	.kunmap = drm_gem_dmabuf_kunmap,
+	.kunmap_atomic = drm_gem_dmabuf_kunmap_atomic,
+	.mmap = drm_gem_dmabuf_mmap,
+	.vmap = drm_gem_dmabuf_vmap,
+	.vunmap = drm_gem_dmabuf_vunmap,
+};
+
+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
+ * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
+ * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ *  - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ *  - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages
+ *
+ *  - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ *  - @gem_prime_import_sg_table (import): produce a GEM object from another
+ *    driver's scatter/gather table
+ */
+
+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+				     struct drm_gem_object *obj, int flags)
+{
+	if (dev->driver->gem_prime_pin) {
+		int ret = dev->driver->gem_prime_pin(obj);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			      0600);
+}
+EXPORT_SYMBOL(drm_gem_prime_export);
+
 int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 		struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 		int *prime_fd)

@@ -117,6 +249,58 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+					    struct dma_buf *dma_buf)

drivers/gpu/drm/drm_prime.c +40 -0

+		obj = dma_buf->priv;
+		if (obj->dev == dev) {
+			/*
+			 * Importing dmabuf exported from out own gem increases
+			 * refcount on gem itself instead of f_count of dmabuf.
+			 */
+			drm_gem_object_reference(obj);
+			dma_buf_put(dma_buf);
+			return obj;
+		}
+	}
+
+	attach = dma_buf_attach(dma_buf, dev->dev);
+	if (IS_ERR(attach))
+		return ERR_PTR(PTR_ERR(attach));
+
+	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+	if (IS_ERR_OR_NULL(sgt)) {
+		ret = PTR_ERR(sgt);
+		goto fail_detach;
+	}
+
+	obj = dev->driver->gem_prime_import_sg_table(dev, dma_buf->size, sgt);
+	if (IS_ERR(obj)) {
+		ret = PTR_ERR(obj);
+		goto fail_unmap;
+	}
+
+	obj->import_attach = attach;
+
+	return obj;
+
+fail_unmap:
+	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
+fail_detach:
+	dma_buf_detach(dma_buf, attach);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(drm_gem_prime_import);
+
 int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 		struct drm_file *file_priv, int prime_fd, uint32_t *handle)
 {

include/drm/drmP.h +12 -0

index fad21c9..2d4ca6f 100644
@@ -919,6 +919,14 @@ struct drm_driver {
 	/* import dmabuf -> GEM */
 	struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
 				struct dma_buf *dma_buf);
+	/* low-level interface used by drm_gem_prime_{import,export} */
+	int (*gem_prime_pin)(struct drm_gem_object *obj);
+	struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
+	struct drm_gem_object *(*gem_prime_import_sg_table)(
+				struct drm_device *dev, size_t size,
+				struct sg_table *sgt);
+	void *(*gem_prime_vmap)(struct drm_gem_object *obj);
+	void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr);
 
 	/* vga arb irq handler */
 	void (*vgaarb_irq)(struct drm_device *dev, bool state);

@@ -1540,9 +1548,13 @@ extern int drm_clients_info(struct seq_file *m, void* data);
 extern int drm_gem_name_info(struct seq_file *m, void *data);
 
 
+extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+		struct drm_gem_object *obj, int flags);
 extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 		struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 		int *prime_fd);
+extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+		struct dma_buf *dma_buf);
 extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 		struct drm_file *file_priv, int prime_fd, uint32_t *handle);
 

Signature · 1 line

1.8.1.1

Quoted 223 lines · Aaron Plattner

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into GTT
gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export
gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures.
- Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---
  Documentation/DocBook/drm.tmpl |   4 +
  drivers/gpu/drm/drm_prime.c    | 186 ++++++++++++++++++++++++++++++++++++++++-
  include/drm/drmP.h             |  12 +++
  3 files changed, 201 insertions(+), 1 deletion(-)

Documentation/DocBook/drm.tmpl +4 -0

index 4ee2304..ed40576 100644
@@ -743,6 +743,10 @@ char *date;</synopsis>
            These two operations are mandatory for GEM drivers that support DRM
            PRIME.
          </para>
+        <sect4>
+          <title>DRM PRIME Helper Functions Reference</title>
+!Pdrivers/gpu/drm/drm_prime.c PRIME Helpers
+        </sect4>
        </sect3>
        <sect3 id="drm-gem-objects-mapping">
          <title>GEM Objects Mapping</title>

drivers/gpu/drm/drm_prime.c +143 -1

index 7f12573..366910d 100644
@@ -53,7 +53,8 @@
   * Self-importing: if userspace is using PRIME as a replacement for flink
   * then it will get a fd->handle request for a GEM object that it created.
   * Drivers should detect this situation and return back the gem object
- * from the dma-buf private.
+ * from the dma-buf private.  Prime will do this automatically for drivers that
+ * use the drm_gem_prime_{import,export} helpers.
   */
 
  struct drm_prime_member {

@@ -62,6 +63,137 @@ struct drm_prime_member {
       uint32_t handle;
  };
 
+static struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
+             enum dma_data_direction dir)
+{
+     struct drm_gem_object *obj = attach->dmabuf->priv;
+     struct sg_table *sgt;
+
+     mutex_lock(&obj->dev->struct_mutex);
+
+     sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
+
+     if (!IS_ERR_OR_NULL(sgt))
+             dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+
+     mutex_unlock(&obj->dev->struct_mutex);
+     return sgt;
+}
+
+static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
+             struct sg_table *sgt, enum dma_data_direction dir)
+{
+     dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+     sg_free_table(sgt);
+     kfree(sgt);
+}
+
+static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+{
+     struct drm_gem_object *obj = dma_buf->priv;
+
+     if (obj->export_dma_buf == dma_buf) {
+             /* drop the reference on the export fd holds */
+             obj->export_dma_buf = NULL;
+             drm_gem_object_unreference_unlocked(obj);
+     }
+}
+
+static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
+{
+     struct drm_gem_object *obj = dma_buf->priv;
+     struct drm_device *dev = obj->dev;
+
+     return dev->driver->gem_prime_vmap(obj);
+}
+
+static 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;
+
+     dev->driver->gem_prime_vunmap(obj, vaddr);
+}
+
+static void *drm_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
+             unsigned long page_num)
+{
+     return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
+             unsigned long page_num, void *addr)
+{
+
+}
+static void *drm_gem_dmabuf_kmap(struct dma_buf *dma_buf,
+             unsigned long page_num)
+{
+     return NULL;
+}
+
+static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
+             unsigned long page_num, void *addr)
+{
+
+}
+
+static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf,
+             struct vm_area_struct *vma)
+{
+     return -EINVAL;
+}
+
+static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =  {
+     .map_dma_buf = drm_gem_map_dma_buf,
+     .unmap_dma_buf = drm_gem_unmap_dma_buf,
+     .release = drm_gem_dmabuf_release,
+     .kmap = drm_gem_dmabuf_kmap,
+     .kmap_atomic = drm_gem_dmabuf_kmap_atomic,
+     .kunmap = drm_gem_dmabuf_kunmap,
+     .kunmap_atomic = drm_gem_dmabuf_kunmap_atomic,
+     .mmap = drm_gem_dmabuf_mmap,
+     .vmap = drm_gem_dmabuf_vmap,
+     .vunmap = drm_gem_dmabuf_vunmap,
+};
+
+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms of
+ * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import.  These functions implement dma-buf support in terms of
+ * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ *  - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ *  - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages
+ *
+ *  - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ *  - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ *  - @gem_prime_import_sg_table (import): produce a GEM object from another
+ *    driver's scatter/gather table
+ */
+
+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+                                  struct drm_gem_object *obj, int flags)
+{
+     if (dev->driver->gem_prime_pin) {
+             int ret = dev->driver->gem_prime_pin(obj);
+             if (ret)
+                     return ERR_PTR(ret);
+     }
+     return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+                           0600);
+}
+EXPORT_SYMBOL(drm_gem_prime_export);
+
  int drm_gem_prime_handle_to_fd(struct drm_device *dev,
               struct drm_file *file_priv, uint32_t handle, uint32_t flags,
               int *prime_fd)

@@ -117,6 +249,58 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
  }
  EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+                                         struct dma_buf *dma_buf)
+{
+     struct dma_buf_attachment *attach;
+     struct sg_table *sgt;
+     struct drm_gem_object *obj;
+     int ret;
+
+     if (!dev->driver->gem_prime_import_sg_table)
+             return ERR_PTR(-EINVAL);

drivers/gpu/drm/drm_prime.c int drm_gem_prime_handle_to_fd(struct drm_device *dev, · line 263

Quoted 2 lines

drivers/gpu/drm/drm_prime.c +2 -0

+
+     if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {

This here breaks self-import checks since it smashes all buffers from all drivers using these helpers into one set. Which means e.g. nouveau will happily import a buffer from radoen as it's own. The only reason afaics that shit didn't completely hit the fan is that i915 still has it's own buffer ops, and everyone seems to only care about sharing with i915.

So after a few months of subconscious pondering (well, just re-read the code to review Dave's patch) I now finally know why I totally didn't like that we move the vtable into the helpers ;-)

I think someone needs to fix this or we need to revert this patch here.

Cheers, Daniel

Quoted 77 lines

drivers/gpu/drm/drm_prime.c +40 -0

+             obj = dma_buf->priv;
+             if (obj->dev == dev) {
+                     /*
+                      * Importing dmabuf exported from out own gem increases
+                      * refcount on gem itself instead of f_count of dmabuf.
+                      */
+                     drm_gem_object_reference(obj);
+                     dma_buf_put(dma_buf);
+                     return obj;
+             }
+     }
+
+     attach = dma_buf_attach(dma_buf, dev->dev);
+     if (IS_ERR(attach))
+             return ERR_PTR(PTR_ERR(attach));
+
+     sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+     if (IS_ERR_OR_NULL(sgt)) {
+             ret = PTR_ERR(sgt);
+             goto fail_detach;
+     }
+
+     obj = dev->driver->gem_prime_import_sg_table(dev, dma_buf->size, sgt);
+     if (IS_ERR(obj)) {
+             ret = PTR_ERR(obj);
+             goto fail_unmap;
+     }
+
+     obj->import_attach = attach;
+
+     return obj;
+
+fail_unmap:
+     dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
+fail_detach:
+     dma_buf_detach(dma_buf, attach);
+     return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(drm_gem_prime_import);
+
  int drm_gem_prime_fd_to_handle(struct drm_device *dev,
               struct drm_file *file_priv, int prime_fd, uint32_t *handle)
  {

include/drm/drmP.h +12 -1

index fad21c9..2d4ca6f 100644
@@ -919,6 +919,14 @@ struct drm_driver {
       /* import dmabuf -> GEM */
       struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
                               struct dma_buf *dma_buf);
+     /* low-level interface used by drm_gem_prime_{import,export} */
+     int (*gem_prime_pin)(struct drm_gem_object *obj);
+     struct sg_table *(*gem_prime_get_sg_table)(struct drm_gem_object *obj);
+     struct drm_gem_object *(*gem_prime_import_sg_table)(
+                             struct drm_device *dev, size_t size,
+                             struct sg_table *sgt);
+     void *(*gem_prime_vmap)(struct drm_gem_object *obj);
+     void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr);
 
       /* vga arb irq handler */
       void (*vgaarb_irq)(struct drm_device *dev, bool state);

@@ -1540,9 +1548,13 @@ extern int drm_clients_info(struct seq_file *m, void* data);
  extern int drm_gem_name_info(struct seq_file *m, void *data);
 
 
+extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+             struct drm_gem_object *obj, int flags);
  extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
               struct drm_file *file_priv, uint32_t handle, uint32_t flags,
               int *prime_fd);
+extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+             struct dma_buf *dma_buf);
  extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
               struct drm_file *file_priv, int prime_fd, uint32_t *handle);
 
--

Quoted 2 lines

1.8.1.1

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Quoted 16 lines

Quoted 16 lines

drivers/gpu/drm/drm_prime.c +11 -0

@@ -117,6 +249,58 @@ int drm_gem_prime_handle_to_fd(struct drm_device
 *dev,
  }
  EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+                                         struct dma_buf *dma_buf)
+{
+     struct dma_buf_attachment *attach;
+     struct sg_table *sgt;
+     struct drm_gem_object *obj;
+     int ret;
+
+     if (!dev->driver->gem_prime_import_sg_table)
+             return ERR_PTR(-EINVAL);
+

Quoted 8 lines

drivers/gpu/drm/drm_prime.c int drm_gem_prime_handle_to_fd(struct drm_device *dev, · line 263

Quoted 1 line

drivers/gpu/drm/drm_prime.c +1 -0

+     if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) {

This here breaks self-import checks since it smashes all buffers from all drivers using these helpers into one set. Which means e.g. nouveau will happily import a buffer from radoen as it's own. The only reason afaics that shit didn't completely hit the fan is that i915 still has it's own buffer ops, and everyone seems to only care about sharing with i915.

Doesn't the (obj->dev == dev) check below guard against that?

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into
GTT gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures. - Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---
 Documentation/DocBook/drm.tmpl |   4 +
 drivers/gpu/drm/drm_prime.c    | 186 +++++++++++++++++++++++++++++++++++++-
 include/drm/drmP.h             |  12 +++
 3 files changed, 201 insertions(+), 1 deletion(-)

drivers/gpu/drm/drm_prime.c +0 -0

index 7f12573..366910d 100644

+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms of + * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import. These functions implement dma-buf support in terms of + * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ * - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ * - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages + *
+ * - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ * - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ * - @gem_prime_import_sg_table (import): produce a GEM object from
another + * driver's scatter/gather table
+ */

drivers/gpu/drm/drm_prime.c +1 -0

+

drivers/gpu/drm/drm_prime.c +2 -0

+}
+EXPORT_SYMBOL(drm_gem_prime_export);

Hi Aaron,

A bit late, but here's a small question.

Quoted 42 lines · On Tuesday 15 January 2013 12:47:42 Aaron Plattner wrote:

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:

gem_prime_pin: callback when a buffer is created, used to pin buffers into
GTT gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export gem_prime_import_sg_table: convert an sg_table into a drm_gem_object gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table. Rename struct sg_table* variables to 'sgt' for clarity.
- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures. - Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec ("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---
  Documentation/DocBook/drm.tmpl |   4 +
  drivers/gpu/drm/drm_prime.c    | 186 +++++++++++++++++++++++++++++++++++++-
  include/drm/drmP.h             |  12 +++
  3 files changed, 201 insertions(+), 1 deletion(-)

[snip]

Quoted 4 lines

drivers/gpu/drm/drm_prime.c +0 -0

index 7f12573..366910d 100644

[snip]

Quoted 28 lines

+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms of + * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import. These functions implement dma-buf support in terms of + * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ * - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ * - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages + *
+ * - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ * - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ * - @gem_prime_import_sg_table (import): produce a GEM object from
another + * driver's scatter/gather table
+ */

drivers/gpu/drm/drm_prime.c +5 -0

+
+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+				     struct drm_gem_object *obj, int flags)
+{
+	if (dev->driver->gem_prime_pin) {

drivers/gpu/drm/drm_prime.c struct drm_prime_member { · line 193

Quoted 6 lines

drivers/gpu/drm/drm_prime.c +6 -0

+		int ret = dev->driver->gem_prime_pin(obj);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			      0600);

Why do you use 0600 instead of the flags passed by the caller ?

Quoted 2 lines

drivers/gpu/drm/drm_prime.c +2 -0

+}
+EXPORT_SYMBOL(drm_gem_prime_export);

Quoted 94 lines · Laurent Pinchart

Hi Aaron,

A bit late, but here's a small question.

Quoted 49 lines · On Tuesday 15 January 2013 12:47:42 Aaron Plattner wrote:

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:
gem_prime_pin: callback when a buffer is created, used to pin buffers into
GTT gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export gem_prime_import_sg_table: convert an sg_table into a drm_gem_object
gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table.

Rename struct sg_table* variables to 'sgt' for clarity.

- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures. - Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but
the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec

("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---

  Documentation/DocBook/drm.tmpl |   4 +
  drivers/gpu/drm/drm_prime.c    | 186
  +++++++++++++++++++++++++++++++++++++-
  include/drm/drmP.h             |  12 +++
  3 files changed, 201 insertions(+), 1 deletion(-)

[snip]

Quoted 4 lines

drivers/gpu/drm/drm_prime.c +0 -0

index 7f12573..366910d 100644

[snip]

Quoted 30 lines

+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms
of + * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import. These functions implement dma-buf support in terms of + * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ * - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ * - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages + *
+ * - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ * - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ * - @gem_prime_import_sg_table (import): produce a GEM object from
another + * driver's scatter/gather table
+ */

drivers/gpu/drm/drm_prime.c +5 -0

+
+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+				     struct drm_gem_object *obj, int flags)
+{
+	if (dev->driver->gem_prime_pin) {

Quoted 8 lines

drivers/gpu/drm/drm_prime.c struct drm_prime_member { · line 193

Quoted 6 lines

drivers/gpu/drm/drm_prime.c +6 -0

+		int ret = dev->driver->gem_prime_pin(obj);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			      0600);

Why do you use 0600 instead of the flags passed by the caller ?

Because I copied & pasted it from i915_gem_prime_export prior to commit 5b42427fc38ecb9056c4e64deaff36d6d6ba1b67 and didn't notice until you pointed it out just now.

Quoted 2 lines

Quoted 2 lines

drivers/gpu/drm/drm_prime.c +2 -0

+}
+EXPORT_SYMBOL(drm_gem_prime_export);

Hi Aaron,

Quoted 104 lines · On Tuesday 18 June 2013 16:28:15 Aaron Plattner wrote:

Quoted 102 lines · Laurent Pinchart

Hi Aaron,

A bit late, but here's a small question.

Quoted 49 lines · On Tuesday 15 January 2013 12:47:42 Aaron Plattner wrote:

Instead of reimplementing all of the dma_buf functionality in every driver, create helpers drm_prime_import and drm_prime_export that implement them in terms of new, lower-level hook functions:
gem_prime_pin: callback when a buffer is created, used to pin buffers into
GTT gem_prime_get_sg_table: convert a drm_gem_object to an sg_table for export gem_prime_import_sg_table: convert an sg_table into a drm_gem_object
gem_prime_vmap, gem_prime_vunmap: map and unmap an object

These hooks are optional; drivers can opt in by using drm_gem_prime_import and drm_gem_prime_export as the .gem_prime_import and .gem_prime_export fields of struct drm_driver.

v2:
- Drop .begin_cpu_access. None of the drivers this code replaces implemented it. Having it here was a leftover from when I was trying to include i915 in this rework.
- Use mutex_lock instead of mutex_lock_interruptible, as these three drivers did. This patch series shouldn't change that behavior.
- Rename helpers to gem_prime_get_sg_table and gem_prime_import_sg_table.

Rename struct sg_table* variables to 'sgt' for clarity.

- Update drm.tmpl for these new hooks.

v3:
- Pass the vaddr down to the driver. This lets drivers that just call vunmap on the pointer avoid having to store the pointer in their GEM private structures. - Move documentation into a /** DOC */ comment in drm_prime.c and include it in drm.tmpl with a !P line. I tried to use !F lines to include documentation of the individual functions from drmP.h, but
the docproc / kernel-doc scripts barf on that file, so hopefully this is good enough for now.
- apply refcount fix from commit be8a42ae60addd8b6092535c11b42d099d6470ec

("drm/prime: drop reference on imported dma-buf come from gem")

Signed-off-by Aaron Plattner <[email protected]>

Cc Daniel Vetter <[email protected]>

Cc David Airlie <[email protected]>

---

   Documentation/DocBook/drm.tmpl |   4 +
   drivers/gpu/drm/drm_prime.c    | 186
   +++++++++++++++++++++++++++++++++++++-
   include/drm/drmP.h             |  12 +++
   3 files changed, 201 insertions(+), 1 deletion(-)

[snip]

Quoted 4 lines

drivers/gpu/drm/drm_prime.c +0 -0

index 7f12573..366910d 100644

[snip]

Quoted 26 lines

+/**
+ * DOC: PRIME Helpers
+ *
+ * Drivers can implement @gem_prime_export and @gem_prime_import in terms
of + * simpler APIs by using the helper functions @drm_gem_prime_export and
+ * @drm_gem_prime_import. These functions implement dma-buf support in terms of + * five lower-level driver callbacks:
+ *
+ * Export callbacks:
+ *
+ * - @gem_prime_pin (optional): prepare a GEM object for exporting
+ *
+ * - @gem_prime_get_sg_table: provide a scatter/gather table of pinned pages + *
+ * - @gem_prime_vmap: vmap a buffer exported by your driver
+ *
+ * - @gem_prime_vunmap: vunmap a buffer exported by your driver
+ *
+ * Import callback:
+ *
+ * - @gem_prime_import_sg_table (import): produce a GEM object from
another + * driver's scatter/gather table
+ */

drivers/gpu/drm/drm_prime.c +1 -0

+

drivers/gpu/drm/drm_prime.c struct drm_prime_member { · line 193

Quoted 10 lines

drivers/gpu/drm/drm_prime.c +10 -0

+struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
+				     struct drm_gem_object *obj, int flags)
+{
+	if (dev->driver->gem_prime_pin) {
+		int ret = dev->driver->gem_prime_pin(obj);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+	return dma_buf_export(obj, &drm_gem_prime_dmabuf_ops, obj->size,
+			      0600);

Why do you use 0600 instead of the flags passed by the caller ?

Quoted 3 lines

Because I copied & pasted it from i915_gem_prime_export prior to commit 5b42427fc38ecb9056c4e64deaff36d6d6ba1b67 and didn't notice until you pointed it out just now.

That's a pretty valid reason I guess :-) Should I submit a patch or are you already working on one ?

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

drivers/gpu/drm/drm_prime.c +2 -0

+}
+EXPORT_SYMBOL(drm_gem_prime_export);