[RFC 19/44] drm/i915: Split i915_dem_do_execbuffer() in half

John.C.Harrison
2014-06-26T17:25:32Z

Split the execbuffer() function in half. The first half collects and validates
all the information requried to process the batch buffer. It also does all the
object pinning, relocations, active list management, etc - basically anything
that must be done upfront before the IOCTL returns and allows the user land side
to start changing/freeing things. The second half does the actual ring
submission.

This change implements the split but leaves the back half being called directly
from the end of the front half.
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 125 +++++++++++++++++++++-------
drivers/gpu/drm/i915/i915_scheduler.h | 25 ++++++
2 files changed, 121 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index fda9187..334e8c6 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1090,10 +1090,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct intel_context *ctx;
struct i915_address_space *vm;
const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
- u64 exec_start = args->batch_start_offset, exec_len;
u32 mask, flags;
- int ret, mode, i;
+ int ret, mode;
bool need_relocs;
+ struct i915_scheduler_queue_entry qe;

if (!i915_gem_check_execbuffer(args))
return -EINVAL;
@@ -1240,6 +1240,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (!USES_FULL_PPGTT(dev))
vm = &dev_priv->gtt.base;

+ memset(&qe, 0x00, sizeof(qe));
+
eb = eb_create(args);
if (eb == NULL) {
i915_gem_context_unreference(ctx);
@@ -1318,10 +1320,27 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (ret)
goto err;

+ /* Save assorted stuff away to pass through to execbuffer_final() */
+ qe.params.dev = dev;
+ qe.params.file = file;
+ qe.params.ring = ring;
+ qe.params.eb_flags = flags;
+ qe.params.args_flags = args->flags;
+ qe.params.args_batch_start_offset = args->batch_start_offset;
+ qe.params.args_batch_len = args->batch_len;
+ qe.params.args_num_cliprects = args->num_cliprects;
+ qe.params.args_DR1 = args->DR1;
+ qe.params.args_DR4 = args->DR4;
+ qe.params.batch_obj = batch_obj;
+ qe.params.cliprects = cliprects;
+ qe.params.ctx = ctx;
+ qe.params.mask = mask;
+ qe.params.mode = mode;
+
if (flags & I915_DISPATCH_SECURE)
- exec_start += i915_gem_obj_ggtt_offset(batch_obj);
+ qe.params.batch_obj_vm_offset = i915_gem_obj_ggtt_offset(batch_obj);
else
- exec_start += i915_gem_obj_offset(batch_obj, vm);
+ qe.params.batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm);

ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
if (ret)
@@ -1329,7 +1348,58 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,

i915_gem_execbuffer_move_to_active(&eb->vmas, ring);

- /* To be split into two functions here... */
+ ret = i915_gem_do_execbuffer_final(&qe.params);
+ if (ret)
+ goto err;
+
+ /* Free everything that was stored in the QE structure (until the
+ * scheduler arrives and does it instead): */
+ kfree(qe.params.cliprects);
+
+ /* The eb list is no longer required. The scheduler has extracted all
+ * the information than needs to persist. */
+ eb_destroy(eb);
+
+ /*
+ * Don't clean up everything that is now saved away in the queue.
+ * Just unlock and return immediately.
+ */
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+
+err:
+ /* the request owns the ref now */
+ i915_gem_context_unreference(ctx);
+
+ eb_destroy(eb);
+
+ mutex_unlock(&dev->struct_mutex);
+
+pre_mutex_err:
+ kfree(cliprects);
+
+ return ret;
+}
+
+/*
+ * This is the main function for adding a batch to the ring.
+ * It is called from the scheduler, with the struct_mutex already held.
+ */
+int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params)
+{
+ struct drm_i915_private *dev_priv = params->dev->dev_private;
+ struct intel_engine_cs *ring = params->ring;
+ u64 exec_start, exec_len;
+ int ret, i;
+
+ /* The mutex must be acquired before calling this function */
+ BUG_ON(!mutex_is_locked(&params->dev->struct_mutex));
+
+ if (dev_priv->ums.mm_suspended) {
+ ret = -EBUSY;
+ goto early_err;
+ }

intel_runtime_pm_get(dev_priv);

@@ -1341,12 +1411,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
goto err;

/* Switch to the correct context for the batch */
- ret = i915_switch_context(ring, ctx);
+ ret = i915_switch_context(ring, params->ctx);
if (ret)
goto err;

if (ring == &dev_priv->ring[RCS] &&
- mode != dev_priv->relative_constants_mode) {
+ params->mode != dev_priv->relative_constants_mode) {
ret = intel_ring_begin(ring, 4);
if (ret)
goto err;
@@ -1354,58 +1424,55 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
intel_ring_emit(ring, MI_NOOP);
intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, INSTPM);
- intel_ring_emit(ring, mask << 16 | mode);
+ intel_ring_emit(ring, params->mask << 16 | params->mode);
intel_ring_advance(ring);

- dev_priv->relative_constants_mode = mode;
+ dev_priv->relative_constants_mode = params->mode;
}

- if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
- ret = i915_reset_gen7_sol_offsets(dev, ring);
+ if (params->args_flags & I915_EXEC_GEN7_SOL_RESET) {
+ ret = i915_reset_gen7_sol_offsets(params->dev, ring);
if (ret)
goto err;
}

- exec_len = args->batch_len;
- if (cliprects) {
- for (i = 0; i < args->num_cliprects; i++) {
- ret = i915_emit_box(dev, &cliprects[i],
- args->DR1, args->DR4);
+ exec_len = params->args_batch_len;
+ exec_start = params->batch_obj_vm_offset +
+ params->args_batch_start_offset;
+
+ if (params->cliprects) {
+ for (i = 0; i < params->args_num_cliprects; i++) {
+ ret = i915_emit_box(params->dev, &params->cliprects[i],
+ params->args_DR1, params->args_DR4);
if (ret)
goto err;

ret = ring->dispatch_execbuffer(ring,
exec_start, exec_len,
- flags);
+ params->eb_flags);
if (ret)
goto err;
}
} else {
ret = ring->dispatch_execbuffer(ring,
exec_start, exec_len,
- flags);
+ params->eb_flags);
if (ret)
goto err;
}

- trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
+ trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), params->eb_flags);

- i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj);
+ i915_gem_execbuffer_retire_commands(params->dev, params->file, ring,
+ params->batch_obj);

err:
- /* the request owns the ref now */
- i915_gem_context_unreference(ctx);
- eb_destroy(eb);
-
- mutex_unlock(&dev->struct_mutex);
-
-pre_mutex_err:
- kfree(cliprects);
-
/* intel_gpu_busy should also get a ref, so it will free when the device
* is really idle. */
intel_runtime_pm_put(dev_priv);
+
+early_err:
return ret;
}

diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index 6b2cc51..68a9543 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -25,6 +25,29 @@
#ifndef _I915_SCHEDULER_H_
#define _I915_SCHEDULER_H_

+struct i915_execbuffer_params {
+ struct drm_device *dev;
+ struct drm_file *file;
+ uint32_t eb_flags;
+ uint32_t args_flags;
+ uint32_t args_batch_start_offset;
+ uint32_t args_batch_len;
+ uint32_t args_num_cliprects;
+ uint32_t args_DR1;
+ uint32_t args_DR4;
+ uint32_t batch_obj_vm_offset;
+ struct intel_engine_cs *ring;
+ struct drm_i915_gem_object *batch_obj;
+ struct drm_clip_rect *cliprects;
+ uint32_t mask;
+ int mode;
+ struct intel_context *ctx;
+};
+
+struct i915_scheduler_queue_entry {
+ struct i915_execbuffer_params params;
+};
+
bool i915_scheduler_is_enabled(struct drm_device *dev);
int i915_scheduler_init(struct drm_device *dev);
int i915_scheduler_closefile(struct drm_device *dev,
@@ -44,4 +67,6 @@ bool i915_scheduler_is_seqno_in_flight(struct intel_engine_cs *ring,

#endif /* CONFIG_DRM_I915_SCHEDULER */

+int i915_gem_do_execbuffer_final(struct i915_execbuffer_params *params);
+
#endif /* _I915_SCHEDULER_H_ */
--
1.7.9.5