[PATCH 0/3] drm/i915: Further Skylake h/w w/a's

[PATCH 0/3] drm/i915: Further Skylake h/w w/a's

[Nick Hoath]

Nick Hoath (3):
  drm/i915: gen 9 h/w w/a (Wa32bitGeneralStateOffset &
    Wa32bitInstructionBaseOffset)
  drm/i915: gen 9 h/w w/a (WaDisablePooledEuLoadBalancingFix)    
    Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
  gen 9 h/w w/a (WaClearFlowControlGpgpuContextSave)     Signed-off-by:
    Nick Hoath <nicholas.hoath@intel.com>

 drivers/gpu/drm/i915/i915_dma.c         |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c         |  4 +++-
 drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c         | 10 ++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  2 ++
 5 files changed, 22 insertions(+), 2 deletions(-)

-- 
2.1.1

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

[PATCH 1/3] drm/i915: gen 9 h/w w/a (Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset)

[Nick Hoath]

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 4 +++-
 drivers/gpu/drm/i915/i915_gem.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5804aa5..df9a3a5 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -748,7 +748,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	 * behaviour if any general state is accessed within a page above 4GB,
 	 * which also needs to be handled carefully.
 	 */
-	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
+	/* Wa32bitGeneralStateOffset:skl */
+	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev) ||
+		IS_SKYLAKE(dev))
 		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
 
 	aperture_size = dev_priv->gtt.mappable_end;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dc10d86..00439b5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4430,7 +4430,9 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
 	}
 
 	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
-	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
+	/* Wa32bitInstructionBaseOffset:skl */
+	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev) ||
+			IS_SKYLAKE(dev)) {
 		/* 965gm cannot relocate objects above 4GiB. */
 		mask &= ~__GFP_HIGHMEM;
 		mask |= __GFP_DMA32;
-- 
2.1.1

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

[PATCH 2/3] drm/i915: gen 9 h/w w/a (WaDisablePooledEuLoadBalancingFix)

[Nick Hoath]

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 39bdbf9..7f9150b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1462,6 +1462,9 @@ enum skl_disp_power_wells {
 #define   MI_AGPBUSY_INT_EN			(1 << 1) /* 85x only */
 #define   MI_AGPBUSY_830_MODE			(1 << 0) /* 85x only */
 
+#define GEN9_FF_SLICE_CS_CHICKEN2				0x020e4
+#define   GEN9_CHICKEN_BIT_POOLED_EU_LOAD_BALANCE_FIX_DISABLE	(1<<10)
+
 #define CACHE_MODE_0	0x02120 /* 915+ only */
 #define   CM0_PIPELINED_RENDER_FLUSH_DISABLE (1<<8)
 #define   CM0_IZ_OPT_DISABLE      (1<<6)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f67e491..4fe71db5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -81,6 +81,15 @@ 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);
+
+	if (INTEL_REVID(dev) == SKL_REVID_A0) {
+		/*
+		 * WaDisablePooledEuLoadBalancingFix:skl
+		 */
+		I915_WRITE(GEN9_FF_SLICE_CS_CHICKEN2,
+			_MASKED_BIT_ENABLE(
+			GEN9_CHICKEN_BIT_POOLED_EU_LOAD_BALANCE_FIX_DISABLE));
+	}
 }
 
 static void i915_pineview_get_mem_freq(struct drm_device *dev)
-- 
2.1.1

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

[PATCH 3/3] gen 9 h/w w/a (WaClearFlowControlGpgpuContextSave)

[Nick Hoath]

Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7f9150b..0326399 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6216,6 +6216,7 @@ enum skl_disp_power_wells {
 #define   GEN9_CCS_TLB_PREFETCH_ENABLE	(1<<3)
 
 #define GEN8_ROW_CHICKEN		0xe4f0
+#define   ENABLE_FLOW_CONTROL_WRITES		(1<<15)
 #define   PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE	(1<<8)
 #define   STALL_DOP_GATING_DISABLE		(1<<5)
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2060610..36df3d4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -950,7 +950,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* WaDisablePartialInstShootdown:skl */
+	/* WaClearFlowControlGpgpuContextSave:skl */
 	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
+			  ENABLE_FLOW_CONTROL_WRITES |
 			  PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
 
 	/* Syncing dependencies between camera and graphics */
-- 
2.1.1

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

Re: [PATCH 1/3] drm/i915: gen 9 h/w w/a (Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset)

[Damien Lespiau]

On Wed, Feb 18, 2015 at 03:16:06PM +0000, Nick Hoath wrote:
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>

That looks reaaaally drastic and without explanations nor W/A
documentation that looks wrong.

Couldn't it be the virtual addresses that need to be on 32 bits? within
a 64bits PPGTT address space? Also this W/A is listed for BDW/CHV. Right
now, we have no way of telling what kind of buffer we're being asked to
relocate into the address space, so there's no way to selectively ensure
some of those buffers end up with a virtual address that remains in the
lower 4GB.

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_dma.c | 4 +++-
>  drivers/gpu/drm/i915/i915_gem.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5804aa5..df9a3a5 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -748,7 +748,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	 * behaviour if any general state is accessed within a page above 4GB,
>  	 * which also needs to be handled carefully.
>  	 */
> -	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
> +	/* Wa32bitGeneralStateOffset:skl */
> +	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev) ||
> +		IS_SKYLAKE(dev))
>  		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
>  
>  	aperture_size = dev_priv->gtt.mappable_end;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index dc10d86..00439b5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4430,7 +4430,9 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
>  	}
>  
>  	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> -	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> +	/* Wa32bitInstructionBaseOffset:skl */
> +	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev) ||
> +			IS_SKYLAKE(dev)) {
>  		/* 965gm cannot relocate objects above 4GiB. */
>  		mask &= ~__GFP_HIGHMEM;
>  		mask |= __GFP_DMA32;
> -- 
> 2.1.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

Re: [PATCH 3/3] gen 9 h/w w/a (WaClearFlowControlGpgpuContextSave)

[Damien Lespiau]

On Wed, Feb 18, 2015 at 03:16:08PM +0000, Nick Hoath wrote:
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>

Can you please prefix subjects by drm/i915/skl or drm/i915/gen9? that
helps people doing selective backports. In any case, we at least use
drm/i915 as prefixes, not empty like here :).

While you're enabling the write to SP here, are the second part of the
W/A (setting it to 0) done in the shader? As always the
documentation/bug report are very thin...

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7f9150b..0326399 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6216,6 +6216,7 @@ enum skl_disp_power_wells {
>  #define   GEN9_CCS_TLB_PREFETCH_ENABLE	(1<<3)
>  
>  #define GEN8_ROW_CHICKEN		0xe4f0
> +#define   ENABLE_FLOW_CONTROL_WRITES		(1<<15)
>  #define   PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE	(1<<8)
>  #define   STALL_DOP_GATING_DISABLE		(1<<5)
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 2060610..36df3d4 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -950,7 +950,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	/* WaDisablePartialInstShootdown:skl */
> +	/* WaClearFlowControlGpgpuContextSave:skl */
>  	WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
> +			  ENABLE_FLOW_CONTROL_WRITES |
>  			  PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
>  
>  	/* Syncing dependencies between camera and graphics */
> -- 
> 2.1.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

Re: [PATCH 2/3] drm/i915: gen 9 h/w w/a (WaDisablePooledEuLoadBalancingFix)

[Damien Lespiau]

On Wed, Feb 18, 2015 at 03:16:07PM +0000, Nick Hoath wrote:
> Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> ---

This one isn't listed for SKL.

-- 
Damien

>  drivers/gpu/drm/i915/i915_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 39bdbf9..7f9150b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1462,6 +1462,9 @@ enum skl_disp_power_wells {
>  #define   MI_AGPBUSY_INT_EN			(1 << 1) /* 85x only */
>  #define   MI_AGPBUSY_830_MODE			(1 << 0) /* 85x only */
>  
> +#define GEN9_FF_SLICE_CS_CHICKEN2				0x020e4
> +#define   GEN9_CHICKEN_BIT_POOLED_EU_LOAD_BALANCE_FIX_DISABLE	(1<<10)
> +
>  #define CACHE_MODE_0	0x02120 /* 915+ only */
>  #define   CM0_PIPELINED_RENDER_FLUSH_DISABLE (1<<8)
>  #define   CM0_IZ_OPT_DISABLE      (1<<6)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f67e491..4fe71db5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -81,6 +81,15 @@ 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);
> +
> +	if (INTEL_REVID(dev) == SKL_REVID_A0) {
> +		/*
> +		 * WaDisablePooledEuLoadBalancingFix:skl
> +		 */
> +		I915_WRITE(GEN9_FF_SLICE_CS_CHICKEN2,
> +			_MASKED_BIT_ENABLE(
> +			GEN9_CHICKEN_BIT_POOLED_EU_LOAD_BALANCE_FIX_DISABLE));
> +	}
>  }
>  
>  static void i915_pineview_get_mem_freq(struct drm_device *dev)
> -- 
> 2.1.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

Re: [PATCH 3/3] gen 9 h/w w/a (WaClearFlowControlGpgpuContextSave)

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5791
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              277/277              276/277
ILK                                  313/313              313/313
SNB                                  309/309              309/309
IVB                 -1              382/382              381/382
BYT                                  296/296              296/296
HSW                                  425/425              425/425
BDW                 -1              318/318              317/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt_gem_userptr_blits_coherency-sync      NO_RESULT(1)CRASH(3)PASS(3)      CRASH(2)
*IVB  igt_gem_storedw_batches_loop_normal      PASS(4)      DMESG_WARN(1)PASS(1)
*BDW  igt_gem_gtt_hog      PASS(9)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: gen 9 h/w w/a (Wa32bitGeneralStateOffset & Wa32bitInstructionBaseOffset)

[Daniel Vetter]

On Wed, Feb 18, 2015 at 06:19:08PM +0000, Damien Lespiau wrote:
> On Wed, Feb 18, 2015 at 03:16:06PM +0000, Nick Hoath wrote:
> > Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> 
> That looks reaaaally drastic and without explanations nor W/A
> documentation that looks wrong.
> 
> Couldn't it be the virtual addresses that need to be on 32 bits? within
> a 64bits PPGTT address space? Also this W/A is listed for BDW/CHV. Right
> now, we have no way of telling what kind of buffer we're being asked to
> relocate into the address space, so there's no way to selectively ensure
> some of those buffers end up with a virtual address that remains in the
> lower 4GB.

I915_GEM_DOMAIN_INSTRUCTION is commonly used for all indirect state by
mesa, so we could restrict the wa to objects with that reloc. Which means
that all the render/texture crap can still be relocated anywhere, which is
the majority.

Of course that means a full audit of mesa/ddx/libva to make sure we don't
break anything. And if there would be breakage we need to make 48bit
address spaces opt-in.

But yeah restricting all objects and then also applying that restriction
to physical objects is _really_ drastic. So please double-check that this
is about physical addresses and not virtual addresses.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] gen 9 h/w w/a (WaClearFlowControlGpgpuContextSave)

[Daniel Vetter]

On Wed, Feb 18, 2015 at 06:48:11PM +0000, Damien Lespiau wrote:
> On Wed, Feb 18, 2015 at 03:16:08PM +0000, Nick Hoath wrote:
> > Signed-off-by: Nick Hoath <nicholas.hoath@intel.com>
> 
> Can you please prefix subjects by drm/i915/skl or drm/i915/gen9? that
> helps people doing selective backports. In any case, we at least use
> drm/i915 as prefixes, not empty like here :).
> 
> While you're enabling the write to SP here, are the second part of the
> W/A (setting it to 0) done in the shader? As always the
> documentation/bug report are very thin...

If there's a part to be done in shaders we need to audit rendercpy, mesa,
libva, ddx and everything else too ... Please cc relevant
lists/maintainers for the next iteration.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx