drm/tilcdc: Convert to DRM managed resources

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>
Link: https://patch.msgid.link/20260123-feature_tilcdc-v5-18-5a44d2aa3f6f@bootlin.com
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 0bd99a2efeeb4bcfd4031e355d4f38f3e4f3bab9..2916de3dce91e4b5755ec09f58186fa8f6d6a150 100644 (file)
--- 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 @@ out:
        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 1a238a22309f4a67cf5903c6e82df619610f1f3b..3b11d296a7e91e67829e87e8504ccc3a2c4808c9 100644 (file)
--- 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 @@ unregister_cpufreq_notif:
 #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 c69e279a2539d11f86a513a194ff83676256a33d..17d152f9f0b69faf9da5adf564eb11fc29cb701a 100644 (file)
--- 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 d42be3e16c536e1624f8c415af991461f6c50570..1ee5761757a8c9eb2b71cdd83471b4dedaea5fdd 100644 (file)
--- 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 a77a5b22ebd969d0538b39f0ce94d129d373855d..d98a1ae0e31f836888f4d0e063e5e11c82b91f60 100644 (file)
--- 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;
 }