[PATCH v3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
Daniel Vetter
daniel at ffwll.ch
Thu Jul 14 08:12:17 UTC 2016
On Thu, Jul 14, 2016 at 08:04:19AM +0100, Chris Wilson wrote:
> vGEM buffers are useful for passing data between software clients and
> hardware renders. By allowing the user to create and attach fences to
> the exported vGEM buffers (on the dma-buf), the user can implement a
> deferred renderer and queue hardware operations like flipping and then
> signal the buffer readiness (i.e. this allows the user to schedule
> operations out-of-order, but have them complete in-order).
>
> This also makes it much easier to write tightly controlled testcases for
> dma-buf fencing and signaling between hardware drivers.
>
> v2: Don't pretend the fences exist in an ordered timeline, but allocate
> a separate fence-context for each fence so that the fences are
> unordered.
> v3: Make the debug output more interesting, and so the signaled status.
>
> Testcase: igt/vgem_basic/dmabuf-fence
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Zach Reizner <zachr at google.com>
> Cc: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Acked-by: Zach Reizner <zachr at google.com>
One thing I completely forgotten: This allows userspace to hang kernel
drivers. i915 (and other gpu drivers) can recover using hangcheck, but
dumber drivers (v4l, if that ever happens) probably never except such a
case. We've had a similar discusion with the userspace fences exposed in
sw_fence, and decided to move all those ioctl into debugfs. I think we
should do the same for this vgem-based debugging of implicit sync. Sorry
for realizing this this late.
-Daniel
> ---
> drivers/gpu/drm/vgem/Makefile | 2 +-
> drivers/gpu/drm/vgem/vgem_drv.c | 34 +++++++
> drivers/gpu/drm/vgem/vgem_drv.h | 16 +++
> drivers/gpu/drm/vgem/vgem_fence.c | 207 ++++++++++++++++++++++++++++++++++++++
> include/uapi/drm/vgem_drm.h | 62 ++++++++++++
> 5 files changed, 320 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/vgem/vgem_fence.c
> create mode 100644 include/uapi/drm/vgem_drm.h
>
> diff --git a/drivers/gpu/drm/vgem/Makefile b/drivers/gpu/drm/vgem/Makefile
> index 3f4c7b842028..bfcdea1330e6 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_fence.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
> index 29c2aab3c1a7..c15bafb06665 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -83,6 +83,34 @@ static const struct vm_operations_struct vgem_gem_vm_ops = {
> .close = drm_gem_vm_close,
> };
>
> +static int vgem_open(struct drm_device *dev, struct drm_file *file)
> +{
> + struct vgem_file *vfile;
> + int ret;
> +
> + vfile = kzalloc(sizeof(*vfile), GFP_KERNEL);
> + if (!vfile)
> + return -ENOMEM;
> +
> + file->driver_priv = vfile;
> +
> + ret = vgem_fence_open(vfile);
> + if (ret) {
> + kfree(vfile);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void vgem_preclose(struct drm_device *dev, struct drm_file *file)
> +{
> + struct vgem_file *vfile = file->driver_priv;
> +
> + vgem_fence_close(vfile);
> + kfree(vfile);
> +}
> +
> /* ioctls */
>
> static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> @@ -164,6 +192,8 @@ unref:
> }
>
> static struct drm_ioctl_desc vgem_ioctls[] = {
> + DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> };
>
> static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
> @@ -271,9 +301,12 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
>
> static struct drm_driver vgem_driver = {
> .driver_features = DRIVER_GEM | DRIVER_PRIME,
> + .open = vgem_open,
> + .preclose = vgem_preclose,
> .gem_free_object_unlocked = vgem_gem_free_object,
> .gem_vm_ops = &vgem_gem_vm_ops,
> .ioctls = vgem_ioctls,
> + .num_ioctls = ARRAY_SIZE(vgem_ioctls),
> .fops = &vgem_driver_fops,
>
> .dumb_create = vgem_gem_dumb_create,
> @@ -328,5 +361,6 @@ module_init(vgem_init);
> module_exit(vgem_exit);
>
> MODULE_AUTHOR("Red Hat, Inc.");
> +MODULE_AUTHOR("Intel Corporation");
> 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
> index 988cbaae7588..1f8798ad329c 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.h
> +++ b/drivers/gpu/drm/vgem/vgem_drv.h
> @@ -32,9 +32,25 @@
> #include <drm/drmP.h>
> #include <drm/drm_gem.h>
>
> +#include <uapi/drm/vgem_drm.h>
> +
> +struct vgem_file {
> + struct idr fence_idr;
> + struct mutex fence_mutex;
> +};
> +
> #define to_vgem_bo(x) container_of(x, struct drm_vgem_gem_object, base)
> struct drm_vgem_gem_object {
> struct drm_gem_object base;
> };
>
> +int vgem_fence_open(struct vgem_file *file);
> +int vgem_fence_attach_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file);
> +int vgem_fence_signal_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file);
> +void vgem_fence_close(struct vgem_file *file);
> +
> #endif
> diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
> new file mode 100644
> index 000000000000..b7da11419ad6
> --- /dev/null
> +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> @@ -0,0 +1,207 @@
> +/*
> + * Copyright 2016 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
> + * 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.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/reservation.h>
> +
> +#include "vgem_drv.h"
> +
> +struct vgem_fence {
> + struct fence base;
> + struct spinlock lock;
> +};
> +
> +static const char *vgem_fence_get_driver_name(struct fence *fence)
> +{
> + return "vgem";
> +}
> +
> +static const char *vgem_fence_get_timeline_name(struct fence *fence)
> +{
> + return "unbound";
> +}
> +
> +static bool vgem_fence_signaled(struct fence *fence)
> +{
> + return false;
> +}
> +
> +static bool vgem_fence_enable_signaling(struct fence *fence)
> +{
> + return true;
> +}
> +
> +static void vgem_fence_value_str(struct fence *fence, char *str, int size)
> +{
> + snprintf(str, size, "%u", fence->seqno);
> +}
> +
> +static void vgem_fence_timeline_value_str(struct fence *fence, char *str,
> + int size)
> +{
> + snprintf(str, size, "%u", fence_is_signaled(fence) ? fence->seqno : 0);
> +}
> +
> +const struct fence_ops vgem_fence_ops = {
> + .get_driver_name = vgem_fence_get_driver_name,
> + .get_timeline_name = vgem_fence_get_timeline_name,
> + .enable_signaling = vgem_fence_enable_signaling,
> + .signaled = vgem_fence_signaled,
> + .wait = fence_default_wait,
> + .fence_value_str = vgem_fence_value_str,
> + .timeline_value_str = vgem_fence_timeline_value_str,
> +};
> +
> +static struct fence *vgem_fence_create(struct vgem_file *vfile)
> +{
> + struct vgem_fence *fence;
> +
> + fence = kzalloc(sizeof(*fence), GFP_KERNEL);
> + if (!fence)
> + return NULL;
> +
> + spin_lock_init(&fence->lock);
> + fence_init(&fence->base, &vgem_fence_ops, &fence->lock,
> + fence_context_alloc(1), 1);
> +
> + return &fence->base;
> +}
> +
> +static int attach_dmabuf(struct drm_device *dev,
> + struct drm_gem_object *obj)
> +{
> + struct dma_buf *dmabuf;
> +
> + if (obj->dma_buf)
> + return 0;
> +
> + dmabuf = dev->driver->gem_prime_export(dev, obj, 0);
> + if (IS_ERR(dmabuf))
> + return PTR_ERR(dmabuf);
> +
> + obj->dma_buf = dmabuf;
> + drm_gem_object_reference(obj);
> + return 0;
> +}
> +
> +int vgem_fence_attach_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file)
> +{
> + struct drm_vgem_fence_attach *arg = data;
> + struct vgem_file *vfile = file->driver_priv;
> + struct reservation_object *resv;
> + struct drm_gem_object *obj;
> + struct fence *fence;
> + int ret;
> +
> + if (arg->flags & ~VGEM_FENCE_WRITE)
> + return -EINVAL;
> +
> + if (arg->pad)
> + return -EINVAL;
> +
> + obj = drm_gem_object_lookup(file, arg->handle);
> + if (!obj)
> + return -ENOENT;
> +
> + ret = attach_dmabuf(dev, obj);
> + if (ret)
> + goto out;
> +
> + fence = vgem_fence_create(vfile);
> + if (!fence) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + ret = 0;
> + resv = obj->dma_buf->resv;
> + mutex_lock(&resv->lock.base);
> + if (arg->flags & VGEM_FENCE_WRITE)
> + reservation_object_add_excl_fence(resv, fence);
> + else if ((ret = reservation_object_reserve_shared(resv)) == 0)
> + reservation_object_add_shared_fence(resv, fence);
> + mutex_unlock(&resv->lock.base);
> +
> + if (ret == 0) {
> + mutex_lock(&vfile->fence_mutex);
> + ret = idr_alloc(&vfile->fence_idr, fence, 1, 0, GFP_KERNEL);
> + mutex_unlock(&vfile->fence_mutex);
> + if (ret > 0) {
> + arg->out_fence = ret;
> + ret = 0;
> + }
> + }
> + if (ret) {
> + fence_signal(fence);
> + fence_put(fence);
> + }
> +out:
> + drm_gem_object_unreference_unlocked(obj);
> + return ret;
> +}
> +
> +int vgem_fence_signal_ioctl(struct drm_device *dev,
> + void *data,
> + struct drm_file *file)
> +{
> + struct vgem_file *vfile = file->driver_priv;
> + struct drm_vgem_fence_signal *arg = data;
> + struct fence *fence;
> +
> + if (arg->flags)
> + return -EINVAL;
> +
> + mutex_lock(&vfile->fence_mutex);
> + fence = idr_replace(&vfile->fence_idr, NULL, arg->fence);
> + mutex_unlock(&vfile->fence_mutex);
> + if (!fence)
> + return -ENOENT;
> + if (IS_ERR(fence))
> + return PTR_ERR(fence);
> +
> + fence_signal(fence);
> + fence_put(fence);
> + return 0;
> +}
> +
> +int vgem_fence_open(struct vgem_file *vfile)
> +{
> + mutex_init(&vfile->fence_mutex);
> + idr_init(&vfile->fence_idr);
> +
> + return 0;
> +}
> +
> +static int __vgem_fence_idr_fini(int id, void *p, void *data)
> +{
> + fence_signal(p);
> + fence_put(p);
> + return 0;
> +}
> +
> +void vgem_fence_close(struct vgem_file *vfile)
> +{
> + idr_for_each(&vfile->fence_idr, __vgem_fence_idr_fini, vfile);
> + idr_destroy(&vfile->fence_idr);
> +}
> diff --git a/include/uapi/drm/vgem_drm.h b/include/uapi/drm/vgem_drm.h
> new file mode 100644
> index 000000000000..352d2fae8de9
> --- /dev/null
> +++ b/include/uapi/drm/vgem_drm.h
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright 2016 Intel Corporation
> + * All Rights Reserved.
> + *
> + * 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, sub license, 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 NON-INFRINGEMENT.
> + * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS 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.
> + *
> + */
> +
> +#ifndef _UAPI_VGEM_DRM_H_
> +#define _UAPI_VGEM_DRM_H_
> +
> +#include "drm.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif
> +
> +/* Please note that modifications to all structs defined here are
> + * subject to backwards-compatibility constraints.
> + */
> +#define DRM_VGEM_FENCE_ATTACH 0x1
> +#define DRM_VGEM_FENCE_SIGNAL 0x2
> +
> +#define DRM_IOCTL_VGEM_FENCE_ATTACH DRM_IOWR( DRM_COMMAND_BASE + DRM_VGEM_FENCE_ATTACH, struct drm_vgem_fence_attach)
> +#define DRM_IOCTL_VGEM_FENCE_SIGNAL DRM_IOW( DRM_COMMAND_BASE + DRM_VGEM_FENCE_SIGNAL, struct drm_vgem_fence_signal)
> +
> +struct drm_vgem_fence_attach {
> + __u32 handle;
> + __u32 flags;
> +#define VGEM_FENCE_WRITE 0x1
> + __u32 out_fence;
> + __u32 pad;
> +};
> +
> +struct drm_vgem_fence_signal {
> + __u32 fence;
> + __u32 flags;
> +};
> +
> +#if defined(__cplusplus)
> +}
> +#endif
> +
> +#endif /* _UAPI_VGEM_DRM_H_ */
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list