[Intel-gfx] [PATCH v5 1/2] drm/i915: Tidy workaround batch buffer emission

[Chris Wilson]

On Thu, Feb 16, 2017 at 01:54:02PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Use the "*batch++ = " style as in the ring emission for better
> readability and also simplify the logic a bit by consolidating
> the offset and size calculations and overflow checking. The
> latter is a programming error so it is not required to check
> for it after each write to the object, but rather do it once the
> whole state has been written and fail the driver if something
> went wrong.
> 
> v2: Rebase.
> 
> v3: Keep track of offsets and sizes in bytes for simplicity
>     and rename function pointer variable to _fn suffix.
>     (Chris Wilson)
> 
> v4: Fix size calc broken in v3 and add alignment warning. (Chris Wilson)
> 
> v5: Fix return code.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> (v2)
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---

> +static u32 *gen9_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
>  {
>  	/* Pad to end of cacheline */
> -	while (index % CACHELINE_DWORDS)
> -		wa_ctx_emit(batch, index, MI_NOOP);
> +	while ((unsigned long)batch % CACHELINE_BYTES)
> +		*batch++ = MI_NOOP;
>  
> -	return wa_ctx_end(wa_ctx, *offset = index, CACHELINE_DWORDS);
> +	return batch;
>  }
>  
> -static int gen9_init_perctx_bb(struct intel_engine_cs *engine,
> -			       struct i915_wa_ctx_bb *wa_ctx,
> -			       uint32_t *batch,
> -			       uint32_t *offset)
> +static u32 *gen9_init_perctx_bb(struct intel_engine_cs *engine, u32 *batch)
>  {
> -	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
> +	*batch++ = MI_BATCH_BUFFER_END;
>  
> -	wa_ctx_emit(batch, index, MI_BATCH_BUFFER_END);
> -
> -	return wa_ctx_end(wa_ctx, *offset = index, 1);
> +	return batch;
>  }

> +	/*
> +	 * Emit the two workaround batch buffers, recording the offset from the
> +	 * start of the workaround batch buffer object for each and their
> +	 * respective sizes.
> +	 */
> +	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))) {
> +			ret = -EINVAL;
>  			goto out;
> +		}

Looks like a trap! If we add a third wa bb and forget to align the end
of the second.

Not a problem today, so
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre