[PATCH v3 5/8] drm/i915/reset: remove I915_RESET_MODESET flag

Matt Roper matthew.d.roper at intel.com
Mon Mar 3 23:11:43 UTC 2025 

On Mon, Mar 03, 2025 at 01:27:07PM +0200, Jani Nikula wrote:
> Since commit d59cf7bb73f3 ("drm/i915/display: Use dma_fence interfaces
> instead of i915_sw_fence") we don't have anyone waiting on the
> I915_RESET_MODESET bit, and there's no need for its semantics. Instead,
> simply return true from intel_display_reset_prepare() to indicate that
> intel_display_reset_finish() should be called.
> 
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

> ---
>  .../drm/i915/display/intel_display_reset.c    | 24 +++++++------------
>  .../drm/i915/display/intel_display_reset.h    |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c         |  8 ++++---
>  drivers/gpu/drm/i915/gt/intel_reset_types.h   |  3 +--
>  4 files changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c
> index 121679b4230f..acc728c75328 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reset.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c
> @@ -27,7 +27,8 @@ bool intel_display_reset_test(struct intel_display *display)
>  	return display->params.force_reset_modeset_test;
>  }
>  
> -void intel_display_reset_prepare(struct intel_display *display)
> +/* returns true if intel_display_reset_finish() needs to be called */
> +bool intel_display_reset_prepare(struct intel_display *display)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  	struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx;
> @@ -35,17 +36,12 @@ void intel_display_reset_prepare(struct intel_display *display)
>  	int ret;
>  
>  	if (!HAS_DISPLAY(display))
> -		return;
> +		return false;
>  
>  	/* reset doesn't touch the display */
>  	if (!intel_display_reset_test(display) &&
>  	    !gpu_reset_clobbers_display(display))
> -		return;
> -
> -	/* We have a modeset vs reset deadlock, defensively unbreak it. */
> -	set_bit(I915_RESET_MODESET, &to_gt(dev_priv)->reset.flags);
> -	smp_mb__after_atomic();
> -	wake_up_bit(&to_gt(dev_priv)->reset.flags, I915_RESET_MODESET);
> +		return false;
>  
>  	if (atomic_read(&display->restore.pending_fb_pin)) {
>  		drm_dbg_kms(display->drm,
> @@ -75,7 +71,7 @@ void intel_display_reset_prepare(struct intel_display *display)
>  		ret = PTR_ERR(state);
>  		drm_err(display->drm, "Duplicating state failed with %i\n",
>  			ret);
> -		return;
> +		return true;
>  	}
>  
>  	ret = drm_atomic_helper_disable_all(display->drm, ctx);
> @@ -83,11 +79,13 @@ void intel_display_reset_prepare(struct intel_display *display)
>  		drm_err(display->drm, "Suspending crtc's failed with %i\n",
>  			ret);
>  		drm_atomic_state_put(state);
> -		return;
> +		return true;
>  	}
>  
>  	display->restore.modeset_state = state;
>  	state->acquire_ctx = ctx;
> +
> +	return true;
>  }
>  
>  void intel_display_reset_finish(struct intel_display *display)
> @@ -100,10 +98,6 @@ void intel_display_reset_finish(struct intel_display *display)
>  	if (!HAS_DISPLAY(display))
>  		return;
>  
> -	/* reset doesn't touch the display */
> -	if (!test_bit(I915_RESET_MODESET, &to_gt(i915)->reset.flags))
> -		return;
> -
>  	state = fetch_and_zero(&display->restore.modeset_state);
>  	if (!state)
>  		goto unlock;
> @@ -141,6 +135,4 @@ void intel_display_reset_finish(struct intel_display *display)
>  	drm_modeset_drop_locks(ctx);
>  	drm_modeset_acquire_fini(ctx);
>  	mutex_unlock(&display->drm->mode_config.mutex);
> -
> -	clear_bit_unlock(I915_RESET_MODESET, &to_gt(i915)->reset.flags);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h
> index c1dd2e8d0914..311b5af8ca0c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reset.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h
> @@ -11,7 +11,7 @@
>  struct intel_display;
>  
>  bool intel_display_reset_test(struct intel_display *display);
> -void intel_display_reset_prepare(struct intel_display *display);
> +bool intel_display_reset_prepare(struct intel_display *display);
>  void intel_display_reset_finish(struct intel_display *display);
>  
>  #endif /* __INTEL_RESET_H__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 835c9081b239..f6c8e4d48b04 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1419,12 +1419,14 @@ static void intel_gt_reset_global(struct intel_gt *gt,
>  	intel_wedge_on_timeout(&w, gt, 60 * HZ) {
>  		struct drm_i915_private *i915 = gt->i915;
>  		struct intel_display *display = &i915->display;
> +		bool reset_display;
>  
> -		intel_display_reset_prepare(display);
> +		reset_display = intel_display_reset_prepare(display);
>  
>  		intel_gt_reset(gt, engine_mask, reason);
>  
> -		intel_display_reset_finish(display);
> +		if (reset_display)
> +			intel_display_reset_finish(display);
>  	}
>  
>  	if (!test_bit(I915_WEDGED, &gt->reset.flags))
> @@ -1492,7 +1494,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
>  	    intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
>  		local_bh_disable();
>  		for_each_engine_masked(engine, gt, engine_mask, tmp) {
> -			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
> +			BUILD_BUG_ON(I915_RESET_BACKOFF >= I915_RESET_ENGINE);
>  			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
>  					     &gt->reset.flags))
>  				continue;
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset_types.h b/drivers/gpu/drm/i915/gt/intel_reset_types.h
> index 80351f0a856c..4f5fd393af6f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_reset_types.h
> @@ -41,8 +41,7 @@ struct intel_reset {
>  	 */
>  	unsigned long flags;
>  #define I915_RESET_BACKOFF	0
> -#define I915_RESET_MODESET	1
> -#define I915_RESET_ENGINE	2
> +#define I915_RESET_ENGINE	1
>  #define I915_WEDGED_ON_INIT	(BITS_PER_LONG - 3)
>  #define I915_WEDGED_ON_FINI	(BITS_PER_LONG - 2)
>  #define I915_WEDGED		(BITS_PER_LONG - 1)
> -- 
> 2.39.5
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

More information about the Intel-gfx mailing list