Re: [PATCH 06/24] drm/omap: Allocate drm_device earlier and unref it as last step

Hi Tomi,

Thank you for the patch.

On Monday, 12 February 2018 11:44:36 EET Tomi Valkeinen wrote:

From: Peter Ujfalusi peter.ujfalusi@ti.com

If we allocate the drm_device earlier we can just return the error code without the need to use goto. Do the unref of the drm_device as a last step when cleaning up. This will make the drm_device available longer for us and makes sure that we only free up the memory when all other cleanups have been already done.

How about making it even simpler by embedding drm_device inside omap_drm_private ?

Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com

---

drivers/gpu/drm/omapdrm/omap_drv.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 3cd9188ab30b..57d11f9aeead 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -533,6 +533,14 @@ static int pdev_probe(struct platform_device *pdev) if (!priv) return -ENOMEM;

• /* Allocate and initialize the DRM device. */

• ddev = drm_dev_alloc(&omap_drm_driver, &pdev->dev);

• if (IS_ERR(ddev))

• return PTR_ERR(ddev);
  

• ddev->dev_private = priv;

• platform_set_drvdata(pdev, ddev);

• omap_crtc_pre_init();

  ret = omap_connect_dssdevs();

@@ -549,16 +557,6 @@ static int pdev_probe(struct platform_device *pdev) 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_destroy_wq;
  

• }
• ddev->dev_private = priv;
• platform_set_drvdata(pdev, ddev);
• /* Get memory bandwidth limits */ if (priv->dispc_ops->get_memory_bandwidth_limit) priv->max_bandwidth =

@@ -569,7 +567,7 @@ static int pdev_probe(struct platform_device *pdev) ret = omap_modeset_init(ddev); if (ret) { dev_err(&pdev->dev, "omap_modeset_init failed: ret=%d\n", ret);

• goto err_free_drm_dev;
  

• goto err_gem_deinit;
  

  }

  /* Initialize vblank handling, start with all CRTCs disabled. */

@@ -605,14 +603,13 @@ static int pdev_probe(struct platform_device *pdev) err_cleanup_modeset: drm_mode_config_cleanup(ddev); omap_drm_irq_uninstall(ddev); -err_free_drm_dev: +err_gem_deinit: omap_gem_deinit(ddev);

• drm_dev_unref(ddev);

-err_destroy_wq: destroy_workqueue(priv->wq); omap_disconnect_dssdevs(); err_crtc_uninit: omap_crtc_pre_uninit();

• drm_dev_unref(ddev); return ret;

}

@@ -637,13 +634,13 @@ static int pdev_remove(struct platform_device *pdev) omap_drm_irq_uninstall(ddev); omap_gem_deinit(ddev);

• drm_dev_unref(ddev);

• destroy_workqueue(priv->wq);

  omap_disconnect_dssdevs(); omap_crtc_pre_uninit();

• drm_dev_unref(ddev);
• return 0;

}