Commit 323401c9 authored by Kory Maincent (TI.com)'s avatar Kory Maincent (TI.com) Committed by Luca Ceresoli
Browse files

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: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
Signed-off-by: default avatarKory Maincent (TI.com) <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20260123-feature_tilcdc-v5-18-5a44d2aa3f6f@bootlin.com


Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
parent 968fefee

drivers/gpu/drm/tilcdc/tilcdc_crtc.c

+28 −27
Original line number Diff line number Diff line
@@ -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;

}

drivers/gpu/drm/tilcdc/tilcdc_drv.c

+1 −4
Original line number Diff line number Diff line
@@ -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);

drivers/gpu/drm/tilcdc/tilcdc_drv.h

+10 −3
Original line number Diff line number Diff line
@@ -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__ */

drivers/gpu/drm/tilcdc/tilcdc_encoder.c

+13 −25
Original line number Diff line number Diff line
@@ -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);

}

drivers/gpu/drm/tilcdc/tilcdc_plane.c

+12 −15
Original line number Diff line number Diff line
@@ -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;

	struct tilcdc_plane *plane;



	ret = drm_universal_plane_init(dev, plane, 1, &tilcdc_plane_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 (ret) {

		dev_err(dev->dev, "Failed to initialize plane: %d\n", ret);

		return ret;

	}

	if (IS_ERR(plane))

		return plane;



	drm_plane_helper_add(plane, &plane_helper_funcs);

	drm_plane_helper_add(&plane->base, &plane_helper_funcs);



	return 0;

	return plane;

}