[Intel-gfx] [PATCH] drm/i915: add a common connector type independent destroy hook
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Oct 9 14:29:20 UTC 2018
On Tue, Oct 09, 2018 at 05:13:36PM +0300, Jani Nikula wrote:
> On Tue, 09 Oct 2018, Jani Nikula <jani.nikula at 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 at linux.intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula at 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 at 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
More information about the Intel-gfx
mailing list