[PATCH 0/9] HW contest support for Ironlake

[PATCH 0/9] HW contest support for Ironlake

[Ben Widawsky]

By the request of Ken, and Chris, I've added support for HW contexts to
Ironlake. This is not the first time I've done this, but I think I'll
skip the somewhat ugly history on the matter.

Our existing support for Ironlake sets up 2 contexts, the powerctx, and
the renderctx. The former is stored in special memory on GEN6+, and the
latter is *extremely* similar to the way we use default context on GEN6+
(ie. if users don't opt in to contexts, they are using it even if they
don't realize).

The patch series first tries to tie that renderctx into our existing
i915 context code, and then rips out the old renderctx. Powerctx is left
alone since it still really is a special case.

Finally, in the series I reenable rc6 on ILK. I've never had an issue
with this on my ILK, but I recommend anyone testing on it who sees
issues to try to disable it via modparam firstly.

The patch series is minimally tested. I told Chris and Ken at the
offset, they'd need to do the heavy lifting on the testing front.

Ben Widawsky (9):
  drm/i915: move ilk rc6 context setup
  drm/i915: Convert renderctx to a regular context
  drm/i915: Make ILK context objects more like others
  drm/i915: Add gen5 support to mi_set_context
  drm/i915: Use do_switch for ILK renderctx
  drm/i915: HW contexts for ILK
  drm/i915: Use only the default context for ILK
  drm/i915: Restore ILK powerctx pin attributes
  drm/i915: Re-enable rc6 on ILK (again^5)

 drivers/gpu/drm/i915/i915_debugfs.c     |  6 ----
 drivers/gpu/drm/i915/i915_drv.h         |  3 +-
 drivers/gpu/drm/i915/i915_gem_context.c | 28 ++++++++++++++---
 drivers/gpu/drm/i915/i915_reg.h         |  1 +
 drivers/gpu/drm/i915/intel_pm.c         | 56 ---------------------------------
 5 files changed, 25 insertions(+), 69 deletions(-)

-- 
1.8.3.2

[PATCH 1/9] drm/i915: move ilk rc6 context setup

[Ben Widawsky]

Put it with the other context code since upcoming rework/enabling will
be easier to handle with the similar code.

Also, remove the comment which is now common knowledge, and incorrect,
given that the powerctx is really the one that allows us to accomplish
this on Ironlake.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c         | 18 +-----------------
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cef35d3..03ab05c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1866,6 +1866,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 				   struct drm_file *file);
+int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv);
 
 /* i915_gem_gtt.c */
 void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2074544..ab72c72 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -577,3 +577,27 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
 	return 0;
 }
+
+int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv)
+{
+	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
+	int ret;
+
+	ret = intel_ring_begin(ring, 6);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
+	intel_ring_emit(ring, MI_SET_CONTEXT);
+	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx) |
+			MI_MM_SPACE_GTT |
+			MI_SAVE_EXT_STATE_EN |
+			MI_RESTORE_EXT_STATE_EN |
+			MI_RESTORE_INHIBIT);
+	intel_ring_emit(ring, MI_SUSPEND_FLUSH);
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_FLUSH);
+	intel_ring_advance(ring);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fb4afaa..2673602 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3687,29 +3687,13 @@ static void ironlake_enable_rc6(struct drm_device *dev)
 	was_interruptible = dev_priv->mm.interruptible;
 	dev_priv->mm.interruptible = false;
 
-	/*
-	 * GPU can automatically power down the render unit if given a page
-	 * to save state.
-	 */
-	ret = intel_ring_begin(ring, 6);
+	ret = ironlake_setup_rc6_hw_context(dev_priv);
 	if (ret) {
 		ironlake_teardown_rc6(dev);
 		dev_priv->mm.interruptible = was_interruptible;
 		return;
 	}
 
-	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
-	intel_ring_emit(ring, MI_SET_CONTEXT);
-	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx) |
-			MI_MM_SPACE_GTT |
-			MI_SAVE_EXT_STATE_EN |
-			MI_RESTORE_EXT_STATE_EN |
-			MI_RESTORE_INHIBIT);
-	intel_ring_emit(ring, MI_SUSPEND_FLUSH);
-	intel_ring_emit(ring, MI_NOOP);
-	intel_ring_emit(ring, MI_FLUSH);
-	intel_ring_advance(ring);
-
 	/*
 	 * Wait for the command parser to advance past MI_SET_CONTEXT. The HW
 	 * does an implicit flush, combined with MI_FLUSH above, it should be
-- 
1.8.3.2

[PATCH 2/9] drm/i915: Convert renderctx to a regular context

[Ben Widawsky]

The medium term plan is to just use the existing context code on ILK as
well. The conversion just assists. Therefore, this is somewhat transient
as I plan to kill renderctx quite soon.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h         |  2 +-
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 drivers/gpu/drm/i915/intel_pm.c         | 18 +++++++++++-------
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8637979..5e88ef6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1347,9 +1347,9 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		seq_putc(m, '\n');
 	}
 
-	if (dev_priv->ips.renderctx) {
+	if (dev_priv->ips.renderctx.obj) {
 		seq_puts(m, "render context ");
-		describe_obj(m, dev_priv->ips.renderctx);
+		describe_obj(m, dev_priv->ips.renderctx.obj);
 		seq_putc(m, '\n');
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 03ab05c..64c6ae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,7 +789,7 @@ struct intel_ilk_power_mgmt {
 	int r_t;
 
 	struct drm_i915_gem_object *pwrctx;
-	struct drm_i915_gem_object *renderctx;
+	struct i915_hw_context renderctx;
 };
 
 /* Power well structure for haswell */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ab72c72..2c25006 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -589,7 +589,7 @@ int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv)
 
 	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
 	intel_ring_emit(ring, MI_SET_CONTEXT);
-	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx) |
+	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx.obj) |
 			MI_MM_SPACE_GTT |
 			MI_SAVE_EXT_STATE_EN |
 			MI_RESTORE_EXT_STATE_EN |
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2673602..10f8f7cd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3615,10 +3615,10 @@ void ironlake_teardown_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx) {
-		i915_gem_object_unpin(dev_priv->ips.renderctx);
-		drm_gem_object_unreference(&dev_priv->ips.renderctx->base);
-		dev_priv->ips.renderctx = NULL;
+	if (dev_priv->ips.renderctx.obj) {
+		i915_gem_object_unpin(dev_priv->ips.renderctx.obj);
+		drm_gem_object_unreference(&dev_priv->ips.renderctx.obj->base);
+		dev_priv->ips.renderctx.obj = NULL;
 	}
 
 	if (dev_priv->ips.pwrctx) {
@@ -3650,10 +3650,14 @@ static int ironlake_setup_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx == NULL)
-		dev_priv->ips.renderctx = intel_alloc_context_page(dev);
-	if (!dev_priv->ips.renderctx)
+	if (dev_priv->ips.renderctx.obj == NULL)
+		dev_priv->ips.renderctx.obj = intel_alloc_context_page(dev);
+	if (!dev_priv->ips.renderctx.obj)
 		return -ENOMEM;
+	else {
+		dev_priv->ips.renderctx.id = DEFAULT_CONTEXT_ID;
+		dev_priv->ips.renderctx.ring = &dev_priv->ring[RCS];
+	}
 
 	if (dev_priv->ips.pwrctx == NULL)
 		dev_priv->ips.pwrctx = intel_alloc_context_page(dev);
-- 
1.8.3.2

[PATCH 3/9] drm/i915: Make ILK context objects more like others

[Ben Widawsky]

This is required so we can use the existing do_switch() which tries to
map objects as non mappable and 64k aligned can work. Again, this is
only required in order to ease the transition over to the existing
context code.

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

fixup! drm/i915: Convert renderctx to a regular context
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 10f8f7cd..fd91b74 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2858,7 +2858,7 @@ intel_alloc_context_page(struct drm_device *dev)
 		return NULL;
 	}
 
-	ret = i915_gem_object_pin(ctx, 4096, true, false);
+	ret = i915_gem_object_pin(ctx, 64 << 10, false, false);
 	if (ret) {
 		DRM_ERROR("failed to pin power context: %d\n", ret);
 		goto err_unref;
-- 
1.8.3.2

[PATCH 4/9] drm/i915: Add gen5 support to mi_set_context

[Ben Widawsky]

It's similar enough to the other gens that we don't really need a
distinct function to do it.

NOTE: The new function removes the MI_FLUSH that was at the end of the
old Ironlake switching code. Recent docs can find neither the
requirement for the MI_FLUSH or the MI_SUSPEND_FLUSH. Since I remember
clearly seeing the latter in a doc at one point however, I am going to
leave it there. I had Ken look at his docs as well, and he could also
find no note of either.

NOTE2: Remember to put NOTE1 in the commit where we start using the new
function for ILK.

CC: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 2c25006..a9beaaa 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -369,11 +369,18 @@ mi_set_context(struct intel_ring_buffer *ring,
 	if (ret)
 		return ret;
 
-	/* WaProgramMiArbOnOffAroundMiSetContext:ivb,vlv,hsw */
-	if (IS_GEN7(ring->dev))
+	switch (INTEL_INFO(ring->dev)->gen) {
+	case 7:
+		/* WaProgramMiArbOnOffAroundMiSetContext:ivb,vlv,hsw */
 		intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_DISABLE);
-	else
+		break;
+	case 5:
+		intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
+		break;
+	default:
 		intel_ring_emit(ring, MI_NOOP);
+		break;
+	}
 
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_emit(ring, MI_SET_CONTEXT);
@@ -385,14 +392,22 @@ mi_set_context(struct intel_ring_buffer *ring,
 	/* w/a: MI_SET_CONTEXT must always be followed by MI_NOOP */
 	intel_ring_emit(ring, MI_NOOP);
 
-	if (IS_GEN7(ring->dev))
+	switch (INTEL_INFO(ring->dev)->gen) {
+	case 7:
 		intel_ring_emit(ring, MI_ARB_ON_OFF | MI_ARB_ENABLE);
-	else
+		break;
+	case 5:
+		intel_ring_emit(ring, MI_SUSPEND_FLUSH);
+		break;
+	default:
 		intel_ring_emit(ring, MI_NOOP);
+		break;
+	}
 
 	intel_ring_advance(ring);
 
 	return ret;
+
 }
 
 static int do_switch(struct i915_hw_context *to)
-- 
1.8.3.2

[PATCH 5/9] drm/i915: Use do_switch for ILK renderctx

[Ben Widawsky]

This patch starts the migration to the core context code for Ironlake
renderctx. It is an excellent place for a bisection point due to
complaining hardware, though notice that our sample size is quite
limited given that only machines with RC6 turned on could notice
problems.

NOTE: As mentioned in a previous commit, this removes an MI_FLUSH. If
issues occur, try adding that to the end of mi_set_context before
complaining.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a9beaaa..f187e13 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -595,24 +595,5 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 
 int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv)
 {
-	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
-	int ret;
-
-	ret = intel_ring_begin(ring, 6);
-	if (ret)
-		return ret;
-
-	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
-	intel_ring_emit(ring, MI_SET_CONTEXT);
-	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx.obj) |
-			MI_MM_SPACE_GTT |
-			MI_SAVE_EXT_STATE_EN |
-			MI_RESTORE_EXT_STATE_EN |
-			MI_RESTORE_INHIBIT);
-	intel_ring_emit(ring, MI_SUSPEND_FLUSH);
-	intel_ring_emit(ring, MI_NOOP);
-	intel_ring_emit(ring, MI_FLUSH);
-	intel_ring_advance(ring);
-
-	return 0;
+	return do_switch(&dev_priv->ips.renderctx);
 }
