On Fri, Jul 15, 2016 at 09:31:11AM +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 show the signaled status. > v4: Automatically signal the fence to prevent userspace from > indefinitely hanging drivers. > > Testcase: igt/vgem_basic/dmabuf-fence > Testcase: igt/vgem_slow/nohang > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Sean Paul <seanpaul@xxxxxxxxxxxx> > Cc: Zach Reizner <zachr@xxxxxxxxxx> > Cc: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Acked-by: Zach Reizner <zachr@xxxxxxxxxx> Applied to drm-misc, thanks. -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 | 283 ++++++++++++++++++++++++++++++++++++++ > include/uapi/drm/vgem_drm.h | 62 +++++++++ > 5 files changed, 396 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..e77b52208699 > --- /dev/null > +++ b/drivers/gpu/drm/vgem/vgem_fence.c > @@ -0,0 +1,283 @@ > +/* > + * 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" > + > +#define VGEM_FENCE_TIMEOUT (10*HZ) > + > +struct vgem_fence { > + struct fence base; > + struct spinlock lock; > + struct timer_list timer; > +}; > + > +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_release(struct fence *base) > +{ > + struct vgem_fence *fence = container_of(base, typeof(*fence), base); > + > + del_timer_sync(&fence->timer); > + fence_free(&fence->base); > +} > + > +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, > + .release = vgem_fence_release, > + > + .fence_value_str = vgem_fence_value_str, > + .timeline_value_str = vgem_fence_timeline_value_str, > +}; > + > +static void vgem_fence_timeout(unsigned long data) > +{ > + struct vgem_fence *fence = (struct vgem_fence *)data; > + > + fence_signal(&fence->base); > +} > + > +static struct fence *vgem_fence_create(struct vgem_file *vfile, > + unsigned int flags) > +{ > + 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); > + > + setup_timer(&fence->timer, vgem_fence_timeout, (unsigned long)fence); > + > + /* We force the fence to expire within 10s to prevent driver hangs */ > + mod_timer(&fence->timer, VGEM_FENCE_TIMEOUT); > + > + 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; > +} > + > +/* > + * vgem_fence_attach_ioctl (DRM_IOCTL_VGEM_FENCE_ATTACH): > + * > + * Create and attach a fence to the vGEM handle. This fence is then exposed > + * via the dma-buf reservation object and visible to consumers of the exported > + * dma-buf. If the flags contain VGEM_FENCE_WRITE, the fence indicates the > + * vGEM buffer is being written to by the client and is exposed as an exclusive > + * fence, otherwise the fence indicates the client is current reading from the > + * buffer and all future writes should wait for the client to signal its > + * completion. Note that if a conflicting fence is already on the dma-buf (i.e. > + * an exclusive fence when adding a read, or any fence when adding a write), > + * -EBUSY is reported. Serialisation between operations should be handled > + * by waiting upon the dma-buf. > + * > + * This returns the handle for the new fence that must be signaled within 10 > + * seconds (or otherwise it will automatically expire). See > + * vgem_fence_signal_ioctl (DRM_IOCTL_VGEM_FENCE_SIGNAL). > + * > + * If the vGEM handle does not exist, vgem_fence_attach_ioctl returns -ENOENT. > + */ > +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 err; > + > + fence = vgem_fence_create(vfile, arg->flags); > + if (!fence) { > + ret = -ENOMEM; > + goto err; > + } > + > + /* Check for a conflicting fence */ > + resv = obj->dma_buf->resv; > + if (!reservation_object_test_signaled_rcu(resv, > + arg->flags & VGEM_FENCE_WRITE)) { > + ret = -EBUSY; > + goto err_fence; > + } > + > + /* Expose the fence via the dma-buf */ > + ret = 0; > + 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); > + > + /* Record the fence in our idr for later signaling */ > + 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; > + } > + } > +err_fence: > + if (ret) { > + fence_signal(fence); > + fence_put(fence); > + } > +err: > + drm_gem_object_unreference_unlocked(obj); > + return ret; > +} > + > +/* > + * vgem_fence_signal_ioctl (DRM_IOCTL_VGEM_FENCE_SIGNAL): > + * > + * Signal and consume a fence ealier attached to a vGEM handle using > + * vgem_fence_attach_ioctl (DRM_IOCTL_VGEM_FENCE_ATTACH). > + * > + * All fences must be signaled within 10s of attachment or otherwise they > + * will automatically expire (and a vgem_fence_signal_ioctl returns -ETIMEDOUT). > + * > + * Signaling a fence indicates to all consumers of the dma-buf that the > + * client has completed the operation associated with the fence, and that the > + * buffer is then ready for consumption. > + * > + * If the fence does not exist (or has already been signaled by the client), > + * vgem_fence_signal_ioctl returns -ENOENT. > + */ > +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; > + int ret; > + > + 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); > + > + if (fence_is_signaled(fence)) > + ret = -ETIMEDOUT; > + > + fence_signal(fence); > + fence_put(fence); > + return ret; > +} > + > +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..bf66f5db6da8 > --- /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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel