Re: [PATCH] drm/i915: Pass vmas to i915_error_object_create

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Oct 31, 2014 at 05:38:31PM +0200, Mika Kuoppala wrote:
> to move the interface to the ppgtt era.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 49 +++++++++++++++++++----------------
>  1 file changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index d17360b..a3e62f00 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -561,16 +561,20 @@ static void i915_error_state_free(struct kref *error_ref)
>  
>  static struct drm_i915_error_object *
>  i915_error_object_create(struct drm_i915_private *dev_priv,
> -			 struct drm_i915_gem_object *src,
> -			 struct i915_address_space *vm)
> +			 struct i915_vma *vma)
>  {
>  	struct drm_i915_error_object *dst;
> -	struct i915_vma *vma = NULL;
> +	struct drm_i915_gem_object *src;
>  	int num_pages;
>  	bool use_ggtt;
>  	int i = 0;
>  	u32 reloc_offset;
>  
> +	if (WARN_ON(vma == NULL))
> +		return NULL;
> +
> +	src = vma->obj;
> +
>  	if (src == NULL || src->pages == NULL)
>  		return NULL;
>  
> @@ -580,23 +584,21 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
>  	if (dst == NULL)
>  		return NULL;
>  
> -	if (i915_gem_obj_bound(src, vm))
> -		dst->gtt_offset = i915_gem_obj_offset(src, vm);
> +	if (i915_gem_obj_bound(src, vma->vm))

If we have a vma, it's bound. At least after execbuf and before retiring
the cmds. So with the above early exit check (vma == NULL) you can remove
this.

> +		dst->gtt_offset = i915_gem_obj_offset(src, vma->vm);

Just look at the vma drm_mm node instead of walking the per-obj vma list
again.

>  	else
>  		dst->gtt_offset = -1;
>  
>  	reloc_offset = dst->gtt_offset;
> -	if (i915_is_ggtt(vm))
> -		vma = i915_gem_obj_to_ggtt(src);
>  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> -		   vma && (vma->bound & GLOBAL_BIND) &&
> -		   reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
> +		    (vma->bound & GLOBAL_BIND) &&
> +		    reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end);
>  
>  	/* Cannot access stolen address directly, try to use the aperture */
>  	if (src->stolen) {
>  		use_ggtt = true;
>  
> -		if (!(vma && vma->bound & GLOBAL_BIND))
> +		if (!(vma->bound & GLOBAL_BIND))
>  			goto unwind;
>  
>  		reloc_offset = i915_gem_obj_ggtt_offset(src);
> @@ -658,13 +660,16 @@ unwind:
>  	kfree(dst);
>  	return NULL;
>  }
> -#define i915_error_ggtt_object_create(dev_priv, src) \
> -	i915_error_object_create((dev_priv), (src), &(dev_priv)->gtt.base)

I would have left this little helper to cut down on diff size, but that's
really just a bikeshed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx