[PATCH 0/7] drm/i915: Some more w/a'ish stuff

[PATCH 0/7] drm/i915: Some more w/a'ish stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

So I accidentally looked at gen6_init_clock_gating() and noticed a few
weird things that should have gotten cleaned up years ago. So I did that.

While doing that I also noticed the WIZ hashing bits, and the fact that
we weren't following the BSpec recommendation. After doing a few tests
on IVB and HSW it looks 16x4 is in fact the best option, 8x4 was second
best and 8x8 (our current default for !SNB) was the worst. I didn't see
much (if any) difference w/o MSAA, but w/ MSAA 4x there was some change.
I used xonotic as my benchmark.

Ville Syrjälä (7):
  drm/i915: Fix SNB GT_MODE register setup
  drm/i915: Assume we implement
    WaStripsFansDisableFastClipPerformanceFix:snb
  drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB
  drm/i915: Disable SF pipelined attribute fetch for SNB
  drm/i915: Change IVB WIZ hashing mode to 16x4
  drm/i915: Change HSW WIZ hashing mode to 16x4
  drm/i915: Change BDW WIZ hashing mode to 16x4

 drivers/gpu/drm/i915/i915_reg.h | 10 +++++++--
 drivers/gpu/drm/i915/intel_pm.c | 47 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 10 deletions(-)

-- 
1.8.3.2

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

[PATCH 1/7] drm/i915: Fix SNB GT_MODE register setup

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On SNB we set up WaSetupGtModeTdRowDispatch:snb early in
gen6_init_clock_gating(). That sets a bit in the GEN6_GT_MODE register.
However later we go and disable all the bits in the same register. And
then we go on to set some other bit. So apparently we never actually
implemented this workaround since the "disable all bits" part was there
already before the w/a got supposedly implemented.

These are the relevant commits:

 commit 6547fbdbfff62c99e4f7b4f985ff8b3454f33b0f
 Author: Daniel Vetter <daniel.vetter@ffwll.ch>
 Date:   Fri Dec 14 23:38:29 2012 +0100

    drm/i915: Implement WaSetupGtModeTdRowDispatch

 commit f8f2ac9a76b0f80a6763ca316116a7bab8486997
 Author: Ben Widawsky <ben@bwidawsk.net>
 Date:   Wed Oct 3 19:34:24 2012 -0700

    drm/i915: Fix GT_MODE default value

So, let's drop the "disable all bits" part, move both writes to
closer proxomity to each other, and name the WIZ hashing bits
appropriately. BSpec is still a bit confused how the bits should
actually be interpreted, but I took the the description for the
high bit since the low bit part only lists values for a single bit.

Also add a comment about our choice of WIZ hashing mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  6 +++++-
 drivers/gpu/drm/i915/intel_pm.c | 12 +++++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9d0f4f7..7aa2cf5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -798,7 +798,11 @@
 # define ASYNC_FLIP_PERF_DISABLE			(1 << 14)
 
 #define GEN6_GT_MODE	0x20d0
-#define   GEN6_GT_MODE_HI				(1 << 9)
+#define   GEN6_WIZ_HASHING(hi, lo)			(((hi) << 9) | ((lo) << 7))
+#define   GEN6_WIZ_HASHING_8x8				GEN6_WIZ_HASHING(0, 0)
+#define   GEN6_WIZ_HASHING_8x4				GEN6_WIZ_HASHING(0, 1)
+#define   GEN6_WIZ_HASHING_16x4				GEN6_WIZ_HASHING(1, 0)
+#define   GEN6_WIZ_HASHING_MASK				(GEN6_WIZ_HASHING(1, 1) << 16)
 #define   GEN6_TD_FOUR_ROW_DISPATCH_DISABLE		(1 << 5)
 
 #define GFX_MODE	0x02520
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9ab3883..056999c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4644,6 +4644,13 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		I915_WRITE(GEN6_GT_MODE,
 			   _MASKED_BIT_ENABLE(GEN6_TD_FOUR_ROW_DISPATCH_DISABLE));
 
+	/*
+	 * BSpec recoomends 8x4 when MSAA is used,
+	 * however in practice 16x4 seems fastest.
+	 */
+	I915_WRITE(GEN6_GT_MODE,
+		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+
 	ilk_init_lp_watermarks(dev);
 
 	I915_WRITE(CACHE_MODE_0,
@@ -4699,11 +4706,6 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 
 	g4x_disable_trickle_feed(dev);
 
-	/* The default value should be 0x200 according to docs, but the two
-	 * platforms I checked have a 0 for this. (Maybe BIOS overrides?) */
-	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_DISABLE(0xffff));
-	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_ENABLE(GEN6_GT_MODE_HI));
-
 	cpt_init_clock_gating(dev);
 
 	gen6_check_mch_setup(dev);
-- 
1.8.3.2

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

[PATCH 2/7] drm/i915: Assume we implement WaStripsFansDisableFastClipPerformanceFix:snb

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Based on the name, the workaround we implement is
WaStripsFansDisableFastClipPerformanceFix. Unfortunately there's no
description in the w/a database, so this is just a guess.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 056999c..1baf56e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4678,6 +4678,7 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		   GEN6_RCPBUNIT_CLOCK_GATE_DISABLE |
 		   GEN6_RCCUNIT_CLOCK_GATE_DISABLE);
 
+	/* WaStripsFansDisableFastClipPerformanceFix:snb */
 	/* Bspec says we need to always set all mask bits. */
 	I915_WRITE(_3D_CHICKEN3, (0xFFFF << 16) |
 		   _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL);
-- 
1.8.3.2

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

[PATCH 3/7] drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The need to set all of the mask bits for 3D_CHICKEN3 was required
only for pre-production hardware.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1baf56e..6a09281 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4679,9 +4679,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		   GEN6_RCCUNIT_CLOCK_GATE_DISABLE);
 
 	/* WaStripsFansDisableFastClipPerformanceFix:snb */
-	/* Bspec says we need to always set all mask bits. */
-	I915_WRITE(_3D_CHICKEN3, (0xFFFF << 16) |
-		   _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL);
+	I915_WRITE(_3D_CHICKEN3,
+		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL));
 
 	/*
 	 * According to the spec the following bits should be
-- 
1.8.3.2

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

[PATCH 4/7] drm/i915: Disable SF pipelined attribute fetch for SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

According to Bspec we need to disable SF pipelined attribute fetch
whenever SF outputs exceed 16 and normal clip mode is used. A quick
glance at Mesa suggests that these conditions could happen. So let's
just always set the magic bit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 ++-
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7aa2cf5..0334507 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -790,7 +790,8 @@
 #define _3D_CHICKEN3	0x02090
 #define  _3D_CHICKEN_SF_DISABLE_OBJEND_CULL		(1 << 10)
 #define  _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL		(1 << 5)
-#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1)
+#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1) /* gen8+ */
+#define  _3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH	(1 << 1) /* gen6 */
 
 #define MI_MODE		0x0209c
 # define VS_TIMER_DISPATCH				(1 << 6)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6a09281..7247084 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4683,6 +4683,14 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL));
 
 	/*
+	 * Bspec says:
+	 * "This bit must be set if 3DSTATE_CLIP clip mode is set to normal and
+	 * 3DSTATE_SF number of SF output attributes is more than 16."
+	 */
+	I915_WRITE(_3D_CHICKEN3,
+		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH));
+
+	/*
 	 * According to the spec the following bits should be
 	 * set in order to enable memory self-refresh and fbc:
 	 * The bit21 and bit22 of 0x42000
-- 
1.8.3.2

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

[PATCH 5/7] drm/i915: Change IVB WIZ hashing mode to 16x4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0334507..647c4bb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -799,6 +799,7 @@
 # define ASYNC_FLIP_PERF_DISABLE			(1 << 14)
 
 #define GEN6_GT_MODE	0x20d0
+#define GEN7_GT_MODE	0x7008
 #define   GEN6_WIZ_HASHING(hi, lo)			(((hi) << 9) | ((lo) << 7))
 #define   GEN6_WIZ_HASHING_8x8				GEN6_WIZ_HASHING(0, 0)
 #define   GEN6_WIZ_HASHING_8x4				GEN6_WIZ_HASHING(0, 1)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7247084..c51b64a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4937,6 +4937,13 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(CACHE_MODE_1,
 		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
 
+	/*
+	 * BSpec recommends 8x4 when MSAA is used,
+	 * however in practice 16x4 seems fastest.
+	 */
+	I915_WRITE(GEN7_GT_MODE,
+		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+
 	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
 	snpcr &= ~GEN6_MBC_SNPCR_MASK;
 	snpcr |= GEN6_MBC_SNPCR_MED;
-- 
1.8.3.2

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

[PATCH 6/7] drm/i915: Change HSW WIZ hashing mode to 16x4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c51b64a..ba5fee4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4856,6 +4856,13 @@ static void haswell_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(CACHE_MODE_1,
 		   _MASKED_BIT_ENABLE(PIXEL_SUBSPAN_COLLECT_OPT_DISABLE));
 
+	/*
+	 * BSpec recommends 8x4 when MSAA is used,
+	 * however in practice 16x4 seems fastest.
+	 */
+	I915_WRITE(GEN7_GT_MODE,
+		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+
 	/* WaSwitchSolVfFArbitrationPriority:hsw */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
 
-- 
1.8.3.2

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

[PATCH 7/7] drm/i915: Change BDW WIZ hashing mode to 16x4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ba5fee4..c3a1362 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4826,6 +4826,13 @@ static void gen8_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(GEN7_FF_THREAD_MODE,
 		   I915_READ(GEN7_FF_THREAD_MODE) &
 		   ~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME));
+
+	/*
+	 * BSpec recommends 8x4 when MSAA is used,
+	 * however in practice 16x4 seems fastest.
+	 */
+	I915_WRITE(GEN7_GT_MODE,
+		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
 }
 
 static void haswell_init_clock_gating(struct drm_device *dev)
-- 
1.8.3.2

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

Re: [PATCH 1/7] drm/i915: Fix SNB GT_MODE register setup

[Chris Wilson]

On Tue, Feb 04, 2014 at 09:59:15PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On SNB we set up WaSetupGtModeTdRowDispatch:snb early in
> gen6_init_clock_gating(). That sets a bit in the GEN6_GT_MODE register.
> However later we go and disable all the bits in the same register. And
> then we go on to set some other bit. So apparently we never actually
> implemented this workaround since the "disable all bits" part was there
> already before the w/a got supposedly implemented.
> 
> These are the relevant commits:
> 
>  commit 6547fbdbfff62c99e4f7b4f985ff8b3454f33b0f
>  Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>  Date:   Fri Dec 14 23:38:29 2012 +0100
> 
>     drm/i915: Implement WaSetupGtModeTdRowDispatch
> 
>  commit f8f2ac9a76b0f80a6763ca316116a7bab8486997
>  Author: Ben Widawsky <ben@bwidawsk.net>
>  Date:   Wed Oct 3 19:34:24 2012 -0700
> 
>     drm/i915: Fix GT_MODE default value
> 
> So, let's drop the "disable all bits" part, move both writes to
> closer proxomity to each other, and name the WIZ hashing bits
> appropriately. BSpec is still a bit confused how the bits should
> actually be interpreted, but I took the the description for the
> high bit since the low bit part only lists values for a single bit.
> 
> Also add a comment about our choice of WIZ hashing mode.

Changing WiZ hashing mode changes the valid number of threads and
userspace assumes best case (WiZ disabled). Worst case we start hanging
the chip.

I have no idea how relevant that piece of lore from the spec is, but it
something to be wary of when making these changes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/7] drm/i915: Fix SNB GT_MODE register setup

[Ville Syrjälä]

On Tue, Feb 04, 2014 at 09:23:06PM +0000, Chris Wilson wrote:
> On Tue, Feb 04, 2014 at 09:59:15PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On SNB we set up WaSetupGtModeTdRowDispatch:snb early in
> > gen6_init_clock_gating(). That sets a bit in the GEN6_GT_MODE register.
> > However later we go and disable all the bits in the same register. And
> > then we go on to set some other bit. So apparently we never actually
> > implemented this workaround since the "disable all bits" part was there
> > already before the w/a got supposedly implemented.
> > 
> > These are the relevant commits:
> > 
> >  commit 6547fbdbfff62c99e4f7b4f985ff8b3454f33b0f
> >  Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >  Date:   Fri Dec 14 23:38:29 2012 +0100
> > 
> >     drm/i915: Implement WaSetupGtModeTdRowDispatch
> > 
> >  commit f8f2ac9a76b0f80a6763ca316116a7bab8486997
> >  Author: Ben Widawsky <ben@bwidawsk.net>
> >  Date:   Wed Oct 3 19:34:24 2012 -0700
> > 
> >     drm/i915: Fix GT_MODE default value
> > 
> > So, let's drop the "disable all bits" part, move both writes to
> > closer proxomity to each other, and name the WIZ hashing bits
> > appropriately. BSpec is still a bit confused how the bits should
> > actually be interpreted, but I took the the description for the
> > high bit since the low bit part only lists values for a single bit.
> > 
> > Also add a comment about our choice of WIZ hashing mode.
> 
> Changing WiZ hashing mode changes the valid number of threads and
> userspace assumes best case (WiZ disabled). Worst case we start hanging
> the chip.
> 
> I have no idea how relevant that piece of lore from the spec is, but it
> something to be wary of when making these changes.

The thread numbers seem to depend only on the WIZ hashing disable bit.
I'm not touching that one.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/7] drm/i915: Fix SNB GT_MODE register setup

[Chris Wilson]

On Wed, Feb 05, 2014 at 11:27:31AM +0200, Ville Syrjälä wrote:
> On Tue, Feb 04, 2014 at 09:23:06PM +0000, Chris Wilson wrote:
> > On Tue, Feb 04, 2014 at 09:59:15PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > On SNB we set up WaSetupGtModeTdRowDispatch:snb early in
> > > gen6_init_clock_gating(). That sets a bit in the GEN6_GT_MODE register.
> > > However later we go and disable all the bits in the same register. And
> > > then we go on to set some other bit. So apparently we never actually
> > > implemented this workaround since the "disable all bits" part was there
> > > already before the w/a got supposedly implemented.
> > > 
> > > These are the relevant commits:
> > > 
> > >  commit 6547fbdbfff62c99e4f7b4f985ff8b3454f33b0f
> > >  Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >  Date:   Fri Dec 14 23:38:29 2012 +0100
> > > 
> > >     drm/i915: Implement WaSetupGtModeTdRowDispatch
> > > 
> > >  commit f8f2ac9a76b0f80a6763ca316116a7bab8486997
> > >  Author: Ben Widawsky <ben@bwidawsk.net>
> > >  Date:   Wed Oct 3 19:34:24 2012 -0700
> > > 
> > >     drm/i915: Fix GT_MODE default value
> > > 
> > > So, let's drop the "disable all bits" part, move both writes to
> > > closer proxomity to each other, and name the WIZ hashing bits
> > > appropriately. BSpec is still a bit confused how the bits should
> > > actually be interpreted, but I took the the description for the
> > > high bit since the low bit part only lists values for a single bit.
> > > 
> > > Also add a comment about our choice of WIZ hashing mode.
> > 
> > Changing WiZ hashing mode changes the valid number of threads and
> > userspace assumes best case (WiZ disabled). Worst case we start hanging
> > the chip.
> > 
> > I have no idea how relevant that piece of lore from the spec is, but it
> > something to be wary of when making these changes.
> 
> The thread numbers seem to depend only on the WIZ hashing disable bit.
> I'm not touching that one.

That's ok then. Can I ask that we have a comment next to the WIZ setup
noting userspace's dependence on the WIZ-disable bit?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: Add a comment about WIZ hashing vs. thread counts

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a comment next to our WIZ hashing setup to remind people about the
link between WIZ hashing disable bit and PS/WM thread counts.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c3a1362..a3b61ca 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4647,6 +4647,10 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	/*
 	 * BSpec recoomends 8x4 when MSAA is used,
 	 * however in practice 16x4 seems fastest.
+	 *
+	 * Note that PS/WM thread counts depend on the WIZ hashing
+	 * disable bit, which we don't touch here, but it's good
+	 * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
 	 */
 	I915_WRITE(GEN6_GT_MODE,
 		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
@@ -4830,6 +4834,10 @@ static void gen8_init_clock_gating(struct drm_device *dev)
 	/*
 	 * BSpec recommends 8x4 when MSAA is used,
 	 * however in practice 16x4 seems fastest.
+	 *
+	 * Note that PS/WM thread counts depend on the WIZ hashing
+	 * disable bit, which we don't touch here, but it's good
+	 * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
 	 */
 	I915_WRITE(GEN7_GT_MODE,
 		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
@@ -4866,6 +4874,10 @@ static void haswell_init_clock_gating(struct drm_device *dev)
 	/*
 	 * BSpec recommends 8x4 when MSAA is used,
 	 * however in practice 16x4 seems fastest.
+	 *
+	 * Note that PS/WM thread counts depend on the WIZ hashing
+	 * disable bit, which we don't touch here, but it's good
+	 * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
 	 */
 	I915_WRITE(GEN7_GT_MODE,
 		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
@@ -4954,6 +4966,10 @@ static void ivybridge_init_clock_gating(struct drm_device *dev)
 	/*
 	 * BSpec recommends 8x4 when MSAA is used,
 	 * however in practice 16x4 seems fastest.
+	 *
+	 * Note that PS/WM thread counts depend on the WIZ hashing
+	 * disable bit, which we don't touch here, but it's good
+	 * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
 	 */
 	I915_WRITE(GEN7_GT_MODE,
 		   GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
-- 
1.8.3.2

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

Re: [PATCH] drm/i915: Add a comment about WIZ hashing vs. thread counts

[Chris Wilson]

On Wed, Feb 05, 2014 at 12:43:47PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a comment next to our WIZ hashing setup to remind people about the
> link between WIZ hashing disable bit and PS/WM thread counts.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, now hopefully I won't make the same mistake again in future.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 4/7] drm/i915: Disable SF pipelined attribute fetch for SNB

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 2197 bytes --]

On 02/04/2014 11:59 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> According to Bspec we need to disable SF pipelined attribute fetch
> whenever SF outputs exceed 16 and normal clip mode is used. A quick
> glance at Mesa suggests that these conditions could happen. So let's
> just always set the magic bit.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 ++-
>  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7aa2cf5..0334507 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -790,7 +790,8 @@
>  #define _3D_CHICKEN3	0x02090
>  #define  _3D_CHICKEN_SF_DISABLE_OBJEND_CULL		(1 << 10)
>  #define  _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL		(1 << 5)
> -#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1)
> +#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1) /* gen8+ */
> +#define  _3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH	(1 << 1) /* gen6 */
>  
>  #define MI_MODE		0x0209c
>  # define VS_TIMER_DISPATCH				(1 << 6)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6a09281..7247084 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4683,6 +4683,14 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>  		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL));
>  
>  	/*
> +	 * Bspec says:
> +	 * "This bit must be set if 3DSTATE_CLIP clip mode is set to normal and
> +	 * 3DSTATE_SF number of SF output attributes is more than 16."
> +	 */
> +	I915_WRITE(_3D_CHICKEN3,
> +		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH));
> +
> +	/*
>  	 * According to the spec the following bits should be
>  	 * set in order to enable memory self-refresh and fbc:
>  	 * The bit21 and bit22 of 0x42000
> 

I'm almost positive Mesa will hit this case.  Nice catch!

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 4/7] drm/i915: Disable SF pipelined attribute fetch for SNB

[Daniel Vetter]

On Fri, Feb 07, 2014 at 12:14:47PM -0800, Kenneth Graunke wrote:
> On 02/04/2014 11:59 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > According to Bspec we need to disable SF pipelined attribute fetch
> > whenever SF outputs exceed 16 and normal clip mode is used. A quick
> > glance at Mesa suggests that these conditions could happen. So let's
> > just always set the magic bit.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 3 ++-
> >  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 7aa2cf5..0334507 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -790,7 +790,8 @@
> >  #define _3D_CHICKEN3	0x02090
> >  #define  _3D_CHICKEN_SF_DISABLE_OBJEND_CULL		(1 << 10)
> >  #define  _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL		(1 << 5)
> > -#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1)
> > +#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1) /* gen8+ */
> > +#define  _3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH	(1 << 1) /* gen6 */
> >  
> >  #define MI_MODE		0x0209c
> >  # define VS_TIMER_DISPATCH				(1 << 6)
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 6a09281..7247084 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4683,6 +4683,14 @@ static void gen6_init_clock_gating(struct drm_device *dev)
> >  		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL));
> >  
> >  	/*
> > +	 * Bspec says:
> > +	 * "This bit must be set if 3DSTATE_CLIP clip mode is set to normal and
> > +	 * 3DSTATE_SF number of SF output attributes is more than 16."
> > +	 */
> > +	I915_WRITE(_3D_CHICKEN3,
> > +		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH));
> > +
> > +	/*
> >  	 * According to the spec the following bits should be
> >  	 * set in order to enable memory self-refresh and fbc:
> >  	 * The bit21 and bit22 of 0x42000
> > 
> 
> I'm almost positive Mesa will hit this case.  Nice catch!
> 
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Queued for -next, thanks for the patch. Any chance that this would apply
to one of our leftover snb gpu hang issues?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/7] drm/i915: Disable SF pipelined attribute fetch for SNB

[Ville Syrjälä]

On Sat, Feb 08, 2014 at 06:28:03PM +0100, Daniel Vetter wrote:
> On Fri, Feb 07, 2014 at 12:14:47PM -0800, Kenneth Graunke wrote:
> > On 02/04/2014 11:59 AM, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > According to Bspec we need to disable SF pipelined attribute fetch
> > > whenever SF outputs exceed 16 and normal clip mode is used. A quick
> > > glance at Mesa suggests that these conditions could happen. So let's
> > > just always set the magic bit.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h | 3 ++-
> > >  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
> > >  2 files changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 7aa2cf5..0334507 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -790,7 +790,8 @@
> > >  #define _3D_CHICKEN3	0x02090
> > >  #define  _3D_CHICKEN_SF_DISABLE_OBJEND_CULL		(1 << 10)
> > >  #define  _3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL		(1 << 5)
> > > -#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1)
> > > +#define  _3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(x)	((x)<<1) /* gen8+ */
> > > +#define  _3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH	(1 << 1) /* gen6 */
> > >  
> > >  #define MI_MODE		0x0209c
> > >  # define VS_TIMER_DISPATCH				(1 << 6)
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 6a09281..7247084 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4683,6 +4683,14 @@ static void gen6_init_clock_gating(struct drm_device *dev)
> > >  		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_FASTCLIP_CULL));
> > >  
> > >  	/*
> > > +	 * Bspec says:
> > > +	 * "This bit must be set if 3DSTATE_CLIP clip mode is set to normal and
> > > +	 * 3DSTATE_SF number of SF output attributes is more than 16."
> > > +	 */
> > > +	I915_WRITE(_3D_CHICKEN3,
> > > +		   _MASKED_BIT_ENABLE(_3D_CHICKEN3_SF_DISABLE_PIPELINED_ATTR_FETCH));
> > > +
> > > +	/*
> > >  	 * According to the spec the following bits should be
> > >  	 * set in order to enable memory self-refresh and fbc:
> > >  	 * The bit21 and bit22 of 0x42000
> > > 
> > 
> > I'm almost positive Mesa will hit this case.  Nice catch!
> > 
> > Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
> 
> Queued for -next, thanks for the patch. Any chance that this would apply
> to one of our leftover snb gpu hang issues?

Unfortunately BSpec doesn't say what might happen if we don't set the
bit, nor did I find anything relevant in the w/a database.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 0/7] drm/i915: Some more w/a'ish stuff

[Antti Koskipää]

On 02/04/2014 09:59 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> So I accidentally looked at gen6_init_clock_gating() and noticed a few
> weird things that should have gotten cleaned up years ago. So I did that.
> 
> While doing that I also noticed the WIZ hashing bits, and the fact that
> we weren't following the BSpec recommendation. After doing a few tests
> on IVB and HSW it looks 16x4 is in fact the best option, 8x4 was second
> best and 8x8 (our current default for !SNB) was the worst. I didn't see
> much (if any) difference w/o MSAA, but w/ MSAA 4x there was some change.
> I used xonotic as my benchmark.
> 
> Ville Syrjälä (7):
>   drm/i915: Fix SNB GT_MODE register setup
>   drm/i915: Assume we implement
>     WaStripsFansDisableFastClipPerformanceFix:snb
>   drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB
>   drm/i915: Disable SF pipelined attribute fetch for SNB
>   drm/i915: Change IVB WIZ hashing mode to 16x4
>   drm/i915: Change HSW WIZ hashing mode to 16x4
>   drm/i915: Change BDW WIZ hashing mode to 16x4
> 
>  drivers/gpu/drm/i915/i915_reg.h | 10 +++++++--
>  drivers/gpu/drm/i915/intel_pm.c | 47 ++++++++++++++++++++++++++++++++++-------
>  2 files changed, 47 insertions(+), 10 deletions(-)

For everything except 4/7, which was reviewed by Ken:

Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>

-- 
- Antti


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

Re: [PATCH 0/7] drm/i915: Some more w/a'ish stuff

[Daniel Vetter]

On Thu, Feb 27, 2014 at 04:07:32PM +0200, Antti Koskipää wrote:
> On 02/04/2014 09:59 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > So I accidentally looked at gen6_init_clock_gating() and noticed a few
> > weird things that should have gotten cleaned up years ago. So I did that.
> > 
> > While doing that I also noticed the WIZ hashing bits, and the fact that
> > we weren't following the BSpec recommendation. After doing a few tests
> > on IVB and HSW it looks 16x4 is in fact the best option, 8x4 was second
> > best and 8x8 (our current default for !SNB) was the worst. I didn't see
> > much (if any) difference w/o MSAA, but w/ MSAA 4x there was some change.
> > I used xonotic as my benchmark.
> > 
> > Ville Syrjälä (7):
> >   drm/i915: Fix SNB GT_MODE register setup
> >   drm/i915: Assume we implement
> >     WaStripsFansDisableFastClipPerformanceFix:snb
> >   drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB
> >   drm/i915: Disable SF pipelined attribute fetch for SNB
> >   drm/i915: Change IVB WIZ hashing mode to 16x4
> >   drm/i915: Change HSW WIZ hashing mode to 16x4
> >   drm/i915: Change BDW WIZ hashing mode to 16x4
> > 
> >  drivers/gpu/drm/i915/i915_reg.h | 10 +++++++--
> >  drivers/gpu/drm/i915/intel_pm.c | 47 ++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 47 insertions(+), 10 deletions(-)
> 
> For everything except 4/7, which was reviewed by Ken:
> 
> Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>

Pulled all remaining patches from this thread into dinq from. Thanks for
them and the review comments.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch