Re: [PATCH] drm/i915/cnl: WaDisableGatherAtSetShaderCommonSlice

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Rodrigo Vivi (2017-10-05 21:22:32)
> Purely empirical. I don't have a better description of the need
> of this workaround. It is not on BSpec and it is not on wa_database.
> 
> However it brings a huge stability to CNL and fix many issues that
> Mesa was facing.
> 
> Empirical history: when Rafael and I were trying to understand
> the misterious WaSendPushConstantsFromMMIO we just had a description
> of it that was "If not using RS, we must send two MMIO registers at
> context create to trigger push constants at 3D primitive"
> 
> And in another place we just saw that WaSendPushConstantsFromMMIO
> was only adding COMMON_SLICE_CHICKEN2 in a white list.
> 
> So we looked to the programmin notes of COMMON_SLICE_CHICKEN2
> and we notice that this bit 12 is marked in association with
> 2 other MMIO registers for SKL+. Apparently for SKL+ we should
> check few MMIOs to decide for set or reset of this bit 12.
> 
> Also "gather" is related to gather and packing of consntant elements
> into "push constants".
> 
> So we give a shot with this workaround in place and achieved
> a good stability.
> 
> Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7d6da130b184..aa1705319d71 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1231,6 +1231,23 @@ static u32 *gen9_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
>         return batch;
>  }
>  
> +static u32 *gen10_init_indirectctx_bb(struct intel_engine_cs *engine,
> +                                     u32 *batch)
> +{
> +       /* WaDisableGatherAtSetShaderCommonSlice:cnl */
> +       *batch++ = MI_LOAD_REGISTER_IMM(1);
> +       *batch++ = i915_mmio_reg_offset(COMMON_SLICE_CHICKEN2);
> +       *batch++ = _MASKED_BIT_DISABLE(
> +                       GEN9_DISABLE_GATHER_AT_SET_SHADER_COMMON_SLICE);
> +       *batch++ = MI_NOOP;

These are context registers, so why not just set them in the context
image? If user space chooses to fiddle with them (first they must be in
the nonpriv section) it will do so with good reason (one presumes).

You should get the same effect by combining this with the other
COMMON_SLICE_CHICKEN2 setup we do in cnl_init_workarounds().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx