Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757571AbcJXO2R (ORCPT ); Mon, 24 Oct 2016 10:28:17 -0400 Received: from foss.arm.com ([217.140.101.70]:53688 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751352AbcJXO2P (ORCPT ); Mon, 24 Oct 2016 10:28:15 -0400 From: Brian Starkey To: rmk+kernel@armlinux.org.uk, dri-devel@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org Subject: [PATCH] drm: tda998x: mali-dp: hdlcd: refactor connector registration Date: Mon, 24 Oct 2016 15:27:59 +0100 Message-Id: <1477319279-21726-1-git-send-email-brian.starkey@arm.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <20161024142312.GA1988@e106950-lin.cambridge.arm.com> References: <20161024142312.GA1988@e106950-lin.cambridge.arm.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5753 Lines: 189 Connectors shouldn't be registered until the rest of the whole device is set up, so that consistent state is presented to userspace. As such, remove the calls to drm_connector_register() and drm_connector_unregister() from tda998x, as these are now handled by drm_dev_(un)register() itself. To work with this change, the mali-dp and hdlcd bind and unbind sequences have to be reordered, to ensure that the componentised encoder/connector is bound before drm_dev_register() registers all connectors. Similarly, the device must be unregistered before the component is unbound. Altogether, this allows other drivers using tda998x to be de-midlayered, and to have less racy initialisation of their components. Splitting this commit into three (one per driver) isn't possible without intermediate breakage, so it is all squashed together here. Suggested-by: Russell King Signed-off-by: Brian Starkey Reviewed-by: Liviu Dudau --- drivers/gpu/drm/arm/hdlcd_drv.c | 19 +++++++++++-------- drivers/gpu/drm/arm/malidp_drv.c | 18 +++++++++++------- drivers/gpu/drm/i2c/tda998x_drv.c | 8 -------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index d83b46a..85aec8b 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -337,14 +337,10 @@ static int hdlcd_drm_bind(struct device *dev) if (ret) goto err_free; - ret = drm_dev_register(drm, 0); - if (ret) - goto err_unload; - ret = component_bind_all(dev, drm); if (ret) { DRM_ERROR("Failed to bind all components\n"); - goto err_unregister; + goto err_unload; } ret = pm_runtime_set_active(dev); @@ -371,8 +367,17 @@ static int hdlcd_drm_bind(struct device *dev) goto err_fbdev; } + ret = drm_dev_register(drm, 0); + if (ret) + goto err_register; + return 0; +err_register: + if (hdlcd->fbdev) { + drm_fbdev_cma_fini(hdlcd->fbdev); + hdlcd->fbdev = NULL; + } err_fbdev: drm_kms_helper_poll_fini(drm); drm_mode_config_cleanup(drm); @@ -381,8 +386,6 @@ err_vblank: pm_runtime_disable(drm->dev); err_pm_active: component_unbind_all(dev, drm); -err_unregister: - drm_dev_unregister(drm); err_unload: drm_irq_uninstall(drm); of_reserved_mem_device_release(drm->dev); @@ -398,6 +401,7 @@ static void hdlcd_drm_unbind(struct device *dev) struct drm_device *drm = dev_get_drvdata(dev); struct hdlcd_drm_private *hdlcd = drm->dev_private; + drm_dev_unregister(drm); if (hdlcd->fbdev) { drm_fbdev_cma_fini(hdlcd->fbdev); hdlcd->fbdev = NULL; @@ -411,7 +415,6 @@ static void hdlcd_drm_unbind(struct device *dev) pm_runtime_disable(drm->dev); of_reserved_mem_device_release(drm->dev); drm_mode_config_cleanup(drm); - drm_dev_unregister(drm); drm_dev_unref(drm); drm->dev_private = NULL; dev_set_drvdata(dev, NULL); diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 82171d2..79bfc13 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -354,10 +354,6 @@ static int malidp_bind(struct device *dev) if (ret < 0) goto init_fail; - ret = drm_dev_register(drm, 0); - if (ret) - goto register_fail; - /* Set the CRTC's port so that the encoder component can find it */ ep = of_graph_get_next_endpoint(dev->of_node, NULL); if (!ep) { @@ -394,8 +390,18 @@ static int malidp_bind(struct device *dev) } drm_kms_helper_poll_init(drm); + + ret = drm_dev_register(drm, 0); + if (ret) + goto register_fail; + return 0; +register_fail: + if (malidp->fbdev) { + drm_fbdev_cma_fini(malidp->fbdev); + malidp->fbdev = NULL; + } fbdev_fail: drm_vblank_cleanup(drm); vblank_fail: @@ -407,8 +413,6 @@ bind_fail: of_node_put(malidp->crtc.port); malidp->crtc.port = NULL; port_fail: - drm_dev_unregister(drm); -register_fail: malidp_de_planes_destroy(drm); drm_mode_config_cleanup(drm); init_fail: @@ -431,6 +435,7 @@ static void malidp_unbind(struct device *dev) struct malidp_drm *malidp = drm->dev_private; struct malidp_hw_device *hwdev = malidp->dev; + drm_dev_unregister(drm); if (malidp->fbdev) { drm_fbdev_cma_fini(malidp->fbdev); malidp->fbdev = NULL; @@ -442,7 +447,6 @@ static void malidp_unbind(struct device *dev) component_unbind_all(dev, drm); of_node_put(malidp->crtc.port); malidp->crtc.port = NULL; - drm_dev_unregister(drm); malidp_de_planes_destroy(drm); drm_mode_config_cleanup(drm); drm->dev_private = NULL; diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index f4315bc..6e6fca2 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1369,7 +1369,6 @@ const struct drm_connector_helper_funcs tda998x_connector_helper_funcs = { static void tda998x_connector_destroy(struct drm_connector *connector) { - drm_connector_unregister(connector); drm_connector_cleanup(connector); } @@ -1441,16 +1440,10 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_connector; - ret = drm_connector_register(&priv->connector); - if (ret) - goto err_sysfs; - drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); return 0; -err_sysfs: - drm_connector_cleanup(&priv->connector); err_connector: drm_encoder_cleanup(&priv->encoder); err_encoder: @@ -1463,7 +1456,6 @@ static void tda998x_unbind(struct device *dev, struct device *master, { struct tda998x_priv *priv = dev_get_drvdata(dev); - drm_connector_unregister(&priv->connector); drm_connector_cleanup(&priv->connector); drm_encoder_cleanup(&priv->encoder); tda998x_destroy(priv); -- 1.7.9.5