From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 300BD108B901 for ; Fri, 20 Mar 2026 11:19:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 93E7A10EAA6; Fri, 20 Mar 2026 11:19:40 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="T21D8vG/"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4091A10E228; Fri, 20 Mar 2026 11:19:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1774005578; x=1805541578; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=l1dhd2+bxgTr6gHHhaRO4PXaDqK4ddAvdqxlmR7YEIQ=; b=T21D8vG/MU6e2TY2xEJm4qv3Rx+jAJqFEq2lZQXYQHFoFPkKzSC3qZDs FiAQyyRsPwAcYpjrb0alTw8LGD6cmKtx81BWKE/OYfq8yjro/UjoCt6w9 ZgLskjXd3ZUIafKMDGsJ6D4hBTFthLfQNKp+xkaPTpTrZNa/iIxOuOUxV wF3CwYINWG/lHioMPlkJtz8uDhi4g5m/Y/cmqxHPhIVUx92Ic1Xvb4Ewu aFMKmeT4/Iz0Vvcac3e4hMzgSth5INl5O+YmTscFflCFwALfCngn6D3Qp RNvH8ye6n6+yszurAAIiZUKKqCJ71mxMKfnuVJYH7ZOkbL3vQLvzUJJMc w==; X-CSE-ConnectionGUID: F3em2WllQqiOIhsrjmbCIw== X-CSE-MsgGUID: /oH0qow0TTy1c/1bbmAzbg== X-IronPort-AV: E=McAfee;i="6800,10657,11734"; a="86450354" X-IronPort-AV: E=Sophos;i="6.23,130,1770624000"; d="scan'208";a="86450354" Received: from fmviesa002.fm.intel.com ([10.60.135.142]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Mar 2026 04:19:38 -0700 X-CSE-ConnectionGUID: Dctu4s2mTD6LuEPH6gmDvQ== X-CSE-MsgGUID: cs26MOWKS6exzCCUqTNgoA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,130,1770624000"; d="scan'208";a="246284632" Received: from jkrzyszt-mobl2.ger.corp.intel.com (HELO localhost) ([10.245.246.197]) by fmviesa002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Mar 2026 04:19:35 -0700 From: Jani Nikula To: =?utf-8?Q?Micha=C5=82?= Grzelak , Imre Deak Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, Uma Shankar , Ville =?utf-8?B?U3lyasOkbMOk?= , stable@vger.kernel.org Subject: Re: [PATCH] drm/i915/dp_tunnel: Fix error handling when clearing stream BW in atomic state In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs Bertel Jungin Aukio 5, 02600 Espoo, Finland References: <20260320092900.13210-1-imre.deak@intel.com> Date: Fri, 20 Mar 2026 13:19:32 +0200 Message-ID: <99351a6c5d3da05614e3132f2dd562a54df5e1c4@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Fri, 20 Mar 2026, Micha=C5=82 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 >> Cc: Ville Syrj=C3=A4l=C3=A4 >> Cc: # v6.9+ >> Fixes: a4efae87ecb2 ("drm/i915/dp: Compute DP tunnel BW during encoder s= tate computation") >> Signed-off-by: Imre Deak >> --- >> 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_atom= ic_state *state, >> struct intel_crtc_state *crtc_state =3D >> intel_atomic_get_new_crtc_state(state, crtc); >> struct intel_crtc_state *saved_state; >> + int err; >> >> saved_state =3D intel_crtc_state_alloc(crtc); >> if (!saved_state) >> @@ -4648,7 +4649,12 @@ intel_crtc_prepare_cleared_state(struct intel_ato= mic_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 =3D 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=C5=82 Grzelak > > BR, > Micha=C5=82 > >> + 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/gp= u/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(struc= t 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 *s= tate, >> + struct intel_crtc_state *crtc_state) >> { >> struct intel_crtc *crtc =3D to_intel_crtc(crtc_state->uapi.crtc); >> + int err; >> >> if (!crtc_state->dp_tunnel_ref.tunnel) >> - return; >> + return 0; >> + >> + err =3D 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/gp= u/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 in= tel_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 *s= tate, >> + 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 *sta= te, >> --=20 >> 2.49.1 >> >> --=20 Jani Nikula, Intel