On Fri, 20 Mar 2026, Michał Grzelak <[email protected]> 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 <[email protected]> >> Cc: Ville Syrjälä <[email protected]> >> Cc: <[email protected]> # v6.9+ >> Fixes: a4efae87ecb2 ("drm/i915/dp: Compute DP tunnel BW during encoder state >> computation") >> Signed-off-by: Imre Deak <[email protected]> >> --- >> 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.
It's a common convention to have a blank line before return. BR, Jani. > > Reviewed-by: Michał Grzelak <[email protected]> > > 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 >> >> -- Jani Nikula, Intel
