|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v2] drm/imx: replace struct drm_simple_display_pipe with regular atomic helpers
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. Wire the vblank lifecycle explicitly with CRTC vblank callbacks and drm_crtc_vblank_on()/drm_crtc_vblank_off(). Use the old CRTC state in the disable path for clock unwinding so the clock reference count remains paired with the previous active state. Signed-off-by: Ze Huang <ze.huang@xxxxxxxxxxxxxxxx> --- struct drm_simple_display_pipe was meant to simplify simple DRM drivers, but instead adds an extra wrapper around normal DRM atomic helper setup. As noted in Documentation/gpu/todo.rst, remaining users should be converted to regular atomic helpers and stop depending on the simple-KMS interfaces. Each patch replaces drm_simple_display_pipe_init() with explicit primary plane, CRTC and encoder setup, and moves the old simple-pipe callbacks into regular plane and CRTC helper callbacks named according to local driver conventions. The conversions preserve helper behavior that used to be implicit in drm_simple_kms_helper.c, including plane-state validation, CRTC primary-plane checks, affected-plane propagation, framebuffer prepare handling, and existing event/vblank flow where applicable. Result is less helper indirection and more explicit driver-side atomic wiring, with no remaining simple-KMS dependency in these drivers. Changes are build-tested only. No hardware testing has been performed. This patch is based on drm-next-2026-06-27. AI usage disclosure: - AI tools were also used to review the code and suggest code changes for the DRM atomic conversion. Thanks, Ze Huang --- Changes in v2: - use 'commit' as name of struct drm_atomic_commit in atomic helpers - improve control flow in *_crtc_helper_atomic_check() and *_plane_helper_atomic_check() - Moved page-flip/vblank event handling out of plane update paths and into CRTC atomic_flush(), using atomic_flush and disable paths - decide clock disable by old crtc state - add calls to drm_crtc_vblank_on() and drm_crtc_vblank_off() in crtc atomic_enable() and atomic_disbale() - test crtc_state->enable before mode size check - Link to v1: https://patch.msgid.link/20260705-drm-simple-kms-removal-v1-0-b4e1ca053623@xxxxxxxxxxxxxxxx --- drivers/gpu/drm/imx/lcdc/imx-lcdc.c | 217 ++++++++++++++++++++++++++---------- 1 file changed, 159 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/imx/lcdc/imx-lcdc.c b/drivers/gpu/drm/imx/lcdc/imx-lcdc.c index f52832b43aca..78eac2c4a54c 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 imx_lcdc *lcdc = imx_lcdc_from_drmdev(crtc->dev); + struct drm_plane_state *new_state = lcdc->plane.state; struct drm_framebuffer *fb = new_state->fb; - struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(pipe->crtc.dev); u32 lpcr, lvcr, lhcr; u32 framesize; dma_addr_t addr; @@ -188,15 +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 *commit) { 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_crtc_state *cstate = drm_atomic_get_new_crtc_state(commit, crtc); + struct drm_plane_state *plane_state = lcdc->plane.state; + struct drm_display_mode *mode = &cstate->mode; struct drm_display_info *disp_info = &lcdc->connector->display_info; const int hsync_pol = (mode->flags & DRM_MODE_FLAG_PHSYNC) ? 0 : 1; const int vsync_pol = (mode->flags & DRM_MODE_FLAG_PVSYNC) ? 0 : 1; @@ -231,40 +233,46 @@ 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); + + drm_crtc_vblank_on(crtc); } -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 *commit) { - struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(pipe->crtc.dev); - struct drm_crtc *crtc = &lcdc->pipe.crtc; + struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(commit, crtc); + struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(commit, crtc); + struct imx_lcdc *lcdc = imx_lcdc_from_drmdev(crtc->dev); struct drm_pending_vblank_event *event; + drm_crtc_vblank_off(crtc); + clk_disable_unprepare(lcdc->clk_ahb); clk_disable_unprepare(lcdc->clk_ipg); - if (pipe->crtc.enabled) + if (old_crtc_state->enable) clk_disable_unprepare(lcdc->clk_per); spin_lock_irq(&lcdc->drm.event_lock); - event = crtc->state->event; + event = new_crtc_state->event; if (event) { - crtc->state->event = NULL; + new_crtc_state->event = NULL; drm_crtc_send_vblank_event(crtc, event); } spin_unlock_irq(&lcdc->drm.event_lock); @@ -273,17 +281,20 @@ 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 *commit) { + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(commit, crtc); + struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(commit, 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 = &old_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", + if (crtc_state->enable && + (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(crtc->dev, "unsupported display mode (%u x %u)\n", mode->hdisplay, mode->vdisplay); return -EINVAL; } @@ -292,46 +303,116 @@ 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) { + ret = drm_atomic_helper_check_crtc_primary_plane(crtc_state); + if (ret) + return ret; + } + + return drm_atomic_add_affected_planes(commit, 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 *commit) { - 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(commit, plane); + struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(commit, plane); + struct drm_crtc *crtc = new_state->crtc ?: old_state->crtc; 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 (!fb) + return; + if (old_fb && old_fb->format != fb->format) mode_changed = true; else if (old_crtc != crtc) mode_changed = true; - imx_lcdc_update_hw_registers(pipe, old_state, mode_changed); + imx_lcdc_update_hw_registers(crtc, old_state, mode_changed); +} - if (event) { - crtc->state->event = NULL; +static int imx_lcdc_plane_helper_atomic_check(struct drm_plane *plane, + struct drm_atomic_commit *commit) +{ + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(commit, plane); + struct drm_crtc *crtc = plane_state->crtc; + struct drm_crtc_state *crtc_state = NULL; - spin_lock_irq(&crtc->dev->event_lock); + if (crtc) + crtc_state = drm_atomic_get_new_crtc_state(commit, crtc); - if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0) - drm_crtc_arm_vblank_event(crtc, event); - else - drm_crtc_send_vblank_event(crtc, event); + return drm_atomic_helper_check_plane_state(plane_state, crtc_state, + DRM_PLANE_NO_SCALING, + DRM_PLANE_NO_SCALING, + false, false); +} - spin_unlock_irq(&crtc->dev->event_lock); - } +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 void imx_lcdc_crtc_helper_atomic_flush(struct drm_crtc *crtc, + struct drm_atomic_commit *commit) +{ + struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(commit, crtc); + struct drm_pending_vblank_event *event = new_crtc_state->event; + + if (!event) + return; + + new_crtc_state->event = NULL; + + spin_lock_irq(&crtc->dev->event_lock); + if (new_crtc_state->active && drm_crtc_vblank_get(crtc) == 0) + drm_crtc_arm_vblank_event(crtc, event); + else + drm_crtc_send_vblank_event(crtc, event); + spin_unlock_irq(&crtc->dev->event_lock); } -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 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, + .atomic_flush = imx_lcdc_crtc_helper_atomic_flush, +}; + +static int imx_lcdc_crtc_enable_vblank(struct drm_crtc *crtc) +{ + return 0; +} + +static void imx_lcdc_crtc_disable_vblank(struct drm_crtc *crtc) +{ +} + +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, + .enable_vblank = imx_lcdc_crtc_enable_vblank, + .disable_vblank = imx_lcdc_crtc_disable_vblank, +}; + +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 +450,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 +468,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 +512,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"); --- base-commit: 3696d07837d1df13a5603d77f667685e7dfb3c53 change-id: 20260704-drm-simple-kms-removal-01a031c6a129 Best regards, -- Ze Huang <ze.huang@xxxxxxxxxxxxxxxx>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |