* [PATCH] drm/i915: Add a way to test the modeset done during gpu reset.
@ 2016-04-05 10:56 Maarten Lankhorst
2016-04-05 11:00 ` [PATCH i-g-t] drv_hangman: Add test for modeset " Maarten Lankhorst
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2016-04-05 10:56 UTC (permalink / raw)
To: intel-gfx; +Cc: drm-intel-fixes
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.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
Cc: drm-intel-fixes@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_params.c | 6 ++
drivers/gpu/drm/i915/i915_params.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 161 ++++++++++++++++++++++++-----------
3 files changed, 116 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 1779f02e6df8..80ce581793dc 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,
@@ -158,6 +159,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 02bc27804291..3934c4300427 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,6 +55,7 @@ struct i915_params {
bool fastboot;
bool prefault_disable;
bool load_detect_test;
+ bool force_reset_modeset_test;
bool reset;
bool disable_display;
bool enable_guc_submission;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index af74cdba7081..acca08797123 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3119,27 +3119,94 @@ static void intel_update_primary_planes(struct drm_device *dev)
}
}
+static int
+__intel_display_resume(struct drm_device *dev,
+ struct drm_atomic_state *state,
+ bool *setup)
+{
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ int i;
+
+ if (!setup || !*setup) {
+ if (setup)
+ *setup = true;
+
+ 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;
+ }
+
+ return drm_atomic_commit(state);
+}
+
void intel_prepare_reset(struct drm_device *dev)
{
+ bool requires_display_reset = true;
+ struct drm_atomic_state *state;
+ struct drm_modeset_acquire_ctx *pctx;
+ int ret;
+
/* no reset support for gen2 */
if (IS_GEN2(dev))
return;
/* reset doesn't touch the display */
- if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
- return;
+ if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
+ if (!i915.force_reset_modeset_test)
+ return;
+
+ requires_display_reset = false;
+ }
drm_modeset_lock_all(dev);
+ pctx = dev->mode_config.acquire_ctx;
+
/*
* Disabling the crtcs gracefully seems nicer. Also the
* g33 docs say we should at least disable all the planes.
*/
- intel_display_suspend(dev);
+
+ state = drm_atomic_helper_duplicate_state(dev, pctx);
+ 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, pctx);
+ if (ret) {
+ DRM_ERROR("Suspending crtc's failed with %i\n", ret);
+ goto err;
+ }
+
+ to_i915(dev)->modeset_restore_state = state;
+ return;
+
+err:
+ drm_atomic_state_free(state);
+ if (!requires_display_reset)
+ drm_modeset_unlock_all(dev);
}
void intel_finish_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_atomic_state *state = dev_priv->modeset_restore_state;
+ bool hpd_init;
+ int ret;
/*
* Flips in the rings will be nuked by the reset,
@@ -3154,36 +3221,48 @@ void intel_finish_reset(struct drm_device *dev)
/* reset doesn't touch the display */
if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
+ hpd_init = false;
+
+ 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);
+ return;
+ }
+ } else {
/*
- * 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.
+ * The display has been reset as well,
+ * so need a full re-initialization.
*/
- intel_update_primary_planes(dev);
- return;
- }
+ hpd_init = true;
+ intel_runtime_pm_disable_interrupts(dev_priv);
+ intel_runtime_pm_enable_interrupts(dev_priv);
- /*
- * 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);
- 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);
+ 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);
- spin_unlock_irq(&dev_priv->irq_lock);
+ dev_priv->modeset_restore_state = NULL;
+ if (state)
+ state->acquire_ctx = dev->mode_config.acquire_ctx;
- intel_display_resume(dev);
+ ret = __intel_display_resume(dev, state, NULL);
+ if (ret)
+ DRM_ERROR("Restoring old state failed with %i\n", ret);
- intel_hpd_init(dev_priv);
+ if (hpd_init)
+ intel_hpd_init(dev_priv);
drm_modeset_unlock_all(dev);
}
@@ -15876,6 +15955,8 @@ void intel_display_resume(struct drm_device *dev)
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
@@ -15888,32 +15969,8 @@ void intel_display_resume(struct drm_device *dev)
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;
-
- 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);
- }
+ if (!ret)
+ ret = __intel_display_resume(dev, state, &setup);
if (ret == -EDEADLK) {
drm_modeset_backoff(&ctx);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH i-g-t] drv_hangman: Add test for modeset during gpu reset.
2016-04-05 10:56 [PATCH] drm/i915: Add a way to test the modeset done during gpu reset Maarten Lankhorst
@ 2016-04-05 11:00 ` Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 1/2] drm/i915: Fix modeset handling " Maarten Lankhorst
2016-04-05 13:01 ` [PATCH] drm/i915: Add a way to test the modeset done " Ville Syrjälä
2016-04-05 13:37 ` ✗ Fi.CI.BAT: failure for " Patchwork
2 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2016-04-05 11:00 UTC (permalink / raw)
To: intel-gfx
This adds a test that forces a modeset during gpu reset, which has broken
a few times in the past during the atomic rework.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com
---
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index f4cdadb80e02..8a17d7acff1b 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -324,6 +324,22 @@ static void hangcheck_unterminated(void)
close(fd);
}
+static void reset_with_forced_modeset(void)
+{
+ igt_hang_ring_t hang;
+ int fd = drm_open_driver(DRIVER_INTEL);
+
+ igt_require_hang_ring(fd, I915_EXEC_DEFAULT);
+
+ igt_set_module_param_int("force_reset_modeset_test", 1);
+
+ hang = igt_hang_ring(fd, I915_EXEC_DEFAULT);
+ igt_post_hang_ring(fd, hang);
+
+ igt_set_module_param_int("force_reset_modeset_test", 0);
+ close(fd);
+}
+
igt_main
{
const struct intel_execution_engine *e;
@@ -353,4 +369,7 @@ igt_main
igt_subtest("hangcheck-unterminated")
hangcheck_unterminated();
+
+ igt_subtest("reset-with-forced-modeset")
+ reset_with_forced_modeset();
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Add a way to test the modeset done during gpu reset.
2016-04-05 10:56 [PATCH] drm/i915: Add a way to test the modeset done during gpu reset Maarten Lankhorst
2016-04-05 11:00 ` [PATCH i-g-t] drv_hangman: Add test for modeset " Maarten Lankhorst
@ 2016-04-05 13:01 ` Ville Syrjälä
2016-04-05 13:37 ` ✗ Fi.CI.BAT: failure for " Patchwork
2 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-05 13:01 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, drm-intel-fixes
On Tue, Apr 05, 2016 at 12:56:37PM +0200, Maarten Lankhorst wrote:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
> Cc: drm-intel-fixes@lists.freedesktop.org
Seems like this should be two patches; One to fix the locking and
another to add the debug knob.
> ---
> drivers/gpu/drm/i915/i915_params.c | 6 ++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 161 ++++++++++++++++++++++++-----------
> 3 files changed, 116 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 1779f02e6df8..80ce581793dc 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,
> @@ -158,6 +159,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 02bc27804291..3934c4300427 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -55,6 +55,7 @@ struct i915_params {
> bool fastboot;
> bool prefault_disable;
> bool load_detect_test;
> + bool force_reset_modeset_test;
> bool reset;
> bool disable_display;
> bool enable_guc_submission;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index af74cdba7081..acca08797123 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3119,27 +3119,94 @@ static void intel_update_primary_planes(struct drm_device *dev)
> }
> }
>
> +static int
> +__intel_display_resume(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + bool *setup)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct drm_crtc *crtc;
> + int i;
> +
> + if (!setup || !*setup) {
> + if (setup)
> + *setup = true;
> +
> + 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;
> + }
> +
> + return drm_atomic_commit(state);
> +}
> +
> void intel_prepare_reset(struct drm_device *dev)
> {
> + bool requires_display_reset = true;
> + struct drm_atomic_state *state;
> + struct drm_modeset_acquire_ctx *pctx;
> + int ret;
> +
> /* no reset support for gen2 */
> if (IS_GEN2(dev))
> return;
>
> /* reset doesn't touch the display */
> - if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> - return;
> + if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> + if (!i915.force_reset_modeset_test)
> + return;
> +
> + requires_display_reset = false;
Can't we just run the full code on g4x+ anyway? Would leave the code
much simpler.
> + }
>
> drm_modeset_lock_all(dev);
> + pctx = dev->mode_config.acquire_ctx;
pctx makes me think of power context.
> +
> /*
> * Disabling the crtcs gracefully seems nicer. Also the
> * g33 docs say we should at least disable all the planes.
> */
> - intel_display_suspend(dev);
> +
> + state = drm_atomic_helper_duplicate_state(dev, pctx);
> + 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, pctx);
> + if (ret) {
> + DRM_ERROR("Suspending crtc's failed with %i\n", ret);
> + goto err;
> + }
> +
> + to_i915(dev)->modeset_restore_state = state;
> + return;
> +
> +err:
> + drm_atomic_state_free(state);
> + if (!requires_display_reset)
> + drm_modeset_unlock_all(dev);
> }
>
> void intel_finish_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_atomic_state *state = dev_priv->modeset_restore_state;
> + bool hpd_init;
> + int ret;
>
> /*
> * Flips in the rings will be nuked by the reset,
> @@ -3154,36 +3221,48 @@ void intel_finish_reset(struct drm_device *dev)
>
> /* reset doesn't touch the display */
> if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> + hpd_init = false;
> +
> + 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);
> + return;
> + }
> + } else {
> /*
> - * 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.
> + * The display has been reset as well,
> + * so need a full re-initialization.
> */
> - intel_update_primary_planes(dev);
> - return;
> - }
> + hpd_init = true;
> + intel_runtime_pm_disable_interrupts(dev_priv);
> + intel_runtime_pm_enable_interrupts(dev_priv);
>
> - /*
> - * 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);
>
> - 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);
> + 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);
> - spin_unlock_irq(&dev_priv->irq_lock);
> + dev_priv->modeset_restore_state = NULL;
> + if (state)
> + state->acquire_ctx = dev->mode_config.acquire_ctx;
>
> - intel_display_resume(dev);
> + ret = __intel_display_resume(dev, state, NULL);
> + if (ret)
> + DRM_ERROR("Restoring old state failed with %i\n", ret);
>
> - intel_hpd_init(dev_priv);
> + if (hpd_init)
> + intel_hpd_init(dev_priv);
>
> drm_modeset_unlock_all(dev);
> }
> @@ -15876,6 +15955,8 @@ void intel_display_resume(struct drm_device *dev)
> 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
> @@ -15888,32 +15969,8 @@ void intel_display_resume(struct drm_device *dev)
>
> 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;
> -
> - 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);
> - }
> + if (!ret)
> + ret = __intel_display_resume(dev, state, &setup);
>
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Add a way to test the modeset done during gpu reset.
2016-04-05 10:56 [PATCH] drm/i915: Add a way to test the modeset done during gpu reset Maarten Lankhorst
2016-04-05 11:00 ` [PATCH i-g-t] drv_hangman: Add test for modeset " Maarten Lankhorst
2016-04-05 13:01 ` [PATCH] drm/i915: Add a way to test the modeset done " Ville Syrjälä
@ 2016-04-05 13:37 ` Patchwork
2 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2016-04-05 13:37 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Add a way to test the modeset done during gpu reset.
URL : https://patchwork.freedesktop.org/series/5314/
State : failure
== Summary ==
Series 5314v1 drm/i915: Add a way to test the modeset done during gpu reset.
http://patchwork.freedesktop.org/api/1.0/series/5314/revisions/1/mbox/
Test drv_getparams_basic:
Subgroup basic-subslice-total:
pass -> DMESG-WARN (bsw-nuc-2)
Test drv_hangman:
Subgroup error-state-basic:
pass -> SKIP (bsw-nuc-2)
Test gem_cs_tlb:
Subgroup basic-default:
pass -> SKIP (bsw-nuc-2)
Test gem_ctx_basic:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_exec_basic:
Subgroup gtt-bsd:
pass -> SKIP (bsw-nuc-2)
Subgroup readonly-default:
pass -> SKIP (bsw-nuc-2)
Subgroup readonly-vebox:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_nop:
Subgroup basic:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_store:
Subgroup basic-default:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> SKIP (bsw-nuc-2)
Test gem_exec_whisper:
Subgroup basic:
pass -> INCOMPLETE (bsw-nuc-2)
Test gem_mmap_gtt:
Subgroup basic-read-write-distinct:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-write-gtt:
pass -> DMESG-WARN (bsw-nuc-2)
Test gem_ringfill:
Subgroup basic-default:
pass -> SKIP (bsw-nuc-2)
Test gem_storedw_loop:
Subgroup basic-blt:
pass -> DMESG-FAIL (bsw-nuc-2)
Subgroup basic-render:
pass -> SKIP (bsw-nuc-2)
Test kms_addfb_basic:
Subgroup addfb25-framebuffer-vs-set-tiling:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup addfb25-x-tiled-mismatch:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup addfb25-y-tiled:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-pitch-1024:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bad-pitch-63:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-x-tiled:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-y-tiled:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup bo-too-small-due-to-tiling:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup clobberred-modifier:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup no-handle:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup unused-modifier:
pass -> DMESG-WARN (bsw-nuc-2)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (snb-x220t)
pass -> FAIL (bsw-nuc-2)
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
pass -> SKIP (ilk-hp8440p)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> DMESG-FAIL (bsw-nuc-2)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
pass -> SKIP (bsw-nuc-2)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> DMESG-FAIL (bsw-nuc-2)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> DMESG-FAIL (bsw-nuc-2)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-rte:
dmesg-warn -> PASS (bsw-nuc-2)
Test prime_self_import:
Subgroup basic-llseek-bad:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-llseek-size:
pass -> DMESG-WARN (bsw-nuc-2)
Subgroup basic-with_fd_dup:
pass -> DMESG-WARN (bsw-nuc-2)
bdw-ultra total:196 pass:175 dwarn:0 dfail:0 fail:0 skip:21
bsw-nuc-2 total:153 pass:88 dwarn:20 dfail:4 fail:1 skip:39
byt-nuc total:196 pass:161 dwarn:0 dfail:0 fail:0 skip:35
hsw-brixbox total:196 pass:174 dwarn:0 dfail:0 fail:0 skip:22
hsw-gt2 total:196 pass:179 dwarn:0 dfail:0 fail:0 skip:17
ilk-hp8440p total:196 pass:130 dwarn:1 dfail:0 fail:0 skip:65
ivb-t430s total:196 pass:171 dwarn:0 dfail:0 fail:0 skip:25
skl-i7k-2 total:196 pass:173 dwarn:0 dfail:0 fail:0 skip:23
skl-nuci5 total:196 pass:185 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:196 pass:162 dwarn:0 dfail:0 fail:0 skip:34
snb-x220t total:196 pass:162 dwarn:0 dfail:0 fail:1 skip:33
BOOT FAILED for bdw-nuci7
Results at /archive/results/CI_IGT_test/Patchwork_1800/
48d2a0497cd1d0630fbb59fb7871e4d678458307 drm-intel-nightly: 2016y-04m-05d-12h-30m-42s UTC integration manifest
e4dd19c811e4ce73edf2919e7244f5484ac76265 drm/i915: Add a way to test the modeset done during gpu reset.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset.
2016-04-05 11:00 ` [PATCH i-g-t] drv_hangman: Add test for modeset " Maarten Lankhorst
@ 2016-04-11 6:23 ` Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2 Maarten Lankhorst
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2016-04-11 6:23 UTC (permalink / raw)
To: intel-gfx; +Cc: drm-intel-fixes
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.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
Cc: drm-intel-fixes@lists.freedesktop.org
---
drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index feb7028341b8..2f76efc86417 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3126,8 +3126,44 @@ static void intel_update_primary_planes(struct drm_device *dev)
}
}
+static int
+__intel_display_resume(struct drm_device *dev,
+ struct drm_atomic_state *state,
+ bool *setup)
+{
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc *crtc;
+ int i;
+
+ if (!setup || !*setup) {
+ if (setup)
+ *setup = true;
+
+ 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;
+ }
+
+ return drm_atomic_commit(state);
+}
+
void intel_prepare_reset(struct drm_device *dev)
{
+ struct drm_atomic_state *state;
+ struct drm_modeset_acquire_ctx *pctx;
+ int ret;
+
/* no reset support for gen2 */
if (IS_GEN2(dev))
return;
@@ -3137,16 +3173,39 @@ void intel_prepare_reset(struct drm_device *dev)
return;
drm_modeset_lock_all(dev);
+ pctx = dev->mode_config.acquire_ctx;
+
/*
* Disabling the crtcs gracefully seems nicer. Also the
* g33 docs say we should at least disable all the planes.
*/
- intel_display_suspend(dev);
+
+ state = drm_atomic_helper_duplicate_state(dev, pctx);
+ 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, pctx);
+ if (ret) {
+ DRM_ERROR("Suspending crtc's failed with %i\n", ret);
+ goto err;
+ }
+
+ to_i915(dev)->modeset_restore_state = state;
+ return;
+
+err:
+ drm_atomic_state_free(state);
}
void intel_finish_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_atomic_state *state = dev_priv->modeset_restore_state;
+ int ret;
/*
* Flips in the rings will be nuked by the reset,
@@ -3172,23 +3231,29 @@ void intel_finish_reset(struct drm_device *dev)
*/
intel_update_primary_planes(dev);
return;
- }
+ } 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);
- /*
- * 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);
- 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);
+ 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);
- spin_unlock_irq(&dev_priv->irq_lock);
+ dev_priv->modeset_restore_state = NULL;
+ if (state)
+ state->acquire_ctx = dev->mode_config.acquire_ctx;
- intel_display_resume(dev);
+ ret = __intel_display_resume(dev, state, NULL);
+ if (ret)
+ DRM_ERROR("Restoring old state failed with %i\n", ret);
intel_hpd_init(dev_priv);
@@ -15885,6 +15950,8 @@ void intel_display_resume(struct drm_device *dev)
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
@@ -15897,32 +15964,8 @@ void intel_display_resume(struct drm_device *dev)
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;
-
- 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);
- }
+ if (!ret)
+ ret = __intel_display_resume(dev, state, &setup);
if (ret == -EDEADLK) {
drm_modeset_backoff(&ctx);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2.
2016-04-11 6:23 ` [PATCH v2 1/2] drm/i915: Fix modeset handling " Maarten Lankhorst
@ 2016-04-11 6:23 ` Maarten Lankhorst
2016-04-25 16:33 ` Ville Syrjälä
2016-04-20 7:02 ` [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset Maarten Lankhorst
2016-04-25 16:10 ` Ville Syrjälä
2 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2016-04-11 6:23 UTC (permalink / raw)
To: intel-gfx
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.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: drv_hangman.reset-with-forced-modeset
---
drivers/gpu/drm/i915/i915_params.c | 6 ++++++
drivers/gpu/drm/i915/i915_params.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++++++++------------
3 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 1779f02e6df8..80ce581793dc 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,
@@ -158,6 +159,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 02bc27804291..3934c4300427 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,6 +55,7 @@ struct i915_params {
bool fastboot;
bool prefault_disable;
bool load_detect_test;
+ bool force_reset_modeset_test;
bool reset;
bool disable_display;
bool enable_guc_submission;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2f76efc86417..faf2da08778f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3160,6 +3160,7 @@ __intel_display_resume(struct drm_device *dev,
void intel_prepare_reset(struct drm_device *dev)
{
+ bool requires_display_reset = true;
struct drm_atomic_state *state;
struct drm_modeset_acquire_ctx *pctx;
int ret;
@@ -3169,8 +3170,12 @@ void intel_prepare_reset(struct drm_device *dev)
return;
/* reset doesn't touch the display */
- if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
- return;
+ if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
+ if (!i915.force_reset_modeset_test)
+ return;
+
+ requires_display_reset = false;
+ }
drm_modeset_lock_all(dev);
pctx = dev->mode_config.acquire_ctx;
@@ -3199,12 +3204,15 @@ void intel_prepare_reset(struct drm_device *dev)
err:
drm_atomic_state_free(state);
+ if (!requires_display_reset)
+ drm_modeset_unlock_all(dev);
}
void intel_finish_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_atomic_state *state = dev_priv->modeset_restore_state;
+ bool hpd_init;
int ret;
/*
@@ -3220,22 +3228,27 @@ void intel_finish_reset(struct drm_device *dev)
/* reset doesn't touch the display */
if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
- /*
- * 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);
- return;
+ hpd_init = false;
+
+ 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);
+ return;
+ }
} else {
/*
* The display has been reset as well,
* so need a full re-initialization.
*/
+ hpd_init = true;
intel_runtime_pm_disable_interrupts(dev_priv);
intel_runtime_pm_enable_interrupts(dev_priv);
@@ -3255,7 +3268,8 @@ void intel_finish_reset(struct drm_device *dev)
if (ret)
DRM_ERROR("Restoring old state failed with %i\n", ret);
- intel_hpd_init(dev_priv);
+ if (hpd_init)
+ intel_hpd_init(dev_priv);
drm_modeset_unlock_all(dev);
}
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset.
2016-04-11 6:23 ` [PATCH v2 1/2] drm/i915: Fix modeset handling " Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2 Maarten Lankhorst
@ 2016-04-20 7:02 ` Maarten Lankhorst
2016-04-25 16:10 ` Ville Syrjälä
2 siblings, 0 replies; 9+ messages in thread
From: Maarten Lankhorst @ 2016-04-20 7:02 UTC (permalink / raw)
To: intel-gfx
Op 11-04-16 om 08:23 schreef Maarten Lankhorst:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
> Cc: drm-intel-fixes@lists.freedesktop.org
>
Ping for review?
Also for "[PATCH i-g-t] drv_hangman: Add test for modeset during gpu reset." from the original series.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset.
2016-04-11 6:23 ` [PATCH v2 1/2] drm/i915: Fix modeset handling " Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2 Maarten Lankhorst
2016-04-20 7:02 ` [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset Maarten Lankhorst
@ 2016-04-25 16:10 ` Ville Syrjälä
2 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-25 16:10 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx, drm-intel-fixes
On Mon, Apr 11, 2016 at 08:23:33AM +0200, Maarten Lankhorst wrote:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")
> Cc: drm-intel-fixes@lists.freedesktop.org
> ---
> drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++------------
> 1 file changed, 83 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index feb7028341b8..2f76efc86417 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3126,8 +3126,44 @@ static void intel_update_primary_planes(struct drm_device *dev)
> }
> }
>
> +static int
> +__intel_display_resume(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + bool *setup)
> +{
> + struct drm_crtc_state *crtc_state;
> + struct drm_crtc *crtc;
> + int i;
> +
> + if (!setup || !*setup) {
> + if (setup)
> + *setup = true;
This is getting nasty. How about dealing with -EDEADLK right after the
modeset_lock_all() in the caller, so that we don't have to be concerned
with getting called mutiple times here?
> +
> + 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;
> + }
> +
> + return drm_atomic_commit(state);
To go along with my suggestion about dealing with -EDEADLK upfront,
we could then have a WARN_ON(ret == -EDEADLK) here to make sure we
didn't make a mess of things.
> +}
> +
> void intel_prepare_reset(struct drm_device *dev)
> {
> + struct drm_atomic_state *state;
> + struct drm_modeset_acquire_ctx *pctx;
> + int ret;
> +
> /* no reset support for gen2 */
> if (IS_GEN2(dev))
> return;
> @@ -3137,16 +3173,39 @@ void intel_prepare_reset(struct drm_device *dev)
> return;
>
> drm_modeset_lock_all(dev);
> + pctx = dev->mode_config.acquire_ctx;
Still looks like "power context" to me.
I was wondering if we should have a private context here, but I guess
that would fall over with some mode_config.acquire_ctx usage in the
display init code?
> +
> /*
> * Disabling the crtcs gracefully seems nicer. Also the
> * g33 docs say we should at least disable all the planes.
> */
> - intel_display_suspend(dev);
> +
> + state = drm_atomic_helper_duplicate_state(dev, pctx);
> + 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, pctx);
> + if (ret) {
> + DRM_ERROR("Suspending crtc's failed with %i\n", ret);
> + goto err;
> + }
> +
> + to_i915(dev)->modeset_restore_state = state;
> + return;
> +
> +err:
> + drm_atomic_state_free(state);
> }
>
> void intel_finish_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_atomic_state *state = dev_priv->modeset_restore_state;
> + int ret;
>
> /*
> * Flips in the rings will be nuked by the reset,
> @@ -3172,23 +3231,29 @@ void intel_finish_reset(struct drm_device *dev)
> */
> intel_update_primary_planes(dev);
> return;
> - }
> + } 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);
>
> - /*
> - * 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);
>
> - 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);
> + spin_unlock_irq(&dev_priv->irq_lock);
> + }
This seems like an unrelated change. More to do with the second patch I
guess, though I'm still wondering if we can't just run through the
whole thing every time?
>
> - spin_lock_irq(&dev_priv->irq_lock);
> - if (dev_priv->display.hpd_irq_setup)
> - dev_priv->display.hpd_irq_setup(dev);
> - spin_unlock_irq(&dev_priv->irq_lock);
> + dev_priv->modeset_restore_state = NULL;
> + if (state)
> + state->acquire_ctx = dev->mode_config.acquire_ctx;
>
> - intel_display_resume(dev);
> + ret = __intel_display_resume(dev, state, NULL);
> + if (ret)
> + DRM_ERROR("Restoring old state failed with %i\n", ret);
>
> intel_hpd_init(dev_priv);
>
> @@ -15885,6 +15950,8 @@ void intel_display_resume(struct drm_device *dev)
> 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
> @@ -15897,32 +15964,8 @@ void intel_display_resume(struct drm_device *dev)
>
> 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;
> -
> - 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);
> - }
> + if (!ret)
> + ret = __intel_display_resume(dev, state, &setup);
>
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> --
> 2.1.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2.
2016-04-11 6:23 ` [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2 Maarten Lankhorst
@ 2016-04-25 16:33 ` Ville Syrjälä
0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2016-04-25 16:33 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, Apr 11, 2016 at 08:23:34AM +0200, Maarten Lankhorst wrote:
> 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.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Testcase: drv_hangman.reset-with-forced-modeset
> ---
> drivers/gpu/drm/i915/i915_params.c | 6 ++++++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++++++++------------
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 1779f02e6df8..80ce581793dc 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,
> @@ -158,6 +159,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 02bc27804291..3934c4300427 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -55,6 +55,7 @@ struct i915_params {
> bool fastboot;
> bool prefault_disable;
> bool load_detect_test;
> + bool force_reset_modeset_test;
> bool reset;
> bool disable_display;
> bool enable_guc_submission;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2f76efc86417..faf2da08778f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3160,6 +3160,7 @@ __intel_display_resume(struct drm_device *dev,
>
> void intel_prepare_reset(struct drm_device *dev)
> {
> + bool requires_display_reset = true;
> struct drm_atomic_state *state;
> struct drm_modeset_acquire_ctx *pctx;
> int ret;
> @@ -3169,8 +3170,12 @@ void intel_prepare_reset(struct drm_device *dev)
> return;
>
> /* reset doesn't touch the display */
> - if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> - return;
> + if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> + if (!i915.force_reset_modeset_test)
> + return;
> +
> + requires_display_reset = false;
> + }
>
> drm_modeset_lock_all(dev);
> pctx = dev->mode_config.acquire_ctx;
> @@ -3199,12 +3204,15 @@ void intel_prepare_reset(struct drm_device *dev)
>
> err:
> drm_atomic_state_free(state);
> + if (!requires_display_reset)
> + drm_modeset_unlock_all(dev);
Why do we want to unlock here?
> }
>
> void intel_finish_reset(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_atomic_state *state = dev_priv->modeset_restore_state;
> + bool hpd_init;
> int ret;
>
> /*
> @@ -3220,22 +3228,27 @@ void intel_finish_reset(struct drm_device *dev)
>
> /* reset doesn't touch the display */
> if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> - /*
> - * 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);
> - return;
> + hpd_init = false;
> +
> + 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);
For this I guess. Hmm. Maybe hang on to the lock anyway, and add a
new flag in dev_priv or something to indicate if we're performing
a display reset or not. I suppose we could check for
(pre-g4x || i915.whatever) on either side, but that could
be problematic if someone changes the modparam mid-flight.
So finish_reset could perhaps looks something like (assuming we don't
want to run through the whole display init sequence on g4x+):
{
if (!dev_priv->display_reset) {
intel_update_primary_planes();
return;
}
if (pre-g4x) {
...
ret = display_resume();
init_hpd();
} else {
ret = display_resume();
}
unlock_all();
}
> + return;
> + }
> } else {
> /*
> * The display has been reset as well,
> * so need a full re-initialization.
> */
> + hpd_init = true;
> intel_runtime_pm_disable_interrupts(dev_priv);
> intel_runtime_pm_enable_interrupts(dev_priv);
>
> @@ -3255,7 +3268,8 @@ void intel_finish_reset(struct drm_device *dev)
> if (ret)
> DRM_ERROR("Restoring old state failed with %i\n", ret);
>
> - intel_hpd_init(dev_priv);
> + if (hpd_init)
> + intel_hpd_init(dev_priv);
>
> drm_modeset_unlock_all(dev);
> }
> --
> 2.1.0
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-04-25 16:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-05 10:56 [PATCH] drm/i915: Add a way to test the modeset done during gpu reset Maarten Lankhorst
2016-04-05 11:00 ` [PATCH i-g-t] drv_hangman: Add test for modeset " Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 1/2] drm/i915: Fix modeset handling " Maarten Lankhorst
2016-04-11 6:23 ` [PATCH v2 2/2] drm/i915: Add a way to test the modeset done during gpu reset, v2 Maarten Lankhorst
2016-04-25 16:33 ` Ville Syrjälä
2016-04-20 7:02 ` [PATCH v2 1/2] drm/i915: Fix modeset handling during gpu reset Maarten Lankhorst
2016-04-25 16:10 ` Ville Syrjälä
2016-04-05 13:01 ` [PATCH] drm/i915: Add a way to test the modeset done " Ville Syrjälä
2016-04-05 13:37 ` ✗ Fi.CI.BAT: failure for " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.