Re: [PATCH 1/2] drm/i915: Drop WARN_ON(flags) from ppgtt_bind_vma()

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, Mar 12, 2014 at 07:32:26PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We will call ppgtt_bind_vma() with flags != 0, so the WARN_ON(flags)
> is bogus. Kill it.
> 

This is not an appropriate commit message to change an invariant. The
case was true, and it apparently no longer holds. At the very least the
commit should have the SHA which changed the invariant, and preferably
an explanation as to why the invariant no longer holds ("is bogus"). I

The reason you have given to remove this WARN_ON can be used for any
assertion we ever hit and simply reiterates what the patch does.

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7727103..0dce6fc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1243,8 +1243,6 @@ ppgtt_bind_vma(struct i915_vma *vma,
>  	       enum i915_cache_level cache_level,
>  	       u32 flags)
>  {
> -	WARN_ON(flags);
> -
>  	vma->vm->insert_entries(vma->vm, vma->obj->pages, vma->node.start,
>  				cache_level);
>  }
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx