Re: [PATCH v3 7/8] drm/i915/reset: pass test only parameter to intel_display_reset_finish()

[Matt Roper <matthew.d.roper@xxxxxxxxx>]

On Mon, Mar 03, 2025 at 01:27:09PM +0200, Jani Nikula wrote:
> Deduplicate the gpu_reset_clobbers_display() part by passing the
> information in from gt side.
> 
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_display_reset.c | 12 ++----------
>  drivers/gpu/drm/i915/display/intel_display_reset.h |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c              |  2 +-
>  3 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.c b/drivers/gpu/drm/i915/display/intel_display_reset.c
> index c48d822db58e..d5ce0ac43377 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reset.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.c
> @@ -14,14 +14,6 @@
>  #include "intel_hotplug.h"
>  #include "intel_pps.h"
>  
> -static bool gpu_reset_clobbers_display(struct intel_display *display)
> -{
> -	struct drm_i915_private *i915 = to_i915(display->drm);
> -
> -	return (INTEL_INFO(i915)->gpu_reset_clobbers_display &&
> -		intel_has_gpu_reset(to_gt(i915)));
> -}
> -
>  bool intel_display_reset_test(struct intel_display *display)
>  {
>  	return display->params.force_reset_modeset_test;
> @@ -83,7 +75,7 @@ bool intel_display_reset_prepare(struct intel_display *display)
>  	return true;
>  }
>  
> -void intel_display_reset_finish(struct intel_display *display)
> +void intel_display_reset_finish(struct intel_display *display, bool test_only)

The name 'test_only' makes me think this is going to be about
DRM_MODE_ATOMIC_TEST_ONLY somehow, rather than "I set a module parameter
to force this, even when it isn't needed, for testing purposes."

Not sure if it's worth renaming or not.  Either way,

Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>


Matt

>  {
>  	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct drm_modeset_acquire_ctx *ctx = &display->restore.reset_ctx;
> @@ -98,7 +90,7 @@ void intel_display_reset_finish(struct intel_display *display)
>  		goto unlock;
>  
>  	/* reset doesn't touch the display */
> -	if (!gpu_reset_clobbers_display(display)) {
> +	if (test_only) {
>  		/* for testing only restore the display */
>  		ret = drm_atomic_helper_commit_duplicated_state(state, ctx);
>  		if (ret) {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_reset.h b/drivers/gpu/drm/i915/display/intel_display_reset.h
> index 311b5af8ca0c..f518147199a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_reset.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_reset.h
> @@ -12,6 +12,6 @@ struct intel_display;
>  
>  bool intel_display_reset_test(struct intel_display *display);
>  bool intel_display_reset_prepare(struct intel_display *display);
> -void intel_display_reset_finish(struct intel_display *display);
> +void intel_display_reset_finish(struct intel_display *display, bool test_only);
>  
>  #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 d4f2829477b4..0f12752d0f24 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1434,7 +1434,7 @@ static void intel_gt_reset_global(struct intel_gt *gt,
>  		intel_gt_reset(gt, engine_mask, reason);
>  
>  		if (reset_display)
> -			intel_display_reset_finish(display);
> +			intel_display_reset_finish(display, !need_display_reset);
>  	}
>  
>  	if (!test_bit(I915_WEDGED, &gt->reset.flags))
> -- 
> 2.39.5
> 

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