Re: [PATCH v2] drm/i915/lrc: Remove superfluous WARN_ON

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 26/01/2018 12:18, Chris Wilson wrote:
> Remove the WARN_ON(ce->state) inside the static function only called
> when ce->state == NULL and downgrade the w/a batch setup warning into a
> developer only mode (GEM_WARN_ON).
> 
> v2: Move the deferred alloc guard into the callee, eliminating the need
> for the WARN_ON:
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-1 (-1)
> Function                                     old     new   delta
> execlists_context_pin                       1819    1818      -1
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 1a3174371c8e..c8a11315b357 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1091,11 +1091,9 @@ execlists_context_pin(struct intel_engine_cs *engine,
>   		goto out;
>   	GEM_BUG_ON(!ce->pin_count); /* no overflow please! */
>   
> -	if (!ce->state) {
> -		ret = execlists_context_deferred_alloc(ctx, engine);
> -		if (ret)
> -			goto err;
> -	}
> +	ret = execlists_context_deferred_alloc(ctx, engine);
> +	if (ret)
> +		goto err;
>   	GEM_BUG_ON(!ce->state);
>   
>   	ret = __context_pin(ctx, ce->state);
> @@ -1419,7 +1417,8 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine)
>   	 */
>   	for (i = 0; i < ARRAY_SIZE(wa_bb_fn); i++) {
>   		wa_bb[i]->offset = batch_ptr - batch;
> -		if (WARN_ON(!IS_ALIGNED(wa_bb[i]->offset, CACHELINE_BYTES))) {
> +		if (GEM_WARN_ON(!IS_ALIGNED(wa_bb[i]->offset,
> +					    CACHELINE_BYTES))) {
>   			ret = -EINVAL;
>   			break;
>   		}
> @@ -2271,7 +2270,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
>   	struct intel_ring *ring;
>   	int ret;
>   
> -	WARN_ON(ce->state);
> +	if (ce->state)
> +		return 0;
>   
>   	context_size = round_up(engine->context_size, I915_GTT_PAGE_SIZE);
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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