Commit 170fa29b authored by Chris Wilson's avatar Chris Wilson

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 fbbd37b3 ("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: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170816085210.4199-4-chris@chris-wilson.co.uk
parent c7c6e46f
...@@ -450,7 +450,9 @@ eb_validate_vma(struct i915_execbuffer *eb, ...@@ -450,7 +450,9 @@ eb_validate_vma(struct i915_execbuffer *eb,
} }
static int 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]; struct drm_i915_gem_exec_object2 *entry = &eb->exec[i];
int err; int err;
...@@ -480,7 +482,7 @@ eb_add_vma(struct i915_execbuffer *eb, unsigned int i, struct i915_vma *vma) ...@@ -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. * to find the right target VMA when doing relocations.
*/ */
eb->vma[i] = vma; eb->vma[i] = vma;
eb->flags[i] = entry->flags; eb->flags[i] = entry->flags | flags;
vma->exec_flags = &eb->flags[i]; vma->exec_flags = &eb->flags[i];
err = 0; err = 0;
...@@ -686,13 +688,9 @@ static int eb_select_context(struct i915_execbuffer *eb) ...@@ -686,13 +688,9 @@ static int eb_select_context(struct i915_execbuffer *eb)
static int eb_lookup_vmas(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_gem_context_vma_lut *lut = &eb->ctx->vma_lut;
struct i915_vma *vma; struct drm_i915_gem_object *uninitialized_var(obj);
struct idr *idr;
unsigned int i; unsigned int i;
int slow_pass = -1;
int err; int err;
if (unlikely(i915_gem_context_is_closed(eb->ctx))) if (unlikely(i915_gem_context_is_closed(eb->ctx)))
...@@ -708,82 +706,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb) ...@@ -708,82 +706,38 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
flush_work(&lut->resize); flush_work(&lut->resize);
GEM_BUG_ON(lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS); GEM_BUG_ON(lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS);
for (i = 0; i < count; i++) { for (i = 0; i < eb->buffer_count; i++) {
hlist_for_each_entry(vma, u32 handle = eb->exec[i].handle;
ht_head(lut, eb->exec[i].handle), struct hlist_head *hl = ht_head(lut, handle);
ctx_node) { unsigned int flags = 0;
if (vma->ctx_handle != eb->exec[i].handle) struct i915_vma *vma;
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: ;
}
if (slow_pass < 0) hlist_for_each_entry(vma, hl, ctx_node) {
goto out; GEM_BUG_ON(vma->ctx != eb->ctx);
spin_lock(&eb->file->table_lock); if (vma->ctx_handle != handle)
/* continue;
* 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 (eb->vma[i]) goto add_vma;
continue; }
obj = to_intel_bo(idr_find(idr, eb->exec[i].handle)); obj = i915_gem_object_lookup(eb->file, handle);
if (unlikely(!obj)) { 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; 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); vma = i915_vma_instance(obj, eb->vm, NULL);
if (unlikely(IS_ERR(vma))) { if (unlikely(IS_ERR(vma))) {
DRM_DEBUG("Failed to lookup VMA\n");
err = PTR_ERR(vma); err = PTR_ERR(vma);
goto err; goto err_obj;
} }
/* First come, first served */ /* First come, first served */
if (!vma->ctx) { if (!vma->ctx) {
vma->ctx = eb->ctx; vma->ctx = eb->ctx;
vma->ctx_handle = eb->exec[i].handle; vma->ctx_handle = handle;
hlist_add_head(&vma->ctx_node, hlist_add_head(&vma->ctx_node, hl);
ht_head(lut, eb->exec[i].handle));
lut->ht_count++; lut->ht_count++;
lut->ht_size |= I915_CTX_RESIZE_IN_PROGRESS; lut->ht_size |= I915_CTX_RESIZE_IN_PROGRESS;
if (i915_vma_is_ggtt(vma)) { if (i915_vma_is_ggtt(vma)) {
...@@ -791,21 +745,19 @@ next_vma: ; ...@@ -791,21 +745,19 @@ next_vma: ;
obj->vma_hashed = 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)) if (unlikely(err))
goto err; goto err_obj;
GEM_BUG_ON(vma != eb->vma[i]); GEM_BUG_ON(vma != eb->vma[i]);
GEM_BUG_ON(vma->exec_flags != &eb->flags[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) { if (lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS) {
...@@ -815,7 +767,6 @@ next_vma: ; ...@@ -815,7 +767,6 @@ next_vma: ;
lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS; lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS;
} }
out:
/* take note of the batch buffer before we might reorder the lists */ /* take note of the batch buffer before we might reorder the lists */
i = eb_batch_index(eb); i = eb_batch_index(eb);
eb->batch = eb->vma[i]; eb->batch = eb->vma[i];
...@@ -838,14 +789,13 @@ next_vma: ; ...@@ -838,14 +789,13 @@ next_vma: ;
eb->args->flags |= __EXEC_VALIDATED; eb->args->flags |= __EXEC_VALIDATED;
return eb_reserve(eb); return eb_reserve(eb);
err: err_obj:
for (i = slow_pass; i < count; i++) { if (obj)
if (ptr_unmask_bits(eb->vma[i], 1)) i915_gem_object_put(obj);
eb->vma[i] = NULL; err_vma:
} eb->vma[i] = NULL;
lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS; lut->ht_size &= ~I915_CTX_RESIZE_IN_PROGRESS;
return err; return err;
#undef INTERMEDIATE
} }
static struct i915_vma * static struct i915_vma *
...@@ -878,7 +828,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb) ...@@ -878,7 +828,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb)
unsigned int flags = eb->flags[i]; unsigned int flags = eb->flags[i];
if (!vma) if (!vma)
continue; break;
GEM_BUG_ON(vma->exec_flags != &eb->flags[i]); GEM_BUG_ON(vma->exec_flags != &eb->flags[i]);
vma->exec_flags = NULL; vma->exec_flags = NULL;
...@@ -2268,8 +2218,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, ...@@ -2268,8 +2218,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
args->flags |= __EXEC_HAS_RELOC; args->flags |= __EXEC_HAS_RELOC;
eb.exec = exec; eb.exec = exec;
eb.vma = memset(exec + args->buffer_count + 1, 0, eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
(args->buffer_count + 1) * sizeof(*eb.vma)); eb.vma[0] = NULL;
eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1); eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment