On Tue, 09 Oct 2018, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Oct 09, 2018 at 05:13:36PM +0300, Jani Nikula wrote:
>> On Tue, 09 Oct 2018, Jani Nikula <jani.nikula@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> > ---
>> > 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@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
