Based upon a hunk from a patch from Chris Wilson, but augmented to:
- Process the batch in the full ppgtt vm so that self-relocations
match again with userspace's expectations..
- Add a comment why plain pin for the global gtt binding is safe at
that point.
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 38 ++++++++++++++----------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 6320a385841b..697b3d5f161a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -138,13 +138,6 @@ eb_lookup_vmas(struct eb_vmas *eb,
goto err;
}
- /* If we have secure dispatch, or the userspace assures us that
- * they know what they're doing, use the GGTT VM.
- */
- if (((args->flags & I915_EXEC_SECURE) &&
- (i == (args->buffer_count - 1))))
- bind_vm = &dev_priv->gtt.base;
-
obj = list_first_entry(&objects,
struct drm_i915_gem_object,
obj_exec_link);
@@ -1387,25 +1380,30 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
/* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
* batch" bit. Hence we need to pin secure batches into the global gtt.
* hsw should have this fixed, but bdw mucks it up again. */
- if (flags & I915_DISPATCH_SECURE &&
- !batch_obj->has_global_gtt_mapping) {
- /* When we have multiple VMs, we'll need to make sure that we
- * allocate space first */
- struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj);
- BUG_ON(!vma);
- vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND);
- }
+ if (flags & I915_DISPATCH_SECURE) {
+ /*
+ * So on first glance it looks freaky that we pin the batch here
+ * outside of the reservation loop. But:
+ * - The batch is already pinned into the relevant ppgtt, so we
+ * already have the backing storage fully allocated.
+ * - No other BO uses the global gtt (well contexts, but meh),
+ * so we don't really have issues with mutliple objects not
+ * fitting due to fragmentation.
+ * So this is actually safe.
+ */
+ ret = i915_gem_obj_ggtt_pin(batch_obj, 0, 0);
+ if (ret)
+ goto err;
- if (flags & I915_DISPATCH_SECURE)
exec_start += i915_gem_obj_ggtt_offset(batch_obj);
- else
+ } else
exec_start += i915_gem_obj_offset(batch_obj, vm);
ret = legacy_ringbuffer_submission(dev, file, ring, ctx,
- args, &eb->vmas, batch_obj, exec_start, flags);
- if (ret)
- goto err;
+ args, &eb->vmas, batch_obj, exec_start, flags);
+ if (flags & I915_DISPATCH_SECURE)
+ i915_gem_object_ggtt_unpin(batch_obj);
err:
/* the request owns the ref now */
i915_gem_context_unreference(ctx);
--
2.0.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
