Hi Imre, Thanks for the review.
On Fri, 5 Jun 2026 at 22:36, Imre Deak <[email protected]> wrote: > > 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 <[email protected]> > > --- > > 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 > > Agreed, clearing the blob reference pointers in intel_crtc_free_hw_state() makes the cleanup safe against repeated calls and avoids changing the ordering in intel_crtc_prepare_cleared_state(). I'll send a v2 with that change. Best regards, Guangshuo
