[Intel-gfx] [PATCH 17/18] drm/i915: Unify legacy/execlists submit_execbuf callbacks
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Fri Jul 22 08:45:07 UTC 2016
On ke, 2016-07-20 at 14:12 +0100, Chris Wilson wrote:
> Now that emitting requests is identical between legacy and execlists, we
> can use the same function to build up the ring for submitting to either
> engine. (With the exception of i915_switch_contexts(), but in time that
> will also be handled gracefully.)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
This series craves for some T-b's.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 20 -----
> drivers/gpu/drm/i915/i915_gem.c | 2 -
> drivers/gpu/drm/i915/i915_gem_context.c | 7 +-
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 24 ++++--
> drivers/gpu/drm/i915/intel_lrc.c | 123 -----------------------------
> drivers/gpu/drm/i915/intel_lrc.h | 4 -
> 6 files changed, 21 insertions(+), 159 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3f67431577e3..f188c9a9b746 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1705,18 +1705,6 @@ 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;
> - uint64_t batch_obj_vm_offset;
> - struct intel_engine_cs *engine;
> - struct drm_i915_gem_object *batch_obj;
> - struct i915_gem_context *ctx;
> - struct drm_i915_gem_request *request;
> -};
> -
> /* used in computing the new watermarks state */
> struct intel_wm_config {
> unsigned int num_pipes_active;
> @@ -2016,9 +2004,6 @@ struct drm_i915_private {
>
> /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> struct {
> - int (*execbuf_submit)(struct i915_execbuffer_params *params,
> - struct drm_i915_gem_execbuffer2 *args,
> - struct list_head *vmas);
> void (*cleanup_engine)(struct intel_engine_cs *engine);
> void (*stop_engine)(struct intel_engine_cs *engine);
>
> @@ -2990,11 +2975,6 @@ int i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> int i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> -void i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> - struct drm_i915_gem_request *req);
> -int i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
> - struct drm_i915_gem_execbuffer2 *args,
> - 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.c b/drivers/gpu/drm/i915/i915_gem.c
> index 77d7c0b012f4..9fdecef34fa8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4531,11 +4531,9 @@ int i915_gem_init(struct drm_device *dev)
> mutex_lock(&dev->struct_mutex);
>
> if (!i915.enable_execlists) {
> - dev_priv->gt.execbuf_submit = i915_gem_ringbuffer_submission;
> dev_priv->gt.cleanup_engine = intel_engine_cleanup;
> dev_priv->gt.stop_engine = intel_engine_stop;
> } else {
> - dev_priv->gt.execbuf_submit = intel_execlists_submission;
> dev_priv->gt.cleanup_engine = intel_logical_ring_cleanup;
> dev_priv->gt.stop_engine = intel_logical_ring_stop;
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index e1eed0f449c6..72b21c7b7547 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -893,8 +893,9 @@ int i915_switch_context(struct drm_i915_gem_request *req)
> {
> struct intel_engine_cs *engine = req->engine;
>
> - WARN_ON(i915.enable_execlists);
> lockdep_assert_held(&req->i915->drm.struct_mutex);
> + if (i915.enable_execlists)
> + return 0;
>
> if (!req->ctx->engine[engine->id].state) {
> struct i915_gem_context *to = req->ctx;
> @@ -942,9 +943,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
> if (IS_ERR(req))
> return PTR_ERR(req);
>
> - ret = 0;
> - if (!i915.enable_execlists)
> - ret = i915_switch_context(req);
> + ret = i915_switch_context(req);
> i915_add_request_no_flush(req);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 2d9f1f4bc058..e302477418d8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -42,6 +42,18 @@
>
> #define BATCH_OFFSET_BIAS (256*1024)
>
> +struct i915_execbuffer_params {
> + struct drm_device *dev;
> + struct drm_file *file;
> + u32 dispatch_flags;
> + u32 args_batch_start_offset;
> + u32 batch_obj_vm_offset;
> + struct intel_engine_cs *engine;
> + struct drm_i915_gem_object *batch_obj;
> + struct i915_gem_context *ctx;
> + struct drm_i915_gem_request *request;
> +};
> +
> struct eb_vmas {
> struct list_head vmas;
> int and;
> @@ -1117,7 +1129,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
> return ctx;
> }
>
> -void
> +static void
> i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> struct drm_i915_gem_request *req)
> {
> @@ -1244,10 +1256,10 @@ err:
> return ERR_PTR(ret);
> }
>
> -int
> -i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
> - struct drm_i915_gem_execbuffer2 *args,
> - struct list_head *vmas)
> +static int
> +execbuf_submit(struct i915_execbuffer_params *params,
> + struct drm_i915_gem_execbuffer2 *args,
> + struct list_head *vmas)
> {
> struct drm_i915_private *dev_priv = params->request->i915;
> u64 exec_start, exec_len;
> @@ -1636,7 +1648,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> params->batch_obj = batch_obj;
> params->ctx = ctx;
>
> - ret = dev_priv->gt.execbuf_submit(params, args, &eb->vmas);
> + ret = execbuf_submit(params, args, &eb->vmas);
> err_request:
> i915_gem_execbuffer_retire_commands(params);
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index bce37d0d431f..8d1589f0ea7e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -642,39 +642,6 @@ static void execlists_context_queue(struct drm_i915_gem_request *request)
> spin_unlock_bh(&engine->execlist_lock);
> }
>
> -static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
> - struct list_head *vmas)
> -{
> - const unsigned other_rings = ~intel_engine_flag(req->engine);
> - struct i915_vma *vma;
> - uint32_t flush_domains = 0;
> - bool flush_chipset = false;
> - int ret;
> -
> - list_for_each_entry(vma, vmas, exec_list) {
> - struct drm_i915_gem_object *obj = vma->obj;
> -
> - if (obj->active & other_rings) {
> - ret = i915_gem_object_sync(obj, req);
> - if (ret)
> - return ret;
> - }
> -
> - if (obj->base.write_domain & I915_GEM_DOMAIN_CPU)
> - flush_chipset |= i915_gem_clflush_object(obj, false);
> -
> - flush_domains |= obj->base.write_domain;
> - }
> -
> - if (flush_domains & I915_GEM_DOMAIN_GTT)
> - wmb();
> -
> - /* Unconditionally invalidate gpu caches and ensure that we do flush
> - * any residual writes from the previous batch.
> - */
> - return req->engine->emit_flush(req, I915_GEM_GPU_DOMAINS, 0);
> -}
> -
> int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request)
> {
> struct intel_engine_cs *engine = request->engine;
> @@ -776,96 +743,6 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
> return 0;
> }
>
> -/**
> - * intel_execlists_submission() - submit a batchbuffer for execution, Execlists style
> - * @params: execbuffer call parameters.
> - * @args: execbuffer call arguments.
> - * @vmas: list of vmas.
> - *
> - * This is the evil twin version of i915_gem_ringbuffer_submission. It abstracts
> - * away the submission details of the execbuffer ioctl call.
> - *
> - * Return: non-zero if the submission fails.
> - */
> -int intel_execlists_submission(struct i915_execbuffer_params *params,
> - struct drm_i915_gem_execbuffer2 *args,
> - struct list_head *vmas)
> -{
> - struct drm_device *dev = params->dev;
> - struct intel_engine_cs *engine = params->engine;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_ring *ring = params->request->ring;
> - u64 exec_start;
> - int instp_mode;
> - u32 instp_mask;
> - int ret;
> -
> - instp_mode = args->flags & I915_EXEC_CONSTANTS_MASK;
> - instp_mask = I915_EXEC_CONSTANTS_MASK;
> - switch (instp_mode) {
> - case I915_EXEC_CONSTANTS_REL_GENERAL:
> - case I915_EXEC_CONSTANTS_ABSOLUTE:
> - case I915_EXEC_CONSTANTS_REL_SURFACE:
> - if (instp_mode != 0 && engine->id != RCS) {
> - DRM_DEBUG("non-0 rel constants mode on non-RCS\n");
> - return -EINVAL;
> - }
> -
> - if (instp_mode != dev_priv->relative_constants_mode) {
> - if (instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) {
> - DRM_DEBUG("rel surface constants mode invalid on gen5+\n");
> - return -EINVAL;
> - }
> -
> - /* The HW changed the meaning on this bit on gen6 */
> - instp_mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
> - }
> - break;
> - default:
> - DRM_DEBUG("execbuf with unknown constants: %d\n", instp_mode);
> - return -EINVAL;
> - }
> -
> - if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
> - DRM_DEBUG("sol reset is gen7 only\n");
> - return -EINVAL;
> - }
> -
> - ret = execlists_move_to_gpu(params->request, vmas);
> - if (ret)
> - return ret;
> -
> - if (engine->id == RCS &&
> - instp_mode != dev_priv->relative_constants_mode) {
> - ret = intel_ring_begin(params->request, 4);
> - if (ret)
> - return ret;
> -
> - intel_ring_emit(ring, MI_NOOP);
> - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> - intel_ring_emit_reg(ring, INSTPM);
> - intel_ring_emit(ring, instp_mask << 16 | instp_mode);
> - intel_ring_advance(ring);
> -
> - dev_priv->relative_constants_mode = instp_mode;
> - }
> -
> - exec_start = params->batch_obj_vm_offset +
> - args->batch_start_offset;
> -
> - ret = engine->emit_bb_start(params->request,
> - exec_start, args->batch_len,
> - params->dispatch_flags);
> - if (ret)
> - return ret;
> -
> - trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
> -
> - i915_gem_execbuffer_move_to_active(vmas, params->request);
> -
> - return 0;
> -}
> -
> void intel_execlists_cancel_requests(struct intel_engine_cs *engine)
> {
> struct drm_i915_gem_request *req, *tmp;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 212ee7c43438..0f9c9925985c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -95,10 +95,6 @@ uint64_t intel_lr_context_descriptor(struct i915_gem_context *ctx,
> /* Execlists */
> int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv,
> int enable_execlists);
> -struct i915_execbuffer_params;
> -int intel_execlists_submission(struct i915_execbuffer_params *params,
> - struct drm_i915_gem_execbuffer2 *args,
> - struct list_head *vmas);
>
> void intel_execlists_cancel_requests(struct intel_engine_cs *engine);
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
More information about the Intel-gfx
mailing list