On Wed, Jul 13, 2016 at 05:29:21PM -0300, Gustavo Padovan wrote:
> 2016-07-12 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>:
>
> > 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@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>
> > ---
> > 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.
That's already stored inside the fence->flags FENCE_FLAG_SIGNALED_BIT.
If it it set (which it will be after we are signaled),
fence->ops->signaled() is not called. So we know that if this function
is ever called, the fence is unsignaled.
> > +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.
Hmm, yes, we can improve the debug output using that:
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
index 63095331c446..b7da11419ad6 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -52,13 +52,13 @@ static bool vgem_fence_enable_signaling(struct fence *fence)
static void vgem_fence_value_str(struct fence *fence, char *str, int size)
{
- snprintf(str, size, "%u", 0);
+ snprintf(str, size, "%u", fence->seqno);
}
static void vgem_fence_timeline_value_str(struct fence *fence, char *str,
int size)
{
- snprintf(str, size, "%u", 0);
+ snprintf(str, size, "%u", fence_is_signaled(fence) ? fence->seqno : 0);
}
const struct fence_ops vgem_fence_ops = {
@@ -81,7 +81,7 @@ static struct fence *vgem_fence_create(struct vgem_file *vfile)
spin_lock_init(&fence->lock);
fence_init(&fence->base, &vgem_fence_ops, &fence->lock,
- fence_context_alloc(1), 0);
+ fence_context_alloc(1), 1);
return &fence->base;
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
