[PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[Jesse Barnes]

GFX_MODE controls important behavior like PPGTT, run lists, and TLB
invalidate behavior.  On the SDV I'm using, the TLB invalidation mode
was defaulting to "pipe control only" which meant regular MI_FLUSHes
wouldn't actually flush the TLB, leading to all sorts of stale data
getting used.

So initialize it to 0 at ring buffer init time until we actually use
PIPE_CONTROL for TLB invalidation.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h         |    2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |    2 ++
 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 7033e01..9f3938c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -382,6 +382,8 @@
 #define   GFX_PSMI_GRANULARITY		(1<<10)
 #define   GFX_PPGTT_ENABLE		(1<<9)
 
+#define GFX_MODE_GEN7	0x0229c
+
 #define SCPD0		0x0209c /* 915+ only */
 #define IER		0x020a0
 #define IIR		0x020a4
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 47b9b27..80fea69 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1293,6 +1293,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->add_request = gen6_add_request;
 		ring->irq_get = gen6_render_ring_get_irq;
 		ring->irq_put = gen6_render_ring_put_irq;
+		if (IS_GEN7(dev))
+			I915_WRITE(GFX_MODE_GEN7, 0xffff0000);
 	} else if (IS_GEN5(dev)) {
 		ring->add_request = pc_render_add_request;
 		ring->get_seqno = pc_render_get_seqno;
-- 
1.7.4.1

[PATCH 2/2] drm/i915: suspend/resume rps and ring freq state on IVB

[Jesse Barnes]

These are enabled at init time, but we need to save/restore them as well.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_suspend.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 87677d6..cd79e55 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -820,7 +820,7 @@ int i915_save_state(struct drm_device *dev)
 
 	if (IS_IRONLAKE_M(dev))
 		ironlake_disable_drps(dev);
-	if (IS_GEN6(dev))
+	if (IS_GEN6(dev) || IS_GEN7(dev))
 		gen6_disable_rps(dev);
 
 	/* Cache mode state */
@@ -878,7 +878,7 @@ int i915_restore_state(struct drm_device *dev)
 		intel_init_emon(dev);
 	}
 
-	if (IS_GEN6(dev)) {
+	if (IS_GEN6(dev) || IS_GEN7(dev)) {
 		gen6_enable_rps(dev_priv);
 		gen6_update_ring_freq(dev_priv);
 	}
-- 
1.7.4.1

Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[Jesse Barnes]

On Fri, 12 Aug 2011 14:55:32 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> GFX_MODE controls important behavior like PPGTT, run lists, and TLB
> invalidate behavior.  On the SDV I'm using, the TLB invalidation mode
> was defaulting to "pipe control only" which meant regular MI_FLUSHes
> wouldn't actually flush the TLB, leading to all sorts of stale data
> getting used.
> 
> So initialize it to 0 at ring buffer init time until we actually use
> PIPE_CONTROL for TLB invalidation.

Ignore this one, see below for an updated patch that uses bit
definitions and makes sure the register gets reset at GPU reset time as
well.

-- 
Jesse Barnes, Intel Open Source Technology Center

>From ac1b88dc2f4cd3a00746063899c6e6be4d5f2065 Mon Sep 17 00:00:00 2001
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Fri, 12 Aug 2011 15:15:06 -0700
Subject: [PATCH] drm/i915: set GFX_MODE to pre-Ivybridge default value even on Ivybridge

Prior to Ivybridge, the GFX_MODE would default to 0x800, meaning that
MI_FLUSH would flush the TLBs in addition to the rest of the caches
indicated in the MI_FLUSH command.  However starting with Ivybridge, the
register defaults to 0x2800 out of reset, meaning that to invalidate the
TLB we need to use PIPE_CONTROL.  Since we're not doing that yet, go
back to the old default so things work.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h         |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.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 7033e01..26641ad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -375,6 +375,7 @@
 # define MI_FLUSH_ENABLE				(1 << 11)
 
 #define GFX_MODE	0x02520
+#define GFX_MODE_GEN7	0x0229c
 #define   GFX_RUN_LIST_ENABLE		(1<<15)
 #define   GFX_TLB_INVALIDATE_ALWAYS	(1<<13)
 #define   GFX_SURFACE_FAULT_ENABLE	(1<<12)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 47b9b27..6dad947 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -290,6 +290,9 @@ static int init_render_ring(struct intel_ring_buffer *ring)
 		if (IS_GEN6(dev) || IS_GEN7(dev))
 			mode |= MI_FLUSH_ENABLE << 16 | MI_FLUSH_ENABLE;
 		I915_WRITE(MI_MODE, mode);
+		if (IS_GEN7(dev))
+			I915_WRITE(GFX_MODE_GEN7, (GFX_REPLAY_MODE << 16) |
+				   GFX_REPLAY_MODE);
 	}
 
 	if (INTEL_INFO(dev)->gen >= 6) {
-- 
1.7.4.1

Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[Jesse Barnes]

On Fri, 12 Aug 2011 15:18:09 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Fri, 12 Aug 2011 14:55:32 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > GFX_MODE controls important behavior like PPGTT, run lists, and TLB
> > invalidate behavior.  On the SDV I'm using, the TLB invalidation mode
> > was defaulting to "pipe control only" which meant regular MI_FLUSHes
> > wouldn't actually flush the TLB, leading to all sorts of stale data
> > getting used.
> > 
> > So initialize it to 0 at ring buffer init time until we actually use
> > PIPE_CONTROL for TLB invalidation.
> 
> Ignore this one, see below for an updated patch that uses bit
> definitions and makes sure the register gets reset at GPU reset time as
> well.

Ignore the last one too.  Third time's the charm!

-- 
Jesse Barnes, Intel Open Source Technology Center


>From 91f24a59e03b338e114ecafe5c589c1f36119c02 Mon Sep 17 00:00:00 2001
From: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Fri, 12 Aug 2011 15:15:06 -0700
Subject: [PATCH] drm/i915: set GFX_MODE to pre-Ivybridge default value even on Ivybridge

Prior to Ivybridge, the GFX_MODE would default to 0x800, meaning that
MI_FLUSH would flush the TLBs in addition to the rest of the caches
indicated in the MI_FLUSH command.  However starting with Ivybridge, the
register defaults to 0x2800 out of reset, meaning that to invalidate the
TLB we need to use PIPE_CONTROL.  Since we're not doing that yet, go
back to the old default so things work.

v2: don't forget to actually *clear* the new bit

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_reg.h         |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.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 7033e01..26641ad 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -375,6 +375,7 @@
 # define MI_FLUSH_ENABLE				(1 << 11)
 
 #define GFX_MODE	0x02520
+#define GFX_MODE_GEN7	0x0229c
 #define   GFX_RUN_LIST_ENABLE		(1<<15)
 #define   GFX_TLB_INVALIDATE_ALWAYS	(1<<13)
 #define   GFX_SURFACE_FAULT_ENABLE	(1<<12)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 47b9b27..0b17036 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -290,6 +290,10 @@ static int init_render_ring(struct intel_ring_buffer *ring)
 		if (IS_GEN6(dev) || IS_GEN7(dev))
 			mode |= MI_FLUSH_ENABLE << 16 | MI_FLUSH_ENABLE;
 		I915_WRITE(MI_MODE, mode);
+		if (IS_GEN7(dev))
+			I915_WRITE(GFX_MODE_GEN7, ((GFX_TLB_INVALIDATE_ALWAYS |
+						    GFX_REPLAY_MODE) << 16) |
+				   GFX_REPLAY_MODE);
 	}
 
 	if (INTEL_INFO(dev)->gen >= 6) {
-- 
1.7.4.1

Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[Chris Wilson]

On Fri, 12 Aug 2011 15:28:32 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>  		I915_WRITE(MI_MODE, mode);
> +		if (IS_GEN7(dev))
> +			I915_WRITE(GFX_MODE_GEN7, ((GFX_TLB_INVALIDATE_ALWAYS |
> +						    GFX_REPLAY_MODE) << 16) |
> +				   GFX_REPLAY_MODE);

That's maximally confusing indentation ;-)
Also it reads like that is a chicken bit, as in order to invalidate always
on flush we need to clear it? Can we play around with the name to avoid
confusion in the code and confusion with the spec?

#define ENABLE(x) ((x) << 16 | (x))
#define DISABLE(x) ((x) << 16 | 0)

Granted finding good names for those is hard. Perhaps ENABLE_16(x),
DISABLE_16(x) to indicate the mask size.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time

[Kenneth Graunke]

On 08/12/2011 03:28 PM, Jesse Barnes wrote:
> On Fri, 12 Aug 2011 15:18:09 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
>> On Fri, 12 Aug 2011 14:55:32 -0700
>> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>>
>>> GFX_MODE controls important behavior like PPGTT, run lists, and TLB
>>> invalidate behavior.  On the SDV I'm using, the TLB invalidation mode
>>> was defaulting to "pipe control only" which meant regular MI_FLUSHes
>>> wouldn't actually flush the TLB, leading to all sorts of stale data
>>> getting used.
>>>
>>> So initialize it to 0 at ring buffer init time until we actually use
>>> PIPE_CONTROL for TLB invalidation.
>>
>> Ignore this one, see below for an updated patch that uses bit
>> definitions and makes sure the register gets reset at GPU reset time as
>> well.
> 
> Ignore the last one too.  Third time's the charm!

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

With drm-intel-next, I've been seeing frequent but intermittent
rendering corruption in GNOME Terminal, rendercheck failures, and the
occasional kernel crash.

After applying the patch, I was able to run for an entire day without
any crashes or rendering corruption.  To double check, I reverted this
patch and was immediately able to reproduce my earlier problems.

With this patch and C0 hardware, my system seems quite stable.  Thanks
Jesse!