On Tue, Jul 05, 2022 at 10:57:17AM +0200, Thomas Hellström wrote:
On Fri, 2022-07-01 at 15:50 -0700, Niranjana Vishwanathapura wrote:
vma_lookup is tied to segment of the object instead of section
of VA space. Hence, it do not support aliasing (ie., multiple
bindings to the same section of the object).
Skip vma_lookup for persistent vmas as it supports aliasing.
Signed-off-by: Niranjana Vishwanathapura
<niranjana.vishwanathapura@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_vma.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c
b/drivers/gpu/drm/i915/i915_vma.c
index 6adb013579be..9aa38b772b5b 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -197,6 +197,10 @@ vma_create(struct drm_i915_gem_object *obj,
__set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma));
}
+ if (!i915_vma_is_ggtt(vma) &&
+ (view && view->type == I915_GGTT_VIEW_PARTIAL))
+ goto skip_rb_insert;
+
Rather than guessing that a vma with this signature is a persistent
vma, which is confusing to the reader, could we have an argument saying
we want to create a persistent vma?
Yah, sounds good. We probably can even check for vm->vm_bind_mode here
instead of passing a new argument. I think i915 won't create any
internal vmas for this VM, so, should be good to check vm->vm_bind_mode.
rb = NULL;
p = &obj->vma.tree.rb_node;
while (*p) {
@@ -221,6 +225,7 @@ vma_create(struct drm_i915_gem_object *obj,
rb_link_node(&vma->obj_node, rb, p);
rb_insert_color(&vma->obj_node, &obj->vma.tree);
+skip_rb_insert:
if (i915_vma_is_ggtt(vma))
/*
* We put the GGTT vma at the start of the vma-list,
followed
@@ -292,13 +297,16 @@ i915_vma_instance(struct drm_i915_gem_object
*obj,
struct i915_address_space *vm,
const struct i915_ggtt_view *view)
{
- struct i915_vma *vma;
+ struct i915_vma *vma = NULL;
GEM_BUG_ON(!kref_read(&vm->ref));
- spin_lock(&obj->vma.lock);
- vma = i915_vma_lookup(obj, vm, view);
- spin_unlock(&obj->vma.lock);
+ if (i915_is_ggtt(vm) || !view ||
+ view->type != I915_GGTT_VIEW_PARTIAL) {
Same here?
We probably can remove this code and have vm_bind ioctl directly
call vma_create.
Niranjana
/Thomas
+ spin_lock(&obj->vma.lock);
+ vma = i915_vma_lookup(obj, vm, view);
+ spin_unlock(&obj->vma.lock);
+ }
/* vma_create() will resolve the race if another creates the
vma */
if (unlikely(!vma))
@@ -1670,7 +1678,8 @@ static void release_references(struct i915_vma
*vma, bool vm_ddestroy)
spin_lock(&obj->vma.lock);
list_del(&vma->obj_link);
- if (!RB_EMPTY_NODE(&vma->obj_node))
+ if (!i915_vma_is_persistent(vma) &&
+ !RB_EMPTY_NODE(&vma->obj_node))
rb_erase(&vma->obj_node, &obj->vma.tree);
spin_unlock(&obj->vma.lock);
