drm/i915: Merged the many do_execbuf() parameters into a structure

The do_execbuf() function takes quite a few parameters. The actual set of
parameters is going to change with the conversion to passing requests around.
Further, it is due to grow massively with the arrival of the GPU scheduler.

This patch simplifies the prototype by passing a parameter structure instead.
Changing the parameter set in the future is then simply a matter of
adding/removing items to the structure.

Note that the structure does not contain absolutely everything that is passed
in. This is because the intention is to use this structure more extensively
later in this patch series and more especially in the GPU scheduler that is
coming soon. The latter requires hanging on to the structure as the final
hardware submission can be delayed until long after the execbuf IOCTL has
returned to user land. Thus it is unsafe to put anything in the structure that
is local to the IOCTL call itself - such as the 'args' parameter. All entries
must be copies of data or pointers to structures that are reference counted in
some way and guaranteed to exist for the duration of the batch buffer's life.

v2: Rebased to newer tree and updated for changes to the command parser.
Specifically, a code shuffle has required saving the batch start address in the
params structure.

For: VIZ-5115
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Tomas Elf <tomas.elf@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b687e9f4dcf9640913861b173a550a7cf301796b..e2d0ed05c1e54e7a3e0a5bd32e8a6205cf917e69 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1648,6 +1648,17 @@ struct i915_virtual_gpu {
        bool active;
 };
 
+struct i915_execbuffer_params {
+       struct drm_device               *dev;
+       struct drm_file                 *file;
+       uint32_t                        dispatch_flags;
+       uint32_t                        args_batch_start_offset;
+       uint32_t                        batch_obj_vm_offset;
+       struct intel_engine_cs          *ring;
+       struct drm_i915_gem_object      *batch_obj;
+       struct intel_context            *ctx;
+};
+
 struct drm_i915_private {
        struct drm_device *dev;
        struct kmem_cache *objects;
@@ -1885,13 +1896,9 @@ struct drm_i915_private {
 
        /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
        struct {
-               int (*execbuf_submit)(struct drm_device *dev, struct drm_file *file,
-                                     struct intel_engine_cs *ring,
-                                     struct intel_context *ctx,
+               int (*execbuf_submit)(struct i915_execbuffer_params *params,
                                      struct drm_i915_gem_execbuffer2 *args,
-                                     struct list_head *vmas,
-                                     struct drm_i915_gem_object *batch_obj,
-                                     u64 exec_start, u32 flags);
+                                     struct list_head *vmas);
                int (*init_rings)(struct drm_device *dev);
                void (*cleanup_ring)(struct intel_engine_cs *ring);
                void (*stop_ring)(struct intel_engine_cs *ring);
@@ -2689,14 +2696,9 @@ void i915_gem_execbuffer_retire_commands(struct drm_device *dev,
                                         struct drm_file *file,
                                         struct intel_engine_cs *ring,
                                         struct drm_i915_gem_object *obj);
-int i915_gem_ringbuffer_submission(struct drm_device *dev,
-                                  struct drm_file *file,
-                                  struct intel_engine_cs *ring,
-                                  struct intel_context *ctx,
+int i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
                                   struct drm_i915_gem_execbuffer2 *args,
-                                  struct list_head *vmas,
-                                  struct drm_i915_gem_object *batch_obj,
-                                  u64 exec_start, u32 flags);
+                                  struct list_head *vmas);
 int i915_gem_execbuffer(struct drm_device *dev, void *data,
                        struct drm_file *file_priv);
 int i915_gem_execbuffer2(struct drm_device *dev, void *data,

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2e41f7281c84f89463036d38d481c96fee392fa1..c5f879e594c4cb76e3141b30dbf213e2ecfeca3a 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1193,17 +1193,15 @@ err:
 }
 
 int
-i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
-                              struct intel_engine_cs *ring,
-                              struct intel_context *ctx,
+i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
                               struct drm_i915_gem_execbuffer2 *args,
-                              struct list_head *vmas,
-                              struct drm_i915_gem_object *batch_obj,
-                              u64 exec_start, u32 dispatch_flags)
+                              struct list_head *vmas)
 {
        struct drm_clip_rect *cliprects = NULL;
+       struct drm_device *dev = params->dev;
+       struct intel_engine_cs *ring = params->ring;
        struct drm_i915_private *dev_priv = dev->dev_private;
-       u64 exec_len;
+       u64 exec_start, exec_len;
        int instp_mode;
        u32 instp_mask;
        int i, ret = 0;
@@ -1255,11 +1253,11 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
        if (ret)
                goto error;
 
-       ret = i915_switch_context(ring, ctx);
+       ret = i915_switch_context(ring, params->ctx);
        if (ret)
                goto error;
 
-       WARN(ctx->ppgtt && ctx->ppgtt->pd_dirty_rings & (1<<ring->id),
+       WARN(params->ctx->ppgtt && params->ctx->ppgtt->pd_dirty_rings & (1<<ring->id),
             "%s didn't clear reload\n", ring->name);
 
        instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
@@ -1320,7 +1318,10 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
                        goto error;
        }
 
-       exec_len = args->batch_len;
+       exec_len   = args->batch_len;
+       exec_start = params->batch_obj_vm_offset +
+                    params->args_batch_start_offset;
+
        if (cliprects) {
                for (i = 0; i < args->num_cliprects; i++) {
                        ret = i915_emit_box(ring, &cliprects[i],
@@ -1330,22 +1331,23 @@ i915_gem_ringbuffer_submission(struct drm_device *dev, struct drm_file *file,
 
                        ret = ring->dispatch_execbuffer(ring,
                                                        exec_start, exec_len,
-                                                       dispatch_flags);
+                                                       params->dispatch_flags);
                        if (ret)
                                goto error;
                }
        } else {
                ret = ring->dispatch_execbuffer(ring,
                                                exec_start, exec_len,
-                                               dispatch_flags);
+                                               params->dispatch_flags);
                if (ret)
                        return ret;
        }
 
-       trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), dispatch_flags);
+       trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), params->dispatch_flags);
 
        i915_gem_execbuffer_move_to_active(vmas, ring);
-       i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
+       i915_gem_execbuffer_retire_commands(params->dev, params->file, ring,
+                                           params->batch_obj);
 
 error:
        kfree(cliprects);
@@ -1415,8 +1417,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
        struct intel_engine_cs *ring;
        struct intel_context *ctx;
        struct i915_address_space *vm;
+       struct i915_execbuffer_params params_master; /* XXX: will be removed later */
+       struct i915_execbuffer_params *params = &params_master;
        const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
-       u64 exec_start = args->batch_start_offset;
        u32 dispatch_flags;
        int ret;
        bool need_relocs;
@@ -1509,6 +1512,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
        else
                vm = &dev_priv->gtt.base;
 
+       memset(&params_master, 0x00, sizeof(params_master));
+
        eb = eb_create(args);
        if (eb == NULL) {
                i915_gem_context_unreference(ctx);
@@ -1551,6 +1556,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
                goto err;
        }
 
+       params->args_batch_start_offset = args->batch_start_offset;
        if (i915_needs_cmd_parser(ring) && args->batch_len) {
                struct drm_i915_gem_object *parsed_batch_obj;
 
@@ -1582,7 +1588,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
                         * command parser has accepted.
                         */
                        dispatch_flags |= I915_DISPATCH_SECURE;
-                       exec_start = 0;
+                       params->args_batch_start_offset = 0;
                        batch_obj = parsed_batch_obj;
                }
        }
@@ -1607,18 +1613,29 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
                if (ret)
                        goto err;
 
-               exec_start += i915_gem_obj_ggtt_offset(batch_obj);
+               params->batch_obj_vm_offset = i915_gem_obj_ggtt_offset(batch_obj);
        } else
-               exec_start += i915_gem_obj_offset(batch_obj, vm);
+               params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm);
 
        /* Allocate a request for this batch buffer nice and early. */
        ret = i915_gem_request_alloc(ring, ctx);
        if (ret)
                goto err_batch_unpin;
 
-       ret = dev_priv->gt.execbuf_submit(dev, file, ring, ctx, args,
-                                         &eb->vmas, batch_obj, exec_start,
-                                         dispatch_flags);
+       /*
+        * Save assorted stuff away to pass through to *_submission().
+        * NB: This data should be 'persistent' and not local as it will
+        * kept around beyond the duration of the IOCTL once the GPU
+        * scheduler arrives.
+        */
+       params->dev                     = dev;
+       params->file                    = file;
+       params->ring                    = ring;
+       params->dispatch_flags          = dispatch_flags;
+       params->batch_obj               = batch_obj;
+       params->ctx                     = ctx;
+
+       ret = dev_priv->gt.execbuf_submit(params, args, &eb->vmas);
 
 err_batch_unpin:
        /*

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6f3ec7197d89653b2476649a75f0c631477227bf..eda3096a5b754137ea1a50b8840e9ab618c7675c 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -858,16 +858,15 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
  *
  * Return: non-zero if the submission fails.
  */
-int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
-                              struct intel_engine_cs *ring,
-                              struct intel_context *ctx,
+int intel_execlists_submission(struct i915_execbuffer_params *params,
                               struct drm_i915_gem_execbuffer2 *args,
-                              struct list_head *vmas,
-                              struct drm_i915_gem_object *batch_obj,
-                              u64 exec_start, u32 dispatch_flags)
+                              struct list_head *vmas)
 {
+       struct drm_device       *dev = params->dev;
+       struct intel_engine_cs  *ring = params->ring;
        struct drm_i915_private *dev_priv = dev->dev_private;
-       struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+       struct intel_ringbuffer *ringbuf = params->ctx->engine[ring->id].ringbuf;
+       u64 exec_start;
        int instp_mode;
        u32 instp_mask;
        int ret;
@@ -918,13 +917,13 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
                return -EINVAL;
        }
 
-       ret = execlists_move_to_gpu(ringbuf, ctx, vmas);
+       ret = execlists_move_to_gpu(ringbuf, params->ctx, vmas);
        if (ret)
                return ret;
 
        if (ring == &dev_priv->ring[RCS] &&
            instp_mode != dev_priv->relative_constants_mode) {
-               ret = intel_logical_ring_begin(ringbuf, ctx, 4);
+               ret = intel_logical_ring_begin(ringbuf, params->ctx, 4);
                if (ret)
                        return ret;
 
@@ -937,14 +936,17 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
                dev_priv->relative_constants_mode = instp_mode;
        }
 
-       ret = ring->emit_bb_start(ringbuf, ctx, exec_start, dispatch_flags);
+       exec_start = params->batch_obj_vm_offset +
+                    args->batch_start_offset;
+
+       ret = ring->emit_bb_start(ringbuf, params->ctx, exec_start, params->dispatch_flags);
        if (ret)
                return ret;
 
-       trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), dispatch_flags);
+       trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), params->dispatch_flags);
 
        i915_gem_execbuffer_move_to_active(vmas, ring);
-       i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
+       i915_gem_execbuffer_retire_commands(params->dev, params->file, ring, params->batch_obj);
 
        return 0;
 }

diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 4148de016931660b4afbfe2ab0c2473a73ac10b0..bf137c43e0a903799ab12a670c6b708539b52286 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -76,13 +76,10 @@ void intel_lr_context_reset(struct drm_device *dev,
 
 /* Execlists */
 int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists);
-int intel_execlists_submission(struct drm_device *dev, struct drm_file *file,
-                              struct intel_engine_cs *ring,
-                              struct intel_context *ctx,
+struct i915_execbuffer_params;
+int intel_execlists_submission(struct i915_execbuffer_params *params,
                               struct drm_i915_gem_execbuffer2 *args,
-                              struct list_head *vmas,
-                              struct drm_i915_gem_object *batch_obj,
-                              u64 exec_start, u32 dispatch_flags);
+                              struct list_head *vmas);
 u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
 
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);