[PATCH v5 18/25] drm/tilcdc: Convert to DRM managed resources

["Kory Maincent (TI.com)" <kory.maincent@bootlin.com>]

Convert the tilcdc driver to use DRM managed resources (drmm_* APIs)
to eliminate resource lifetime issues, particularly in probe deferral
scenarios.

This conversion addresses potential use-after-free bugs by ensuring
proper cleanup ordering through the DRM managed resource framework.
The changes include:
- Replace drm_crtc_init_with_planes() with drmm_crtc_alloc_with_planes()
- Replace drm_universal_plane_init() with drmm_universal_plane_alloc()
- Replace drm_simple_encoder_init() with drmm_simple_encoder_alloc()
- Remove manual cleanup in tilcdc_crtc_destroy() and error paths
- Remove drm_encoder_cleanup() from encoder error handling paths
- Use drmm_add_action_or_reset() for remaining cleanup operations

This approach is recommended by the DRM subsystem for improved resource
lifetime management and is particularly important for drivers that may
experience probe deferral.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: Kory Maincent (TI.com) <kory.maincent@bootlin.com>
---

Change in v5:
- Move drmm_add_action_or_reset() up to avoid missing cleanup in case
  of dmam_alloc_coherent() returning an error.

Change in v4:
- New patch.
- Move on to DRM managed resources to fix null pointer dereference koops
  in case drm_of_find_panel_or_bridge() return EPROBE_DEFER.
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c    | 55 +++++++++++++++++----------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c     |  5 +--
 drivers/gpu/drm/tilcdc/tilcdc_drv.h     | 13 ++++++--
 drivers/gpu/drm/tilcdc/tilcdc_encoder.c | 38 ++++++++---------------
 drivers/gpu/drm/tilcdc/tilcdc_plane.c   | 27 +++++++---------
 5 files changed, 64 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 0bd99a2efeeb4..2916de3dce91e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -16,6 +16,7 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
@@ -30,7 +31,7 @@
 struct tilcdc_crtc {
 	struct drm_crtc base;
 
-	struct drm_plane primary;
+	struct tilcdc_plane *primary;
 	struct drm_pending_vblank_event *event;
 	struct mutex enable_lock;
 	bool enabled;
@@ -555,16 +556,15 @@ static void tilcdc_crtc_recover_work(struct work_struct *work)
 	drm_modeset_unlock(&crtc->mutex);
 }
 
-void tilcdc_crtc_destroy(struct drm_crtc *crtc)
+static void tilcdc_crtc_destroy(struct drm_device *dev, void *data)
 {
-	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(crtc->dev);
+	struct tilcdc_drm_private *priv = (struct tilcdc_drm_private *)data;
 
-	tilcdc_crtc_shutdown(crtc);
+	tilcdc_crtc_shutdown(priv->crtc);
 
 	flush_workqueue(priv->wq);
 
-	of_node_put(crtc->port);
-	drm_crtc_cleanup(crtc);
+	of_node_put(priv->crtc->port);
 }
 
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
@@ -714,7 +714,6 @@ static void tilcdc_crtc_reset(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
-	.destroy        = tilcdc_crtc_destroy,
 	.set_config     = drm_atomic_helper_set_config,
 	.page_flip      = drm_atomic_helper_page_flip,
 	.reset		= tilcdc_crtc_reset,
@@ -960,12 +959,31 @@ int tilcdc_crtc_create(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(dev);
 	struct tilcdc_crtc *tilcdc_crtc;
+	struct tilcdc_plane *primary;
 	struct drm_crtc *crtc;
 	int ret;
 
-	tilcdc_crtc = devm_kzalloc(dev->dev, sizeof(*tilcdc_crtc), GFP_KERNEL);
-	if (!tilcdc_crtc)
-		return -ENOMEM;
+	primary = tilcdc_plane_init(dev);
+	if (IS_ERR(primary)) {
+		dev_err(dev->dev, "Failed to initialize plane: %pe\n", primary);
+		return PTR_ERR(primary);
+	}
+
+	tilcdc_crtc = drmm_crtc_alloc_with_planes(dev, struct tilcdc_crtc, base,
+						  &primary->base,
+						  NULL,
+						  &tilcdc_crtc_funcs,
+						  "tilcdc crtc");
+	if (IS_ERR(tilcdc_crtc)) {
+		dev_err(dev->dev, "Failed to init CRTC: %pe\n", tilcdc_crtc);
+		return PTR_ERR(tilcdc_crtc);
+	}
+
+	tilcdc_crtc->primary = primary;
+	priv->crtc = &tilcdc_crtc->base;
+	ret = drmm_add_action_or_reset(dev, tilcdc_crtc_destroy, priv);
+	if (ret)
+		return ret;
 
 	init_completion(&tilcdc_crtc->palette_loaded);
 	tilcdc_crtc->palette_base = dmam_alloc_coherent(dev->dev,
@@ -978,10 +996,6 @@ int tilcdc_crtc_create(struct drm_device *dev)
 
 	crtc = &tilcdc_crtc->base;
 
-	ret = tilcdc_plane_init(dev, &tilcdc_crtc->primary);
-	if (ret < 0)
-		goto fail;
-
 	mutex_init(&tilcdc_crtc->enable_lock);
 
 	init_waitqueue_head(&tilcdc_crtc->frame_done_wq);
@@ -989,20 +1003,7 @@ int tilcdc_crtc_create(struct drm_device *dev)
 	spin_lock_init(&tilcdc_crtc->irq_lock);
 	INIT_WORK(&tilcdc_crtc->recover_work, tilcdc_crtc_recover_work);
 
-	ret = drm_crtc_init_with_planes(dev, crtc,
-					&tilcdc_crtc->primary,
-					NULL,
-					&tilcdc_crtc_funcs,
-					"tilcdc crtc");
-	if (ret < 0)
-		goto fail;
-
 	drm_crtc_helper_add(crtc, &tilcdc_crtc_helper_funcs);
 
-	priv->crtc = crtc;
 	return 0;
-
-fail:
-	tilcdc_crtc_destroy(crtc);
-	return ret;
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 1a238a22309f4..3b11d296a7e91 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -392,7 +392,7 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(dev, "failed to register cpufreq notifier\n");
 		priv->freq_transition.notifier_call = NULL;
-		goto destroy_crtc;
+		goto disable_pm;
 	}
 #endif
 
@@ -442,9 +442,7 @@ static int tilcdc_pdev_probe(struct platform_device *pdev)
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&priv->freq_transition,
 				    CPUFREQ_TRANSITION_NOTIFIER);
-destroy_crtc:
 #endif
-	tilcdc_crtc_destroy(priv->crtc);
 disable_pm:
 	pm_runtime_disable(dev);
 	clk_put(priv->clk);
@@ -466,7 +464,6 @@ static void tilcdc_pdev_remove(struct platform_device *pdev)
 	cpufreq_unregister_notifier(&priv->freq_transition,
 				    CPUFREQ_TRANSITION_NOTIFIER);
 #endif
-	tilcdc_crtc_destroy(priv->crtc);
 	pm_runtime_disable(&pdev->dev);
 	clk_put(priv->clk);
 	destroy_workqueue(priv->wq);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index c69e279a2539d..17d152f9f0b69 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -77,7 +77,7 @@ struct tilcdc_drm_private {
 
 	struct drm_crtc *crtc;
 
-	struct drm_encoder *encoder;
+	struct tilcdc_encoder *encoder;
 	struct drm_connector *connector;
 
 	bool irq_enabled;
@@ -91,11 +91,18 @@ int tilcdc_crtc_create(struct drm_device *dev);
 irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc);
 void tilcdc_crtc_update_clk(struct drm_crtc *crtc);
 void tilcdc_crtc_shutdown(struct drm_crtc *crtc);
-void tilcdc_crtc_destroy(struct drm_crtc *crtc);
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
 		struct drm_framebuffer *fb,
 		struct drm_pending_vblank_event *event);
 
-int tilcdc_plane_init(struct drm_device *dev, struct drm_plane *plane);
+struct tilcdc_plane {
+	struct drm_plane base;
+};
+
+struct tilcdc_encoder {
+	struct drm_encoder base;
+};
+
+struct tilcdc_plane *tilcdc_plane_init(struct drm_device *dev);
 
 #endif /* __TILCDC_DRV_H__ */
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_encoder.c b/drivers/gpu/drm/tilcdc/tilcdc_encoder.c
index d42be3e16c536..1ee5761757a8c 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_encoder.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_encoder.c
@@ -37,13 +37,13 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
 	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(ddev);
 	int ret;
 
-	priv->encoder->possible_crtcs = BIT(0);
+	priv->encoder->base.possible_crtcs = BIT(0);
 
-	ret = drm_bridge_attach(priv->encoder, bridge, NULL, 0);
+	ret = drm_bridge_attach(&priv->encoder->base, bridge, NULL, 0);
 	if (ret)
 		return ret;
 
-	priv->connector = tilcdc_encoder_find_connector(ddev, priv->encoder);
+	priv->connector = tilcdc_encoder_find_connector(ddev, &priv->encoder->base);
 	if (!priv->connector)
 		return -ENODEV;
 
@@ -53,6 +53,7 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
 int tilcdc_encoder_create(struct drm_device *ddev)
 {
 	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(ddev);
+	struct tilcdc_encoder *encoder;
 	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	int ret;
@@ -64,33 +65,20 @@ int tilcdc_encoder_create(struct drm_device *ddev)
 	else if (ret)
 		return ret;
 
-	priv->encoder = devm_kzalloc(ddev->dev, sizeof(*priv->encoder), GFP_KERNEL);
-	if (!priv->encoder)
-		return -ENOMEM;
-
-	ret = drm_simple_encoder_init(ddev, priv->encoder,
-				      DRM_MODE_ENCODER_NONE);
-	if (ret) {
-		dev_err(ddev->dev, "drm_encoder_init() failed %d\n", ret);
-		return ret;
+	encoder = drmm_simple_encoder_alloc(ddev, struct tilcdc_encoder,
+					    base, DRM_MODE_ENCODER_NONE);
+	if (IS_ERR(encoder)) {
+		dev_err(ddev->dev, "drm_encoder_init() failed %pe\n", encoder);
+		return PTR_ERR(encoder);
 	}
+	priv->encoder = encoder;
 
 	if (panel) {
 		bridge = devm_drm_panel_bridge_add_typed(ddev->dev, panel,
 							 DRM_MODE_CONNECTOR_DPI);
-		if (IS_ERR(bridge)) {
-			ret = PTR_ERR(bridge);
-			goto err_encoder_cleanup;
-		}
+		if (IS_ERR(bridge))
+			return PTR_ERR(bridge);
 	}
 
-	ret = tilcdc_attach_bridge(ddev, bridge);
-	if (ret)
-		goto err_encoder_cleanup;
-
-	return 0;
-
-err_encoder_cleanup:
-	drm_encoder_cleanup(priv->encoder);
-	return ret;
+	return tilcdc_attach_bridge(ddev, bridge);
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
index a77a5b22ebd96..d98a1ae0e31f8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
@@ -14,7 +14,6 @@
 static const struct drm_plane_funcs tilcdc_plane_funcs = {
 	.update_plane	= drm_atomic_helper_update_plane,
 	.disable_plane	= drm_atomic_helper_disable_plane,
-	.destroy	= drm_plane_cleanup,
 	.reset		= drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
@@ -98,22 +97,20 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = {
 	.atomic_update = tilcdc_plane_atomic_update,
 };
 
-int tilcdc_plane_init(struct drm_device *dev,
-		      struct drm_plane *plane)
+struct tilcdc_plane *tilcdc_plane_init(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = ddev_to_tilcdc_priv(dev);
-	int ret;
-
-	ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_funcs,
-				       priv->pixelformats,
-				       priv->num_pixelformats,
-				       NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
-	if (ret) {
-		dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);
-		return ret;
-	}
+	struct tilcdc_plane *plane;
 
-	drm_plane_helper_add(plane, &plane_helper_funcs);
+	plane = drmm_universal_plane_alloc(dev, struct tilcdc_plane, base,
+					   1, &tilcdc_plane_funcs,
+					   priv->pixelformats,
+					   priv->num_pixelformats,
+					   NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (IS_ERR(plane))
+		return plane;
 
-	return 0;
+	drm_plane_helper_add(&plane->base, &plane_helper_funcs);
+
+	return plane;
 }

-- 
2.43.0