[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251126-feature_tilcdc-v1-15-49b9ef2e3aa0@bootlin.com>
Date: Wed, 26 Nov 2025 18:35:57 +0100
From: "Kory Maincent (TI.com)" <kory.maincent@...tlin.com>
To: Jyri Sarha <jyri.sarha@....fi>,
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Russell King <linux@...linux.org.uk>,
Bartosz Golaszewski <brgl@...ev.pl>, Tony Lindgren <tony@...mide.com>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>
Cc: Markus Schneider-Pargmann <msp@...libre.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
Louis Chauvet <louis.chauvet@...tlin.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Miguel Gazquez <miguel.gazquez@...tlin.com>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-omap@...r.kernel.org,
"Kory Maincent (TI.com)" <kory.maincent@...tlin.com>
Subject: [PATCH 15/21] drm/tilcdc: Modernize driver initialization and
cleanup paths
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@...tlin.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 9b31eaafd2be9..e08bea30585e0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -134,65 +134,149 @@ 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 int tilcdc_init(const struct drm_driver *ddrv, struct device *dev)
+static DEFINE_SIMPLE_DEV_PM_OPS(tilcdc_pm_ops,
+ tilcdc_pm_suspend, tilcdc_pm_resume);
+
+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;
+ struct device *dev = &pdev->dev;
struct tilcdc_drm_private *priv;
+ 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);
+ 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)) {
@@ -355,152 +439,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 const struct of_device_id tilcdc_of_match[] = {
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
Powered by blists - more mailing lists