On 05/08/2020 13:22, Chris Wilson wrote:
Since we can control placement in the ppGTT explicitly, we can specify
our desired starting offset exactly on a per-vma basis. This prevents us
falling down a few corner cases where we confuse the user with our choices.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 67 +++++++++----------
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 19cab5541dbc..0839397c7e50 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -36,6 +36,7 @@ struct eb_vma {
/** This vma's place in the execbuf reservation list */
struct drm_i915_gem_exec_object2 *exec;
+ u32 bias;
struct list_head bind_link;
struct list_head unbound_link;
@@ -61,15 +62,12 @@ struct eb_vma_array {
#define __EXEC_OBJECT_HAS_PIN BIT(31)
#define __EXEC_OBJECT_HAS_FENCE BIT(30)
#define __EXEC_OBJECT_NEEDS_MAP BIT(29)
-#define __EXEC_OBJECT_NEEDS_BIAS BIT(28)
-#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 28) /* all of the above */
+#define __EXEC_OBJECT_INTERNAL_FLAGS (~0u << 29) /* all of the above */
#define __EXEC_HAS_RELOC BIT(31)
#define __EXEC_INTERNAL_FLAGS (~0u << 31)
#define UPDATE PIN_OFFSET_FIXED
-#define BATCH_OFFSET_BIAS (256*1024)
-
#define __I915_EXEC_ILLEGAL_FLAGS \
(__I915_EXEC_UNKNOWN_FLAGS | \
I915_EXEC_CONSTANTS_MASK | \
@@ -291,7 +289,7 @@ struct i915_execbuffer {
} parser;
u64 invalid_flags; /** Set of execobj.flags that are invalid */
- u32 context_flags; /** Set of execobj.flags to insert from the ctx */
+ u32 context_bias;
u32 batch_start_offset; /** Location within object of batch */
u32 batch_len; /** Length of batch within object */
@@ -491,11 +489,12 @@ static int eb_create(struct i915_execbuffer *eb)
return 0;
}
-static bool
-eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
- const struct i915_vma *vma,
- unsigned int flags)
+static bool eb_vma_misplaced(const struct eb_vma *ev)
{
+ const struct drm_i915_gem_exec_object2 *entry = ev->exec;
+ const struct i915_vma *vma = ev->vma;
+ unsigned int flags = ev->flags;
+
if (test_bit(I915_VMA_ERROR_BIT, __i915_vma_flags(vma)))
return true;
@@ -509,8 +508,7 @@ eb_vma_misplaced(const struct drm_i915_gem_exec_object2 *entry,
vma->node.start != entry->offset)
return true;
- if (flags & __EXEC_OBJECT_NEEDS_BIAS &&
- vma->node.start < BATCH_OFFSET_BIAS)
+ if (vma->node.start < ev->bias)
return true;
if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) &&
@@ -529,10 +527,7 @@ static bool eb_pin_vma_fence_inplace(struct eb_vma *ev)
return false; /* We need to add some new fence serialisation */
}
-static inline bool
-eb_pin_vma_inplace(struct i915_execbuffer *eb,
- const struct drm_i915_gem_exec_object2 *entry,
- struct eb_vma *ev)
+static inline bool eb_pin_vma_inplace(struct eb_vma *ev)
{
struct i915_vma *vma = ev->vma;
unsigned int pin_flags;
@@ -541,7 +536,7 @@ eb_pin_vma_inplace(struct i915_execbuffer *eb,
if (!i915_active_is_idle(&vma->vm->binding))
return false;
- if (eb_vma_misplaced(entry, vma, ev->flags))
+ if (eb_vma_misplaced(ev))
return false;
pin_flags = PIN_USER;
@@ -559,7 +554,7 @@ eb_pin_vma_inplace(struct i915_execbuffer *eb,
}
}
- GEM_BUG_ON(eb_vma_misplaced(entry, vma, ev->flags));
+ GEM_BUG_ON(eb_vma_misplaced(ev));
ev->flags |= __EXEC_OBJECT_HAS_PIN;
return true;
@@ -608,9 +603,6 @@ eb_validate_vma(struct i915_execbuffer *eb,
entry->flags |= EXEC_OBJECT_NEEDS_GTT | __EXEC_OBJECT_NEEDS_MAP;
}
- if (!(entry->flags & EXEC_OBJECT_PINNED))
- entry->flags |= eb->context_flags;
-
return 0;
}
@@ -627,6 +619,7 @@ eb_add_vma(struct i915_execbuffer *eb,
ev->vma = vma;
ev->exec = entry;
ev->flags = entry->flags;
+ ev->bias = eb->context_bias;
if (eb->lut_size > 0) {
ev->handle = entry->handle;
@@ -653,7 +646,8 @@ eb_add_vma(struct i915_execbuffer *eb,
if (i == batch_idx) {
if (entry->relocation_count &&
!(ev->flags & EXEC_OBJECT_PINNED))
- ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+ ev->bias = max_t(u32, ev->bias, SZ_256K);
What dictates the 256KiB border? Wondering if this is too hidden in here
or not.
Regards,
Tvrtko
+
if (eb->has_fence)
ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
@@ -979,8 +973,9 @@ static int eb_reserve_vma(struct eb_vm_work *work, struct eb_bind_vma *bind)
const unsigned int exec_flags = bind->ev->flags;
struct i915_vma *vma = bind->ev->vma;
struct i915_address_space *vm = vma->vm;
- u64 start = 0, end = vm->total;
u64 align = entry->alignment ?: I915_GTT_MIN_ALIGNMENT;
+ u64 start = round_up(bind->ev->bias, align);
+ u64 end = vm->total;
unsigned int bind_flags;
int err;
@@ -999,7 +994,7 @@ static int eb_reserve_vma(struct eb_vm_work *work, struct eb_bind_vma *bind)
GEM_BUG_ON(!vma->size);
/* Reuse old address (if it doesn't conflict with new requirements) */
- if (eb_vma_misplaced(entry, vma, exec_flags)) {
+ if (eb_vma_misplaced(bind->ev)) {
vma->node.start = entry->offset & PIN_OFFSET_MASK;
vma->node.size = max(entry->pad_to_size, vma->size);
vma->node.color = 0;
@@ -1021,11 +1016,8 @@ static int eb_reserve_vma(struct eb_vm_work *work, struct eb_bind_vma *bind)
align = max_t(u64, align, vma->fence_alignment);
}
- if (exec_flags & __EXEC_OBJECT_NEEDS_BIAS)
- start = BATCH_OFFSET_BIAS;
-
GEM_BUG_ON(!vma->node.size);
- if (vma->node.size > end - start)
+ if (start > end || vma->node.size > end - start)
return -E2BIG;
/* Try the user's preferred location first (mandatory if soft-pinned) */
@@ -1108,7 +1100,7 @@ static int eb_reserve_vma(struct eb_vm_work *work, struct eb_bind_vma *bind)
}
bind->ev->flags |= __EXEC_OBJECT_HAS_PIN;
- GEM_BUG_ON(eb_vma_misplaced(entry, vma, bind->ev->flags));
+ GEM_BUG_ON(eb_vma_misplaced(bind->ev));
if (unlikely(exec_flags & EXEC_OBJECT_NEEDS_FENCE)) {
err = __i915_vma_pin_fence_async(vma, &work->base);
@@ -1341,8 +1333,7 @@ static int wait_for_unbinds(struct i915_execbuffer *eb,
GEM_BUG_ON(ev->flags & __EXEC_OBJECT_HAS_PIN);
- if (drm_mm_node_allocated(&vma->node) &&
- eb_vma_misplaced(ev->exec, vma, ev->flags)) {
+ if (drm_mm_node_allocated(&vma->node) && eb_vma_misplaced(ev)) {
err = i915_vma_unbind(vma);
if (err)
return err;
@@ -1391,10 +1382,10 @@ static int eb_reserve_vm(struct i915_execbuffer *eb)
count = 0;
INIT_LIST_HEAD(&unbound);
list_for_each_entry(ev, &eb->bind_list, bind_link) {
- struct drm_i915_gem_exec_object2 *entry = ev->exec;
- struct i915_vma *vma = ev->vma;
+ if (eb_pin_vma_inplace(ev)) {
+ struct drm_i915_gem_exec_object2 *entry = ev->exec;
+ struct i915_vma *vma = ev->vma;
- if (eb_pin_vma_inplace(eb, entry, ev)) {
if (entry != &no_entry &&
entry->offset != vma->node.start) {
entry->offset = vma->node.start | UPDATE;
@@ -1478,7 +1469,7 @@ static int eb_reserve_vm(struct i915_execbuffer *eb)
* we cannot handle migrating the vma inside the worker.
*/
if (drm_mm_node_allocated(&vma->node)) {
- if (eb_vma_misplaced(ev->exec, vma, ev->flags)) {
+ if (eb_vma_misplaced(ev)) {
err = -ENOSPC;
break;
}
@@ -1736,9 +1727,13 @@ static int eb_select_context(struct i915_execbuffer *eb)
if (rcu_access_pointer(ctx->vm))
eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
- eb->context_flags = 0;
+ /*
+ * At the user's discretion, ensure that we place no objects at
+ * offset 0, allowing them to use 0 as a 'NULL' pointer.
+ */
+ eb->context_bias = 0;
if (test_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags))
- eb->context_flags |= __EXEC_OBJECT_NEEDS_BIAS;
+ eb->context_bias = I915_GTT_MIN_ALIGNMENT;
return 0;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
