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

Submitted by Maarten Lankhorst on April 11, 2016, 6:23 a.m.

Details

Message ID 1460355814-3599-2-git-send-email-maarten.lankhorst@linux.intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Maarten Lankhorst April 11, 2016, 6:23 a.m.
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(-)

Patch hide | download patch | download mbox

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);
 }

Comments

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