On 04/19/2016 05:40 PM, Daniel Vetter wrote:
> On Tue, Apr 19, 2016 at 04:26:35PM +0530, Archit Taneja wrote:
>> Move the drm_connector registration from the encoder(HDMI/DSI etc) drivers
>> to the msm platform driver. This will simplify the task of ensuring that
>> the connectors are registered only after the drm_device itself is
>> registered.
>>
>> The connectors' destroy ops are made to use kzalloc instead of
>> devm_kzalloc to ensure that that the connectors can be successfully
>> unregistered when the msm driver module is removed. The memory for the
>> connectors is unallocated when drm_mode_config_cleanup() is called
>> during either during an error or during driver remove.
>>
>> Signed-off-by: Archit Taneja <architt at codeaurora.org>
>
> There's an in-flight patch series to add a common
> connector_(un)register_all(). Would be good to pick that up and rebase on
> top.
>
> The patch series is from Alexey Brodkin <Alexey.Brodkin at synopsys.com>.
Cool. I'll do that. Thanks for the tip.
Archit
> -Daniel
>
>> ---
>> drivers/gpu/drm/msm/dsi/dsi_manager.c | 27
>> ++++++++--------------
>> drivers/gpu/drm/msm/edp/edp_connector.c | 20 ++++------------
>> drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 17 +++-----------
>> drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c | 16 ++-----------
>> drivers/gpu/drm/msm/msm_drv.c | 15 ++++++++++++
>> 5 files changed, 33 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> index 58ba7ec..c8d1f19 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>> @@ -198,9 +198,13 @@ static enum drm_connector_status
>> dsi_mgr_connector_detect(
>>
>> static void dsi_mgr_connector_destroy(struct drm_connector *connector)
>> {
>> + struct dsi_connector *dsi_connector = to_dsi_connector(connector);
>> +
>> DBG("");
>> - drm_connector_unregister(connector);
>> +
>> drm_connector_cleanup(connector);
>> +
>> + kfree(dsi_connector);
>> }
>>
>> static void dsi_dual_connector_fix_modes(struct drm_connector *connector)
>> @@ -538,12 +542,9 @@ struct drm_connector *msm_dsi_manager_connector_init(u8
>> id)
>> struct dsi_connector *dsi_connector;
>> int ret, i;
>>
>> - dsi_connector = devm_kzalloc(msm_dsi->dev->dev,
>> - sizeof(*dsi_connector), GFP_KERNEL);
>> - if (!dsi_connector) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + dsi_connector = kzalloc(sizeof(*dsi_connector), GFP_KERNEL);
>> + if (!dsi_connector)
>> + return ERR_PTR(-ENOMEM);
>>
>> dsi_connector->id = id;
>>
>> @@ -552,7 +553,7 @@ struct drm_connector *msm_dsi_manager_connector_init(u8
>> id)
>> ret = drm_connector_init(msm_dsi->dev, connector,
>> &dsi_mgr_connector_funcs, DRM_MODE_CONNECTOR_DSI);
>> if (ret)
>> - goto fail;
>> + return ERR_PTR(ret);
>>
>> drm_connector_helper_add(connector, &dsi_mgr_conn_helper_funcs);
>>
>> @@ -565,21 +566,11 @@ struct drm_connector
>> *msm_dsi_manager_connector_init(u8 id)
>> connector->interlace_allowed = 0;
>> connector->doublescan_allowed = 0;
>>
>> - ret = drm_connector_register(connector);
>> - if (ret)
>> - goto fail;
>> -
>> for (i = 0; i < MSM_DSI_ENCODER_NUM; i++)
>> drm_mode_connector_attach_encoder(connector,
>> msm_dsi->encoders[i]);
>>
>> return connector;
>> -
>> -fail:
>> - if (connector)
>> - dsi_mgr_connector_destroy(connector);
>> -
>> - return ERR_PTR(ret);
>> }
>>
>> /* initialize bridge */
>> diff --git a/drivers/gpu/drm/msm/edp/edp_connector.c
>> b/drivers/gpu/drm/msm/edp/edp_connector.c
>> index b4d1b46..72360cd 100644
>> --- a/drivers/gpu/drm/msm/edp/edp_connector.c
>> +++ b/drivers/gpu/drm/msm/edp/edp_connector.c
>> @@ -37,7 +37,7 @@ static void edp_connector_destroy(struct drm_connector
>> *connector)
>> struct edp_connector *edp_connector = to_edp_connector(connector);
>>
>> DBG("");
>> - drm_connector_unregister(connector);
>> +
>> drm_connector_cleanup(connector);
>>
>> kfree(edp_connector);
>> @@ -124,10 +124,8 @@ struct drm_connector *msm_edp_connector_init(struct
>> msm_edp *edp)
>> int ret;
>>
>> edp_connector = kzalloc(sizeof(*edp_connector), GFP_KERNEL);
>> - if (!edp_connector) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + if (!edp_connector)
>> + return ERR_PTR(-ENOMEM);
>>
>> edp_connector->edp = edp;
>>
>> @@ -136,7 +134,7 @@ struct drm_connector *msm_edp_connector_init(struct
>> msm_edp *edp)
>> ret = drm_connector_init(edp->dev, connector, &edp_connector_funcs,
>> DRM_MODE_CONNECTOR_eDP);
>> if (ret)
>> - goto fail;
>> + return ERR_PTR(ret);
>>
>> drm_connector_helper_add(connector, &edp_connector_helper_funcs);
>>
>> @@ -147,17 +145,7 @@ struct drm_connector *msm_edp_connector_init(struct
>> msm_edp *edp)
>> connector->interlace_allowed = false;
>> connector->doublescan_allowed = false;
>>
>> - ret = drm_connector_register(connector);
>> - if (ret)
>> - goto fail;
>> -
>> drm_mode_connector_attach_encoder(connector, edp->encoder);
>>
>> return connector;
>> -
>> -fail:
>> - if (connector)
>> - edp_connector_destroy(connector);
>> -
>> - return ERR_PTR(ret);
>> }
>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
>> b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
>> index ce86117..9cc84ce 100644
>> --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
>> @@ -346,7 +346,6 @@ static void hdmi_connector_destroy(struct drm_connector
>> *connector)
>>
>> hdp_disable(hdmi_connector);
>>
>> - drm_connector_unregister(connector);
>> drm_connector_cleanup(connector);
>>
>> kfree(hdmi_connector);
>> @@ -438,10 +437,8 @@ struct drm_connector *msm_hdmi_connector_init(struct
>> hdmi *hdmi)
>> int ret;
>>
>> hdmi_connector = kzalloc(sizeof(*hdmi_connector), GFP_KERNEL);
>> - if (!hdmi_connector) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + if (!hdmi_connector)
>> + return ERR_PTR(-ENOMEM);
>>
>> hdmi_connector->hdmi = hdmi;
>> INIT_WORK(&hdmi_connector->hpd_work, msm_hdmi_hotplug_work);
>> @@ -458,21 +455,13 @@ struct drm_connector *msm_hdmi_connector_init(struct
>> hdmi *hdmi)
>> connector->interlace_allowed = 0;
>> connector->doublescan_allowed = 0;
>>
>> - drm_connector_register(connector);
>> -
>> ret = hpd_enable(hdmi_connector);
>> if (ret) {
>> dev_err(&hdmi->pdev->dev, "failed to enable HPD: %d\n", ret);
>> - goto fail;
>> + return ERR_PTR(ret);
>> }
>>
>> drm_mode_connector_attach_encoder(connector, hdmi->encoder);
>>
>> return connector;
>> -
>> -fail:
>> - if (connector)
>> - hdmi_connector_destroy(connector);
>> -
>> - return ERR_PTR(ret);
>> }
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
>> b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
>> index e73e174..2648cd7 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
>> @@ -48,7 +48,6 @@ static void mdp4_lvds_connector_destroy(struct
>> drm_connector *connector)
>> struct mdp4_lvds_connector *mdp4_lvds_connector =
>> to_mdp4_lvds_connector(connector);
>>
>> - drm_connector_unregister(connector);
>> drm_connector_cleanup(connector);
>>
>> kfree(mdp4_lvds_connector);
>> @@ -121,13 +120,10 @@ struct drm_connector *mdp4_lvds_connector_init(struct
>> drm_device *dev,
>> {
>> struct drm_connector *connector = NULL;
>> struct mdp4_lvds_connector *mdp4_lvds_connector;
>> - int ret;
>>
>> mdp4_lvds_connector = kzalloc(sizeof(*mdp4_lvds_connector), GFP_KERNEL);
>> - if (!mdp4_lvds_connector) {
>> - ret = -ENOMEM;
>> - goto fail;
>> - }
>> + if (!mdp4_lvds_connector)
>> + return ERR_PTR(-ENOMEM);
>>
>> mdp4_lvds_connector->encoder = encoder;
>> mdp4_lvds_connector->panel_node = panel_node;
>> @@ -143,15 +139,7 @@ struct drm_connector *mdp4_lvds_connector_init(struct
>> drm_device *dev,
>> connector->interlace_allowed = 0;
>> connector->doublescan_allowed = 0;
>>
>> - drm_connector_register(connector);
>> -
>> drm_mode_connector_attach_encoder(connector, encoder);
>>
>> return connector;
>> -
>> -fail:
>> - if (connector)
>> - mdp4_lvds_connector_destroy(connector);
>> -
>> - return ERR_PTR(ret);
>> }
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> index c03b967..47f40d9 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -197,6 +197,8 @@ static int msm_unload(struct drm_device *dev)
>>
>> drm_kms_helper_poll_fini(dev);
>>
>> + drm_connector_unregister_all(dev);
>> +
>> #ifdef CONFIG_DRM_FBDEV_EMULATION
>> if (fbdev && priv->fbdev)
>> msm_fbdev_free(dev);
>> @@ -326,6 +328,7 @@ static int msm_load(struct drm_device *dev, unsigned
>> long flags)
>> struct platform_device *pdev = dev->platformdev;
>> struct msm_drm_private *priv;
>> struct msm_kms *kms;
>> + struct drm_connector *connector;
>> int ret;
>>
>> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> @@ -410,6 +413,18 @@ static int msm_load(struct drm_device *dev, unsigned
>> long flags)
>> goto fail;
>> }
>>
>> + mutex_lock(&dev->mode_config.mutex);
>> +
>> + drm_for_each_connector(connector, dev) {
>> + ret = drm_connector_register(connector);
>> + if (ret) {
>> + mutex_unlock(&dev->mode_config.mutex);
>> + goto fail;
>> + }
>> + }
>> +
>> + mutex_unlock(&dev->mode_config.mutex);
>> +
>> drm_mode_config_reset(dev);
>>
>> #ifdef CONFIG_DRM_FBDEV_EMULATION
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
