From 170fa29b14fadf2deb361589cefe6a78b21b1b22 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 16 Aug 2017 09:52:07 +0100 Subject: [PATCH] drm/i915: Simplify eb_lookup_vmas() Since the introduction of being able to perform a lockless lookup of an object (i915_gem_object_get_rcu() in fbbd37b36fa5 ("drm/i915: Move object release to a freelist + worker") we no longer need to split the object/vma lookup into 3 phases and so combine them into a much simpler single loop. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20170816085210.4199-4-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 122 ++++++--------------- 1 file changed, 36 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index da6cb2fe5f85..95e461259d24 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -450,7 +450,9 @@ eb_validate_vma(struct i915_execbuffer *eb, } static int -eb_add_vma(struct i915_execbuffer *eb, unsigned int i, struct i915_vma *vma) +eb_add_vma(struct i915_execbuffer *eb, + unsigned int i, struct i915_vma *vma, + unsigned int flags) { struct drm_i915_gem_exec_object2 *entry = &eb->exec[i]; int err; @@ -480,7 +482,7 @@ eb_add_vma(struct i915_execbuffer *eb, unsigned int i, struct i915_vma *vma) * to find the right target VMA when doing relocations. */ eb->vma[i] = vma; - eb->flags[i] = entry->flags; + eb->flags[i] = entry->flags | flags; vma->exec_flags = &eb->flags[i]; err = 0; @@ -686,13 +688,9 @@ static int eb_select_context(struct i915_execbuffer *eb) static int eb_lookup_vmas(struct i915_execbuffer *eb) { -#define INTERMEDIATE BIT(0) - const unsigned int count = eb->buffer_count; struct i915_gem_context_vma_lut *lut = &eb->ctx->vma_lut; - struct i915_vma *vma; - struct idr *idr; + struct drm_i915_gem_object *uninitialized_var(obj); unsigned int i; - int slow_pass = -1; int err; if (unlikely(i915_gem_context_is_closed(eb->ctx))) @@ -708,82 +706,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) flush_work(&lut->resize); GEM_BUG_ON(lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS); - for (i = 0; i < count; i++) { - hlist_for_each_entry(vma, - ht_head(lut, eb->exec[i].handle), - ctx_node) { - if (vma->ctx_handle != eb->exec[i].handle) - continue; - - err = eb_add_vma(eb, i, vma); - if (unlikely(err)) - return err; - goto next_vma; - } - - if (slow_pass < 0) - slow_pass = i; -next_vma: ; - } + for (i = 0; i < eb->buffer_count; i++) { + u32 handle = eb->exec[i].handle; + struct hlist_head *hl = ht_head(lut, handle); + unsigned int flags = 0; + struct i915_vma *vma; - if (slow_pass < 0) - goto out; + hlist_for_each_entry(vma, hl, ctx_node) { + GEM_BUG_ON(vma->ctx != eb->ctx); - spin_lock(&eb->file->table_lock); - /* - * Grab a reference to the object and release the lock so we can lookup - * or create the VMA without using GFP_ATOMIC - */ - idr = &eb->file->object_idr; - for (i = slow_pass; i < count; i++) { - struct drm_i915_gem_object *obj; + if (vma->ctx_handle != handle) + continue; - if (eb->vma[i]) - continue; + goto add_vma; + } - obj = to_intel_bo(idr_find(idr, eb->exec[i].handle)); + obj = i915_gem_object_lookup(eb->file, handle); if (unlikely(!obj)) { - spin_unlock(&eb->file->table_lock); - DRM_DEBUG("Invalid object handle %d at index %d\n", - eb->exec[i].handle, i); err = -ENOENT; - goto err; + goto err_vma; } - eb->vma[i] = (struct i915_vma *) - ptr_pack_bits(obj, INTERMEDIATE, 1); - } - spin_unlock(&eb->file->table_lock); - - for (i = slow_pass; i < count; i++) { - struct drm_i915_gem_object *obj; - unsigned int is_obj; - - obj = (typeof(obj))ptr_unpack_bits(eb->vma[i], &is_obj, 1); - if (!is_obj) - continue; - - /* - * NOTE: We can leak any vmas created here when something fails - * later on. But that's no issue since vma_unbind can deal with - * vmas which are not actually bound. And since only - * lookup_or_create exists as an interface to get at the vma - * from the (obj, vm) we don't run the risk of creating - * duplicated vmas for the same vm. - */ vma = i915_vma_instance(obj, eb->vm, NULL); if (unlikely(IS_ERR(vma))) { - DRM_DEBUG("Failed to lookup VMA\n"); err = PTR_ERR(vma); - goto err; + goto err_obj; } /* First come, first served */ if (!vma->ctx) { vma->ctx = eb->ctx; - vma->ctx_handle = eb->exec[i].handle; - hlist_add_head(&vma->ctx_node, - ht_head(lut, eb->exec[i].handle)); + vma->ctx_handle = handle; + hlist_add_head(&vma->ctx_node, hl); lut->ht_count++; lut->ht_size |= I915_CTX_RESIZE_IN_PROGRESS; if (i915_vma_is_ggtt(vma)) { @@ -791,21 +745,19 @@ next_vma: ; obj->vma_hashed = vma; } - i915_vma_get(vma); + /* transfer ref to ctx */ + obj = NULL; + } else { + flags = __EXEC_OBJECT_HAS_REF; } - err = eb_add_vma(eb, i, vma); +add_vma: + err = eb_add_vma(eb, i, vma, flags); if (unlikely(err)) - goto err; + goto err_obj; GEM_BUG_ON(vma != eb->vma[i]); GEM_BUG_ON(vma->exec_flags != &eb->flags[i]); - - /* Only after we validated the user didn't use our bits */ - if (vma->ctx != eb->ctx) { - i915_vma_get(vma); - *vma->exec_flags |= __EXEC_OBJECT_HAS_REF; - } } if (lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS) { @@ -815,7 +767,6 @@ next_vma: ; lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS; } -out: /* take note of the batch buffer before we might reorder the lists */ i = eb_batch_index(eb); eb->batch = eb->vma[i]; @@ -838,14 +789,13 @@ next_vma: ; eb->args->flags |= __EXEC_VALIDATED; return eb_reserve(eb); -err: - for (i = slow_pass; i < count; i++) { - if (ptr_unmask_bits(eb->vma[i], 1)) - eb->vma[i] = NULL; - } +err_obj: + if (obj) + i915_gem_object_put(obj); +err_vma: + eb->vma[i] = NULL; lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS; return err; -#undef INTERMEDIATE } static struct i915_vma * @@ -878,7 +828,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb) unsigned int flags = eb->flags[i]; if (!vma) - continue; + break; GEM_BUG_ON(vma->exec_flags != &eb->flags[i]); vma->exec_flags = NULL; @@ -2268,8 +2218,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, args->flags |= __EXEC_HAS_RELOC; eb.exec = exec; - eb.vma = memset(exec + args->buffer_count + 1, 0, - (args->buffer_count + 1) * sizeof(*eb.vma)); + eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1); + eb.vma[0] = NULL; eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; -- 2.45.2