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

[Imre Deak <imre.deak@intel.com>]

On Fri, Mar 20, 2026 at 11:20:28AM +0100, Michał Grzelak wrote:
> 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,

Yes, it's used for readability before returns.

> 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
> > 
> >