On Mon, Nov 23, 2015 at 03:12:35PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <[email protected]>
> 
> Current code moves _any_ VMA to the inactive list only when
> _all_ rendering on an object (so from any context or VM) has
> been completed.
> 
> This creates an un-natural situation where the context (and
> VM) destructors can run with VMAs still on the respective
> active list.
> 
> Change here is to move VMAs to the inactive list as the
> requests are getting retired.
> 
> Signed-off-by: Tvrtko Ursulin <[email protected]>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638
> Testcase: igt/gem_request_retire/retire-vma-not-inactive
> Cc: Daniel Vetter <[email protected]>
> Cc: Chris Wilson <[email protected]>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index cd7e102720f4..47a743246d2c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2413,17 +2413,32 @@ static void
>  i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
>  {
>       struct i915_vma *vma;
> +     struct i915_address_space *vm;
>  
>       RQ_BUG_ON(obj->last_read_req[ring] == NULL);
>       RQ_BUG_ON(!(obj->active & (1 << ring)));
>  
>       list_del_init(&obj->ring_list[ring]);
> -     i915_gem_request_assign(&obj->last_read_req[ring], NULL);
>  
>       if (obj->last_write_req && obj->last_write_req->ring->id == ring)
>               i915_gem_object_retire__write(obj);
>  
>       obj->active &= ~(1 << ring);
> +
> +     if (obj->last_read_req[ring]->ctx->ppgtt)
> +             vm = &obj->last_read_req[ring]->ctx->ppgtt->base;
> +     else
> +             vm = &obj->last_read_req[ring]->i915->gtt.base;
> +
> +     list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +             if (vma->vm == vm &&
> +                 vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL &&
> +                 !list_empty(&vma->mm_list))
> +                     list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
> +     }

This is only a partial solution since with schedulers and semaphores and a
few depencies on a given object, but in different vm you can still end up
with an object that is idle in a vm, but slipped through here.

Also, checking for the view type is some strange layering. Why that?
-Daniel

> +
> +     i915_gem_request_assign(&obj->last_read_req[ring], NULL);
> +
>       if (obj->active)
>               return;
>  
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to