[PATCH v2 14/20] drm/tilcdc: Modernize driver initialization and cleanup paths

Kory Maincent (TI.com)
2025-12-11T16:40:24Z

Refactor the driver initialization to use modern DRM managed resource
APIs, simplifying the code.

The tilcdc_init and tilcdc_fini wrapper functions are removed since they
served no purpose after the component framework was eliminated. Their
logic is integrated directly into probe and remove.

Key changes:
- Use devm_drm_dev_alloc() instead of drm_dev_alloc().
- Use drmm_mode_config_init() instead of drm_mode_config_init().
- Align the remove path with the probe error path to ensure consistent
cleanup ordering in both success and failure cases.
- Adjust platform_set_drvdata() to store the private structure instead
of the drm_device, matching the new allocation pattern.

These changes reduce error-prone code while maintaining the same
functional behavior.

Signed-off-by: Kory Maincent (TI.com) <kory.maincent@bootlin.com>
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 308 ++++++++++++++++--------------------
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 +
2 files changed, 139 insertions(+), 171 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4630e56c3f028..2cbe390a1ee54 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -139,39 +139,125 @@ static void tilcdc_irq_uninstall(struct drm_device *dev)
* DRM operations:
*/

-static void tilcdc_fini(struct drm_device *dev)
+#if defined(CONFIG_DEBUG_FS)
+static const struct {
+ const char *name;
+ uint8_t rev;
+ uint8_t save;
+ uint32_t reg;
+} registers[] = {
+#define REG(rev, save, reg) { #reg, rev, save, reg }
+ /* exists in revision 1: */
+ REG(1, false, LCDC_PID_REG),
+ REG(1, true, LCDC_CTRL_REG),
+ REG(1, false, LCDC_STAT_REG),
+ REG(1, true, LCDC_RASTER_CTRL_REG),
+ REG(1, true, LCDC_RASTER_TIMING_0_REG),
+ REG(1, true, LCDC_RASTER_TIMING_1_REG),
+ REG(1, true, LCDC_RASTER_TIMING_2_REG),
+ REG(1, true, LCDC_DMA_CTRL_REG),
+ REG(1, true, LCDC_DMA_FB_BASE_ADDR_0_REG),
+ REG(1, true, LCDC_DMA_FB_CEILING_ADDR_0_REG),
+ REG(1, true, LCDC_DMA_FB_BASE_ADDR_1_REG),
+ REG(1, true, LCDC_DMA_FB_CEILING_ADDR_1_REG),
+ /* new in revision 2: */
+ REG(2, false, LCDC_RAW_STAT_REG),
+ REG(2, false, LCDC_MASKED_STAT_REG),
+ REG(2, true, LCDC_INT_ENABLE_SET_REG),
+ REG(2, false, LCDC_INT_ENABLE_CLR_REG),
+ REG(2, false, LCDC_END_OF_INT_IND_REG),
+ REG(2, true, LCDC_CLK_ENABLE_REG),
+#undef REG
+};
+
+static int tilcdc_regs_show(struct seq_file *m, void *arg)
{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_device *dev = node->minor->dev;
struct tilcdc_drm_private *priv = dev->dev_private;
+ unsigned i;

-#ifdef CONFIG_CPU_FREQ
- if (priv->freq_transition.notifier_call)
- cpufreq_unregister_notifier(&priv->freq_transition,
- CPUFREQ_TRANSITION_NOTIFIER);
+ pm_runtime_get_sync(dev->dev);
+
+ seq_printf(m, "revision: %d\n", priv->rev);
+
+ for (i = 0; i < ARRAY_SIZE(registers); i++)
+ if (priv->rev >= registers[i].rev)
+ seq_printf(m, "%s:\t %08x\n", registers[i].name,
+ tilcdc_read(dev, registers[i].reg));
+
+ pm_runtime_put_sync(dev->dev);
+
+ return 0;
+}
+
+static int tilcdc_mm_show(struct seq_file *m, void *arg)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_printer p = drm_seq_file_printer(m);
+ drm_mm_print(&dev->vma_offset_manager->vm_addr_space_mm, &p);
+ return 0;
+}
+
+static struct drm_info_list tilcdc_debugfs_list[] = {
+ { "regs", tilcdc_regs_show, 0, NULL },
+ { "mm", tilcdc_mm_show, 0, NULL },
+};
+
+static void tilcdc_debugfs_init(struct drm_minor *minor)
+{
+ drm_debugfs_create_files(tilcdc_debugfs_list,
+ ARRAY_SIZE(tilcdc_debugfs_list),
+ minor->debugfs_root, minor);
+}
#endif

- if (priv->crtc)
- tilcdc_crtc_shutdown(priv->crtc);
+DEFINE_DRM_GEM_DMA_FOPS(fops);

- drm_dev_unregister(dev);
+static const struct drm_driver tilcdc_driver = {
+ .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+ DRM_GEM_DMA_DRIVER_OPS,
+ DRM_FBDEV_DMA_DRIVER_OPS,
+#ifdef CONFIG_DEBUG_FS
+ .debugfs_init = tilcdc_debugfs_init,
+#endif
+ .fops = &fops,
+ .name = "tilcdc",
+ .desc = "TI LCD Controller DRM",
+ .major = 1,
+ .minor = 0,
+};

- drm_kms_helper_poll_fini(dev);
- drm_atomic_helper_shutdown(dev);
- tilcdc_irq_uninstall(dev);
- drm_mode_config_cleanup(dev);
+/*
+ * Power management:
+ */

- if (priv->clk)
- clk_put(priv->clk);
+static int tilcdc_pm_suspend(struct device *dev)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ int ret = 0;

- if (priv->wq)
- destroy_workqueue(priv->wq);
+ ret = drm_mode_config_helper_suspend(ddev);
+
+ /* Select sleep pin state */
+ pinctrl_pm_select_sleep_state(dev);

- dev->dev_private = NULL;
+ return ret;
+}

- pm_runtime_disable(dev->dev);
+static int tilcdc_pm_resume(struct device *dev)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);

- drm_dev_put(dev);
+ /* Select default pin state */
+ pinctrl_pm_select_default_state(dev);
+ return drm_mode_config_helper_resume(ddev);
}

+static DEFINE_SIMPLE_DEV_PM_OPS(tilcdc_pm_ops,
+ tilcdc_pm_suspend, tilcdc_pm_resume);
+
static const struct of_device_id tilcdc_of_match[] = {
{ .compatible = "ti,am33xx-tilcdc", .data = (void *)AM33XX_TILCDC},
{ .compatible = "ti,da850-tilcdc", .data = (void *)DA850_TILCDC},
@@ -179,37 +265,35 @@ static const struct of_device_id tilcdc_of_match[] = {
};
MODULE_DEVICE_TABLE(of, tilcdc_of_match);

-static int tilcdc_init(const struct drm_driver *ddrv, struct device *dev)
+static int tilcdc_pdev_probe(struct platform_device *pdev)
{
- struct drm_device *ddev;
- struct platform_device *pdev = to_platform_device(dev);
- struct device_node *node = dev->of_node;
+ struct device_node *node = pdev->dev.of_node;
const struct of_device_id *of_id;
struct tilcdc_drm_private *priv;
+ struct device *dev = &pdev->dev;
+ struct drm_device *ddev;
u32 bpp = 0;
int ret;

- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- ddev = drm_dev_alloc(ddrv, dev);
- if (IS_ERR(ddev))
- return PTR_ERR(ddev);
+ priv = devm_drm_dev_alloc(dev, &tilcdc_driver,
+ struct tilcdc_drm_private, ddev);
+ if (IS_ERR(priv))
+ return PTR_ERR(priv);

of_id = of_match_node(tilcdc_of_match, node);
if (!of_id)
return -ENODEV;

+ platform_set_drvdata(pdev, priv);
+ ddev = &priv->ddev;
ddev->dev_private = priv;
- platform_set_drvdata(pdev, ddev);
- drm_mode_config_init(ddev);
+ ret = drmm_mode_config_init(ddev);
+ if (ret)
+ return ret;

priv->wq = alloc_ordered_workqueue("tilcdc", 0);
- if (!priv->wq) {
- ret = -ENOMEM;
- goto put_drm;
- }
+ if (!priv->wq)
+ return -ENOMEM;

priv->mmio = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(priv->mmio)) {
@@ -375,152 +459,34 @@ static int tilcdc_init(const struct drm_driver *ddrv, struct device *dev)
clk_put(priv->clk);
free_wq:
destroy_workqueue(priv->wq);
-put_drm:
- platform_set_drvdata(pdev, NULL);
- ddev->dev_private = NULL;
- drm_dev_put(ddev);

return ret;
}

-#if defined(CONFIG_DEBUG_FS)
-static const struct {
- const char *name;
- uint8_t rev;
- uint8_t save;
- uint32_t reg;
-} registers[] = {
-#define REG(rev, save, reg) { #reg, rev, save, reg }
- /* exists in revision 1: */
- REG(1, false, LCDC_PID_REG),
- REG(1, true, LCDC_CTRL_REG),
- REG(1, false, LCDC_STAT_REG),
- REG(1, true, LCDC_RASTER_CTRL_REG),
- REG(1, true, LCDC_RASTER_TIMING_0_REG),
- REG(1, true, LCDC_RASTER_TIMING_1_REG),
- REG(1, true, LCDC_RASTER_TIMING_2_REG),
- REG(1, true, LCDC_DMA_CTRL_REG),
- REG(1, true, LCDC_DMA_FB_BASE_ADDR_0_REG),
- REG(1, true, LCDC_DMA_FB_CEILING_ADDR_0_REG),
- REG(1, true, LCDC_DMA_FB_BASE_ADDR_1_REG),
- REG(1, true, LCDC_DMA_FB_CEILING_ADDR_1_REG),
- /* new in revision 2: */
- REG(2, false, LCDC_RAW_STAT_REG),
- REG(2, false, LCDC_MASKED_STAT_REG),
- REG(2, true, LCDC_INT_ENABLE_SET_REG),
- REG(2, false, LCDC_INT_ENABLE_CLR_REG),
- REG(2, false, LCDC_END_OF_INT_IND_REG),
- REG(2, true, LCDC_CLK_ENABLE_REG),
-#undef REG
-};
-
-static int tilcdc_regs_show(struct seq_file *m, void *arg)
-{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
- struct tilcdc_drm_private *priv = dev->dev_private;
- unsigned i;
-
- pm_runtime_get_sync(dev->dev);
-
- seq_printf(m, "revision: %d\n", priv->rev);
-
- for (i = 0; i < ARRAY_SIZE(registers); i++)
- if (priv->rev >= registers[i].rev)
- seq_printf(m, "%s:\t %08x\n", registers[i].name,
- tilcdc_read(dev, registers[i].reg));
-
- pm_runtime_put_sync(dev->dev);
-
- return 0;
-}
-
-static int tilcdc_mm_show(struct seq_file *m, void *arg)
-{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
- struct drm_printer p = drm_seq_file_printer(m);
- drm_mm_print(&dev->vma_offset_manager->vm_addr_space_mm, &p);
- return 0;
-}
-
-static struct drm_info_list tilcdc_debugfs_list[] = {
- { "regs", tilcdc_regs_show, 0, NULL },
- { "mm", tilcdc_mm_show, 0, NULL },
-};
-
-static void tilcdc_debugfs_init(struct drm_minor *minor)
+static void tilcdc_pdev_remove(struct platform_device *pdev)
{
- drm_debugfs_create_files(tilcdc_debugfs_list,
- ARRAY_SIZE(tilcdc_debugfs_list),
- minor->debugfs_root, minor);
-}
-#endif
-
-DEFINE_DRM_GEM_DMA_FOPS(fops);
+ struct tilcdc_drm_private *priv = platform_get_drvdata(pdev);
+ struct drm_device *ddev = &priv->ddev;

-static const struct drm_driver tilcdc_driver = {
- .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- DRM_GEM_DMA_DRIVER_OPS,
- DRM_FBDEV_DMA_DRIVER_OPS,
-#ifdef CONFIG_DEBUG_FS
- .debugfs_init = tilcdc_debugfs_init,
+ drm_dev_unregister(ddev);
+ drm_kms_helper_poll_fini(ddev);
+ tilcdc_irq_uninstall(ddev);
+#ifdef CONFIG_CPU_FREQ
+ cpufreq_unregister_notifier(&priv->freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
#endif
- .fops = &fops,
- .name = "tilcdc",
- .desc = "TI LCD Controller DRM",
- .major = 1,
- .minor = 0,
-};
-
-/*
- * Power management:
- */
-
-static int tilcdc_pm_suspend(struct device *dev)
-{
- struct drm_device *ddev = dev_get_drvdata(dev);
- int ret = 0;
-
- ret = drm_mode_config_helper_suspend(ddev);
-
- /* Select sleep pin state */
- pinctrl_pm_select_sleep_state(dev);
-
- return ret;
-}
-
-static int tilcdc_pm_resume(struct device *dev)
-{
- struct drm_device *ddev = dev_get_drvdata(dev);
-
- /* Select default pin state */
- pinctrl_pm_select_default_state(dev);
- return drm_mode_config_helper_resume(ddev);
-}
-
-static DEFINE_SIMPLE_DEV_PM_OPS(tilcdc_pm_ops,
- tilcdc_pm_suspend, tilcdc_pm_resume);
-
-static int tilcdc_pdev_probe(struct platform_device *pdev)
-{
- /* bail out early if no DT data: */
- if (!pdev->dev.of_node) {
- dev_err(&pdev->dev, "device-tree data is missing\n");
- return -ENXIO;
- }
-
- return tilcdc_init(&tilcdc_driver, &pdev->dev);
-}
-
-static void tilcdc_pdev_remove(struct platform_device *pdev)
-{
- tilcdc_fini(platform_get_drvdata(pdev));
+ tilcdc_crtc_destroy(priv->crtc);
+ pm_runtime_disable(&pdev->dev);
+ clk_put(priv->clk);
+ destroy_workqueue(priv->wq);
}

static void tilcdc_pdev_shutdown(struct platform_device *pdev)
{
- drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
+ struct tilcdc_drm_private *priv = platform_get_drvdata(pdev);
+ struct drm_device *ddev = &priv->ddev;
+
+ drm_atomic_helper_shutdown(ddev);
}

static struct platform_driver tilcdc_platform_driver = {
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 60e85e29b1063..e3d04a3eb25b4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -48,6 +48,8 @@ struct tilcdc_drm_private {

unsigned int irq;

+ struct drm_device ddev;
+
/* don't attempt resolutions w/ higher W * H * Hz: */
uint32_t max_bandwidth;
/*

--
2.43.0