All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: add a common connector type independent destroy hook
@ 2018-10-09 14:11 Jani Nikula
  2018-10-09 14:13 ` Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2018-10-09 14:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

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,
 	.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,
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: add a common connector type independent destroy hook
  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
  2018-10-09 14:29   ` Ville Syrjälä
  2018-10-09 14:34 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-10-09 17:29 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2018-10-09 14:13 UTC (permalink / raw)
  To: intel-gfx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: add a common connector type independent destroy hook
  2018-10-09 14:13 ` Jani Nikula
@ 2018-10-09 14:29   ` Ville Syrjälä
  2018-10-09 14:46     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2018-10-09 14:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Oct 09, 2018 at 05:13:36PM +0300, Jani Nikula wrote:
> 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.

On account of sdvo_connector having its own struct? I think we can rely
on the offsetof(base)==0 trick. That assumption is baked in all over the
place (should really resurrect my BUILD_BUG_ON() patches to make sure).

On a related note, https://patchwork.freedesktop.org/patch/250038/ would
be nice to get in ;)

Also looks like lvds is in the same boat as sdvo what with both having
their own connector struct. The difference is that in the case of lvds
that custom struct is entirely pointless.

Patch is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> 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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: add a common connector type independent destroy hook
  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
@ 2018-10-09 14:34 ` Patchwork
  2018-10-09 17:29 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-10-09 14:34 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: add a common connector type independent destroy hook
URL   : https://patchwork.freedesktop.org/series/50752/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4953 -> Patchwork_10400 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50752/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10400 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@prime_vgem@basic-fence-flip:
      fi-cfl-8700k:       PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-ivb-3520m:       FAIL (fdo#103375) -> PASS +2
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (48 -> 42) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4953 -> Patchwork_10400

  CI_DRM_4953: b99ae4fdd4e03fd206144caff6a4c58ce6ca4866 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10400: e658bba7c62cbdc8ef287643f584a109257abe30 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e658bba7c62c drm/i915: add a common connector type independent destroy hook

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10400/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: add a common connector type independent destroy hook
  2018-10-09 14:29   ` Ville Syrjälä
@ 2018-10-09 14:46     ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2018-10-09 14:46 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 09 Oct 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Oct 09, 2018 at 05:13:36PM +0300, Jani Nikula wrote:
>> 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.
>
> On account of sdvo_connector having its own struct? I think we can rely
> on the offsetof(base)==0 trick. That assumption is baked in all over the
> place (should really resurrect my BUILD_BUG_ON() patches to make sure).

Okay, I'll go with that.

> On a related note, https://patchwork.freedesktop.org/patch/250038/ would
> be nice to get in ;)
>
> Also looks like lvds is in the same boat as sdvo what with both having
> their own connector struct. The difference is that in the case of lvds
> that custom struct is entirely pointless.

I have a follow-up to nuke that.

> Patch is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks, will push once I get CI results.

BR,
Jani.


>
>> 
>> 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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: add a common connector type independent destroy hook
  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
  2018-10-09 14:34 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-10-09 17:29 ` Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-10-09 17:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: add a common connector type independent destroy hook
URL   : https://patchwork.freedesktop.org/series/50752/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4953_full -> Patchwork_10400_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10400_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10400_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10400_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10400_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_reloc@basic-softpin:
      shard-apl:          PASS -> DMESG-WARN (fdo#103558, fdo#105602) +1

    igt@gem_exec_schedule@pi-ringfull-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_cursor_crc@cursor-128x42-onscreen:
      shard-apl:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538)

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render:
      shard-skl:          PASS -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167) +3

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-render:
      shard-skl:          PASS -> FAIL (fdo#103167)

    {igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max}:
      shard-glk:          PASS -> FAIL (fdo#108145)

    
    ==== Possible fixes ====

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          INCOMPLETE (fdo#108074) -> PASS

    igt@kms_cursor_crc@cursor-256x85-offscreen:
      shard-skl:          FAIL (fdo#103232) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbcpsr-indfb-scaledprimary:
      shard-skl:          FAIL (fdo#103167) -> PASS

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    {igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}:
      shard-skl:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-hsw:          FAIL (fdo#99912) -> PASS
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@pm_rpm@gem-execbuf-stress:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@pm_rpm@legacy-planes-dpms:
      shard-skl:          INCOMPLETE (fdo#107807, fdo#105959) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4953 -> Patchwork_10400

  CI_DRM_4953: b99ae4fdd4e03fd206144caff6a4c58ce6ca4866 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10400: e658bba7c62cbdc8ef287643f584a109257abe30 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10400/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-09 17:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.