Re: [Intel-gfx] [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a

2015-08-04 Thread Siluvery, Arun 

On 04/08/2015 00:21, Ben Widawsky wrote:

On Mon, Aug 03, 2015 at 08:24:57PM +0100, Arun Siluvery wrote:

This WA is implemented in init_context as well as WA batch init.
There are also some dependent bits need to be set in other registers
for this to be complete.

Cc: Ben Widawsky benjamin.widaw...@intel.com
Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_reg.h |  3 +++
  drivers/gpu/drm/i915/intel_lrc.c|  8 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 16 
  3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8991cd5..24b8bb9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1720,7 +1720,9 @@ enum skl_disp_power_wells {
  #define   MEM_DISPLAY_A_TRICKLE_FEED_DISABLE (12) /* 830/845 only */
  #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (12) /* 85x only */
  #define FW_BLC0x020d8
+#define   GEN9_DISABLE_GATHER_AT_SET_SHADER(17)
  #define FW_BLC2   0x020dc
+#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (12)


Neither of these belong here. BLC is for backlight. Create a new define if we
don't have one.

#define RS_CHICKEN  0x20dc

I thought of reusing existing define but created a new one as you suggested.




  #define FW_BLC_SELF   0x020e0 /* 915+ only */
  #define   FW_BLC_SELF_EN_MASK  (131)
  #define   FW_BLC_SELF_FIFO_MASK(116) /* 945 only */
@@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
  # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC((110) | (126))
  # define GEN9_RHWO_OPTIMIZATION_DISABLE   (114)
  #define COMMON_SLICE_CHICKEN2 0x7014
+#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (112)
  # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (10)

  #define HIZ_CHICKEN   0x7018
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9faad82..d3a03f3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1292,6 +1292,14 @@ 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);

+   /* WA to reset disable gather at set shader slice bit */
+   if (IS_SKYLAKE(dev)) {
+   wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
+   wa_ctx_emit(batch, index, COMMON_SLICE_CHICKEN2);
+   wa_ctx_emit(batch, index,
+   
_MASKED_BIT_DISABLE(GEN9_DISABLE_GATHER_SET_SHADER_SLICE));
+   }
+


Shouldn't this be for BXT as well? Also, why bother with the revid check below
and not here?


spec says only SKL+




/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
(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 dcd1b8f..4fc4b5e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -985,6 +985,15 @@ static int gen9_init_workarounds(struct intel_engine_cs 
*ring)
tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);

+   /* WA to gather at set shader - skl,bxt
+* These are dependent bits need to be set for the WA.
+*/
+   if (IS_SKYLAKE(dev)  (INTEL_REVID(dev)  SKL_REVID_D0) ||
+   (IS_BROXTON(dev)  INTEL_REVID(dev)  BXT_REVID_A0)) {
+   WA_SET_BIT_MASKED(FW_BLC, GEN9_DISABLE_GATHER_AT_SET_SHADER);
+   WA_SET_BIT_MASKED(FW_BLC2, 
GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
+   }
+
return 0;
  }

@@ -1068,6 +1077,13 @@ static int skl_init_workarounds(struct intel_engine_cs 
*ring)
  HDC_FENCE_DEST_SLM_DISABLE |
  HDC_BARRIER_PERFORMANCE_DISABLE);

+   /* WA to Disable gather at set shader - skl
+* This bit needs to be reset in Per ctx WA batch and it is also
+* dependent on other bits in different register, all of them need
+* be set for the WA to be complete.
+*/
+   WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, 
GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
+
return skl_tune_iz_hashing(ring);
  }



I wouldn't set both 20dc, and 20d8, I am not sure what implication it has.
Instead, set or read bit 15 of 0x20e0 and then just set one. To me, it seems
like the best way to do this is to set 115 of 0x20e0, and then use bit 2 of
0x20dc for the workaround. We don't need per context controls of something we
have to disable always anyway.


changed it to use 0x20e0

regards
Arun


___
Intel-gfx mailing list

Re: [Intel-gfx] [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a

2015-08-03 Thread Ben Widawsky 
On Mon, Aug 03, 2015 at 08:24:57PM +0100, Arun Siluvery wrote:
 This WA is implemented in init_context as well as WA batch init.
 There are also some dependent bits need to be set in other registers
 for this to be complete.
 
 Cc: Ben Widawsky benjamin.widaw...@intel.com
 Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com
 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_reg.h |  3 +++
  drivers/gpu/drm/i915/intel_lrc.c|  8 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 16 
  3 files changed, 27 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 8991cd5..24b8bb9 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -1720,7 +1720,9 @@ enum skl_disp_power_wells {
  #define   MEM_DISPLAY_A_TRICKLE_FEED_DISABLE (12) /* 830/845 only */
  #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (12) /* 85x only */
  #define FW_BLC   0x020d8
 +#define   GEN9_DISABLE_GATHER_AT_SET_SHADER(17)
  #define FW_BLC2  0x020dc
 +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (12)

Neither of these belong here. BLC is for backlight. Create a new define if we
don't have one.

#define RS_CHICKEN  0x20dc

  #define FW_BLC_SELF  0x020e0 /* 915+ only */
  #define   FW_BLC_SELF_EN_MASK  (131)
  #define   FW_BLC_SELF_FIFO_MASK(116) /* 945 only */
 @@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
  # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC   ((110) | (126))
  # define GEN9_RHWO_OPTIMIZATION_DISABLE  (114)
  #define COMMON_SLICE_CHICKEN20x7014
 +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (112)
  # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE(10)
  
  #define HIZ_CHICKEN  0x7018
 diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
 b/drivers/gpu/drm/i915/intel_lrc.c
 index 9faad82..d3a03f3 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.c
 +++ b/drivers/gpu/drm/i915/intel_lrc.c
 @@ -1292,6 +1292,14 @@ 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);
  
 + /* WA to reset disable gather at set shader slice bit */
 + if (IS_SKYLAKE(dev)) {
 + wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
 + wa_ctx_emit(batch, index, COMMON_SLICE_CHICKEN2);
 + wa_ctx_emit(batch, index,
 + 
 _MASKED_BIT_DISABLE(GEN9_DISABLE_GATHER_SET_SHADER_SLICE));
 + }
 +

Shouldn't this be for BXT as well? Also, why bother with the revid check below
and not here?

   /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
   if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
   (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 dcd1b8f..4fc4b5e 100644
 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
 +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
 @@ -985,6 +985,15 @@ static int gen9_init_workarounds(struct intel_engine_cs 
 *ring)
   tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
   WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
  
 + /* WA to gather at set shader - skl,bxt
 +  * These are dependent bits need to be set for the WA.
 +  */
 + if (IS_SKYLAKE(dev)  (INTEL_REVID(dev)  SKL_REVID_D0) ||
 + (IS_BROXTON(dev)  INTEL_REVID(dev)  BXT_REVID_A0)) {
 + WA_SET_BIT_MASKED(FW_BLC, GEN9_DISABLE_GATHER_AT_SET_SHADER);
 + WA_SET_BIT_MASKED(FW_BLC2, 
 GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
 + }
 +
   return 0;
  }
  
 @@ -1068,6 +1077,13 @@ static int skl_init_workarounds(struct intel_engine_cs 
 *ring)
 HDC_FENCE_DEST_SLM_DISABLE |
 HDC_BARRIER_PERFORMANCE_DISABLE);
  
 + /* WA to Disable gather at set shader - skl
 +  * This bit needs to be reset in Per ctx WA batch and it is also
 +  * dependent on other bits in different register, all of them need
 +  * be set for the WA to be complete.
 +  */
 + WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, 
 GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
 +
   return skl_tune_iz_hashing(ring);
  }
  

I wouldn't set both 20dc, and 20d8, I am not sure what implication it has.
Instead, set or read bit 15 of 0x20e0 and then just set one. To me, it seems
like the best way to do this is to set 115 of 0x20e0, and then use bit 2 of
0x20dc for the workaround. We don't need per context controls of something we
have to disable always anyway.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a

2015-08-03 Thread Arun Siluvery 
This WA is implemented in init_context as well as WA batch init.
There are also some dependent bits need to be set in other registers
for this to be complete.

Cc: Ben Widawsky benjamin.widaw...@intel.com
Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 drivers/gpu/drm/i915/intel_lrc.c|  8 
 drivers/gpu/drm/i915/intel_ringbuffer.c | 16 
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8991cd5..24b8bb9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1720,7 +1720,9 @@ enum skl_disp_power_wells {
 #define   MEM_DISPLAY_A_TRICKLE_FEED_DISABLE (12) /* 830/845 only */
 #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (12) /* 85x only */
 #define FW_BLC 0x020d8
+#define   GEN9_DISABLE_GATHER_AT_SET_SHADER(17)
 #define FW_BLC20x020dc
+#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (12)
 #define FW_BLC_SELF0x020e0 /* 915+ only */
 #define   FW_BLC_SELF_EN_MASK  (131)
 #define   FW_BLC_SELF_FIFO_MASK(116) /* 945 only */
@@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((110) | (126))
 # define GEN9_RHWO_OPTIMIZATION_DISABLE(114)
 #define COMMON_SLICE_CHICKEN2  0x7014
+#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (112)
 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE  (10)
 
 #define HIZ_CHICKEN0x7018
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9faad82..d3a03f3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1292,6 +1292,14 @@ 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);
 
+   /* WA to reset disable gather at set shader slice bit */
+   if (IS_SKYLAKE(dev)) {
+   wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
+   wa_ctx_emit(batch, index, COMMON_SLICE_CHICKEN2);
+   wa_ctx_emit(batch, index,
+   
_MASKED_BIT_DISABLE(GEN9_DISABLE_GATHER_SET_SHADER_SLICE));
+   }
+
/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
if ((IS_SKYLAKE(dev)  (INTEL_REVID(dev) = SKL_REVID_B0)) ||
(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 dcd1b8f..4fc4b5e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -985,6 +985,15 @@ static int gen9_init_workarounds(struct intel_engine_cs 
*ring)
tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
 
+   /* WA to gather at set shader - skl,bxt
+* These are dependent bits need to be set for the WA.
+*/
+   if (IS_SKYLAKE(dev)  (INTEL_REVID(dev)  SKL_REVID_D0) ||
+   (IS_BROXTON(dev)  INTEL_REVID(dev)  BXT_REVID_A0)) {
+   WA_SET_BIT_MASKED(FW_BLC, GEN9_DISABLE_GATHER_AT_SET_SHADER);
+   WA_SET_BIT_MASKED(FW_BLC2, 
GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
+   }
+
return 0;
 }
 
@@ -1068,6 +1077,13 @@ static int skl_init_workarounds(struct intel_engine_cs 
*ring)
  HDC_FENCE_DEST_SLM_DISABLE |
  HDC_BARRIER_PERFORMANCE_DISABLE);
 
+   /* WA to Disable gather at set shader - skl
+* This bit needs to be reset in Per ctx WA batch and it is also
+* dependent on other bits in different register, all of them need
+* be set for the WA to be complete.
+*/
+   WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, 
GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
+
return skl_tune_iz_hashing(ring);
 }
 
-- 
1.9.1

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