[Intel-gfx] [PATCH] drm/vgem: Couple in drm_syncobj support

[Jason Ekstrand]

On Wed, Aug 9, 2017 at 12:03 PM, Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> To further facilitate GEM testing, allow testing of drm_syncobj by
> attaching them to vgem fences. These fences are already employed by igt
> for testing inter-driver fence handling (across dmabuf and sync_file).
>
> An igt example use would be like:
>
>    int vgem = drm_driver_open(DRIVER_VGEM);
>    uint32_t handle = vgem_create_dummy(vgem);
>

This is a bit nasty... Why do we need a BO?  Why can't we just create and
attach a fence without the BO?


>    uint32_t syncobj = drm_syncobj_create(vgem);
>    uint32_t fence = drmIoctl(vgem,
>                              DRM_IOCTL_VGEM_FENCE_ATTACH,
>                              &(struct vgem_fence_attach){
>                                 .handle = handle,
>                                 .flags = VGEM_FENCE_SYNCOBJ,
>                                 .syncobj = syncobj,
>                              });
>
>    /* ... use syncobj for profit ... */
>
>    vgem_fence_signal(vgem, fence);
>
> For wider use though, there is little immediate benefit to syncobj
> over the vgem fence as both are handles in an idr (the fence here is not
> a sync-file fd like in most other drivers). The main benefit for syncobj
> is that it allows to create channels between objects and drivers by
> virtue of its persistence beyond the vgem fence itself.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c   |  4 +++-
>  drivers/gpu/drm/vgem/vgem_fence.c | 26 ++++++++++++++++++++++----
>  include/drm/drm_syncobj.h         |  2 ++
>  include/uapi/drm/vgem_drm.h       |  3 ++-
>  4 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_
> drv.c
> index 12289673f457..a0202e1eaf6b 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -432,7 +432,9 @@ static void vgem_release(struct drm_device *dev)
>  }
>
>  static struct drm_driver vgem_driver = {
> -       .driver_features                = DRIVER_GEM | DRIVER_PRIME,
> +       .driver_features                = (DRIVER_GEM |
> +                                          DRIVER_PRIME |
> +                                          DRIVER_SYNCOBJ),
>         .release                        = vgem_release,
>         .open                           = vgem_open,
>         .postclose                      = vgem_postclose,
> diff --git a/drivers/gpu/drm/vgem/vgem_fence.c
> b/drivers/gpu/drm/vgem/vgem_fence.c
> index 3109c8308eb5..988e860c03d3 100644
> --- a/drivers/gpu/drm/vgem/vgem_fence.c
> +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> @@ -23,6 +23,8 @@
>  #include <linux/dma-buf.h>
>  #include <linux/reservation.h>
>
> +#include <drm/drm_syncobj.h>
> +
>  #include "vgem_drv.h"
>
>  #define VGEM_FENCE_TIMEOUT (10*HZ)
> @@ -156,20 +158,30 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
>         struct drm_vgem_fence_attach *arg = data;
>         struct vgem_file *vfile = file->driver_priv;
>         struct reservation_object *resv;
> +       struct drm_syncobj *sync = NULL;
>         struct drm_gem_object *obj;
>         struct dma_fence *fence;
>         int ret;
>
> -       if (arg->flags & ~VGEM_FENCE_WRITE)
> -               return -EINVAL;
> -
> -       if (arg->pad)
> +       if (arg->flags & ~(VGEM_FENCE_WRITE | VGEM_FENCE_SYNCOBJ))
>                 return -EINVAL;
>
>         obj = drm_gem_object_lookup(file, arg->handle);
>         if (!obj)
>                 return -ENOENT;
>
> +       if (arg->flags & VGEM_FENCE_SYNCOBJ) {
> +               sync = drm_syncobj_find(file, arg->syncobj);
> +               if (!sync) {
> +                       ret = -ENOENT;
> +                       goto err;
> +               }
> +
> +               /* We don't check if the current syncobj is busy or not, we
> +                * will just replace it with ourselves.
> +                */
> +       }
> +
>         ret = attach_dmabuf(dev, obj);
>         if (ret)
>                 goto err;
> @@ -207,12 +219,18 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
>                         ret = 0;
>                 }
>         }
> +
> +       if (ret == 0 && sync)
> +               drm_syncobj_replace_fence(sync, fence);
> +
>  err_fence:
>         if (ret) {
>                 dma_fence_signal(fence);
>                 dma_fence_put(fence);
>         }
>  err:
> +       if (sync)
> +               drm_syncobj_put(sync);
>         drm_gem_object_unreference_unlocked(obj);
>         return ret;
>  }
> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> index 89976da542b1..9010ab8343e5 100644
> --- a/include/drm/drm_syncobj.h
> +++ b/include/drm/drm_syncobj.h
> @@ -28,6 +28,8 @@
>
>  #include "linux/dma-fence.h"
>
> +struct drm_file;
> +
>  /**
>   * struct drm_syncobj - sync object.
>   *
> diff --git a/include/uapi/drm/vgem_drm.h b/include/uapi/drm/vgem_drm.h
> index bf66f5db6da8..94777197e561 100644
> --- a/include/uapi/drm/vgem_drm.h
> +++ b/include/uapi/drm/vgem_drm.h
> @@ -46,8 +46,9 @@ struct drm_vgem_fence_attach {
>         __u32 handle;
>         __u32 flags;
>  #define VGEM_FENCE_WRITE       0x1
> +#define VGEM_FENCE_SYNCOBJ     0x2
>         __u32 out_fence;
> -       __u32 pad;
> +       __u32 syncobj;
>  };
>
>  struct drm_vgem_fence_signal {
> --
> 2.13.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170809/21c3a748/attachment-0001.html>