Re: [PATCH] drm/i915: Move VMAs to inactive as request are retired

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 23, 2015 at 03:12:35PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> 
> 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 <tvrtko.ursulin@xxxxxxxxx>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638
> Testcase: igt/gem_request_retire/retire-vma-not-inactive
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>  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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux