[PATCH 0/7] A set of SNB workarounds

[PATCH 0/7] A set of SNB workarounds

[Daniel Vetter]

Hi all,

I've stitched together the following set of workarounds from internal sources in
the vain hope that they fix our VT-d troubles. Unfortunately that's not the
case. I also haven't seen any bug where these could apply to.

Anyway I think it's better to implement them, so please review.

Yours, Daniel

PS: With this set we now implement all but 2 workarounds on from this internal
list (when filtering out everything that only applies for pre-production
silicon). These 2 concern gl features I don't have much clue about, I've instead
forwarded the relevant entries to Ken and Paul from our gl team.

Daniel Vetter (7):
  drm/i915: implement ColorBlt w/a
  drm/i915: implement a media hang w/a
  drm/i915: set w/a bit for snb pagefaults
  drm/i915: properly set ppgtt cacheability on snb
  drm/i915: implement w/a for incorrect guarband clipping
  drm/i915: implement async flush w/a
  drm/i915: set stc evict disable lra evict w/a

 drivers/gpu/drm/i915/i915_gem.c      |   10 +++++++++-
 drivers/gpu/drm/i915/i915_reg.h      |   14 ++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++++++
 3 files changed, 38 insertions(+), 1 deletions(-)

-- 
1.7.7.6

[PATCH 1/7] drm/i915: implement ColorBlt w/a

[Daniel Vetter]

According to an internal workaround master list, we need to set bit 5
of register 9400 to avoid issues with color blits.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    3 +++
 drivers/gpu/drm/i915/intel_display.c |    4 ++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3886cf0..233dbd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3727,6 +3727,9 @@
 #define  GT_FIFO_FREE_ENTRIES			0x120008
 #define    GT_FIFO_NUM_RESERVED_ENTRIES		20
 
+#define GEN6_UCGCTL1				0x9400
+# define GEN6_BLBUNIT_CLOCK_GATE_DISABLE		(1 << 5)
+
 #define GEN6_UCGCTL2				0x9404
 # define GEN6_RCZUNIT_CLOCK_GATE_DISABLE		(1 << 13)
 # define GEN6_RCPBUNIT_CLOCK_GATE_DISABLE		(1 << 12)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d514719..aa0c6df 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8510,6 +8510,10 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(WM2_LP_ILK, 0);
 	I915_WRITE(WM1_LP_ILK, 0);
 
+	I915_WRITE(GEN6_UCGCTL1,
+		   I915_READ(GEN6_UCGCTL1) |
+		   GEN6_BLBUNIT_CLOCK_GATE_DISABLE);
+
 	/* According to the BSpec vol1g, bit 12 (RCPBUNIT) clock
 	 * gating disable must be set.  Failure to set it results in
 	 * flickering pixels due to Z write ordering failures after
-- 
1.7.7.6

[PATCH 2/7] drm/i915: implement a media hang w/a

[Daniel Vetter]

Contrary to the other clock gating w/a in GEN6_UCGCTL1, this one is
actually documented in Bspec, vol1g "GT Interface Registers [SNB]",
Section 1.5.1 "UCGCTL1 - Unit Level Clock Gating Control 1".

Supposedly this can prevent hangs on the media ring.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 233dbd5..7e0a2b5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3729,6 +3729,7 @@
 
 #define GEN6_UCGCTL1				0x9400
 # define GEN6_BLBUNIT_CLOCK_GATE_DISABLE		(1 << 5)
+# define GEN6_CSUNIT_CLOCK_GATE_DISABLE			(1 << 7)
 
 #define GEN6_UCGCTL2				0x9404
 # define GEN6_RCZUNIT_CLOCK_GATE_DISABLE		(1 << 13)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index aa0c6df..2134996 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8512,7 +8512,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 
 	I915_WRITE(GEN6_UCGCTL1,
 		   I915_READ(GEN6_UCGCTL1) |
-		   GEN6_BLBUNIT_CLOCK_GATE_DISABLE);
+		   GEN6_BLBUNIT_CLOCK_GATE_DISABLE |
+		   GEN6_CSUNIT_CLOCK_GATE_DISABLE);
 
 	/* According to the BSpec vol1g, bit 12 (RCPBUNIT) clock
 	 * gating disable must be set.  Failure to set it results in
-- 
1.7.7.6

[PATCH 3/7] drm/i915: set w/a bit for snb pagefaults

[Daniel Vetter]

Bspec says that we need to set this: vol1c.3 "Blitter Command
Streamer", Section 1.1.2.1 "GAB_CTL_REG - GAB Unit Control Register".

We don't really rely on pagefaults, but who knows what this all
affects.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    7 ++++++-
 drivers/gpu/drm/i915/i915_reg.h |    3 +++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1f441f5..5dc5f42 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3764,7 +3764,12 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 	pd_offset <<= 16;
 
 	if (INTEL_INFO(dev)->gen == 6) {
-		uint32_t ecochk = I915_READ(GAM_ECOCHK);
+		uint32_t ecochk, gab_ctl, ecobits;
+
+		gab_ctl = I915_READ(GAB_CTL);
+		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
+
+		ecochk = I915_READ(GAM_ECOCHK);
 		I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
 				       ECOCHK_PPGTT_CACHE64B);
 		I915_WRITE(GFX_MODE, GFX_MODE_ENABLE(GFX_PPGTT_ENABLE));
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7e0a2b5..e72c251 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -125,6 +125,9 @@
 #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
 #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
 
+#define GAB_CTL				0x24000
+#define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
+
 /* VGA stuff */
 
 #define VGA_ST01_MDA 0x3ba
-- 
1.7.7.6

[PATCH 4/7] drm/i915: properly set ppgtt cacheability on snb

[Daniel Vetter]

For some reason snb has 2 fields to set ppgtt cacheability. This one
here does not exist on gen7.

This might explain why ppgtt wasn't a win on snb like on ivb - not
enough pte caching.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    3 +++
 drivers/gpu/drm/i915/i915_reg.h |    4 ++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5dc5f42..cab70ef 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3766,6 +3766,9 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen == 6) {
 		uint32_t ecochk, gab_ctl, ecobits;
 
+		ecobits = I915_READ(GAC_ECO_BITS); 
+		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
+
 		gab_ctl = I915_READ(GAB_CTL);
 		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e72c251..5457980 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -125,6 +125,10 @@
 #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
 #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
 
+#define GAC_ECO_BITS			0x14090
+#define   ECOBITS_PPGTT_CACHE64B	(3<<8)
+#define   ECOBITS_PPGTT_CACHE4B		(0<<8)
+
 #define GAB_CTL				0x24000
 #define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
 
-- 
1.7.7.6

[PATCH 5/7] drm/i915: implement w/a for incorrect guarband clipping

[Daniel Vetter]

According to Bsepc, this should be set by default, but isn't. See vo1c.4
"Render Engine Command Streamer", Section 1.1.14.3 "3D_CHICKEN3"

Bspec also says that we always need to set all mask bits.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5457980..a7ef74a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -439,6 +439,7 @@
  */
 # define _3D_CHICKEN2_WM_READ_PIPELINED			(1 << 14)
 #define _3D_CHICKEN3	0x02090
+#define  _3D_CHICKEN_SF_DISABLE_FASTCLIP_CULL		(1 << 5)
 
 #define MI_MODE		0x0209c
 # define VS_TIMER_DISPATCH				(1 << 6)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2134996..dab2381 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8529,6 +8529,9 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 		   GEN6_RCPBUNIT_CLOCK_GATE_DISABLE |
 		   GEN6_RCCUNIT_CLOCK_GATE_DISABLE);
 
+	I915_WRITE(_3D_CHICKEN, (0xFFFF << 16) |
+		   _3D_CHICKEN_SF_DISABLE_FASTCLIP_CULL);
+
 	/*
 	 * According to the spec the following bits should be
 	 * set in order to enable memory self-refresh and fbc:
-- 
1.7.7.6

[PATCH 6/7] drm/i915: implement async flush w/a

[Daniel Vetter]

Note that async flush also means things like VT-d IOTLB invalidation.

See Bspec vol1c.4 "Render Engine Command Streamer", Section "ECOSKPD -
Eco Scratch Pad".

It doesn't seem to help in for any of our VT-d related bugs thoug.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a7ef74a..5b23d5c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -583,6 +583,7 @@
 #define BB_ADDR		0x02140 /* 8 bytes */
 #define GFX_FLSH_CNTL	0x02170 /* 915+ only */
 #define ECOSKPD		0x021d0
+#define   ECO_ASYNC_FLUSH_FIX_REVERT (1<<7)
 #define   ECO_GATING_CX_ONLY	(1<<3)
 #define   ECO_FLIP_DONE		(1<<0)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dab2381..dce64d8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8510,6 +8510,9 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(WM2_LP_ILK, 0);
 	I915_WRITE(WM1_LP_ILK, 0);
 
+	I915_WRITE(ECOSKPD, (ECO_ASYNC_FLUSH_FIX_REVERT << 16) |
+			    ECO_ASYNC_FLUSH_FIX_REVERT);
+
 	I915_WRITE(GEN6_UCGCTL1,
 		   I915_READ(GEN6_UCGCTL1) |
 		   GEN6_BLBUNIT_CLOCK_GATE_DISABLE |
-- 
1.7.7.6

[PATCH 7/7] drm/i915: set stc evict disable lra evict w/a

[Daniel Vetter]

Our workaround list kindly lists that this new default value needs to
be updated in Bspec. Naturally, this did not happen.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5b23d5c..d0d0e2f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -576,6 +576,7 @@
 #define   CM0_MASK_SHIFT          16
 #define   CM0_IZ_OPT_DISABLE      (1<<6)
 #define   CM0_ZR_OPT_DISABLE      (1<<5)
+#define	  CM0_STC_EVICT_DISABLE_LRA_SNB	(1<<5)
 #define   CM0_DEPTH_EVICT_DISABLE (1<<4)
 #define   CM0_COLOR_EVICT_DISABLE (1<<3)
 #define   CM0_DEPTH_WRITE_DISABLE (1<<1)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dce64d8..e162d8a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8510,6 +8510,10 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(WM2_LP_ILK, 0);
 	I915_WRITE(WM1_LP_ILK, 0);
 
+	/* clear masked bit */
+	I915_WRITE(CACHE_MODE_0,
+		   CM0_STC_EVICT_DISABLE_LRA_SNB << CM0_MASK_SHIFT);
+
 	I915_WRITE(ECOSKPD, (ECO_ASYNC_FLUSH_FIX_REVERT << 16) |
 			    ECO_ASYNC_FLUSH_FIX_REVERT);
 
-- 
1.7.7.6

Re: [PATCH 6/7] drm/i915: implement async flush w/a

[Chris Wilson]

On Sat, 31 Mar 2012 11:22:02 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Note that async flush also means things like VT-d IOTLB invalidation.
> 
> See Bspec vol1c.4 "Render Engine Command Streamer", Section "ECOSKPD -
> Eco Scratch Pad".
> 
> It doesn't seem to help in for any of our VT-d related bugs thoug.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    3 +++
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a7ef74a..5b23d5c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -583,6 +583,7 @@
>  #define BB_ADDR		0x02140 /* 8 bytes */
>  #define GFX_FLSH_CNTL	0x02170 /* 915+ only */
>  #define ECOSKPD		0x021d0
> +#define   ECO_ASYNC_FLUSH_FIX_REVERT (1<<7)
>  #define   ECO_GATING_CX_ONLY	(1<<3)
>  #define   ECO_FLIP_DONE		(1<<0)
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dab2381..dce64d8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8510,6 +8510,9 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(WM2_LP_ILK, 0);
>  	I915_WRITE(WM1_LP_ILK, 0);
>  
> +	I915_WRITE(ECOSKPD, (ECO_ASYNC_FLUSH_FIX_REVERT << 16) |
> +			    ECO_ASYNC_FLUSH_FIX_REVERT);
Just a request for a standardised means of setting masked bits:

#define MASKED_ENABLE(x) ((x) << 16) | (x))
#define MASKED_DISABLE(x) ((x) << 16) | 0)

see MI_MODE, GFX_MODE.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/7] drm/i915: implement ColorBlt w/a

[Chris Wilson]

On Sat, 31 Mar 2012 11:21:57 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> According to an internal workaround master list, we need to set bit 5
> of register 9400 to avoid issues with color blits.

This sounds like it could be the root cause behind the FBC + BLT hangs.
But not the XY_COPY hangs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/7] drm/i915: implement ColorBlt w/a

[Chris Wilson]

On Sat, 31 Mar 2012 11:54:01 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Sat, 31 Mar 2012 11:21:57 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > According to an internal workaround master list, we need to set bit 5
> > of register 9400 to avoid issues with color blits.
> 
> This sounds like it could be the root cause behind the FBC + BLT hangs.

This looks like it could be true, I just tested the w/a with the FBC
enabled and it survives!

Tested-by: Chris Wilson <chris@chris-wilson.co.uk>

I'm fully expecting it to blow up later, but one step forward.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/7] drm/i915: implement ColorBlt w/a

[Michael Groh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello everyone,

it looks like this does indeed fix the fbc problem. I applied the
patch, and did some testing. I am using a Dell Latitude e6420, this is
my setup:

brotscheibe brot # cat /sys/kernel/debug/dri/0/i915_fbc_status
FBC enabled
brotscheibe brot # uname -a
Linux brotscheibe 3.4.0-rc2+ #52 SMP PREEMPT Mon Apr 9 21:51:57 CEST
2012 x86_64 Intel(R) Core(TM) i5-2540M CPU @ 2.60GHz GenuineIntel
GNU/Linux

After nearly an hour of testing, i got no hangs. Even when using rxvt
and "while (true) do dmesg; done".

I will do more testing tomorrow, but i dont think anything will fail.

You can add my tested-by: Michael "brot" Groh <michael.groh@minad.de>

Thanks for your work and have a nice day,
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iF4EAREIAAYFAk+D+I8ACgkQ+SqCF1IBR0SzSgD/bkf6W7axjuaWe855f/5oqFSC
Opt4J1pz6HP/qNYFD/AA/0Hw4XvvY085OkEQ++Hd+Q1dvvWXLeukuSf6dDfgokFq
=Dt1V
-----END PGP SIGNATURE-----

Re: [PATCH 6/7] drm/i915: implement async flush w/a

[Paul Menzel]

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

Am Samstag, den 31.03.2012, 11:52 +0100 schrieb Chris Wilson:
> On Sat, 31 Mar 2012 11:22:02 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Note that async flush also means things like VT-d IOTLB invalidation.
> > 
> > See Bspec vol1c.4 "Render Engine Command Streamer", Section "ECOSKPD -
> > Eco Scratch Pad".
> > 
> > It doesn't seem to help in for any of our VT-d related bugs thoug.

While we are at it: thoug*h*.

> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |    1 +
> >  drivers/gpu/drm/i915/intel_display.c |    3 +++
> >  2 files changed, 4 insertions(+), 0 deletions(-)

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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 1/7] drm/i915: implement ColorBlt w/a

[Paul Menzel]

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

Am Samstag, den 31.03.2012, 11:21 +0200 schrieb Daniel Vetter:
> According to an internal workaround master list, we need to set bit 5
> of register 9400 to avoid issues with color blits.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    3 +++
>  drivers/gpu/drm/i915/intel_display.c |    4 ++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3886cf0..233dbd5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3727,6 +3727,9 @@
>  #define  GT_FIFO_FREE_ENTRIES			0x120008
>  #define    GT_FIFO_NUM_RESERVED_ENTRIES		20
>  
> +#define GEN6_UCGCTL1				0x9400
> +# define GEN6_BLBUNIT_CLOCK_GATE_DISABLE		(1 << 5)
> +
>  #define GEN6_UCGCTL2				0x9404
>  # define GEN6_RCZUNIT_CLOCK_GATE_DISABLE		(1 << 13)
>  # define GEN6_RCPBUNIT_CLOCK_GATE_DISABLE		(1 << 12)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d514719..aa0c6df 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8510,6 +8510,10 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(WM2_LP_ILK, 0);
>  	I915_WRITE(WM1_LP_ILK, 0);
>  
> +	I915_WRITE(GEN6_UCGCTL1,
> +		   I915_READ(GEN6_UCGCTL1) |
> +		   GEN6_BLBUNIT_CLOCK_GATE_DISABLE);
> +

Should a comment be added to the header file or here that this is from
an internal workaround list? (Same for patch 7/7 I think.)

>  	/* According to the BSpec vol1g, bit 12 (RCPBUNIT) clock
>  	 * gating disable must be set.  Failure to set it results in
>  	 * flickering pixels due to Z write ordering failures after


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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 2/7] drm/i915: implement a media hang w/a

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:21:58AM +0200, Daniel Vetter wrote:
> Contrary to the other clock gating w/a in GEN6_UCGCTL1, this one is
> actually documented in Bspec, vol1g "GT Interface Registers [SNB]",
> Section 1.5.1 "UCGCTL1 - Unit Level Clock Gating Control 1".
> 
> Supposedly this can prevent hangs on the media ring.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

According to my bspec, this is IVB A0 only. ie. nobody should ever need
this.

Re: [PATCH 2/7] drm/i915: implement a media hang w/a

[Ben Widawsky]

On Tue, Apr 10, 2012 at 02:30:20PM -0700, Ben Widawsky wrote:
> On Sat, Mar 31, 2012 at 11:21:58AM +0200, Daniel Vetter wrote:
> > Contrary to the other clock gating w/a in GEN6_UCGCTL1, this one is
> > actually documented in Bspec, vol1g "GT Interface Registers [SNB]",
> > Section 1.5.1 "UCGCTL1 - Unit Level Clock Gating Control 1".
> > 
> > Supposedly this can prevent hangs on the media ring.
> > 
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> 
> According to my bspec, this is IVB A0 only. ie. nobody should ever need
> this.

My bad... needed on SNB only, and not IVB.

Re: [PATCH 2/7] drm/i915: implement a media hang w/a

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:21:58AM +0200, Daniel Vetter wrote:
> Contrary to the other clock gating w/a in GEN6_UCGCTL1, this one is
> actually documented in Bspec, vol1g "GT Interface Registers [SNB]",
> Section 1.5.1 "UCGCTL1 - Unit Level Clock Gating Control 1".
> 
> Supposedly this can prevent hangs on the media ring.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 3/7] drm/i915: set w/a bit for snb pagefaults

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:21:59AM +0200, Daniel Vetter wrote:
> Bspec says that we need to set this: vol1c.3 "Blitter Command
> Streamer", Section 1.1.2.1 "GAB_CTL_REG - GAB Unit Control Register".
> 
> We don't really rely on pagefaults, but who knows what this all
> affects.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 4/7] drm/i915: properly set ppgtt cacheability on snb

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:22:00AM +0200, Daniel Vetter wrote:
> For some reason snb has 2 fields to set ppgtt cacheability. This one
> here does not exist on gen7.
> 
> This might explain why ppgtt wasn't a win on snb like on ivb - not
> enough pte caching.

So, is it a win now?


> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |    3 +++
>  drivers/gpu/drm/i915/i915_reg.h |    4 ++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5dc5f42..cab70ef 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3766,6 +3766,9 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
>  	if (INTEL_INFO(dev)->gen == 6) {
>  		uint32_t ecochk, gab_ctl, ecobits;
>  
> +		ecobits = I915_READ(GAC_ECO_BITS); 
> +		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
> +
>  		gab_ctl = I915_READ(GAB_CTL);
>  		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
>  

Is this a bad rebase? This doesn't apply to whatever I am using.

> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e72c251..5457980 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -125,6 +125,10 @@
>  #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
>  #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
>  
> +#define GAC_ECO_BITS			0x14090
> +#define   ECOBITS_PPGTT_CACHE64B	(3<<8)
> +#define   ECOBITS_PPGTT_CACHE4B		(0<<8)
> +
>  #define GAB_CTL				0x24000
>  #define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
>  

According to bspec, but 13 must be set to a 1 also.

Re: [PATCH 5/7] drm/i915: implement w/a for incorrect guarband clipping

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:22:01AM +0200, Daniel Vetter wrote:
> According to Bsepc, this should be set by default, but isn't. See vo1c.4
> "Render Engine Command Streamer", Section 1.1.14.3 "3D_CHICKEN3"
> 
> Bspec also says that we always need to set all mask bits.
I think this deserves to be a comment in the code itself. That really
made me say WTF when looking at the patch.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 5/7] drm/i915: implement w/a for incorrect guarband clipping

[Daniel Vetter]

On Tue, Apr 10, 2012 at 03:13:52PM -0700, Ben Widawsky wrote:
> On Sat, Mar 31, 2012 at 11:22:01AM +0200, Daniel Vetter wrote:
> > According to Bsepc, this should be set by default, but isn't. See vo1c.4
> > "Render Engine Command Streamer", Section 1.1.14.3 "3D_CHICKEN3"
> > 
> > Bspec also says that we always need to set all mask bits.
> I think this deserves to be a comment in the code itself. That really
> made me say WTF when looking at the patch.

Agreed, I'll add a comment to that effect.

> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 6/7] drm/i915: implement async flush w/a

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:22:02AM +0200, Daniel Vetter wrote:
> Note that async flush also means things like VT-d IOTLB invalidation.
> 
> See Bspec vol1c.4 "Render Engine Command Streamer", Section "ECOSKPD -
> Eco Scratch Pad".
> 
> It doesn't seem to help in for any of our VT-d related bugs thoug.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I think we don't want this one from what the bspec says, though I am a
bit unclear.

So I intend to not r-b this one unless I manage to learn how to work our
internal bug database better.

Ben

Re: [PATCH 1/7] drm/i915: implement ColorBlt w/a

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:21:57AM +0200, Daniel Vetter wrote:
> According to an internal workaround master list, we need to set bit 5
> of register 9400 to avoid issues with color blits.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm having a lot of trouble actually tracking this one down in something
other than the magical spreadsheet.

So I'll for now, this is only
Acked-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 4/7] drm/i915: properly set ppgtt cacheability on snb

[Daniel Vetter]

On Tue, Apr 10, 2012 at 03:11:07PM -0700, Ben Widawsky wrote:
> On Sat, Mar 31, 2012 at 11:22:00AM +0200, Daniel Vetter wrote:
> > For some reason snb has 2 fields to set ppgtt cacheability. This one
> > here does not exist on gen7.
> > 
> > This might explain why ppgtt wasn't a win on snb like on ivb - not
> > enough pte caching.
> 
> So, is it a win now?

I've later noticed that the register is part of the bsd block, so I've
figured I'm not gonna benchmark vaapi. I'm a lazy bastard.

> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c |    3 +++
> >  drivers/gpu/drm/i915/i915_reg.h |    4 ++++
> >  2 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 5dc5f42..cab70ef 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3766,6 +3766,9 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> >  	if (INTEL_INFO(dev)->gen == 6) {
> >  		uint32_t ecochk, gab_ctl, ecobits;
> >  
> > +		ecobits = I915_READ(GAC_ECO_BITS); 
> > +		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
> > +
> >  		gab_ctl = I915_READ(GAB_CTL);
> >  		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
> >  
> 
> Is this a bad rebase? This doesn't apply to whatever I am using.

Mucks around in the same area as the previous patch and hence conflicts
without that applied.

> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e72c251..5457980 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -125,6 +125,10 @@
> >  #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
> >  #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
> >  
> > +#define GAC_ECO_BITS			0x14090
> > +#define   ECOBITS_PPGTT_CACHE64B	(3<<8)
> > +#define   ECOBITS_PPGTT_CACHE4B		(0<<8)
> > +
> >  #define GAB_CTL				0x24000
> >  #define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
> >  
> 
> According to bspec, but 13 must be set to a 1 also.

I've checked and bit13 seems to be set already on my snb, so I've just
or'ed in the missing bits.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 7/7] drm/i915: set stc evict disable lra evict w/a

[Ben Widawsky]

On Sat, Mar 31, 2012 at 11:22:03AM +0200, Daniel Vetter wrote:
> Our workaround list kindly lists that this new default value needs to
> be updated in Bspec. Naturally, this did not happen.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Since the bspec says nothing, I think we need to do some performance
testing on this one. I don't even know what STC is, or how changing it's
replacement policy can impact thing.


So again, a-b because I looked at the spreadsheet, but I can't r-b this
without more info than I have at present.

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    4 ++++
>  2 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5b23d5c..d0d0e2f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -576,6 +576,7 @@
>  #define   CM0_MASK_SHIFT          16
>  #define   CM0_IZ_OPT_DISABLE      (1<<6)
>  #define   CM0_ZR_OPT_DISABLE      (1<<5)
> +#define	  CM0_STC_EVICT_DISABLE_LRA_SNB	(1<<5)
>  #define   CM0_DEPTH_EVICT_DISABLE (1<<4)
>  #define   CM0_COLOR_EVICT_DISABLE (1<<3)
>  #define   CM0_DEPTH_WRITE_DISABLE (1<<1)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dce64d8..e162d8a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8510,6 +8510,10 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(WM2_LP_ILK, 0);
>  	I915_WRITE(WM1_LP_ILK, 0);
>  
> +	/* clear masked bit */
> +	I915_WRITE(CACHE_MODE_0,
> +		   CM0_STC_EVICT_DISABLE_LRA_SNB << CM0_MASK_SHIFT);
> +
>  	I915_WRITE(ECOSKPD, (ECO_ASYNC_FLUSH_FIX_REVERT << 16) |
>  			    ECO_ASYNC_FLUSH_FIX_REVERT);
>  
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: properly set ppgtt cacheability on snb

[Ben Widawsky]

On Wed, Apr 11, 2012 at 12:27:25AM +0200, Daniel Vetter wrote:
> On Tue, Apr 10, 2012 at 03:11:07PM -0700, Ben Widawsky wrote:
> > On Sat, Mar 31, 2012 at 11:22:00AM +0200, Daniel Vetter wrote:
> > > For some reason snb has 2 fields to set ppgtt cacheability. This one
> > > here does not exist on gen7.
> > > 
> > > This might explain why ppgtt wasn't a win on snb like on ivb - not
> > > enough pte caching.
> > 
> > So, is it a win now?
> 
> I've later noticed that the register is part of the bsd block, so I've
> figured I'm not gonna benchmark vaapi. I'm a lazy bastard.
> 
> > > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c |    3 +++
> > >  drivers/gpu/drm/i915/i915_reg.h |    4 ++++
> > >  2 files changed, 7 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 5dc5f42..cab70ef 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3766,6 +3766,9 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
> > >  	if (INTEL_INFO(dev)->gen == 6) {
> > >  		uint32_t ecochk, gab_ctl, ecobits;
> > >  
> > > +		ecobits = I915_READ(GAC_ECO_BITS); 
> > > +		I915_WRITE(GAC_ECO_BITS, ecobits | ECOBITS_PPGTT_CACHE64B);
> > > +
> > >  		gab_ctl = I915_READ(GAB_CTL);
> > >  		I915_WRITE(GAB_CTL, gab_ctl | GAB_CTL_CONT_AFTER_PAGEFAULT);
> > >  
> > 
> > Is this a bad rebase? This doesn't apply to whatever I am using.
> 
> Mucks around in the same area as the previous patch and hence conflicts
> without that applied.

Rebase fail was from previous patch, as discussed on IRC> ecobits was
defined before its time.

> 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index e72c251..5457980 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -125,6 +125,10 @@
> > >  #define   ECOCHK_PPGTT_CACHE64B		(0x3<<3)
> > >  #define   ECOCHK_PPGTT_CACHE4B		(0x0<<3)
> > >  
> > > +#define GAC_ECO_BITS			0x14090
> > > +#define   ECOBITS_PPGTT_CACHE64B	(3<<8)
> > > +#define   ECOBITS_PPGTT_CACHE4B		(0<<8)
> > > +
> > >  #define GAB_CTL				0x24000
> > >  #define   GAB_CTL_CONT_AFTER_PAGEFAULT	(1<<8)
> > >  
> > 
> > According to bspec, but 13 must be set to a 1 also.
> 
> I've checked and bit13 seems to be set already on my snb, so I've just
> or'ed in the missing bits.

I think if we're going to be pedantic about things, which I approve of,
and this series is doing, you may as well set 13 also... but whatever
you want. I don't care too much.

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

> -Daniel
> -- 
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915: implement ColorBlt w/a

[Daniel Vetter]

On Tue, Apr 10, 2012 at 03:24:13PM -0700, Ben Widawsky wrote:
> On Sat, Mar 31, 2012 at 11:21:57AM +0200, Daniel Vetter wrote:
> > According to an internal workaround master list, we need to set bit 5
> > of register 9400 to avoid issues with color blits.
> > 
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'm having a lot of trouble actually tracking this one down in something
> other than the magical spreadsheet.
> 
> So I'll for now, this is only
> Acked-by: Ben Widawsky <ben@bwidawsk.net>
I've picked this up for -fixes, thanks for wrestling through hsd and that
ugly xls.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48