Hi,

the imx driver is well maintained IIRC. I'd advise you to split off this patch from the series and send it out separately. That is more likely to catch the attention of the driver's maintainers.

Best regards
Thomas

Am 04.07.26 um 20:31 schrieb Ze Huang:
Convert i.MX LCDC to explicit primary plane, CRTC and encoder objects.

Keep no-scaling plane check and GEM framebuffer prepare callback from
simple-KMS path. Only touch hardware state when framebuffer exists,
since atomic plane updates can run on disabling transitions.

Signed-off-by: Ze Huang <[email protected]>
---
  drivers/gpu/drm/imx/lcdc/imx-lcdc.c | 178 ++++++++++++++++++++++++++----------
  1 file changed, 130 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c 
b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
index f52832b43aca..d091dc562098 100644
--- a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
+++ b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c
@@ -14,9 +14,9 @@
  #include <drm/drm_gem_dma_helper.h>
  #include <drm/drm_gem_framebuffer_helper.h>
  #include <drm/drm_of.h>
+#include <drm/drm_plane_helper.h>
  #include <drm/drm_print.h>
  #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
  #include <drm/drm_vblank.h>
  #include <linux/bitfield.h>
  #include <linux/clk.h>
@@ -102,7 +102,9 @@
struct imx_lcdc {
        struct drm_device drm;
-       struct drm_simple_display_pipe pipe;
+       struct drm_plane plane;
+       struct drm_crtc crtc;
+       struct drm_encoder encoder;
        struct drm_connector *connector;
        void __iomem *base;
@@ -135,14 +137,13 @@ static unsigned int imx_lcdc_get_format(unsigned int drm_format)
        }
  }
-static void imx_lcdc_update_hw_registers(struct drm_simple_display_pipe *pipe,
+static void imx_lcdc_update_hw_registers(struct drm_crtc *crtc,
                                         struct drm_plane_state *old_state,
                                         bool mode_set)
  {
-       struct drm_crtc *crtc = &pipe->crtc;
-       struct drm_plane_state *new_state = pipe->plane.state;
+       struct drm_plane_state *new_state = crtc->primary->state;
        struct drm_framebuffer *fb = new_state->fb;
-       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(pipe->crtc.dev);
+       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(crtc->dev);
        u32 lpcr, lvcr, lhcr;
        u32 framesize;
        dma_addr_t addr;
@@ -188,16 +189,16 @@ static void imx_lcdc_update_hw_registers(struct 
drm_simple_display_pipe *pipe,
                clk_prepare_enable(lcdc->clk_per);
  }
-static void imx_lcdc_pipe_enable(struct drm_simple_display_pipe *pipe,
-                                struct drm_crtc_state *crtc_state,
-                                struct drm_plane_state *plane_state)
+static void imx_lcdc_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+                                              struct drm_atomic_commit *state)
  {
        int ret;
        int clk_div;
        int bpp;
-       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(pipe->crtc.dev);
-       struct drm_display_mode *mode = &pipe->crtc.mode;
+       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(crtc->dev);
+       struct drm_display_mode *mode = &crtc->mode;
        struct drm_display_info *disp_info = &lcdc->connector->display_info;
+       struct drm_plane_state *plane_state = crtc->primary->state;
        const int hsync_pol = (mode->flags & DRM_MODE_FLAG_PHSYNC) ? 0 : 1;
        const int vsync_pol = (mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : 1;
        const int data_enable_pol =
@@ -231,34 +232,34 @@ static void imx_lcdc_pipe_enable(struct 
drm_simple_display_pipe *pipe,
ret = clk_prepare_enable(lcdc->clk_ipg);
        if (ret) {
-               dev_err(pipe->crtc.dev->dev, "Cannot enable ipg clock: %pe\n", 
ERR_PTR(ret));
+               dev_err(crtc->dev->dev, "Cannot enable ipg clock: %pe\n", 
ERR_PTR(ret));
                return;
        }
        ret = clk_prepare_enable(lcdc->clk_ahb);
        if (ret) {
-               dev_err(pipe->crtc.dev->dev, "Cannot enable ahb clock: %pe\n", 
ERR_PTR(ret));
+               dev_err(crtc->dev->dev, "Cannot enable ahb clock: %pe\n", 
ERR_PTR(ret));
clk_disable_unprepare(lcdc->clk_ipg); return;
        }
- imx_lcdc_update_hw_registers(pipe, NULL, true);
+       imx_lcdc_update_hw_registers(crtc, NULL, true);
/* Enable VBLANK Interrupt */
        writel(INTR_EOF, lcdc->base + IMX21LCDC_LIER);
  }
-static void imx_lcdc_pipe_disable(struct drm_simple_display_pipe *pipe)
+static void imx_lcdc_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+                                               struct drm_atomic_commit *state)
  {
-       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(pipe->crtc.dev);
-       struct drm_crtc *crtc = &lcdc->pipe.crtc;
+       struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(crtc->dev);
        struct drm_pending_vblank_event *event;
clk_disable_unprepare(lcdc->clk_ahb);
        clk_disable_unprepare(lcdc->clk_ipg);
- if (pipe->crtc.enabled)
+       if (crtc->enabled)
                clk_disable_unprepare(lcdc->clk_per);
spin_lock_irq(&lcdc->drm.event_lock);
@@ -273,17 +274,18 @@ static void imx_lcdc_pipe_disable(struct 
drm_simple_display_pipe *pipe)
        writel(0, lcdc->base + IMX21LCDC_LIER);
  }
-static int imx_lcdc_pipe_check(struct drm_simple_display_pipe *pipe,
-                              struct drm_plane_state *plane_state,
-                              struct drm_crtc_state *crtc_state)
+static int imx_lcdc_crtc_helper_atomic_check(struct drm_crtc *crtc,
+                                            struct drm_atomic_commit *state)
  {
+       struct drm_crtc_state *crtc_state = 
drm_atomic_get_new_crtc_state(state, crtc);
        const struct drm_display_mode *mode = &crtc_state->mode;
-       const struct drm_display_mode *old_mode = &pipe->crtc.state->mode;
+       const struct drm_display_mode *old_mode = &crtc->state->mode;
+       int ret;
if (mode->hdisplay < LCDC_MIN_XRES || mode->hdisplay > LCDC_MAX_XRES ||
            mode->vdisplay < LCDC_MIN_YRES || mode->vdisplay > LCDC_MAX_YRES ||
            mode->hdisplay % 0x10) { /* must be multiple of 16 */
-               drm_err(pipe->crtc.dev, "unsupported display mode (%u x %u)\n",
+               drm_err(crtc->dev, "unsupported display mode (%u x %u)\n",
                        mode->hdisplay, mode->vdisplay);
                return -EINVAL;
        }
@@ -292,27 +294,42 @@ static int imx_lcdc_pipe_check(struct 
drm_simple_display_pipe *pipe,
                old_mode->hdisplay != mode->hdisplay ||
                old_mode->vdisplay != mode->vdisplay;
- return 0;
+       if (!crtc_state->enable)
+               goto out;
+
+       ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+       if (ret)
+               return ret;
+
+out:
+       return drm_atomic_add_affected_planes(state, crtc);
  }
-static void imx_lcdc_pipe_update(struct drm_simple_display_pipe *pipe,
-                                struct drm_plane_state *old_state)
+static void imx_lcdc_plane_helper_atomic_update(struct drm_plane *plane,
+                                               struct drm_atomic_commit *state)
  {
-       struct drm_crtc *crtc = &pipe->crtc;
-       struct drm_pending_vblank_event *event = crtc->state->event;
-       struct drm_plane_state *new_state = pipe->plane.state;
+       struct drm_plane_state *old_state = 
drm_atomic_get_old_plane_state(state, plane);
+       struct drm_plane_state *new_state = plane->state;
+       struct drm_crtc *crtc = new_state->crtc ?: old_state->crtc;
+       struct drm_pending_vblank_event *event;
        struct drm_framebuffer *fb = new_state->fb;
        struct drm_framebuffer *old_fb = old_state->fb;
        struct drm_crtc *old_crtc = old_state->crtc;
        bool mode_changed = false;
- if (old_fb && old_fb->format != fb->format)
-               mode_changed = true;
-       else if (old_crtc != crtc)
-               mode_changed = true;
+       if (!crtc)
+               return;
- imx_lcdc_update_hw_registers(pipe, old_state, mode_changed);
+       if (fb) {
+               if (old_fb && old_fb->format != fb->format)
+                       mode_changed = true;
+               else if (old_crtc != crtc)
+                       mode_changed = true;
+ imx_lcdc_update_hw_registers(crtc, old_state, mode_changed);
+       }
+
+       event = crtc->state->event;
        if (event) {
                crtc->state->event = NULL;
@@ -327,11 +344,56 @@ static void imx_lcdc_pipe_update(struct drm_simple_display_pipe *pipe,
        }
  }
-static const struct drm_simple_display_pipe_funcs imx_lcdc_pipe_funcs = {
-       .enable = imx_lcdc_pipe_enable,
-       .disable = imx_lcdc_pipe_disable,
-       .check = imx_lcdc_pipe_check,
-       .update = imx_lcdc_pipe_update,
+static int imx_lcdc_plane_helper_atomic_check(struct drm_plane *plane,
+                                             struct drm_atomic_commit *state)
+{
+       struct drm_plane_state *plane_state = 
drm_atomic_get_new_plane_state(state, plane);
+       struct drm_crtc *crtc = plane_state->crtc;
+       struct drm_crtc_state *crtc_state = NULL;
+       int ret;
+
+       if (crtc)
+               crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+       ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+                                                 DRM_PLANE_NO_SCALING,
+                                                 DRM_PLANE_NO_SCALING,
+                                                 false, false);
+       return ret;
+}
+
+static const struct drm_plane_helper_funcs imx_lcdc_plane_helper_funcs = {
+       .prepare_fb     = drm_gem_plane_helper_prepare_fb,
+       .atomic_check   = imx_lcdc_plane_helper_atomic_check,
+       .atomic_update  = imx_lcdc_plane_helper_atomic_update,
+};
+
+static const struct drm_plane_funcs imx_lcdc_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,
+};
+
+static const struct drm_crtc_helper_funcs imx_lcdc_crtc_helper_funcs = {
+       .atomic_check   = imx_lcdc_crtc_helper_atomic_check,
+       .atomic_enable  = imx_lcdc_crtc_helper_atomic_enable,
+       .atomic_disable = imx_lcdc_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs imx_lcdc_crtc_funcs = {
+       .reset                  = drm_atomic_helper_crtc_reset,
+       .destroy                = drm_crtc_cleanup,
+       .set_config             = drm_atomic_helper_set_config,
+       .page_flip              = drm_atomic_helper_page_flip,
+       .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+       .atomic_destroy_state   = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_encoder_funcs imx_lcdc_encoder_funcs = {
+       .destroy = drm_encoder_cleanup,
  };
static const struct drm_mode_config_funcs imx_lcdc_mode_config_funcs = {
@@ -369,7 +431,7 @@ MODULE_DEVICE_TABLE(of, imx_lcdc_of_dev_id);
  static irqreturn_t imx_lcdc_irq_handler(int irq, void *arg)
  {
        struct imx_lcdc *lcdc = arg;
-       struct drm_crtc *crtc = &lcdc->pipe.crtc;
+       struct drm_crtc *crtc = &lcdc->crtc;
        unsigned int status;
status = readl(lcdc->base + IMX21LCDC_LISR);
@@ -387,6 +449,9 @@ static int imx_lcdc_probe(struct platform_device *pdev)
        struct imx_lcdc *lcdc;
        struct drm_device *drm;
        struct drm_bridge *bridge;
+       struct drm_plane *plane;
+       struct drm_crtc *crtc;
+       struct drm_encoder *encoder;
        int irq;
        int ret;
        struct device *dev = &pdev->dev;
@@ -428,23 +493,40 @@ static int imx_lcdc_probe(struct platform_device *pdev)
        if (ret)
                return dev_err_probe(dev, ret, "Cannot initialize mode configuration 
structure\n");
- /* CRTC, Plane, Encoder */
-       ret = drm_simple_display_pipe_init(drm, &lcdc->pipe,
-                                          &imx_lcdc_pipe_funcs,
-                                          imx_lcdc_formats,
-                                          ARRAY_SIZE(imx_lcdc_formats), NULL, 
NULL);
+       plane = &lcdc->plane;
+       ret = drm_universal_plane_init(drm, plane, 0,
+                                      &imx_lcdc_plane_funcs,
+                                      imx_lcdc_formats,
+                                      ARRAY_SIZE(imx_lcdc_formats),
+                                      NULL,
+                                      DRM_PLANE_TYPE_PRIMARY, NULL);
+       if (ret < 0)
+               return dev_err_probe(drm->dev, ret, "Cannot initialize primary 
plane\n");
+       drm_plane_helper_add(plane, &imx_lcdc_plane_helper_funcs);
+
+       crtc = &lcdc->crtc;
+       ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL,
+                                       &imx_lcdc_crtc_funcs, NULL);
+       if (ret < 0)
+               return dev_err_probe(drm->dev, ret, "Cannot initialize CRTC\n");
+       drm_crtc_helper_add(crtc, &imx_lcdc_crtc_helper_funcs);
+
+       encoder = &lcdc->encoder;
+       ret = drm_encoder_init(drm, encoder, &imx_lcdc_encoder_funcs,
+                              DRM_MODE_ENCODER_NONE, NULL);
        if (ret < 0)
-               return dev_err_probe(drm->dev, ret, "Cannot setup simple display 
pipe\n");
+               return dev_err_probe(drm->dev, ret, "Cannot initialize 
encoder\n");
+       encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
        if (ret < 0)
                return dev_err_probe(drm->dev, ret, "Failed to initialize 
vblank\n");
- ret = drm_bridge_attach(&lcdc->pipe.encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+       ret = drm_bridge_attach(encoder, bridge, NULL, 
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
        if (ret)
                return dev_err_probe(drm->dev, ret, "Cannot attach bridge\n");
- lcdc->connector = drm_bridge_connector_init(drm, &lcdc->pipe.encoder);
+       lcdc->connector = drm_bridge_connector_init(drm, encoder);
        if (IS_ERR(lcdc->connector))
                return dev_err_probe(drm->dev, PTR_ERR(lcdc->connector), "Cannot 
init bridge connector\n");

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)


Reply via email to