From 8f2a1057d6ec217aefb8bf0de6996294452a2577 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 25 Apr 2019 06:01:43 +0100 Subject: drm/i915: Explicitly pin the logical context for execbuf In order to separate the reservation phase of building a request from its emission phase, we need to pull some of the request alloc activities from deep inside i915_request to the surface, GEM_EXECBUFFER. v2: Be frivolous, use a local drm_i915_private. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190425050143.811-1-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 108 ++++++++++++++++++----------- 1 file changed, 69 insertions(+), 39 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3d672c9edb94..794af8edc6a2 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -34,6 +34,8 @@ #include #include +#include "gt/intel_gt_pm.h" + #include "i915_drv.h" #include "i915_gem_clflush.h" #include "i915_trace.h" @@ -236,7 +238,8 @@ struct i915_execbuffer { unsigned int *flags; struct intel_engine_cs *engine; /** engine to queue the request to */ - struct i915_gem_context *ctx; /** context for building the request */ + struct intel_context *context; /* logical state for the request */ + struct i915_gem_context *gem_context; /** caller's context */ struct i915_address_space *vm; /** GTT and vma for the request */ struct i915_request *request; /** our request to build */ @@ -738,7 +741,7 @@ static int eb_select_context(struct i915_execbuffer *eb) if (unlikely(!ctx)) return -ENOENT; - eb->ctx = ctx; + eb->gem_context = ctx; if (ctx->ppgtt) { eb->vm = &ctx->ppgtt->vm; eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; @@ -784,7 +787,6 @@ static struct i915_request *__eb_wait_for_ring(struct intel_ring *ring) static int eb_wait_for_ring(const struct i915_execbuffer *eb) { - const struct intel_context *ce; struct i915_request *rq; int ret = 0; @@ -794,11 +796,7 @@ static int eb_wait_for_ring(const struct i915_execbuffer *eb) * keeping all of their resources pinned. */ - ce = intel_context_lookup(eb->ctx, eb->engine); - if (!ce || !ce->ring) /* first use, assume empty! */ - return 0; - - rq = __eb_wait_for_ring(ce->ring); + rq = __eb_wait_for_ring(eb->context->ring); if (rq) { mutex_unlock(&eb->i915->drm.struct_mutex); @@ -817,15 +815,15 @@ static int eb_wait_for_ring(const struct i915_execbuffer *eb) static int eb_lookup_vmas(struct i915_execbuffer *eb) { - struct radix_tree_root *handles_vma = &eb->ctx->handles_vma; + struct radix_tree_root *handles_vma = &eb->gem_context->handles_vma; struct drm_i915_gem_object *obj; unsigned int i, batch; int err; - if (unlikely(i915_gem_context_is_closed(eb->ctx))) + if (unlikely(i915_gem_context_is_closed(eb->gem_context))) return -ENOENT; - if (unlikely(i915_gem_context_is_banned(eb->ctx))) + if (unlikely(i915_gem_context_is_banned(eb->gem_context))) return -EIO; INIT_LIST_HEAD(&eb->relocs); @@ -870,8 +868,8 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) if (!vma->open_count++) i915_vma_reopen(vma); list_add(&lut->obj_link, &obj->lut_list); - list_add(&lut->ctx_link, &eb->ctx->handles_list); - lut->ctx = eb->ctx; + list_add(&lut->ctx_link, &eb->gem_context->handles_list); + lut->ctx = eb->gem_context; lut->handle = handle; add_vma: @@ -1227,7 +1225,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb, if (err) goto err_unmap; - rq = i915_request_alloc(eb->engine, eb->ctx); + rq = i915_request_create(eb->context); if (IS_ERR(rq)) { err = PTR_ERR(rq); goto err_unpin; @@ -2088,31 +2086,65 @@ static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { [I915_EXEC_VEBOX] = VECS0 }; -static struct intel_engine_cs * -eb_select_engine(struct drm_i915_private *dev_priv, +static int eb_pin_context(struct i915_execbuffer *eb, + struct intel_engine_cs *engine) +{ + struct intel_context *ce; + int err; + + /* + * ABI: Before userspace accesses the GPU (e.g. execbuffer), report + * EIO if the GPU is already wedged. + */ + err = i915_terminally_wedged(eb->i915); + if (err) + return err; + + /* + * Pinning the contexts may generate requests in order to acquire + * GGTT space, so do this first before we reserve a seqno for + * ourselves. + */ + ce = intel_context_pin(eb->gem_context, engine); + if (IS_ERR(ce)) + return PTR_ERR(ce); + + eb->engine = engine; + eb->context = ce; + return 0; +} + +static void eb_unpin_context(struct i915_execbuffer *eb) +{ + intel_context_unpin(eb->context); +} + +static int +eb_select_engine(struct i915_execbuffer *eb, struct drm_file *file, struct drm_i915_gem_execbuffer2 *args) { + struct drm_i915_private *i915 = eb->i915; unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); - return NULL; + return -EINVAL; } if ((user_ring_id != I915_EXEC_BSD) && ((args->flags & I915_EXEC_BSD_MASK) != 0)) { DRM_DEBUG("execbuf with non bsd ring but with invalid " "bsd dispatch flags: %d\n", (int)(args->flags)); - return NULL; + return -EINVAL; } - if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(dev_priv, VCS1)) { + if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(i915, VCS1)) { unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK; if (bsd_idx == I915_EXEC_BSD_DEFAULT) { - bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file); + bsd_idx = gen8_dispatch_bsd_engine(i915, file); } else if (bsd_idx >= I915_EXEC_BSD_RING1 && bsd_idx <= I915_EXEC_BSD_RING2) { bsd_idx >>= I915_EXEC_BSD_SHIFT; @@ -2120,20 +2152,20 @@ eb_select_engine(struct drm_i915_private *dev_priv, } else { DRM_DEBUG("execbuf with unknown bsd ring: %u\n", bsd_idx); - return NULL; + return -EINVAL; } - engine = dev_priv->engine[_VCS(bsd_idx)]; + engine = i915->engine[_VCS(bsd_idx)]; } else { - engine = dev_priv->engine[user_ring_map[user_ring_id]]; + engine = i915->engine[user_ring_map[user_ring_id]]; } if (!engine) { DRM_DEBUG("execbuf with invalid ring: %u\n", user_ring_id); - return NULL; + return -EINVAL; } - return engine; + return eb_pin_context(eb, engine); } static void @@ -2275,7 +2307,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, struct i915_execbuffer eb; struct dma_fence *in_fence = NULL; struct sync_file *out_fence = NULL; - intel_wakeref_t wakeref; int out_fence_fd = -1; int err; @@ -2335,12 +2366,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (unlikely(err)) goto err_destroy; - eb.engine = eb_select_engine(eb.i915, file, args); - if (!eb.engine) { - err = -EINVAL; - goto err_engine; - } - /* * Take a local wakeref for preparing to dispatch the execbuf as * we expect to access the hardware fairly frequently in the @@ -2348,16 +2373,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, * wakeref that we hold until the GPU has been idle for at least * 100ms. */ - wakeref = intel_runtime_pm_get(eb.i915); + intel_gt_pm_get(eb.i915); err = i915_mutex_lock_interruptible(dev); if (err) goto err_rpm; - err = eb_wait_for_ring(&eb); /* may temporarily drop struct_mutex */ + err = eb_select_engine(&eb, file, args); if (unlikely(err)) goto err_unlock; + err = eb_wait_for_ring(&eb); /* may temporarily drop struct_mutex */ + if (unlikely(err)) + goto err_engine; + err = eb_relocate(&eb); if (err) { /* @@ -2441,7 +2470,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, GEM_BUG_ON(eb.reloc_cache.rq); /* Allocate a request for this batch buffer nice and early. */ - eb.request = i915_request_alloc(eb.engine, eb.ctx); + eb.request = i915_request_create(eb.context); if (IS_ERR(eb.request)) { err = PTR_ERR(eb.request); goto err_batch_unpin; @@ -2479,8 +2508,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, trace_i915_request_queue(eb.request, eb.batch_flags); err = eb_submit(&eb); err_request: - i915_request_add(eb.request); add_to_client(eb.request, file); + i915_request_add(eb.request); if (fences) signal_fence_array(&eb, fences); @@ -2502,12 +2531,13 @@ err_batch_unpin: err_vma: if (eb.exec) eb_release_vmas(&eb); +err_engine: + eb_unpin_context(&eb); err_unlock: mutex_unlock(&dev->struct_mutex); err_rpm: - intel_runtime_pm_put(eb.i915, wakeref); -err_engine: - i915_gem_context_put(eb.ctx); + intel_gt_pm_put(eb.i915); + i915_gem_context_put(eb.gem_context); err_destroy: eb_destroy(&eb); err_out_fence: -- cgit v1.2.3 From fa9f668141f4e5590837845ffc1dc4f5aca7a0a5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 26 Apr 2019 17:33:29 +0100 Subject: drm/i915: Export intel_context_instance() We want to pass in a intel_context into intel_context_pin() and that requires us to first be able to lookup the intel_context! Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190426163336.15906-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_context.c | 37 +++++++++++++++--------------- drivers/gpu/drm/i915/gt/intel_context.h | 19 ++++++++++----- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 ++++++- drivers/gpu/drm/i915/gt/mock_engine.c | 8 ++++++- drivers/gpu/drm/i915/gvt/scheduler.c | 7 +++++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 ++++++--- drivers/gpu/drm/i915/i915_perf.c | 21 +++++++++++------ drivers/gpu/drm/i915/i915_request.c | 11 ++++++++- 8 files changed, 83 insertions(+), 39 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 298e463ad082..8b386202b374 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -104,7 +104,7 @@ void __intel_context_remove(struct intel_context *ce) spin_unlock(&ctx->hw_contexts_lock); } -static struct intel_context * +struct intel_context * intel_context_instance(struct i915_gem_context *ctx, struct intel_engine_cs *engine) { @@ -112,7 +112,7 @@ intel_context_instance(struct i915_gem_context *ctx, ce = intel_context_lookup(ctx, engine); if (likely(ce)) - return ce; + return intel_context_get(ce); ce = intel_context_alloc(); if (!ce) @@ -125,7 +125,7 @@ intel_context_instance(struct i915_gem_context *ctx, intel_context_free(ce); GEM_BUG_ON(intel_context_lookup(ctx, engine) != pos); - return pos; + return intel_context_get(pos); } struct intel_context * @@ -139,30 +139,30 @@ intel_context_pin_lock(struct i915_gem_context *ctx, if (IS_ERR(ce)) return ce; - if (mutex_lock_interruptible(&ce->pin_mutex)) + if (mutex_lock_interruptible(&ce->pin_mutex)) { + intel_context_put(ce); return ERR_PTR(-EINTR); + } return ce; } -struct intel_context * -intel_context_pin(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) +void intel_context_pin_unlock(struct intel_context *ce) + __releases(ce->pin_mutex) { - struct intel_context *ce; - int err; - - ce = intel_context_instance(ctx, engine); - if (IS_ERR(ce)) - return ce; + mutex_unlock(&ce->pin_mutex); + intel_context_put(ce); +} - if (likely(atomic_inc_not_zero(&ce->pin_count))) - return ce; +int __intel_context_do_pin(struct intel_context *ce) +{ + int err; if (mutex_lock_interruptible(&ce->pin_mutex)) - return ERR_PTR(-EINTR); + return -EINTR; if (likely(!atomic_read(&ce->pin_count))) { + struct i915_gem_context *ctx = ce->gem_context; intel_wakeref_t wakeref; err = 0; @@ -172,7 +172,6 @@ intel_context_pin(struct i915_gem_context *ctx, goto err; i915_gem_context_get(ctx); - GEM_BUG_ON(ce->gem_context != ctx); mutex_lock(&ctx->mutex); list_add(&ce->active_link, &ctx->active_engines); @@ -186,11 +185,11 @@ intel_context_pin(struct i915_gem_context *ctx, GEM_BUG_ON(!intel_context_is_pinned(ce)); /* no overflow! */ mutex_unlock(&ce->pin_mutex); - return ce; + return 0; err: mutex_unlock(&ce->pin_mutex); - return ERR_PTR(err); + return err; } void intel_context_unpin(struct intel_context *ce) diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index 60379eb37949..b9a574587eb3 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -49,11 +49,7 @@ intel_context_is_pinned(struct intel_context *ce) return atomic_read(&ce->pin_count); } -static inline void intel_context_pin_unlock(struct intel_context *ce) -__releases(ce->pin_mutex) -{ - mutex_unlock(&ce->pin_mutex); -} +void intel_context_pin_unlock(struct intel_context *ce); struct intel_context * __intel_context_insert(struct i915_gem_context *ctx, @@ -63,7 +59,18 @@ void __intel_context_remove(struct intel_context *ce); struct intel_context * -intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine); +intel_context_instance(struct i915_gem_context *ctx, + struct intel_engine_cs *engine); + +int __intel_context_do_pin(struct intel_context *ce); + +static inline int intel_context_pin(struct intel_context *ce) +{ + if (likely(atomic_inc_not_zero(&ce->pin_count))) + return 0; + + return __intel_context_do_pin(ce); +} static inline void __intel_context_pin(struct intel_context *ce) { diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 7c899984b0d4..5a6b81836902 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -713,11 +713,17 @@ static int pin_context(struct i915_gem_context *ctx, struct intel_context **out) { struct intel_context *ce; + int err; - ce = intel_context_pin(ctx, engine); + ce = intel_context_instance(ctx, engine); if (IS_ERR(ce)) return PTR_ERR(ce); + err = intel_context_pin(ce); + intel_context_put(ce); + if (err) + return err; + *out = ce; return 0; } diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index a97a0ab35703..21f413ff5b8e 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -239,6 +239,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, int id) { struct mock_engine *engine; + int err; GEM_BUG_ON(id >= I915_NUM_ENGINES); @@ -278,10 +279,15 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, INIT_LIST_HEAD(&engine->hw_queue); engine->base.kernel_context = - intel_context_pin(i915->kernel_context, &engine->base); + intel_context_instance(i915->kernel_context, &engine->base); if (IS_ERR(engine->base.kernel_context)) goto err_breadcrumbs; + err = intel_context_pin(engine->base.kernel_context); + intel_context_put(engine->base.kernel_context); + if (err) + goto err_breadcrumbs; + return &engine->base; err_breadcrumbs: diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 5da50201eb41..da6b52de5b16 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -1183,12 +1183,17 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu) INIT_LIST_HEAD(&s->workload_q_head[i]); s->shadow[i] = ERR_PTR(-EINVAL); - ce = intel_context_pin(ctx, engine); + ce = intel_context_instance(ctx, engine); if (IS_ERR(ce)) { ret = PTR_ERR(ce); goto out_shadow_ctx; } + ret = intel_context_pin(ce); + intel_context_put(ce); + if (ret) + goto out_shadow_ctx; + s->shadow[i] = ce; } diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 794af8edc6a2..166a33c0d3ed 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2100,14 +2100,19 @@ static int eb_pin_context(struct i915_execbuffer *eb, if (err) return err; + ce = intel_context_instance(eb->gem_context, engine); + if (IS_ERR(ce)) + return PTR_ERR(ce); + /* * Pinning the contexts may generate requests in order to acquire * GGTT space, so do this first before we reserve a seqno for * ourselves. */ - ce = intel_context_pin(eb->gem_context, engine); - if (IS_ERR(ce)) - return PTR_ERR(ce); + err = intel_context_pin(ce); + intel_context_put(ce); + if (err) + return err; eb->engine = engine; eb->context = ce; diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 328a740e72cb..afaeabe5e531 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1205,11 +1205,17 @@ static struct intel_context *oa_pin_context(struct drm_i915_private *i915, { struct intel_engine_cs *engine = i915->engine[RCS0]; struct intel_context *ce; - int ret; + int err; - ret = i915_mutex_lock_interruptible(&i915->drm); - if (ret) - return ERR_PTR(ret); + ce = intel_context_instance(ctx, engine); + if (IS_ERR(ce)) + return ce; + + err = i915_mutex_lock_interruptible(&i915->drm); + if (err) { + intel_context_put(ce); + return ERR_PTR(err); + } /* * As the ID is the gtt offset of the context's vma we @@ -1217,10 +1223,11 @@ static struct intel_context *oa_pin_context(struct drm_i915_private *i915, * * NB: implied RCS engine... */ - ce = intel_context_pin(ctx, engine); + err = intel_context_pin(ce); mutex_unlock(&i915->drm.struct_mutex); - if (IS_ERR(ce)) - return ce; + intel_context_put(ce); + if (err) + return ERR_PTR(err); i915->perf.oa.pinned_ctx = ce; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5869c37a35e1..1a03ebcaf52e 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -785,6 +785,7 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) struct drm_i915_private *i915 = engine->i915; struct intel_context *ce; struct i915_request *rq; + int err; /* * Preempt contexts are reserved for exclusive use to inject a @@ -798,13 +799,21 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) * GGTT space, so do this first before we reserve a seqno for * ourselves. */ - ce = intel_context_pin(ctx, engine); + ce = intel_context_instance(ctx, engine); if (IS_ERR(ce)) return ERR_CAST(ce); + err = intel_context_pin(ce); + if (err) { + rq = ERR_PTR(err); + goto err_put; + } + rq = i915_request_create(ce); intel_context_unpin(ce); +err_put: + intel_context_put(ce); return rq; } -- cgit v1.2.3 From 5e2a0419ef7cb25d0f9a5fd6a62372bb47ce948d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 26 Apr 2019 17:33:34 +0100 Subject: drm/i915: Switch back to an array of logical per-engine HW contexts We switched to a tree of per-engine HW context to accommodate the introduction of virtual engines. However, we plan to also support multiple instances of the same engine within the GEM context, defeating our use of the engine as a key to looking up the HW context. Just allocate a logical per-engine instance and always use an index into the ctx->engines[]. Later on, this ctx->engines[] may be replaced by a user specified map. v2: Add for_each_gem_engine() helper to iterator within the engines lock v3: intel_context_create_request() helper v4: s/unsigned long/unsigned int/ 4 billion engines is quite enough. v5: Push iterator locking to caller Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190426163336.15906-7-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/gt/intel_context.c | 112 ++++------------------ drivers/gpu/drm/i915/gt/intel_context.h | 27 +----- drivers/gpu/drm/i915/gt/intel_context_types.h | 2 - drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/mock_engine.c | 3 +- drivers/gpu/drm/i915/gvt/scheduler.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 24 ++--- drivers/gpu/drm/i915/i915_gem_context.c | 96 ++++++++++++++++--- drivers/gpu/drm/i915/i915_gem_context.h | 58 +++++++++++ drivers/gpu/drm/i915/i915_gem_context_types.h | 40 +++++++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 70 +++++++------- drivers/gpu/drm/i915/i915_perf.c | 80 +++++++++------- drivers/gpu/drm/i915/i915_request.c | 15 +-- drivers/gpu/drm/i915/intel_guc_submission.c | 22 +++-- drivers/gpu/drm/i915/selftests/i915_gem_context.c | 2 +- drivers/gpu/drm/i915/selftests/mock_context.c | 14 ++- 16 files changed, 328 insertions(+), 241 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 15ac99c5dd4a..5e506e648454 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -17,7 +17,7 @@ static struct i915_global_context { struct kmem_cache *slab_ce; } global; -struct intel_context *intel_context_alloc(void) +static struct intel_context *intel_context_alloc(void) { return kmem_cache_zalloc(global.slab_ce, GFP_KERNEL); } @@ -28,104 +28,17 @@ void intel_context_free(struct intel_context *ce) } struct intel_context * -intel_context_lookup(struct i915_gem_context *ctx, +intel_context_create(struct i915_gem_context *ctx, struct intel_engine_cs *engine) { - struct intel_context *ce = NULL; - struct rb_node *p; - - spin_lock(&ctx->hw_contexts_lock); - p = ctx->hw_contexts.rb_node; - while (p) { - struct intel_context *this = - rb_entry(p, struct intel_context, node); - - if (this->engine == engine) { - GEM_BUG_ON(this->gem_context != ctx); - ce = this; - break; - } - - if (this->engine < engine) - p = p->rb_right; - else - p = p->rb_left; - } - spin_unlock(&ctx->hw_contexts_lock); - - return ce; -} - -struct intel_context * -__intel_context_insert(struct i915_gem_context *ctx, - struct intel_engine_cs *engine, - struct intel_context *ce) -{ - struct rb_node **p, *parent; - int err = 0; - - spin_lock(&ctx->hw_contexts_lock); - - parent = NULL; - p = &ctx->hw_contexts.rb_node; - while (*p) { - struct intel_context *this; - - parent = *p; - this = rb_entry(parent, struct intel_context, node); - - if (this->engine == engine) { - err = -EEXIST; - ce = this; - break; - } - - if (this->engine < engine) - p = &parent->rb_right; - else - p = &parent->rb_left; - } - if (!err) { - rb_link_node(&ce->node, parent, p); - rb_insert_color(&ce->node, &ctx->hw_contexts); - } - - spin_unlock(&ctx->hw_contexts_lock); - - return ce; -} - -void __intel_context_remove(struct intel_context *ce) -{ - struct i915_gem_context *ctx = ce->gem_context; - - spin_lock(&ctx->hw_contexts_lock); - rb_erase(&ce->node, &ctx->hw_contexts); - spin_unlock(&ctx->hw_contexts_lock); -} - -struct intel_context * -intel_context_instance(struct i915_gem_context *ctx, - struct intel_engine_cs *engine) -{ - struct intel_context *ce, *pos; - - ce = intel_context_lookup(ctx, engine); - if (likely(ce)) - return intel_context_get(ce); + struct intel_context *ce; ce = intel_context_alloc(); if (!ce) return ERR_PTR(-ENOMEM); intel_context_init(ce, ctx, engine); - - pos = __intel_context_insert(ctx, engine, ce); - if (unlikely(pos != ce)) /* Beaten! Use their HW context instead */ - intel_context_free(ce); - - GEM_BUG_ON(intel_context_lookup(ctx, engine) != pos); - return intel_context_get(pos); + return ce; } int __intel_context_do_pin(struct intel_context *ce) @@ -204,6 +117,8 @@ intel_context_init(struct intel_context *ce, struct i915_gem_context *ctx, struct intel_engine_cs *engine) { + GEM_BUG_ON(!engine->cops); + kref_init(&ce->ref); ce->gem_context = ctx; @@ -254,3 +169,18 @@ void intel_context_exit_engine(struct intel_context *ce) { intel_engine_pm_put(ce->engine); } + +struct i915_request *intel_context_create_request(struct intel_context *ce) +{ + struct i915_request *rq; + int err; + + err = intel_context_pin(ce); + if (unlikely(err)) + return ERR_PTR(err); + + rq = i915_request_create(ce); + intel_context_unpin(ce); + + return rq; +} diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index b746add6b71d..63392c88cd98 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -12,24 +12,16 @@ #include "intel_context_types.h" #include "intel_engine_types.h" -struct intel_context *intel_context_alloc(void); -void intel_context_free(struct intel_context *ce); - void intel_context_init(struct intel_context *ce, struct i915_gem_context *ctx, struct intel_engine_cs *engine); -/** - * intel_context_lookup - Find the matching HW context for this (ctx, engine) - * @ctx - the parent GEM context - * @engine - the target HW engine - * - * May return NULL if the HW context hasn't been instantiated (i.e. unused). - */ struct intel_context * -intel_context_lookup(struct i915_gem_context *ctx, +intel_context_create(struct i915_gem_context *ctx, struct intel_engine_cs *engine); +void intel_context_free(struct intel_context *ce); + /** * intel_context_lock_pinned - Stablises the 'pinned' status of the HW context * @ce - the context @@ -71,17 +63,6 @@ static inline void intel_context_unlock_pinned(struct intel_context *ce) mutex_unlock(&ce->pin_mutex); } -struct intel_context * -__intel_context_insert(struct i915_gem_context *ctx, - struct intel_engine_cs *engine, - struct intel_context *ce); -void -__intel_context_remove(struct intel_context *ce); - -struct intel_context * -intel_context_instance(struct i915_gem_context *ctx, - struct intel_engine_cs *engine); - int __intel_context_do_pin(struct intel_context *ce); static inline int intel_context_pin(struct intel_context *ce) @@ -144,4 +125,6 @@ static inline void intel_context_timeline_unlock(struct intel_context *ce) mutex_unlock(&ce->ring->timeline->mutex); } +struct i915_request *intel_context_create_request(struct intel_context *ce); + #endif /* __INTEL_CONTEXT_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index f02d27734e3b..3579c2708321 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -10,7 +10,6 @@ #include #include #include -#include #include #include "i915_active_types.h" @@ -61,7 +60,6 @@ struct intel_context { struct i915_active_request active_tracker; const struct intel_context_ops *ops; - struct rb_node node; /** sseu: Control eu/slice partitioning */ struct intel_sseu sseu; diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 7682f16fa567..f7308479d511 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -732,7 +732,7 @@ static int pin_context(struct i915_gem_context *ctx, struct intel_context *ce; int err; - ce = intel_context_instance(ctx, engine); + ce = i915_gem_context_get_engine(ctx, engine->id); if (IS_ERR(ce)) return PTR_ERR(ce); diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index 85cdbfe1d989..2941916b37bf 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -23,6 +23,7 @@ */ #include "i915_drv.h" +#include "i915_gem_context.h" #include "intel_context.h" #include "intel_engine_pm.h" @@ -286,7 +287,7 @@ int mock_engine_init(struct intel_engine_cs *engine) i915_timeline_set_subclass(&engine->timeline, TIMELINE_ENGINE); engine->kernel_context = - intel_context_instance(i915->kernel_context, engine); + i915_gem_context_get_engine(i915->kernel_context, engine->id); if (IS_ERR(engine->kernel_context)) goto err_timeline; diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index da6b52de5b16..7ae42f2ebfe8 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -1183,7 +1183,7 @@ int intel_vgpu_setup_submission(struct intel_vgpu *vgpu) INIT_LIST_HEAD(&s->workload_q_head[i]); s->shadow[i] = ERR_PTR(-EINVAL); - ce = intel_context_instance(ctx, engine); + ce = i915_gem_context_get_engine(ctx, i); if (IS_ERR(ce)) { ret = PTR_ERR(ce); goto out_shadow_ctx; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 08c66e76d712..4c1793b1012e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4289,8 +4289,9 @@ out: static int __intel_engines_record_defaults(struct drm_i915_private *i915) { - struct i915_gem_context *ctx; struct intel_engine_cs *engine; + struct i915_gem_context *ctx; + struct i915_gem_engines *e; enum intel_engine_id id; int err = 0; @@ -4307,18 +4308,21 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) if (IS_ERR(ctx)) return PTR_ERR(ctx); + e = i915_gem_context_lock_engines(ctx); + for_each_engine(engine, i915, id) { + struct intel_context *ce = e->engines[id]; struct i915_request *rq; - rq = i915_request_alloc(engine, ctx); + rq = intel_context_create_request(ce); if (IS_ERR(rq)) { err = PTR_ERR(rq); - goto out_ctx; + goto err_active; } err = 0; - if (engine->init_context) - err = engine->init_context(rq); + if (rq->engine->init_context) + err = rq->engine->init_context(rq); i915_request_add(rq); if (err) @@ -4332,15 +4336,10 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) } for_each_engine(engine, i915, id) { - struct intel_context *ce; - struct i915_vma *state; + struct intel_context *ce = e->engines[id]; + struct i915_vma *state = ce->state; void *vaddr; - ce = intel_context_lookup(ctx, engine); - if (!ce) - continue; - - state = ce->state; if (!state) continue; @@ -4396,6 +4395,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915) } out_ctx: + i915_gem_context_unlock_engines(ctx); i915_gem_context_set_closed(ctx); i915_gem_context_put(ctx); return err; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index d9db3fea151c..3ea199ca834b 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -150,7 +150,7 @@ lookup_user_engine(struct i915_gem_context *ctx, u16 class, u16 instance) if (!engine) return ERR_PTR(-EINVAL); - return intel_context_instance(ctx, engine); + return i915_gem_context_get_engine(ctx, engine->id); } static inline int new_hw_id(struct drm_i915_private *i915, gfp_t gfp) @@ -242,10 +242,51 @@ static void release_hw_id(struct i915_gem_context *ctx) mutex_unlock(&i915->contexts.mutex); } -static void i915_gem_context_free(struct i915_gem_context *ctx) +static void __free_engines(struct i915_gem_engines *e, unsigned int count) { - struct intel_context *it, *n; + while (count--) { + if (!e->engines[count]) + continue; + + intel_context_put(e->engines[count]); + } + kfree(e); +} + +static void free_engines(struct i915_gem_engines *e) +{ + __free_engines(e, e->num_engines); +} + +static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx) +{ + struct intel_engine_cs *engine; + struct i915_gem_engines *e; + enum intel_engine_id id; + + e = kzalloc(struct_size(e, engines, I915_NUM_ENGINES), GFP_KERNEL); + if (!e) + return ERR_PTR(-ENOMEM); + + e->i915 = ctx->i915; + for_each_engine(engine, ctx->i915, id) { + struct intel_context *ce; + + ce = intel_context_create(ctx, engine); + if (IS_ERR(ce)) { + __free_engines(e, id); + return ERR_CAST(ce); + } + e->engines[id] = ce; + } + e->num_engines = id; + + return e; +} + +static void i915_gem_context_free(struct i915_gem_context *ctx) +{ lockdep_assert_held(&ctx->i915->drm.struct_mutex); GEM_BUG_ON(!i915_gem_context_is_closed(ctx)); GEM_BUG_ON(!list_empty(&ctx->active_engines)); @@ -253,8 +294,8 @@ static void i915_gem_context_free(struct i915_gem_context *ctx) release_hw_id(ctx); i915_ppgtt_put(ctx->ppgtt); - rbtree_postorder_for_each_entry_safe(it, n, &ctx->hw_contexts, node) - intel_context_put(it); + free_engines(rcu_access_pointer(ctx->engines)); + mutex_destroy(&ctx->engines_mutex); if (ctx->timeline) i915_timeline_put(ctx->timeline); @@ -363,6 +404,8 @@ static struct i915_gem_context * __create_context(struct drm_i915_private *dev_priv) { struct i915_gem_context *ctx; + struct i915_gem_engines *e; + int err; int i; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); @@ -376,8 +419,13 @@ __create_context(struct drm_i915_private *dev_priv) INIT_LIST_HEAD(&ctx->active_engines); mutex_init(&ctx->mutex); - ctx->hw_contexts = RB_ROOT; - spin_lock_init(&ctx->hw_contexts_lock); + mutex_init(&ctx->engines_mutex); + e = default_engines(ctx); + if (IS_ERR(e)) { + err = PTR_ERR(e); + goto err_free; + } + RCU_INIT_POINTER(ctx->engines, e); INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); INIT_LIST_HEAD(&ctx->handles_list); @@ -399,6 +447,10 @@ __create_context(struct drm_i915_private *dev_priv) ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES; return ctx; + +err_free: + kfree(ctx); + return ERR_PTR(err); } static struct i915_hw_ppgtt * @@ -857,7 +909,8 @@ static int context_barrier_task(struct i915_gem_context *ctx, { struct drm_i915_private *i915 = ctx->i915; struct context_barrier_task *cb; - struct intel_context *ce, *next; + struct i915_gem_engines_iter it; + struct intel_context *ce; int err = 0; lockdep_assert_held(&i915->drm.struct_mutex); @@ -870,20 +923,19 @@ static int context_barrier_task(struct i915_gem_context *ctx, i915_active_init(i915, &cb->base, cb_retire); i915_active_acquire(&cb->base); - rbtree_postorder_for_each_entry_safe(ce, next, &ctx->hw_contexts, node) { - struct intel_engine_cs *engine = ce->engine; + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { struct i915_request *rq; - if (!(engine->mask & engines)) + if (!(ce->engine->mask & engines)) continue; if (I915_SELFTEST_ONLY(context_barrier_inject_fault & - engine->mask)) { + ce->engine->mask)) { err = -ENXIO; break; } - rq = i915_request_alloc(engine, ctx); + rq = intel_context_create_request(ce); if (IS_ERR(rq)) { err = PTR_ERR(rq); break; @@ -899,6 +951,7 @@ static int context_barrier_task(struct i915_gem_context *ctx, if (err) break; } + i915_gem_context_unlock_engines(ctx); cb->task = err ? NULL : task; /* caller needs to unwind instead */ cb->data = data; @@ -1729,6 +1782,23 @@ out_unlock: return err; } +/* GEM context-engines iterator: for_each_gem_engine() */ +struct intel_context * +i915_gem_engines_iter_next(struct i915_gem_engines_iter *it) +{ + const struct i915_gem_engines *e = it->engines; + struct intel_context *ctx; + + do { + if (it->idx >= e->num_engines) + return NULL; + + ctx = e->engines[it->idx++]; + } while (!ctx); + + return ctx; +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/mock_context.c" #include "selftests/i915_gem_context.c" diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 5a8e080499fb..272e183ebc0c 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -176,6 +176,64 @@ static inline void i915_gem_context_put(struct i915_gem_context *ctx) kref_put(&ctx->ref, i915_gem_context_release); } +static inline struct i915_gem_engines * +i915_gem_context_engines(struct i915_gem_context *ctx) +{ + return rcu_dereference_protected(ctx->engines, + lockdep_is_held(&ctx->engines_mutex)); +} + +static inline struct i915_gem_engines * +i915_gem_context_lock_engines(struct i915_gem_context *ctx) + __acquires(&ctx->engines_mutex) +{ + mutex_lock(&ctx->engines_mutex); + return i915_gem_context_engines(ctx); +} + +static inline void +i915_gem_context_unlock_engines(struct i915_gem_context *ctx) + __releases(&ctx->engines_mutex) +{ + mutex_unlock(&ctx->engines_mutex); +} + +static inline struct intel_context * +i915_gem_context_lookup_engine(struct i915_gem_context *ctx, unsigned int idx) +{ + return i915_gem_context_engines(ctx)->engines[idx]; +} + +static inline struct intel_context * +i915_gem_context_get_engine(struct i915_gem_context *ctx, unsigned int idx) +{ + struct intel_context *ce = ERR_PTR(-EINVAL); + + rcu_read_lock(); { + struct i915_gem_engines *e = rcu_dereference(ctx->engines); + if (likely(idx < e->num_engines && e->engines[idx])) + ce = intel_context_get(e->engines[idx]); + } rcu_read_unlock(); + + return ce; +} + +static inline void +i915_gem_engines_iter_init(struct i915_gem_engines_iter *it, + struct i915_gem_engines *engines) +{ + GEM_BUG_ON(!engines); + it->engines = engines; + it->idx = 0; +} + +struct intel_context * +i915_gem_engines_iter_next(struct i915_gem_engines_iter *it); + +#define for_each_gem_engine(ce, engines, it) \ + for (i915_gem_engines_iter_init(&(it), (engines)); \ + ((ce) = i915_gem_engines_iter_next(&(it)));) + struct i915_lut_handle *i915_lut_handle_alloc(void); void i915_lut_handle_free(struct i915_lut_handle *lut); diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h b/drivers/gpu/drm/i915/i915_gem_context_types.h index d282a6ab3b9f..5f84618cf7db 100644 --- a/drivers/gpu/drm/i915/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/i915_gem_context_types.h @@ -29,6 +29,18 @@ struct i915_hw_ppgtt; struct i915_timeline; struct intel_ring; +struct i915_gem_engines { + struct rcu_work rcu; + struct drm_i915_private *i915; + unsigned int num_engines; + struct intel_context *engines[]; +}; + +struct i915_gem_engines_iter { + unsigned int idx; + const struct i915_gem_engines *engines; +}; + /** * struct i915_gem_context - client state * @@ -42,6 +54,30 @@ struct i915_gem_context { /** file_priv: owning file descriptor */ struct drm_i915_file_private *file_priv; + /** + * @engines: User defined engines for this context + * + * Various uAPI offer the ability to lookup up an + * index from this array to select an engine operate on. + * + * Multiple logically distinct instances of the same engine + * may be defined in the array, as well as composite virtual + * engines. + * + * Execbuf uses the I915_EXEC_RING_MASK as an index into this + * array to select which HW context + engine to execute on. For + * the default array, the user_ring_map[] is used to translate + * the legacy uABI onto the approprate index (e.g. both + * I915_EXEC_DEFAULT and I915_EXEC_RENDER select the same + * context, and I915_EXEC_BSD is weird). For a use defined + * array, execbuf uses I915_EXEC_RING_MASK as a plain index. + * + * User defined by I915_CONTEXT_PARAM_ENGINE (when the + * CONTEXT_USER_ENGINES flag is set). + */ + struct i915_gem_engines __rcu *engines; + struct mutex engines_mutex; /* guards writes to engines */ + struct i915_timeline *timeline; /** @@ -134,10 +170,6 @@ struct i915_gem_context { struct i915_sched_attr sched; - /** hw_contexts: per-engine logical HW state */ - struct rb_root hw_contexts; - spinlock_t hw_contexts_lock; - /** ring_size: size for allocating the per-engine ring buffer */ u32 ring_size; /** desc_template: invariant fields for the HW context descriptor */ diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 166a33c0d3ed..679f7c1561ba 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2076,9 +2076,7 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, return file_priv->bsd_engine; } -#define I915_USER_RINGS (4) - -static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { +static const enum intel_engine_id user_ring_map[] = { [I915_EXEC_DEFAULT] = RCS0, [I915_EXEC_RENDER] = RCS0, [I915_EXEC_BLT] = BCS0, @@ -2086,10 +2084,8 @@ static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { [I915_EXEC_VEBOX] = VECS0 }; -static int eb_pin_context(struct i915_execbuffer *eb, - struct intel_engine_cs *engine) +static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce) { - struct intel_context *ce; int err; /* @@ -2100,21 +2096,16 @@ static int eb_pin_context(struct i915_execbuffer *eb, if (err) return err; - ce = intel_context_instance(eb->gem_context, engine); - if (IS_ERR(ce)) - return PTR_ERR(ce); - /* * Pinning the contexts may generate requests in order to acquire * GGTT space, so do this first before we reserve a seqno for * ourselves. */ err = intel_context_pin(ce); - intel_context_put(ce); if (err) return err; - eb->engine = engine; + eb->engine = ce->engine; eb->context = ce; return 0; } @@ -2124,25 +2115,19 @@ static void eb_unpin_context(struct i915_execbuffer *eb) intel_context_unpin(eb->context); } -static int -eb_select_engine(struct i915_execbuffer *eb, - struct drm_file *file, - struct drm_i915_gem_execbuffer2 *args) +static unsigned int +eb_select_legacy_ring(struct i915_execbuffer *eb, + struct drm_file *file, + struct drm_i915_gem_execbuffer2 *args) { struct drm_i915_private *i915 = eb->i915; unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; - struct intel_engine_cs *engine; - - if (user_ring_id > I915_USER_RINGS) { - DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); - return -EINVAL; - } - if ((user_ring_id != I915_EXEC_BSD) && - ((args->flags & I915_EXEC_BSD_MASK) != 0)) { + if (user_ring_id != I915_EXEC_BSD && + (args->flags & I915_EXEC_BSD_MASK)) { DRM_DEBUG("execbuf with non bsd ring but with invalid " "bsd dispatch flags: %d\n", (int)(args->flags)); - return -EINVAL; + return -1; } if (user_ring_id == I915_EXEC_BSD && HAS_ENGINE(i915, VCS1)) { @@ -2157,20 +2142,39 @@ eb_select_engine(struct i915_execbuffer *eb, } else { DRM_DEBUG("execbuf with unknown bsd ring: %u\n", bsd_idx); - return -EINVAL; + return -1; } - engine = i915->engine[_VCS(bsd_idx)]; - } else { - engine = i915->engine[user_ring_map[user_ring_id]]; + return _VCS(bsd_idx); } - if (!engine) { - DRM_DEBUG("execbuf with invalid ring: %u\n", user_ring_id); - return -EINVAL; + if (user_ring_id >= ARRAY_SIZE(user_ring_map)) { + DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); + return -1; } - return eb_pin_context(eb, engine); + return user_ring_map[user_ring_id]; +} + +static int +eb_select_engine(struct i915_execbuffer *eb, + struct drm_file *file, + struct drm_i915_gem_execbuffer2 *args) +{ + struct intel_context *ce; + unsigned int idx; + int err; + + idx = eb_select_legacy_ring(eb, file, args); + + ce = i915_gem_context_get_engine(eb->gem_context, idx); + if (IS_ERR(ce)) + return PTR_ERR(ce); + + err = eb_pin_context(eb, ce); + intel_context_put(ce); + + return err; } static void diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index afaeabe5e531..c4995d5a16d2 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1203,35 +1203,35 @@ static int i915_oa_read(struct i915_perf_stream *stream, static struct intel_context *oa_pin_context(struct drm_i915_private *i915, struct i915_gem_context *ctx) { - struct intel_engine_cs *engine = i915->engine[RCS0]; + struct i915_gem_engines_iter it; struct intel_context *ce; int err; - ce = intel_context_instance(ctx, engine); - if (IS_ERR(ce)) - return ce; - err = i915_mutex_lock_interruptible(&i915->drm); - if (err) { - intel_context_put(ce); + if (err) return ERR_PTR(err); + + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { + if (ce->engine->class != RENDER_CLASS) + continue; + + /* + * As the ID is the gtt offset of the context's vma we + * pin the vma to ensure the ID remains fixed. + */ + err = intel_context_pin(ce); + if (err == 0) { + i915->perf.oa.pinned_ctx = ce; + break; + } } + i915_gem_context_unlock_engines(ctx); - /* - * As the ID is the gtt offset of the context's vma we - * pin the vma to ensure the ID remains fixed. - * - * NB: implied RCS engine... - */ - err = intel_context_pin(ce); mutex_unlock(&i915->drm.struct_mutex); - intel_context_put(ce); if (err) return ERR_PTR(err); - i915->perf.oa.pinned_ctx = ce; - - return ce; + return i915->perf.oa.pinned_ctx; } /** @@ -1717,7 +1717,6 @@ gen8_update_reg_state_unlocked(struct intel_context *ce, static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, const struct i915_oa_config *oa_config) { - struct intel_engine_cs *engine = dev_priv->engine[RCS0]; unsigned int map_type = i915_coherent_map_type(dev_priv); struct i915_gem_context *ctx; struct i915_request *rq; @@ -1746,30 +1745,43 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, /* Update all contexts now that we've stalled the submission. */ list_for_each_entry(ctx, &dev_priv->contexts.list, link) { - struct intel_context *ce = intel_context_lookup(ctx, engine); - u32 *regs; - - /* OA settings will be set upon first use */ - if (!ce || !ce->state) - continue; - - regs = i915_gem_object_pin_map(ce->state->obj, map_type); - if (IS_ERR(regs)) - return PTR_ERR(regs); + struct i915_gem_engines_iter it; + struct intel_context *ce; + + for_each_gem_engine(ce, + i915_gem_context_lock_engines(ctx), + it) { + u32 *regs; + + if (ce->engine->class != RENDER_CLASS) + continue; + + /* OA settings will be set upon first use */ + if (!ce->state) + continue; + + regs = i915_gem_object_pin_map(ce->state->obj, + map_type); + if (IS_ERR(regs)) { + i915_gem_context_unlock_engines(ctx); + return PTR_ERR(regs); + } - ce->state->obj->mm.dirty = true; - regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs); + ce->state->obj->mm.dirty = true; + regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs); - gen8_update_reg_state_unlocked(ce, regs, oa_config); + gen8_update_reg_state_unlocked(ce, regs, oa_config); - i915_gem_object_unpin_map(ce->state->obj); + i915_gem_object_unpin_map(ce->state->obj); + } + i915_gem_context_unlock_engines(ctx); } /* * Apply the configuration by doing one context restore of the edited * context image. */ - rq = i915_request_create(engine->kernel_context); + rq = i915_request_create(dev_priv->engine[RCS0]->kernel_context); if (IS_ERR(rq)) return PTR_ERR(rq); diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 1a03ebcaf52e..7638a5e5ec9e 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -785,7 +785,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) struct drm_i915_private *i915 = engine->i915; struct intel_context *ce; struct i915_request *rq; - int err; /* * Preempt contexts are reserved for exclusive use to inject a @@ -799,21 +798,13 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx) * GGTT space, so do this first before we reserve a seqno for * ourselves. */ - ce = intel_context_instance(ctx, engine); + ce = i915_gem_context_get_engine(ctx, engine->id); if (IS_ERR(ce)) return ERR_CAST(ce); - err = intel_context_pin(ce); - if (err) { - rq = ERR_PTR(err); - goto err_put; - } - - rq = i915_request_create(ce); - intel_context_unpin(ce); - -err_put: + rq = intel_context_create_request(ce); intel_context_put(ce); + return rq; } diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 1b6d6403ee92..4c814344809c 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -364,11 +364,10 @@ static void guc_stage_desc_pool_destroy(struct intel_guc *guc) static void guc_stage_desc_init(struct intel_guc_client *client) { struct intel_guc *guc = client->guc; - struct drm_i915_private *dev_priv = guc_to_i915(guc); - struct intel_engine_cs *engine; struct i915_gem_context *ctx = client->owner; + struct i915_gem_engines_iter it; struct guc_stage_desc *desc; - unsigned int tmp; + struct intel_context *ce; u32 gfx_addr; desc = __get_stage_desc(client); @@ -382,10 +381,11 @@ static void guc_stage_desc_init(struct intel_guc_client *client) desc->priority = client->priority; desc->db_id = client->doorbell_id; - for_each_engine_masked(engine, dev_priv, client->engines, tmp) { - struct intel_context *ce = intel_context_lookup(ctx, engine); - u32 guc_engine_id = engine->guc_id; - struct guc_execlist_context *lrc = &desc->lrc[guc_engine_id]; + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { + struct guc_execlist_context *lrc; + + if (!(ce->engine->mask & client->engines)) + continue; /* TODO: We have a design issue to be solved here. Only when we * receive the first batch, we know which engine is used by the @@ -394,7 +394,7 @@ static void guc_stage_desc_init(struct intel_guc_client *client) * for now who owns a GuC client. But for future owner of GuC * client, need to make sure lrc is pinned prior to enter here. */ - if (!ce || !ce->state) + if (!ce->state) break; /* XXX: continue? */ /* @@ -404,6 +404,7 @@ static void guc_stage_desc_init(struct intel_guc_client *client) * Instead, the GuC uses the LRCA of the user mode context (see * guc_add_request below). */ + lrc = &desc->lrc[ce->engine->guc_id]; lrc->context_desc = lower_32_bits(ce->lrc_desc); /* The state page is after PPHWSP */ @@ -414,15 +415,16 @@ static void guc_stage_desc_init(struct intel_guc_client *client) * here. In proxy submission, it wants the stage id */ lrc->context_id = (client->stage_id << GUC_ELC_CTXID_OFFSET) | - (guc_engine_id << GUC_ELC_ENGINE_OFFSET); + (ce->engine->guc_id << GUC_ELC_ENGINE_OFFSET); lrc->ring_begin = intel_guc_ggtt_offset(guc, ce->ring->vma); lrc->ring_end = lrc->ring_begin + ce->ring->size - 1; lrc->ring_next_free_location = lrc->ring_begin; lrc->ring_current_tail_pointer_value = 0; - desc->engines_used |= (1 << guc_engine_id); + desc->engines_used |= BIT(ce->engine->guc_id); } + i915_gem_context_unlock_engines(ctx); DRM_DEBUG_DRIVER("Host engines 0x%x => GuC engines used 0x%x\n", client->engines, desc->engines_used); diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index 214d1fd2f4dc..7fd224a4ca4c 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -1094,7 +1094,7 @@ __igt_ctx_sseu(struct drm_i915_private *i915, wakeref = intel_runtime_pm_get(i915); - ce = intel_context_instance(ctx, i915->engine[RCS0]); + ce = i915_gem_context_get_engine(ctx, RCS0); if (IS_ERR(ce)) { ret = PTR_ERR(ce); goto out_rpm; diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c index 0426093bf1d9..71c750693585 100644 --- a/drivers/gpu/drm/i915/selftests/mock_context.c +++ b/drivers/gpu/drm/i915/selftests/mock_context.c @@ -30,6 +30,7 @@ mock_context(struct drm_i915_private *i915, const char *name) { struct i915_gem_context *ctx; + struct i915_gem_engines *e; int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); @@ -40,8 +41,11 @@ mock_context(struct drm_i915_private *i915, INIT_LIST_HEAD(&ctx->link); ctx->i915 = i915; - ctx->hw_contexts = RB_ROOT; - spin_lock_init(&ctx->hw_contexts_lock); + mutex_init(&ctx->engines_mutex); + e = default_engines(ctx); + if (IS_ERR(e)) + goto err_free; + RCU_INIT_POINTER(ctx->engines, e); INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); INIT_LIST_HEAD(&ctx->handles_list); @@ -51,7 +55,7 @@ mock_context(struct drm_i915_private *i915, ret = i915_gem_context_pin_hw_id(ctx); if (ret < 0) - goto err_handles; + goto err_engines; if (name) { struct i915_hw_ppgtt *ppgtt; @@ -69,7 +73,9 @@ mock_context(struct drm_i915_private *i915, return ctx; -err_handles: +err_engines: + free_engines(rcu_access_pointer(ctx->engines)); +err_free: kfree(ctx); return NULL; -- cgit v1.2.3 From 976b55f0e1db5cb8fccb0a42f68ea77ae42604a6 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 21 May 2019 22:11:26 +0100 Subject: drm/i915: Allow a context to define its set of engines Over the last few years, we have debated how to extend the user API to support an increase in the number of engines, that may be sparse and even be heterogeneous within a class (not all video decoders created equal). We settled on using (class, instance) tuples to identify a specific engine, with an API for the user to construct a map of engines to capabilities. Into this picture, we then add a challenge of virtual engines; one user engine that maps behind the scenes to any number of physical engines. To keep it general, we want the user to have full control over that mapping. To that end, we allow the user to constrain a context to define the set of engines that it can access, order fully controlled by the user via (class, instance). With such precise control in context setup, we can continue to use the existing execbuf uABI of specifying a single index; only now it doesn't automagically map onto the engines, it uses the user defined engine map from the context. v2: Fixup freeing of local on success of get_engines() v3: Allow empty engines[] v4: s/nengine/num_engines/ v5: Replace 64 limit on num_engines with a note that execbuf is currently limited to only using the first 64 engines. v6: Actually use the engines_mutex to guard the ctx->engines. Testcase: igt/gem_ctx_engines Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190521211134.16117-2-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_context.c | 265 ++++++++++++++++++++++++-- drivers/gpu/drm/i915/i915_gem_context.h | 18 ++ drivers/gpu/drm/i915/i915_gem_context_types.h | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 +- drivers/gpu/drm/i915/i915_utils.h | 34 ++++ include/uapi/drm/i915_drm.h | 31 +++ 6 files changed, 341 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 413c4529191d..c501e2848ca0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -90,7 +90,6 @@ #include #include "gt/intel_lrc_reg.h" -#include "gt/intel_workarounds.h" #include "i915_drv.h" #include "i915_globals.h" @@ -141,15 +140,31 @@ static void lut_close(struct i915_gem_context *ctx) } static struct intel_context * -lookup_user_engine(struct i915_gem_context *ctx, u16 class, u16 instance) +lookup_user_engine(struct i915_gem_context *ctx, + unsigned long flags, + const struct i915_engine_class_instance *ci) +#define LOOKUP_USER_INDEX BIT(0) { - struct intel_engine_cs *engine; + int idx; - engine = intel_engine_lookup_user(ctx->i915, class, instance); - if (!engine) + if (!!(flags & LOOKUP_USER_INDEX) != i915_gem_context_user_engines(ctx)) return ERR_PTR(-EINVAL); - return i915_gem_context_get_engine(ctx, engine->id); + if (!i915_gem_context_user_engines(ctx)) { + struct intel_engine_cs *engine; + + engine = intel_engine_lookup_user(ctx->i915, + ci->engine_class, + ci->engine_instance); + if (!engine) + return ERR_PTR(-EINVAL); + + idx = engine->id; + } else { + idx = ci->engine_instance; + } + + return i915_gem_context_get_engine(ctx, idx); } static inline int new_hw_id(struct drm_i915_private *i915, gfp_t gfp) @@ -257,6 +272,17 @@ static void free_engines(struct i915_gem_engines *e) __free_engines(e, e->num_engines); } +static void free_engines_rcu(struct work_struct *wrk) +{ + struct i915_gem_engines *e = + container_of(wrk, struct i915_gem_engines, rcu.work); + struct drm_i915_private *i915 = e->i915; + + mutex_lock(&i915->drm.struct_mutex); + free_engines(e); + mutex_unlock(&i915->drm.struct_mutex); +} + static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx) { struct intel_engine_cs *engine; @@ -1352,9 +1378,7 @@ static int set_sseu(struct i915_gem_context *ctx, if (user_sseu.flags || user_sseu.rsvd) return -EINVAL; - ce = lookup_user_engine(ctx, - user_sseu.engine.engine_class, - user_sseu.engine.engine_instance); + ce = lookup_user_engine(ctx, 0, &user_sseu.engine); if (IS_ERR(ce)) return PTR_ERR(ce); @@ -1379,6 +1403,217 @@ out_ce: return ret; } +struct set_engines { + struct i915_gem_context *ctx; + struct i915_gem_engines *engines; +}; + +static const i915_user_extension_fn set_engines__extensions[] = { +}; + +static int +set_engines(struct i915_gem_context *ctx, + const struct drm_i915_gem_context_param *args) +{ + struct i915_context_param_engines __user *user = + u64_to_user_ptr(args->value); + struct set_engines set = { .ctx = ctx }; + unsigned int num_engines, n; + u64 extensions; + int err; + + if (!args->size) { /* switch back to legacy user_ring_map */ + if (!i915_gem_context_user_engines(ctx)) + return 0; + + set.engines = default_engines(ctx); + if (IS_ERR(set.engines)) + return PTR_ERR(set.engines); + + goto replace; + } + + BUILD_BUG_ON(!IS_ALIGNED(sizeof(*user), sizeof(*user->engines))); + if (args->size < sizeof(*user) || + !IS_ALIGNED(args->size, sizeof(*user->engines))) { + DRM_DEBUG("Invalid size for engine array: %d\n", + args->size); + return -EINVAL; + } + + /* + * Note that I915_EXEC_RING_MASK limits execbuf to only using the + * first 64 engines defined here. + */ + num_engines = (args->size - sizeof(*user)) / sizeof(*user->engines); + + set.engines = kmalloc(struct_size(set.engines, engines, num_engines), + GFP_KERNEL); + if (!set.engines) + return -ENOMEM; + + set.engines->i915 = ctx->i915; + for (n = 0; n < num_engines; n++) { + struct i915_engine_class_instance ci; + struct intel_engine_cs *engine; + + if (copy_from_user(&ci, &user->engines[n], sizeof(ci))) { + __free_engines(set.engines, n); + return -EFAULT; + } + + if (ci.engine_class == (u16)I915_ENGINE_CLASS_INVALID && + ci.engine_instance == (u16)I915_ENGINE_CLASS_INVALID_NONE) { + set.engines->engines[n] = NULL; + continue; + } + + engine = intel_engine_lookup_user(ctx->i915, + ci.engine_class, + ci.engine_instance); + if (!engine) { + DRM_DEBUG("Invalid engine[%d]: { class:%d, instance:%d }\n", + n, ci.engine_class, ci.engine_instance); + __free_engines(set.engines, n); + return -ENOENT; + } + + set.engines->engines[n] = intel_context_create(ctx, engine); + if (!set.engines->engines[n]) { + __free_engines(set.engines, n); + return -ENOMEM; + } + } + set.engines->num_engines = num_engines; + + err = -EFAULT; + if (!get_user(extensions, &user->extensions)) + err = i915_user_extensions(u64_to_user_ptr(extensions), + set_engines__extensions, + ARRAY_SIZE(set_engines__extensions), + &set); + if (err) { + free_engines(set.engines); + return err; + } + +replace: + mutex_lock(&ctx->engines_mutex); + if (args->size) + i915_gem_context_set_user_engines(ctx); + else + i915_gem_context_clear_user_engines(ctx); + rcu_swap_protected(ctx->engines, set.engines, 1); + mutex_unlock(&ctx->engines_mutex); + + INIT_RCU_WORK(&set.engines->rcu, free_engines_rcu); + queue_rcu_work(system_wq, &set.engines->rcu); + + return 0; +} + +static struct i915_gem_engines * +__copy_engines(struct i915_gem_engines *e) +{ + struct i915_gem_engines *copy; + unsigned int n; + + copy = kmalloc(struct_size(e, engines, e->num_engines), GFP_KERNEL); + if (!copy) + return ERR_PTR(-ENOMEM); + + copy->i915 = e->i915; + for (n = 0; n < e->num_engines; n++) { + if (e->engines[n]) + copy->engines[n] = intel_context_get(e->engines[n]); + else + copy->engines[n] = NULL; + } + copy->num_engines = n; + + return copy; +} + +static int +get_engines(struct i915_gem_context *ctx, + struct drm_i915_gem_context_param *args) +{ + struct i915_context_param_engines __user *user; + struct i915_gem_engines *e; + size_t n, count, size; + int err = 0; + + err = mutex_lock_interruptible(&ctx->engines_mutex); + if (err) + return err; + + e = NULL; + if (i915_gem_context_user_engines(ctx)) + e = __copy_engines(i915_gem_context_engines(ctx)); + mutex_unlock(&ctx->engines_mutex); + if (IS_ERR_OR_NULL(e)) { + args->size = 0; + return PTR_ERR_OR_ZERO(e); + } + + count = e->num_engines; + + /* Be paranoid in case we have an impedance mismatch */ + if (!check_struct_size(user, engines, count, &size)) { + err = -EINVAL; + goto err_free; + } + if (overflows_type(size, args->size)) { + err = -EINVAL; + goto err_free; + } + + if (!args->size) { + args->size = size; + goto err_free; + } + + if (args->size < size) { + err = -EINVAL; + goto err_free; + } + + user = u64_to_user_ptr(args->value); + if (!access_ok(user, size)) { + err = -EFAULT; + goto err_free; + } + + if (put_user(0, &user->extensions)) { + err = -EFAULT; + goto err_free; + } + + for (n = 0; n < count; n++) { + struct i915_engine_class_instance ci = { + .engine_class = I915_ENGINE_CLASS_INVALID, + .engine_instance = I915_ENGINE_CLASS_INVALID_NONE, + }; + + if (e->engines[n]) { + ci.engine_class = e->engines[n]->engine->uabi_class; + ci.engine_instance = e->engines[n]->engine->instance; + } + + if (copy_to_user(&user->engines[n], &ci, sizeof(ci))) { + err = -EFAULT; + goto err_free; + } + } + + args->size = size; + +err_free: + INIT_RCU_WORK(&e->rcu, free_engines_rcu); + queue_rcu_work(system_wq, &e->rcu); + return err; +} + static int ctx_setparam(struct drm_i915_file_private *fpriv, struct i915_gem_context *ctx, struct drm_i915_gem_context_param *args) @@ -1452,6 +1687,10 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, ret = set_ppgtt(fpriv, ctx, args); break; + case I915_CONTEXT_PARAM_ENGINES: + ret = set_engines(ctx, args); + break; + case I915_CONTEXT_PARAM_BAN_PERIOD: default: ret = -EINVAL; @@ -1596,9 +1835,7 @@ static int get_sseu(struct i915_gem_context *ctx, if (user_sseu.flags || user_sseu.rsvd) return -EINVAL; - ce = lookup_user_engine(ctx, - user_sseu.engine.engine_class, - user_sseu.engine.engine_instance); + ce = lookup_user_engine(ctx, 0, &user_sseu.engine); if (IS_ERR(ce)) return PTR_ERR(ce); @@ -1682,6 +1919,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, ret = get_ppgtt(file_priv, ctx, args); break; + case I915_CONTEXT_PARAM_ENGINES: + ret = get_engines(ctx, args); + break; + case I915_CONTEXT_PARAM_BAN_PERIOD: default: ret = -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 272e183ebc0c..9ad4a6362438 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -112,6 +112,24 @@ static inline void i915_gem_context_set_force_single_submission(struct i915_gem_ __set_bit(CONTEXT_FORCE_SINGLE_SUBMISSION, &ctx->flags); } +static inline bool +i915_gem_context_user_engines(const struct i915_gem_context *ctx) +{ + return test_bit(CONTEXT_USER_ENGINES, &ctx->flags); +} + +static inline void +i915_gem_context_set_user_engines(struct i915_gem_context *ctx) +{ + set_bit(CONTEXT_USER_ENGINES, &ctx->flags); +} + +static inline void +i915_gem_context_clear_user_engines(struct i915_gem_context *ctx) +{ + clear_bit(CONTEXT_USER_ENGINES, &ctx->flags); +} + int __i915_gem_context_pin_hw_id(struct i915_gem_context *ctx); static inline int i915_gem_context_pin_hw_id(struct i915_gem_context *ctx) { diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h b/drivers/gpu/drm/i915/i915_gem_context_types.h index d5cb4f121aad..fb965ded2508 100644 --- a/drivers/gpu/drm/i915/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/i915_gem_context_types.h @@ -146,6 +146,7 @@ struct i915_gem_context { #define CONTEXT_BANNED 0 #define CONTEXT_CLOSED 1 #define CONTEXT_FORCE_SINGLE_SUBMISSION 2 +#define CONTEXT_USER_ENGINES 3 /** * @hw_id: - unique identifier for the context diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 679f7c1561ba..d6c5220addd0 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2165,7 +2165,10 @@ eb_select_engine(struct i915_execbuffer *eb, unsigned int idx; int err; - idx = eb_select_legacy_ring(eb, file, args); + if (i915_gem_context_user_engines(eb->gem_context)) + idx = args->flags & I915_EXEC_RING_MASK; + else + idx = eb_select_legacy_ring(eb, file, args); ce = i915_gem_context_get_engine(eb->gem_context, idx); if (IS_ERR(ce)) diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 5c94c7ab4607..e52866084891 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -26,6 +26,7 @@ #define __I915_UTILS_H #include +#include #include #include #include @@ -78,6 +79,39 @@ #define overflows_type(x, T) \ (sizeof(x) > sizeof(T) && (x) >> BITS_PER_TYPE(T)) +static inline bool +__check_struct_size(size_t base, size_t arr, size_t count, size_t *size) +{ + size_t sz; + + if (check_mul_overflow(count, arr, &sz)) + return false; + + if (check_add_overflow(sz, base, &sz)) + return false; + + *size = sz; + return true; +} + +/** + * check_struct_size() - Calculate size of structure with trailing array. + * @p: Pointer to the structure. + * @member: Name of the array member. + * @n: Number of elements in the array. + * @sz: Total size of structure and array + * + * Calculates size of memory needed for structure @p followed by an + * array of @n @member elements, like struct_size() but reports + * whether it overflowed, and the resultant size in @sz + * + * Return: false if the calculation overflowed. + */ +#define check_struct_size(p, member, n, sz) \ + likely(__check_struct_size(sizeof(*(p)), \ + sizeof(*(p)->member) + __must_be_array((p)->member), \ + n, sz)) + #define ptr_mask_bits(ptr, n) ({ \ unsigned long __v = (unsigned long)(ptr); \ (typeof(ptr))(__v & -BIT(n)); \ diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index d6ad4a15b2b9..8e1bb22926e4 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -136,6 +136,7 @@ enum drm_i915_gem_engine_class { struct i915_engine_class_instance { __u16 engine_class; /* see enum drm_i915_gem_engine_class */ __u16 engine_instance; +#define I915_ENGINE_CLASS_INVALID_NONE -1 }; /** @@ -1522,6 +1523,26 @@ struct drm_i915_gem_context_param { * See DRM_I915_GEM_VM_CREATE and DRM_I915_GEM_VM_DESTROY. */ #define I915_CONTEXT_PARAM_VM 0x9 + +/* + * I915_CONTEXT_PARAM_ENGINES: + * + * Bind this context to operate on this subset of available engines. Henceforth, + * the I915_EXEC_RING selector for DRM_IOCTL_I915_GEM_EXECBUFFER2 operates as + * an index into this array of engines; I915_EXEC_DEFAULT selecting engine[0] + * and upwards. Slots 0...N are filled in using the specified (class, instance). + * Use + * engine_class: I915_ENGINE_CLASS_INVALID, + * engine_instance: I915_ENGINE_CLASS_INVALID_NONE + * to specify a gap in the array that can be filled in later, e.g. by a + * virtual engine used for load balancing. + * + * Setting the number of engines bound to the context to 0, by passing a zero + * sized argument, will revert back to default settings. + * + * See struct i915_context_param_engines. + */ +#define I915_CONTEXT_PARAM_ENGINES 0xa /* Must be kept compact -- no holes and well documented */ __u64 value; @@ -1585,6 +1606,16 @@ struct drm_i915_gem_context_param_sseu { __u32 rsvd; }; +struct i915_context_param_engines { + __u64 extensions; /* linked chain of extension blocks, 0 terminates */ + struct i915_engine_class_instance engines[0]; +} __attribute__((packed)); + +#define I915_DEFINE_CONTEXT_PARAM_ENGINES(name__, N__) struct { \ + __u64 extensions; \ + struct i915_engine_class_instance engines[N__]; \ +} __attribute__((packed)) name__ + struct drm_i915_gem_context_create_ext_setparam { #define I915_CONTEXT_CREATE_EXT_SETPARAM 0 struct i915_user_extension base; -- cgit v1.2.3 From a88b6e4cbafd6f23b3450c087acdbe23d90e7606 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 21 May 2019 22:11:34 +0100 Subject: drm/i915: Allow specification of parallel execbuf There is a desire to split a task onto two engines and have them run at the same time, e.g. scanline interleaving to spread the workload evenly. Through the use of the out-fence from the first execbuf, we can coordinate secondary execbuf to only become ready simultaneously with the first, so that with all things idle the second execbufs are executed in parallel with the first. The key difference here between the new EXEC_FENCE_SUBMIT and the existing EXEC_FENCE_IN is that the in-fence waits for the completion of the first request (so that all of its rendering results are visible to the second execbuf, the more common userspace fence requirement). Since we only have a single input fence slot, userspace cannot mix an in-fence and a submit-fence. It has to use one or the other! This is not such a harsh requirement, since by virtue of the submit-fence, the secondary execbuf inherit all of the dependencies from the first request, and for the application the dependencies should be common between the primary and secondary execbuf. Suggested-by: Tvrtko Ursulin Testcase: igt/gem_exec_fence/parallel Link: https://github.com/intel/media-driver/pull/546 Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190521211134.16117-10-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 25 ++++++++++++++++++++++++- include/uapi/drm/i915_drm.h | 17 ++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 5061cb32856b..83d2eb9e74cb 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -443,6 +443,7 @@ static int i915_getparam_ioctl(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_CAPTURE: case I915_PARAM_HAS_EXEC_BATCH_FIRST: case I915_PARAM_HAS_EXEC_FENCE_ARRAY: + case I915_PARAM_HAS_EXEC_SUBMIT_FENCE: /* For the time being all of these are always true; * if some supported hardware does not have one of these * features this value needs to be provided from diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d6c5220addd0..7ce25b54c57b 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -2318,6 +2318,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, { struct i915_execbuffer eb; struct dma_fence *in_fence = NULL; + struct dma_fence *exec_fence = NULL; struct sync_file *out_fence = NULL; int out_fence_fd = -1; int err; @@ -2360,11 +2361,24 @@ i915_gem_do_execbuffer(struct drm_device *dev, return -EINVAL; } + if (args->flags & I915_EXEC_FENCE_SUBMIT) { + if (in_fence) { + err = -EINVAL; + goto err_in_fence; + } + + exec_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); + if (!exec_fence) { + err = -EINVAL; + goto err_in_fence; + } + } + if (args->flags & I915_EXEC_FENCE_OUT) { out_fence_fd = get_unused_fd_flags(O_CLOEXEC); if (out_fence_fd < 0) { err = out_fence_fd; - goto err_in_fence; + goto err_exec_fence; } } @@ -2494,6 +2508,13 @@ i915_gem_do_execbuffer(struct drm_device *dev, goto err_request; } + if (exec_fence) { + err = i915_request_await_execution(eb.request, exec_fence, + eb.engine->bond_execute); + if (err < 0) + goto err_request; + } + if (fences) { err = await_fence_array(&eb, fences); if (err) @@ -2555,6 +2576,8 @@ err_destroy: err_out_fence: if (out_fence_fd != -1) put_unused_fd(out_fence_fd); +err_exec_fence: + dma_fence_put(exec_fence); err_in_fence: dma_fence_put(in_fence); return err; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index e2da9027bcdf..bdb00ec1f8be 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -604,6 +604,12 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_MMAP_GTT_COHERENT 52 +/* + * Query whether DRM_I915_GEM_EXECBUFFER2 supports coordination of parallel + * execution through use of explicit fence support. + * See I915_EXEC_FENCE_OUT and I915_EXEC_FENCE_SUBMIT. + */ +#define I915_PARAM_HAS_EXEC_SUBMIT_FENCE 53 /* Must be kept compact -- no holes and well documented */ typedef struct drm_i915_getparam { @@ -1126,7 +1132,16 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_ARRAY (1<<19) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_ARRAY<<1)) +/* + * Setting I915_EXEC_FENCE_SUBMIT implies that lower_32_bits(rsvd2) represent + * a sync_file fd to wait upon (in a nonblocking manner) prior to executing + * the batch. + * + * Returns -EINVAL if the sync_file fd cannot be found. + */ +#define I915_EXEC_FENCE_SUBMIT (1 << 20) + +#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SUBMIT << 1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ -- cgit v1.2.3