Re: [PATCH 02/12] drm/i915: Always initialize connector->modeset_retry_work

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

On Thu, 13 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Since we have all the necessary bits in intel_connector.c might
> as well always initialize the modeset_retry_work for every connector.
> Avoids yet another init function you have to remember to call.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_connector.c | 9 +++------
>  drivers/gpu/drm/i915/display/intel_connector.h | 1 -
>  drivers/gpu/drm/i915/display/intel_display.c   | 3 +--
>  drivers/gpu/drm/i915/display/intel_dp.c        | 3 ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c    | 2 --
>  5 files changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 4f54c9249a84..b1240be2ec6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -70,12 +70,6 @@ void intel_connector_queue_modeset_retry_work(struct intel_connector *connector)
>  		drm_connector_put(&connector->base);
>  }
>  
> -void intel_connector_init_modeset_retry_work(struct intel_connector *connector)
> -{
> -	INIT_WORK(&connector->modeset_retry_work,
> -		  intel_connector_modeset_retry_work_fn);
> -}
> -
>  int intel_connector_init(struct intel_connector *connector)
>  {
>  	struct intel_digital_connector_state *conn_state;
> @@ -95,6 +89,9 @@ int intel_connector_init(struct intel_connector *connector)
>  
>  	intel_panel_init_alloc(connector);
>  
> +	INIT_WORK(&connector->modeset_retry_work,
> +		  intel_connector_modeset_retry_work_fn);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h b/drivers/gpu/drm/i915/display/intel_connector.h
> index 10a1779201ed..aafb25a814fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.h
> +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> @@ -33,7 +33,6 @@ void intel_attach_aspect_ratio_property(struct drm_connector *connector);
>  void intel_attach_hdmi_colorspace_property(struct drm_connector *connector);
>  void intel_attach_dp_colorspace_property(struct drm_connector *connector);
>  void intel_attach_scaling_mode_property(struct drm_connector *connector);
> -void intel_connector_init_modeset_retry_work(struct intel_connector *connector);
>  void intel_connector_queue_modeset_retry_work(struct intel_connector *connector);
>  void intel_connector_cancel_modeset_retry_work(struct intel_connector *connector);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cc51576353fe..858886eea6cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8787,8 +8787,7 @@ void intel_hpd_poll_fini(struct drm_i915_private *i915)
>  	/* Kill all the work that may have been queued by hpd. */
>  	drm_connector_list_iter_begin(&i915->drm, &conn_iter);
>  	for_each_intel_connector_iter(connector, &conn_iter) {
> -		if (connector->modeset_retry_work.func &&
> -		    cancel_work_sync(&connector->modeset_retry_work))
> +		if (cancel_work_sync(&connector->modeset_retry_work))
>  			drm_connector_put(&connector->base);
>  		if (connector->hdcp.shim) {
>  			cancel_delayed_work_sync(&connector->hdcp.check_work);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8bcf400b0239..03ca2e02ab02 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6487,9 +6487,6 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>  	enum port port = encoder->port;
>  	int type;
>  
> -	/* Initialize the work for modeset in case of link train failure */
> -	intel_connector_init_modeset_retry_work(connector);
> -
>  	if (drm_WARN(dev, dig_port->max_lanes < 1,
>  		     "Not enough lanes (%d) for DP on [ENCODER:%d:%s]\n",
>  		     dig_port->max_lanes, encoder->base.base.id,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 0044b2a9a16b..167e4a70ab12 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1747,8 +1747,6 @@ mst_topology_add_connector(struct drm_dp_mst_topology_mgr *mgr,
>  	connector->port = port;
>  	drm_dp_mst_get_port_malloc(port);
>  
> -	intel_connector_init_modeset_retry_work(connector);
> -
>  	ret = drm_connector_dynamic_init(display->drm, &connector->base, &mst_connector_funcs,
>  					 DRM_MODE_CONNECTOR_DisplayPort, NULL);
>  	if (ret)

-- 
Jani Nikula, Intel