All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] RFCv3 VGEM Prime (dma-buf)
@ 2012-02-22 19:29 Ben Widawsky
  2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Daniel Vetter, Ben Widawsky, Dave Airlie

I'm going to be off doing other things for the next couple of weeks, so
I'm dropping these now to give it a nice soak while I'm gone.

Dave/Daniel: if you could look these over and tell me if the general
direction seems good.

Ajax: anything you missing in the basic vgem stuff?

Since the last time:
Squashed down the original vgem patches
Use dumb_bo functions/ditched VGEM ioctls
Hooked up prime import and export support

On the prime side, the major difference from what Dave has done before
is a per driver hash of the previously used dma bufs/gem objects.

The prime stuff is of particularly low quality at this point, like I
said, trying to get something out before I disappear for a while. So
please don't yell at me about obvious bugs :). After getting feedback on
what I have now, I will incorporate Dave's earlier work on i915 prime,
and get some better test cases going.

On my todos:
Ascii chart of dmabuf/drm object life cycle
hashsify the per file list
i915 per driver hash
vgem-i915 and vice versa test cases

As before, the very basic tools are here:
git://people.freedesktop.org/~bwidawsk/vgem-gpu-tools

Once we get cpu maps that I think Daniel wants to work on, I can even do
better tests with just VGEM.

Adam Jackson (1):
  drm/vgem: virtual GEM provider

Ben Widawsky (5):
  drm: DRM_DEBUG_PRIME
  drm: per device prime dma buf hash
  drm/vgem: prime export support
  drm/vgem: import support
  drm: actually enable PRIME

Dave Airlie (1):
  drm: base prime support

 drivers/gpu/drm/Kconfig             |    9 +
 drivers/gpu/drm/Makefile            |    3 +-
 drivers/gpu/drm/drm_drv.c           |    3 +
 drivers/gpu/drm/drm_gem.c           |    4 +
 drivers/gpu/drm/drm_prime.c         |  172 +++++++++++++++
 drivers/gpu/drm/drm_stub.c          |    8 +
 drivers/gpu/drm/vgem/Makefile       |    4 +
 drivers/gpu/drm/vgem/vgem_dma_buf.c |  248 ++++++++++++++++++++++
 drivers/gpu/drm/vgem/vgem_drv.c     |  389 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/vgem/vgem_drv.h     |   61 ++++++
 include/drm/drm.h                   |   10 +-
 include/drm/drmP.h                  |   55 +++++
 12 files changed, 964 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_prime.c
 create mode 100644 drivers/gpu/drm/vgem/Makefile
 create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c
 create mode 100644 drivers/gpu/drm/vgem/vgem_drv.c
 create mode 100644 drivers/gpu/drm/vgem/vgem_drv.h

-- 
1.7.9.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] drm: base prime support
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-22 19:58   ` Kristian Høgsberg
  2012-02-27  2:49   ` InKi Dae
  2012-02-22 19:29 ` [PATCH 2/7] DRM_DEBUG_PRIME Ben Widawsky
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Dave Airlie

From: Dave Airlie <airlied@redhat.com>

---
 drivers/gpu/drm/Makefile    |    2 +-
 drivers/gpu/drm/drm_drv.c   |    3 +
 drivers/gpu/drm/drm_gem.c   |    3 +-
 drivers/gpu/drm/drm_prime.c |  126 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm.h           |   10 +++-
 include/drm/drmP.h          |   35 ++++++++++++
 6 files changed, 176 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_prime.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0cde1b8..202f650 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
 		drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
 		drm_crtc.o drm_modes.o drm_edid.o \
 		drm_info.o drm_debugfs.o drm_encoder_slave.o \
-		drm_trace_points.o drm_global.o drm_usb.o
+		drm_trace_points.o drm_global.o drm_usb.o drm_prime.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index ebf7d3f..786b134 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -135,6 +135,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
 
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index f8625e2..e19a958 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -145,6 +145,7 @@ int drm_gem_object_init(struct drm_device *dev,
 	kref_init(&obj->refcount);
 	atomic_set(&obj->handle_count, 0);
 	obj->size = size;
+	obj->prime_fd = -1;
 
 	return 0;
 }
@@ -166,7 +167,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
 	kref_init(&obj->refcount);
 	atomic_set(&obj->handle_count, 0);
 	obj->size = size;
-
+	obj->prime_fd = -1;
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_private_object_init);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
new file mode 100644
index 0000000..11f142f
--- /dev/null
+++ b/drivers/gpu/drm/drm_prime.c
@@ -0,0 +1,126 @@
+#include <linux/export.h>
+#include <linux/dma-buf.h>
+#include "drmP.h"
+
+struct drm_prime_member {
+	struct list_head entry;
+	struct dma_buf *dma_buf;
+	uint32_t handle;
+};
+
+int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+				 struct drm_file *file_priv)
+{
+	struct drm_prime_handle *args = data;
+
+	if (!drm_core_check_feature(dev, DRIVER_PRIME))
+		return -EINVAL;
+
+	return dev->driver->prime_handle_to_fd(dev, file_priv, args->handle, &args->fd);
+}
+
+int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
+				 struct drm_file *file_priv)
+{
+	struct drm_prime_handle *args = data;
+
+	if (!drm_core_check_feature(dev, DRIVER_PRIME))
+		return -EINVAL;
+
+	return dev->driver->prime_fd_to_handle(dev, file_priv, args->fd, &args->handle);
+}
+
+struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages)
+{
+	struct sg_table *sg = NULL;
+	struct scatterlist *iter;
+	int i;
+	int ret;
+
+	sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
+	if (!sg)
+		goto out;
+
+	ret = sg_alloc_table(sg, nr_pages, GFP_KERNEL);
+	if (ret)
+		goto out;
+
+	for_each_sg(sg->sgl, iter, nr_pages, i)
+		sg_set_page(iter, pages[i], PAGE_SIZE, 0);
+
+	return sg;
+out:
+	kfree(sg);
+	return NULL;
+}
+EXPORT_SYMBOL(drm_prime_pages_to_sg);
+
+/* helper function to cleanup a GEM/prime object */
+void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg)
+{
+	struct dma_buf_attachment *attach;
+
+	attach = obj->import_attach;
+	if (sg)
+		dma_buf_unmap_attachment(attach, sg);
+	dma_buf_detach(attach->dmabuf, attach);
+}
+EXPORT_SYMBOL(drm_prime_gem_destroy);
+
+void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
+{
+	INIT_LIST_HEAD(&prime_fpriv->head);
+}
+EXPORT_SYMBOL(drm_prime_init_file_private);
+
+void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
+{
+	struct drm_prime_member *member, *safe;
+	list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
+		list_del(&member->entry);
+		kfree(member);
+	}	
+}
+EXPORT_SYMBOL(drm_prime_destroy_file_private);
+
+int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
+{
+	struct drm_prime_member *member;
+
+	member = kmalloc(sizeof(*member), GFP_KERNEL);
+	if (!member)
+		return -ENOMEM;
+
+	member->dma_buf = dma_buf;
+	member->handle = handle;
+	list_add(&member->entry, &prime_fpriv->head);
+	return 0;
+}
+EXPORT_SYMBOL(drm_prime_insert_fd_handle_mapping);
+
+int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
+{
+	struct drm_prime_member *member;
+
+	list_for_each_entry(member, &prime_fpriv->head, entry) {
+		if (member->dma_buf == dma_buf) {
+			*handle = member->handle;
+			return 0;
+		}
+	}
+	return -ENOENT;
+}
+EXPORT_SYMBOL(drm_prime_lookup_fd_handle_mapping);
+
+void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
+{
+	struct drm_prime_member *member, *safe;
+
+	list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
+		if (member->dma_buf == dma_buf) {
+			list_del(&member->entry);
+			kfree(member);
+		}
+	}
+}
+EXPORT_SYMBOL(drm_prime_remove_fd_handle_mapping);
diff --git a/include/drm/drm.h b/include/drm/drm.h
index 49d94ed..3dcae79 100644
--- a/include/drm/drm.h
+++ b/include/drm/drm.h
@@ -617,6 +617,13 @@ struct drm_get_cap {
 	__u64 value;
 };
 
+struct drm_prime_handle {
+	__u32 handle;
+
+	/* returned fd for prime */
+	__s32 fd;
+};
+
 #include "drm_mode.h"
 
 #define DRM_IOCTL_BASE			'd'
@@ -673,7 +680,8 @@ struct drm_get_cap {
 #define DRM_IOCTL_UNLOCK		DRM_IOW( 0x2b, struct drm_lock)
 #define DRM_IOCTL_FINISH		DRM_IOW( 0x2c, struct drm_lock)
 
-#define DRM_IOCTL_GEM_PRIME_OPEN        DRM_IOWR(0x2e, struct drm_gem_open)
+#define DRM_IOCTL_PRIME_HANDLE_TO_FD    DRM_IOWR(0x2d, struct drm_prime_handle)
+#define DRM_IOCTL_PRIME_FD_TO_HANDLE    DRM_IOWR(0x2e, struct drm_prime_handle)
 
 #define DRM_IOCTL_AGP_ACQUIRE		DRM_IO(  0x30)
 #define DRM_IOCTL_AGP_RELEASE		DRM_IO(  0x31)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 92f0981..9558111 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -150,6 +150,7 @@ int drm_err(const char *func, const char *format, ...);
 #define DRIVER_IRQ_VBL2    0x800
 #define DRIVER_GEM         0x1000
 #define DRIVER_MODESET     0x2000
+#define DRIVER_PRIME       0x4000
 
 #define DRIVER_BUS_PCI 0x1
 #define DRIVER_BUS_PLATFORM 0x2
@@ -652,6 +653,19 @@ struct drm_gem_object {
 	uint32_t pending_write_domain;
 
 	void *driver_private;
+
+	/* prime fd exporting this object, -1 for no fd */
+	int prime_fd;
+	/* dma buf exported from this GEM object */
+	struct dma_buf *export_dma_buf;
+
+	/* dma buf attachment backing this object */
+	struct dma_buf_attachment *import_attach;
+};
+
+/* initial implementaton using a linked list - todo hashtab */
+struct drm_prime_file_private {
+	struct list_head head;
 };
 
 #include "drm_crtc.h"
@@ -890,6 +904,13 @@ struct drm_driver {
 	int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
 	void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
 
+	/* prime */
+	int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv,
+				  uint32_t handle, int *prime_fd);
+
+	int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv,
+				  int prime_fd, uint32_t *handle);
+
 	/* vga arb irq handler */
 	void (*vgaarb_irq)(struct drm_device *dev, bool state);
 
@@ -1502,6 +1523,20 @@ extern int drm_vblank_info(struct seq_file *m, void *data);
 extern int drm_clients_info(struct seq_file *m, void* data);
 extern int drm_gem_name_info(struct seq_file *m, void *data);
 
+extern int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv);
+extern int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
+					struct drm_file *file_priv);
+
+extern struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages);
+extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg);
+
+void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
+void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
+int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
+int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
+void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
+
 #if DRM_DEBUG_CODE
 extern int drm_vma_info(struct seq_file *m, void *data);
 #endif
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] DRM_DEBUG_PRIME
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
  2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-22 19:29 ` [PATCH 2/7] drm: DRM_DEBUG_PRIME Ben Widawsky
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Ben Widawsky

---
 include/drm/drmP.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9558111..5ed9b41 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -91,6 +91,7 @@ struct drm_device;
 #define DRM_UT_CORE 		0x01
 #define DRM_UT_DRIVER		0x02
 #define DRM_UT_KMS		0x04
+#define DRM_UT_PRIME		0x08
 /*
  * Three debug levels are defined.
  * drm_core, drm_driver, drm_kms
@@ -216,6 +217,11 @@ int drm_err(const char *func, const char *format, ...);
 		drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, 		\
 					 __func__, fmt, ##args);	\
 	} while (0)
+#define DRM_DEBUG_PRIME(fmt, args...)					\
+	do {								\
+		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
+					__func__, fmt, ##args);		\
+	} while (0)
 #define DRM_LOG(fmt, args...)						\
 	do {								\
 		drm_ut_debug_printk(DRM_UT_CORE, NULL,			\
@@ -239,6 +245,7 @@ int drm_err(const char *func, const char *format, ...);
 #else
 #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
 #define DRM_DEBUG_KMS(fmt, args...)	do { } while (0)
+#define DRM_DEBUG_PRIME(fmt, args...)	do { } while (0)
 #define DRM_DEBUG(fmt, arg...)		 do { } while (0)
 #define DRM_LOG(fmt, arg...)		do { } while (0)
 #define DRM_LOG_KMS(fmt, args...) do { } while (0)
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] drm: DRM_DEBUG_PRIME
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
  2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
  2012-02-22 19:29 ` [PATCH 2/7] DRM_DEBUG_PRIME Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-22 19:29 ` [PATCH 3/7] drm/vgem: virtual GEM provider Ben Widawsky
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Ben Widawsky

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 include/drm/drmP.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9558111..5ed9b41 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -91,6 +91,7 @@ struct drm_device;
 #define DRM_UT_CORE 		0x01
 #define DRM_UT_DRIVER		0x02
 #define DRM_UT_KMS		0x04
+#define DRM_UT_PRIME		0x08
 /*
  * Three debug levels are defined.
  * drm_core, drm_driver, drm_kms
@@ -216,6 +217,11 @@ int drm_err(const char *func, const char *format, ...);
 		drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, 		\
 					 __func__, fmt, ##args);	\
 	} while (0)
+#define DRM_DEBUG_PRIME(fmt, args...)					\
+	do {								\
+		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
+					__func__, fmt, ##args);		\
+	} while (0)
 #define DRM_LOG(fmt, args...)						\
 	do {								\
 		drm_ut_debug_printk(DRM_UT_CORE, NULL,			\
@@ -239,6 +245,7 @@ int drm_err(const char *func, const char *format, ...);
 #else
 #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
 #define DRM_DEBUG_KMS(fmt, args...)	do { } while (0)
+#define DRM_DEBUG_PRIME(fmt, args...)	do { } while (0)
 #define DRM_DEBUG(fmt, arg...)		 do { } while (0)
 #define DRM_LOG(fmt, arg...)		do { } while (0)
 #define DRM_LOG_KMS(fmt, args...) do { } while (0)
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/7] drm/vgem: virtual GEM provider
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
                   ` (2 preceding siblings ...)
  2012-02-22 19:29 ` [PATCH 2/7] drm: DRM_DEBUG_PRIME Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-22 19:29 ` [PATCH 4/7] drm: per device prime dma buf hash Ben Widawsky
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Ben Widawsky

From: Adam Jackson <ajax@redhat.com>

In addition's to Adam's requirements below, I would like to use VGEM to
help support dma-buf development. VGEM is without quirks and makes both
development and debug of other importers and exporters substantially
easier.

v2:
Use fault based mmap instead of do_mmap

v3:
Drop the getparam
use the dumb bo interface

[From Adam Jackson, RFCv1]
This is about as minimal of a virtual GEM service as possible.  My plan
is to use this with non-native-3D hardware for buffer sharing between X
and DRI.

The current drisw winsys assumes an unmodified X server, which means
it's hopelessly inefficient for both the push direction of SwapBuffers/
DrawPixels and the pull direction of GLX_EXT_texture_from_pixmap.  I'm
still working through the details of what the xserver support will look
like, but in broad strokes it's "use vgem for CreatePixmap and
optionally the shadowfb".

Obviously alpha quality, mostly looking for feedback on the approach or
any glaring bugs.  Eventually I'd like to see solutions for sharing gem
objects between drm devices and/or the dma_buf API, but that's a ways
down the road.

[RFCv1]
Signed-off-by: Adam Jackson <ajax@redhat.com>
[RFCv2]
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/Kconfig         |    8 +
 drivers/gpu/drm/Makefile        |    1 +
 drivers/gpu/drm/vgem/Makefile   |    4 +
 drivers/gpu/drm/vgem/vgem_drv.c |  382 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/vgem/vgem_drv.h |   54 ++++++
 5 files changed, 449 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpu/drm/vgem/Makefile
 create mode 100644 drivers/gpu/drm/vgem/vgem_drv.c
 create mode 100644 drivers/gpu/drm/vgem/vgem_drv.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2418429..566c468 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -159,6 +159,14 @@ config DRM_SAVAGE
 	  Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister
 	  chipset. If M is selected the module will be called savage.
 
+config DRM_VGEM
+	tristate "Virtual GEM provider"
+	depends on DRM
+	help
+	  Choose this option to get a virtual graphics memory manager,
+	  as used by Mesa's software renderer for enhanced performance.
+	  If M is selected the module will be called vgem.
+
 source "drivers/gpu/drm/exynos/Kconfig"
 
 source "drivers/gpu/drm/vmwgfx/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 202f650..d025e54 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_DRM_SIS)   += sis/
 obj-$(CONFIG_DRM_SAVAGE)+= savage/
 obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
 obj-$(CONFIG_DRM_VIA)	+=via/
+obj-$(CONFIG_DRM_VGEM)	+= vgem/
 obj-$(CONFIG_DRM_NOUVEAU) +=nouveau/
 obj-$(CONFIG_DRM_EXYNOS) +=exynos/
 obj-$(CONFIG_DRM_GMA500) += gma500/
diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
new file mode 100644
index 0000000..3f4c7b8
--- /dev/null
+++ b/drivers/gpu/drm/vgem/Makefile
@@ -0,0 +1,4 @@
+ccflags-y := -Iinclude/drm
+vgem-y := vgem_drv.o
+
+obj-$(CONFIG_DRM_VGEM)	+= vgem.o
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
new file mode 100644
index 0000000..cd6ab42
--- /dev/null
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -0,0 +1,382 @@
+/*
+ * Copyright 2011 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software")
+ * to deal in the software without restriction, including without limitation
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, and/or sell copies of the Software, and to permit persons to whom
+ * them Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTIBILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES, OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT, OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *	Adam Jackson <ajax@redhat.com>
+ *	Ben Widawsky <ben@bwidawsk.net>
+ */
+
+/**
+ * This is vgem, a (non-hardware-backed) GEM service.  This is used by Mesa's
+ * software renderer and the X server for efficient buffer sharing.
+ */
+
+#include "drmP.h"
+#include "drm.h"
+#include <linux/module.h>
+#include <linux/ramfs.h>
+#include <linux/shmem_fs.h>
+#include "vgem_drv.h"
+
+#define DRIVER_NAME	"vgem"
+#define DRIVER_DESC	"Virtual GEM provider"
+#define DRIVER_DATE	"20120112"
+#define DRIVER_MAJOR	1
+#define DRIVER_MINOR	0
+
+static int vgem_load(struct drm_device *dev, unsigned long flags)
+{
+	struct drm_vgem_private *dev_priv;
+
+	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
+	if (!dev_priv)
+		return -ENOMEM;
+
+	dev->dev_private = dev_priv;
+	dev_priv->dev = dev;
+
+	return 0;
+}
+
+static int vgem_unload(struct drm_device *dev)
+{
+	return 0;
+}
+
+static int vgem_open(struct drm_device *dev, struct drm_file *file)
+{
+	struct drm_vgem_file_private *file_priv;
+
+	file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
+	if (!file_priv)
+		return -ENOMEM;
+
+	file->driver_priv = file_priv;
+
+	return 0;
+}
+
+static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
+{
+	struct drm_vgem_file_private *file_priv = file->driver_priv;
+	kfree(file_priv);
+}
+
+static void vgem_lastclose(struct drm_device *dev)
+{
+}
+
+static int vgem_gem_init_object(struct drm_gem_object *obj)
+{
+	return 0;
+}
+
+void vgem_gem_put_pages(struct drm_vgem_gem_object *obj)
+{
+	int num_pages = obj->base.size / PAGE_SIZE;
+	int i;
+
+	for (i = 0; i < num_pages; i++) {
+		if (obj->pages[i] == NULL)
+			break;
+		page_cache_release(obj->pages[i]);
+	}
+
+	drm_free_large(obj->pages);
+	obj->pages = NULL;
+}
+
+static void vgem_gem_free_object(struct drm_gem_object *obj)
+{
+	struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
+
+	if (obj)
+		drm_gem_free_mmap_offset(obj);
+
+	drm_gem_object_release(obj);
+
+	if (vgem_obj->pages)
+		vgem_gem_put_pages(vgem_obj);
+
+	vgem_obj->pages = NULL;
+
+	kfree(vgem_obj);
+}
+
+int vgem_gem_get_pages(struct drm_vgem_gem_object *obj)
+{
+	struct address_space *mapping;
+	gfp_t gfpmask = __GFP_NORETRY | __GFP_NOWARN;
+	int num_pages, i, ret = 0;
+
+	num_pages = obj->base.size / PAGE_SIZE;
+
+	if (!obj->pages) {
+		obj->pages = drm_malloc_ab(num_pages, sizeof(struct page *));
+		if (obj->pages == NULL)
+			return -ENOMEM;
+	} else
+		return 0;
+
+	mapping = obj->base.filp->f_path.dentry->d_inode->i_mapping;
+	gfpmask |= mapping_gfp_mask(mapping);
+
+	for (i = 0; i < num_pages; i++) {
+		struct page *page;
+		page = shmem_read_mapping_page_gfp(mapping, i, gfpmask);
+		if (IS_ERR(page)) {
+			ret = PTR_ERR(page);
+			goto err_out;
+		}
+		obj->pages[i] = page;
+	}
+
+	return ret;
+
+err_out:
+	vgem_gem_put_pages(obj);
+	return ret;
+}
+
+static int vgem_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	struct drm_vgem_gem_object *obj = to_vgem_bo(vma->vm_private_data);
+	loff_t num_pages;
+	pgoff_t page_offset;
+	int ret;
+
+	/* We don't use vmf->pgoff since that has the fake offset */
+	page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
+		PAGE_SHIFT;
+
+	num_pages = obj->base.size / PAGE_SIZE;
+
+	if (WARN_ON(page_offset > num_pages))
+		return VM_FAULT_SIGBUS;
+
+	ret = vgem_gem_get_pages(obj);
+	if (ret)
+		return ret;
+
+	ret = vm_insert_page(vma, (unsigned long)vmf->virtual_address,
+			     obj->pages[page_offset]);
+
+	/* Pretty dumb handler for now */
+	switch (ret) {
+	case 0:
+	case -ERESTARTSYS:
+	case -EINTR:
+		return VM_FAULT_NOPAGE;
+	default:
+		return VM_FAULT_SIGBUS;
+	}
+}
+
+static struct vm_operations_struct vgem_gem_vm_ops = {
+	.fault = vgem_gem_fault,
+	.open = drm_gem_vm_open,
+	.close = drm_gem_vm_close,
+};
+
+/* ioctls */
+
+static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
+					      struct drm_file *file,
+					      unsigned int *handle,
+					      unsigned long size)
+{
+	struct drm_vgem_gem_object *obj;
+	struct drm_gem_object *gem_object;
+	int err;
+
+	size = roundup(size, PAGE_SIZE);
+
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return ERR_PTR(-ENOMEM);
+
+	gem_object = &obj->base;
+
+	if ((err = drm_gem_object_init(dev, gem_object, size)))
+		goto out;
+
+	if ((err = drm_gem_create_mmap_offset(gem_object)))
+		goto mmap_out;
+
+	if ((err = drm_gem_handle_create(file, gem_object, handle)))
+		goto handle_out;
+
+	drm_gem_object_unreference_unlocked(gem_object);
+
+	return gem_object;
+
+handle_out:
+	drm_gem_free_mmap_offset(gem_object);
+
+mmap_out:
+	drm_gem_object_release(gem_object);
+
+out:
+	kfree(gem_object);
+
+	return ERR_PTR(err);
+}
+
+static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
+				struct drm_mode_create_dumb *args)
+{
+	struct drm_gem_object *gem_object;
+	uint64_t size;
+
+	size = args->height * args->width * args->bpp;
+
+	gem_object = vgem_gem_create(dev, file, &args->handle, size);
+
+	if (IS_ERR(gem_object)) {
+		DRM_DEBUG_DRIVER("object creation failed\n");
+		return PTR_ERR(gem_object);
+	}
+
+	args->size = size;
+	args->pitch = args->width;
+
+	DRM_DEBUG_DRIVER("Created object of size %lld\n", size);
+
+	return 0;
+}
+
+int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev,
+		      uint32_t handle, uint64_t *offset)
+{
+	struct drm_gem_object *obj;
+
+	obj = drm_gem_object_lookup(dev, file, handle);
+	if (!obj)
+		return -ENOENT;
+
+	obj->filp->private_data = obj;
+
+	BUG_ON(!obj->map_list.map);
+
+	*offset = (uint64_t)obj->map_list.hash.key << PAGE_SHIFT;
+
+	drm_gem_object_unreference_unlocked(obj);
+
+	return 0;
+}
+
+static struct drm_ioctl_desc vgem_ioctls[] = {
+};
+
+static const struct file_operations vgem_driver_fops = {
+	.owner		= THIS_MODULE,
+	.open		= drm_open,
+	.mmap		= drm_gem_mmap,
+	.poll		= drm_poll,
+	.read		= drm_read,
+	.unlocked_ioctl = drm_ioctl,
+	.release	= drm_release,
+};
+
+static struct drm_driver vgem_driver = {
+	.driver_features	= DRIVER_BUS_PLATFORM | DRIVER_GEM,
+	.load			= vgem_load,
+	.unload			= vgem_unload,
+	.open			= vgem_open,
+	.preclose		= vgem_preclose,
+	.lastclose		= vgem_lastclose,
+	.gem_init_object	= vgem_gem_init_object,
+	.gem_free_object	= vgem_gem_free_object,
+	.gem_vm_ops		= &vgem_gem_vm_ops,
+	.ioctls			= vgem_ioctls,
+	.fops			= &vgem_driver_fops,
+
+	.dumb_create		= vgem_gem_dumb_create,
+	.dumb_map_offset	= vgem_gem_dumb_map,
+
+	.name	= DRIVER_NAME,
+	.desc	= DRIVER_DESC,
+	.date	= DRIVER_DATE,
+	.major	= DRIVER_MAJOR,
+	.minor	= DRIVER_MINOR,
+};
+
+static int vgem_platform_probe(struct platform_device *pdev)
+{
+	vgem_driver.num_ioctls = DRM_ARRAY_SIZE(vgem_ioctls);
+
+	return drm_platform_init(&vgem_driver, pdev);
+}
+
+static int vgem_platform_remove(struct platform_device *pdev)
+{
+	drm_platform_exit(&vgem_driver, pdev);
+
+	return 0;
+}
+
+static struct platform_driver vgem_platform_driver = {
+	.probe		= vgem_platform_probe,
+	.remove		= __devexit_p(vgem_platform_remove),
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= DRIVER_NAME,
+	},
+};
+
+static struct platform_device *vgem_device;
+
+static int __init vgem_init(void)
+{
+	int ret;
+
+	if ((ret = platform_driver_register(&vgem_platform_driver)))
+		return ret;
+
+	vgem_device = platform_device_alloc("vgem", -1);
+	if (!vgem_device) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = platform_device_add(vgem_device);
+	if (!ret)
+		return 0;
+
+out:
+	platform_device_put(vgem_device);
+	platform_driver_unregister(&vgem_platform_driver);
+
+	return ret;
+}
+
+static void __exit vgem_exit(void)
+{
+	platform_device_unregister(vgem_device);
+	platform_driver_unregister(&vgem_platform_driver);
+}
+
+module_init(vgem_init);
+module_exit(vgem_exit);
+
+MODULE_AUTHOR("Red Hat, Inc.");
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
new file mode 100644
index 0000000..ca63fa0b
--- /dev/null
+++ b/drivers/gpu/drm/vgem/vgem_drv.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Ben Widawsky <ben@bwidawsk.net>
+ *
+ */
+
+#ifndef _VGEM_DRV_H_
+#define _VGEM_DRV_H_
+
+#include "drmP.h"
+#include <linux/types.h>
+
+struct drm_vgem_private {
+	struct drm_device *dev;
+};
+
+#define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
+struct drm_vgem_gem_object {
+	struct drm_gem_object base;
+	struct page **pages;
+};
+
+struct drm_vgem_file_private {
+};
+
+/* vgem_drv.c */
+extern void vgem_gem_put_pages(struct drm_vgem_gem_object *obj);
+extern int vgem_gem_get_pages(struct drm_vgem_gem_object *obj);
+
+/* vgem_dma_buf.c */
+
+
+#endif
-- 
1.7.9.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] drm: per device prime dma buf hash
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
                   ` (3 preceding siblings ...)
  2012-02-22 19:29 ` [PATCH 3/7] drm/vgem: virtual GEM provider Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Daniel Vetter, Ben Widawsky, Dave Airlie

Dave's original prime patches had a per file lookup (currently a list,
but should probably be a hash as mentioned in the comment). This is
handy for when an importer tries to import the same BO multiple times as
it will prevent object duplication in that case. The problem is it does
not account for multiple importers. If an exporter passes a dma-buf fd
to multiple importers, a new object would be created for each one, which
is bad.

To handle multiple importers, we create a per device dma buf hash table
where the key is the dma_buf pointer, and the value is the gem object.
There is also a lock introduced to help with the various ref count
dances (and subsequent hash list traversals).

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/drm_gem.c   |    3 ++
 drivers/gpu/drm/drm_prime.c |   46 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_stub.c  |    8 +++++++
 include/drm/drmP.h          |   13 ++++++++++++
 4 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index e19a958..c9bf515 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -146,6 +146,7 @@ int drm_gem_object_init(struct drm_device *dev,
 	atomic_set(&obj->handle_count, 0);
 	obj->size = size;
 	obj->prime_fd = -1;
+	INIT_HLIST_NODE(&obj->brown);
 
 	return 0;
 }
@@ -168,6 +169,8 @@ int drm_gem_private_object_init(struct drm_device *dev,
 	atomic_set(&obj->handle_count, 0);
 	obj->size = size;
 	obj->prime_fd = -1;
+	INIT_HLIST_NODE(&obj->brown);
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_private_object_init);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 11f142f..b148d5c 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -1,5 +1,6 @@
 #include <linux/export.h>
 #include <linux/dma-buf.h>
+#include <linux/hash.h>
 #include "drmP.h"
 
 struct drm_prime_member {
@@ -124,3 +125,48 @@ void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpr
 	}
 }
 EXPORT_SYMBOL(drm_prime_remove_fd_handle_mapping);
+
+static struct drm_gem_object *get_obj_from_dma_buf(struct drm_device *dev,
+						   struct dma_buf *buf)
+{
+	struct drm_gem_object *obj;
+	struct hlist_head *bucket =
+		&dev->dma_buf_hash[hash_ptr(buf, DRM_DMA_BUF_HASH_BITS)];
+	struct hlist_node *tmp;
+
+	hlist_for_each_entry(obj, tmp, bucket, brown) {
+		if (obj->export_dma_buf == buf)
+			return obj;
+	}
+
+	return NULL;
+}
+
+int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj)
+{
+	struct drm_gem_object *tmp;
+	unsigned long hash;
+
+	if ((tmp = get_obj_from_dma_buf(dev, obj->export_dma_buf))) {
+		DRM_DEBUG_PRIME("%p found DRM hash\n", obj->export_dma_buf);
+		if (WARN_ON(tmp != obj))
+			return -1;
+		return 0;
+	}
+
+	hash = hash_ptr(obj->export_dma_buf, DRM_DMA_BUF_HASH_BITS);
+	hlist_add_head(&obj->brown, &dev->dma_buf_hash[hash]);
+
+	DRM_DEBUG_PRIME("%p added to DRM hash\n", obj->export_dma_buf);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_prime_add_dma_buf);
+
+int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf,
+			 struct drm_gem_object **obj)
+{
+	*obj = get_obj_from_dma_buf(dev, buf);
+	return 0;
+}
+EXPORT_SYMBOL(drm_prime_lookup_obj);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 6d7b083..78a614c 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -313,6 +313,14 @@ int drm_fill_in_dev(struct drm_device *dev,
 		}
 	}
 
+	if (driver->driver_features & DRIVER_PRIME) {
+		int i;
+		for (i = 0; i < DRM_DMA_BUF_HASH_ENTRIES; i++)
+			INIT_HLIST_HEAD(&dev->dma_buf_hash[i]);
+
+		mutex_init(&dev->prime_mutex);
+	}
+
 	return 0;
 
       error_out_unreg:
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 5ed9b41..84dde0d 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -668,6 +668,8 @@ struct drm_gem_object {
 
 	/* dma buf attachment backing this object */
 	struct dma_buf_attachment *import_attach;
+
+	struct hlist_node brown;
 };
 
 /* initial implementaton using a linked list - todo hashtab */
@@ -730,6 +732,9 @@ struct drm_bus {
 
 };
 
+#define DRM_DMA_BUF_HASH_BITS 5
+#define DRM_DMA_BUF_HASH_ENTRIES (1<<DRM_DMA_BUF_HASH_BITS)
+
 /**
  * DRM driver structure. This structure represent the common code for
  * a family of cards. There will one drm_device for each card present
@@ -1198,6 +1203,9 @@ struct drm_device {
 	struct idr object_name_idr;
 	/*@} */
 	int switch_power_state;
+
+	struct mutex prime_mutex;
+	struct hlist_head dma_buf_hash[DRM_DMA_BUF_HASH_ENTRIES];
 };
 
 #define DRM_SWITCH_POWER_ON 0
@@ -1538,12 +1546,17 @@ extern int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 extern struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages);
 extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg);
 
+
 void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
 void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
 int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
 int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
 void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
 
+int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj);
+int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf,
+			 struct drm_gem_object **obj);
+
 #if DRM_DEBUG_CODE
 extern int drm_vma_info(struct seq_file *m, void *data);
 #endif
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] drm/vgem: prime export support
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
                   ` (4 preceding siblings ...)
  2012-02-22 19:29 ` [PATCH 4/7] drm: per device prime dma buf hash Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-23 19:00   ` [Linaro-mm-sig] " Chris Wilson
                     ` (2 more replies)
  2012-02-22 19:29 ` [PATCH 6/7] drm/vgem: import support Ben Widawsky
  2012-02-22 19:29 ` [PATCH 7/7] drm: actually enable PRIME Ben Widawsky
  7 siblings, 3 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Daniel Vetter, Ben Widawsky, Dave Airlie

dma-buf export implementation. Heavily influenced by Dave Airlie's proof
of concept work.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/vgem/Makefile       |    2 +-
 drivers/gpu/drm/vgem/vgem_dma_buf.c |  128 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/vgem/vgem_drv.c     |    6 ++
 drivers/gpu/drm/vgem/vgem_drv.h     |    7 ++
 4 files changed, 142 insertions(+), 1 deletions(-)
 create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c

diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
index 3f4c7b8..1055cb7 100644
--- a/drivers/gpu/drm/vgem/Makefile
+++ b/drivers/gpu/drm/vgem/Makefile
@@ -1,4 +1,4 @@
 ccflags-y := -Iinclude/drm
-vgem-y := vgem_drv.o
+vgem-y := vgem_drv.o vgem_dma_buf.o
 
 obj-$(CONFIG_DRM_VGEM)	+= vgem.o
diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c
new file mode 100644
index 0000000..eca9445
--- /dev/null
+++ b/drivers/gpu/drm/vgem/vgem_dma_buf.c
@@ -0,0 +1,128 @@
+/*
+ * Copyright © 2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Ben Widawsky <ben@bwidawsk.net>
+ *
+ */
+
+#include <linux/dma-buf.h>
+#include "vgem_drv.h"
+
+#define VGEM_FD_PERMS 0600
+
+struct sg_table *vgem_gem_map_dma_buf(struct dma_buf_attachment *attachment,
+                                      enum dma_data_direction dir)
+{
+	struct drm_vgem_gem_object *obj = attachment->dmabuf->priv;
+	struct sg_table *sg;
+	int ret;
+
+	ret = vgem_gem_get_pages(obj);
+	if (ret) {
+		vgem_gem_put_pages(obj);
+		return NULL;
+	}
+
+	BUG_ON(obj->pages == NULL);
+
+	sg = drm_prime_pages_to_sg(obj->pages, obj->base.size / PAGE_SIZE);
+	return sg;
+}
+
+void vgem_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
+			    struct sg_table *sg)
+{
+	sg_free_table(sg);
+	kfree(sg);
+}
+
+void vgem_gem_release(struct dma_buf *buf)
+{
+	struct drm_vgem_gem_object *obj = buf->priv;
+
+	BUG_ON(buf != obj->base.export_dma_buf);
+
+	obj->base.prime_fd = -1;
+	obj->base.export_dma_buf = NULL;
+	drm_gem_object_unreference_unlocked(&obj->base);
+}
+
+struct dma_buf_ops vgem_dmabuf_ops = {
+	.map_dma_buf = vgem_gem_map_dma_buf,
+	.unmap_dma_buf = vgem_gem_unmap_dma_buf,
+	.release = vgem_gem_release
+};
+
+int vgem_prime_to_fd(struct drm_device *dev, struct drm_file *file,
+		     uint32_t handle, int *prime_fd)
+{
+	struct drm_vgem_file_private *file_priv = file->driver_priv;
+	struct drm_vgem_gem_object *obj;
+	int ret;
+
+	DRM_DEBUG_PRIME("Request fd for handle %d\n", handle);
+
+	obj = to_vgem_bo(drm_gem_object_lookup(dev, file, handle));
+	if (!obj)
+		return -EBADF;
+
+	/* This means a user has already called get_fd on this */
+	if (obj->base.prime_fd != -1) {
+		DRM_DEBUG_PRIME("User requested a previously exported buffer "
+				"%d %d\n", handle, obj->base.prime_fd);
+		drm_gem_object_unreference(&obj->base);
+		goto out_fd;
+	}
+
+	/* Make a dma buf out of our vgem object */
+	obj->base.export_dma_buf = dma_buf_export(obj, &vgem_dmabuf_ops,
+						  obj->base.size,
+						  VGEM_FD_PERMS);
+	if (IS_ERR(obj->base.export_dma_buf)) {
+		DRM_DEBUG_PRIME("export fail\n");
+		return PTR_ERR(obj->base.export_dma_buf);
+	} else
+		obj->base.prime_fd = dma_buf_fd(obj->base.export_dma_buf);
+
+	mutex_lock(&dev->prime_mutex);
+	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
+						 obj->base.export_dma_buf,
+						 handle);
+	WARN_ON(ret);
+	ret = drm_prime_add_dma_buf(dev, &obj->base);
+	mutex_unlock(&dev->prime_mutex);
+	if (ret)
+		return ret;
+
+out_fd:
+	*prime_fd = obj->base.prime_fd;
+
+	return 0;
+}
+
+int vgem_prime_to_handle(struct drm_device *dev,
+			 struct drm_file *file, int prime_fd,
+			 uint32_t *handle)
+{
+	return 0;
+}
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index cd6ab42..9cd1ed4 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -71,12 +71,15 @@ static int vgem_open(struct drm_device *dev, struct drm_file *file)
 
 	file->driver_priv = file_priv;
 
+	drm_prime_init_file_private(&file_priv->prime);
+
 	return 0;
 }
 
 static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_vgem_file_private *file_priv = file->driver_priv;
+	drm_prime_destroy_file_private(&file_priv->prime);
 	kfree(file_priv);
 }
 
@@ -312,6 +315,9 @@ static struct drm_driver vgem_driver = {
 	.dumb_create		= vgem_gem_dumb_create,
 	.dumb_map_offset	= vgem_gem_dumb_map,
 
+	.prime_handle_to_fd	= vgem_prime_to_fd,
+	.prime_fd_to_handle	= vgem_prime_to_handle,
+
 	.name	= DRIVER_NAME,
 	.desc	= DRIVER_DESC,
 	.date	= DRIVER_DATE,
diff --git a/drivers/gpu/drm/vgem/vgem_drv.h b/drivers/gpu/drm/vgem/vgem_drv.h
index ca63fa0b..56d1c0f 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.h
+++ b/drivers/gpu/drm/vgem/vgem_drv.h
@@ -42,6 +42,7 @@ struct drm_vgem_gem_object {
 };
 
 struct drm_vgem_file_private {
+	struct drm_prime_file_private prime;
 };
 
 /* vgem_drv.c */
@@ -49,6 +50,12 @@ extern void vgem_gem_put_pages(struct drm_vgem_gem_object *obj);
 extern int vgem_gem_get_pages(struct drm_vgem_gem_object *obj);
 
 /* vgem_dma_buf.c */
+extern int vgem_prime_to_fd(struct drm_device *dev,
+			    struct drm_file *file_priv,
+			    uint32_t handle, int *prime_fd);
 
+extern int vgem_prime_to_handle(struct drm_device *dev,
+				struct drm_file *file_priv,
+				int prime_fd, uint32_t *handle);
 
 #endif
-- 
1.7.9.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] drm/vgem: import support
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
                   ` (5 preceding siblings ...)
  2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  2012-02-23 19:10   ` [Linaro-mm-sig] " Chris Wilson
  2012-02-23 23:00   ` Chris Wilson
  2012-02-22 19:29 ` [PATCH 7/7] drm: actually enable PRIME Ben Widawsky
  7 siblings, 2 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Daniel Vetter, Ben Widawsky, Dave Airlie

dma-buf import support. The function definitely needs some cleanup.

When reading through this code, there are 3 cases to consider:
1. vgem exporter, vgem importer, same fd
2. vgem exporter, vgem importer, different fd
3. X expoter, vgem importer - not yet tested

See the comments in the code for detailed explanation.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/vgem/vgem_dma_buf.c |  120 +++++++++++++++++++++++++++++++++++
 1 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c
index eca9445..92c1823 100644
--- a/drivers/gpu/drm/vgem/vgem_dma_buf.c
+++ b/drivers/gpu/drm/vgem/vgem_dma_buf.c
@@ -120,9 +120,129 @@ out_fd:
 	return 0;
 }
 
+/*
+ * Convert a dma-buf fd to a drm object handle, creating new object/handle as
+ * needed.
+ *
+ * There are 2 "interesting" cases we have to consider. The other, less interesting
+ * case is when importer == exporter, and drm_files are the same.
+ * vgem exporter
+ *   The original exporter may or may not still hold a reference to the
+ *   object by the time we reach here. Once we get a dma_buf reference though
+ *   we know the original object cannot go away. Next we grab the prime mutex
+ *   to prevent our lists from being screwed up from under us. We should next
+ *   find the object in our global dma_buf hash. To make everything cool
+ *   though we need to
+ *     create a handle for the importer
+ *     add the handle to the per file list
+ *     drop the dma_buf reference
+ *       the object can't go away due to us owning a file handle for it.
+ *   In the end there should be 2 handle references, and 1 dma-buf reference.
+ *
+ * other exporter
+ *   This case is very similar to the previous one. The primary difference is we
+ *   do not want to drop the dma_buf reference since we know nothing about the
+ *   reference counting from the exporter. So instead, we hold the dma_buf
+ *   reference, but can drop the object reference. In the end of this case there
+ *   should be 1 handle reference, and 1 dma-buf reference.
+ */
 int vgem_prime_to_handle(struct drm_device *dev,
 			 struct drm_file *file, int prime_fd,
 			 uint32_t *handle)
 {
+	struct drm_vgem_file_private *file_priv = file->driver_priv;
+	struct drm_vgem_gem_object *vobj = NULL;
+	struct drm_gem_object *obj = NULL;
+	struct dma_buf *dma_buf;
+	struct dma_buf_attachment *attach = NULL;
+	struct sg_table *sg = NULL;
+	bool drop_dma_buf_ref = false;
+	int ret;
+
+	dma_buf = dma_buf_get(prime_fd);
+	if (IS_ERR(dma_buf))
+		return PTR_ERR(dma_buf);
+
+	mutex_lock(&dev->prime_mutex);
+	/* First check that we don't dup on this file */
+	ret = drm_prime_lookup_fd_handle_mapping(&file_priv->prime, dma_buf,
+						 handle);
+	if (ret == 0) {
+		DRM_DEBUG_PRIME("file_priv has an object for this dma_buf\n");
+		dma_buf_put(dma_buf);
+		mutex_unlock(&dev->prime_mutex);
+		return 0;
+	}
+
+	/* Now check if we've already created/imported this object */
+	ret = drm_prime_lookup_obj(dev, dma_buf, &obj);
+	if (ret == 0 && obj != NULL) {
+		DRM_DEBUG_PRIME("driver has an object for this dma_buf\n");
+		drop_dma_buf_ref = true;
+		vobj = to_vgem_bo(obj);
+		goto handle_create;
+	}
+
+	DRM_DEBUG_PRIME("Creating a new object for dma_buf\n");
+
+	attach = dma_buf_attach(dma_buf, dev->dev);
+	if (IS_ERR(attach)) {
+		ret = PTR_ERR(attach);
+		goto fail_put;
+	}
+
+	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+	if (IS_ERR(sg)) {
+		ret = PTR_ERR(sg);
+		goto fail_detach;
+	}
+
+	vobj = kzalloc(sizeof(*vobj), GFP_KERNEL);
+	if (vobj == NULL) {
+		ret = -ENOMEM;
+		goto fail_unmap;
+	}
+
+	/* As a result of this mmap will not work -yet- */
+	ret = drm_gem_private_object_init(dev, &vobj->base, dma_buf->size);
+	if (ret) {
+		kfree(vobj);
+		ret = -ENOMEM;
+		goto fail_unmap;
+	}
+
+	obj = &vobj->base;
+
+handle_create:
+	ret = drm_gem_handle_create(file, obj, handle);
+	if (ret)
+		return ret;
+
+	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
+						 dma_buf, *handle);
+	if (ret)
+		goto fail_handle;
+
+	mutex_unlock(&dev->prime_mutex);
+
+	if (drop_dma_buf_ref) {
+		/* This should mean we found this in the global hash */
+		dma_buf_put(dma_buf);
+	} else {
+		/* Handle holds the reference for the object we created */
+		drm_gem_object_unreference(obj);
+	}
+	return 0;
+
+fail_handle:
+	drm_gem_object_handle_unreference_unlocked(obj);
+fail_unmap:
+	dma_buf_unmap_attachment(attach, sg);
+fail_detach:
+	dma_buf_detach(dma_buf, attach);
+fail_put:
+	dma_buf_put(dma_buf);
+	mutex_unlock(&dev->prime_mutex);
+
 	return 0;
 }
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/7] drm: actually enable PRIME
  2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
                   ` (6 preceding siblings ...)
  2012-02-22 19:29 ` [PATCH 6/7] drm/vgem: import support Ben Widawsky
