[PATCH v2 0/3] drm/tilcdc: Cleanup tilcdc init sequence

[PATCH v2 0/3] drm/tilcdc: Cleanup tilcdc init sequence

[Jyri Sarha]

This series depends on tda998x dropping the drm_connector_register().
Please keep me in loop so that I know when it gets merged so that I
know when it is safe send a pull request for these.

Since first version of this series:
- Dropped "drm/i2c: tda998x: Remove obsolete drm_connector_register() call"
  - an earlier instance of the same patch already exists
- Got rid of unload callback and deprecated drm_put_dev() calls

A patch[1] that crashed all drm drivers using load() hook was briefly
part of linux-next, so became aware that tilcdc should stop using
that too.

The two first patches should be merged before the third can be
merged. So the merge order should be agreed with tda998x before going
forward.

The last patch is just a cleanup, but depends on earlier ones.

Best regards,
Jyri

[1] https://patchwork.kernel.org/patch/9322771/

Jyri Sarha (3):
  drm/tilcdc: Remove obsolete drm_connector_register() calls
  drm/tilcdc: Stop using struct drm_driver load() callback
  drm/tilcdc: Use unload to handle initialization failures

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c   |  10 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c    | 185 +++++++++++++++------------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h    |   3 +-
 drivers/gpu/drm/tilcdc/tilcdc_panel.c  |   2 -
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c |   2 -
 5 files changed, 93 insertions(+), 109 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/3] drm/tilcdc: Remove obsolete drm_connector_register() calls

[Jyri Sarha]

Remove obsolete drm_connector_register() calls from tilcdc_panel.c and
tilcdc_tfp410.c. All connectors are registered when drm_dev_register()
is called.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_panel.c  | 2 --
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 2134bb20..ad7a0e8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -240,8 +240,6 @@ static struct drm_connector *panel_connector_create(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	drm_connector_register(connector);
-
 	return connector;
 
 fail:
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index c32c1ef..eea82b4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -251,8 +251,6 @@ static struct drm_connector *tfp410_connector_create(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	drm_connector_register(connector);
-
 	return connector;
 
 fail:
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm/tilcdc: Stop using struct drm_driver load() callback

[Jyri Sarha]

Stop using struct drm_driver load() and unload() callbacks. The
callbacks should not be used anymore. Instead of using load the
drm_device is allocated with drm_dev_alloc() and registered with
drm_dev_register() only after the driver is completely initialized.
The deinitialization is done directly either in component unbind
callback or in platform driver demove callback.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 124 ++++++++++++++++++++----------------
 1 file changed, 70 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 231f2c7..1967ceb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -196,18 +196,22 @@ static int cpufreq_transition(struct notifier_block *nb,
  * DRM operations:
  */
 
-static int tilcdc_unload(struct drm_device *dev)
+static void tilcdc_fini(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
 
-	tilcdc_remove_external_encoders(dev);
+	drm_modeset_lock_crtc(priv->crtc, NULL);
+	tilcdc_crtc_disable(priv->crtc);
+	drm_modeset_unlock_crtc(priv->crtc);
+
+	drm_dev_unregister(dev);
 
-	drm_fbdev_cma_fini(priv->fbdev);
 	drm_kms_helper_poll_fini(dev);
+	drm_fbdev_cma_fini(priv->fbdev);
+	drm_irq_uninstall(dev);
 	drm_mode_config_cleanup(dev);
-	drm_vblank_cleanup(dev);
 
-	drm_irq_uninstall(dev);
+	tilcdc_remove_external_encoders(dev);
 
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&priv->freq_transition,
@@ -227,28 +231,34 @@ static int tilcdc_unload(struct drm_device *dev)
 
 	pm_runtime_disable(dev->dev);
 
-	return 0;
+	drm_dev_unref(dev);
 }
 
-static int tilcdc_load(struct drm_device *dev, unsigned long flags)
+static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 {
-	struct platform_device *pdev = dev->platformdev;
-	struct device_node *node = pdev->dev.of_node;
+	struct drm_device *ddev;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *node = dev->of_node;
 	struct tilcdc_drm_private *priv;
 	struct resource *res;
 	u32 bpp = 0;
 	int ret;
 
-	priv = devm_kzalloc(dev->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
-		dev_err(dev->dev, "failed to allocate private data\n");
+		dev_err(dev, "failed to allocate private data\n");
 		return -ENOMEM;
 	}
 
-	dev->dev_private = priv;
+	ddev = drm_dev_alloc(ddrv, dev);
+	if (IS_ERR(ddev))
+		return PTR_ERR(ddev);
+
+	ddev->platformdev = pdev;
+	ddev->dev_private = priv;
 
 	priv->is_componentized =
-		tilcdc_get_external_components(dev->dev, NULL) > 0;
+		tilcdc_get_external_components(dev, NULL) > 0;
 
 	priv->wq = alloc_ordered_workqueue("tilcdc", 0);
 	if (!priv->wq) {
@@ -258,21 +268,21 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
-		dev_err(dev->dev, "failed to get memory resource\n");
+		dev_err(dev, "failed to get memory resource\n");
 		ret = -EINVAL;
 		goto fail_free_wq;
 	}
 
 	priv->mmio = ioremap_nocache(res->start, resource_size(res));
 	if (!priv->mmio) {
-		dev_err(dev->dev, "failed to ioremap\n");
+		dev_err(dev, "failed to ioremap\n");
 		ret = -ENOMEM;
 		goto fail_free_wq;
 	}
 
-	priv->clk = clk_get(dev->dev, "fck");
+	priv->clk = clk_get(dev, "fck");
 	if (IS_ERR(priv->clk)) {
-		dev_err(dev->dev, "failed to get functional clock\n");
+		dev_err(dev, "failed to get functional clock\n");
 		ret = -ENODEV;
 		goto fail_iounmap;
 	}
@@ -282,7 +292,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	ret = cpufreq_register_notifier(&priv->freq_transition,
 			CPUFREQ_TRANSITION_NOTIFIER);
 	if (ret) {
-		dev_err(dev->dev, "failed to register cpufreq notifier\n");
+		dev_err(dev, "failed to register cpufreq notifier\n");
 		goto fail_put_clk;
 	}
 #endif
@@ -303,11 +313,11 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 
 	DBG("Maximum Pixel Clock Value %dKHz", priv->max_pixelclock);
 
-	pm_runtime_enable(dev->dev);
+	pm_runtime_enable(dev);
 
 	/* Determine LCD IP Version */
-	pm_runtime_get_sync(dev->dev);
-	switch (tilcdc_read(dev, LCDC_PID_REG)) {
+	pm_runtime_get_sync(dev);
+	switch (tilcdc_read(ddev, LCDC_PID_REG)) {
 	case 0x4c100102:
 		priv->rev = 1;
 		break;
@@ -316,14 +326,14 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 		priv->rev = 2;
 		break;
 	default:
-		dev_warn(dev->dev, "Unknown PID Reg value 0x%08x, "
-				"defaulting to LCD revision 1\n",
-				tilcdc_read(dev, LCDC_PID_REG));
+		dev_warn(dev, "Unknown PID Reg value 0x%08x, "
+			"defaulting to LCD revision 1\n",
+			tilcdc_read(ddev, LCDC_PID_REG));
 		priv->rev = 1;
 		break;
 	}
 
-	pm_runtime_put_sync(dev->dev);
+	pm_runtime_put_sync(dev);
 
 	if (priv->rev == 1) {
 		DBG("Revision 1 LCDC supports only RGB565 format");
@@ -356,74 +366,82 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 		}
 	}
 
-	ret = modeset_init(dev);
+	ret = modeset_init(ddev);
 	if (ret < 0) {
-		dev_err(dev->dev, "failed to initialize mode setting\n");
+		dev_err(dev, "failed to initialize mode setting\n");
 		goto fail_cpufreq_unregister;
 	}
 
-	platform_set_drvdata(pdev, dev);
+	platform_set_drvdata(pdev, ddev);
 
 	if (priv->is_componentized) {
-		ret = component_bind_all(dev->dev, dev);
+		ret = component_bind_all(dev, ddev);
 		if (ret < 0)
 			goto fail_mode_config_cleanup;
 
-		ret = tilcdc_add_external_encoders(dev);
+		ret = tilcdc_add_external_encoders(ddev);
 		if (ret < 0)
 			goto fail_component_cleanup;
 	}
 
 	if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
-		dev_err(dev->dev, "no encoders/connectors found\n");
+		dev_err(dev, "no encoders/connectors found\n");
 		ret = -ENXIO;
 		goto fail_external_cleanup;
 	}
 
-	ret = drm_vblank_init(dev, 1);
+	ret = drm_vblank_init(ddev, 1);
 	if (ret < 0) {
-		dev_err(dev->dev, "failed to initialize vblank\n");
+		dev_err(dev, "failed to initialize vblank\n");
 		goto fail_external_cleanup;
 	}
 
-	ret = drm_irq_install(dev, platform_get_irq(dev->platformdev, 0));
+	ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
 	if (ret < 0) {
-		dev_err(dev->dev, "failed to install IRQ handler\n");
+		dev_err(dev, "failed to install IRQ handler\n");
 		goto fail_vblank_cleanup;
 	}
 
-	drm_mode_config_reset(dev);
+	drm_mode_config_reset(ddev);
 
-	priv->fbdev = drm_fbdev_cma_init(dev, bpp,
-			dev->mode_config.num_crtc,
-			dev->mode_config.num_connector);
+	priv->fbdev = drm_fbdev_cma_init(ddev, bpp,
+			ddev->mode_config.num_crtc,
+			ddev->mode_config.num_connector);
 	if (IS_ERR(priv->fbdev)) {
 		ret = PTR_ERR(priv->fbdev);
 		goto fail_irq_uninstall;
 	}
 
-	drm_kms_helper_poll_init(dev);
+	drm_kms_helper_poll_init(ddev);
+
+	ret = drm_dev_register(ddev, 0);
+	if (ret)
+		goto fail_platform_init;
 
 	return 0;
 
+fail_platform_init:
+	drm_kms_helper_poll_fini(ddev);
+	drm_fbdev_cma_fini(priv->fbdev);
+
 fail_irq_uninstall:
-	drm_irq_uninstall(dev);
+	drm_irq_uninstall(ddev);
 
 fail_vblank_cleanup:
-	drm_vblank_cleanup(dev);
+	drm_vblank_cleanup(ddev);
 
 fail_component_cleanup:
 	if (priv->is_componentized)
-		component_unbind_all(dev->dev, dev);
+		component_unbind_all(dev, dev);
 
 fail_mode_config_cleanup:
-	drm_mode_config_cleanup(dev);
+	drm_mode_config_cleanup(ddev);
 
 fail_external_cleanup:
-	tilcdc_remove_external_encoders(dev);
+	tilcdc_remove_external_encoders(ddev);
 
 fail_cpufreq_unregister:
-	pm_runtime_disable(dev->dev);
+	pm_runtime_disable(dev);
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&priv->freq_transition,
 			CPUFREQ_TRANSITION_NOTIFIER);
@@ -440,7 +458,8 @@ fail_free_wq:
 	destroy_workqueue(priv->wq);
 
 fail_unset_priv:
-	dev->dev_private = NULL;
+	ddev->dev_private = NULL;
+	drm_dev_unref(ddev);
 
 	return ret;
 }
@@ -587,8 +606,6 @@ static const struct file_operations fops = {
 static struct drm_driver tilcdc_driver = {
 	.driver_features    = (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET |
 			       DRIVER_PRIME | DRIVER_ATOMIC),
-	.load               = tilcdc_load,
-	.unload             = tilcdc_unload,
 	.lastclose          = tilcdc_lastclose,
 	.irq_handler        = tilcdc_irq,
 	.get_vblank_counter = drm_vblank_no_hw_counter,
@@ -662,10 +679,9 @@ static const struct dev_pm_ops tilcdc_pm_ops = {
 /*
  * Platform driver:
  */
-
 static int tilcdc_bind(struct device *dev)
 {
-	return drm_platform_init(&tilcdc_driver, to_platform_device(dev));
+	return tilcdc_init(&tilcdc_driver, dev);
 }
 
 static void tilcdc_unbind(struct device *dev)
@@ -676,7 +692,7 @@ static void tilcdc_unbind(struct device *dev)
 	if (!ddev->dev_private)
 		return;
 
-	drm_put_dev(dev_get_drvdata(dev));
+	tilcdc_fini(dev_get_drvdata(dev));
 }
 
 static const struct component_master_ops tilcdc_comp_ops = {
@@ -699,7 +715,7 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 	else if (ret == 0)
-		return drm_platform_init(&tilcdc_driver, pdev);
+		return tilcdc_init(&tilcdc_driver, &pdev->dev);
 	else
 		return component_master_add_with_match(&pdev->dev,
 						       &tilcdc_comp_ops,
@@ -714,7 +730,7 @@ static int tilcdc_pdev_remove(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 	else if (ret == 0)
-		drm_put_dev(platform_get_drvdata(pdev));
+		tilcdc_fini(platform_get_drvdata(pdev));
 	else
 		component_master_del(&pdev->dev, &tilcdc_comp_ops);
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm/tilcdc: Use unload to handle initialization failures

[Jyri Sarha]

Use unload to handle initialization failures instead of complex goto
label mess. To do this the initialization sequence needed slight
reordering and some unload functions needed to become conditional.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  10 ++--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 101 ++++++++++++-----------------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |   3 +-
 3 files changed, 43 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 606ae7d..6221150 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -176,14 +176,12 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
 	tilcdc_crtc->enabled = true;
 }
 
-void tilcdc_crtc_disable(struct drm_crtc *crtc)
+void tilcdc_crtc_off(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
 	if (!tilcdc_crtc->enabled)
 		return;
 
@@ -227,6 +225,12 @@ void tilcdc_crtc_disable(struct drm_crtc *crtc)
 	tilcdc_crtc->enabled = false;
 }
 
+static void tilcdc_crtc_disable(struct drm_crtc *crtc)
+{
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	tilcdc_crtc_off(crtc);
+}
+
 static bool tilcdc_crtc_is_on(struct drm_crtc *crtc)
 {
 	return crtc->state && crtc->state->enable && crtc->state->active;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 1967ceb..71f4b12 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -160,8 +160,6 @@ static int modeset_init(struct drm_device *dev)
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_module *mod;
 
-	drm_mode_config_init(dev);
-
 	priv->crtc = tilcdc_crtc_create(dev);
 
 	list_for_each_entry(mod, &module_list, list) {
@@ -200,22 +198,25 @@ static void tilcdc_fini(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
 
-	drm_modeset_lock_crtc(priv->crtc, NULL);
-	tilcdc_crtc_disable(priv->crtc);
-	drm_modeset_unlock_crtc(priv->crtc);
+	if (priv->crtc)
+		tilcdc_crtc_off(priv->crtc);
 
-	drm_dev_unregister(dev);
+	if (priv->is_registered)
+		drm_dev_unregister(dev);
 
 	drm_kms_helper_poll_fini(dev);
-	drm_fbdev_cma_fini(priv->fbdev);
+
+	if (priv->fbdev)
+		drm_fbdev_cma_fini(priv->fbdev);
+
 	drm_irq_uninstall(dev);
 	drm_mode_config_cleanup(dev);
-
 	tilcdc_remove_external_encoders(dev);
 
 #ifdef CONFIG_CPU_FREQ
-	cpufreq_unregister_notifier(&priv->freq_transition,
-			CPUFREQ_TRANSITION_NOTIFIER);
+	if (priv->freq_transition.notifier_call)
+		cpufreq_unregister_notifier(&priv->freq_transition,
+					    CPUFREQ_TRANSITION_NOTIFIER);
 #endif
 
 	if (priv->clk)
@@ -224,8 +225,10 @@ static void tilcdc_fini(struct drm_device *dev)
 	if (priv->mmio)
 		iounmap(priv->mmio);
 
-	flush_workqueue(priv->wq);
-	destroy_workqueue(priv->wq);
+	if (priv->wq) {
+		flush_workqueue(priv->wq);
+		destroy_workqueue(priv->wq);
+	}
 
 	dev->dev_private = NULL;
 
@@ -256,6 +259,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 
 	ddev->platformdev = pdev;
 	ddev->dev_private = priv;
+	platform_set_drvdata(pdev, ddev);
+	drm_mode_config_init(ddev);
 
 	priv->is_componentized =
 		tilcdc_get_external_components(dev, NULL) > 0;
@@ -263,28 +268,28 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 	priv->wq = alloc_ordered_workqueue("tilcdc", 0);
 	if (!priv->wq) {
 		ret = -ENOMEM;
-		goto fail_unset_priv;
+		goto init_failed;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(dev, "failed to get memory resource\n");
 		ret = -EINVAL;
-		goto fail_free_wq;
+		goto init_failed;
 	}
 
 	priv->mmio = ioremap_nocache(res->start, resource_size(res));
 	if (!priv->mmio) {
 		dev_err(dev, "failed to ioremap\n");
 		ret = -ENOMEM;
-		goto fail_free_wq;
+		goto init_failed;
 	}
 
 	priv->clk = clk_get(dev, "fck");
 	if (IS_ERR(priv->clk)) {
 		dev_err(dev, "failed to get functional clock\n");
 		ret = -ENODEV;
-		goto fail_iounmap;
+		goto init_failed;
 	}
 
 #ifdef CONFIG_CPU_FREQ
@@ -293,7 +298,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 			CPUFREQ_TRANSITION_NOTIFIER);
 	if (ret) {
 		dev_err(dev, "failed to register cpufreq notifier\n");
-		goto fail_put_clk;
+		priv->freq_transition.notifier_call = NULL;
+		goto init_failed;
 	}
 #endif
 
@@ -369,37 +375,35 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 	ret = modeset_init(ddev);
 	if (ret < 0) {
 		dev_err(dev, "failed to initialize mode setting\n");
-		goto fail_cpufreq_unregister;
+		goto init_failed;
 	}
 
-	platform_set_drvdata(pdev, ddev);
-
 	if (priv->is_componentized) {
 		ret = component_bind_all(dev, ddev);
 		if (ret < 0)
-			goto fail_mode_config_cleanup;
+			goto init_failed;
 
 		ret = tilcdc_add_external_encoders(ddev);
 		if (ret < 0)
-			goto fail_component_cleanup;
+			goto init_failed;
 	}
 
 	if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
 		dev_err(dev, "no encoders/connectors found\n");
 		ret = -ENXIO;
-		goto fail_external_cleanup;
+		goto init_failed;
 	}
 
 	ret = drm_vblank_init(ddev, 1);
 	if (ret < 0) {
 		dev_err(dev, "failed to initialize vblank\n");
-		goto fail_external_cleanup;
+		goto init_failed;
 	}
 
 	ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
 	if (ret < 0) {
 		dev_err(dev, "failed to install IRQ handler\n");
-		goto fail_vblank_cleanup;
+		goto init_failed;
 	}
 
 	drm_mode_config_reset(ddev);
@@ -409,57 +413,20 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 			ddev->mode_config.num_connector);
 	if (IS_ERR(priv->fbdev)) {
 		ret = PTR_ERR(priv->fbdev);
-		goto fail_irq_uninstall;
+		goto init_failed;
 	}
 
 	drm_kms_helper_poll_init(ddev);
 
 	ret = drm_dev_register(ddev, 0);
 	if (ret)
-		goto fail_platform_init;
+		goto init_failed;
 
+	priv->is_registered = true;
 	return 0;
 
-fail_platform_init:
-	drm_kms_helper_poll_fini(ddev);
-	drm_fbdev_cma_fini(priv->fbdev);
-
-fail_irq_uninstall:
-	drm_irq_uninstall(ddev);
-
-fail_vblank_cleanup:
-	drm_vblank_cleanup(ddev);
-
-fail_component_cleanup:
-	if (priv->is_componentized)
-		component_unbind_all(dev, dev);
-
-fail_mode_config_cleanup:
-	drm_mode_config_cleanup(ddev);
-
-fail_external_cleanup:
-	tilcdc_remove_external_encoders(ddev);
-
-fail_cpufreq_unregister:
-	pm_runtime_disable(dev);
-#ifdef CONFIG_CPU_FREQ
-	cpufreq_unregister_notifier(&priv->freq_transition,
-			CPUFREQ_TRANSITION_NOTIFIER);
-
-fail_put_clk:
-#endif
-	clk_put(priv->clk);
-
-fail_iounmap:
-	iounmap(priv->mmio);
-
-fail_free_wq:
-	flush_workqueue(priv->wq);
-	destroy_workqueue(priv->wq);
-
-fail_unset_priv:
-	ddev->dev_private = NULL;
-	drm_dev_unref(ddev);
+init_failed:
+	tilcdc_fini(ddev);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 9780c37..7db23f2 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -89,6 +89,7 @@ struct tilcdc_drm_private {
 	struct drm_connector *connectors[8];
 	const struct drm_connector_helper_funcs *connector_funcs[8];
 
+	bool is_registered;
 	bool is_componentized;
 };
 
@@ -172,7 +173,7 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
 					bool simulate_vesa_sync);
 int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode);
 int tilcdc_crtc_max_width(struct drm_crtc *crtc);
-void tilcdc_crtc_disable(struct drm_crtc *crtc);
+void tilcdc_crtc_off(struct drm_crtc *crtc);
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		struct drm_framebuffer *fb,
 		struct drm_pending_vblank_event *event);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/3] drm/tilcdc: Cleanup tilcdc init sequence

[Russell King - ARM Linux]

On Mon, Oct 24, 2016 at 06:34:42PM +0300, Jyri Sarha wrote:
> This series depends on tda998x dropping the drm_connector_register().
> Please keep me in loop so that I know when it gets merged so that I
> know when it is safe send a pull request for these.

This suggests that you think that delaying a merge somehow solves issues.
It doesn't really solve anything.

While it may make a particular tree remain correctly functional before
the merge and after the merge, it breaks bisecting.  Consider this:


   A----B----C----D-----Merge
    `E----F----G----H---^

Let's say that commit F depends on commit C for the result to work.
Bisect can choose _any_ of those commits as a potential point to test,
and if it were to choose G, then we will test commits A, E, F and G,
without the dependency - resulting in failure.

Hence, this is broken.  Delaying git pulls really solves nothing,
except giving the illusion of solving a problem.

The only way to avoid that is to base the changes on C.  In other words:

   A----B----C----D---------------Merge
              `E----F----G----H---^

which then means that, when bisect chooses G, it gets A, B, C, E, F,
and G, and the dependencies for E are correctly satisfied.

Of course, if F were to depend on several other independent commits
(eg, let's say that F depends on the TDA998x changes and some other
tilcdc changes) then the right thing would be to merge the dependencies
before the affected commit.

In other words, let's say that the TDA998x changes you need are in
my drm-tda998x-mali branch, but you already have some development
in your tilcdc tree which your changes will depend on.  You'd pull my
drm-tda998x-mali into your tilcdc tree before committing the dependent
changes.

Now, what about sending it up - the polite thing to do is to wait for
the dependencies to be merged upstream first, as asking for your tree
to be pulled upstream will cause the dependencies to also be pulled
upstream.

So... where do we stand at the moment...

I've merged the mali changes from Brian Starkey into a separate branch
called drm-tda998x-mali, which we can all share (I'm waiting for
agreement before pushing it out.)  drm-tda998x-mali is already merged
into the bottom of my unmerged drm-tda998x-devel branch, which has ten
patches on top.  It's also going to be merged into my drm-armada-devel
branch, so I can add the de-midlayering patch for armada there too.
You will also need to merge the drm-tda998x-mali branch before your
patches as well.

It's rather unfortunate that kernel development effectively shuts down
when the kernel summit happens, so I guess it will end up being next
week before we can move forward on this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/3] drm/tilcdc: Cleanup tilcdc init sequence

[Brian Starkey]

On Tue, Nov 01, 2016 at 03:04:14PM +0000, Russell King - ARM Linux wrote:
>
>So... where do we stand at the moment...
>
>I've merged the mali changes from Brian Starkey into a separate branch
>called drm-tda998x-mali, which we can all share (I'm waiting for
>agreement before pushing it out.)

I think it's fine to send that. I don't have a branch to send a pull
request for and I'm struggling with a flaky connection, so if you've
got a branch, just send it.

Thanks,
-Brian
>
>-- 
>RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
>according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel