[PATCH 5/7] drm/vgem: prime export support
On Mon, Feb 27, 2012 at 16:43, Tomasz Stanislawski 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 at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 5/7] drm/vgem: prime export support
On Mon, Feb 27, 2012 at 16:43, Tomasz Stanislawski t.stanisl...@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.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/vgem: prime export support
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
> Cc: Dave Airlie
> Signed-off-by: Ben Widawsky
> ---
> 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(>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
[PATCH 5/7] drm/vgem: prime export support
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
> Cc: Dave Airlie
> Signed-off-by: Ben Widawsky
> ---
> 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(>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,_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(>prime_mutex);
> + ret = drm_prime_insert_fd_handle_mapping(_priv->prime,
> + obj->base.export_dma_buf,
> + handle);
> + WARN_ON(ret);
> + ret = drm_prime_add_dma_buf(dev,>base);
> + mutex_unlock(>prime_mutex);
> + if (ret)
> + return ret;
> +
> +out_fd:
> + *prime_fd = obj->base.prime_fd;
> +
> + return 0;
> +}
> +
Regards,
Tomasz Stanislawski
Re: [PATCH 5/7] drm/vgem: prime export support
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 Vetterdaniel.vet...@ffwll.ch
Cc: Dave Airlieairl...@redhat.com
Signed-off-by: Ben Widawskyb...@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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/vgem: prime export support
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 Vetterdaniel.vet...@ffwll.ch
Cc: Dave Airlieairl...@redhat.com
Signed-off-by: Ben Widawskyb...@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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Linaro-mm-sig] [PATCH 5/7] drm/vgem: prime export support
On Wed, 22 Feb 2012 20:29:18 +0100, Ben Widawsky wrote:
> dma-buf export implementation. Heavily influenced by Dave Airlie's proof
> of concept work.
>
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Signed-off-by: Ben Widawsky
> ---
> 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 000..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
> + *
> + */
> +
> +#include
> +#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(>base);
> + goto out_fd;
> + }
> +
> + /* Make a dma buf out of our vgem object */
> + obj->base.export_dma_buf = dma_buf_export(obj, _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(>prime_mutex);
Per-device mutex for a per-file hash-table?
> + ret = drm_prime_insert_fd_handle_mapping(_priv->prime,
> + obj->base.export_dma_buf,
> + handle);
> + WARN_ON(ret);
> + ret = drm_prime_add_dma_buf(dev, >base);
> + mutex_unlock(>prime_mutex);
> + if (ret)
> + return ret;
> +
> +out_fd:
> + *prime_fd = obj->base.prime_fd;
> +
> + return 0;
> +}
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
Re: [Linaro-mm-sig] [PATCH 5/7] drm/vgem: prime export support
On Wed, 22 Feb 2012 20:29:18 +0100, Ben Widawsky b...@bwidawsk.net wrote:
dma-buf export implementation. Heavily influenced by Dave Airlie's proof
of concept work.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Signed-off-by: Ben Widawsky b...@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 000..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 b...@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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/vgem: prime export support
dma-buf export implementation. Heavily influenced by Dave Airlie's proof
of concept work.
Cc: Daniel Vetter
Cc: Dave Airlie
Signed-off-by: Ben Widawsky
---
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 000..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
+ *
+ */
+
+#include
+#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(>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(>base);
+ goto out_fd;
+ }
+
+ /* Make a dma buf out of our vgem object */
+ obj->base.export_dma_buf = dma_buf_export(obj, _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(>prime_mutex);
+ ret = drm_prime_insert_fd_handle_mapping(_priv->prime,
+obj->base.export_dma_buf,
+handle);
+
[PATCH 5/7] drm/vgem: prime export support
dma-buf export implementation. Heavily influenced by Dave Airlie's proof
of concept work.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Signed-off-by: Ben Widawsky b...@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 000..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 b...@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,
+
