Re: [PATCH v2 3/4] drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> In Indirect context w/a batch buffer,
> +WaFlushCoherentL3CacheLinesAtContextSwitch:bdw

s/bdw/skl ?

>
> v2: address static checker warning where unsigned value was checked for
> less than zero which is never true.
>
Add                                   ^^ (Dan Carpenter)

> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

And remove this line as this would mean the workaround/bug
in question would be reported by Dan.

> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6a0b128..7536682 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1253,6 +1253,7 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>  				    uint32_t *const batch,
>  				    uint32_t *offset)
>  {
> +	int ret;
>  	struct drm_device *dev = ring->dev;
>  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>  
> @@ -1261,6 +1262,12 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>  	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
>  		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
>  
> +	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
> +	ret = gen8_emit_flush_coherentl3_wa(ring, batch, index);

Not shown in this patch but the above function assumes default
value for GEN8_L3SQCREG4 which doesn't match what we have by
default.

This is due to  skl_init_clock_gating() setting up one bit
in this register.

I think the proper way to fix this would be remove the
write from skl_init_clock_gating() and setup all
the bits in this register, even the default ones with
WA_SET_BIT() in gen9_init_workarounds(). 

And then search the default value out from the wa list,
when you build the batch.

But if you choose to go with default skl value of
0x48400000, make a comment to intel_pm.c and also
the gen8_emit_flush_coherentl3_wa() that you
have a dependency.

-Mika

> +	if (ret < 0)
> +		return ret;
> +	index = ret;
> +


>  	/* Pad to end of cacheline */
>  	while (index % CACHELINE_DWORDS)
>  		wa_ctx_emit(batch, MI_NOOP);
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx