[Intel-gfx] [PATCH v4 6/6] drm/i915/gen8: Add WaRsRestoreWithPerCtxtBb workaround

[Chris Wilson]

On Tue, Jun 16, 2015 at 08:25:25PM +0100, Arun Siluvery wrote:
> v3: Length defined in current definitions of LRM, LRR instructions is not correct.
> Correct and use existing definitions and also move them out of command parser
> placeholder to appropriate place.

Not that it wasn't correct. Common form has to be use 0 for instructions
whose length vary between platforms and then to fill in (X - 2) as
required. (At least that is what we do in userspace.)

> +	/* WaRsRestoreWithPerCtxtBb:bdw,chv */
> +	cmd[index++] = MI_LOAD_REGISTER_IMM(1);
> +	cmd[index++] = INSTPM;
> +	cmd[index++] = _MASKED_BIT_DISABLE(INSTPM_FORCE_ORDERING);
> +
> +	flags = MI_ATOMIC_MEMORY_TYPE_GGTT |
> +		MI_ATOMIC_INLINE_DATA |
> +		MI_ATOMIC_CS_STALL |
> +		MI_ATOMIC_RETURN_DATA_CTL |
> +		MI_ATOMIC_MOVE;
> +
> +	cmd[index++] = MI_ATOMIC(5) | flags;

This is very inconsistent style, and indeed flags doesn't make the code
any more readable than simply specifying each in the cmd.

> +	cmd[index++] = scratch_addr;
> +	cmd[index++] = 0;
> +	cmd[index++] = _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING);
> +	cmd[index++] = _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING);
> +
> +	/*
> +	 * BSpec says MI_LOAD_REGISTER_MEM, MI_LOAD_REGISTER_REG and
> +	 * MI_BATCH_BUFFER_END instructions in this sequence need to be
> +	 * in the same cacheline. To satisfy this case even if more WA are
> +	 * added in future, pad current cacheline and start remaining sequence
> +	 * in new cacheline.
> +	 */
> +	while (((unsigned long) (cmd + index) % CACHELINE_BYTES) != 0)
> +		cmd[index++] = MI_NOOP;
> +
> +	cmd[index++] = MI_LOAD_REGISTER_MEM_GEN8 |
> +		MI_LRM_USE_GLOBAL_GTT |
> +		MI_LRM_ASYNC_MODE_ENABLE;

Use brackets to force alignment when it's ugly like this.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre