[PATCH 5/9] drm/pl111: replace struct drm_simple_display_pipe with regular atomic helpers
Thomas Zimmermann
tzimmermann at suse.de
Wed Jul 8 06:03:45 PDT 2026
Am 04.07.26 um 20:31 schrieb Ze Huang:
> Replace PL111 simple display pipe with explicit plane, CRTC and encoder
> objects.
>
> Keep existing hardware programming and vblank behavior, and install GEM
> framebuffer prepare helper explicitly.
>
> Signed-off-by: Ze Huang <ze.huang at oss.qualcomm.com>
> ---
> drivers/gpu/drm/pl111/pl111_display.c | 174 ++++++++++++++++++++++++++--------
> drivers/gpu/drm/pl111/pl111_drm.h | 5 +-
> drivers/gpu/drm/pl111/pl111_drv.c | 3 +-
> 3 files changed, 136 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
> index 5d10bc5fdf1f..b1bdd4c9dbe6 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -15,6 +15,7 @@
> #include <linux/media-bus-format.h>
> #include <linux/of_graph.h>
>
> +#include <drm/drm_atomic_helper.h>
> #include <drm/drm_fb_dma_helper.h>
> #include <drm/drm_fourcc.h>
> #include <drm/drm_framebuffer.h>
> @@ -37,7 +38,7 @@ irqreturn_t pl111_irq(int irq, void *data)
> return IRQ_NONE;
>
> if (irq_stat & CLCD_IRQ_NEXTBASE_UPDATE) {
> - drm_crtc_handle_vblank(&priv->pipe.crtc);
> + drm_crtc_handle_vblank(&priv->crtc);
>
> status = IRQ_HANDLED;
> }
> @@ -49,10 +50,10 @@ irqreturn_t pl111_irq(int irq, void *data)
> }
>
> static enum drm_mode_status
> -pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> - const struct drm_display_mode *mode)
> +pl111_crtc_helper_mode_valid(struct drm_crtc *crtc,
> + const struct drm_display_mode *mode)
> {
> - struct drm_device *drm = pipe->crtc.dev;
> + struct drm_device *drm = crtc->dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
> u32 cpp = DIV_ROUND_UP(priv->variant->fb_depth, 8);
> u64 bw;
> @@ -83,13 +84,35 @@ pl111_mode_valid(struct drm_simple_display_pipe *pipe,
> return MODE_OK;
> }
>
> -static int pl111_display_check(struct drm_simple_display_pipe *pipe,
> - struct drm_plane_state *pstate,
> - struct drm_crtc_state *cstate)
> +static int pl111_plane_helper_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_commit *state)
> {
> - const struct drm_display_mode *mode = &cstate->mode;
> - struct drm_framebuffer *old_fb = pipe->plane.state->fb;
> + struct drm_plane_state *pstate = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_crtc *crtc = pstate->crtc;
> + struct drm_crtc_state *cstate;
> + const struct drm_display_mode *mode;
> + struct drm_framebuffer *old_fb = plane->state->fb;
> struct drm_framebuffer *fb = pstate->fb;
> + int ret;
> +
> + if (!crtc)
> + return 0;
> +
> + cstate = drm_atomic_get_new_crtc_state(state, crtc);
> + if (!cstate)
> + return 0;
> +
> + ret = drm_atomic_helper_check_plane_state(pstate, cstate,
> + DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING,
> + false, false);
This is another case of _check_plane_state() running conditionally.
> + if (ret)
> + return ret;
> +
> + if (!pstate->visible)
> + return 0;
> +
> + mode = &cstate->mode;
>
> if (mode->hdisplay % 16)
> return -EINVAL;
> @@ -117,16 +140,14 @@ static int pl111_display_check(struct drm_simple_display_pipe *pipe,
> return 0;
> }
>
> -static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
> - struct drm_crtc_state *cstate,
> - struct drm_plane_state *plane_state)
> +static void pl111_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> + struct drm_atomic_commit *state)
> {
> - struct drm_crtc *crtc = &pipe->crtc;
> - struct drm_plane *plane = &pipe->plane;
> struct drm_device *drm = crtc->dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
> + struct drm_crtc_state *cstate = crtc->state;
> const struct drm_display_mode *mode = &cstate->mode;
> - struct drm_framebuffer *fb = plane->state->fb;
> + struct drm_framebuffer *fb = priv->plane.state->fb;
> struct drm_connector *connector = priv->connector;
> struct drm_bridge *bridge = priv->bridge;
> bool grayscale = false;
> @@ -355,9 +376,9 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe,
> drm_crtc_vblank_on(crtc);
> }
>
> -static void pl111_display_disable(struct drm_simple_display_pipe *pipe)
> +static void pl111_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_commit *state)
> {
> - struct drm_crtc *crtc = &pipe->crtc;
> struct drm_device *drm = crtc->dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
> u32 cntl;
> @@ -387,17 +408,23 @@ static void pl111_display_disable(struct drm_simple_display_pipe *pipe)
> clk_disable_unprepare(priv->clk);
> }
>
> -static void pl111_display_update(struct drm_simple_display_pipe *pipe,
> - struct drm_plane_state *old_pstate)
> +static void pl111_plane_helper_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_commit *state)
> {
> - struct drm_crtc *crtc = &pipe->crtc;
> - struct drm_device *drm = crtc->dev;
> - struct pl111_drm_dev_private *priv = drm->dev_private;
> - struct drm_pending_vblank_event *event = crtc->state->event;
> - struct drm_plane *plane = &pipe->plane;
> + struct drm_crtc *crtc = plane->state->crtc;
> + struct drm_device *drm;
> + struct pl111_drm_dev_private *priv;
> + struct drm_pending_vblank_event *event;
> struct drm_plane_state *pstate = plane->state;
> struct drm_framebuffer *fb = pstate->fb;
>
> + if (!crtc)
> + return;
> +
> + drm = crtc->dev;
> + priv = drm->dev_private;
> + event = crtc->state->event;
> +
> if (fb) {
> u32 addr = drm_fb_dma_get_gem_addr(fb, pstate, 0);
>
> @@ -416,9 +443,8 @@ static void pl111_display_update(struct drm_simple_display_pipe *pipe,
> }
> }
>
> -static int pl111_display_enable_vblank(struct drm_simple_display_pipe *pipe)
> +static int pl111_display_enable_vblank(struct drm_crtc *crtc)
> {
> - struct drm_crtc *crtc = &pipe->crtc;
> struct drm_device *drm = crtc->dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
>
> @@ -427,21 +453,63 @@ static int pl111_display_enable_vblank(struct drm_simple_display_pipe *pipe)
> return 0;
> }
>
> -static void pl111_display_disable_vblank(struct drm_simple_display_pipe *pipe)
> +static void pl111_display_disable_vblank(struct drm_crtc *crtc)
> {
> - struct drm_crtc *crtc = &pipe->crtc;
> struct drm_device *drm = crtc->dev;
> struct pl111_drm_dev_private *priv = drm->dev_private;
>
> writel(0, priv->regs + priv->ienb);
> }
>
> -static struct drm_simple_display_pipe_funcs pl111_display_funcs = {
> - .mode_valid = pl111_mode_valid,
> - .check = pl111_display_check,
> - .enable = pl111_display_enable,
> - .disable = pl111_display_disable,
> - .update = pl111_display_update,
> +static int pl111_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);
> + int ret;
> +
> + 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 struct drm_crtc_funcs pl111_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_crtc_helper_funcs pl111_crtc_helper_funcs = {
> + .mode_valid = pl111_crtc_helper_mode_valid,
> + .atomic_check = pl111_crtc_helper_atomic_check,
> + .atomic_enable = pl111_crtc_helper_atomic_enable,
> + .atomic_disable = pl111_crtc_helper_atomic_disable,
> +};
> +
> +static const struct drm_plane_funcs pl111_plane_funcs = {
> + .update_plane = drm_atomic_helper_update_plane,
> + .disable_plane = drm_atomic_helper_disable_plane,
> + .reset = drm_atomic_helper_plane_reset,
> + .destroy = drm_plane_cleanup,
> + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +};
> +
> +static const struct drm_plane_helper_funcs pl111_plane_helper_funcs = {
> + .prepare_fb = drm_gem_plane_helper_prepare_fb,
> + .atomic_check = pl111_plane_helper_atomic_check,
> + .atomic_update = pl111_plane_helper_atomic_update,
> +};
> +
> +static const struct drm_encoder_funcs pl111_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> };
>
> static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
> @@ -583,18 +651,40 @@ int pl111_display_init(struct drm_device *drm)
> return ret;
>
> if (!priv->variant->broken_vblank) {
> - pl111_display_funcs.enable_vblank = pl111_display_enable_vblank;
> - pl111_display_funcs.disable_vblank = pl111_display_disable_vblank;
> + pl111_crtc_funcs.enable_vblank = pl111_display_enable_vblank;
> + pl111_crtc_funcs.disable_vblank = pl111_display_disable_vblank;
> }
>
> - ret = drm_simple_display_pipe_init(drm, &priv->pipe,
> - &pl111_display_funcs,
> - priv->variant->formats,
> - priv->variant->nformats,
> - NULL,
> - priv->connector);
> + ret = drm_universal_plane_init(drm, &priv->plane, 0,
> + &pl111_plane_funcs,
> + priv->variant->formats,
> + priv->variant->nformats,
> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
> if (ret)
> return ret;
>
> + drm_plane_helper_add(&priv->plane, &pl111_plane_helper_funcs);
> +
> + ret = drm_crtc_init_with_planes(drm, &priv->crtc, &priv->plane,
> + NULL, &pl111_crtc_funcs, NULL);
> + if (ret)
> + return ret;
> +
> + drm_crtc_helper_add(&priv->crtc, &pl111_crtc_helper_funcs);
> +
> + ret = drm_encoder_init(drm, &priv->encoder, &pl111_encoder_funcs,
> + DRM_MODE_ENCODER_NONE, NULL);
> + if (ret)
> + return ret;
> +
> + priv->encoder.possible_crtcs = drm_crtc_mask(&priv->crtc);
> +
> + if (priv->connector) {
> + ret = drm_connector_attach_encoder(priv->connector,
> + &priv->encoder);
> + if (ret)
> + return ret;
> + }
> +
> return 0;
> }
> diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h
> index d1fe756444ee..ec92a5a180a8 100644
> --- a/drivers/gpu/drm/pl111/pl111_drm.h
> +++ b/drivers/gpu/drm/pl111/pl111_drm.h
> @@ -21,7 +21,6 @@
> #include <drm/drm_encoder.h>
> #include <drm/drm_gem.h>
> #include <drm/drm_panel.h>
> -#include <drm/drm_simple_kms_helper.h>
>
> /*
> * CLCD Controller Internal Register addresses
> @@ -135,7 +134,9 @@ struct pl111_drm_dev_private {
> struct drm_connector *connector;
> struct drm_panel *panel;
> struct drm_bridge *bridge;
> - struct drm_simple_display_pipe pipe;
> + struct drm_plane plane;
> + struct drm_crtc crtc;
> + struct drm_encoder encoder;
>
> void *regs;
> u32 memory_bw;
> diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
> index ac7b1d12a0f5..f649c266c33a 100644
> --- a/drivers/gpu/drm/pl111/pl111_drv.c
> +++ b/drivers/gpu/drm/pl111/pl111_drv.c
> @@ -168,8 +168,7 @@ static int pl111_modeset_init(struct drm_device *dev)
> goto out_bridge;
> }
>
> - ret = drm_simple_display_pipe_attach_bridge(&priv->pipe,
> - bridge);
> + ret = drm_bridge_attach(&priv->encoder, bridge, NULL, 0);
> if (ret)
> return ret;
>
>
--
--
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)
More information about the linux-arm-kernel
mailing list