Re: [PATCH v2] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)

[Gustavo Padovan <gustavo@padovan.org>]

2016-07-12 Chris Wilson <chris@chris-wilson.co.uk>:

> 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.
> 
> Testcase: igt/vgem_basic/dmabuf-fence
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Zach Reizner <zachr@google.com>
> Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Acked-by: Zach Reizner <zachr@google.com>
> ---
>  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..63095331c446
> --- /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;

With one fence per context you can actually return if the fence was
signalled or not. May be useful for debug.

> +}
> +
> +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", 0);
> +}
> +
> +static void vgem_fence_timeline_value_str(struct fence *fence, char *str,
> +					  int size)
> +{
> +	snprintf(str, size, "%u", 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), 0);

Usually 0 would mean that the fence is already signalled, so I'd better
set it to 1.

	Gustavo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx