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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 20 Mar 2026, Michał Grzelak <michal.grzelak@intel.com> 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, 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 <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
>>
>>

-- 
Jani Nikula, Intel