Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: add a common connector type independent destroy hook
Date: Tue, 09 Oct 2018 17:13:36 +0300	[thread overview]
Message-ID: <87lg77nr73.fsf@intel.com> (raw)
In-Reply-To: <20181009141103.20387-1-jani.nikula@intel.com>

On Tue, 09 Oct 2018, Jani Nikula <jani.nikula@intel.com> wrote:
> Almost all of the connector destroy functions do the same thing. The
> differences are in the edid, detect_edid and panel cleanups, but those
> are safely NULL when not initialized. Roll out a common connector
> destroy hook.
>
> Inspired by commit bc3213c44415 ("drm/i915: Drop the eDP check from
> intel_dp_connector_destroy()").
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  8 +-------
>  drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_dp.c      | 18 +-----------------
>  drivers/gpu/drm/i915/intel_dp_mst.c  | 14 +-------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_dvo.c     |  9 +--------
>  drivers/gpu/drm/i915/intel_hdmi.c    |  5 ++---
>  drivers/gpu/drm/i915/intel_lvds.c    | 23 +----------------------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 16 ++++------------
>  drivers/gpu/drm/i915/intel_tv.c      |  9 +--------
>  drivers/gpu/drm/i915/vlv_dsi.c       | 12 +-----------
>  11 files changed, 33 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 0c6bf82bb059..ab3d6b074222 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -849,12 +849,6 @@ intel_crt_detect(struct drm_connector *connector,
>  	return status;
>  }
>  
> -static void intel_crt_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  static int intel_crt_get_modes(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
> @@ -909,7 +903,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_crt_destroy,
> +	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e14e3268a84c..a145efba9157 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6365,7 +6365,7 @@ struct intel_connector *intel_connector_alloc(void)
>   * This should only be used after intel_connector_alloc has returned
>   * successfully, and before drm_connector_init returns successfully.
>   * Otherwise the destroy callbacks for the connector and the state should
> - * take care of proper cleanup/free
> + * take care of proper cleanup/free (see intel_connector_destroy).
>   */
>  void intel_connector_free(struct intel_connector *connector)
>  {
> @@ -6373,6 +6373,24 @@ void intel_connector_free(struct intel_connector *connector)
>  	kfree(connector);
>  }
>  
> +/*
> + * Connector type independent destroy hook for drm_connector_funcs.
> + */
> +void intel_connector_destroy(struct drm_connector *connector)
> +{
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> +	kfree(intel_connector->detect_edid);
> +
> +	if (!IS_ERR_OR_NULL(intel_connector->edid))
> +		kfree(intel_connector->edid);
> +
> +	intel_panel_fini(&intel_connector->panel);
> +
> +	drm_connector_cleanup(connector);
> +	kfree(connector);
> +}
> +
>  /* Simple connector->get_hw_state implementation for encoders that support only
>   * one connector and no cloning and hence the encoder state determines the state
>   * of the connector. */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d12f987a6c43..0855b9785f7b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5251,22 +5251,6 @@ intel_dp_connector_unregister(struct drm_connector *connector)
>  	intel_connector_unregister(connector);
>  }
>  
> -static void
> -intel_dp_connector_destroy(struct drm_connector *connector)
> -{
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -
> -	kfree(intel_connector->detect_edid);
> -
> -	if (!IS_ERR_OR_NULL(intel_connector->edid))
> -		kfree(intel_connector->edid);
> -
> -	intel_panel_fini(&intel_connector->panel);
> -
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  {
>  	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> @@ -5613,7 +5597,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,
>  	.late_register = intel_dp_connector_register,
>  	.early_unregister = intel_dp_connector_unregister,
> -	.destroy = intel_dp_connector_destroy,
> +	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 43db2e9ac575..9ad497e8ae36 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -329,24 +329,12 @@ intel_dp_mst_detect(struct drm_connector *connector, bool force)
>  	return drm_dp_mst_detect_port(connector, &intel_dp->mst_mgr, intel_connector->port);
>  }
>  
> -static void
> -intel_dp_mst_connector_destroy(struct drm_connector *connector)
> -{
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -
> -	if (!IS_ERR_OR_NULL(intel_connector->edid))
> -		kfree(intel_connector->edid);
> -
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
>  	.detect = intel_dp_mst_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_dp_mst_connector_destroy,
> +	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8050d06c722a..4b8fec74ad49 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1510,6 +1510,7 @@ void intel_encoder_destroy(struct drm_encoder *encoder);
>  int intel_connector_init(struct intel_connector *);
>  struct intel_connector *intel_connector_alloc(void);
>  void intel_connector_free(struct intel_connector *connector);
> +void intel_connector_destroy(struct drm_connector *connector);
>  bool intel_connector_get_hw_state(struct intel_connector *connector);
>  void intel_connector_attach_encoder(struct intel_connector *connector,
>  				    struct intel_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 4e142ff49708..be3c0a5f447d 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -333,18 +333,11 @@ static int intel_dvo_get_modes(struct drm_connector *connector)
>  	return 0;
>  }
>  
> -static void intel_dvo_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -	intel_panel_fini(&to_intel_connector(connector)->panel);
> -	kfree(connector);
> -}
> -
>  static const struct drm_connector_funcs intel_dvo_connector_funcs = {
>  	.detect = intel_dvo_detect,
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_dvo_destroy,
> +	.destroy = intel_connector_destroy,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 454f570275e9..2c53efc463e6 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2073,9 +2073,8 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
>  {
>  	if (intel_attached_hdmi(connector)->cec_notifier)
>  		cec_notifier_put(intel_attached_hdmi(connector)->cec_notifier);
> -	kfree(to_intel_connector(connector)->detect_edid);
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> +
> +	intel_connector_destroy(connector);
>  }
>  
>  static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index f9f3b0885ba5..1fe970cf9909 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -477,27 +477,6 @@ static int intel_lvds_get_modes(struct drm_connector *connector)
>  	return 1;
>  }
>  
> -/**
> - * intel_lvds_destroy - unregister and free LVDS structures
> - * @connector: connector to free
> - *
> - * Unregister the DDC bus for this connector then free the driver private
> - * structure.
> - */
> -static void intel_lvds_destroy(struct drm_connector *connector)
> -{
> -	struct intel_lvds_connector *lvds_connector =
> -		to_lvds_connector(connector);
> -
> -	if (!IS_ERR_OR_NULL(lvds_connector->base.edid))
> -		kfree(lvds_connector->base.edid);
> -
> -	intel_panel_fini(&lvds_connector->base.panel);
> -
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
>  	.get_modes = intel_lvds_get_modes,
>  	.mode_valid = intel_lvds_mode_valid,
> @@ -511,7 +490,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_lvds_destroy,
> +	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 701372e512a8..1824d94ae123 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2058,14 +2058,6 @@ static int intel_sdvo_get_modes(struct drm_connector *connector)
>  	return !list_empty(&connector->probed_modes);
>  }
>  
> -static void intel_sdvo_destroy(struct drm_connector *connector)
> -{
> -	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
> -
> -	drm_connector_cleanup(connector);
> -	kfree(intel_sdvo_connector);
> -}
> -
>  static int
>  intel_sdvo_connector_atomic_get_property(struct drm_connector *connector,
>  					 const struct drm_connector_state *state,
> @@ -2228,7 +2220,7 @@ static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
>  	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
>  	.late_register = intel_sdvo_connector_register,
>  	.early_unregister = intel_sdvo_connector_unregister,
> -	.destroy = intel_sdvo_destroy,
> +	.destroy = intel_connector_destroy,

Okay, this works by accident. I'll probably drop the sdvo hunks.

BR,
Jani.

>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = intel_sdvo_connector_duplicate_state,
>  };
> @@ -2583,7 +2575,7 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>  	return true;
>  
>  err:
> -	intel_sdvo_destroy(connector);
> +	intel_connector_destroy(connector);
>  	return false;
>  }
>  
> @@ -2675,7 +2667,7 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  	return true;
>  
>  err:
> -	intel_sdvo_destroy(connector);
> +	intel_connector_destroy(connector);
>  	return false;
>  }
>  
> @@ -2745,7 +2737,7 @@ static void intel_sdvo_output_cleanup(struct intel_sdvo *intel_sdvo)
>  				 &dev->mode_config.connector_list, head) {
>  		if (intel_attached_encoder(connector) == &intel_sdvo->base) {
>  			drm_connector_unregister(connector);
> -			intel_sdvo_destroy(connector);
> +			intel_connector_destroy(connector);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index b5b04cb892e9..8b9ce0dc78e5 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1377,17 +1377,10 @@ intel_tv_get_modes(struct drm_connector *connector)
>  	return count;
>  }
>  
> -static void
> -intel_tv_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  static const struct drm_connector_funcs intel_tv_connector_funcs = {
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_tv_destroy,
> +	.destroy = intel_connector_destroy,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index 435a2c35ee8c..5accd0c360f9 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -1642,16 +1642,6 @@ static int intel_dsi_get_modes(struct drm_connector *connector)
>  	return 1;
>  }
>  
> -static void intel_dsi_connector_destroy(struct drm_connector *connector)
> -{
> -	struct intel_connector *intel_connector = to_intel_connector(connector);
> -
> -	DRM_DEBUG_KMS("\n");
> -	intel_panel_fini(&intel_connector->panel);
> -	drm_connector_cleanup(connector);
> -	kfree(connector);
> -}
> -
>  static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
>  {
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> @@ -1676,7 +1666,7 @@ static const struct drm_connector_helper_funcs intel_dsi_connector_helper_funcs
>  static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>  	.late_register = intel_connector_register,
>  	.early_unregister = intel_connector_unregister,
> -	.destroy = intel_dsi_connector_destroy,
> +	.destroy = intel_connector_destroy,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_get_property = intel_digital_connector_atomic_get_property,
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-10-09 14:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-09 14:11 [PATCH] drm/i915: add a common connector type independent destroy hook Jani Nikula
2018-10-09 14:13 ` Jani Nikula [this message]
2018-10-09 14:29   ` Ville Syrjälä
2018-10-09 14:46     ` Jani Nikula
2018-10-09 14:34 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-10-09 17:29 ` ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lg77nr73.fsf@intel.com \
    --to=jani.nikula@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox