[Intel-gfx] [PATCH v1 2/2] drm/i915/gen9: Disable gather at set shader bit

[Dave Gordon]

On 11/08/15 15:44, Arun Siluvery wrote:
>  From Gen9, Push constant instruction parsing behaviour varies according to
> whether set shader is enabled or not. If we want legacy behaviour then it
> can be achieved by disabling set shader.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89959
>
> Cc: Ben Widawsky <benjamin.widawsky at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++++++++++
>   2 files changed, 15 insertions(+)

[snip]

> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index cf61262..7d284ed 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -983,6 +983,16 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>   		tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
>   	WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
>
> +	/* Chicken bits to disable set shader is in multiple places,
> +	 * set bits in all required registers to disable it correctly
> +	 */
> +	WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
> +	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
> +	    (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0))
> +		WA_SET_BIT_MASKED(RS_CHICKEN, RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
> +	else
> +		WA_SET_BIT_MASKED(CS_RCS_BE, CS_RCS_DISABLE_GATHER_AT_SHADER);
> +
>   	return 0;
>   }

This workaround isn't tagged with a specific /* WaXyz:chip */ comment.
Also, the style isn't consistent with the other paragraphs earlier in 
this function: those have braces round the body part even when there's 
only one line of code, possibly to make it clear where the WA comment
applies (of course, this is why the buggy WA_REG() macro wasn't spotted 
earlier).

So, maybe prettify this a bit, if possible? The code actually looks 
correct, just ugly.

Oh, and keep patch 1 even if you decide to abandon this one!

.Dave.