@ 2012-02-22 19:29 ` Ben Widawsky
  7 siblings, 0 replies; 17+ messages in thread
From: Ben Widawsky @ 2012-02-22 19:29 UTC (permalink / raw)
  To: dri-devel; +Cc: linaro-mm-sig, Ben Widawsky

Enable prime in both Kconfig and vgem.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/Kconfig         |    1 +
 drivers/gpu/drm/vgem/vgem_drv.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 566c468..c893f1e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -9,6 +9,7 @@ menuconfig DRM
 	depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU
 	select I2C
 	select I2C_ALGOBIT
+	select DMA_SHARED_BUFFER
 	help
 	  Kernel-level support for the Direct Rendering Infrastructure (DRI)
 	  introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 9cd1ed4..66ed123 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -300,7 +300,8 @@ static const struct file_operations vgem_driver_fops = {
 };
 
 static struct drm_driver vgem_driver = {
-	.driver_features	= DRIVER_BUS_PLATFORM | DRIVER_GEM,
+	.driver_features	=
+		DRIVER_BUS_PLATFORM | DRIVER_GEM | DRIVER_PRIME,
 	.load			= vgem_load,
 	.unload			= vgem_unload,
 	.open			= vgem_open,
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] drm: base prime support
  2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
@ 2012-02-22 19:58   ` Kristian Høgsberg
  2012-02-27  2:49   ` InKi Dae
  1 sibling, 0 replies; 17+ messages in thread
From: Kristian Høgsberg @ 2012-02-22 19:58 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: linaro-mm-sig, Dave Airlie, dri-devel

On Wed, Feb 22, 2012 at 2:29 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> ---
>  drivers/gpu/drm/Makefile    |    2 +-
>  drivers/gpu/drm/drm_drv.c   |    3 +
>  drivers/gpu/drm/drm_gem.c   |    3 +-
>  drivers/gpu/drm/drm_prime.c |  126 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm.h           |   10 +++-
>  include/drm/drmP.h          |   35 ++++++++++++
>  6 files changed, 176 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_prime.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 0cde1b8..202f650 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -12,7 +12,7 @@ drm-y       :=        drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>                drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>                drm_crtc.o drm_modes.o drm_edid.o \
>                drm_info.o drm_debugfs.o drm_encoder_slave.o \
> -               drm_trace_points.o drm_global.o drm_usb.o
> +               drm_trace_points.o drm_global.o drm_usb.o drm_prime.o
>
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index ebf7d3f..786b134 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -135,6 +135,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
>
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED),
> +
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index f8625e2..e19a958 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -145,6 +145,7 @@ int drm_gem_object_init(struct drm_device *dev,
>        kref_init(&obj->refcount);
>        atomic_set(&obj->handle_count, 0);
>        obj->size = size;
> +       obj->prime_fd = -1;
>
>        return 0;
>  }
> @@ -166,7 +167,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
>        kref_init(&obj->refcount);
>        atomic_set(&obj->handle_count, 0);
>        obj->size = size;
> -
> +       obj->prime_fd = -1;
>        return 0;
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_init);
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> new file mode 100644
> index 0000000..11f142f
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -0,0 +1,126 @@
> +#include <linux/export.h>
> +#include <linux/dma-buf.h>
> +#include "drmP.h"
> +
> +struct drm_prime_member {
> +       struct list_head entry;
> +       struct dma_buf *dma_buf;
> +       uint32_t handle;
> +};
> +
> +int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> +                                struct drm_file *file_priv)
> +{
> +       struct drm_prime_handle *args = data;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_PRIME))
> +               return -EINVAL;
> +
> +       return dev->driver->prime_handle_to_fd(dev, file_priv, args->handle, &args->fd);

We need a way to pass flags to this so we can pass DRM_PRIME_CLOEXEC
(or whatever, but not O_CLOEXEC) to create the fd in close-on-exec
mode, and it needs to be available in the libdrm API.  See man
epoll_create1 for an example where we had to add a new brown-bag
syscall to allow this.

> +}
> +
> +int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> +                                struct drm_file *file_priv)
> +{
> +       struct drm_prime_handle *args = data;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_PRIME))
> +               return -EINVAL;
> +
> +       return dev->driver->prime_fd_to_handle(dev, file_priv, args->fd, &args->handle);
> +}
> +
> +struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages)
> +{
> +       struct sg_table *sg = NULL;
> +       struct scatterlist *iter;
> +       int i;
> +       int ret;
> +
> +       sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
> +       if (!sg)
> +               goto out;
> +
> +       ret = sg_alloc_table(sg, nr_pages, GFP_KERNEL);
> +       if (ret)
> +               goto out;
> +
> +       for_each_sg(sg->sgl, iter, nr_pages, i)
> +               sg_set_page(iter, pages[i], PAGE_SIZE, 0);
> +
> +       return sg;
> +out:
> +       kfree(sg);
> +       return NULL;
> +}
> +EXPORT_SYMBOL(drm_prime_pages_to_sg);
> +
> +/* helper function to cleanup a GEM/prime object */
> +void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg)
> +{
> +       struct dma_buf_attachment *attach;
> +
> +       attach = obj->import_attach;
> +       if (sg)
> +               dma_buf_unmap_attachment(attach, sg);
> +       dma_buf_detach(attach->dmabuf, attach);
> +}
> +EXPORT_SYMBOL(drm_prime_gem_destroy);
> +
> +void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
> +{
> +       INIT_LIST_HEAD(&prime_fpriv->head);
> +}
> +EXPORT_SYMBOL(drm_prime_init_file_private);
> +
> +void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
> +{
> +       struct drm_prime_member *member, *safe;
> +       list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
> +               list_del(&member->entry);
> +               kfree(member);
> +       }
> +}
> +EXPORT_SYMBOL(drm_prime_destroy_file_private);
> +
> +int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
> +{
> +       struct drm_prime_member *member;
> +
> +       member = kmalloc(sizeof(*member), GFP_KERNEL);
> +       if (!member)
> +               return -ENOMEM;
> +
> +       member->dma_buf = dma_buf;
> +       member->handle = handle;
> +       list_add(&member->entry, &prime_fpriv->head);
> +       return 0;
> +}
> +EXPORT_SYMBOL(drm_prime_insert_fd_handle_mapping);
> +
> +int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
> +{
> +       struct drm_prime_member *member;
> +
> +       list_for_each_entry(member, &prime_fpriv->head, entry) {
> +               if (member->dma_buf == dma_buf) {
> +                       *handle = member->handle;
> +                       return 0;
> +               }
> +       }
> +       return -ENOENT;
> +}
> +EXPORT_SYMBOL(drm_prime_lookup_fd_handle_mapping);
> +
> +void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
> +{
> +       struct drm_prime_member *member, *safe;
> +
> +       list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
> +               if (member->dma_buf == dma_buf) {
> +                       list_del(&member->entry);
> +                       kfree(member);
> +               }
> +       }
> +}
> +EXPORT_SYMBOL(drm_prime_remove_fd_handle_mapping);
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 49d94ed..3dcae79 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -617,6 +617,13 @@ struct drm_get_cap {
>        __u64 value;
>  };
>
> +struct drm_prime_handle {
> +       __u32 handle;
> +
> +       /* returned fd for prime */
> +       __s32 fd;

__u32 flags;

> +};
> +
>  #include "drm_mode.h"
>
>  #define DRM_IOCTL_BASE                 'd'
> @@ -673,7 +680,8 @@ struct drm_get_cap {
>  #define DRM_IOCTL_UNLOCK               DRM_IOW( 0x2b, struct drm_lock)
>  #define DRM_IOCTL_FINISH               DRM_IOW( 0x2c, struct drm_lock)
>
> -#define DRM_IOCTL_GEM_PRIME_OPEN        DRM_IOWR(0x2e, struct drm_gem_open)
> +#define DRM_IOCTL_PRIME_HANDLE_TO_FD    DRM_IOWR(0x2d, struct drm_prime_handle)
> +#define DRM_IOCTL_PRIME_FD_TO_HANDLE    DRM_IOWR(0x2e, struct drm_prime_handle)
>

#define DRM_PRIME_CLOEXEC 0x01

>  #define DRM_IOCTL_AGP_ACQUIRE          DRM_IO(  0x30)
>  #define DRM_IOCTL_AGP_RELEASE          DRM_IO(  0x31)
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 92f0981..9558111 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -150,6 +150,7 @@ int drm_err(const char *func, const char *format, ...);
>  #define DRIVER_IRQ_VBL2    0x800
>  #define DRIVER_GEM         0x1000
>  #define DRIVER_MODESET     0x2000
> +#define DRIVER_PRIME       0x4000
>
>  #define DRIVER_BUS_PCI 0x1
>  #define DRIVER_BUS_PLATFORM 0x2
> @@ -652,6 +653,19 @@ struct drm_gem_object {
>        uint32_t pending_write_domain;
>
>        void *driver_private;
> +
> +       /* prime fd exporting this object, -1 for no fd */
> +       int prime_fd;
> +       /* dma buf exported from this GEM object */
> +       struct dma_buf *export_dma_buf;
> +
> +       /* dma buf attachment backing this object */
> +       struct dma_buf_attachment *import_attach;
> +};
> +
> +/* initial implementaton using a linked list - todo hashtab */
> +struct drm_prime_file_private {
> +       struct list_head head;
>  };
>
>  #include "drm_crtc.h"
> @@ -890,6 +904,13 @@ struct drm_driver {
>        int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
>        void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
>
> +       /* prime */
> +       int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv,
> +                                 uint32_t handle, int *prime_fd);
> +
> +       int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv,
> +                                 int prime_fd, uint32_t *handle);
> +
>        /* vga arb irq handler */
>        void (*vgaarb_irq)(struct drm_device *dev, bool state);
>
> @@ -1502,6 +1523,20 @@ extern int drm_vblank_info(struct seq_file *m, void *data);
>  extern int drm_clients_info(struct seq_file *m, void* data);
>  extern int drm_gem_name_info(struct seq_file *m, void *data);
>
> +extern int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> +                                       struct drm_file *file_priv);
> +extern int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> +                                       struct drm_file *file_priv);
> +
> +extern struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages);
> +extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg);
> +
> +void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
> +void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
> +int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
> +int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
> +void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
> +
>  #if DRM_DEBUG_CODE
>  extern int drm_vma_info(struct seq_file *m, void *data);
>  #endif
> --
> 1.7.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Linaro-mm-sig] [PATCH 5/7] drm/vgem: prime export support
  2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
@ 2012-02-23 19:00   ` Chris Wilson
  2012-02-27 13:37   ` Tomasz Stanislawski
  2012-02-27 15:43   ` Tomasz Stanislawski
  2 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2012-02-23 19:00 UTC (permalink / raw)
  To: Ben Widawsky, dri-devel; +Cc: linaro-mm-sig, Dave Airlie

