All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Add SKL Workarounds
@ 2015-08-03 19:24 Arun Siluvery
  2015-08-03 19:24 ` [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix Arun Siluvery
  2015-08-03 19:24 ` [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a Arun Siluvery
  0 siblings, 2 replies; 14+ messages in thread
From: Arun Siluvery @ 2015-08-03 19:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

The following WA are helping in resolving GPU hang observed
with gem_ringfill and fbo-depth-array tests.

Arun Siluvery (2):
  drm/i915:skl: Add WaEnableGapsTsvCreditFix
  drm/i915:gen9: Add disable gather at set shader w/a

 drivers/gpu/drm/i915/i915_reg.h         |  6 ++++++
 drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++++++
 drivers/gpu/drm/i915/intel_pm.c         |  6 ++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 16 ++++++++++++++++
 4 files changed, 36 insertions(+)

-- 
1.9.1

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-03 19:24 [PATCH v1 0/2] Add SKL Workarounds Arun Siluvery
@ 2015-08-03 19:24 ` Arun Siluvery
  2015-08-03 23:01   ` Ben Widawsky
  2015-08-03 19:24 ` [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a Arun Siluvery
  1 sibling, 1 reply; 14+ messages in thread
From: Arun Siluvery @ 2015-08-03 19:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 77967ca..8991cd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
 #define GEN7_MISCCPCTL			(0x9424)
 #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
 
+#define GEN8_GARBCNTL                   0xB004
+#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
+
 /* IVYBRIDGE DPF */
 #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
 #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c23cab6..9152113 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct drm_device *dev)
 		/* WaDisableLSQCROPERFforOCL:skl */
 		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
 			   GEN8_LQSC_RO_PERF_DIS);
+
+	/* WaEnableGapsTsvCreditFix:skl */
+	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
+		I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
+					   GEN9_GAPS_TSV_CREDIT_DISABLE));
+	}
 }
 
 static void bxt_init_clock_gating(struct drm_device *dev)
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-03 19:24 [PATCH v1 0/2] Add SKL Workarounds Arun Siluvery
  2015-08-03 19:24 ` [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix Arun Siluvery
@ 2015-08-03 19:24 ` Arun Siluvery
  2015-08-03 23:21   ` Ben Widawsky
  2015-08-04 10:21   ` [PATCH v2 " Arun Siluvery
  1 sibling, 2 replies; 14+ messages in thread
From: Arun Siluvery @ 2015-08-03 19:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

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.widawsky@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@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 (1<<2) /* 830/845 only */
 #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
 #define FW_BLC		0x020d8
+#define   GEN9_DISABLE_GATHER_AT_SET_SHADER    (1<<7)
 #define FW_BLC2		0x020dc
+#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
 #define FW_BLC_SELF	0x020e0 /* 915+ only */
 #define   FW_BLC_SELF_EN_MASK      (1<<31)
 #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
@@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
 # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
 #define COMMON_SLICE_CHICKEN2			0x7014
+#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
 
 #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));
+	}
+
 	/* 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-03 19:24 ` [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix Arun Siluvery
@ 2015-08-03 23:01   ` Ben Widawsky
  2015-08-04  8:58     ` Mika Kuoppala
  0 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2015-08-03 23:01 UTC (permalink / raw)
  To: Arun Siluvery; +Cc: intel-gfx

On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 77967ca..8991cd5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
>  #define GEN7_MISCCPCTL			(0x9424)
>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>  
> +#define GEN8_GARBCNTL                   0xB004
> +#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
> +
>  /* IVYBRIDGE DPF */
>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c23cab6..9152113 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct drm_device *dev)
>  		/* WaDisableLSQCROPERFforOCL:skl */
>  		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
>  			   GEN8_LQSC_RO_PERF_DIS);
> +
> +	/* WaEnableGapsTsvCreditFix:skl */
> +	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
> +		I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
> +					   GEN9_GAPS_TSV_CREDIT_DISABLE));
> +	}
>  }
>  
>  static void bxt_init_clock_gating(struct drm_device *dev)

