[PATCH 0/4] drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff

[PATCH 0/4] drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I got annoyed at the pre-g4x gpu reset regression again, so I
picked up the latest (I think it was the latest anyway) version of
Maarten's locking fix from the list and rebased it. I also changed
one comment in there about mode_config.mutex as the original didn't
make sense to me.

I tossed in a few polishing patches on top, and tested the whole lot
on my 946gz, and I also ran it on hsw w/ and w/o the new modparam set.
No problems whatsoever.

Maarten Lankhorst (2):
  drm/i915: Fix modeset handling during gpu reset, v5.
  drm/i915: Add a way to test the modeset done during gpu reset, v3.

Ville Syrjälä (2):
  drm/i915: Introduce gpu_reset_clobbers_display()
  drm/i915: Use the g4x+ approach on gen2 for handling display stuff
    around GPU reset

 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_params.c   |   6 +
 drivers/gpu/drm/i915/i915_params.h   |   1 +
 drivers/gpu/drm/i915/intel_display.c | 205 ++++++++++++++++++++++-------------
 4 files changed, 138 insertions(+), 75 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[ville.syrjala]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This function would call drm_modeset_lock_all, while the suspend/resume
functions already have their own locking. Fix this by factoring out
__intel_display_resume, and calling the atomic helpers for duplicating
atomic state and disabling all crtc's during suspend.

Changes since v1:
- Deal with -EDEADLK right after lock_all and clean up calls
  to hw readout.
- Always take all modeset locks so updates during gpu reset are blocked.
Changes since v2:
- Fix deadlock in intel_update_primary_planes.
- Move WARN_ON(EDEADLK) to __intel_display_resume.
- pctx -> ctx
- only call __intel_display_resume on success in intel_display_resume.
Changes since v3:
- Rebase on top of dev_priv -> dev change.
- Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
Changes since v4 [by vsyrjala]:
- Deal with skip_intermediate_wm
- Update comment w.r.t. mode_config.mutex vs. ->detect()
- Rebase due to INTEL_GEN() etc.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
Cc: stable@vger.kernel.org
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/intel_display.c | 170 ++++++++++++++++++++++-------------
 2 files changed, 111 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index feec00f769e1..94a333c44bee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1840,6 +1840,7 @@ struct drm_i915_private {
 	enum modeset_restore modeset_restore;
 	struct mutex modeset_restore_lock;
 	struct drm_atomic_state *modeset_restore_state;
+	struct drm_modeset_acquire_ctx reset_ctx;
 
 	struct list_head vm_list; /* Global list of all address spaces */
 	struct i915_ggtt ggtt; /* VM representing the global address space */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9cbf5431c1e3..1e2fb444eb93 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3093,40 +3093,110 @@ static void intel_update_primary_planes(struct drm_device *dev)
 
 	for_each_crtc(dev, crtc) {
 		struct intel_plane *plane = to_intel_plane(crtc->primary);
-		struct intel_plane_state *plane_state;
-
-		drm_modeset_lock_crtc(crtc, &plane->base);
-		plane_state = to_intel_plane_state(plane->base.state);
+		struct intel_plane_state *plane_state =
+			to_intel_plane_state(plane->base.state);
 
 		if (plane_state->visible)
 			plane->update_plane(&plane->base,
 					    to_intel_crtc_state(crtc->state),
 					    plane_state);
+	}
+}
+
+static int
+__intel_display_resume(struct drm_device *dev,
+		       struct drm_atomic_state *state)
+{
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc *crtc;
+	int i, ret;
 
-		drm_modeset_unlock_crtc(crtc);
+	intel_modeset_setup_hw_state(dev);
+	i915_redisable_vga(dev);
+
+	if (!state)
+		return 0;
+
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		/*
+		 * Force recalculation even if we restore
+		 * current state. With fast modeset this may not result
+		 * in a modeset when the state is compatible.
+		 */
+		crtc_state->mode_changed = true;
 	}
+
+	/* ignore any reset values/BIOS leftovers in the WM registers */
+	to_intel_atomic_state(state)->skip_intermediate_wm = true;
+
+	ret = drm_atomic_commit(state);
+
+	WARN_ON(ret == -EDEADLK);
+	return ret;
 }
 
 void intel_prepare_reset(struct drm_i915_private *dev_priv)
 {
+	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx *ctx = &dev_priv->reset_ctx;
+	struct drm_atomic_state *state;
+	int ret;
+
 	/* no reset support for gen2 */
 	if (IS_GEN2(dev_priv))
 		return;
 
-	/* reset doesn't touch the display */
+	/*
+	 * Need mode_config.mutex so that we don't
+	 * trample ongoing ->detect() and whatnot.
+	 */
+	mutex_lock(&dev->mode_config.mutex);
+	drm_modeset_acquire_init(ctx, 0);
+	while (1) {
+		ret = drm_modeset_lock_all_ctx(dev, ctx);
+		if (ret != -EDEADLK)
+			break;
+
+		drm_modeset_backoff(ctx);
+	}
+
+	/* reset doesn't touch the display, but flips might get nuked anyway, */
 	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
 		return;
 
-	drm_modeset_lock_all(&dev_priv->drm);
 	/*
 	 * Disabling the crtcs gracefully seems nicer. Also the
 	 * g33 docs say we should at least disable all the planes.
 	 */
-	intel_display_suspend(&dev_priv->drm);
+	state = drm_atomic_helper_duplicate_state(dev, ctx);
+	if (IS_ERR(state)) {
+		ret = PTR_ERR(state);
+		state = NULL;
+		DRM_ERROR("Duplicating state failed with %i\n", ret);
+		goto err;
+	}
+
+	ret = drm_atomic_helper_disable_all(dev, ctx);
+	if (ret) {
+		DRM_ERROR("Suspending crtc's failed with %i\n", ret);
+		goto err;
+	}
+
+	dev_priv->modeset_restore_state = state;
+	state->acquire_ctx = ctx;
+	return;
+
+err:
+	drm_atomic_state_free(state);
 }
 
 void intel_finish_reset(struct drm_i915_private *dev_priv)
 {
+	struct drm_device *dev = &dev_priv->drm;
+	struct drm_modeset_acquire_ctx *ctx = &dev_priv->reset_ctx;
+	struct drm_atomic_state *state = dev_priv->modeset_restore_state;
+	int ret;
+
 	/*
 	 * Flips in the rings will be nuked by the reset,
 	 * so complete all pending flips so that user space
@@ -3138,6 +3208,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 	if (IS_GEN2(dev_priv))
 		return;
 
+	dev_priv->modeset_restore_state = NULL;
+
 	/* reset doesn't touch the display */
 	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
 		/*
@@ -3149,29 +3221,32 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 		 * FIXME: Atomic will make this obsolete since we won't schedule
 		 * CS-based flips (which might get lost in gpu resets) any more.
 		 */
-		intel_update_primary_planes(&dev_priv->drm);
-		return;
-	}
-
-	/*
-	 * The display has been reset as well,
-	 * so need a full re-initialization.
-	 */
-	intel_runtime_pm_disable_interrupts(dev_priv);
-	intel_runtime_pm_enable_interrupts(dev_priv);
+		intel_update_primary_planes(dev);
+	} else {
+		/*
+		 * The display has been reset as well,
+		 * so need a full re-initialization.
+		 */
+		intel_runtime_pm_disable_interrupts(dev_priv);
+		intel_runtime_pm_enable_interrupts(dev_priv);
 
-	intel_modeset_init_hw(&dev_priv->drm);
+		intel_modeset_init_hw(dev);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.hpd_irq_setup)
-		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
+		spin_lock_irq(&dev_priv->irq_lock);
+		if (dev_priv->display.hpd_irq_setup)
+			dev_priv->display.hpd_irq_setup(dev_priv);
+		spin_unlock_irq(&dev_priv->irq_lock);
 
-	intel_display_resume(&dev_priv->drm);
+		ret = __intel_display_resume(dev, state);
+		if (ret)
+			DRM_ERROR("Restoring old state failed with %i\n", ret);
 
-	intel_hpd_init(dev_priv);
+		intel_hpd_init(dev_priv);
+	}
 
-	drm_modeset_unlock_all(&dev_priv->drm);
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+	mutex_unlock(&dev->mode_config.mutex);
 }
 
 static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
@@ -16163,9 +16238,10 @@ void intel_display_resume(struct drm_device *dev)
 	struct drm_atomic_state *state = dev_priv->modeset_restore_state;
 	struct drm_modeset_acquire_ctx ctx;
 	int ret;
-	bool setup = false;
 
 	dev_priv->modeset_restore_state = NULL;
+	if (state)
+		state->acquire_ctx = &ctx;
 
 	/*
 	 * This is a cludge because with real atomic modeset mode_config.mutex
@@ -16176,43 +16252,17 @@ void intel_display_resume(struct drm_device *dev)
 	mutex_lock(&dev->mode_config.mutex);
 	drm_modeset_acquire_init(&ctx, 0);
 
-retry:
-	ret = drm_modeset_lock_all_ctx(dev, &ctx);
-
-	if (ret == 0 && !setup) {
-		setup = true;
-
-		intel_modeset_setup_hw_state(dev);
-		i915_redisable_vga(dev);
-	}
-
-	if (ret == 0 && state) {
-		struct drm_crtc_state *crtc_state;
-		struct drm_crtc *crtc;
-		int i;
-
-		state->acquire_ctx = &ctx;
-
-		/* ignore any reset values/BIOS leftovers in the WM registers */
-		to_intel_atomic_state(state)->skip_intermediate_wm = true;
-
-		for_each_crtc_in_state(state, crtc, crtc_state, i) {
-			/*
-			 * Force recalculation even if we restore
-			 * current state. With fast modeset this may not result
-			 * in a modeset when the state is compatible.
-			 */
-			crtc_state->mode_changed = true;
-		}
-
-		ret = drm_atomic_commit(state);
-	}
+	while (1) {
+		ret = drm_modeset_lock_all_ctx(dev, &ctx);
+		if (ret != -EDEADLK)
+			break;
 
-	if (ret == -EDEADLK) {
 		drm_modeset_backoff(&ctx);
-		goto retry;
 	}
 
+	if (!ret)
+		ret = __intel_display_resume(dev, state);
+
 	drm_modeset_drop_locks(&ctx);
 	drm_modeset_acquire_fini(&ctx);
 	mutex_unlock(&dev->mode_config.mutex);
-- 
2.7.4

[PATCH v2 2/4] drm/i915: Add a way to test the modeset done during gpu reset, v3.

[ville.syrjala]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Add force_reset_modeset_test as a parameter to force the modeset path during gpu reset.
This allows a IGT test to set the knob and trigger a hang to force the gpu reset,
even on platforms that wouldn't otherwise require it.

Changes since v1:
- Split out fix to separate commit.
Changes since v2:
- This commit is purely about force_reset_modeset_test now.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: drv_hangman.reset-with-forced-modeset
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.c   |  6 ++++++
 drivers/gpu/drm/i915/i915_params.h   |  1 +
 drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6e404c91eed..768ad89d9cd4 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -45,6 +45,7 @@ struct i915_params i915 __read_mostly = {
 	.fastboot = 0,
 	.prefault_disable = 0,
 	.load_detect_test = 0,
+	.force_reset_modeset_test = 0,
 	.reset = true,
 	.invert_brightness = 0,
 	.disable_display = 0,
@@ -161,6 +162,11 @@ MODULE_PARM_DESC(load_detect_test,
 	"Force-enable the VGA load detect code for testing (default:false). "
 	"For developers only.");
 
+module_param_named_unsafe(force_reset_modeset_test, i915.force_reset_modeset_test, bool, 0600);
+MODULE_PARM_DESC(force_reset_modeset_test,
+	"Force a modeset during gpu reset for testing (default:false). "
+	"For developers only.");
+
 module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, 0600);
 MODULE_PARM_DESC(invert_brightness,
 	"Invert backlight brightness "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 0ad020b4a925..3a0dd78ddb38 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -57,6 +57,7 @@ struct i915_params {
 	bool fastboot;
 	bool prefault_disable;
 	bool load_detect_test;
+	bool force_reset_modeset_test;
 	bool reset;
 	bool disable_display;
 	bool verbose_state_checks;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1e2fb444eb93..6bd60a9ece06 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3161,7 +3161,8 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
 	}
 
 	/* reset doesn't touch the display, but flips might get nuked anyway, */
-	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv))
+	if (!i915.force_reset_modeset_test &&
+	    (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)))
 		return;
 
 	/*
@@ -3212,16 +3213,22 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 
 	/* reset doesn't touch the display */
 	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
-		/*
-		 * Flips in the rings have been nuked by the reset,
-		 * so update the base address of all primary
-		 * planes to the the last fb to make sure we're
-		 * showing the correct fb after a reset.
-		 *
-		 * FIXME: Atomic will make this obsolete since we won't schedule
-		 * CS-based flips (which might get lost in gpu resets) any more.
-		 */
-		intel_update_primary_planes(dev);
+		if (!state) {
+			/*
+			 * Flips in the rings have been nuked by the reset,
+			 * so update the base address of all primary
+			 * planes to the the last fb to make sure we're
+			 * showing the correct fb after a reset.
+			 *
+			 * FIXME: Atomic will make this obsolete since we won't schedule
+			 * CS-based flips (which might get lost in gpu resets) any more.
+			 */
+			intel_update_primary_planes(dev);
+		} else {
+			ret = __intel_display_resume(dev, state);
+			if (ret)
+				DRM_ERROR("Restoring old state failed with %i\n", ret);
+		}
 	} else {
 		/*
 		 * The display has been reset as well,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: Introduce gpu_reset_clobbers_display()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Factor out the "does the GPU reset clobber the display?" check into a
small helper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6bd60a9ece06..632739f9cc44 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3135,6 +3135,11 @@ __intel_display_resume(struct drm_device *dev,
 	return ret;
 }
 
+static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
+{
+	return INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
+}
+
 void intel_prepare_reset(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
@@ -3162,7 +3167,7 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
 
 	/* reset doesn't touch the display, but flips might get nuked anyway, */
 	if (!i915.force_reset_modeset_test &&
-	    (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)))
+	    !gpu_reset_clobbers_display(dev_priv))
 		return;
 
 	/*
@@ -3212,7 +3217,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 	dev_priv->modeset_restore_state = NULL;
 
 	/* reset doesn't touch the display */
-	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
+	if (!gpu_reset_clobbers_display(dev_priv)) {
 		if (!state) {
 			/*
 			 * Flips in the rings have been nuked by the reset,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Use the g4x+ approach on gen2 for handling display stuff around GPU reset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't have GPU reset support for gen2, which means the display
hardware is unaffected when a GPU hang is handled. However as the ring
has in fact stopped, any flips still in the ring will never complete,
and thus the display base address updates will never happen. So we
really need to fix that up manually just like we do on g4x+.

In fact, let's just use intel_has_gpu_reset() instead of IS_GEN2()
since that'll also handle cases where someone would disable the GPU
reset support on gen3/4 for whatever reason.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 632739f9cc44..74375ec651e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3137,7 +3137,8 @@ __intel_display_resume(struct drm_device *dev,
 
 static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
 {
-	return INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
+	return intel_has_gpu_reset(dev_priv) &&
+		INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
 }
 
 void intel_prepare_reset(struct drm_i915_private *dev_priv)
@@ -3147,10 +3148,6 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
 	struct drm_atomic_state *state;
 	int ret;
 
-	/* no reset support for gen2 */
-	if (IS_GEN2(dev_priv))
-		return;
-
 	/*
 	 * Need mode_config.mutex so that we don't
 	 * trample ongoing ->detect() and whatnot.
@@ -3210,10 +3207,6 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 	 */
 	intel_complete_page_flips(dev_priv);
 
-	/* no reset support for gen2 */
-	if (IS_GEN2(dev_priv))
-		return;
-
 	dev_priv->modeset_restore_state = NULL;
 
 	/* reset doesn't touch the display */
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Introduce gpu_reset_clobbers_display()

[Daniel Vetter]

On Fri, Aug 05, 2016 at 11:28:29PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Factor out the "does the GPU reset clobber the display?" check into a
> small helper.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6bd60a9ece06..632739f9cc44 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3135,6 +3135,11 @@ __intel_display_resume(struct drm_device *dev,
>  	return ret;
>  }
>  
> +static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
> +{
> +	return INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
> +}
> +
>  void intel_prepare_reset(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = &dev_priv->drm;
> @@ -3162,7 +3167,7 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
>  
>  	/* reset doesn't touch the display, but flips might get nuked anyway, */
>  	if (!i915.force_reset_modeset_test &&
> -	    (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)))
> +	    !gpu_reset_clobbers_display(dev_priv))
>  		return;
>  
>  	/*
> @@ -3212,7 +3217,7 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  	dev_priv->modeset_restore_state = NULL;
>  
>  	/* reset doesn't touch the display */
> -	if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
> +	if (!gpu_reset_clobbers_display(dev_priv)) {
>  		if (!state) {
>  			/*
>  			 * Flips in the rings have been nuked by the reset,
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Use the g4x+ approach on gen2 for handling display stuff around GPU reset

[Daniel Vetter]

On Fri, Aug 05, 2016 at 11:28:30PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We don't have GPU reset support for gen2, which means the display
> hardware is unaffected when a GPU hang is handled. However as the ring
> has in fact stopped, any flips still in the ring will never complete,
> and thus the display base address updates will never happen. So we
> really need to fix that up manually just like we do on g4x+.
> 
> In fact, let's just use intel_has_gpu_reset() instead of IS_GEN2()
> since that'll also handle cases where someone would disable the GPU
> reset support on gen3/4 for whatever reason.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 632739f9cc44..74375ec651e5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3137,7 +3137,8 @@ __intel_display_resume(struct drm_device *dev,
>  
>  static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
>  {
> -	return INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
> +	return intel_has_gpu_reset(dev_priv) &&
> +		INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
>  }
>  
>  void intel_prepare_reset(struct drm_i915_private *dev_priv)
> @@ -3147,10 +3148,6 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
>  	struct drm_atomic_state *state;
>  	int ret;
>  
> -	/* no reset support for gen2 */
> -	if (IS_GEN2(dev_priv))
> -		return;
> -
>  	/*
>  	 * Need mode_config.mutex so that we don't
>  	 * trample ongoing ->detect() and whatnot.
> @@ -3210,10 +3207,6 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  	 */
>  	intel_complete_page_flips(dev_priv);
>  
> -	/* no reset support for gen2 */
> -	if (IS_GEN2(dev_priv))
> -		return;
> -
>  	dev_priv->modeset_restore_state = NULL;
>  
>  	/* reset doesn't touch the display */
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff

[Patchwork]

== Series Details ==

Series: drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff
URL   : https://patchwork.freedesktop.org/series/10731/
State : failure

== Summary ==

Series 10731v1 drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff
http://patchwork.freedesktop.org/api/1.0/series/10731/revisions/1/mbox

Test kms_cursor_legacy:
        Subgroup basic-cursor-vs-flip-varying-size:
                pass       -> FAIL       (ro-ilk1-i5-650)
        Subgroup basic-flip-vs-cursor-varying-size:
                pass       -> FAIL       (ro-snb-i7-2620M)
                fail       -> PASS       (ro-bdw-i5-5250u)

fi-kbl-qkkr      total:244  pass:185  dwarn:29  dfail:0   fail:3   skip:27 
ro-bdw-i5-5250u  total:240  pass:219  dwarn:4   dfail:0   fail:1   skip:16 
ro-bdw-i7-5557U  total:240  pass:224  dwarn:0   dfail:0   fail:0   skip:16 
ro-bdw-i7-5600u  total:240  pass:207  dwarn:0   dfail:0   fail:1   skip:32 
ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-hsw-i7-4770r  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
ro-ilk-i7-620lm  total:240  pass:173  dwarn:1   dfail:0   fail:1   skip:65 
ro-ilk1-i5-650   total:235  pass:173  dwarn:0   dfail:0   fail:2   skip:60 
ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35 
ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31 
ro-skl3-i5-6260u total:240  pass:223  dwarn:0   dfail:0   fail:3   skip:14 
ro-snb-i7-2620M  total:240  pass:197  dwarn:0   dfail:0   fail:2   skip:41 
ro-byt-n2820 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1741/

b834992 drm-intel-nightly: 2016y-08m-05d-20h-40m-44s UTC integration manifest
ed2ac6e drm/i915: Use the g4x+ approach on gen2 for handling display stuff around GPU reset
d29861b drm/i915: Introduce gpu_reset_clobbers_display()
893b403 drm/i915: Add a way to test the modeset done during gpu reset, v3.
617092d drm/i915: Fix modeset handling during gpu reset, v5.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Maarten Lankhorst]

Hey,

Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> This function would call drm_modeset_lock_all, while the suspend/resume
> functions already have their own locking. Fix this by factoring out
> __intel_display_resume, and calling the atomic helpers for duplicating
> atomic state and disabling all crtc's during suspend.
>
> Changes since v1:
> - Deal with -EDEADLK right after lock_all and clean up calls
>   to hw readout.
> - Always take all modeset locks so updates during gpu reset are blocked.
> Changes since v2:
> - Fix deadlock in intel_update_primary_planes.
> - Move WARN_ON(EDEADLK) to __intel_display_resume.
> - pctx -> ctx
> - only call __intel_display_resume on success in intel_display_resume.
> Changes since v3:
> - Rebase on top of dev_priv -> dev change.
> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> Changes since v4 [by vsyrjala]:
> - Deal with skip_intermediate_wm
> - Update comment w.r.t. mode_config.mutex vs. ->detect()
> - Rebase due to INTEL_GEN() etc.

Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
until all planes are enabled.

I fear this may blow up in interesting ways. And it should probably be using
dev_priv->wm.distrust_bios_wm instead like on SKL.

~Maarten

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Maarten Lankhorst]

Hey,

Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>
> This function would call drm_modeset_lock_all, while the suspend/resume
> functions already have their own locking. Fix this by factoring out
> __intel_display_resume, and calling the atomic helpers for duplicating
> atomic state and disabling all crtc's during suspend.
>
> Changes since v1:
> - Deal with -EDEADLK right after lock_all and clean up calls
>   to hw readout.
> - Always take all modeset locks so updates during gpu reset are blocked.
> Changes since v2:
> - Fix deadlock in intel_update_primary_planes.
> - Move WARN_ON(EDEADLK) to __intel_display_resume.
> - pctx -> ctx
> - only call __intel_display_resume on success in intel_display_resume.
> Changes since v3:
> - Rebase on top of dev_priv -> dev change.
> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> Changes since v4 [by vsyrjala]:
> - Deal with skip_intermediate_wm
> - Update comment w.r.t. mode_config.mutex vs. ->detect()
> - Rebase due to INTEL_GEN() etc.

Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
until all planes are enabled.

I fear this may blow up in interesting ways. And it should probably be using
dev_priv->wm.distrust_bios_wm instead like on SKL.

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Ville Syrjälä]

