[PATCH] drm/i915/dp_tunnel: fix CRTC hw state cleanup ordering

[PATCH] drm/i915/dp_tunnel: fix CRTC hw state cleanup ordering

[Guangshuo Li]

intel_crtc_prepare_cleared_state() frees the old CRTC hw state before
calling intel_dp_tunnel_atomic_clear_stream_bw(). The latter can fail
while looking up the DP tunnel group state, for example with -EDEADLK.

If that happens, the function returns without completing the cleared
state preparation, leaving the duplicated CRTC state with its hw color
blob references already dropped. The failed atomic state will then be
cleared by the atomic core, which can drop the same references again.

Move the hw state release after the DP tunnel stream BW clear has
succeeded. Keep the temporary state allocation before the failable DP
tunnel operation so an allocation failure cannot happen after the stream
BW has already been cleared.

Fixes: fb69d0076e68 ("drm/i915/dp_tunnel: Fix error handling when clearing stream BW in atomic state")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ad2fe10b6b1f..0c015bc6c9fd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4646,9 +4646,6 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
 	if (!saved_state)
 		return -ENOMEM;
 
-	/* free the old crtc_state->hw members */
-	intel_crtc_free_hw_state(crtc_state);
-
 	err = intel_dp_tunnel_atomic_clear_stream_bw(state, crtc_state);
 	if (err) {
 		kfree(saved_state);
@@ -4656,6 +4653,9 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
 		return err;
 	}
 
+	/* free the old crtc_state->hw members */
+	intel_crtc_free_hw_state(crtc_state);
+
 	/* FIXME: before the switch to atomic started, a new pipe_config was
 	 * kzalloc'd. Code that depends on any field being zero should be
 	 * fixed, so that the crtc_state can be safely duplicated. For now,
-- 
2.43.0

Re: [PATCH] drm/i915/dp_tunnel: fix CRTC hw state cleanup ordering

[Imre Deak]

On Fri, Jun 05, 2026 at 02:05:27PM +0800, Guangshuo Li wrote:
> intel_crtc_prepare_cleared_state() frees the old CRTC hw state before
> calling intel_dp_tunnel_atomic_clear_stream_bw(). The latter can fail
> while looking up the DP tunnel group state, for example with -EDEADLK.
> 
> If that happens, the function returns without completing the cleared
> state preparation, leaving the duplicated CRTC state with its hw color
> blob references already dropped. The failed atomic state will then be
> cleared by the atomic core, which can drop the same references again.
> 
> Move the hw state release after the DP tunnel stream BW clear has
> succeeded. Keep the temporary state allocation before the failable DP
> tunnel operation so an allocation failure cannot happen after the stream
> BW has already been cleared.
> 
> Fixes: fb69d0076e68 ("drm/i915/dp_tunnel: Fix error handling when clearing stream BW in atomic state")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ad2fe10b6b1f..0c015bc6c9fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4646,9 +4646,6 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
>  	if (!saved_state)
>  		return -ENOMEM;
>  
> -	/* free the old crtc_state->hw members */
> -	intel_crtc_free_hw_state(crtc_state);

The fix is valid, however I think instead of this change it'd be better
to clear the blob reference pointers in intel_crtc_free_hw_state().

> -
>  	err = intel_dp_tunnel_atomic_clear_stream_bw(state, crtc_state);
>  	if (err) {
>  		kfree(saved_state);
> @@ -4656,6 +4653,9 @@ intel_crtc_prepare_cleared_state(struct intel_atomic_state *state,
>  		return err;
>  	}
>  
> +	/* free the old crtc_state->hw members */
> +	intel_crtc_free_hw_state(crtc_state);
> +
>  	/* FIXME: before the switch to atomic started, a new pipe_config was
>  	 * kzalloc'd. Code that depends on any field being zero should be
>  	 * fixed, so that the crtc_state can be safely duplicated. For now,
> -- 
> 2.43.0
>