-- 
1.8.3.2

[PATCH 6/9] drm/i915: HW contexts for ILK

[Ben Widawsky]

Turn on hardware contexts for Ironlake. This leaves the code in an
awkward place where renderctx accomplishes nothing, but the code
compiles and runs, and it makes the series overall more bisectable.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h         | 2 +-
 drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 64c6ae0..499caac 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1488,7 +1488,7 @@ struct drm_i915_file_private {
 #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
 #define I915_NEED_GFX_HWS(dev)	(INTEL_INFO(dev)->need_gfx_hws)
 
-#define HAS_HW_CONTEXTS(dev)	(INTEL_INFO(dev)->gen >= 6)
+#define HAS_HW_CONTEXTS(dev)	(INTEL_INFO(dev)->gen >= 5)
 #define HAS_ALIASING_PPGTT(dev)	(INTEL_INFO(dev)->gen >=6 && !IS_VALLEYVIEW(dev))
 
 #define HAS_OVERLAY(dev)		(INTEL_INFO(dev)->has_overlay)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index f187e13..5d10b37 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -106,6 +106,9 @@ static int get_context_size(struct drm_device *dev)
 	u32 reg;
 
 	switch (INTEL_INFO(dev)->gen) {
+	case 5:
+		ret = ILK_CXT_TOTAL_SIZE;
+		break;
 	case 6:
 		reg = I915_READ(CXT_SIZE);
 		ret = GEN6_CXT_TOTAL_SIZE(reg) * 64;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc3d6a7..9cb8b2b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1698,6 +1698,7 @@
 #define CCID			0x2180
 #define   CCID_EN		(1<<0)
 #define CXT_SIZE		0x21a0
+#define ILK_CXT_TOTAL_SIZE		(1 * PAGE_SIZE)
 #define GEN6_CXT_POWER_SIZE(cxt_reg)	((cxt_reg >> 24) & 0x3f)
 #define GEN6_CXT_RING_SIZE(cxt_reg)	((cxt_reg >> 18) & 0x3f)
 #define GEN6_CXT_RENDER_SIZE(cxt_reg)	((cxt_reg >> 12) & 0x3f)
-- 
1.8.3.2

[PATCH 7/9] drm/i915: Use only the default context for ILK

[Ben Widawsky]

The ILK renderctx predates the generic i915 context code. With the
patches before this, I believe I have properly eased the transition to
simply using the regular default context instead of the special
renderctx.

This untangles the weirdness from the last commit, and finishes the
transition.

The powerctx is still needed since that is a special ILK only thing. VLV
has a similar funkiness, so we could probably try to join those two, but
I see no point.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  6 -----
 drivers/gpu/drm/i915/i915_drv.h         |  2 --
 drivers/gpu/drm/i915/i915_gem_context.c |  5 -----
 drivers/gpu/drm/i915/intel_pm.c         | 40 ---------------------------------
 4 files changed, 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5e88ef6..4812493 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1347,12 +1347,6 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		seq_putc(m, '\n');
 	}
 
-	if (dev_priv->ips.renderctx.obj) {
-		seq_puts(m, "render context ");
-		describe_obj(m, dev_priv->ips.renderctx.obj);
-		seq_putc(m, '\n');
-	}
-
 	for_each_ring(ring, dev_priv, i) {
 		if (ring->default_context) {
 			seq_printf(m, "HW default context %s ring ", ring->name);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 499caac..23f14d7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,7 +789,6 @@ struct intel_ilk_power_mgmt {
 	int r_t;
 
 	struct drm_i915_gem_object *pwrctx;
-	struct i915_hw_context renderctx;
 };
 
 /* Power well structure for haswell */
@@ -1866,7 +1865,6 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 				   struct drm_file *file);
-int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv);
 
 /* i915_gem_gtt.c */
 void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5d10b37..6556f3d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -595,8 +595,3 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
 	return 0;
 }
-
-int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv)
-{
-	return do_switch(&dev_priv->ips.renderctx);
-}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fd91b74..21e61fc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3615,12 +3615,6 @@ void ironlake_teardown_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx.obj) {
-		i915_gem_object_unpin(dev_priv->ips.renderctx.obj);
-		drm_gem_object_unreference(&dev_priv->ips.renderctx.obj->base);
-		dev_priv->ips.renderctx.obj = NULL;
-	}
-
 	if (dev_priv->ips.pwrctx) {
 		i915_gem_object_unpin(dev_priv->ips.pwrctx);
 		drm_gem_object_unreference(&dev_priv->ips.pwrctx->base);
@@ -3650,15 +3644,6 @@ static int ironlake_setup_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx.obj == NULL)
-		dev_priv->ips.renderctx.obj = intel_alloc_context_page(dev);
-	if (!dev_priv->ips.renderctx.obj)
-		return -ENOMEM;
-	else {
-		dev_priv->ips.renderctx.id = DEFAULT_CONTEXT_ID;
-		dev_priv->ips.renderctx.ring = &dev_priv->ring[RCS];
-	}
-
 	if (dev_priv->ips.pwrctx == NULL)
 		dev_priv->ips.pwrctx = intel_alloc_context_page(dev);
 	if (!dev_priv->ips.pwrctx) {
@@ -3672,8 +3657,6 @@ static int ironlake_setup_rc6(struct drm_device *dev)
 static void ironlake_enable_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
-	bool was_interruptible;
 	int ret;
 
 	/* rc6 disabled by default due to repeated reports of hanging during
@@ -3688,29 +3671,6 @@ static void ironlake_enable_rc6(struct drm_device *dev)
 	if (ret)
 		return;
 
-	was_interruptible = dev_priv->mm.interruptible;
-	dev_priv->mm.interruptible = false;
-
-	ret = ironlake_setup_rc6_hw_context(dev_priv);
-	if (ret) {
-		ironlake_teardown_rc6(dev);
-		dev_priv->mm.interruptible = was_interruptible;
-		return;
-	}
-
-	/*
-	 * Wait for the command parser to advance past MI_SET_CONTEXT. The HW
-	 * does an implicit flush, combined with MI_FLUSH above, it should be
-	 * safe to assume that renderctx is valid
-	 */
-	ret = intel_ring_idle(ring);
-	dev_priv->mm.interruptible = was_interruptible;
-	if (ret) {
-		DRM_ERROR("failed to enable ironlake power savings\n");
-		ironlake_teardown_rc6(dev);
-		return;
-	}
-
 	I915_WRITE(PWRCTXA, i915_gem_obj_ggtt_offset(dev_priv->ips.pwrctx) | PWRCTX_EN);
 	I915_WRITE(RSTDBYCTL, I915_READ(RSTDBYCTL) & ~RCX_SW_EXIT);
 }
-- 
1.8.3.2

[PATCH 8/9] drm/i915: Restore ILK powerctx pin attributes

[Ben Widawsky]

Now that I've killed renderctx, and the ILK pm code no longer has
anything shared with the regular i915 context code, make the pin
arguments the same as how they were before I started.

I do not know the reason for the original pin arguments, so it's totally
possible this commit isn't necessary (and conversely that I temporarily
broke things earlier in the patch series; though I saw no such result).
However, since ILK RC6 worked very well for those of us where it worked,
I think messing with any of the code is unjustified.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 21e61fc..03b90aa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2858,7 +2858,7 @@ intel_alloc_context_page(struct drm_device *dev)
 		return NULL;
 	}
 
-	ret = i915_gem_object_pin(ctx, 64 << 10, false, false);
+	ret = i915_gem_object_pin(ctx, 4096, true, false);
 	if (ret) {
 		DRM_ERROR("failed to pin power context: %d\n", ret);
 		goto err_unref;
-- 
1.8.3.2

[PATCH 9/9] drm/i915: Re-enable rc6 on ILK (again^5)

[Ben Widawsky]

With the conversion to use the existing, well tested HW context code for
the ILK RC6 render context, let's once again try to enable RC6 by
default on ILK.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 03b90aa..c094aa3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3172,10 +3172,6 @@ int intel_enable_rc6(const struct drm_device *dev)
 	if (i915_enable_rc6 >= 0)
 		return i915_enable_rc6;
 
-	/* Disable RC6 on Ironlake */
-	if (INTEL_INFO(dev)->gen == 5)
-		return 0;
-
 	if (IS_HASWELL(dev)) {
 		DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
 		return INTEL_RC6_ENABLE;
-- 
1.8.3.2

[PATCH] i965: Gen5: Use HW contexts on Ironlake

[Ben Widawsky]

NOTE: The error messages might need updating if the patches don't make
3.11.

NOTE2: I'm not sure if mesa devs want to make HW contexts a hard
requirement on GEN5, as it is on GEN6+. I think letting the patches soak
for a few releases first, won't be a bad idea.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 src/mesa/drivers/dri/i965/brw_context.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index a72e226..4a0a6ad 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -311,7 +311,7 @@ brwCreateContext(int api,
    /* Reinitialize the context point state.  It depends on ctx->Const values. */
    _mesa_init_point(ctx);
 
-   if (brw->gen >= 6) {
+   if (brw->gen >= 5) {
       /* Create a new hardware context.  Using a hardware context means that
        * our GPU state will be saved/restored on context switch, allowing us
        * to assume that the GPU is in the same state we left it in.
@@ -322,7 +322,10 @@ brwCreateContext(int api,
       brw->hw_ctx = drm_intel_gem_context_create(brw->bufmgr);
 
       if (!brw->hw_ctx) {
-         fprintf(stderr, "Gen6+ requires Kernel 3.6 or later.\n");
+	 if (brw->gen == 5)
+	    fprintf(stderr, "Gen5 requires Kernel 3.11 or later.\n");
+	 else
+	    fprintf(stderr, "Gen6+ requires Kernel 3.6 or later.\n");
          ralloc_free(brw);
          return false;
       }
-- 
1.8.3.2

Re: [PATCH 0/9] HW contest support for Ironlake

[Ben Widawsky]

Why do I always forget this...?

You guys can easily clone the kernel sources here for easier testing:

http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=ilk_contexts

On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> By the request of Ken, and Chris, I've added support for HW contexts to
> Ironlake. This is not the first time I've done this, but I think I'll
> skip the somewhat ugly history on the matter.
> 
> Our existing support for Ironlake sets up 2 contexts, the powerctx, and
> the renderctx. The former is stored in special memory on GEN6+, and the
> latter is *extremely* similar to the way we use default context on GEN6+
> (ie. if users don't opt in to contexts, they are using it even if they
> don't realize).
> 
> The patch series first tries to tie that renderctx into our existing
> i915 context code, and then rips out the old renderctx. Powerctx is left
> alone since it still really is a special case.
> 
> Finally, in the series I reenable rc6 on ILK. I've never had an issue
> with this on my ILK, but I recommend anyone testing on it who sees
> issues to try to disable it via modparam firstly.
> 
> The patch series is minimally tested. I told Chris and Ken at the
> offset, they'd need to do the heavy lifting on the testing front.
> 
> Ben Widawsky (9):
>   drm/i915: move ilk rc6 context setup
>   drm/i915: Convert renderctx to a regular context
>   drm/i915: Make ILK context objects more like others
>   drm/i915: Add gen5 support to mi_set_context
>   drm/i915: Use do_switch for ILK renderctx
>   drm/i915: HW contexts for ILK
>   drm/i915: Use only the default context for ILK
>   drm/i915: Restore ILK powerctx pin attributes
>   drm/i915: Re-enable rc6 on ILK (again^5)
> 
>  drivers/gpu/drm/i915/i915_debugfs.c     |  6 ----
>  drivers/gpu/drm/i915/i915_drv.h         |  3 +-
>  drivers/gpu/drm/i915/i915_gem_context.c | 28 ++++++++++++++---
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  drivers/gpu/drm/i915/intel_pm.c         | 56 ---------------------------------
>  5 files changed, 25 insertions(+), 69 deletions(-)
> 
> -- 
> 1.8.3.2
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 0/9] HW context support for Ironlake (was: Re: [PATCH 0/9] HW contest support for Ironlake)

[Ben Widawsky]

Changed the mail subject... I'm not even sure what HW Contest support
might be.

On Sun, Jul 14, 2013 at 09:24:16AM -0700, Ben Widawsky wrote:
> Why do I always forget this...?
> 
> You guys can easily clone the kernel sources here for easier testing:
> 
> http://cgit.freedesktop.org/~bwidawsk/drm-intel/log/?h=ilk_contexts
> 
> On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> > By the request of Ken, and Chris, I've added support for HW contexts to
> > Ironlake. This is not the first time I've done this, but I think I'll
> > skip the somewhat ugly history on the matter.
> > 
> > Our existing support for Ironlake sets up 2 contexts, the powerctx, and
> > the renderctx. The former is stored in special memory on GEN6+, and the
> > latter is *extremely* similar to the way we use default context on GEN6+
> > (ie. if users don't opt in to contexts, they are using it even if they
> > don't realize).
> > 
> > The patch series first tries to tie that renderctx into our existing
> > i915 context code, and then rips out the old renderctx. Powerctx is left
> > alone since it still really is a special case.
> > 
> > Finally, in the series I reenable rc6 on ILK. I've never had an issue
> > with this on my ILK, but I recommend anyone testing on it who sees
> > issues to try to disable it via modparam firstly.
> > 
> > The patch series is minimally tested. I told Chris and Ken at the
> > offset, they'd need to do the heavy lifting on the testing front.
> > 
> > Ben Widawsky (9):
> >   drm/i915: move ilk rc6 context setup
> >   drm/i915: Convert renderctx to a regular context
> >   drm/i915: Make ILK context objects more like others
> >   drm/i915: Add gen5 support to mi_set_context
> >   drm/i915: Use do_switch for ILK renderctx
> >   drm/i915: HW contexts for ILK
> >   drm/i915: Use only the default context for ILK
> >   drm/i915: Restore ILK powerctx pin attributes
> >   drm/i915: Re-enable rc6 on ILK (again^5)
> > 
> >  drivers/gpu/drm/i915/i915_debugfs.c     |  6 ----
> >  drivers/gpu/drm/i915/i915_drv.h         |  3 +-
> >  drivers/gpu/drm/i915/i915_gem_context.c | 28 ++++++++++++++---
> >  drivers/gpu/drm/i915/i915_reg.h         |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c         | 56 ---------------------------------
> >  5 files changed, 25 insertions(+), 69 deletions(-)
> > 
> > -- 
> > 1.8.3.2
> > 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 9/9] drm/i915: Re-enable rc6 on ILK (again^5)

[Daniel Vetter]

On Sun, Jul 14, 2013 at 09:22:53AM -0700, Ben Widawsky wrote:
> With the conversion to use the existing, well tested HW context code for
> the ILK RC6 render context, let's once again try to enable RC6 by
> default on ILK.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

I think this commit should cite the last commit which reverted rc6 on ilk:

commit 6567d748c4e94e3481e523803ec07ebd825c80d6
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Nov 10 10:00:06 2012 +0000

    Revert "drm/i915: enable rc6 on ilk again"

I agree that we could try this again (at least since the relocation
regression in 3.7 is fixed), but iirc it was rather unmistakably broken
with vt-d enabled, see:

commit cd7988eea561a70a4f98e431c1395f913672d626
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Aug 26 20:33:18 2012 +0200

    drm/i915: disable rc6 on ilk when vt-d is enabled

So I think you need to pimp your commit a bit to essentially revert
6567d748c4e94e3481e523803 and also cite the above two commits.

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 03b90aa..c094aa3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3172,10 +3172,6 @@ int intel_enable_rc6(const struct drm_device *dev)
>  	if (i915_enable_rc6 >= 0)
>  		return i915_enable_rc6;
>  
> -	/* Disable RC6 on Ironlake */
> -	if (INTEL_INFO(dev)->gen == 5)
> -		return 0;
> -
>  	if (IS_HASWELL(dev)) {
>  		DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
>  		return INTEL_RC6_ENABLE;
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 9/9] [v2] drm/i915: enable rc6 on ILK again^5

[Ben Widawsky]

With the conversion to use the existing, well tested HW context code for
the ILK RC6 render context, let's once again try to enable RC6 by
default on ILK.

This is basically a revert of a revert and reapply of an existing patch.
RC6 has been enabled, and reverted several times on Ironlake. The most
recent:

commit 6567d748c4e94e3481e523803ec07ebd825c80d6
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Nov 10 10:00:06 2012 +0000
    Revert "drm/i915: enable rc6 on ilk again"

Also, as noted here:

commit cd7988eea561a70a4f98e431c1395f913672d626
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Aug 26 20:33:18 2012 +0200
    drm/i915: disable rc6 on ilk when vt-d is enabled

Do not try to turn on for VT-d case.

v2: Updated commit message (Daniel)
Disable with VT-d (Daniel)

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 03b90aa..4128824 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3172,9 +3172,11 @@ int intel_enable_rc6(const struct drm_device *dev)
 	if (i915_enable_rc6 >= 0)
 		return i915_enable_rc6;
 
-	/* Disable RC6 on Ironlake */
-	if (INTEL_INFO(dev)->gen == 5)
+#ifdef CONFIG_INTEL_IOMMU
+	/* Ironlake + RC6 + VT-d empirically blows up */
+	if (IS_GEN5(dev) && intel_iommu_gfx_mapped)
 		return 0;
+#endif
 
 	if (IS_HASWELL(dev)) {
 		DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
-- 
1.8.3.2

[PATCH 3/9] [v2] drm/i915: Make ILK context objects more like others

[Ben Widawsky]

This is required so we can use the existing do_switch() which tries to
map objects as non mappable and 64k aligned can work. Again, this is
only required in order to ease the transition over to the existing
context code.

This commit is left as distinct from the previous one because it also
effects the power context.

v2: Updated commit message.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 10f8f7cd..fd91b74 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2858,7 +2858,7 @@ intel_alloc_context_page(struct drm_device *dev)
 		return NULL;
 	}
 
-	ret = i915_gem_object_pin(ctx, 4096, true, false);
+	ret = i915_gem_object_pin(ctx, 64 << 10, false, false);
 	if (ret) {
 		DRM_ERROR("failed to pin power context: %d\n", ret);
 		goto err_unref;
-- 
1.8.3.2

Re: [PATCH 2/9] drm/i915: Convert renderctx to a regular context

[Chris Wilson]

On Sun, Jul 14, 2013 at 09:22:46AM -0700, Ben Widawsky wrote:
> @@ -3650,10 +3650,14 @@ static int ironlake_setup_rc6(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (dev_priv->ips.renderctx == NULL)
> -		dev_priv->ips.renderctx = intel_alloc_context_page(dev);
> -	if (!dev_priv->ips.renderctx)
> +	if (dev_priv->ips.renderctx.obj == NULL)
> +		dev_priv->ips.renderctx.obj = intel_alloc_context_page(dev);
> +	if (!dev_priv->ips.renderctx.obj)
>  		return -ENOMEM;
> +	else {
> +		dev_priv->ips.renderctx.id = DEFAULT_CONTEXT_ID;
> +		dev_priv->ips.renderctx.ring = &dev_priv->ring[RCS];
> +	}

Make this a bit more consistent style wise. You are mixing here !obj and
obj == NULL, along with a superfluous else{}.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 8/9] drm/i915: Restore ILK powerctx pin attributes

[Chris Wilson]

On Sun, Jul 14, 2013 at 09:22:52AM -0700, Ben Widawsky wrote:
> Now that I've killed renderctx, and the ILK pm code no longer has
> anything shared with the regular i915 context code, make the pin
> arguments the same as how they were before I started.
> 
> I do not know the reason for the original pin arguments, so it's totally
> possible this commit isn't necessary (and conversely that I temporarily
> broke things earlier in the patch series; though I saw no such result).
> However, since ILK RC6 worked very well for those of us where it worked,
> I think messing with any of the code is unjustified.

We should move it out of the aperture as there is no reason for this to
be GTT accessible. That requires an extra patch to request top-down
allocation.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/9] HW contest support for Ironlake

[Chris Wilson]

On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> By the request of Ken, and Chris, I've added support for HW contexts to
> Ironlake. This is not the first time I've done this, but I think I'll
> skip the somewhat ugly history on the matter.
> 
> Our existing support for Ironlake sets up 2 contexts, the powerctx, and
> the renderctx. The former is stored in special memory on GEN6+, and the
> latter is *extremely* similar to the way we use default context on GEN6+
> (ie. if users don't opt in to contexts, they are using it even if they
> don't realize).
> 
> The patch series first tries to tie that renderctx into our existing
> i915 context code, and then rips out the old renderctx. Powerctx is left
> alone since it still really is a special case.

It's a neat series, killing the special case for renderctx is a big
improvement.
 
> Finally, in the series I reenable rc6 on ILK. I've never had an issue
> with this on my ILK, but I recommend anyone testing on it who sees
> issues to try to disable it via modparam firstly.

Optimist.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/9] HW contest support for Ironlake

[Chris Wilson]

On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> Finally, in the series I reenable rc6 on ILK. I've never had an issue
> with this on my ILK, but I recommend anyone testing on it who sees
> issues to try to disable it via modparam firstly.
> 
> The patch series is minimally tested. I told Chris and Ken at the
> offset, they'd need to do the heavy lifting on the testing front.

My initial impression is that it seems to prevent IPS from kicking in
and speeding up the igfx, so peak performance is 2x slower than before
applying the series.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/9] HW contest support for Ironlake

[Ben Widawsky]

On Mon, Jul 15, 2013 at 01:44:13PM +0100, Chris Wilson wrote:
> On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> > Finally, in the series I reenable rc6 on ILK. I've never had an issue
> > with this on my ILK, but I recommend anyone testing on it who sees
> > issues to try to disable it via modparam firstly.
> > 
> > The patch series is minimally tested. I told Chris and Ken at the
> > offset, they'd need to do the heavy lifting on the testing front.
> 
> My initial impression is that it seems to prevent IPS from kicking in
> and speeding up the igfx, so peak performance is 2x slower than before
> applying the series.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

Are you sure this isn't just another attempt to get a module parameter
to shut it off :P

What's the benchmark - I'll take a look when I get back on my ILK.
Chris(f), Ken - maybe you guys can have a looksee?

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 8/9] drm/i915: Restore ILK powerctx pin attributes

[Ben Widawsky]

On Mon, Jul 15, 2013 at 11:30:15AM +0100, Chris Wilson wrote:
> On Sun, Jul 14, 2013 at 09:22:52AM -0700, Ben Widawsky wrote:
> > Now that I've killed renderctx, and the ILK pm code no longer has
> > anything shared with the regular i915 context code, make the pin
> > arguments the same as how they were before I started.
> > 
> > I do not know the reason for the original pin arguments, so it's totally
> > possible this commit isn't necessary (and conversely that I temporarily
> > broke things earlier in the patch series; though I saw no such result).
> > However, since ILK RC6 worked very well for those of us where it worked,
> > I think messing with any of the code is unjustified.
> 
> We should move it out of the aperture as there is no reason for this to
> be GTT accessible. That requires an extra patch to request top-down
> allocation.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

I am perfectly fine with dropping this patch, which I think solves the
problem. I see no reason to go back to a 4k aligned alloc, do you?

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 2/9] [v2] drm/i915: Convert renderctx to a regular context

[Ben Widawsky]

The medium term plan is to just use the existing context code on ILK as
well. The conversion just assists. Therefore, this is somewhat transient
as I plan to kill renderctx quite soon.

v2: Make NULL check style consistent (Chris)
Ditch superfluous 'else' (Chris)

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h         |  2 +-
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 drivers/gpu/drm/i915/intel_pm.c         | 17 ++++++++++-------
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8637979..5e88ef6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1347,9 +1347,9 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		seq_putc(m, '\n');
 	}
 
-	if (dev_priv->ips.renderctx) {
+	if (dev_priv->ips.renderctx.obj) {
 		seq_puts(m, "render context ");
-		describe_obj(m, dev_priv->ips.renderctx);
+		describe_obj(m, dev_priv->ips.renderctx.obj);
 		seq_putc(m, '\n');
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 03ab05c..64c6ae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,7 +789,7 @@ struct intel_ilk_power_mgmt {
 	int r_t;
 
 	struct drm_i915_gem_object *pwrctx;
-	struct drm_i915_gem_object *renderctx;
+	struct i915_hw_context renderctx;
 };
 
 /* Power well structure for haswell */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ab72c72..2c25006 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -589,7 +589,7 @@ int ironlake_setup_rc6_hw_context(struct drm_i915_private *dev_priv)
 
 	intel_ring_emit(ring, MI_SUSPEND_FLUSH | MI_SUSPEND_FLUSH_EN);
 	intel_ring_emit(ring, MI_SET_CONTEXT);
-	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx) |
+	intel_ring_emit(ring, i915_gem_obj_ggtt_offset(dev_priv->ips.renderctx.obj) |
 			MI_MM_SPACE_GTT |
 			MI_SAVE_EXT_STATE_EN |
 			MI_RESTORE_EXT_STATE_EN |
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2673602..39db531 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3615,10 +3615,10 @@ void ironlake_teardown_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx) {
-		i915_gem_object_unpin(dev_priv->ips.renderctx);
-		drm_gem_object_unreference(&dev_priv->ips.renderctx->base);
-		dev_priv->ips.renderctx = NULL;
+	if (dev_priv->ips.renderctx.obj) {
+		i915_gem_object_unpin(dev_priv->ips.renderctx.obj);
+		drm_gem_object_unreference(&dev_priv->ips.renderctx.obj->base);
+		dev_priv->ips.renderctx.obj = NULL;
 	}
 
 	if (dev_priv->ips.pwrctx) {
@@ -3650,9 +3650,12 @@ static int ironlake_setup_rc6(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (dev_priv->ips.renderctx == NULL)
-		dev_priv->ips.renderctx = intel_alloc_context_page(dev);
-	if (!dev_priv->ips.renderctx)
+	if (dev_priv->ips.renderctx.obj == NULL) {
+		dev_priv->ips.renderctx.id = DEFAULT_CONTEXT_ID;
+		dev_priv->ips.renderctx.ring = &dev_priv->ring[RCS];
+		dev_priv->ips.renderctx.obj = intel_alloc_context_page(dev);
+	}
+	if (dev_priv->ips.renderctx.obj == NULL)
 		return -ENOMEM;
 
 	if (dev_priv->ips.pwrctx == NULL)
-- 
1.8.3.2

Re: [PATCH 0/9] HW contest support for Ironlake

[Chris Wilson]

On Mon, Jul 15, 2013 at 10:24:19AM -0700, Ben Widawsky wrote:
> On Mon, Jul 15, 2013 at 01:44:13PM +0100, Chris Wilson wrote:
> > On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> > > Finally, in the series I reenable rc6 on ILK. I've never had an issue
> > > with this on my ILK, but I recommend anyone testing on it who sees
> > > issues to try to disable it via modparam firstly.
> > > 
> > > The patch series is minimally tested. I told Chris and Ken at the
> > > offset, they'd need to do the heavy lifting on the testing front.
> > 
> > My initial impression is that it seems to prevent IPS from kicking in
> > and speeding up the igfx, so peak performance is 2x slower than before
> > applying the series.
> Are you sure this isn't just another attempt to get a module parameter
> to shut it off :P

A static renderctx I can bear. :)

I do wonder if my machine is simply overheating at the moment, though.
It might just a quirk of environmental conditions rather than something
more nefarious.
 
> What's the benchmark - I'll take a look when I get back on my ILK.
> Chris(f), Ken - maybe you guys can have a looksee?

I first noticed the problem when fishietank never speeded up, but it
affects everything from scrolling in firefox to aa10text.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 8/9] drm/i915: Restore ILK powerctx pin attributes

[Chris Wilson]

On Mon, Jul 15, 2013 at 10:58:57AM -0700, Ben Widawsky wrote:
> I am perfectly fine with dropping this patch, which I think solves the
> problem. I see no reason to go back to a 4k aligned alloc, do you?

It still introduces a fair amount of fragmentation - but you can argue
that we should be allocating enough single pages for it not to matter.
At some point, I am going to notice the hole and wonder what insanity
lies within. ;-)

However, just dropping the patch isn't quite enough, we need the
'prefer top-down allocations for !mappable' as well to move it out of
the aperture. But that is a patch for another time.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/9] HW contest support for Ironlake

[Ben Widawsky]

On Mon, Jul 15, 2013 at 01:44:13PM +0100, Chris Wilson wrote:
> On Sun, Jul 14, 2013 at 09:22:44AM -0700, Ben Widawsky wrote:
> > Finally, in the series I reenable rc6 on ILK. I've never had an issue
> > with this on my ILK, but I recommend anyone testing on it who sees
> > issues to try to disable it via modparam firstly.
> > 
> > The patch series is minimally tested. I told Chris and Ken at the
> > offset, they'd need to do the heavy lifting on the testing front.
> 
> My initial impression is that it seems to prevent IPS from kicking in
> and speeding up the igfx, so peak performance is 2x slower than before
> applying the series.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

As noted today on IRC... I'm not really able to reproduce this result. I
see /something/ going on, but can't say conclusively it's so bad. Also
as discussed, please try without RC6, and if that doesn't help, please
try to bisect since I can't really see any other patch changing much.

-- 
Ben Widawsky, Intel Open Source Technology Center