[-- Attachment #1: Type: text/plain, Size: 4595 bytes --]

On Wed, 22 Feb 2012 20:29:18 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> dma-buf export implementation. Heavily influenced by Dave Airlie's proof
> of concept work.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/vgem/Makefile       |    2 +-
>  drivers/gpu/drm/vgem/vgem_dma_buf.c |  128 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/vgem/vgem_drv.c     |    6 ++
>  drivers/gpu/drm/vgem/vgem_drv.h     |    7 ++
>  4 files changed, 142 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c
> 
> diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
> index 3f4c7b8..1055cb7 100644
> --- a/drivers/gpu/drm/vgem/Makefile
> +++ b/drivers/gpu/drm/vgem/Makefile
> @@ -1,4 +1,4 @@
>  ccflags-y := -Iinclude/drm
> -vgem-y := vgem_drv.o
> +vgem-y := vgem_drv.o vgem_dma_buf.o
>  
>  obj-$(CONFIG_DRM_VGEM)	+= vgem.o
> diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> new file mode 100644
> index 0000000..eca9445
> --- /dev/null
> +++ b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> @@ -0,0 +1,128 @@
> +/*
> + * Copyright © 2012 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Ben Widawsky <ben@bwidawsk.net>
> + *
> + */
> +
> +#include <linux/dma-buf.h>
> +#include "vgem_drv.h"
> +
> +#define VGEM_FD_PERMS 0600
> +
> +struct sg_table *vgem_gem_map_dma_buf(struct dma_buf_attachment *attachment,
> +                                      enum dma_data_direction dir)
I guess I'm just not quite happy with the dma-buf interface, but I'd
have liked to have avoided the whole sg_table allocation and have made
the dma_buf embeddable. Daniel is slacking!

> +int vgem_prime_to_fd(struct drm_device *dev, struct drm_file *file,
> +		     uint32_t handle, int *prime_fd)
> +{
> +	struct drm_vgem_file_private *file_priv = file->driver_priv;
> +	struct drm_vgem_gem_object *obj;
> +	int ret;
> +
> +	DRM_DEBUG_PRIME("Request fd for handle %d\n", handle);
> +
> +	obj = to_vgem_bo(drm_gem_object_lookup(dev, file, handle));
> +	if (!obj)
> +		return -EBADF;
-ENOENT; EBADF is for reporting that ioctl was itself called on an invalid fd.

locking fail.

> +	/* This means a user has already called get_fd on this */
> +	if (obj->base.prime_fd != -1) {
> +		DRM_DEBUG_PRIME("User requested a previously exported buffer "
> +				"%d %d\n", handle, obj->base.prime_fd);
> +		drm_gem_object_unreference(&obj->base);
> +		goto out_fd;
> +	}
> +
> +	/* Make a dma buf out of our vgem object */
> +	obj->base.export_dma_buf = dma_buf_export(obj, &vgem_dmabuf_ops,
> +						  obj->base.size,
> +						  VGEM_FD_PERMS);
locking fail.

> +	if (IS_ERR(obj->base.export_dma_buf)) {
> +		DRM_DEBUG_PRIME("export fail\n");
> +		return PTR_ERR(obj->base.export_dma_buf);
> +	} else
> +		obj->base.prime_fd = dma_buf_fd(obj->base.export_dma_buf);
locking fail... ;-)

> +
> +	mutex_lock(&dev->prime_mutex);
Per-device mutex for a per-file hash-table?

> +	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
> +						 obj->base.export_dma_buf,
> +						 handle);
> +	WARN_ON(ret);
> +	ret = drm_prime_add_dma_buf(dev, &obj->base);
> +	mutex_unlock(&dev->prime_mutex);
> +	if (ret)
> +		return ret;
> +
> +out_fd:
> +	*prime_fd = obj->base.prime_fd;
> +
> +	return 0;
> +}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Linaro-mm-sig] [PATCH 6/7] drm/vgem: import support
  2012-02-22 19:29 ` [PATCH 6/7] drm/vgem: import support Ben Widawsky
@ 2012-02-23 19:10   ` Chris Wilson
  2012-02-23 23:00   ` Chris Wilson
  1 sibling, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2012-02-23 19:10 UTC (permalink / raw)
  To: Ben Widawsky, dri-devel; +Cc: linaro-mm-sig, Dave Airlie

On Wed, 22 Feb 2012 20:29:19 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> dma-buf import support. The function definitely needs some cleanup.
> 
> When reading through this code, there are 3 cases to consider:
> 1. vgem exporter, vgem importer, same fd
> 2. vgem exporter, vgem importer, different fd
> 3. X expoter, vgem importer - not yet tested
> 
> See the comments in the code for detailed explanation.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/vgem/vgem_dma_buf.c |  120 +++++++++++++++++++++++++++++++++++
>  1 files changed, 120 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> index eca9445..92c1823 100644
> --- a/drivers/gpu/drm/vgem/vgem_dma_buf.c
> +++ b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> @@ -120,9 +120,129 @@ out_fd:
>  	return 0;
>  }
>  
> +/*
> + * Convert a dma-buf fd to a drm object handle, creating new object/handle as
> + * needed.
> + *
> + * There are 2 "interesting" cases we have to consider. The other, less interesting
> + * case is when importer == exporter, and drm_files are the same.
> + * vgem exporter
> + *   The original exporter may or may not still hold a reference to the
> + *   object by the time we reach here. Once we get a dma_buf reference though
> + *   we know the original object cannot go away. Next we grab the prime mutex
> + *   to prevent our lists from being screwed up from under us. We should next
> + *   find the object in our global dma_buf hash. To make everything cool
> + *   though we need to
> + *     create a handle for the importer
> + *     add the handle to the per file list
> + *     drop the dma_buf reference
> + *       the object can't go away due to us owning a file handle for it.
> + *   In the end there should be 2 handle references, and 1 dma-buf reference.
> + *
> + * other exporter
> + *   This case is very similar to the previous one. The primary difference is we
> + *   do not want to drop the dma_buf reference since we know nothing about the
> + *   reference counting from the exporter. So instead, we hold the dma_buf
> + *   reference, but can drop the object reference. In the end of this case there
> + *   should be 1 handle reference, and 1 dma-buf reference.
> + */
>  int vgem_prime_to_handle(struct drm_device *dev,
>  			 struct drm_file *file, int prime_fd,
>  			 uint32_t *handle)
>  {
> +	struct drm_vgem_file_private *file_priv = file->driver_priv;
> +	struct drm_vgem_gem_object *vobj = NULL;
> +	struct drm_gem_object *obj = NULL;
> +	struct dma_buf *dma_buf;
> +	struct dma_buf_attachment *attach = NULL;
> +	struct sg_table *sg = NULL;
> +	bool drop_dma_buf_ref = false;
> +	int ret;
> +
> +	dma_buf = dma_buf_get(prime_fd);
> +	if (IS_ERR(dma_buf))
> +		return PTR_ERR(dma_buf);
> +
> +	mutex_lock(&dev->prime_mutex);
> +	/* First check that we don't dup on this file */
> +	ret = drm_prime_lookup_fd_handle_mapping(&file_priv->prime, dma_buf,
> +						 handle);
> +	if (ret == 0) {
> +		DRM_DEBUG_PRIME("file_priv has an object for this dma_buf\n");
> +		dma_buf_put(dma_buf);
> +		mutex_unlock(&dev->prime_mutex);
> +		return 0;
> +	}
> +
> +	/* Now check if we've already created/imported this object */
> +	ret = drm_prime_lookup_obj(dev, dma_buf, &obj);
> +	if (ret == 0 && obj != NULL) {
> +		DRM_DEBUG_PRIME("driver has an object for this dma_buf\n");
> +		drop_dma_buf_ref = true;
> +		vobj = to_vgem_bo(obj);
> +		goto handle_create;
> +	}
> +
> +	DRM_DEBUG_PRIME("Creating a new object for dma_buf\n");
> +
> +	attach = dma_buf_attach(dma_buf, dev->dev);
> +	if (IS_ERR(attach)) {
> +		ret = PTR_ERR(attach);
> +		goto fail_put;
> +	}
> +
> +	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> +	if (IS_ERR(sg)) {
> +		ret = PTR_ERR(sg);
> +		goto fail_detach;
> +	}
> +
> +	vobj = kzalloc(sizeof(*vobj), GFP_KERNEL);
> +	if (vobj == NULL) {
> +		ret = -ENOMEM;
> +		goto fail_unmap;
> +	}
> +
> +	/* As a result of this mmap will not work -yet- */
> +	ret = drm_gem_private_object_init(dev, &vobj->base, dma_buf->size);
> +	if (ret) {
> +		kfree(vobj);
> +		ret = -ENOMEM;
> +		goto fail_unmap;
> +	}
> +
> +	obj = &vobj->base;
Don't we need to store the backing attachment and prime_fd on the newly
created vgem obj?

> +
> +handle_create:
> +	ret = drm_gem_handle_create(file, obj, handle);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
> +						 dma_buf, *handle);
> +	if (ret)
> +		goto fail_handle;
> +
> +	mutex_unlock(&dev->prime_mutex);
> +
> +	if (drop_dma_buf_ref) {
> +		/* This should mean we found this in the global hash */
> +		dma_buf_put(dma_buf);
> +	} else {
> +		/* Handle holds the reference for the object we created */
> +		drm_gem_object_unreference(obj);
> +	}
> +	return 0;
> +
> +fail_handle:
> +	drm_gem_object_handle_unreference_unlocked(obj);
> +fail_unmap:
> +	dma_buf_unmap_attachment(attach, sg);
> +fail_detach:
> +	dma_buf_detach(dma_buf, attach);
> +fail_put:
> +	dma_buf_put(dma_buf);
> +	mutex_unlock(&dev->prime_mutex);
> +
>  	return 0;
>  }

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Linaro-mm-sig] [PATCH 6/7] drm/vgem: import support
  2012-02-22 19:29 ` [PATCH 6/7] drm/vgem: import support Ben Widawsky
  2012-02-23 19:10   ` [Linaro-mm-sig] " Chris Wilson
@ 2012-02-23 23:00   ` Chris Wilson
  1 sibling, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2012-02-23 23:00 UTC (permalink / raw)
  To: Ben Widawsky, dri-devel; +Cc: linaro-mm-sig, Dave Airlie

On Wed, 22 Feb 2012 20:29:19 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> +	mutex_lock(&dev->prime_mutex);
> +	/* First check that we don't dup on this file */
> +	ret = drm_prime_lookup_fd_handle_mapping(&file_priv->prime, dma_buf,
> +						 handle);

The other example of importing bo we have already in GEM is through the
use of flink and global names. There we create a new handle every time a
process opens a name, and de-duplication is indeed handled in userspace
if it so desires.

Multiple handles pointing to the same object simplifies the code without
risking kernel integrity, so why bother with kernel de-dupe? And allow
userspace the extra bit of freedom to shoot itself in the foot!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] drm: base prime support
  2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
  2012-02-22 19:58   ` Kristian Høgsberg
@ 2012-02-27  2:49   ` InKi Dae
  1 sibling, 0 replies; 17+ messages in thread
From: InKi Dae @ 2012-02-27  2:49 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: linaro-mm-sig, Dave Airlie, dri-devel

2012년 2월 23일 오전 4:29, Ben Widawsky <ben@bwidawsk.net>님의 말:
> From: Dave Airlie <airlied@redhat.com>
>
> ---
>  drivers/gpu/drm/Makefile    |    2 +-
>  drivers/gpu/drm/drm_drv.c   |    3 +
>  drivers/gpu/drm/drm_gem.c   |    3 +-
>  drivers/gpu/drm/drm_prime.c |  126 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm.h           |   10 +++-
>  include/drm/drmP.h          |   35 ++++++++++++
>  6 files changed, 176 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_prime.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 0cde1b8..202f650 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -12,7 +12,7 @@ drm-y       :=        drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
>                drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
>                drm_crtc.o drm_modes.o drm_edid.o \
>                drm_info.o drm_debugfs.o drm_encoder_slave.o \
> -               drm_trace_points.o drm_global.o drm_usb.o
> +               drm_trace_points.o drm_global.o drm_usb.o drm_prime.o
>
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index ebf7d3f..786b134 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -135,6 +135,9 @@ static struct drm_ioctl_desc drm_ioctls[] = {
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
>
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED),
> +       DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED),
> +
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index f8625e2..e19a958 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -145,6 +145,7 @@ int drm_gem_object_init(struct drm_device *dev,
>        kref_init(&obj->refcount);
>        atomic_set(&obj->handle_count, 0);
>        obj->size = size;
> +       obj->prime_fd = -1;
>
>        return 0;
>  }
> @@ -166,7 +167,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
>        kref_init(&obj->refcount);
>        atomic_set(&obj->handle_count, 0);
>        obj->size = size;
> -
> +       obj->prime_fd = -1;
>        return 0;
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_init);
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> new file mode 100644
> index 0000000..11f142f
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -0,0 +1,126 @@
> +#include <linux/export.h>
> +#include <linux/dma-buf.h>
> +#include "drmP.h"
> +
> +struct drm_prime_member {
> +       struct list_head entry;
> +       struct dma_buf *dma_buf;
> +       uint32_t handle;
> +};
> +
> +int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> +                                struct drm_file *file_priv)
> +{
> +       struct drm_prime_handle *args = data;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_PRIME))
> +               return -EINVAL;
> +
> +       return dev->driver->prime_handle_to_fd(dev, file_priv, args->handle, &args->fd);
> +}
> +
> +int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> +                                struct drm_file *file_priv)
> +{
> +       struct drm_prime_handle *args = data;
> +
> +       if (!drm_core_check_feature(dev, DRIVER_PRIME))
> +               return -EINVAL;
> +
> +       return dev->driver->prime_fd_to_handle(dev, file_priv, args->fd, &args->handle);
> +}
> +
> +struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages)
> +{
> +       struct sg_table *sg = NULL;
> +       struct scatterlist *iter;
> +       int i;
> +       int ret;
> +
> +       sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
> +       if (!sg)
> +               goto out;
> +
> +       ret = sg_alloc_table(sg, nr_pages, GFP_KERNEL);
> +       if (ret)
> +               goto out;
> +
> +       for_each_sg(sg->sgl, iter, nr_pages, i)
> +               sg_set_page(iter, pages[i], PAGE_SIZE, 0);
> +
> +       return sg;
> +out:
> +       kfree(sg);
> +       return NULL;
> +}
> +EXPORT_SYMBOL(drm_prime_pages_to_sg);
> +
> +/* helper function to cleanup a GEM/prime object */
> +void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg)
> +{
> +       struct dma_buf_attachment *attach;
> +
> +       attach = obj->import_attach;
> +       if (sg)
> +               dma_buf_unmap_attachment(attach, sg);
> +       dma_buf_detach(attach->dmabuf, attach);
> +}
> +EXPORT_SYMBOL(drm_prime_gem_destroy);
> +
> +void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
> +{
> +       INIT_LIST_HEAD(&prime_fpriv->head);
> +}
> +EXPORT_SYMBOL(drm_prime_init_file_private);
> +
> +void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
> +{
> +       struct drm_prime_member *member, *safe;
> +       list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
> +               list_del(&member->entry);
> +               kfree(member);
> +       }
> +}
> +EXPORT_SYMBOL(drm_prime_destroy_file_private);
> +
> +int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
> +{
> +       struct drm_prime_member *member;
> +
> +       member = kmalloc(sizeof(*member), GFP_KERNEL);
> +       if (!member)
> +               return -ENOMEM;
> +
> +       member->dma_buf = dma_buf;
> +       member->handle = handle;
> +       list_add(&member->entry, &prime_fpriv->head);
> +       return 0;
> +}
> +EXPORT_SYMBOL(drm_prime_insert_fd_handle_mapping);
> +
> +int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
> +{
> +       struct drm_prime_member *member;
> +
> +       list_for_each_entry(member, &prime_fpriv->head, entry) {
> +               if (member->dma_buf == dma_buf) {
> +                       *handle = member->handle;
> +                       return 0;
> +               }
> +       }
> +       return -ENOENT;
> +}
> +EXPORT_SYMBOL(drm_prime_lookup_fd_handle_mapping);
> +
> +void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
> +{
> +       struct drm_prime_member *member, *safe;
> +
> +       list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
> +               if (member->dma_buf == dma_buf) {
> +                       list_del(&member->entry);
> +                       kfree(member);
> +               }
> +       }
> +}
> +EXPORT_SYMBOL(drm_prime_remove_fd_handle_mapping);
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 49d94ed..3dcae79 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -617,6 +617,13 @@ struct drm_get_cap {
>        __u64 value;
>  };
>
> +struct drm_prime_handle {
> +       __u32 handle;
> +
> +       /* returned fd for prime */
> +       __s32 fd;
> +};
> +
>  #include "drm_mode.h"
>
>  #define DRM_IOCTL_BASE                 'd'
> @@ -673,7 +680,8 @@ struct drm_get_cap {
>  #define DRM_IOCTL_UNLOCK               DRM_IOW( 0x2b, struct drm_lock)
>  #define DRM_IOCTL_FINISH               DRM_IOW( 0x2c, struct drm_lock)
>
> -#define DRM_IOCTL_GEM_PRIME_OPEN        DRM_IOWR(0x2e, struct drm_gem_open)
> +#define DRM_IOCTL_PRIME_HANDLE_TO_FD    DRM_IOWR(0x2d, struct drm_prime_handle)
> +#define DRM_IOCTL_PRIME_FD_TO_HANDLE    DRM_IOWR(0x2e, struct drm_prime_handle)
>
>  #define DRM_IOCTL_AGP_ACQUIRE          DRM_IO(  0x30)
>  #define DRM_IOCTL_AGP_RELEASE          DRM_IO(  0x31)
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 92f0981..9558111 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -150,6 +150,7 @@ int drm_err(const char *func, const char *format, ...);
>  #define DRIVER_IRQ_VBL2    0x800
>  #define DRIVER_GEM         0x1000
>  #define DRIVER_MODESET     0x2000
> +#define DRIVER_PRIME       0x4000
>
>  #define DRIVER_BUS_PCI 0x1
>  #define DRIVER_BUS_PLATFORM 0x2
> @@ -652,6 +653,19 @@ struct drm_gem_object {
>        uint32_t pending_write_domain;
>
>        void *driver_private;
> +
> +       /* prime fd exporting this object, -1 for no fd */
> +       int prime_fd;
> +       /* dma buf exported from this GEM object */
> +       struct dma_buf *export_dma_buf;
> +
> +       /* dma buf attachment backing this object */
> +       struct dma_buf_attachment *import_attach;
> +};
> +
> +/* initial implementaton using a linked list - todo hashtab */
> +struct drm_prime_file_private {
> +       struct list_head head;
>  };
>
>  #include "drm_crtc.h"
> @@ -890,6 +904,13 @@ struct drm_driver {
>        int (*gem_open_object) (struct drm_gem_object *, struct drm_file *);
>        void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
>
> +       /* prime */
> +       int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv,
> +                                 uint32_t handle, int *prime_fd);
> +
> +       int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv,
> +                                 int prime_fd, uint32_t *handle);
> +
>        /* vga arb irq handler */
>        void (*vgaarb_irq)(struct drm_device *dev, bool state);
>
> @@ -1502,6 +1523,20 @@ extern int drm_vblank_info(struct seq_file *m, void *data);
>  extern int drm_clients_info(struct seq_file *m, void* data);
>  extern int drm_gem_name_info(struct seq_file *m, void *data);
>
> +extern int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
> +                                       struct drm_file *file_priv);
> +extern int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> +                                       struct drm_file *file_priv);
> +
> +extern struct sg_table *drm_prime_pages_to_sg(struct page **pages, int nr_pages);
> +extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg);
> +
> +void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
> +void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
> +int drm_prime_insert_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
> +int drm_prime_lookup_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
> +void drm_prime_remove_fd_handle_mapping(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
> +
>  #if DRM_DEBUG_CODE
>  extern int drm_vma_info(struct seq_file *m, void *data);
>  #endif
> --
> 1.7.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


Tested-by: Inki Dae <inki.dae@samsung.com>

for this, you can refer to following link:
http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/exynos-drm-dmabuf

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] drm/vgem: prime export support
  2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
  2012-02-23 19:00   ` [Linaro-mm-sig] " Chris Wilson
@ 2012-02-27 13:37   ` Tomasz Stanislawski
  2012-02-27 15:43   ` Tomasz Stanislawski
  2 siblings, 0 replies; 17+ messages in thread
From: Tomasz Stanislawski @ 2012-02-27 13:37 UTC (permalink / raw)
  Cc: linaro-mm-sig, Daniel Vetter, dri-devel, Dave Airlie

Hi Ben,
Please refer to comments below.

On 02/22/2012 08:29 PM, Ben Widawsky wrote:
> dma-buf export implementation. Heavily influenced by Dave Airlie's proof
> of concept work.
>
> Cc: Daniel Vetter<daniel.vetter@ffwll.ch>
> Cc: Dave Airlie<airlied@redhat.com>
> Signed-off-by: Ben Widawsky<ben@bwidawsk.net>
> ---
>   drivers/gpu/drm/vgem/Makefile       |    2 +-
>   drivers/gpu/drm/vgem/vgem_dma_buf.c |  128 +++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/vgem/vgem_drv.c     |    6 ++
>   drivers/gpu/drm/vgem/vgem_drv.h     |    7 ++
>   4 files changed, 142 insertions(+), 1 deletions(-)
>   create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c
>
[snip]
> +
> +int vgem_prime_to_fd(struct drm_device *dev, struct drm_file *file,
> +		     uint32_t handle, int *prime_fd)
> +{
> +	struct drm_vgem_file_private *file_priv = file->driver_priv;
> +	struct drm_vgem_gem_object *obj;
> +	int ret;
> +
> +	DRM_DEBUG_PRIME("Request fd for handle %d\n", handle);
> +
> +	obj = to_vgem_bo(drm_gem_object_lookup(dev, file, handle));
> +	if (!obj)
> +		return -EBADF;
> +
> +	/* This means a user has already called get_fd on this */
> +	if (obj->base.prime_fd != -1) {
> +		DRM_DEBUG_PRIME("User requested a previously exported buffer "
> +				"%d %d\n", handle, obj->base.prime_fd);
> +		drm_gem_object_unreference(&obj->base);
> +		goto out_fd;

IMO, it is not safe to cache a number of file descriptor. This number 
may unexpectedly become invalid introducing a bug. Please refer to the 
scenario:
- application possess a GEM buffer called A
- call DRM_IOCTL_PRIME_HANDLE_TO_FD on A to obtain a file descriptor fd_A
- application calls fd_B = dup(fd_A). Now both fd_A and fd_B point to 
the same DMABUF instance.
- application calls close(fd_A), now fd_A is no longer a valid file 
descriptor
- application tries to export the buffer again and calls ioctl 
DRM_IOCTL_PRIME_HANDLE_TO_FD on GEM buffer. The field obj->base.prime_fd 
is already set to fd_A so value fd_A is returned to userspace. Notice 
that this is a bug because fd_A is no longer valid.

I think that field prime_fd should be removed because it is too 
unreliable. The driver should call dma_buf_fd every time when the buffer 
is exported.

> +	}
> +
> +	/* Make a dma buf out of our vgem object */
> +	obj->base.export_dma_buf = dma_buf_export(obj,&vgem_dmabuf_ops,
> +						  obj->base.size,
> +						  VGEM_FD_PERMS);
> +	if (IS_ERR(obj->base.export_dma_buf)) {
> +		DRM_DEBUG_PRIME("export fail\n");
> +		return PTR_ERR(obj->base.export_dma_buf);
> +	} else
> +		obj->base.prime_fd = dma_buf_fd(obj->base.export_dma_buf);
> +
> +	mutex_lock(&dev->prime_mutex);
> +	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
> +						 obj->base.export_dma_buf,
> +						 handle);
> +	WARN_ON(ret);
> +	ret = drm_prime_add_dma_buf(dev,&obj->base);
> +	mutex_unlock(&dev->prime_mutex);
> +	if (ret)
> +		return ret;
> +
> +out_fd:
> +	*prime_fd = obj->base.prime_fd;
> +
> +	return 0;
> +}
> +

Regards,
Tomasz Stanislawski

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] drm/vgem: prime export support
  2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
  2012-02-23 19:00   ` [Linaro-mm-sig] " Chris Wilson
  2012-02-27 13:37   ` Tomasz Stanislawski
@ 2012-02-27 15:43   ` Tomasz Stanislawski
  2012-02-29 15:50     ` Daniel Vetter
  2 siblings, 1 reply; 17+ messages in thread
From: Tomasz Stanislawski @ 2012-02-27 15:43 UTC (permalink / raw)
  Cc: arnd, Daniel Vetter, dri-devel, linaro-mm-sig,
	'박경민',
	Rob Clark, Dave Airlie, Marek Szyprowski

Hi Ben and Sumit,

Please refer to comments below:

On 02/22/2012 08:29 PM, Ben Widawsky wrote:
> dma-buf export implementation. Heavily influenced by Dave Airlie's proof
> of concept work.
>
> Cc: Daniel Vetter<daniel.vetter@ffwll.ch>
> Cc: Dave Airlie<airlied@redhat.com>
> Signed-off-by: Ben Widawsky<ben@bwidawsk.net>
> ---
>   drivers/gpu/drm/vgem/Makefile       |    2 +-
>   drivers/gpu/drm/vgem/vgem_dma_buf.c |  128 +++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/vgem/vgem_drv.c     |    6 ++
>   drivers/gpu/drm/vgem/vgem_drv.h     |    7 ++
>   4 files changed, 142 insertions(+), 1 deletions(-)
>   create mode 100644 drivers/gpu/drm/vgem/vgem_dma_buf.c
>

[snip]

> +struct sg_table *vgem_gem_map_dma_buf(struct dma_buf_attachment *attachment,
> +                                      enum dma_data_direction dir)
> +{
> +	struct drm_vgem_gem_object *obj = attachment->dmabuf->priv;
> +	struct sg_table *sg;
> +	int ret;
> +
> +	ret = vgem_gem_get_pages(obj);
> +	if (ret) {
> +		vgem_gem_put_pages(obj);

is it safe to call vgem_gem_put_pages if vgem_gem_get_pages failed (I 
mean ret < 0 was returned) ?

> +		return NULL;
> +	}
> +
> +	BUG_ON(obj->pages == NULL);
> +
> +	sg = drm_prime_pages_to_sg(obj->pages, obj->base.size / PAGE_SIZE);

if drm_prime_pages_to_sg fails then you should call vgem_gem_put_pages 
for cleanup.

> +	return sg;
> +}

The documentation of DMABUF says fallowing words about map_dma_buf callback.

"It is one of the buffer operations that must be implemented by the 
exporter. It should return the sg_table containing scatterlist for this 
buffer, mapped into caller's address space."

The scatterlist returned by drm_prime_pages_to_sg is not mapped to any 
device. The map_dma_buf callback should call dma_map_sg for caller 
device, which is attachment->dev. Otherwise the implementation is not 
consistent with the DMABUF spec.

I think that it should be chosen to change implementation in map_dma_buf 
in DRM drivers or to drop mentioned sentence from the DMABUF spec.

I bear to the second solution because IMO there is no reliable way of 
mapping a scatterlist to importer device by an exporter. The dma_map_sg 
could be used but not all drivers makes use of DMA framework.

Sumit, what is your opinion about it?

> +
> +void vgem_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
> +			    struct sg_table *sg)
> +{
> +	sg_free_table(sg);
> +	kfree(sg);
> +}
> +
> +void vgem_gem_release(struct dma_buf *buf)
> +{
> +	struct drm_vgem_gem_object *obj = buf->priv;
> +
> +	BUG_ON(buf != obj->base.export_dma_buf);
> +
> +	obj->base.prime_fd = -1;
> +	obj->base.export_dma_buf = NULL;
> +	drm_gem_object_unreference_unlocked(&obj->base);
> +}
> +
> +struct dma_buf_ops vgem_dmabuf_ops = {
> +	.map_dma_buf = vgem_gem_map_dma_buf,
> +	.unmap_dma_buf = vgem_gem_unmap_dma_buf,
> +	.release = vgem_gem_release
> +};
> +

Regards,
Tomasz Stanislawski

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] drm/vgem: prime export support
  2012-02-27 15:43   ` Tomasz Stanislawski
@ 2012-02-29 15:50     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-02-29 15:50 UTC (permalink / raw)
  To: Tomasz Stanislawski
  Cc: Ben Widawsky, arnd, dri-devel, linaro-mm-sig,
	박경민,
	Rob Clark, Dave Airlie, Sumit Semwal, Marek Szyprowski

On Mon, Feb 27, 2012 at 16:43, Tomasz Stanislawski
<t.stanislaws@samsung.com> wrote:
> The documentation of DMABUF says fallowing words about map_dma_buf callback.
>
> "It is one of the buffer operations that must be implemented by the
> exporter. It should return the sg_table containing scatterlist for this
> buffer, mapped into caller's address space."
>
> The scatterlist returned by drm_prime_pages_to_sg is not mapped to any
> device. The map_dma_buf callback should call dma_map_sg for caller device,
> which is attachment->dev. Otherwise the implementation is not consistent
> with the DMABUF spec.
>
> I think that it should be chosen to change implementation in map_dma_buf in
> DRM drivers or to drop mentioned sentence from the DMABUF spec.
>
> I bear to the second solution because IMO there is no reliable way of
> mapping a scatterlist to importer device by an exporter. The dma_map_sg
> could be used but not all drivers makes use of DMA framework.

Yep, drm/gem dma_buf integration is in violation of the spec here, and
imo the spec is right. To fix this we need to extend dma_buf to also
cover use-cases it currently can't work in (like cpu access). See

http://cgit.freedesktop.org/~danvet/drm/log/?h=dma_buf-cpu-access

for my wip rfc to make that possible. We obviously need to fix up the
drm prime stuff before merging, so the current emphasis for these
rfc's is on getting the lifecycles and similar stuff right for
integration buffer-sharing with gem (and not yet so much actually
getting at the data).

Yours, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-02-29 15:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-22 19:29 [PATCH 0/7] RFCv3 VGEM Prime (dma-buf) Ben Widawsky
2012-02-22 19:29 ` [PATCH 1/7] drm: base prime support Ben Widawsky
2012-02-22 19:58   ` Kristian Høgsberg
2012-02-27  2:49   ` InKi Dae
2012-02-22 19:29 ` [PATCH 2/7] DRM_DEBUG_PRIME Ben Widawsky
2012-02-22 19:29 ` [PATCH 2/7] drm: DRM_DEBUG_PRIME Ben Widawsky
2012-02-22 19:29 ` [PATCH 3/7] drm/vgem: virtual GEM provider Ben Widawsky
2012-02-22 19:29 ` [PATCH 4/7] drm: per device prime dma buf hash Ben Widawsky
2012-02-22 19:29 ` [PATCH 5/7] drm/vgem: prime export support Ben Widawsky
2012-02-23 19:00   ` [Linaro-mm-sig] " Chris Wilson
2012-02-27 13:37   ` Tomasz Stanislawski
2012-02-27 15:43   ` Tomasz Stanislawski
2012-02-29 15:50     ` Daniel Vetter
2012-02-22 19:29 ` [PATCH 6/7] drm/vgem: import support Ben Widawsky
2012-02-23 19:10   ` [Linaro-mm-sig] " Chris Wilson
2012-02-23 23:00   ` Chris Wilson
2012-02-22 19:29 ` [PATCH 7/7] drm: actually enable PRIME Ben Widawsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.