On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > This function would call drm_modeset_lock_all, while the suspend/resume
> > functions already have their own locking. Fix this by factoring out
> > __intel_display_resume, and calling the atomic helpers for duplicating
> > atomic state and disabling all crtc's during suspend.
> >
> > Changes since v1:
> > - Deal with -EDEADLK right after lock_all and clean up calls
> >   to hw readout.
> > - Always take all modeset locks so updates during gpu reset are blocked.
> > Changes since v2:
> > - Fix deadlock in intel_update_primary_planes.
> > - Move WARN_ON(EDEADLK) to __intel_display_resume.
> > - pctx -> ctx
> > - only call __intel_display_resume on success in intel_display_resume.
> > Changes since v3:
> > - Rebase on top of dev_priv -> dev change.
> > - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> > Changes since v4 [by vsyrjala]:
> > - Deal with skip_intermediate_wm
> > - Update comment w.r.t. mode_config.mutex vs. ->detect()
> > - Rebase due to INTEL_GEN() etc.
> 
> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
> until all planes are enabled.

What blows up and how?

Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
this time anyway, so -ENOCARE at this point really.

> 
> I fear this may blow up in interesting ways. And it should probably be using
> dev_priv->wm.distrust_bios_wm instead like on SKL.

Sigh. How many ways do we need to do the same thing?

Anywyas, what we should really do is sanitize the current wms better
at readout time, and then we shouldn't need these flags at all.

-- 
Ville Syrj�l�
Intel OTC

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Ville Syrjälä]

On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >
> > This function would call drm_modeset_lock_all, while the suspend/resume
> > functions already have their own locking. Fix this by factoring out
> > __intel_display_resume, and calling the atomic helpers for duplicating
> > atomic state and disabling all crtc's during suspend.
> >
> > Changes since v1:
> > - Deal with -EDEADLK right after lock_all and clean up calls
> >   to hw readout.
> > - Always take all modeset locks so updates during gpu reset are blocked.
> > Changes since v2:
> > - Fix deadlock in intel_update_primary_planes.
> > - Move WARN_ON(EDEADLK) to __intel_display_resume.
> > - pctx -> ctx
> > - only call __intel_display_resume on success in intel_display_resume.
> > Changes since v3:
> > - Rebase on top of dev_priv -> dev change.
> > - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> > Changes since v4 [by vsyrjala]:
> > - Deal with skip_intermediate_wm
> > - Update comment w.r.t. mode_config.mutex vs. ->detect()
> > - Rebase due to INTEL_GEN() etc.
> 
> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
> until all planes are enabled.

What blows up and how?

Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
this time anyway, so -ENOCARE at this point really.

> 
> I fear this may blow up in interesting ways. And it should probably be using
> dev_priv->wm.distrust_bios_wm instead like on SKL.

Sigh. How many ways do we need to do the same thing?

Anywyas, what we should really do is sanitize the current wms better
at readout time, and then we shouldn't need these flags at all.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Maarten Lankhorst]

Op 08-08-16 om 10:05 schreef Ville Syrjälä:
> On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
>>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>
>>> This function would call drm_modeset_lock_all, while the suspend/resume
>>> functions already have their own locking. Fix this by factoring out
>>> __intel_display_resume, and calling the atomic helpers for duplicating
>>> atomic state and disabling all crtc's during suspend.
>>>
>>> Changes since v1:
>>> - Deal with -EDEADLK right after lock_all and clean up calls
>>>   to hw readout.
>>> - Always take all modeset locks so updates during gpu reset are blocked.
>>> Changes since v2:
>>> - Fix deadlock in intel_update_primary_planes.
>>> - Move WARN_ON(EDEADLK) to __intel_display_resume.
>>> - pctx -> ctx
>>> - only call __intel_display_resume on success in intel_display_resume.
>>> Changes since v3:
>>> - Rebase on top of dev_priv -> dev change.
>>> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
>>> Changes since v4 [by vsyrjala]:
>>> - Deal with skip_intermediate_wm
>>> - Update comment w.r.t. mode_config.mutex vs. ->detect()
>>> - Rebase due to INTEL_GEN() etc.
>> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
>> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
>> until all planes are enabled.
> What blows up and how?
>
> Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
> this time anyway, so -ENOCARE at this point really.
>
>> I fear this may blow up in interesting ways. And it should probably be using
>> dev_priv->wm.distrust_bios_wm instead like on SKL.
> Sigh. How many ways do we need to do the same thing?
>
> Anywyas, what we should really do is sanitize the current wms better
> at readout time, and then we shouldn't need these flags at all.
>
Yeah, slightly different approach of accomplishing the same. :-/

distrust_bios_wm pulls in the whole state and recalculates it, while sanitize_watermarks runs at the end of initial config.
Maybe get_hw_state for ILK should set the flag too, and  then stuff final wm in intermediate. And then kill off the skip_intermediate_wm flag.

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Ville Syrjälä]

On Mon, Aug 08, 2016 at 10:40:42AM +0200, Maarten Lankhorst wrote:
> Op 08-08-16 om 10:05 schreef Ville Syrjälä:
> > On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
> >> Hey,
> >>
> >> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> >>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>
> >>> This function would call drm_modeset_lock_all, while the suspend/resume
> >>> functions already have their own locking. Fix this by factoring out
> >>> __intel_display_resume, and calling the atomic helpers for duplicating
> >>> atomic state and disabling all crtc's during suspend.
> >>>
> >>> Changes since v1:
> >>> - Deal with -EDEADLK right after lock_all and clean up calls
> >>>   to hw readout.
> >>> - Always take all modeset locks so updates during gpu reset are blocked.
> >>> Changes since v2:
> >>> - Fix deadlock in intel_update_primary_planes.
> >>> - Move WARN_ON(EDEADLK) to __intel_display_resume.
> >>> - pctx -> ctx
> >>> - only call __intel_display_resume on success in intel_display_resume.
> >>> Changes since v3:
> >>> - Rebase on top of dev_priv -> dev change.
> >>> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> >>> Changes since v4 [by vsyrjala]:
> >>> - Deal with skip_intermediate_wm
> >>> - Update comment w.r.t. mode_config.mutex vs. ->detect()
> >>> - Rebase due to INTEL_GEN() etc.
> >> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
> >> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
> >> until all planes are enabled.
> > What blows up and how?
> >
> > Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
> > this time anyway, so -ENOCARE at this point really.
> >
> >> I fear this may blow up in interesting ways. And it should probably be using
> >> dev_priv->wm.distrust_bios_wm instead like on SKL.
> > Sigh. How many ways do we need to do the same thing?
> >
> > Anywyas, what we should really do is sanitize the current wms better
> > at readout time, and then we shouldn't need these flags at all.
> >
> Yeah, slightly different approach of accomplishing the same. :-/
> 
> distrust_bios_wm pulls in the whole state and recalculates it, while sanitize_watermarks runs at the end of initial config.
> Maybe get_hw_state for ILK should set the flag too, and  then stuff final wm in intermediate. And then kill off the skip_intermediate_wm flag.

Or just kill both flags and sanitize better.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Maarten Lankhorst]

Op 08-08-16 om 10:57 schreef Ville Syrjälä:
> On Mon, Aug 08, 2016 at 10:40:42AM +0200, Maarten Lankhorst wrote:
>> Op 08-08-16 om 10:05 schreef Ville Syrjälä:
>>> On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
>>>> Hey,
>>>>
>>>> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
>>>>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>>
>>>>> This function would call drm_modeset_lock_all, while the suspend/resume
>>>>> functions already have their own locking. Fix this by factoring out
>>>>> __intel_display_resume, and calling the atomic helpers for duplicating
>>>>> atomic state and disabling all crtc's during suspend.
>>>>>
>>>>> Changes since v1:
>>>>> - Deal with -EDEADLK right after lock_all and clean up calls
>>>>>   to hw readout.
>>>>> - Always take all modeset locks so updates during gpu reset are blocked.
>>>>> Changes since v2:
>>>>> - Fix deadlock in intel_update_primary_planes.
>>>>> - Move WARN_ON(EDEADLK) to __intel_display_resume.
>>>>> - pctx -> ctx
>>>>> - only call __intel_display_resume on success in intel_display_resume.
>>>>> Changes since v3:
>>>>> - Rebase on top of dev_priv -> dev change.
>>>>> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
>>>>> Changes since v4 [by vsyrjala]:
>>>>> - Deal with skip_intermediate_wm
>>>>> - Update comment w.r.t. mode_config.mutex vs. ->detect()
>>>>> - Rebase due to INTEL_GEN() etc.
>>>> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
>>>> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
>>>> until all planes are enabled.
>>> What blows up and how?
>>>
>>> Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
>>> this time anyway, so -ENOCARE at this point really.
>>>
>>>> I fear this may blow up in interesting ways. And it should probably be using
>>>> dev_priv->wm.distrust_bios_wm instead like on SKL.
>>> Sigh. How many ways do we need to do the same thing?
>>>
>>> Anywyas, what we should really do is sanitize the current wms better
>>> at readout time, and then we shouldn't need these flags at all.
>>>
>> Yeah, slightly different approach of accomplishing the same. :-/
>>
>> distrust_bios_wm pulls in the whole state and recalculates it, while sanitize_watermarks runs at the end of initial config.
>> Maybe get_hw_state for ILK should set the flag too, and  then stuff final wm in intermediate. And then kill off the skip_intermediate_wm flag.
> Or just kill both flags and sanitize better.
>
The first flag is used by the watermark sanitization, second flag seems to be useful to make the driver init faster, first commit (either by fbcon or userspace) will incur the real penalty. This is similar to fastset, which is also recalculated on the first modeset.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v5 1/4] drm/i915: Fix modeset handling during gpu reset, v5.

[Ville Syrjälä]

On Mon, Aug 08, 2016 at 11:08:33AM +0200, Maarten Lankhorst wrote:
> Op 08-08-16 om 10:57 schreef Ville Syrjälä:
> > On Mon, Aug 08, 2016 at 10:40:42AM +0200, Maarten Lankhorst wrote:
> >> Op 08-08-16 om 10:05 schreef Ville Syrjälä:
> >>> On Mon, Aug 08, 2016 at 09:52:49AM +0200, Maarten Lankhorst wrote:
> >>>> Hey,
> >>>>
> >>>> Op 05-08-16 om 22:28 schreef ville.syrjala@linux.intel.com:
> >>>>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>>>
> >>>>> This function would call drm_modeset_lock_all, while the suspend/resume
> >>>>> functions already have their own locking. Fix this by factoring out
> >>>>> __intel_display_resume, and calling the atomic helpers for duplicating
> >>>>> atomic state and disabling all crtc's during suspend.
> >>>>>
> >>>>> Changes since v1:
> >>>>> - Deal with -EDEADLK right after lock_all and clean up calls
> >>>>>   to hw readout.
> >>>>> - Always take all modeset locks so updates during gpu reset are blocked.
> >>>>> Changes since v2:
> >>>>> - Fix deadlock in intel_update_primary_planes.
> >>>>> - Move WARN_ON(EDEADLK) to __intel_display_resume.
> >>>>> - pctx -> ctx
> >>>>> - only call __intel_display_resume on success in intel_display_resume.
> >>>>> Changes since v3:
> >>>>> - Rebase on top of dev_priv -> dev change.
> >>>>> - Use drm_modeset_lock_all_ctx instead of drm_modeset_lock_all.
> >>>>> Changes since v4 [by vsyrjala]:
> >>>>> - Deal with skip_intermediate_wm
> >>>>> - Update comment w.r.t. mode_config.mutex vs. ->detect()
> >>>>> - Rebase due to INTEL_GEN() etc.
> >>>> Setting skip_intermediate_wm seems to have already been upstreamed and I missed it, but
> >>>> this may blow up in .crtc_enable, which programs in the intermediate wm's which is used
> >>>> until all planes are enabled.
> >>> What blows up and how?
> >>>
> >>> Even if it can blow up we don't have any two stage wm stuff for pre-g4x at
> >>> this time anyway, so -ENOCARE at this point really.
> >>>
> >>>> I fear this may blow up in interesting ways. And it should probably be using
> >>>> dev_priv->wm.distrust_bios_wm instead like on SKL.
> >>> Sigh. How many ways do we need to do the same thing?
> >>>
> >>> Anywyas, what we should really do is sanitize the current wms better
> >>> at readout time, and then we shouldn't need these flags at all.
> >>>
> >> Yeah, slightly different approach of accomplishing the same. :-/
> >>
> >> distrust_bios_wm pulls in the whole state and recalculates it, while sanitize_watermarks runs at the end of initial config.
> >> Maybe get_hw_state for ILK should set the flag too, and  then stuff final wm in intermediate. And then kill off the skip_intermediate_wm flag.
> > Or just kill both flags and sanitize better.
> >
> The first flag is used by the watermark sanitization, second flag seems to be useful to make the driver init faster, first commit (either by fbcon or userspace) will incur the real penalty. This is similar to fastset, which is also recalculated on the first modeset.

The skl flag looks to be about the DDB. Irrelevant on other platforms,
except gen2-4 if/when someone decides to implement DDB reallocation
for them.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff

[Ville Syrjälä]

On Sat, Aug 06, 2016 at 08:45:37AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff
> URL   : https://patchwork.freedesktop.org/series/10731/
> State : failure
> 
> == Summary ==
> 
> Series 10731v1 drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff
> http://patchwork.freedesktop.org/api/1.0/series/10731/revisions/1/mbox
> 
> Test kms_cursor_legacy:
>         Subgroup basic-cursor-vs-flip-varying-size:
>                 pass       -> FAIL       (ro-ilk1-i5-650)

(kms_cursor_legacy:8107) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:8107) DEBUG: Using a target of 32 cursor updates per half-vblank
(kms_cursor_legacy:8107) WARNING: page flip 11 was delayed, missed 2 frames
(kms_cursor_legacy:8107) CRITICAL: Test assertion failure function basic_cursor_vs_flip, file kms_cursor_legacy.c:670:
(kms_cursor_legacy:8107) CRITICAL: Failed assertion: vbl.sequence == vblank_start + 60
(kms_cursor_legacy:8107) CRITICAL: error: 11538 != 11536

https://bugs.freedesktop.org/show_bug.cgi?id=96701

>         Subgroup basic-flip-vs-cursor-varying-size:
>                 pass       -> FAIL       (ro-snb-i7-2620M)
>                 fail       -> PASS       (ro-bdw-i5-5250u)

(kms_cursor_legacy:7751) DEBUG: Test requirement passed: target > 1
(kms_cursor_legacy:7751) DEBUG: Using a target of 64 cursor updates per half-vblank
(kms_cursor_legacy:7751) CRITICAL: Test assertion failure function basic_flip_vs_cursor, file kms_cursor_legacy.c:514:
(kms_cursor_legacy:7751) CRITICAL: Failed assertion: get_vblank(display->drm_fd, pipe, 0) == vblank_start
(kms_cursor_legacy:7751) CRITICAL: error: 13763 != 13762

https://bugs.freedesktop.org/show_bug.cgi?id=97188

> 
> fi-kbl-qkkr      total:244  pass:185  dwarn:29  dfail:0   fail:3   skip:27 
> ro-bdw-i5-5250u  total:240  pass:219  dwarn:4   dfail:0   fail:1   skip:16 
> ro-bdw-i7-5557U  total:240  pass:224  dwarn:0   dfail:0   fail:0   skip:16 
> ro-bdw-i7-5600u  total:240  pass:207  dwarn:0   dfail:0   fail:1   skip:32 
> ro-bsw-n3050     total:240  pass:194  dwarn:0   dfail:0   fail:4   skip:42 
> ro-hsw-i3-4010u  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
> ro-hsw-i7-4770r  total:240  pass:214  dwarn:0   dfail:0   fail:0   skip:26 
> ro-ilk-i7-620lm  total:240  pass:173  dwarn:1   dfail:0   fail:1   skip:65 
> ro-ilk1-i5-650   total:235  pass:173  dwarn:0   dfail:0   fail:2   skip:60 
> ro-ivb-i7-3770   total:240  pass:205  dwarn:0   dfail:0   fail:0   skip:35 
> ro-ivb2-i7-3770  total:240  pass:209  dwarn:0   dfail:0   fail:0   skip:31 
> ro-skl3-i5-6260u total:240  pass:223  dwarn:0   dfail:0   fail:3   skip:14 
> ro-snb-i7-2620M  total:240  pass:197  dwarn:0   dfail:0   fail:2   skip:41 
> ro-byt-n2820 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1741/
> 
> b834992 drm-intel-nightly: 2016y-08m-05d-20h-40m-44s UTC integration manifest
> ed2ac6e drm/i915: Use the g4x+ approach on gen2 for handling display stuff around GPU reset
> d29861b drm/i915: Introduce gpu_reset_clobbers_display()
> 893b403 drm/i915: Add a way to test the modeset done during gpu reset, v3.
> 617092d drm/i915: Fix modeset handling during gpu reset, v5.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/4] drm/i915: Maarten's pre-g4x GPU reset regression fix + other reset stuff

[Ville Syrjälä]

On Fri, Aug 05, 2016 at 11:28:26PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I got annoyed at the pre-g4x gpu reset regression again, so I
> picked up the latest (I think it was the latest anyway) version of
> Maarten's locking fix from the list and rebased it. I also changed
> one comment in there about mode_config.mutex as the original didn't
> make sense to me.
> 
> I tossed in a few polishing patches on top, and tested the whole lot
> on my 946gz, and I also ran it on hsw w/ and w/o the new modparam set.
> No problems whatsoever.
> 
> Maarten Lankhorst (2):
>   drm/i915: Fix modeset handling during gpu reset, v5.
>   drm/i915: Add a way to test the modeset done during gpu reset, v3.
> 
> Ville Syrjälä (2):
>   drm/i915: Introduce gpu_reset_clobbers_display()
>   drm/i915: Use the g4x+ approach on gen2 for handling display stuff
>     around GPU reset

Entire series pushed to dinq. Thanks for the patches and reviews.

Maarten, did you have the igt for utilizing the new module param around
somewhere? Could push it now I suppose.

> 
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_params.c   |   6 +
>  drivers/gpu/drm/i915/i915_params.h   |   1 +
>  drivers/gpu/drm/i915/intel_display.c | 205 ++++++++++++++++++++++-------------
>  4 files changed, 138 insertions(+), 75 deletions(-)
> 
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx