Re: [PATCH v3 2/3] drm/i915: Do not WARN_ON in i915_vm_to_ppgtt

[Dave Gordon <david.s.gordon@intel.com>]

On 01/04/16 08:46, Joonas Lahtinen wrote:
> According to Chris, use of i915_vm_to_ppgtt is visible in benchmark
> unless WARN_ON is removed, so lets get rid of it.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d3ebb2f..0035dc3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3157,7 +3157,6 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj);
>   static inline struct i915_hw_ppgtt *
>   i915_vm_to_ppgtt(struct i915_address_space *vm)
>   {
> -	WARN_ON(i915_is_ggtt(vm));
>   	return container_of(vm, struct i915_hw_ppgtt, base);
>   }

I shouldn't think the actual check (i915_is_ggtt()) is slowing things 
down, so it must be the overhead of an unactivated WARN_ON(), which 
probably generates quite a bit of code and may disable optimisations, 
even when the code block isn't entered. As an alternative (if you *want* 
things to break if this is used incorrectly) you could have it return 
NULL for that case:

   static inline struct i915_hw_ppgtt *
   i915_vm_to_ppgtt(struct i915_address_space *vm)
   {
     return i915_is_ggtt(vm) ? NULL :
            container_of(vm, struct i915_hw_ppgtt, base);
   }

Caller will then very likely get an OOPS!

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx