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@xxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
