[PATCH v4.1 22/22] drm: omapdrm: Perform initialization/cleanup at probe/remove time

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Dec 19 09:15:42 UTC 2016


The drm driver .load() operation is prone to race conditions as it
initializes the driver after registering the device nodes. Its usage is
deprecated, inline it in the probe function and call drm_dev_alloc() and
drm_dev_register() explicitly.

For consistency inline the .unload() handler in the remove function as
well.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
Changes since v4:

- Dropped drm_vblank_cleanup() in probe error path
- Removed a duplicate call to omap_crtc_pre_uninit() in probe

Changes since v1:

- Call drm_kms_helper_poll_fini() before omap_fbdev_free() in the remove
  handler.
- Keep storing drm_device in the platform device private data.
---
 drivers/gpu/drm/omapdrm/omap_connector.c |   2 -
 drivers/gpu/drm/omapdrm/omap_drv.c       | 211 +++++++++++++++----------------
 2 files changed, 105 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 691cffebb76e..f90e2d22c5ec 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -240,8 +240,6 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
-	drm_connector_register(connector);
-
 	return connector;
 
 fail:
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 0a2d461d62cf..00aa214b7560 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -493,11 +493,6 @@ static int omap_modeset_init(struct drm_device *dev)
 	return 0;
 }
 
-static void omap_modeset_free(struct drm_device *dev)
-{
-	drm_mode_config_cleanup(dev);
-}
-
 /*
  * drm ioctl funcs
  */
@@ -633,95 +628,6 @@ static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END - DRM_COMMAND_BASE] =
  * drm driver funcs
  */
 
