Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

2015-07-14 Thread Mika Kuoppala 

Hi Dave,

Dave Gordon david.s.gor...@intel.com writes:

 On 13/07/15 14:16, Mika Kuoppala wrote:
 Arun Siluvery arun.siluv...@linux.intel.com writes:

 In Indirect context w/a batch buffer,
 +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

 v2: SKL revision id was used for BXT, copy paste error found during
 internal review (Bob Beckett).

 Cc: Robert Beckett robert.beck...@intel.com
 Cc: Imre Deak imre.d...@intel.com
 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
 ---
   drivers/gpu/drm/i915/intel_lrc.c| 9 +
   drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
   2 files changed, 14 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
 b/drivers/gpu/drm/i915/intel_lrc.c
 index 58247f0..61ed92b 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.c
 +++ b/drivers/gpu/drm/i915/intel_lrc.c
 @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct 
 intel_engine_cs *ring,
 struct drm_device *dev = ring-dev;
 uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);

 +   /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
 +   if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
 +   (IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0))) {
 +   wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
 +   wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
 +   wa_ctx_emit(batch, 
 _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
 +   wa_ctx_emit(batch, MI_NOOP);
 +   }
 +
 /* WaDisableCtxRestoreArbitration:skl,bxt */
 if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_D0)) ||
 (IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0)))
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index af7c12e..66dde7f 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct 
 intel_engine_cs *ring)
 /* 
 WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
 WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
   GEN9_RHWO_OPTIMIZATION_DISABLE);
 -   WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
 - DISABLE_PIXEL_MASK_CAMMING);
 +   /*
 +* WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be 
 set,
 +* but that register is write only hence it is set
 +* in per ctx batch buffer

 Why the need to move to per ctx bb? Why the write would not stick
 with this? Is the rationale that we get fails with the gem_workarounds?

 If so, perhaps we need a write_only marker for workaround list?

 -Mika

 Surely it's the fact that it's a context-saved register that matters, 
 rather than it being write-only (generally we treat masked registers as 
 write-only anyway, since the masking means that you can set or clear a 
 bit without having to read the other bits first).


But with workaround list, we push these writes into the ringbuffer
of the context being created. So I still fail to see how they
would end up being out of context.

The per ctx bb would make sense if this register was not part of
contexts and thus would not be saved. Is this the case here?

-Mika

 Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context 
 register, so it can be set directly rather than having to be done in a 
 batchbuffer.

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

Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

2015-07-14 Thread Dave Gordon 

On 13/07/15 14:16, Mika Kuoppala wrote:

Arun Siluvery arun.siluv...@linux.intel.com writes:


In Indirect context w/a batch buffer,
+WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).

Cc: Robert Beckett robert.beck...@intel.com
Cc: Imre Deak imre.d...@intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
  drivers/gpu/drm/i915/intel_lrc.c| 9 +
  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
  2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 58247f0..61ed92b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs 
*ring,
struct drm_device *dev = ring-dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);

+   /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
+   if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
+   (IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0))) {
+   wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
+   wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
+   wa_ctx_emit(batch, 
_MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
+   wa_ctx_emit(batch, MI_NOOP);
+   }
+
/* WaDisableCtxRestoreArbitration:skl,bxt */
if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_D0)) ||
(IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0)))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af7c12e..66dde7f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs 
*ring)
/* 
WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
  GEN9_RHWO_OPTIMIZATION_DISABLE);
-   WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
- DISABLE_PIXEL_MASK_CAMMING);
+   /*
+* WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be 
set,
+* but that register is write only hence it is set
+* in per ctx batch buffer


Why the need to move to per ctx bb? Why the write would not stick
with this? Is the rationale that we get fails with the gem_workarounds?

If so, perhaps we need a write_only marker for workaround list?

-Mika


Surely it's the fact that it's a context-saved register that matters, 
rather than it being write-only (generally we treat masked registers as 
write-only anyway, since the masking means that you can set or clear a 
bit without having to read the other bits first).


Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context 
register, so it can be set directly rather than having to be done in a 
batchbuffer.


.Dave.

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

Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

2015-07-13 Thread Mika Kuoppala 
Arun Siluvery arun.siluv...@linux.intel.com writes:

 In Indirect context w/a batch buffer,
 +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

 v2: SKL revision id was used for BXT, copy paste error found during
 internal review (Bob Beckett).

 Cc: Robert Beckett robert.beck...@intel.com
 Cc: Imre Deak imre.d...@intel.com
 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_lrc.c| 9 +
  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
  2 files changed, 14 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
 b/drivers/gpu/drm/i915/intel_lrc.c
 index 58247f0..61ed92b 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.c
 +++ b/drivers/gpu/drm/i915/intel_lrc.c
 @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs 
 *ring,
   struct drm_device *dev = ring-dev;
   uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
  
 + /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
 + if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
 + (IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0))) {
 + wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
 + wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
 + wa_ctx_emit(batch, 
 _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
 + wa_ctx_emit(batch, MI_NOOP);
 + }
 +
   /* WaDisableCtxRestoreArbitration:skl,bxt */
   if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_D0)) ||
   (IS_BROXTON(dev)  (INTEL_REVID(dev) == BXT_REVID_A0)))
 diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
 b/drivers/gpu/drm/i915/intel_ringbuffer.c
 index af7c12e..66dde7f 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs 
 *ring)
   /* 
 WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
   WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
 GEN9_RHWO_OPTIMIZATION_DISABLE);
 - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
 -   DISABLE_PIXEL_MASK_CAMMING);
 + /*
 +  * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be 
 set,
 +  * but that register is write only hence it is set
 +  * in per ctx batch buffer

Why the need to move to per ctx bb? Why the write would not stick
with this? Is the rationale that we get fails with the gem_workarounds?

If so, perhaps we need a write_only marker for workaround list?

-Mika


 +  */
   }
  
   if ((IS_SKYLAKE(dev)  INTEL_REVID(dev) = SKL_REVID_C0) ||
 -- 
 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