Re: [PATCH] drm/i915/dp_tunnel: Fix error handling when clearing stream BW in atomic state

["Michał Grzelak" <michal.grzelak@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5225 bytes --]

On Fri, 20 Mar 2026, Imre Deak wrote:
> Clearing the DP tunnel stream BW in the atomic state involves getting
> the tunnel group state, which can fail. Handle the error accordingly.
>
> This fixes at least one issue where drm_dp_tunnel_atomic_set_stream_bw()
> failed to get the tunnel group state returning -EDEADLK, which wasn't
> handled. This lead to the ctx->contended warn later in modeset_lock()
> while taking a WW mutex for another object in the same atomic state, and
> thus within the same already contended WW context.
>
> Moving intel_crtc_state_alloc() later would avoid freeing saved_state on
> the error path; this stable patch leaves that simplification for a
> follow-up.
>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v6.9+
> Fixes: a4efae87ecb2 ("drm/i915/dp: Compute DP tunnel BW during encoder state computation")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c  |  8 +++++++-
> .../gpu/drm/i915/display/intel_dp_tunnel.c    | 20 +++++++++++++------
> .../gpu/drm/i915/display/intel_dp_tunnel.h    | 11 ++++++----
> 3 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ee501009a251f..882db77c0bbcd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4640,6 +4640,7 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
> 	struct intel_crtc_state *crtc_state =
> 		intel_atomic_get_new_crtc_state(state, crtc);
> 	struct intel_crtc_state *saved_state;
> +	int err;
>
> 	saved_state = intel_crtc_state_alloc(crtc);
> 	if (!saved_state)
> @@ -4648,7 +4649,12 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
> 	/* free the old crtc_state->hw members */
> 	intel_crtc_free_hw_state(crtc_state);
>
> -	intel_dp_tunnel_atomic_clear_stream_bw(state, crtc_state);
> +	err = intel_dp_tunnel_atomic_clear_stream_bw(state, crtc_state);
> +	if (err) {
> +		kfree(saved_state);
> +

I am unsure if the blank line above is neccessary, but I might be also
missing style guidelines. Otherwise looks good to me.

Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>

BR,
Michał

> +		return err;
> +	}
>
> 	/* FIXME: before the switch to atomic started, a new pipe_config was
> 	 * kzalloc'd. Code that depends on any field being zero should be
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_tunnel.c b/drivers/gpu/drm/i915/display/intel_dp_tunnel.c
> index 1fd1ac8d556d8..7363c98172971 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_tunnel.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_tunnel.c
> @@ -659,19 +659,27 @@ int intel_dp_tunnel_atomic_compute_stream_bw(struct intel_atomic_state *state,
>  *
>  * Clear any DP tunnel stream BW requirement set by
>  * intel_dp_tunnel_atomic_compute_stream_bw().
> + *
> + * Returns 0 in case of success, a negative error code otherwise.
>  */
> -void intel_dp_tunnel_atomic_clear_stream_bw(struct intel_atomic_state *state,
> -					    struct intel_crtc_state *crtc_state)
> +int intel_dp_tunnel_atomic_clear_stream_bw(struct intel_atomic_state *state,
> +					   struct intel_crtc_state *crtc_state)
> {
> 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	int err;
>
> 	if (!crtc_state->dp_tunnel_ref.tunnel)
> -		return;
> +		return 0;
> +
> +	err = drm_dp_tunnel_atomic_set_stream_bw(&state->base,
> +						 crtc_state->dp_tunnel_ref.tunnel,
> +						 crtc->pipe, 0);
> +	if (err)
> +		return err;
>
> -	drm_dp_tunnel_atomic_set_stream_bw(&state->base,
> -					   crtc_state->dp_tunnel_ref.tunnel,
> -					   crtc->pipe, 0);
> 	drm_dp_tunnel_ref_put(&crtc_state->dp_tunnel_ref);
> +
> +	return 0;
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_tunnel.h b/drivers/gpu/drm/i915/display/intel_dp_tunnel.h
> index 7f0f720e8dcad..10ab9eebcef69 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_tunnel.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp_tunnel.h
> @@ -40,8 +40,8 @@ int intel_dp_tunnel_atomic_compute_stream_bw(struct intel_atomic_state *state,
> 					     struct intel_dp *intel_dp,
> 					     const struct intel_connector *connector,
> 					     struct intel_crtc_state *crtc_state);
> -void intel_dp_tunnel_atomic_clear_stream_bw(struct intel_atomic_state *state,
> -					    struct intel_crtc_state *crtc_state);
> +int intel_dp_tunnel_atomic_clear_stream_bw(struct intel_atomic_state *state,
> +					   struct intel_crtc_state *crtc_state);
>
> int intel_dp_tunnel_atomic_add_state_for_crtc(struct intel_atomic_state *state,
> 					      struct intel_crtc *crtc);
> @@ -88,9 +88,12 @@ intel_dp_tunnel_atomic_compute_stream_bw(struct intel_atomic_state *state,
> 	return 0;
> }
>
> -static inline void
> +static inline int
> intel_dp_tunnel_atomic_clear_stream_bw(struct intel_atomic_state *state,
> -				       struct intel_crtc_state *crtc_state) {}
> +				       struct intel_crtc_state *crtc_state)
> +{
> +	return 0;
> +}
>
> static inline int
> intel_dp_tunnel_atomic_add_state_for_crtc(struct intel_atomic_state *state,
> -- 
> 2.49.1
>
>