public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time
@ 2011-08-12 21:55 Jesse Barnes
  2011-08-12 21:55 ` [PATCH 2/2] drm/i915: suspend/resume rps and ring freq state on IVB Jesse Barnes
  2011-08-12 22:18 ` [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
  0 siblings, 2 replies; 6+ messages in thread
From: Jesse Barnes @ 2011-08-12 21:55 UTC (permalink / raw)
  To: intel-gfx

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

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

* [PATCH 2/2] drm/i915: suspend/resume rps and ring freq state on IVB
  2011-08-12 21:55 [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
@ 2011-08-12 21:55 ` Jesse Barnes
  2011-08-12 22:18 ` [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
  1 sibling, 0 replies; 6+ messages in thread
From: Jesse Barnes @ 2011-08-12 21:55 UTC (permalink / raw)
  To: intel-gfx

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

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

* Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time
  2011-08-12 21:55 [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
  2011-08-12 21:55 ` [PATCH 2/2] drm/i915: suspend/resume rps and ring freq state on IVB Jesse Barnes
@ 2011-08-12 22:18 ` Jesse Barnes
  2011-08-12 22:28   ` Jesse Barnes
  1 sibling, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2011-08-12 22:18 UTC (permalink / raw)
  To: intel-gfx

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

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

* Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time
  2011-08-12 22:18 ` [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
@ 2011-08-12 22:28   ` Jesse Barnes
  2011-08-12 22:56     ` Chris Wilson
  2011-08-15 23:59     ` Kenneth Graunke
  0 siblings, 2 replies; 6+ messages in thread
From: Jesse Barnes @ 2011-08-12 22:28 UTC (permalink / raw)
  Cc: intel-gfx

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

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

* Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time
  2011-08-12 22:28   ` Jesse Barnes
@ 2011-08-12 22:56     ` Chris Wilson
  2011-08-15 23:59     ` Kenneth Graunke
  1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2011-08-12 22:56 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

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

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

* Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time
  2011-08-12 22:28   ` Jesse Barnes
  2011-08-12 22:56     ` Chris Wilson
@ 2011-08-15 23:59     ` Kenneth Graunke
  1 sibling, 0 replies; 6+ messages in thread
From: Kenneth Graunke @ 2011-08-15 23:59 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

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!

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

end of thread, other threads:[~2011-08-15 23:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-12 21:55 [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
2011-08-12 21:55 ` [PATCH 2/2] drm/i915: suspend/resume rps and ring freq state on IVB Jesse Barnes
2011-08-12 22:18 ` [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init time Jesse Barnes
2011-08-12 22:28   ` Jesse Barnes
2011-08-12 22:56     ` Chris Wilson
2011-08-15 23:59     ` Kenneth Graunke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox