Re: [PATCH 2/2] drm/i915/ddi: Fix cleanup after HDMI connector init failure

[Imre Deak <imre.deak@intel.com>]

On Fri, May 08, 2026 at 05:29:33PM +0300, Hogander, Jouni wrote:
> On Thu, 2026-05-07 at 09:59 +0300, Imre Deak wrote:
> > Perform the missing DDI DP connector cleanup steps after HDMI
> > connector
> > initialization failure during DDI encoder/connector initialization.
> > 
> > This fixes the leaked DP MST encoder, AUX state, and connector
> > object.
> > 
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c | 14 +++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_dp.c  | 13 +++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dp.h  |  3 +++
> >  3 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index e37cc32ee83ed..cd61ddb7f4696 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4729,6 +4729,15 @@ static int intel_ddi_init_dp_connector(struct
> > intel_digital_port *dig_port)
> >  	return 0;
> >  }
> >  
> > +static void intel_ddi_cleanup_dp_connector(struct intel_digital_port
> > *dig_port)
> > +{
> > +	struct intel_dp *intel_dp = &dig_port->dp;
> > +	struct intel_connector *connector = intel_dp-
> > >attached_connector;
> > +
> > +	intel_dp_cleanup_connector(dig_port, connector);
> > +	kfree(connector);
> > +}
> > +
> >  static int intel_hdmi_reset_link(struct intel_encoder *encoder,
> >  				 struct drm_modeset_acquire_ctx
> > *ctx)
> >  {
> > @@ -5492,11 +5501,14 @@ void intel_ddi_init(struct intel_display
> > *display,
> >  	 */
> >  	if (encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
> >  		if (intel_ddi_init_hdmi_connector(dig_port))
> > -			goto err_dp_connector_init;
> > +			goto err_hdmi_connector_init;
> >  	}
> >  
> >  	return;
> >  
> > +err_hdmi_connector_init:
> > +	if (init_dp)
> > +		intel_ddi_cleanup_dp_connector(dig_port);
> >  err_dp_connector_init:
> >  	if (intel_encoder_is_tc(encoder))
> >  		intel_tc_port_cleanup(dig_port);
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 86123614b7bae..97c572e5a5710 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -7346,6 +7346,19 @@ intel_dp_init_connector(struct
> > intel_digital_port *dig_port,
> >  	return false;
> >  }
> >  
> > +void intel_dp_cleanup_connector(struct intel_digital_port *dig_port,
> > +				struct intel_connector *connector)
> > +{
> > +	struct intel_display *display = to_intel_display(connector);
> > +	struct intel_dp *intel_dp = &dig_port->dp;
> > +
> > +	intel_display_power_flush_work(display);
> 
> Why do you need to have this as this is targeted for clean-up on error
> case during initialization phase. I wouldn't expect power get/put async
> at that point?

intel_edp_init_connector() does AUX accesses required for its
initialization. That in turn takes power references which are put in a
deferred way, putting the reference requiring the encoder object. The
above call ensures that all such power references are dropped and the
currepsonding put operation is not called after the encoder object is
freed.

> 
> BR,
> Jouni Högander
> 
> > +
> > +	intel_dp_mst_encoder_cleanup(dig_port);
> > +	intel_dp_aux_fini(intel_dp);
> > +	drm_connector_cleanup(&connector->base);
> > +}
> > +
> >  void intel_dp_mst_suspend(struct intel_display *display)
> >  {
> >  	struct intel_encoder *encoder;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> > b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 2849b9ecdc71a..f41480d247142 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -48,6 +48,9 @@ intel_dp_queue_modeset_retry_for_link(struct
> > intel_atomic_state *state,
> >  				      const struct intel_crtc_state
> > *crtc_state);
> >  bool intel_dp_init_connector(struct intel_digital_port *dig_port,
> >  			     struct intel_connector
> > *intel_connector);
> > +void intel_dp_cleanup_connector(struct intel_digital_port *dig_port,
> > +				struct intel_connector *connector);
> > +
> >  void intel_dp_connector_sync_state(struct intel_connector
> > *connector,
> >  				   const struct intel_crtc_state
> > *crtc_state);
> >  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>