On Thu, Sep 11, 2014 at 11:39:46AM +0300, Mika Kuoppala wrote:
> When PPGGT is in use, we need to bind secure batches also to ggtt.
> We used to pin, exec and unpin. This trick worked as there was nothing
> competing in ggtt space and the ppggt vma was used to tracking.
> But this left the ggtt vma untracked and potentially it could vanish
> during the batch execution.
>
> Track ggtt vma properly by piggypacking it into the eb->vmas list
> just before batch submission is made. This ensures that vma gets
> into the active list properly.
>
> Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 1a0611b..06c71e9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -217,6 +217,10 @@ i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
>
> entry = vma->exec_entry;
>
> + /* Check if piggypacked ggtt batch entry */
> + if (!entry)
> + return;
> +
> if (entry->flags & __EXEC_OBJECT_HAS_FENCE)
> i915_gem_object_unpin_fence(obj);
>
> @@ -224,6 +228,8 @@ i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma)
> vma->pin_count--;
>
> entry->flags &= ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN);
> +
> + vma->exec_entry = NULL;
> }
>
> static void eb_destroy(struct eb_vmas *eb)
> @@ -970,7 +976,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> /* update for the implicit flush after a batch */
> obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
> }
> - if (entry->flags & EXEC_OBJECT_NEEDS_FENCE) {
> + if (entry && entry->flags & EXEC_OBJECT_NEEDS_FENCE) {
> obj->last_fenced_seqno = seqno;
> if (entry->flags & __EXEC_OBJECT_HAS_FENCE) {
> struct drm_i915_private *dev_priv = to_i915(ring->dev);
> @@ -1385,6 +1391,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> * 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) {
> + struct i915_vma *vma;
> /*
> * So on first glance it looks freaky that we pin the batch here
> * outside of the reservation loop. But:
> @@ -1399,6 +1406,18 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> if (ret)
> goto err;
>
> + vma = i915_gem_obj_to_ggtt(batch_obj);
> +
> + /* If there is no exec_entry in this stage, ggtt vma
> + * is not in the eb->vmas list. Piggypack our ggtt
> + * address to the list in order for it to be added as
> + * an active vma in do_execbuf()
> + */
> + if (vma->exec_entry == NULL) {
> + drm_gem_object_reference(&batch_obj->base);
> + list_add_tail(&vma->exec_list, &eb->vmas);
vma->exec_entry = memset(&dummy_exec_entry, 0, sizeof(dummy_exec_entry);
Then you can kill the additional if (entry) checks.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
