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

["Siluvery, Arun" <arun.siluvery@linux.intel.com>]

On 12/08/2015 16:41, Dave Gordon wrote:
> 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@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@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!
>

Hi Dave,

This patch can be ignored if we use below patch,
[Intel-gfx] [PATCH] lib/rendercopy_gen9: Setup Push constant    pointer 
before sending BTP commands
http://lists.freedesktop.org/archives/intel-gfx/2015-August/073483.html

I think the correct option would be to ignore this patch.

regards
Arun

> .Dave.
>
>

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