HW contest support for Ironlake

PATCH In review Ben Widawsky 3 participants 26 messages Jul 14, 2013 – Jul 16, 2013

Subsystems (2)

DRM drivers Intel i915

lore

Share & tools

lore

Ben Widawsky

author

2013-07-14 16:19 UTC #0 lore raw

cover HW contest support for Ironlake

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(-)

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #1 lore raw

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

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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +1 -0

index cef35d3..03ab05c 100644
@@ -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);

drivers/gpu/drm/i915/i915_gem_context.c +24 -0

index 2074544..ab72c72 100644
@@ -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;
+}

drivers/gpu/drm/i915/intel_pm.c +1 ~~-17~~

index fb4afaa..2673602 100644
@@ -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

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_debugfs.c +2 -2

index 8637979..5e88ef6 100644
@@ -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');
 	}

drivers/gpu/drm/i915/i915_drv.h +1 -1

index 03ab05c..64c6ae0 100644
@@ -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 */

drivers/gpu/drm/i915/i915_gem_context.c +1 -1

index ab72c72..2c25006 100644
@@ -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 |

drivers/gpu/drm/i915/intel_pm.c +11 -7

index 2673602..10f8f7cd 100644
@@ -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);

Signature · 1 line

1.8.3.2

Chris Wilson

2013-07-15 10:24 UTC #3 lore raw

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

Quoted 5 lines · Ben Widawsky

drivers/gpu/drm/i915/intel_pm.c +0 -1

@@ -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)

drivers/gpu/drm/i915/intel_pm.c static int ironlake_setup_rc6(struct drm_device *dev) · line 3660

Quoted 10 lines

drivers/gpu/drm/i915/intel_pm.c +7 -2

-		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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Ben Widawsky

author

2013-07-15 18:01 UTC #4 lore raw

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

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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_debugfs.c +2 -2

index 8637979..5e88ef6 100644
@@ -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');
 	}

drivers/gpu/drm/i915/i915_drv.h +1 -1

index 03ab05c..64c6ae0 100644
@@ -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 */

drivers/gpu/drm/i915/i915_gem_context.c +1 -1

index ab72c72..2c25006 100644
@@ -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 |

drivers/gpu/drm/i915/intel_pm.c +10 -7

index 2673602..39db531 100644
@@ -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)

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #5 lore raw

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

In reply to 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 <[email protected]>

fixup! drm/i915: Convert renderctx to a regular context

---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_pm.c +1 -1

index 10f8f7cd..fd91b74 100644
@@ -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;

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 19:07 UTC #6 lore raw

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

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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_pm.c +1 -1

index 10f8f7cd..fd91b74 100644
@@ -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;

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #7 lore raw

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

In reply to 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 <[email protected]>

Signed-off-by Ben Widawsky <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_context.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

drivers/gpu/drm/i915/i915_gem_context.c +20 -5

index 2c25006..a9beaaa 100644
@@ -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)

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #8 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/i915/i915_gem_context.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

drivers/gpu/drm/i915/i915_gem_context.c +1 ~~-20~~

index a9beaaa..f187e13 100644
@@ -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);
 }

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_drv.h +1 -1

index 64c6ae0..499caac 100644
@@ -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)

drivers/gpu/drm/i915/i915_gem_context.c +3 -0

index f187e13..5d10b37 100644
@@ -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;

drivers/gpu/drm/i915/i915_reg.h +1 -0

index dc3d6a7..9cb8b2b 100644
@@ -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)

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #10 lore raw

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

In reply to 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 <[email protected]>

---
 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(-)

drivers/gpu/drm/i915/i915_debugfs.c +0 -6

index 5e88ef6..4812493 100644
@@ -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);

drivers/gpu/drm/i915/i915_drv.h +0 -2

index 499caac..23f14d7 100644
@@ -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);

drivers/gpu/drm/i915/i915_gem_context.c +0 -5

index 5d10b37..6556f3d 100644
@@ -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);
-}

drivers/gpu/drm/i915/intel_pm.c +0 ~~-40~~

index fd91b74..21e61fc 100644
@@ -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);
 }

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #11 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/i915/intel_pm.c +1 -1

index 21e61fc..03b90aa 100644
@@ -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;

Signature · 1 line

1.8.3.2

Chris Wilson

2013-07-15 10:30 UTC #12 lore raw

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

Quoted 9 lines · 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.

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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Ben Widawsky

author

2013-07-15 17:59 UTC #13 lore raw

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

Quoted 11 lines · Chris Wilson

Quoted 9 lines · 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.

Quoted 7 lines

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
Signature · 1 line
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?

Signature · 1 line

Ben Widawsky, Intel Open Source Technology Center

Chris Wilson

2013-07-15 22:12 UTC #14 lore raw

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

Quoted 2 lines · Ben Widawsky

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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Ben Widawsky

author

2013-07-14 16:19 UTC #15 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +0 -4

index 03b90aa..c094aa3 100644
@@ -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;

Signature · 1 line

1.8.3.2

Daniel Vetter

2013-07-14 18:35 UTC #16 lore raw

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

Quoted 5 lines · 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 <[email protected]>

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

commit 6567d748c4e94e3481e523803ec07ebd825c80d6

Author: Chris Wilson <[email protected]>
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 <[email protected]>
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

Quoted 26 lines

---
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +0 -4

index 03b90aa..c094aa3 100644
@@ -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;

Signature · 6 lines

1.8.3.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 3 lines

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

Ben Widawsky

author

2013-07-14 19:06 UTC #17 lore raw

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

In reply to 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/gpu/drm/i915/intel_pm.c +4 -2

index 03b90aa..4128824 100644
@@ -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");

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:19 UTC #18 lore raw

PATCH 1/1 i965: Gen5: Use HW contexts on Ironlake

In reply to 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 <[email protected]>

---
 src/mesa/drivers/dri/i965/brw_context.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

src/mesa/drivers/dri/i965/brw_context.c +5 -2

index a72e226..4a0a6ad 100644
@@ -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;
       }

Signature · 1 line

1.8.3.2

Ben Widawsky

author

2013-07-14 16:20 UTC #19 lore raw

In reply to 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

Quoted 42 lines · 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(-)
Signature · 1 line
1.8.3.2

Signature · 1 line

Ben Widawsky, Intel Open Source Technology Center

Ben Widawsky

author

2013-07-14 17:35 UTC #20 lore raw

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

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

Quoted 56 lines · 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
Quoted 42 lines · 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(-)
Signature · 1 line
1.8.3.2
Signature · 5 lines
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 1 line

Ben Widawsky, Intel Open Source Technology Center

Chris Wilson

2013-07-15 10:32 UTC #21 lore raw

In reply to Ben Widawsky

Quoted 10 lines · 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).

Quoted 3 lines

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.

Quoted 3 lines

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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Chris Wilson

2013-07-15 12:44 UTC #22 lore raw

In reply to Ben Widawsky

Quoted 6 lines · Ben Widawsky

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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Ben Widawsky

author

2013-07-15 17:24 UTC #23 lore raw

Quoted 5 lines · Chris Wilson

Quoted 4 lines · Ben Widawsky
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.

Quoted 10 lines

Quoted 2 lines
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
Signature · 1 line
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?

Signature · 1 line

Ben Widawsky, Intel Open Source Technology Center

Chris Wilson

2013-07-15 18:52 UTC #24 lore raw

Quoted 6 lines · Ben Widawsky

Quoted 5 lines · Chris Wilson
Quoted 4 lines · Ben Widawsky
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.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
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.

Quoted 2 lines

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

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Ben Widawsky

author

2013-07-16 18:21 UTC #25 lore raw

In reply to Chris Wilson

Quoted 5 lines · Chris Wilson

Quoted 4 lines · Ben Widawsky
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.

Quoted 10 lines

Quoted 2 lines
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
Signature · 1 line
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.

Signature · 1 line

Ben Widawsky, Intel Open Source Technology Center