FWIW, the docs make it sound like BIOS should be doing this. Did you verify we
actually don't have the bit set with more recent BKC?

Tested-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-03 19:24 ` [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a Arun Siluvery
@ 2015-08-03 23:21   ` Ben Widawsky
  2015-08-04  9:29     ` Siluvery, Arun
  2015-08-04 10:21   ` [PATCH v2 " Arun Siluvery
  1 sibling, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2015-08-03 23:21 UTC (permalink / raw)
  To: Arun Siluvery; +Cc: intel-gfx

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.widawsky@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@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 (1<<2) /* 830/845 only */
>  #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
>  #define FW_BLC		0x020d8
> +#define   GEN9_DISABLE_GATHER_AT_SET_SHADER    (1<<7)
>  #define FW_BLC2		0x020dc
> +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)

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      (1<<31)
>  #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
> @@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
>  # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
>  # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
>  #define COMMON_SLICE_CHICKEN2			0x7014
> +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
>  # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>  
>  #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 1<<15 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-03 23:01   ` Ben Widawsky
@ 2015-08-04  8:58     ` Mika Kuoppala
  2015-08-04  9:01       ` Siluvery, Arun
  0 siblings, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-04  8:58 UTC (permalink / raw)
  To: Ben Widawsky, Arun Siluvery; +Cc: intel-gfx

Ben Widawsky <benjamin.widawsky@intel.com> writes:

> On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
>> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>>  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>>  2 files changed, 9 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 77967ca..8991cd5 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
>>  #define GEN7_MISCCPCTL			(0x9424)
>>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>>  
>> +#define GEN8_GARBCNTL                   0xB004
>> +#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
>> +
>>  /* IVYBRIDGE DPF */
>>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index c23cab6..9152113 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct drm_device *dev)
>>  		/* WaDisableLSQCROPERFforOCL:skl */
>>  		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
>>  			   GEN8_LQSC_RO_PERF_DIS);
>> +
>> +	/* WaEnableGapsTsvCreditFix:skl */
>> +	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
>> +		I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
>> +					   GEN9_GAPS_TSV_CREDIT_DISABLE));
>> +	}
>>  }
>>  
>>  static void bxt_init_clock_gating(struct drm_device *dev)
>
> FWIW, the docs make it sound like BIOS should be doing this. Did you verify we
> actually don't have the bit set with more recent BKC?
>

I have pretty recent BIOS and the bit was not set.

> Tested-by: Ben Widawsky <ben@bwidawsk.net>
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90854
Tested-by: Mika Kuoppala <mika.kuoppala@intel.com>

> -- 
> 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 mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-04  8:58     ` Mika Kuoppala
@ 2015-08-04  9:01       ` Siluvery, Arun
  2015-08-05  9:17         ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Siluvery, Arun @ 2015-08-04  9:01 UTC (permalink / raw)
  To: Mika Kuoppala, Ben Widawsky; +Cc: intel-gfx

On 04/08/2015 09:58, Mika Kuoppala wrote:
> Ben Widawsky <benjamin.widawsky@intel.com> writes:
>
>> On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
>>> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_reg.h | 3 +++
>>>   drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>>>   2 files changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 77967ca..8991cd5 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
>>>   #define GEN7_MISCCPCTL			(0x9424)
>>>   #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
>>>
>>> +#define GEN8_GARBCNTL                   0xB004
>>> +#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
>>> +
>>>   /* IVYBRIDGE DPF */
>>>   #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
>>>   #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index c23cab6..9152113 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct drm_device *dev)
>>>   		/* WaDisableLSQCROPERFforOCL:skl */
>>>   		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
>>>   			   GEN8_LQSC_RO_PERF_DIS);
>>> +
>>> +	/* WaEnableGapsTsvCreditFix:skl */
>>> +	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
>>> +		I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
>>> +					   GEN9_GAPS_TSV_CREDIT_DISABLE));
>>> +	}
>>>   }
>>>
>>>   static void bxt_init_clock_gating(struct drm_device *dev)
>>
>> FWIW, the docs make it sound like BIOS should be doing this. Did you verify we
>> actually don't have the bit set with more recent BKC?
>>
>
> I have pretty recent BIOS and the bit was not set.

I checked about this, it should be done in driver.

regards
Arun

>
>> Tested-by: Ben Widawsky <ben@bwidawsk.net>
>> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
>>
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90854
> Tested-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
>> --
>> 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 mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-03 23:21   ` Ben Widawsky
@ 2015-08-04  9:29     ` Siluvery, Arun
  0 siblings, 0 replies; 14+ messages in thread
From: Siluvery, Arun @ 2015-08-04  9:29 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

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.widawsky@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@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 (1<<2) /* 830/845 only */
>>   #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
>>   #define FW_BLC		0x020d8
>> +#define   GEN9_DISABLE_GATHER_AT_SET_SHADER    (1<<7)
>>   #define FW_BLC2		0x020dc
>> +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
>
> 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      (1<<31)
>>   #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
>> @@ -5836,6 +5838,7 @@ enum skl_disp_power_wells {
>>   # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
>>   # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
>>   #define COMMON_SLICE_CHICKEN2			0x7014
>> +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
>>   # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>>
>>   #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 1<<15 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
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-03 19:24 ` [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a Arun Siluvery
  2015-08-03 23:21   ` Ben Widawsky
@ 2015-08-04 10:21   ` Arun Siluvery
  2015-08-04 21:06     ` Ben Widawsky
  2015-08-05 14:45     ` Mika Kuoppala
  1 sibling, 2 replies; 14+ messages in thread
From: Arun Siluvery @ 2015-08-04 10:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

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.

v2: behaviour of disable gather at set shader bit can be specified by
two different registers, use a better option (Ben).

Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
 drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8991cd5..8719a5a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1721,6 +1721,10 @@ enum skl_disp_power_wells {
 #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
 #define FW_BLC		0x020d8
 #define FW_BLC2		0x020dc
+#define GEN7_RS_CHICKEN  0x20DC
+#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
+#define GEN7_FF_SLICE_CHICKEN1    0x20E0
+#define   GEN9_PER_CTX_DISABLE_GATHER_CONTROL  (1<<15)
 #define FW_BLC_SELF	0x020e0 /* 915+ only */
 #define   FW_BLC_SELF_EN_MASK      (1<<31)
 #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
@@ -5836,6 +5840,7 @@ enum skl_disp_power_wells {
 # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
 # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
 #define COMMON_SLICE_CHICKEN2			0x7014
+#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
 # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
 
 #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));
+	}
+
 	/* 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..5e8e5f9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -985,6 +985,17 @@ 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(GEN7_FF_SLICE_CHICKEN1,
+				  GEN9_PER_CTX_DISABLE_GATHER_CONTROL);
+		WA_SET_BIT_MASKED(GEN7_RS_CHICKEN,
+				  GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
+	}
+
 	return 0;
 }
 
@@ -1068,6 +1079,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-04 10:21   ` [PATCH v2 " Arun Siluvery
@ 2015-08-04 21:06     ` Ben Widawsky
  2015-08-05 14:45     ` Mika Kuoppala
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Widawsky @ 2015-08-04 21:06 UTC (permalink / raw)
  To: Arun Siluvery; +Cc: intel-gfx

On Tue, Aug 04, 2015 at 11:21:53AM +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.
> 
> v2: behaviour of disable gather at set shader bit can be specified by
> two different registers, use a better option (Ben).
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
>  drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++++++++++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8991cd5..8719a5a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1721,6 +1721,10 @@ enum skl_disp_power_wells {
>  #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
>  #define FW_BLC		0x020d8
>  #define FW_BLC2		0x020dc
> +#define GEN7_RS_CHICKEN  0x20DC
> +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
> +#define GEN7_FF_SLICE_CHICKEN1    0x20E0
> +#define   GEN9_PER_CTX_DISABLE_GATHER_CONTROL  (1<<15)
>  #define FW_BLC_SELF	0x020e0 /* 915+ only */
>  #define   FW_BLC_SELF_EN_MASK      (1<<31)
>  #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
> @@ -5836,6 +5840,7 @@ enum skl_disp_power_wells {
>  # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
>  # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
>  #define COMMON_SLICE_CHICKEN2			0x7014
> +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
>  # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>  
>  #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));
> +	}
> +

The workaround is confusing because it says "SKL+, but explicitly excludes the
next GEN, which leads me to believe it either shouldn't be SKL+ (it should be
SKL), or we need it for BXT. We can always add it later if needed. Also, several
of the ancillary bits do refer to BXT.

(Also, still confused why you don't do a revid check here, but if you do what I
request below, we can ignore that).

>  	/* 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..5e8e5f9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -985,6 +985,17 @@ 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(GEN7_FF_SLICE_CHICKEN1,
> +				  GEN9_PER_CTX_DISABLE_GATHER_CONTROL);
> +		WA_SET_BIT_MASKED(GEN7_RS_CHICKEN,
> +				  GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
> +	}
> +

This is a very poorly documented workaround...  Reading this again, I believe
this doesn't follow the text from the documentation and this hunk does the wrong
thing. The text clearly states we must clear bit 12 of CS_COMMON2 to workaround
the issue (before each context restore, but it shouldn't hurt to do it
initially).

"If this field is set, Disable at Set Shader Common Slice in register 0x20DC bit
2 must also be set."

To clear this up even further, I think the reason these two registers are talked
about is that the CS, or RS need to match the behavior for the common slice.
Since the default is to always use the BTP for the streamers, and we're
explicitly forcing the CS_COMMON to do the gather at BTP parsing, this has no
meaning.

As we're explicitly clearing the field, I don't think we should be doing
anything with RS_CHICKEN (20dc) or CS_DEBUG_MODE2 (20d8).

I tested on my platform with the hunk removed, and it seems to make no
difference. (Interestingly, I learned that the patch before this is at least as
important as this one). I did not test BXT.

So unless you can justify keeping this hunk, I'd like it gone, and the defines
removed. Sorry for the churn. With that changed, I am happy with no REVID check
too, so:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Tested-by: Ben Widawsky <ben@bwidawsk.net>

>  	return 0;
>  }
>  
> @@ -1068,6 +1079,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);
>  }
>  

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-04  9:01       ` Siluvery, Arun
@ 2015-08-05  9:17         ` Daniel Vetter
  2015-08-05 13:36           ` Joonas Lahtinen
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2015-08-05  9:17 UTC (permalink / raw)
  To: Siluvery, Arun; +Cc: intel-gfx, Ben Widawsky

On Tue, Aug 04, 2015 at 10:01:43AM +0100, Siluvery, Arun wrote:
> On 04/08/2015 09:58, Mika Kuoppala wrote:
> >Ben Widawsky <benjamin.widawsky@intel.com> writes:
> >
> >>On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
> >>>Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> >>>Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>>Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> >>>---
> >>>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
> >>>  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
> >>>  2 files changed, 9 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>>index 77967ca..8991cd5 100644
> >>>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>@@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
> >>>  #define GEN7_MISCCPCTL			(0x9424)
> >>>  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
> >>>
> >>>+#define GEN8_GARBCNTL                   0xB004
> >>>+#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
> >>>+
> >>>  /* IVYBRIDGE DPF */
> >>>  #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
> >>>  #define HSW_L3CDERRST11			0xB208 /* L3CD Error Status register 1 slice 1 */
> >>>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >>>index c23cab6..9152113 100644
> >>>--- a/drivers/gpu/drm/i915/intel_pm.c
> >>>+++ b/drivers/gpu/drm/i915/intel_pm.c
> >>>@@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct drm_device *dev)
> >>>  		/* WaDisableLSQCROPERFforOCL:skl */
> >>>  		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
> >>>  			   GEN8_LQSC_RO_PERF_DIS);
> >>>+
> >>>+	/* WaEnableGapsTsvCreditFix:skl */
> >>>+	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= SKL_REVID_C0)) {
> >>>+		I915_WRITE(GEN8_GARBCNTL, (I915_READ(GEN8_GARBCNTL) |
> >>>+					   GEN9_GAPS_TSV_CREDIT_DISABLE));
> >>>+	}
> >>>  }
> >>>
> >>>  static void bxt_init_clock_gating(struct drm_device *dev)
> >>
> >>FWIW, the docs make it sound like BIOS should be doing this. Did you verify we
> >>actually don't have the bit set with more recent BKC?
> >>
> >
> >I have pretty recent BIOS and the bit was not set.
> 
> I checked about this, it should be done in driver.
> 
> regards
> Arun
> 
> >
> >>Tested-by: Ben Widawsky <ben@bwidawsk.net>
> >>Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> >>
> >
> >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90854
> >Tested-by: Mika Kuoppala <mika.kuoppala@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> >
> >>--
> >>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 mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
  2015-08-05  9:17         ` Daniel Vetter
@ 2015-08-05 13:36           ` Joonas Lahtinen
  0 siblings, 0 replies; 14+ messages in thread
From: Joonas Lahtinen @ 2015-08-05 13:36 UTC (permalink / raw)
  To: Daniel Vetter, Siluvery, Arun; +Cc: intel-gfx, Ben Widawsky

Hi,

Just applying this patch is enough to make gem_ringfill --r render work
on my SKL 2+2 DT GT2 system I have at Espoo. I do not need the second
patch which seems to be required for Arun's SKL.

Also working with just that patch on one SKL Y system here at Egham
office. It's some months older than Arun's SKL U.

Regards, Joonas

On ke, 2015-08-05 at 11:17 +0200, Daniel Vetter wrote:
> On Tue, Aug 04, 2015 at 10:01:43AM +0100, Siluvery, Arun wrote:
> > On 04/08/2015 09:58, Mika Kuoppala wrote:
> > > Ben Widawsky <benjamin.widawsky@intel.com> writes:
> > > 
> > > > On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
> > > > > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > > > >  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
> > > > >  2 files changed, 9 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index 77967ca..8991cd5 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
> > > > >  #define GEN7_MISCCPCTL			(0x9424)
> > > > >  #define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
> > > > > 
> > > > > +#define GEN8_GARBCNTL                   0xB004
> > > > > +#define   GEN9_GAPS_TSV_CREDIT_DISABLE  (1<<7)
> > > > > +
> > > > >  /* IVYBRIDGE DPF */
> > > > >  #define GEN7_L3CDERRST1			0xB008 /* 
> > > > > L3CD Error Status 1 */
> > > > >  #define HSW_L3CDERRST11			0xB208 /* 
> > > > > L3CD Error Status register 1 slice 1 */
> > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > > > > b/drivers/gpu/drm/i915/intel_pm.c
> > > > > index c23cab6..9152113 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > > @@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct 
> > > > > drm_device *dev)
> > > > >  		/* WaDisableLSQCROPERFforOCL:skl */
> > > > >  		I915_WRITE(GEN8_L3SQCREG4, 
> > > > > I915_READ(GEN8_L3SQCREG4) |
> > > > >  			   GEN8_LQSC_RO_PERF_DIS);
> > > > > +
> > > > > +	/* WaEnableGapsTsvCreditFix:skl */
> > > > > +	if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >= 
> > > > > SKL_REVID_C0)) {
> > > > > +		I915_WRITE(GEN8_GARBCNTL, 
> > > > > (I915_READ(GEN8_GARBCNTL) |
> > > > > +					  
> > > > >  GEN9_GAPS_TSV_CREDIT_DISABLE));
> > > > > +	}
> > > > >  }
> > > > > 
> > > > >  static void bxt_init_clock_gating(struct drm_device *dev)
> > > > 
> > > > FWIW, the docs make it sound like BIOS should be doing this. 
> > > > Did you verify we
> > > > actually don't have the bit set with more recent BKC?
> > > > 
> > > 
> > > I have pretty recent BIOS and the bit was not set.
> > 
> > I checked about this, it should be done in driver.
> > 
> > regards
> > Arun
> > 
> > > 
> > > > Tested-by: Ben Widawsky <ben@bwidawsk.net>
> > > > Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> > > > 
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90854
> > > Tested-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Queued for -next, thanks for the patch.
> -Daniel
> 
> > > 
> > > > --
> > > > 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 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-04 10:21   ` [PATCH v2 " Arun Siluvery
  2015-08-04 21:06     ` Ben Widawsky
@ 2015-08-05 14:45     ` Mika Kuoppala
  2015-08-05 15:18       ` Siluvery, Arun
  1 sibling, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2015-08-05 14:45 UTC (permalink / raw)
  To: Arun Siluvery, intel-gfx; +Cc: Ben Widawsky

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

> 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.
>
> v2: behaviour of disable gather at set shader bit can be specified by
> two different registers, use a better option (Ben).
>

For me it looks like there are 2 orthogonal goals for this patch.

I think the actual workaround should be one patch, the resetting
of the set shader bit and the patch named accordingly.

Then the set shader initialization in a different patch,
if there is justification for it (that I have not managed yet
to find).

But lets concentrate on the workaround itself...

> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
>  drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++++++++++++
>  3 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8991cd5..8719a5a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1721,6 +1721,10 @@ enum skl_disp_power_wells {
>  #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
>  #define FW_BLC		0x020d8
>  #define FW_BLC2		0x020dc
> +#define GEN7_RS_CHICKEN  0x20DC
> +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
> +#define GEN7_FF_SLICE_CHICKEN1    0x20E0
> +#define   GEN9_PER_CTX_DISABLE_GATHER_CONTROL  (1<<15)
>  #define FW_BLC_SELF	0x020e0 /* 915+ only */
>  #define   FW_BLC_SELF_EN_MASK      (1<<31)
>  #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
> @@ -5836,6 +5840,7 @@ enum skl_disp_power_wells {
>  # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
>  # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
>  #define COMMON_SLICE_CHICKEN2			0x7014
> +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
>  # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>  
>  #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 */

I am thinking how we could also alert the reader that this workaround
needs to be revisited when it has been given a name.
By adding WaNoName:skl,bxt along with the comment above?

> +	if (IS_SKYLAKE(dev)) {

As Ben noted, documentation is rather sparse. But the reference
to previous problems with this bit being save/restored in wrong order,
we can conclude that this should be for BXT also.

> +		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));
> +	}
> +

The actual value of 'disable set shader' is orthogonal and beyond scope
of this Workaround so the rest should be strip out to different patch.

-Mika

>  	/* 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..5e8e5f9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -985,6 +985,17 @@ 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(GEN7_FF_SLICE_CHICKEN1,
> +				  GEN9_PER_CTX_DISABLE_GATHER_CONTROL);
> +		WA_SET_BIT_MASKED(GEN7_RS_CHICKEN,
> +				  GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1068,6 +1079,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915:gen9: Add disable gather at set shader w/a
  2015-08-05 14:45     ` Mika Kuoppala
@ 2015-08-05 15:18       ` Siluvery, Arun
  0 siblings, 0 replies; 14+ messages in thread
From: Siluvery, Arun @ 2015-08-05 15:18 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: Ben Widawsky

On 05/08/2015 15:45, Mika Kuoppala wrote:
> Arun Siluvery <arun.siluvery@linux.intel.com> writes:
>
>> 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.
>>
>> v2: behaviour of disable gather at set shader bit can be specified by
>> two different registers, use a better option (Ben).
>>
>
> For me it looks like there are 2 orthogonal goals for this patch.
>
> I think the actual workaround should be one patch, the resetting
> of the set shader bit and the patch named accordingly.
>
> Then the set shader initialization in a different patch,
> if there is justification for it (that I have not managed yet
> to find).

I agree it needs to be split into two patches.

>
> But lets concentrate on the workaround itself...
>
>> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h         |  5 +++++
>>   drivers/gpu/drm/i915/intel_lrc.c        |  8 ++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++++++++++++++++++
>>   3 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 8991cd5..8719a5a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1721,6 +1721,10 @@ enum skl_disp_power_wells {
>>   #define   MEM_DISPLAY_TRICKLE_FEED_DISABLE (1<<2) /* 85x only */
>>   #define FW_BLC		0x020d8
>>   #define FW_BLC2		0x020dc
>> +#define GEN7_RS_CHICKEN  0x20DC
>> +#define   GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
>> +#define GEN7_FF_SLICE_CHICKEN1    0x20E0
>> +#define   GEN9_PER_CTX_DISABLE_GATHER_CONTROL  (1<<15)
>>   #define FW_BLC_SELF	0x020e0 /* 915+ only */
>>   #define   FW_BLC_SELF_EN_MASK      (1<<31)
>>   #define   FW_BLC_SELF_FIFO_MASK    (1<<16) /* 945 only */
>> @@ -5836,6 +5840,7 @@ enum skl_disp_power_wells {
>>   # define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC	((1<<10) | (1<<26))
>>   # define GEN9_RHWO_OPTIMIZATION_DISABLE		(1<<14)
>>   #define COMMON_SLICE_CHICKEN2			0x7014
>> +#define  GEN9_DISABLE_GATHER_SET_SHADER_SLICE   (1<<12)
>>   # define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE	(1<<0)
>>
>>   #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 */
>
> I am thinking how we could also alert the reader that this workaround
> needs to be revisited when it has been given a name.
> By adding WaNoName:skl,bxt along with the comment above?
>
>> +	if (IS_SKYLAKE(dev)) {
>
> As Ben noted, documentation is rather sparse. But the reference
> to previous problems with this bit being save/restored in wrong order,
> we can conclude that this should be for BXT also.
>
>> +		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));
>> +	}
>> +
>
> The actual value of 'disable set shader' is orthogonal and beyond scope
> of this Workaround so the rest should be strip out to different patch.
>
As you mentioned on irc we first need to know whether we need to disable 
the set shader or not (set ox7014[12:12] to 1), because the WA only 
talks about resetting this bit in per ctx batch.
The following bits can be ignore if there is not need to set that bit in 
the first place.

with reference to gem_ringfill, on my system it only completes without 
any hang if I add this patch completely but on some system this patch 
doesn't seem to be necessary.

regards
Arun

> -Mika
>
>>   	/* 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..5e8e5f9 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -985,6 +985,17 @@ 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(GEN7_FF_SLICE_CHICKEN1,
>> +				  GEN9_PER_CTX_DISABLE_GATHER_CONTROL);
>> +		WA_SET_BIT_MASKED(GEN7_RS_CHICKEN,
>> +				  GEN9_RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
>> +	}
>> +
>>   	return 0;
>>   }
>>
>> @@ -1068,6 +1079,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
>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-11-18 12:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-03 19:24 [PATCH v1 0/2] Add SKL Workarounds Arun Siluvery
2015-08-03 19:24 ` [PATCH v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix Arun Siluvery
2015-08-03 23:01   ` Ben Widawsky
2015-08-04  8:58     ` Mika Kuoppala
2015-08-04  9:01       ` Siluvery, Arun
2015-08-05  9:17         ` Daniel Vetter
2015-08-05 13:36           ` Joonas Lahtinen
2015-08-03 19:24 ` [PATCH v1 2/2] drm/i915:gen9: Add disable gather at set shader w/a Arun Siluvery
2015-08-03 23:21   ` Ben Widawsky
2015-08-04  9:29     ` Siluvery, Arun
2015-08-04 10:21   ` [PATCH v2 " Arun Siluvery
2015-08-04 21:06     ` Ben Widawsky
2015-08-05 14:45     ` Mika Kuoppala
2015-08-05 15:18       ` Siluvery, Arun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.