-/**
- * load - setup chip and create an initial config
- * @dev: DRM device
- * @flags: startup flags
- *
- * The driver load routine has to do several things:
- *   - initialize the memory manager
- *   - allocate initial config memory
- *   - setup the DRM framebuffer with the allocated memory
- */
-static int dev_load(struct drm_device *dev, unsigned long flags)
-{
-	struct omap_drm_platform_data *pdata = dev->dev->platform_data;
-	struct omap_drm_private *priv;
-	unsigned int i;
-	int ret;
-
-	DBG("load: dev=%p", dev);
-
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	priv->omaprev = pdata->omaprev;
-
-	dev->dev_private = priv;
-
-	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
-	init_waitqueue_head(&priv->commit.wait);
-	spin_lock_init(&priv->commit.lock);
-
-	spin_lock_init(&priv->list_lock);
-	INIT_LIST_HEAD(&priv->obj_list);
-
-	omap_gem_init(dev);
-
-	ret = omap_modeset_init(dev);
-	if (ret) {
-		dev_err(dev->dev, "omap_modeset_init failed: ret=%d\n", ret);
-		dev->dev_private = NULL;
-		kfree(priv);
-		return ret;
-	}
-
-	/* Initialize vblank handling, start with all CRTCs disabled. */
-	ret = drm_vblank_init(dev, priv->num_crtcs);
-	if (ret)
-		dev_warn(dev->dev, "could not init vblank\n");
-
-	for (i = 0; i < priv->num_crtcs; i++)
-		drm_crtc_vblank_off(priv->crtcs[i]);
-
-	priv->fbdev = omap_fbdev_init(dev);
-
-	/* store off drm_device for use in pm ops */
-	dev_set_drvdata(dev->dev, dev);
-
-	drm_kms_helper_poll_init(dev);
-
-	return 0;
-}
-
-static int dev_unload(struct drm_device *dev)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-
-	DBG("unload: dev=%p", dev);
-
-	drm_kms_helper_poll_fini(dev);
-
-	if (priv->fbdev)
-		omap_fbdev_free(dev);
-
-	omap_modeset_free(dev);
-	omap_gem_deinit(dev);
-
-	destroy_workqueue(priv->wq);
-
-	drm_vblank_cleanup(dev);
-	omap_drm_irq_uninstall(dev);
-
-	kfree(dev->dev_private);
-	dev->dev_private = NULL;
-
-	dev_set_drvdata(dev->dev, NULL);
-
-	return 0;
-}
-
 static int dev_open(struct drm_device *dev, struct drm_file *file)
 {
 	file->driver_priv = NULL;
@@ -806,8 +712,6 @@ static const struct file_operations omapdriver_fops = {
 static struct drm_driver omap_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM  | DRIVER_PRIME |
 		DRIVER_ATOMIC,
-	.load = dev_load,
-	.unload = dev_unload,
 	.open = dev_open,
 	.lastclose = dev_lastclose,
 	.get_vblank_counter = drm_vblank_no_hw_counter,
@@ -837,30 +741,125 @@ static struct drm_driver omap_drm_driver = {
 	.patchlevel = DRIVER_PATCHLEVEL,
 };
 
-static int pdev_probe(struct platform_device *device)
+static int pdev_probe(struct platform_device *pdev)
 {
-	int r;
+	struct omap_drm_platform_data *pdata = pdev->dev.platform_data;
+	struct omap_drm_private *priv;
+	struct drm_device *ddev;
+	unsigned int i;
+	int ret;
+
+	DBG("%s", pdev->name);
 
 	if (omapdss_is_initialized() == false)
 		return -EPROBE_DEFER;
 
 	omap_crtc_pre_init();
 
-	r = omap_connect_dssdevs();
-	if (r) {
-		omap_crtc_pre_uninit();
-		return r;
+	ret = omap_connect_dssdevs();
+	if (ret)
+		goto err_crtc_uninit;
+
+	/* Allocate and initialize the driver private structure. */
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err_disconnect_dssdevs;
+	}
+
+	priv->omaprev = pdata->omaprev;
+	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
+
+	init_waitqueue_head(&priv->commit.wait);
+	spin_lock_init(&priv->commit.lock);
+	spin_lock_init(&priv->list_lock);
+	INIT_LIST_HEAD(&priv->obj_list);
+
+	/* Allocate and initialize the DRM device. */
+	ddev = drm_dev_alloc(&omap_drm_driver, &pdev->dev);
+	if (IS_ERR(ddev)) {
+		ret = PTR_ERR(ddev);
+		goto err_free_priv;
+	}
+
+	ddev->dev_private = priv;
+	platform_set_drvdata(pdev, ddev);
+
+	omap_gem_init(ddev);
+
+	ret = omap_modeset_init(ddev);
+	if (ret) {
+		dev_err(&pdev->dev, "omap_modeset_init failed: ret=%d\n", ret);
+		goto err_free_drm_dev;
+	}
+
+	/* Initialize vblank handling, start with all CRTCs disabled. */
+	ret = drm_vblank_init(ddev, priv->num_crtcs);
+	if (ret) {
+		dev_err(&pdev->dev, "could not init vblank\n");
+		goto err_cleanup_modeset;
 	}
 
-	DBG("%s", device->name);
-	return drm_platform_init(&omap_drm_driver, device);
+	for (i = 0; i < priv->num_crtcs; i++)
+		drm_crtc_vblank_off(priv->crtcs[i]);
+
+	priv->fbdev = omap_fbdev_init(ddev);
+
+	drm_kms_helper_poll_init(ddev);
+
+	/*
+	 * Register the DRM device with the core and the connectors with
+	 * sysfs.
+	 */
+	ret = drm_dev_register(ddev, 0);
+	if (ret)
+		goto err_cleanup_helpers;
+
+	return 0;
+
+err_cleanup_helpers:
+	drm_kms_helper_poll_fini(ddev);
+	if (priv->fbdev)
+		omap_fbdev_free(ddev);
+err_cleanup_modeset:
+	drm_mode_config_cleanup(ddev);
+	omap_drm_irq_uninstall(ddev);
+err_free_drm_dev:
+	omap_gem_deinit(ddev);
+	drm_dev_unref(ddev);
+err_free_priv:
+	destroy_workqueue(priv->wq);
+	kfree(priv);
+err_disconnect_dssdevs:
+	omap_disconnect_dssdevs();
+err_crtc_uninit:
+	omap_crtc_pre_uninit();
+	return ret;
 }
 
-static int pdev_remove(struct platform_device *device)
+static int pdev_remove(struct platform_device *pdev)
 {
+	struct drm_device *ddev = platform_get_drvdata(pdev);
+	struct omap_drm_private *priv = ddev->dev_private;
+
 	DBG("");
 
-	drm_put_dev(platform_get_drvdata(device));
+	drm_dev_unregister(ddev);
+
+	drm_kms_helper_poll_fini(ddev);
+
+	if (priv->fbdev)
+		omap_fbdev_free(ddev);
+
+	drm_mode_config_cleanup(ddev);
+
+	omap_drm_irq_uninstall(ddev);
+	omap_gem_deinit(ddev);
+
+	drm_dev_unref(ddev);
+
+	destroy_workqueue(priv->wq);
+	kfree(priv);
 
 	omap_disconnect_dssdevs();
 	omap_crtc_pre_uninit();
-- 
Regards,

Laurent Pinchart



More information about the dri-devel mailing list