[Intel-gfx] [PATCH] drm/i915/cnl: Avoid WA_SET_BIT_MASK on write-only CNL_HDC_CHICKEN0.

[Rodrigo Vivi]

On Wed, Sep 27, 2017 at 09:08:10PM +0000, Oscar Mateo wrote:
> 
> 
> On 09/27/2017 02:01 PM, Rodrigo Vivi wrote:
> > On CNL, HDC_CHICKEN0 "is write-only from LRI command.
> > However, it is readable for context save."
> > 
> > So we have no ways to check the coherency by reading it back on
> > our tests.
> > 
> > So let's just write that bit directly without saving it to
> > dev_priv->workarounds.
> 
> Wait, it's not that simple: CNL_HDC_CHICKEN0 lives in the context image, so
> you are going to revert the value as soon as you restore the next context
> (you want to save it in dev_priv->workarounds so that
> intel_ring_workarounds_emit applies it to every newly created context).

hm... that makes sense...

I was only with the WA description in my head that states to set only once at boot:

" To avoid a potential hang condition with TLB invalidation
driver should enable masked bit 5 of MMIO 0xE5F0 at boot."

But apparently just a bad description in spec, right?!

If this is the case probably dev_priv->workarouds will need another component
"permission" to handle write-only registers and avoid reading that on debugfs?
Or any thing smarter?

> 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> > Fixes: acfb5554c769 ("drm/i915/cnl: WaForceContextSaveRestoreNonCoherent")
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index a28e2a864cf1..047ba0a0308d 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -1251,8 +1251,8 @@ static int cnl_init_workarounds(struct intel_engine_cs *engine)
> >   			    GAMT_CHKN_DISABLE_I2M_CYCLE_ON_WR_PORT));
> >   	/* WaForceContextSaveRestoreNonCoherent:cnl */
> > -	WA_SET_BIT_MASKED(CNL_HDC_CHICKEN0,
> > -			  HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT);
> > +	I915_WRITE(CNL_HDC_CHICKEN0,
> > +		   _MASKED_BIT_ENABLE(HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT));
> >   	/* WaThrottleEUPerfToAvoidTDBackPressure:cnl(pre-prod) */
> >   	if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0))
>