[PATCH 3/3] drm/vgem: Attach sw fences to exported vGEM dma-buf (ioctl)
Daniel Vetter
daniel at ffwll.ch
Tue Jul 12 10:44:17 UTC 2016
On Mon, Jul 11, 2016 at 04:24:45PM +0100, Chris Wilson wrote:
> On Mon, Jul 11, 2016 at 12:10:40PM -0300, Gustavo Padovan wrote:
> > 2016-07-11 Chris Wilson <chris at 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.
> > >
> > > 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>
> > > Acked-by: Zach Reizner <zachr at google.com>
> > > ---
> > > drivers/gpu/drm/vgem/Makefile | 2 +-
> > > drivers/gpu/drm/vgem/vgem_drv.c | 34 ++++++
> > > drivers/gpu/drm/vgem/vgem_drv.h | 18 ++++
> > > drivers/gpu/drm/vgem/vgem_fence.c | 220 ++++++++++++++++++++++++++++++++++++++
> > > include/uapi/drm/vgem_drm.h | 62 +++++++++++
> > > 5 files changed, 335 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 b5fb968d2d5c..2659e5cda857 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..88ce21010e28 100644
> > > --- a/drivers/gpu/drm/vgem/vgem_drv.h
> > > +++ b/drivers/gpu/drm/vgem/vgem_drv.h
> > > @@ -32,9 +32,27 @@
> > > #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;
> > > + u64 fence_context;
> > > + atomic_t fence_seqno;
> > > +};
> > > +
> > > #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..649e9e1cee35
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> > > @@ -0,0 +1,220 @@
> > > +/*
> > > + * 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 "file";
> > > +}
> > > +
> > > +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", 0);
> > > +}
> >
> > I think this would be mainly used for debug purposes but it would be
> > nice to actually return the current timeline value here, i.e., the
> > seqno of the last signalled fence.
>
> The fences can and will be signaled in any order, there is no timeline
> here under the control of userspace, we only give them fences.
>
> > This would also allow to return if the fence was signaled or not
> > in vgem_fence_signaled() instead of just return false.
>
> That doesn't fit the out-of-order unbound nature of the interface. The
> interface is just a collection of fences that userspace associates with
> the buffer that it may signal at any time. (Having no strict timeline is
> an advantage!)
Fences on the same timeline are supposed to be signalled in-order. If you
want full out-of-order fences then you need to grab a new timeline number
for each one. Drivers can and do merge fences on the same timeline and
just carry the one with the largest seqno around.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list