[Intel-gfx] [PATCH v6 06/11] drm/i915: introduce a mechanism to extend execbuf2

[Chris Wilson]

Quoting Lionel Landwerlin (2019-07-01 12:34:32)
> We're planning to use this for a couple of new feature where we need
> to provide additional parameters to execbuf.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

Looks ok, are you convinced by I915_EXEC_EXT? It doesn't roll off the
tongue too well for me, but I guess EXT is a bit more ingrained in
your cerebral cortex.

> ---
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 32 ++++++++++++++++++-
>  include/uapi/drm/i915_drm.h                   | 25 +++++++++++++--
>  2 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 1c5dfbfad71b..9887fa9e3ac8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -23,6 +23,7 @@
>  #include "i915_gem_clflush.h"
>  #include "i915_gem_context.h"
>  #include "i915_trace.h"
> +#include "i915_user_extensions.h"
>  #include "intel_drv.h"
>  
>  enum {
> @@ -271,6 +272,10 @@ struct i915_execbuffer {
>          */
>         int lut_size;
>         struct hlist_head *buckets; /** ht for relocation handles */
> +
> +       struct {
> +               u64 flags; /** Available extensions parameters */
> +       } extensions;
>  };
>  
>  #define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
> @@ -1969,7 +1974,7 @@ static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
>                 return false;
>  
>         /* Kernel clipping was a DRI1 misfeature */
> -       if (!(exec->flags & I915_EXEC_FENCE_ARRAY)) {
> +       if (!(exec->flags & (I915_EXEC_FENCE_ARRAY | I915_EXEC_EXT))) {
>                 if (exec->num_cliprects || exec->cliprects_ptr)
>                         return false;
>         }
> @@ -2347,6 +2352,27 @@ signal_fence_array(struct i915_execbuffer *eb,
>         }
>  }
>  
> +static const i915_user_extension_fn execbuf_extensions[] = {
> +};
> +
> +static int
> +parse_execbuf2_extensions(struct drm_i915_gem_execbuffer2 *args,
> +                         struct i915_execbuffer *eb)
> +{
> +       eb->extensions.flags = 0;
> +
> +       if (!(args->flags & I915_EXEC_EXT))
> +               return 0;
> +
> +       if (args->num_cliprects != 0)
> +               return -EINVAL;
> +
> +       return i915_user_extensions(u64_to_user_ptr(args->cliprects_ptr),
> +                                   execbuf_extensions,
> +                                   ARRAY_SIZE(execbuf_extensions),
> +                                   eb);
> +}
> +
>  static int
>  i915_gem_do_execbuffer(struct drm_device *dev,
>                        struct drm_file *file,
> @@ -2393,6 +2419,10 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>         if (args->flags & I915_EXEC_IS_PINNED)
>                 eb.batch_flags |= I915_DISPATCH_PINNED;
>  
> +       err = parse_execbuf2_extensions(args, &eb);
> +       if (err)
> +               return err;
> +
>         if (args->flags & I915_EXEC_FENCE_IN) {
>                 in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2));
>                 if (!in_fence)
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index e27a8eda9121..efa195d6994e 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1013,6 +1013,10 @@ struct drm_i915_gem_exec_fence {
>         __u32 flags;
>  };
>  
> +enum drm_i915_gem_execbuffer_ext {
> +       DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */

We have a weird mix of trying to avoid drm_i915_gem and yet it's
plastered all over the structs. Sigh.

> +};

enums next to uABI make me nervous :)

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris