On 09/27/2017 02:17 PM, Rodrigo Vivi wrote:
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?


Well, to begin with, the current debugfs interface does not make a lot of sense for things that live inside the context image. That's why Chris sent this patch earlier:

"igt/gem_workarounds: Read the workaround registers from the active context"

But this depends on LRI commands and thus makes it impossible to work with this particular register :/

Maybe create a context, force the HW to switch in & out of it and then a debugfs interface to read the WAs from inside the context image? Another possibility: in CNL+, the good old MI_SET_CONTEXT command is back, so maybe it can be used imaginatively to retrieve the context image :)

In any case, this looks like a lot of work only to debug that one WA is applied correctly...

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
Fixes: acfb5554c769 ("drm/i915/cnl: WaForceContextSaveRestoreNonCoherent")
Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Cc: Oscar Mateo <oscar.ma...@intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.v...@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))

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

Reply via email to