* Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
2026-03-11 10:10 [PATCH v2 00/16] " Thomas Zimmermann
@ 2026-03-11 21:03 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 21:03 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 17
Reviewed: 2026-03-12T07:03:36.620218
---
This is a well-structured 16-patch series by Thomas Zimmermann that converts all mipi-dbi-based DRM drivers from the `drm_simple_display_pipe` abstraction to regular atomic modesetting helpers (explicit plane, CRTC, encoder, connector). The goal is to deprecate the simple-kms helpers entirely, since their continued use and documentation attracts new drivers to a deprecated API.
The approach is sound:
1. Fix commit ordering so planes update after CRTC enable (patch 1)
2. Factor out device init from pipeline setup (patch 2)
3. Provide new atomic helper callbacks and initializer macros (patch 3)
4. Convert each driver mechanically (patches 4-14)
5. Remove dead simple-pipe code from mipi-dbi (patch 15)
6. Deprecate simple-kms entirely (patch 16)
**Key concerns:**
- **Bug:** Uninitialized `new_crtc_state` in `drm_mipi_dbi_plane_helper_atomic_check()` (patch 3) — this will use stack garbage when the plane has no CRTC.
- **Multiple typos** in patch 16's deprecation text.
- The series is compile-tested only; hardware testing is explicitly requested.
The driver conversions (patches 4-14) are mechanical and consistent. The series is ready after fixing the bug and typos noted below.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
@ 2026-03-19 15:59 Thomas Zimmermann
2026-03-19 15:59 ` [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
` (16 more replies)
0 siblings, 17 replies; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
We keep getting new drivers based on the obsolete simple-display pipeline,
such as the recent driver for st7789v panels. [1] But submitters cannot
know, as the simple-display helpers are fully documented and still used
in several places.
Hence, convert all the mipi-dbi drivers over to regular atomic helpers
and remove simple-display support from mipi-dbi in this series. Then
undocument the helpers and add TODO items for final their removal.
Patch 1 changes the mode-setting logic to get CRTC and panel updates into
the correct order. This patch is probably the most fragile change in the
series.
Patches 2 and 3 prepare mipi-dbi to allow for atomic helpers to be used.
Patches 5 to 14 update mipi-dbi drivers one by one. The st7735r driver
requires some additional minor preparation. Overall, the refactoring is
always the same.
Patches 15 and 16 clean up mipi-dbi and the simple-display helpers.
These patches need some testing, as I can only compile them. At least
one of the drivers should be tried on hardware. Regressions should be
easy to fix, as these changes are mostly refactoring jobs.
[1] https://lore.kernel.org/dri-devel/20260221071351.22772-1-architanant5@gmail.com/
v3:
- fix uninitialized variables (David)
- improve documentation (David)
- clean up
v2:
- fix connector initialization (David)
- remove unused variables (David)
- clean up
Thomas Zimmermann (16):
drm/mipi-dbi: Only modify planes on enabled CRTCs
drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init()
drm/mipi-dbi: Provide callbacks for atomic interfaces
drm/hx8357d: Use regular atomic helpers; drop simple-display helpers
drm/ili9163: Use regular atomic helpers; drop simple-display helpers
drm/ili9225: Use regular atomic helpers; drop simple-display helpers
drm/ili9341: Use regular atomic helpers; drop simple-display helpers
drm/ili9486: Use regular atomic helpers; drop simple-display helpers
drm/mi0283qt: Use regular atomic helpers; drop simple-display helpers
drm/panel-mipi-dbi: Use regular atomic helpers; drop simple-display
helpers
drm/st7586: Use regular atomic helpers; drop simple-display helpers
drm/st7735r: Rename struct st7735r_priv to struct st7735r_device
drm/st7735r: Rename priv variable to st7735r
drm/st7735r: Use regular atomic helpers; drop simple-display helpers
drm/mipi-dbi: Remove simple-display helpers from mipi-dbi
drm/simple-kms: Deprecate simple-kms helpers
Documentation/gpu/drm-kms-helpers.rst | 12 -
Documentation/gpu/introduction.rst | 6 -
Documentation/gpu/todo.rst | 32 ++
drivers/gpu/drm/drm_crtc.c | 6 +-
drivers/gpu/drm/drm_gem_atomic_helper.c | 22 --
drivers/gpu/drm/drm_mipi_dbi.c | 376 ++++++++----------------
drivers/gpu/drm/drm_modeset_helper.c | 3 -
drivers/gpu/drm/drm_simple_kms_helper.c | 83 ------
drivers/gpu/drm/sitronix/st7586.c | 185 +++++++++---
drivers/gpu/drm/sitronix/st7735r.c | 142 +++++++--
drivers/gpu/drm/tiny/hx8357d.c | 138 ++++++++-
drivers/gpu/drm/tiny/ili9163.c | 138 ++++++++-
drivers/gpu/drm/tiny/ili9225.c | 192 +++++++++---
drivers/gpu/drm/tiny/ili9341.c | 138 ++++++++-
drivers/gpu/drm/tiny/ili9486.c | 139 ++++++++-
drivers/gpu/drm/tiny/mi0283qt.c | 138 ++++++++-
drivers/gpu/drm/tiny/panel-mipi-dbi.c | 141 +++++++--
include/drm/drm_mipi_dbi.h | 145 +++++----
include/drm/drm_simple_kms_helper.h | 216 +-------------
19 files changed, 1397 insertions(+), 855 deletions(-)
base-commit: 0009f17b2480f65a0f569c87c8a99f95d7aefd0e
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: a5a973e527c88a5b47053d7a72aefe0b550197cb
prerequisite-patch-id: 719d09751d38f5da743beed6266585ee063e1e29
prerequisite-patch-id: 5030de433a01c2e99056cadb676a8e2ba35f055a
prerequisite-patch-id: f7831e8fe81681bbe7fcd7b8455753f5ab4ada2d
--
2.53.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
` (15 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Use drm_atomic_helper_commit_tail_rpm() as commit tail to update the
plane after enabling the CRTC. Then remove the plane-update code from
mipi_dbi_enable_flush() and inline the remaining backlight code where
necessary.
Mipi-dbi's current commit tail drm_atomic_helper_commit_tail() first
updates the plane and then enables the CRTC. But the CRTC enablement
includes power management that prevents the initial plane update from
working. Hence, each mipi-dbi driver includes a plane update in their
CRTC enablement; in the form of mipi_dbi_enable_flush() or custom code.
Using drm_atomic_helper_commit_tail_rpm() enables the CRTC before any
plane updates. Hence the additional plane update can be removed from
mipi_dbi_enable_flush() and a number of drivers.
This leaves backlight_enable() in the helper, which can now be inlined
into affected drivers. Drivers now enable the CRTC plus an optional
backlight and then automatically update the plane.
In the case of disabling the display, drm_atomic_helper_commit_tail_rpm()
only disables the CRTC without touching the plane at all. Mipi-dbi's
mipi_dbi_pipe_disable() already contains the necessary logic.
Removing the plane update from the CRTC enablement will also help with
converting mipi-dbi from simple-pipe helpers to regular atomic helpers.
v3:
- st7586: remove unused variable
v2:
- ili9225: remove unused variables (David)
- st7586: remove unused variables (David)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/drm_mipi_dbi.c | 44 ++++-----------------------
drivers/gpu/drm/sitronix/st7586.c | 11 -------
drivers/gpu/drm/sitronix/st7735r.c | 2 +-
drivers/gpu/drm/tiny/hx8357d.c | 3 +-
drivers/gpu/drm/tiny/ili9163.c | 3 +-
drivers/gpu/drm/tiny/ili9225.c | 11 -------
drivers/gpu/drm/tiny/ili9341.c | 3 +-
drivers/gpu/drm/tiny/ili9486.c | 3 +-
drivers/gpu/drm/tiny/mi0283qt.c | 3 +-
drivers/gpu/drm/tiny/panel-mipi-dbi.c | 2 +-
include/drm/drm_mipi_dbi.h | 3 --
11 files changed, 18 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 00482227a9cd..bb6cebc583be 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -368,44 +368,6 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
}
EXPORT_SYMBOL(mipi_dbi_pipe_update);
-/**
- * mipi_dbi_enable_flush - MIPI DBI enable helper
- * @dbidev: MIPI DBI device structure
- * @crtc_state: CRTC state
- * @plane_state: Plane state
- *
- * Flushes the whole framebuffer and enables the backlight. Drivers can use this
- * in their &drm_simple_display_pipe_funcs->enable callback.
- *
- * Note: Drivers which don't use mipi_dbi_pipe_update() because they have custom
- * framebuffer flushing, can't use this function since they both use the same
- * flushing code.
- */
-void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
-{
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
- struct drm_framebuffer *fb = plane_state->fb;
- struct drm_rect rect = {
- .x1 = 0,
- .x2 = fb->width,
- .y1 = 0,
- .y2 = fb->height,
- };
- int idx;
-
- if (!drm_dev_enter(&dbidev->drm, &idx))
- return;
-
- mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
- &shadow_plane_state->fmtcnv_state);
- backlight_enable(dbidev->backlight);
-
- drm_dev_exit(idx);
-}
-EXPORT_SYMBOL(mipi_dbi_enable_flush);
-
static void mipi_dbi_blank(struct mipi_dbi_dev *dbidev)
{
struct drm_device *drm = &dbidev->drm;
@@ -577,6 +539,10 @@ static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
}
}
+static const struct drm_mode_config_helper_funcs mipi_dbi_mode_config_helper_funcs = {
+ .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+};
+
static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = {
.fb_create = drm_gem_fb_create_with_dirty,
.atomic_check = drm_atomic_helper_check,
@@ -660,6 +626,8 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
drm->mode_config.max_width = dbidev->mode.hdisplay;
drm->mode_config.min_height = dbidev->mode.vdisplay;
drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.helper_private = &mipi_dbi_mode_config_helper_funcs;
+
dbidev->rotation = rotation;
dbidev->pixel_format = formats[0];
if (formats[0] == DRM_FORMAT_RGB888)
diff --git a/drivers/gpu/drm/sitronix/st7586.c b/drivers/gpu/drm/sitronix/st7586.c
index 16b6b4e368af..b6f38e03e1e5 100644
--- a/drivers/gpu/drm/sitronix/st7586.c
+++ b/drivers/gpu/drm/sitronix/st7586.c
@@ -174,15 +174,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state)
{
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
- struct drm_framebuffer *fb = plane_state->fb;
struct mipi_dbi *dbi = &dbidev->dbi;
- struct drm_rect rect = {
- .x1 = 0,
- .x2 = fb->width,
- .y1 = 0,
- .y2 = fb->height,
- };
int idx, ret;
u8 addr_mode;
@@ -242,9 +234,6 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
msleep(100);
- st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
- &shadow_plane_state->fmtcnv_state);
-
mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
out_exit:
drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/sitronix/st7735r.c b/drivers/gpu/drm/sitronix/st7735r.c
index c1f8228495f6..1a34c7ba460b 100644
--- a/drivers/gpu/drm/sitronix/st7735r.c
+++ b/drivers/gpu/drm/sitronix/st7735r.c
@@ -129,7 +129,7 @@ static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
msleep(20);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 9f26aaca0bfa..5115be8854bb 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -177,7 +177,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
break;
}
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index 7c154c008344..c616f56af5b5 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -96,7 +96,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
}
addr_mode |= ILI9163_MADCTL_BGR;
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index d32538b1eb09..3eaf6b3a055a 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -184,16 +184,8 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state)
{
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
- struct drm_framebuffer *fb = plane_state->fb;
struct device *dev = pipe->crtc.dev->dev;
struct mipi_dbi *dbi = &dbidev->dbi;
- struct drm_rect rect = {
- .x1 = 0,
- .x2 = fb->width,
- .y1 = 0,
- .y2 = fb->height,
- };
int ret, idx;
u8 am_id;
@@ -284,9 +276,6 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
ili9225_command(dbi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
- ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
- &shadow_plane_state->fmtcnv_state);
-
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 2ab750cba505..972811564d6a 100644
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ b/drivers/gpu/drm/tiny/ili9341.c
@@ -133,7 +133,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
}
addr_mode |= ILI9341_MADCTL_BGR;
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index 1e411a0f4567..52b14f2cb0e1 100644
--- a/drivers/gpu/drm/tiny/ili9486.c
+++ b/drivers/gpu/drm/tiny/ili9486.c
@@ -155,7 +155,8 @@ static void waveshare_enable(struct drm_simple_display_pipe *pipe,
}
addr_mode |= ILI9486_MADCTL_BGR;
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index a063eff77624..f121e1a8a303 100644
--- a/drivers/gpu/drm/tiny/mi0283qt.c
+++ b/drivers/gpu/drm/tiny/mi0283qt.c
@@ -137,7 +137,8 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
}
addr_mode |= ILI9341_MADCTL_BGR;
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
index 82dfa169f762..4907945ab507 100644
--- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
+++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
@@ -252,7 +252,7 @@ static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
if (!ret)
panel_mipi_dbi_commands_execute(dbi, dbidev->driver_private);
- mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
+ backlight_enable(dbidev->backlight);
out_exit:
drm_dev_exit(idx);
}
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index f45f9612c0bc..637be84d3d5a 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -176,9 +176,6 @@ enum drm_mode_status mipi_dbi_pipe_mode_valid(struct drm_simple_display_pipe *pi
const struct drm_display_mode *mode);
void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *old_state);
-void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plan_state);
void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init()
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
2026-03-19 15:59 ` [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
` (14 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Initialize the mipi-dbi device with drm_mipi_dbi_dev_init() without
creating a modesetting pipeline. Will allow for mipi-dbi drivers
without simple-display helpers.
As the new helper is a DRM function, add the drm_ prefix. Mipi-dbi
interfaces currently lack this.
v3:
- document tx_buf_size parameter (David)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/drm_mipi_dbi.c | 80 ++++++++++++++++++++++++----------
include/drm/drm_mipi_dbi.h | 4 ++
2 files changed, 61 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index bb6cebc583be..bc5444ca7ac6 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -554,6 +554,59 @@ static const uint32_t mipi_dbi_formats[] = {
DRM_FORMAT_XRGB8888,
};
+/**
+ * drm_mipi_dbi_dev_init - MIPI DBI device initialization
+ * @dbidev: MIPI DBI device structure to initialize
+ * @mode: Hardware display mode
+ * @format: Hardware color format (DRM_FORMAT\_\*).
+ * @rotation: Initial rotation in degrees Counter Clock Wise
+ * @tx_buf_size: Allocate a transmit buffer of at least this size.
+ *
+ * Initializes a MIPI-DBI device. The minimum size of the transmit buffer
+ * in @tx_buf_size is optional. Pass 0 to allocate enough memory to transmit
+ * a single scanline of the display.
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int drm_mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_display_mode *mode,
+ u32 format, unsigned int rotation, size_t tx_buf_size)
+{
+ struct drm_device *drm = &dbidev->drm;
+ int ret;
+
+ if (!dbidev->dbi.command)
+ return -EINVAL;
+
+ if (!tx_buf_size) {
+ const struct drm_format_info *info = drm_format_info(format);
+
+ tx_buf_size = drm_format_info_min_pitch(info, 0, mode->hdisplay) *
+ mode->vdisplay;
+ }
+
+ dbidev->tx_buf = devm_kmalloc(drm->dev, tx_buf_size, GFP_KERNEL);
+ if (!dbidev->tx_buf)
+ return -ENOMEM;
+
+ drm_mode_copy(&dbidev->mode, mode);
+ ret = mipi_dbi_rotate_mode(&dbidev->mode, rotation);
+ if (ret) {
+ drm_err(drm, "Illegal rotation value %u\n", rotation);
+ return -EINVAL;
+ }
+
+ dbidev->rotation = rotation;
+ drm_dbg(drm, "rotation = %u\n", rotation);
+
+ dbidev->pixel_format = format;
+ if (dbidev->pixel_format == DRM_FORMAT_RGB888)
+ dbidev->dbi.write_memory_bpw = 8;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mipi_dbi_dev_init);
+
/**
* mipi_dbi_dev_init_with_formats - MIPI DBI device initialization with custom formats
* @dbidev: MIPI DBI device structure to initialize
@@ -590,24 +643,14 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
struct drm_device *drm = &dbidev->drm;
int ret;
- if (!dbidev->dbi.command)
- return -EINVAL;
+ ret = drm_mipi_dbi_dev_init(dbidev, mode, formats[0], rotation, tx_buf_size);
+ if (ret)
+ return ret;
ret = drmm_mode_config_init(drm);
if (ret)
return ret;
- dbidev->tx_buf = devm_kmalloc(drm->dev, tx_buf_size, GFP_KERNEL);
- if (!dbidev->tx_buf)
- return -ENOMEM;
-
- drm_mode_copy(&dbidev->mode, mode);
- ret = mipi_dbi_rotate_mode(&dbidev->mode, rotation);
- if (ret) {
- DRM_ERROR("Illegal rotation value %u\n", rotation);
- return -EINVAL;
- }
-
drm_connector_helper_add(&dbidev->connector, &mipi_dbi_connector_hfuncs);
ret = drm_connector_init(drm, &dbidev->connector, &mipi_dbi_connector_funcs,
DRM_MODE_CONNECTOR_SPI);
@@ -628,13 +671,6 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
drm->mode_config.max_height = dbidev->mode.vdisplay;
drm->mode_config.helper_private = &mipi_dbi_mode_config_helper_funcs;
- dbidev->rotation = rotation;
- dbidev->pixel_format = formats[0];
- if (formats[0] == DRM_FORMAT_RGB888)
- dbidev->dbi.write_memory_bpw = 8;
-
- DRM_DEBUG_KMS("rotation = %u\n", rotation);
-
return 0;
}
EXPORT_SYMBOL(mipi_dbi_dev_init_with_formats);
@@ -660,13 +696,11 @@ int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
const struct drm_simple_display_pipe_funcs *funcs,
const struct drm_display_mode *mode, unsigned int rotation)
{
- size_t bufsize = (u32)mode->vdisplay * mode->hdisplay * sizeof(u16);
-
dbidev->drm.mode_config.preferred_depth = 16;
return mipi_dbi_dev_init_with_formats(dbidev, funcs, mipi_dbi_formats,
ARRAY_SIZE(mipi_dbi_formats), mode,
- rotation, bufsize);
+ rotation, 0);
}
EXPORT_SYMBOL(mipi_dbi_dev_init);
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 637be84d3d5a..9c0e015dd929 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -164,6 +164,10 @@ static inline struct mipi_dbi_dev *drm_to_mipi_dbi_dev(struct drm_device *drm)
int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *dbi,
struct gpio_desc *dc);
+
+int drm_mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_display_mode *mode,
+ u32 format, unsigned int rotation, size_t tx_buf_size);
+
int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
const struct drm_simple_display_pipe_funcs *funcs,
const uint32_t *formats, unsigned int format_count,
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
2026-03-19 15:59 ` [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
2026-03-19 15:59 ` [PATCH v3 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
` (13 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Refactor the existing simple-display callbacks such that they invoke
helpers compatible with regular atomic modesetting. Allows for adding
mipi-dbi drives that do not require simple-display helpers.
Provide initializer macro for elements of the regular modesetting
pipeline. These will be used by drivers to integrate mipi-dbi helpers.
Also provide initializer macros for the plane formats.
As the new helpers are DRM functions, add the drm_ prefix. Mipi-dbi
interfaces currently lack this.
v3:
- fix uninitialized variable (David)
- document public interfaces (David)
- mention format macros in commit message (David)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/drm_mipi_dbi.c | 203 +++++++++++++++++++++++++--------
include/drm/drm_mipi_dbi.h | 86 ++++++++++++++
2 files changed, 242 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index bc5444ca7ac6..0374fd0c527d 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -14,6 +14,7 @@
#include <linux/regulator/consumer.h>
#include <linux/spi/spi.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_connector.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
@@ -22,12 +23,9 @@
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem.h>
-#include <drm/drm_gem_atomic_helper.h>
-#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_mipi_dbi.h>
#include <drm/drm_modes.h>
#include <drm/drm_print.h>
-#include <drm/drm_probe_helper.h>
#include <drm/drm_rect.h>
#include <video/mipi_display.h>
@@ -316,6 +314,24 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
drm_err_once(fb->dev, "Failed to update display %d\n", ret);
}
+/**
+ * drm_mipi_dbi_crtc_helper_mode_valid - MIPI DBI mode-valid helper
+ * @crtc: The CRTC
+ * @mode: The mode to test
+ *
+ * This function validates a given display mode against the MIPI DBI's hardware
+ * display. Drivers can use this as their struct &drm_crtc_helper_funcs.mode_valid
+ * callback.
+ */
+enum drm_mode_status drm_mipi_dbi_crtc_helper_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode)
+{
+ struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(crtc->dev);
+
+ return drm_crtc_helper_mode_valid_fixed(crtc, mode, &dbidev->mode);
+}
+EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_mode_valid);
+
/**
* mipi_dbi_pipe_mode_valid - MIPI DBI mode-valid helper
* @pipe: Simple display pipe
@@ -328,43 +344,87 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
enum drm_mode_status mipi_dbi_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
-
- return drm_crtc_helper_mode_valid_fixed(&pipe->crtc, mode, &dbidev->mode);
+ return drm_mipi_dbi_crtc_helper_mode_valid(&pipe->crtc, mode);
}
EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
/**
- * mipi_dbi_pipe_update - Display pipe update helper
- * @pipe: Simple display pipe
- * @old_state: Old plane state
+ * drm_mipi_dbi_plane_helper_atomic_check - MIPI DBI plane check helper
+ * @plane: Plane to check
+ * @state: Atomic state
+ *
+ * This function performs the default checks on the primary plane
+ * of a MIPI DBI device. Drivers can use this as their
+ * struct &drm_crtc_helper_funcs.atomic_check callback.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_mipi_dbi_plane_helper_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_state *state)
+{
+ struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+ struct drm_crtc_state *new_crtc_state = NULL;
+ int ret;
+
+ if (new_plane_state->crtc)
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
+
+ ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, false);
+ if (ret)
+ return ret;
+ else if (!new_plane_state->visible)
+ return 0;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_mipi_dbi_plane_helper_atomic_check);
+
+/**
+ * drm_mipi_dbi_plane_helper_atomic_update - Display update helper
+ * @plane: Plane
+ * @state: Atomic state
*
* This function handles framebuffer flushing and vblank events. Drivers can use
- * this as their &drm_simple_display_pipe_funcs->update callback.
+ * this as their struct &drm_plane_helper_funcs.atomic_update callback.
*/
-void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
+void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state *state)
{
- struct drm_plane_state *state = pipe->plane.state;
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
- struct drm_framebuffer *fb = state->fb;
+ struct drm_plane_state *plane_state = plane->state;
+ struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
- if (!pipe->crtc.state->active)
+ if (!fb)
return;
- if (WARN_ON(!fb))
- return;
-
- if (!drm_dev_enter(fb->dev, &idx))
- return;
-
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
- mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
- &shadow_plane_state->fmtcnv_state);
+ if (drm_dev_enter(plane->dev, &idx)) {
+ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
+ mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
+ &shadow_plane_state->fmtcnv_state);
+ drm_dev_exit(idx);
+ }
+}
+EXPORT_SYMBOL(drm_mipi_dbi_plane_helper_atomic_update);
- drm_dev_exit(idx);
+/**
+ * mipi_dbi_pipe_update - Display pipe update helper
+ * @pipe: Simple display pipe
+ * @old_state: Old plane state
+ *
+ * This function handles framebuffer flushing and vblank events. Drivers can use
+ * this as their &drm_simple_display_pipe_funcs->update callback.
+ */
+void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
+ struct drm_plane_state *old_state)
+{
+ return drm_mipi_dbi_plane_helper_atomic_update(&pipe->plane, old_state->state);
}
EXPORT_SYMBOL(mipi_dbi_pipe_update);
@@ -394,18 +454,48 @@ static void mipi_dbi_blank(struct mipi_dbi_dev *dbidev)
}
/**
- * mipi_dbi_pipe_disable - MIPI DBI pipe disable helper
- * @pipe: Display pipe
+ * drm_mipi_dbi_crtc_helper_atomic_check - MIPI DBI CRTC check helper
+ * @crtc: CRTC to check
+ * @state: Atomic state
+ *
+ * This function performs the default checks on the CRTC of a MIPI DBI
+ * device and ensures that the primary plane as been set up correctly.
+ * Drivers can use this as their struct &drm_crtc_helper_funcs.atomic_check
+ * callback.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_mipi_dbi_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *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);
+}
+EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_atomic_check);
+
+/**
+ * drm_mipi_dbi_crtc_helper_atomic_disable - MIPI DBI CRTC disable helper
+ * @crtc: CRTC to disable
+ * @state: Atomic state
*
* This function disables backlight if present, if not the display memory is
* blanked. The regulator is disabled if in use. Drivers can use this as their
- * &drm_simple_display_pipe_funcs->disable callback.
+ * struct &drm_crtc_helper_funcs.atomic_disable callback.
*/
-void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
+void drm_mipi_dbi_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
-
- DRM_DEBUG_KMS("\n");
+ struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(crtc->dev);
if (dbidev->backlight)
backlight_disable(dbidev->backlight);
@@ -417,6 +507,20 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
if (dbidev->io_regulator)
regulator_disable(dbidev->io_regulator);
}
+EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_atomic_disable);
+
+/**
+ * mipi_dbi_pipe_disable - MIPI DBI pipe disable helper
+ * @pipe: Display pipe
+ *
+ * This function disables backlight if present, if not the display memory is
+ * blanked. The regulator is disabled if in use. Drivers can use this as their
+ * &drm_simple_display_pipe_funcs->disable callback.
+ */
+void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+ drm_mipi_dbi_crtc_helper_atomic_disable(&pipe->crtc, pipe->crtc.state->state);
+}
EXPORT_SYMBOL(mipi_dbi_pipe_disable);
/**
@@ -503,23 +607,32 @@ void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
}
EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
-static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
+/**
+ * drm_mipi_dbi_connector_helper_get_modes - Duplicates the MIPI DBI mode for the connector
+ * @connector: The connector
+ *
+ * Sets the connecto rmodes from the MIPI DBI mode. Drivers can use this as their
+ * &drm_connector_helper_funcs->get_mods callback. See drm_gem_destroy_shadow_plane_state()
+ * for additional details.
+ *
+ * Returns:
+ * The number of created modes.
+ */
+int drm_mipi_dbi_connector_helper_get_modes(struct drm_connector *connector)
{
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
return drm_connector_helper_get_modes_fixed(connector, &dbidev->mode);
}
+EXPORT_SYMBOL(drm_mipi_dbi_connector_helper_get_modes);
static const struct drm_connector_helper_funcs mipi_dbi_connector_hfuncs = {
- .get_modes = mipi_dbi_connector_get_modes,
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
};
static const struct drm_connector_funcs mipi_dbi_connector_funcs = {
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
.destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
@@ -540,18 +653,15 @@ static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
}
static const struct drm_mode_config_helper_funcs mipi_dbi_mode_config_helper_funcs = {
- .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
};
static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = {
- .fb_create = drm_gem_fb_create_with_dirty,
- .atomic_check = drm_atomic_helper_check,
- .atomic_commit = drm_atomic_helper_commit,
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const uint32_t mipi_dbi_formats[] = {
- DRM_FORMAT_RGB565,
- DRM_FORMAT_XRGB8888,
+ DRM_MIPI_DBI_PLANE_FORMATS,
};
/**
@@ -637,8 +747,7 @@ int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
unsigned int rotation, size_t tx_buf_size)
{
static const uint64_t modifiers[] = {
- DRM_FORMAT_MOD_LINEAR,
- DRM_FORMAT_MOD_INVALID
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
};
struct drm_device *drm = &dbidev->drm;
int ret;
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 9c0e015dd929..ae92a5e8d13b 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -9,7 +9,12 @@
#define __LINUX_MIPI_DBI_H
#include <linux/mutex.h>
+
+#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_device.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
struct drm_format_conv_state;
@@ -230,6 +235,87 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
ret; \
})
+/*
+ * Plane
+ */
+
+#define DRM_MIPI_DBI_PLANE_FORMATS \
+ DRM_FORMAT_RGB565, \
+ DRM_FORMAT_XRGB8888
+
+#define DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS \
+ DRM_FORMAT_MOD_LINEAR, \
+ DRM_FORMAT_MOD_INVALID
+
+#define DRM_MIPI_DBI_PLANE_FUNCS \
+ DRM_GEM_SHADOW_PLANE_FUNCS, \
+ .update_plane = drm_atomic_helper_update_plane, \
+ .disable_plane = drm_atomic_helper_disable_plane
+
+int drm_mipi_dbi_plane_helper_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_state *state);
+void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state *state);
+
+#define DRM_MIPI_DBI_PLANE_HELPER_FUNCS \
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, \
+ .atomic_check = drm_mipi_dbi_plane_helper_atomic_check, \
+ .atomic_update = drm_mipi_dbi_plane_helper_atomic_update
+
+/*
+ * CRTC
+ */
+
+#define DRM_MIPI_DBI_CRTC_FUNCS \
+ .reset = drm_atomic_helper_crtc_reset, \
+ .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
+
+enum drm_mode_status drm_mipi_dbi_crtc_helper_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode);
+int drm_mipi_dbi_crtc_helper_atomic_check(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+void drm_mipi_dbi_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+
+#define DRM_MIPI_DBI_CRTC_HELPER_FUNCS \
+ .mode_valid = drm_mipi_dbi_crtc_helper_mode_valid, \
+ .atomic_check = drm_mipi_dbi_crtc_helper_atomic_check, \
+ .atomic_disable = drm_mipi_dbi_crtc_helper_atomic_disable
+
+/*
+ * Connector
+ */
+
+#define DRM_MIPI_DBI_CONNECTOR_FUNCS \
+ .reset = drm_atomic_helper_connector_reset, \
+ .fill_modes = drm_helper_probe_single_connector_modes, \
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, \
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state
+
+int drm_mipi_dbi_connector_helper_get_modes(struct drm_connector *connector);
+
+#define DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS \
+ .get_modes = drm_mipi_dbi_connector_helper_get_modes
+
+/*
+ * Mode config
+ */
+
+#define DRM_MIPI_DBI_MODE_CONFIG_FUNCS \
+ .fb_create = drm_gem_fb_create_with_dirty, \
+ .atomic_check = drm_atomic_helper_check, \
+ .atomic_commit = drm_atomic_helper_commit
+
+#define DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS \
+ .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm
+
+/*
+ * Debug FS
+ */
+
#ifdef CONFIG_DEBUG_FS
void mipi_dbi_debugfs_init(struct drm_minor *minor);
#else
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (2 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 05/16] drm/ili9163: " Thomas Zimmermann
` (12 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct hx8357d_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Hx8357d requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/hx8357d.c | 135 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 5115be8854bb..53b152fc6b04 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -46,16 +46,48 @@
#define HX8357D_MADCTL_BGR 0x08
#define HX8357D_MADCTL_MH 0x04
-static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+struct hx8357d_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct hx8357d_device *to_hx8357d_device(struct drm_device *dev)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ return container_of(drm_to_mipi_dbi_dev(dev), struct hx8357d_device, dbidev);
+}
+
+static const u32 hx8357d_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 hx8357d_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs hx8357d_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs hx8357d_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void hx8357d_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *drm = crtc->dev;
+ struct hx8357d_device *hx8357d = to_hx8357d_device(drm);
+ struct mipi_dbi_dev *dbidev = &hx8357d->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
u8 addr_mode;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -183,8 +215,35 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
+static const struct drm_crtc_helper_funcs hx8357d_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = hx8357d_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs hx8357d_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs hx8357d_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs hx8357d_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs hx8357d_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs hx8357d_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs hx8357d_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode yx350hv15_mode = {
@@ -220,17 +279,21 @@ MODULE_DEVICE_TABLE(spi, hx8357d_id);
static int hx8357d_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct hx8357d_device *hx8357d;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &hx8357d_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ hx8357d = devm_drm_dev_alloc(dev, &hx8357d_driver, struct hx8357d_device, dbidev.drm);
+ if (IS_ERR(hx8357d))
+ return PTR_ERR(hx8357d);
+ dbidev = &hx8357d->dbidev;
drm = &dbidev->drm;
dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
@@ -247,7 +310,53 @@ static int hx8357d_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = mipi_dbi_dev_init(dbidev, &hx8357d_pipe_funcs, &yx350hv15_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &yx350hv15_mode, hx8357d_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &hx8357d_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &hx8357d_mode_config_helper_funcs;
+
+ plane = &hx8357d->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &hx8357d_plane_funcs,
+ hx8357d_plane_formats, ARRAY_SIZE(hx8357d_plane_formats),
+ hx8357d_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &hx8357d_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &hx8357d->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &hx8357d_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &hx8357d_crtc_helper_funcs);
+
+ encoder = &hx8357d->encoder;
+ ret = drm_encoder_init(drm, encoder, &hx8357d_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &hx8357d->connector;
+ ret = drm_connector_init(drm, connector, &hx8357d_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &hx8357d_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 05/16] drm/ili9163: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (3 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 06/16] drm/ili9225: " Thomas Zimmermann
` (11 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct ili9163_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Ili9163 requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v3:
- set dbi variable (David)
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/ili9163.c | 135 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index c616f56af5b5..1e581576fc7e 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -35,16 +35,48 @@
#define ILI9163_MADCTL_MX BIT(6)
#define ILI9163_MADCTL_MY BIT(7)
-static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+struct ili9163_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct ili9163_device *to_ili9163_device(struct drm_device *dev)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ return container_of(drm_to_mipi_dbi_dev(dev), struct ili9163_device, dbidev);
+}
+
+static const u32 ili9163_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 ili9163_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs ili9163_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs ili9163_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void ili9163_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *drm = crtc->dev;
+ struct ili9163_device *ili9163 = to_ili9163_device(drm);
+ struct mipi_dbi_dev *dbidev = &ili9163->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
u8 addr_mode;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -102,8 +134,35 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs ili9163_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
+static const struct drm_crtc_helper_funcs ili9163_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = ili9163_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs ili9163_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs ili9163_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs ili9163_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs ili9163_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs ili9163_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs ili9163_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode yx240qv29_mode = {
@@ -139,18 +198,22 @@ MODULE_DEVICE_TABLE(spi, ili9163_id);
static int ili9163_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct ili9163_device *ili9163;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &ili9163_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ ili9163 = devm_drm_dev_alloc(dev, &ili9163_driver, struct ili9163_device, dbidev.drm);
+ if (IS_ERR(ili9163))
+ return PTR_ERR(ili9163);
+ dbidev = &ili9163->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -178,7 +241,53 @@ static int ili9163_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = mipi_dbi_dev_init(dbidev, &ili9163_pipe_funcs, &yx240qv29_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &yx240qv29_mode, ili9163_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &ili9163_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &ili9163_mode_config_helper_funcs;
+
+ plane = &ili9163->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &ili9163_plane_funcs,
+ ili9163_plane_formats, ARRAY_SIZE(ili9163_plane_formats),
+ ili9163_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &ili9163_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &ili9163->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &ili9163_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &ili9163_crtc_helper_funcs);
+
+ encoder = &ili9163->encoder;
+ ret = drm_encoder_init(drm, encoder, &ili9163_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &ili9163->connector;
+ ret = drm_connector_init(drm, connector, &ili9163_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &ili9163_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 06/16] drm/ili9225: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (4 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 05/16] drm/ili9163: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 07/16] drm/ili9341: " Thomas Zimmermann
` (10 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct ili9225_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Ili9225 requires custom helpers for
various pipeline elements, and non-freeing cleanup of the pipeline.
v3:
- set dbi variable (David)
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/ili9225.c | 181 ++++++++++++++++++++++++++-------
1 file changed, 145 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 3eaf6b3a055a..0371527002f9 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -17,6 +17,7 @@
#include <video/mipi_display.h>
#include <drm/clients/drm_client_setup.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
@@ -24,9 +25,7 @@
#include <drm/drm_fbdev_dma.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
-#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_gem_dma_helper.h>
-#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_managed.h>
#include <drm/drm_mipi_dbi.h>
#include <drm/drm_print.h>
@@ -72,6 +71,20 @@
#define ILI9225_GAMMA_CONTROL_9 0x58
#define ILI9225_GAMMA_CONTROL_10 0x59
+struct ili9225_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct ili9225_device *to_ili9225_device(struct drm_device *dev)
+{
+ return container_of(drm_to_mipi_dbi_dev(dev), struct ili9225_device, dbidev);
+}
+
static inline int ili9225_command(struct mipi_dbi *dbi, u8 cmd, u16 data)
{
u8 par[2] = { data >> 8, data & 0xff };
@@ -157,39 +170,61 @@ static void ili9225_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
}
-static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
+static const u32 ili9225_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 ili9225_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static void ili9225_plane_helper_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state *state)
{
- struct drm_plane_state *state = pipe->plane.state;
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
- struct drm_framebuffer *fb = state->fb;
+ struct drm_device *drm = plane->dev;
+ struct drm_plane_state *plane_state = plane->state;
+ struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
- if (!pipe->crtc.state->active)
+ if (!plane_state->fb)
return;
- if (!drm_dev_enter(fb->dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
&shadow_plane_state->fmtcnv_state);
drm_dev_exit(idx);
}
-static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+static const struct drm_plane_helper_funcs ili9225_plane_helper_funcs = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = drm_mipi_dbi_plane_helper_atomic_check,
+ .atomic_update = ili9225_plane_helper_atomic_update,
+};
+
+static const struct drm_plane_funcs ili9225_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void ili9225_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct device *dev = pipe->crtc.dev->dev;
+ struct drm_device *drm = crtc->dev;
+ struct ili9225_device *ili9225 = to_ili9225_device(drm);
+ struct mipi_dbi_dev *dbidev = &ili9225->dbidev;
+ struct device *dev = drm->dev;
struct mipi_dbi *dbi = &dbidev->dbi;
int ret, idx;
u8 am_id;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -280,9 +315,12 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
+static void ili9225_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct ili9225_device *ili9225 = to_ili9225_device(drm);
+ struct mipi_dbi_dev *dbidev = &ili9225->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
DRM_DEBUG_KMS("\n");
@@ -301,6 +339,39 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
ili9225_command(dbi, ILI9225_POWER_CONTROL_1, 0x0a02);
}
+static const struct drm_crtc_helper_funcs ili9225_crtc_helper_funcs = {
+ .mode_valid = drm_mipi_dbi_crtc_helper_mode_valid,
+ .atomic_check = drm_mipi_dbi_crtc_helper_atomic_check,
+ .atomic_enable = ili9225_crtc_helper_atomic_enable,
+ .atomic_disable = ili9225_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs ili9225_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs ili9225_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs ili9225_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs ili9225_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs ili9225_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs ili9225_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
+};
+
static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 *cmd, u8 *par,
size_t num)
{
@@ -329,18 +400,6 @@ static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 *cmd, u8 *par,
return ret;
}
-static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
- .mode_valid = mipi_dbi_pipe_mode_valid,
- .enable = ili9225_pipe_enable,
- .disable = ili9225_pipe_disable,
- .update = ili9225_pipe_update,
- .begin_fb_access = mipi_dbi_pipe_begin_fb_access,
- .end_fb_access = mipi_dbi_pipe_end_fb_access,
- .reset_plane = mipi_dbi_pipe_reset_plane,
- .duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
- .destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
-};
-
static const struct drm_display_mode ili9225_mode = {
DRM_SIMPLE_MODE(176, 220, 35, 44),
};
@@ -373,18 +432,22 @@ MODULE_DEVICE_TABLE(spi, ili9225_id);
static int ili9225_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct ili9225_device *ili9225;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *rs;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &ili9225_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ ili9225 = devm_drm_dev_alloc(dev, &ili9225_driver, struct ili9225_device, dbidev.drm);
+ if (IS_ERR(ili9225))
+ return PTR_ERR(ili9225);
+ dbidev = &ili9225->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -405,7 +468,53 @@ static int ili9225_probe(struct spi_device *spi)
/* override the command function set in mipi_dbi_spi_init() */
dbi->command = ili9225_dbi_command;
- ret = mipi_dbi_dev_init(dbidev, &ili9225_pipe_funcs, &ili9225_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &ili9225_mode, ili9225_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &ili9225_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &ili9225_mode_config_helper_funcs;
+
+ plane = &ili9225->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &ili9225_plane_funcs,
+ ili9225_plane_formats, ARRAY_SIZE(ili9225_plane_formats),
+ ili9225_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &ili9225_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &ili9225->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &ili9225_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &ili9225_crtc_helper_funcs);
+
+ encoder = &ili9225->encoder;
+ ret = drm_encoder_init(drm, encoder, &ili9225_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &ili9225->connector;
+ ret = drm_connector_init(drm, connector, &ili9225_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &ili9225_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 07/16] drm/ili9341: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (5 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 06/16] drm/ili9225: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 08/16] drm/ili9486: " Thomas Zimmermann
` (9 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct ili9341_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Ili9341 requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v3:
- set dbi variable (David)
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/ili9341.c | 135 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 972811564d6a..e623bd8a1827 100644
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ b/drivers/gpu/drm/tiny/ili9341.c
@@ -52,16 +52,48 @@
#define ILI9341_MADCTL_MX BIT(6)
#define ILI9341_MADCTL_MY BIT(7)
-static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+struct ili9341_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct ili9341_device *to_ili9341_device(struct drm_device *dev)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ return container_of(drm_to_mipi_dbi_dev(dev), struct ili9341_device, dbidev);
+}
+
+static const u32 ili9341_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 ili9341_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs ili9341_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs ili9341_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void ili9341_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *drm = crtc->dev;
+ struct ili9341_device *ili9341 = to_ili9341_device(drm);
+ struct mipi_dbi_dev *dbidev = &ili9341->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
u8 addr_mode;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -139,8 +171,35 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
+static const struct drm_crtc_helper_funcs ili9341_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = ili9341_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs ili9341_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs ili9341_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs ili9341_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs ili9341_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs ili9341_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs ili9341_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode yx240qv29_mode = {
@@ -176,18 +235,22 @@ MODULE_DEVICE_TABLE(spi, ili9341_id);
static int ili9341_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct ili9341_device *ili9341;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &ili9341_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ ili9341 = devm_drm_dev_alloc(dev, &ili9341_driver, struct ili9341_device, dbidev.drm);
+ if (IS_ERR(ili9341))
+ return PTR_ERR(ili9341);
+ dbidev = &ili9341->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -209,7 +272,53 @@ static int ili9341_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = mipi_dbi_dev_init(dbidev, &ili9341_pipe_funcs, &yx240qv29_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &yx240qv29_mode, ili9341_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &ili9341_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &ili9341_mode_config_helper_funcs;
+
+ plane = &ili9341->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &ili9341_plane_funcs,
+ ili9341_plane_formats, ARRAY_SIZE(ili9341_plane_formats),
+ ili9341_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &ili9341_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &ili9341->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &ili9341_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &ili9341_crtc_helper_funcs);
+
+ encoder = &ili9341->encoder;
+ ret = drm_encoder_init(drm, encoder, &ili9341_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &ili9341->connector;
+ ret = drm_connector_init(drm, connector, &ili9341_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &ili9341_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 08/16] drm/ili9486: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (6 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 07/16] drm/ili9341: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 09/16] drm/mi0283qt: " Thomas Zimmermann
` (8 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct ili9486_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Ili9486 requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v3:
- set ili9486 variable (David)
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/ili9486.c | 136 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index 52b14f2cb0e1..030e87a8a3f0 100644
--- a/drivers/gpu/drm/tiny/ili9486.c
+++ b/drivers/gpu/drm/tiny/ili9486.c
@@ -36,6 +36,20 @@
#define ILI9486_MADCTL_MX BIT(6)
#define ILI9486_MADCTL_MY BIT(7)
+struct ili9486_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct ili9486_device *to_ili9486_device(struct drm_device *dev)
+{
+ return container_of(drm_to_mipi_dbi_dev(dev), struct ili9486_device, dbidev);
+}
+
/*
* The PiScreen/waveshare rpi-lcd-35 has a SPI to 16-bit parallel bus converter
* in front of the display controller. This means that 8-bit values have to be
@@ -94,16 +108,34 @@ static int waveshare_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
return ret;
}
-static void waveshare_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+static const u32 ili9486_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 ili9486_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs ili9486_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs ili9486_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void ili9486_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct ili9486_device *ili9486 = to_ili9486_device(drm);
+ struct mipi_dbi_dev *dbidev = &ili9486->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
u8 addr_mode;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -161,8 +193,35 @@ static void waveshare_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs waveshare_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(waveshare_enable),
+static const struct drm_crtc_helper_funcs ili9486_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = ili9486_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs ili9486_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs ili9486_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs ili9486_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs ili9486_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs ili9486_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs ili9486_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode waveshare_mode = {
@@ -201,18 +260,22 @@ MODULE_DEVICE_TABLE(spi, ili9486_id);
static int ili9486_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct ili9486_device *ili9486;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &ili9486_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ ili9486 = devm_drm_dev_alloc(dev, &ili9486_driver, struct ili9486_device, dbidev.drm);
+ if (IS_ERR(ili9486))
+ return PTR_ERR(ili9486);
+ dbidev = &ili9486->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -237,8 +300,53 @@ static int ili9486_probe(struct spi_device *spi)
dbi->command = waveshare_command;
dbi->read_commands = NULL;
- ret = mipi_dbi_dev_init(dbidev, &waveshare_pipe_funcs,
- &waveshare_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &waveshare_mode, ili9486_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &ili9486_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &ili9486_mode_config_helper_funcs;
+
+ plane = &ili9486->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &ili9486_plane_funcs,
+ ili9486_plane_formats, ARRAY_SIZE(ili9486_plane_formats),
+ ili9486_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &ili9486_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &ili9486->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &ili9486_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &ili9486_crtc_helper_funcs);
+
+ encoder = &ili9486->encoder;
+ ret = drm_encoder_init(drm, encoder, &ili9486_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &ili9486->connector;
+ ret = drm_connector_init(drm, connector, &ili9486_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &ili9486_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 09/16] drm/mi0283qt: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (7 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 08/16] drm/ili9486: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
` (7 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct mi0283qt_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Mi0283qt requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v3:
- set dbi variable (David)
v2:
- fix connector initialization
- fix coding style
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/mi0283qt.c | 135 +++++++++++++++++++++++++++++---
1 file changed, 122 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index f121e1a8a303..ee597d023390 100644
--- a/drivers/gpu/drm/tiny/mi0283qt.c
+++ b/drivers/gpu/drm/tiny/mi0283qt.c
@@ -50,16 +50,48 @@
#define ILI9341_MADCTL_MX BIT(6)
#define ILI9341_MADCTL_MY BIT(7)
-static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+struct mi0283qt_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct mi0283qt_device *to_mi0283qt_device(struct drm_device *dev)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ return container_of(drm_to_mipi_dbi_dev(dev), struct mi0283qt_device, dbidev);
+}
+
+static const u32 mi0283qt_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 mi0283qt_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs mi0283qt_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs mi0283qt_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void mi0283qt_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *drm = crtc->dev;
+ struct mi0283qt_device *mi0283qt = to_mi0283qt_device(drm);
+ struct mipi_dbi_dev *dbidev = &mi0283qt->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
u8 addr_mode;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -143,8 +175,35 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(mi0283qt_enable),
+static const struct drm_crtc_helper_funcs mi0283qt_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = mi0283qt_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs mi0283qt_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs mi0283qt_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs mi0283qt_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs mi0283qt_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs mi0283qt_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs mi0283qt_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode mi0283qt_mode = {
@@ -180,18 +239,22 @@ MODULE_DEVICE_TABLE(spi, mi0283qt_id);
static int mi0283qt_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct mi0283qt_device *mi0283qt;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &mi0283qt_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ mi0283qt = devm_drm_dev_alloc(dev, &mi0283qt_driver, struct mi0283qt_device, dbidev.drm);
+ if (IS_ERR(mi0283qt))
+ return PTR_ERR(mi0283qt);
+ dbidev = &mi0283qt->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -217,7 +280,53 @@ static int mi0283qt_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = mipi_dbi_dev_init(dbidev, &mi0283qt_pipe_funcs, &mi0283qt_mode, rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &mi0283qt_mode, mi0283qt_plane_formats[0],
+ rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &mi0283qt_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &mi0283qt_mode_config_helper_funcs;
+
+ plane = &mi0283qt->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &mi0283qt_plane_funcs,
+ mi0283qt_plane_formats, ARRAY_SIZE(mi0283qt_plane_formats),
+ mi0283qt_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &mi0283qt_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &mi0283qt->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &mi0283qt_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &mi0283qt_crtc_helper_funcs);
+
+ encoder = &mi0283qt->encoder;
+ ret = drm_encoder_init(drm, encoder, &mi0283qt_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &mi0283qt->connector;
+ ret = drm_connector_init(drm, connector, &mi0283qt_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &mi0283qt_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 10/16] drm/panel-mipi-dbi: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (8 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 09/16] drm/mi0283qt: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 11/16] drm/st7586: " Thomas Zimmermann
` (6 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct panel_mipi_dbi_device and initialize them as
part of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. Panel-mipi-dbi requires a custom helper
for CRTC enablement, and non-freeing cleanup of the pipeline.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
Tested-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/tiny/panel-mipi-dbi.c | 139 +++++++++++++++++++++++---
1 file changed, 123 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
index 4907945ab507..581e14528608 100644
--- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
+++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
@@ -233,18 +233,46 @@ static void panel_mipi_dbi_commands_execute(struct mipi_dbi *dbi,
}
}
-static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+struct panel_mipi_dbi_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct panel_mipi_dbi_device *to_panel_mipi_dbi_device(struct drm_device *dev)
+{
+ return container_of(drm_to_mipi_dbi_dev(dev), struct panel_mipi_dbi_device, dbidev);
+}
+
+static const u64 panel_mipi_dbi_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs panel_mipi_dbi_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs panel_mipi_dbi_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void panel_mipi_dbi_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct panel_mipi_dbi_device *panel_mipi_dbi = to_panel_mipi_dbi_device(drm);
+ struct mipi_dbi_dev *dbidev = &panel_mipi_dbi->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
int ret, idx;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
- drm_dbg(pipe->crtc.dev, "\n");
+ drm_dbg(drm, "\n");
ret = mipi_dbi_poweron_conditional_reset(dbidev);
if (ret < 0)
@@ -257,8 +285,35 @@ static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(panel_mipi_dbi_enable),
+static const struct drm_crtc_helper_funcs panel_mipi_dbi_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = panel_mipi_dbi_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs panel_mipi_dbi_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs panel_mipi_dbi_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs panel_mipi_dbi_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs panel_mipi_dbi_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs panel_mipi_dbi_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs panel_mipi_dbi_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
DEFINE_DRM_GEM_DMA_FOPS(panel_mipi_dbi_fops);
@@ -317,20 +372,27 @@ static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_displ
static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
- struct drm_display_mode mode;
+ struct panel_mipi_dbi_device *panel_mipi_dbi;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
+ struct drm_display_mode mode;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
unsigned int bpp;
size_t buf_size;
u32 formats[2];
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver, struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ panel_mipi_dbi = devm_drm_dev_alloc(dev, &panel_mipi_dbi_driver,
+ struct panel_mipi_dbi_device,
+ dbidev.drm);
+ if (IS_ERR(panel_mipi_dbi))
+ return PTR_ERR(panel_mipi_dbi);
+ dbidev = &panel_mipi_dbi->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -377,9 +439,54 @@ static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
return ret;
buf_size = DIV_ROUND_UP(mode.hdisplay * mode.vdisplay * bpp, 8);
- ret = mipi_dbi_dev_init_with_formats(dbidev, &panel_mipi_dbi_pipe_funcs,
- formats, ARRAY_SIZE(formats),
- &mode, 0, buf_size);
+ ret = drm_mipi_dbi_dev_init(dbidev, &mode, formats[0], 0, buf_size);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &panel_mipi_dbi_mode_config_funcs;
+ drm->mode_config.preferred_depth = bpp;
+ drm->mode_config.helper_private = &panel_mipi_dbi_mode_config_helper_funcs;
+
+ plane = &panel_mipi_dbi->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &panel_mipi_dbi_plane_funcs,
+ formats, ARRAY_SIZE(formats),
+ panel_mipi_dbi_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &panel_mipi_dbi_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &panel_mipi_dbi->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &panel_mipi_dbi_crtc_funcs,
+ NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &panel_mipi_dbi_crtc_helper_funcs);
+
+ encoder = &panel_mipi_dbi->encoder;
+ ret = drm_encoder_init(drm, encoder, &panel_mipi_dbi_encoder_funcs,
+ DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &panel_mipi_dbi->connector;
+ ret = drm_connector_init(drm, connector, &panel_mipi_dbi_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &panel_mipi_dbi_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 11/16] drm/st7586: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (9 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
` (5 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct st7586_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. St7586 requires custom helpers for
various pipeline elements, and non-freeing cleanup of the pipeline.
v3:
- return early in st7586_plane_helper_atomic_update (David)
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
Tested-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/sitronix/st7586.c | 174 ++++++++++++++++++++++++------
1 file changed, 139 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7586.c b/drivers/gpu/drm/sitronix/st7586.c
index b6f38e03e1e5..fa58b3d83564 100644
--- a/drivers/gpu/drm/sitronix/st7586.c
+++ b/drivers/gpu/drm/sitronix/st7586.c
@@ -13,6 +13,7 @@
#include <video/mipi_display.h>
#include <drm/clients/drm_client_setup.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
@@ -48,6 +49,20 @@
#define ST7586_DISP_CTRL_MX BIT(6)
#define ST7586_DISP_CTRL_MY BIT(7)
+struct st7586_device {
+ struct mipi_dbi_dev dbidev;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+};
+
+static struct st7586_device *to_st7586_device(struct drm_device *dev)
+{
+ return container_of(drm_to_mipi_dbi_dev(dev), struct st7586_device, dbidev);
+}
+
/*
* The ST7586 controller has an unusual pixel format where 2bpp grayscale is
* packed 3 pixels per byte with the first two pixels using 3 bits and the 3rd
@@ -147,38 +162,59 @@ static void st7586_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
}
-static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
+static const u32 st7586_plane_formats[] = {
+ DRM_FORMAT_XRGB8888,
+};
+
+static const u64 st7586_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static void st7586_plane_helper_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_state *state)
{
- struct drm_plane_state *state = pipe->plane.state;
- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
- struct drm_framebuffer *fb = state->fb;
+ struct drm_plane_state *plane_state = plane->state;
+ struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_rect rect;
int idx;
- if (!pipe->crtc.state->active)
+ if (!fb)
return;
- if (!drm_dev_enter(fb->dev, &idx))
+ if (!drm_dev_enter(plane->dev, &idx))
return;
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+ if (drm_atomic_helper_damage_merged(old_plane_state, plane_state, &rect))
st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
&shadow_plane_state->fmtcnv_state);
drm_dev_exit(idx);
}
-static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+static const struct drm_plane_helper_funcs st7586_plane_helper_funcs = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = drm_mipi_dbi_plane_helper_atomic_check,
+ .atomic_update = st7586_plane_helper_atomic_update
+};
+
+static const struct drm_plane_funcs st7586_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void st7586_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct st7586_device *st7586 = to_st7586_device(drm);
+ struct mipi_dbi_dev *dbidev = &st7586->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
int idx, ret;
u8 addr_mode;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -239,9 +275,12 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
+static void st7586_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct st7586_device *st7586 = to_st7586_device(drm);
+ struct mipi_dbi_dev *dbidev = &st7586->dbidev;
/*
* This callback is not protected by drm_dev_enter/exit since we want to
@@ -255,20 +294,37 @@ static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
mipi_dbi_command(&dbidev->dbi, MIPI_DCS_SET_DISPLAY_OFF);
}
-static const u32 st7586_formats[] = {
- DRM_FORMAT_XRGB8888,
+static const struct drm_crtc_helper_funcs st7586_crtc_helper_funcs = {
+ .mode_valid = drm_mipi_dbi_crtc_helper_mode_valid,
+ .atomic_check = drm_mipi_dbi_crtc_helper_atomic_check,
+ .atomic_enable = st7586_crtc_helper_atomic_enable,
+ .atomic_disable = st7586_crtc_helper_atomic_disable,
+};
+
+static const struct drm_crtc_funcs st7586_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
};
-static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
- .mode_valid = mipi_dbi_pipe_mode_valid,
- .enable = st7586_pipe_enable,
- .disable = st7586_pipe_disable,
- .update = st7586_pipe_update,
- .begin_fb_access = mipi_dbi_pipe_begin_fb_access,
- .end_fb_access = mipi_dbi_pipe_end_fb_access,
- .reset_plane = mipi_dbi_pipe_reset_plane,
- .duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
- .destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
+static const struct drm_encoder_funcs st7586_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs st7586_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs st7586_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs st7586_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs st7586_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct drm_display_mode st7586_mode = {
@@ -304,19 +360,23 @@ MODULE_DEVICE_TABLE(spi, st7586_id);
static int st7586_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ struct st7586_device *st7586;
struct mipi_dbi_dev *dbidev;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *a0;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
size_t bufsize;
int ret;
- dbidev = devm_drm_dev_alloc(dev, &st7586_driver,
- struct mipi_dbi_dev, drm);
- if (IS_ERR(dbidev))
- return PTR_ERR(dbidev);
-
+ st7586 = devm_drm_dev_alloc(dev, &st7586_driver, struct st7586_device, dbidev.drm);
+ if (IS_ERR(st7586))
+ return PTR_ERR(st7586);
+ dbidev = &st7586->dbidev;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
@@ -345,9 +405,53 @@ static int st7586_probe(struct spi_device *spi)
/* Cannot read from this controller via SPI */
dbi->read_commands = NULL;
- ret = mipi_dbi_dev_init_with_formats(dbidev, &st7586_pipe_funcs,
- st7586_formats, ARRAY_SIZE(st7586_formats),
- &st7586_mode, rotation, bufsize);
+ ret = drm_mipi_dbi_dev_init(dbidev, &st7586_mode, st7586_plane_formats[0],
+ rotation, bufsize);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &st7586_mode_config_funcs;
+ drm->mode_config.preferred_depth = 24;
+ drm->mode_config.helper_private = &st7586_mode_config_helper_funcs;
+
+ plane = &st7586->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &st7586_plane_funcs,
+ st7586_plane_formats, ARRAY_SIZE(st7586_plane_formats),
+ st7586_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &st7586_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &st7586->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &st7586_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &st7586_crtc_helper_funcs);
+
+ encoder = &st7586->encoder;
+ ret = drm_encoder_init(drm, encoder, &st7586_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &st7586->connector;
+ ret = drm_connector_init(drm, connector, &st7586_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &st7586_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (10 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 11/16] drm/st7586: " Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
` (4 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Rename the driver's device struct according to DRM conventions. Also
add a helper to upcast from struct drm_device. No functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/sitronix/st7735r.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7735r.c b/drivers/gpu/drm/sitronix/st7735r.c
index 1a34c7ba460b..9dd915765a6b 100644
--- a/drivers/gpu/drm/sitronix/st7735r.c
+++ b/drivers/gpu/drm/sitronix/st7735r.c
@@ -52,18 +52,22 @@ struct st7735r_cfg {
unsigned int rgb:1; /* RGB (vs. BGR) */
};
-struct st7735r_priv {
+struct st7735r_device {
struct mipi_dbi_dev dbidev; /* Must be first for .release() */
const struct st7735r_cfg *cfg;
};
+static struct st7735r_device *to_st7735r_device(struct drm_device *drm)
+{
+ return container_of(drm_to_mipi_dbi_dev(drm), struct st7735r_device, dbidev);
+}
+
static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
struct drm_crtc_state *crtc_state,
struct drm_plane_state *plane_state)
{
- struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
- struct st7735r_priv *priv = container_of(dbidev, struct st7735r_priv,
- dbidev);
+ struct st7735r_device *priv = to_st7735r_device(pipe->crtc.dev);
+ struct mipi_dbi_dev *dbidev = &priv->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
int ret, idx;
u8 addr_mode;
@@ -184,7 +188,7 @@ static int st7735r_probe(struct spi_device *spi)
struct device *dev = &spi->dev;
const struct st7735r_cfg *cfg;
struct mipi_dbi_dev *dbidev;
- struct st7735r_priv *priv;
+ struct st7735r_device *priv;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
@@ -196,7 +200,7 @@ static int st7735r_probe(struct spi_device *spi)
cfg = (void *)spi_get_device_id(spi)->driver_data;
priv = devm_drm_dev_alloc(dev, &st7735r_driver,
- struct st7735r_priv, dbidev.drm);
+ struct st7735r_device, dbidev.drm);
if (IS_ERR(priv))
return PTR_ERR(priv);
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 13/16] drm/st7735r: Rename priv variable to st7735r
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (11 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
` (3 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Rename the driver's device variable according to DRM conventions. No
functional changes.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/sitronix/st7735r.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7735r.c b/drivers/gpu/drm/sitronix/st7735r.c
index 9dd915765a6b..e70c46df0299 100644
--- a/drivers/gpu/drm/sitronix/st7735r.c
+++ b/drivers/gpu/drm/sitronix/st7735r.c
@@ -66,8 +66,8 @@ static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
struct drm_crtc_state *crtc_state,
struct drm_plane_state *plane_state)
{
- struct st7735r_device *priv = to_st7735r_device(pipe->crtc.dev);
- struct mipi_dbi_dev *dbidev = &priv->dbidev;
+ struct st7735r_device *st7735r = to_st7735r_device(pipe->crtc.dev);
+ struct mipi_dbi_dev *dbidev = &st7735r->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
int ret, idx;
u8 addr_mode;
@@ -113,7 +113,7 @@ static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
break;
}
- if (priv->cfg->rgb)
+ if (st7735r->cfg->rgb)
addr_mode |= ST7735R_RGB;
mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
@@ -188,7 +188,7 @@ static int st7735r_probe(struct spi_device *spi)
struct device *dev = &spi->dev;
const struct st7735r_cfg *cfg;
struct mipi_dbi_dev *dbidev;
- struct st7735r_device *priv;
+ struct st7735r_device *st7735r;
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
@@ -199,13 +199,12 @@ static int st7735r_probe(struct spi_device *spi)
if (!cfg)
cfg = (void *)spi_get_device_id(spi)->driver_data;
- priv = devm_drm_dev_alloc(dev, &st7735r_driver,
- struct st7735r_device, dbidev.drm);
- if (IS_ERR(priv))
- return PTR_ERR(priv);
+ st7735r = devm_drm_dev_alloc(dev, &st7735r_driver, struct st7735r_device, dbidev.drm);
+ if (IS_ERR(st7735r))
+ return PTR_ERR(st7735r);
- dbidev = &priv->dbidev;
- priv->cfg = cfg;
+ dbidev = &st7735r->dbidev;
+ st7735r->cfg = cfg;
dbi = &dbidev->dbi;
drm = &dbidev->drm;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (12 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
` (2 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Replace simple-display helpers with regular atomic helpers. Store the
pipeline elements in struct st7735r_device and initialize them as part
of probing the device. Use mipi-dbi's existing helpers and initializer
macros where possible.
Effectively open-codes the modesetting code in the initializer helpers
of mipi-dbi and simple-display. St7735r requires a custom helper for
CRTC enablement, and non-freeing cleanup of the pipeline.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/sitronix/st7735r.c | 115 ++++++++++++++++++++++++++---
1 file changed, 106 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7735r.c b/drivers/gpu/drm/sitronix/st7735r.c
index e70c46df0299..5a75716ed5ff 100644
--- a/drivers/gpu/drm/sitronix/st7735r.c
+++ b/drivers/gpu/drm/sitronix/st7735r.c
@@ -55,6 +55,11 @@ struct st7735r_cfg {
struct st7735r_device {
struct mipi_dbi_dev dbidev; /* Must be first for .release() */
const struct st7735r_cfg *cfg;
+
+ struct drm_plane plane;
+ struct drm_crtc crtc;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
};
static struct st7735r_device *to_st7735r_device(struct drm_device *drm)
@@ -62,17 +67,34 @@ static struct st7735r_device *to_st7735r_device(struct drm_device *drm)
return container_of(drm_to_mipi_dbi_dev(drm), struct st7735r_device, dbidev);
}
-static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+static const u32 st7735r_plane_formats[] = {
+ DRM_MIPI_DBI_PLANE_FORMATS,
+};
+
+static const u64 st7735r_plane_format_modifiers[] = {
+ DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
+};
+
+static const struct drm_plane_helper_funcs st7735r_plane_helper_funcs = {
+ DRM_MIPI_DBI_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs st7735r_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
+
+static void st7735r_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
{
- struct st7735r_device *st7735r = to_st7735r_device(pipe->crtc.dev);
+ struct drm_device *drm = crtc->dev;
+ struct st7735r_device *st7735r = to_st7735r_device(drm);
struct mipi_dbi_dev *dbidev = &st7735r->dbidev;
struct mipi_dbi *dbi = &dbidev->dbi;
int ret, idx;
u8 addr_mode;
- if (!drm_dev_enter(pipe->crtc.dev, &idx))
+ if (!drm_dev_enter(drm, &idx))
return;
DRM_DEBUG_KMS("\n");
@@ -138,8 +160,35 @@ static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
drm_dev_exit(idx);
}
-static const struct drm_simple_display_pipe_funcs st7735r_pipe_funcs = {
- DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(st7735r_pipe_enable),
+static const struct drm_crtc_helper_funcs st7735r_crtc_helper_funcs = {
+ DRM_MIPI_DBI_CRTC_HELPER_FUNCS,
+ .atomic_enable = st7735r_crtc_helper_atomic_enable,
+};
+
+static const struct drm_crtc_funcs st7735r_crtc_funcs = {
+ DRM_MIPI_DBI_CRTC_FUNCS,
+ .destroy = drm_crtc_cleanup,
+};
+
+static const struct drm_encoder_funcs st7735r_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs st7735r_connector_helper_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
+};
+
+static const struct drm_connector_funcs st7735r_connector_funcs = {
+ DRM_MIPI_DBI_CONNECTOR_FUNCS,
+ .destroy = drm_connector_cleanup,
+};
+
+static const struct drm_mode_config_helper_funcs st7735r_mode_config_helper_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
+};
+
+static const struct drm_mode_config_funcs st7735r_mode_config_funcs = {
+ DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
};
static const struct st7735r_cfg jd_t18003_t01_cfg = {
@@ -192,6 +241,10 @@ static int st7735r_probe(struct spi_device *spi)
struct drm_device *drm;
struct mipi_dbi *dbi;
struct gpio_desc *dc;
+ struct drm_plane *plane;
+ struct drm_crtc *crtc;
+ struct drm_encoder *encoder;
+ struct drm_connector *connector;
u32 rotation = 0;
int ret;
@@ -233,8 +286,52 @@ static int st7735r_probe(struct spi_device *spi)
dbidev->left_offset = cfg->left_offset;
dbidev->top_offset = cfg->top_offset;
- ret = mipi_dbi_dev_init(dbidev, &st7735r_pipe_funcs, &cfg->mode,
- rotation);
+ ret = drm_mipi_dbi_dev_init(dbidev, &cfg->mode, st7735r_plane_formats[0], rotation, 0);
+ if (ret)
+ return ret;
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.min_width = dbidev->mode.hdisplay;
+ drm->mode_config.max_width = dbidev->mode.hdisplay;
+ drm->mode_config.min_height = dbidev->mode.vdisplay;
+ drm->mode_config.max_height = dbidev->mode.vdisplay;
+ drm->mode_config.funcs = &st7735r_mode_config_funcs;
+ drm->mode_config.preferred_depth = 16;
+ drm->mode_config.helper_private = &st7735r_mode_config_helper_funcs;
+
+ plane = &st7735r->plane;
+ ret = drm_universal_plane_init(drm, plane, 0, &st7735r_plane_funcs,
+ st7735r_plane_formats, ARRAY_SIZE(st7735r_plane_formats),
+ st7735r_plane_format_modifiers,
+ DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(plane, &st7735r_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(plane);
+
+ crtc = &st7735r->crtc;
+ ret = drm_crtc_init_with_planes(drm, crtc, plane, NULL, &st7735r_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(crtc, &st7735r_crtc_helper_funcs);
+
+ encoder = &st7735r->encoder;
+ ret = drm_encoder_init(drm, encoder, &st7735r_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+ connector = &st7735r->connector;
+ ret = drm_connector_init(drm, connector, &st7735r_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+ drm_connector_helper_add(connector, &st7735r_connector_helper_funcs);
+
+ ret = drm_connector_attach_encoder(connector, encoder);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (13 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
2026-03-21 18:15 ` Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Claude Code Review Bot
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
With the conversion to regular atomic helpers, mipi-dbi's support
for simple-display helpers is unused. Removed it.
v2:
- remove unused connector from struct mipi_dbi_dev
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
drivers/gpu/drm/drm_mipi_dbi.c | 247 ---------------------------------
include/drm/drm_mipi_dbi.h | 54 -------
2 files changed, 301 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 0374fd0c527d..9ab6113d74a2 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -15,7 +15,6 @@
#include <linux/spi/spi.h>
#include <drm/drm_atomic.h>
-#include <drm/drm_connector.h>
#include <drm/drm_damage_helper.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
@@ -332,22 +331,6 @@ enum drm_mode_status drm_mipi_dbi_crtc_helper_mode_valid(struct drm_crtc *crtc,
}
EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_mode_valid);
-/**
- * mipi_dbi_pipe_mode_valid - MIPI DBI mode-valid helper
- * @pipe: Simple display pipe
- * @mode: The mode to test
- *
- * This function validates a given display mode against the MIPI DBI's hardware
- * display. Drivers can use this as their &drm_simple_display_pipe_funcs->mode_valid
- * callback.
- */
-enum drm_mode_status mipi_dbi_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
- const struct drm_display_mode *mode)
-{
- return drm_mipi_dbi_crtc_helper_mode_valid(&pipe->crtc, mode);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
-
/**
* drm_mipi_dbi_plane_helper_atomic_check - MIPI DBI plane check helper
* @plane: Plane to check
@@ -413,21 +396,6 @@ void drm_mipi_dbi_plane_helper_atomic_update(struct drm_plane *plane,
}
EXPORT_SYMBOL(drm_mipi_dbi_plane_helper_atomic_update);
-/**
- * mipi_dbi_pipe_update - Display pipe update helper
- * @pipe: Simple display pipe
- * @old_state: Old plane state
- *
- * This function handles framebuffer flushing and vblank events. Drivers can use
- * this as their &drm_simple_display_pipe_funcs->update callback.
- */
-void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state)
-{
- return drm_mipi_dbi_plane_helper_atomic_update(&pipe->plane, old_state->state);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_update);
-
static void mipi_dbi_blank(struct mipi_dbi_dev *dbidev)
{
struct drm_device *drm = &dbidev->drm;
@@ -509,104 +477,6 @@ void drm_mipi_dbi_crtc_helper_atomic_disable(struct drm_crtc *crtc,
}
EXPORT_SYMBOL(drm_mipi_dbi_crtc_helper_atomic_disable);
-/**
- * mipi_dbi_pipe_disable - MIPI DBI pipe disable helper
- * @pipe: Display pipe
- *
- * This function disables backlight if present, if not the display memory is
- * blanked. The regulator is disabled if in use. Drivers can use this as their
- * &drm_simple_display_pipe_funcs->disable callback.
- */
-void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
-{
- drm_mipi_dbi_crtc_helper_atomic_disable(&pipe->crtc, pipe->crtc.state->state);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_disable);
-
-/**
- * mipi_dbi_pipe_begin_fb_access - MIPI DBI pipe begin-access helper
- * @pipe: Display pipe
- * @plane_state: Plane state
- *
- * This function implements struct &drm_simple_display_funcs.begin_fb_access.
- *
- * See drm_gem_begin_shadow_fb_access() for details and mipi_dbi_pipe_cleanup_fb()
- * for cleanup.
- *
- * Returns:
- * 0 on success, or a negative errno code otherwise.
- */
-int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state)
-{
- return drm_gem_begin_shadow_fb_access(&pipe->plane, plane_state);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_begin_fb_access);
-
-/**
- * mipi_dbi_pipe_end_fb_access - MIPI DBI pipe end-access helper
- * @pipe: Display pipe
- * @plane_state: Plane state
- *
- * This function implements struct &drm_simple_display_funcs.end_fb_access.
- *
- * See mipi_dbi_pipe_begin_fb_access().
- */
-void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state)
-{
- drm_gem_end_shadow_fb_access(&pipe->plane, plane_state);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_end_fb_access);
-
-/**
- * mipi_dbi_pipe_reset_plane - MIPI DBI plane-reset helper
- * @pipe: Display pipe
- *
- * This function implements struct &drm_simple_display_funcs.reset_plane
- * for MIPI DBI planes.
- */
-void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe)
-{
- drm_gem_reset_shadow_plane(&pipe->plane);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_reset_plane);
-
-/**
- * mipi_dbi_pipe_duplicate_plane_state - duplicates MIPI DBI plane state
- * @pipe: Display pipe
- *
- * This function implements struct &drm_simple_display_funcs.duplicate_plane_state
- * for MIPI DBI planes.
- *
- * See drm_gem_duplicate_shadow_plane_state() for additional details.
- *
- * Returns:
- * A pointer to a new plane state on success, or NULL otherwise.
- */
-struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe)
-{
- return drm_gem_duplicate_shadow_plane_state(&pipe->plane);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_duplicate_plane_state);
-
-/**
- * mipi_dbi_pipe_destroy_plane_state - cleans up MIPI DBI plane state
- * @pipe: Display pipe
- * @plane_state: Plane state
- *
- * This function implements struct drm_simple_display_funcs.destroy_plane_state
- * for MIPI DBI planes.
- *
- * See drm_gem_destroy_shadow_plane_state() for additional details.
- */
-void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state)
-{
- drm_gem_destroy_shadow_plane_state(&pipe->plane, plane_state);
-}
-EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
-
/**
* drm_mipi_dbi_connector_helper_get_modes - Duplicates the MIPI DBI mode for the connector
* @connector: The connector
@@ -626,15 +496,6 @@ int drm_mipi_dbi_connector_helper_get_modes(struct drm_connector *connector)
}
EXPORT_SYMBOL(drm_mipi_dbi_connector_helper_get_modes);
-static const struct drm_connector_helper_funcs mipi_dbi_connector_hfuncs = {
- DRM_MIPI_DBI_CONNECTOR_HELPER_FUNCS,
-};
-
-static const struct drm_connector_funcs mipi_dbi_connector_funcs = {
- DRM_MIPI_DBI_CONNECTOR_FUNCS,
- .destroy = drm_connector_cleanup,
-};
-
static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
unsigned int rotation)
{
@@ -652,18 +513,6 @@ static int mipi_dbi_rotate_mode(struct drm_display_mode *mode,
}
}
-static const struct drm_mode_config_helper_funcs mipi_dbi_mode_config_helper_funcs = {
- DRM_MIPI_DBI_MODE_CONFIG_HELPER_FUNCS,
-};
-
-static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = {
- DRM_MIPI_DBI_MODE_CONFIG_FUNCS,
-};
-
-static const uint32_t mipi_dbi_formats[] = {
- DRM_MIPI_DBI_PLANE_FORMATS,
-};
-
/**
* drm_mipi_dbi_dev_init - MIPI DBI device initialization
* @dbidev: MIPI DBI device structure to initialize
@@ -717,102 +566,6 @@ int drm_mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_display_
}
EXPORT_SYMBOL(drm_mipi_dbi_dev_init);
-/**
- * mipi_dbi_dev_init_with_formats - MIPI DBI device initialization with custom formats
- * @dbidev: MIPI DBI device structure to initialize
- * @funcs: Display pipe functions
- * @formats: Array of supported formats (DRM_FORMAT\_\*).
- * @format_count: Number of elements in @formats
- * @mode: Display mode
- * @rotation: Initial rotation in degrees Counter Clock Wise
- * @tx_buf_size: Allocate a transmit buffer of this size.
- *
- * This function sets up a &drm_simple_display_pipe with a &drm_connector that
- * has one fixed &drm_display_mode which is rotated according to @rotation.
- * This mode is used to set the mode config min/max width/height properties.
- *
- * Use mipi_dbi_dev_init() if you want native RGB565 and emulated XRGB8888 format.
- *
- * Note:
- * Some of the helper functions expects RGB565 to be the default format and the
- * transmit buffer sized to fit that.
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
- const struct drm_simple_display_pipe_funcs *funcs,
- const uint32_t *formats, unsigned int format_count,
- const struct drm_display_mode *mode,
- unsigned int rotation, size_t tx_buf_size)
-{
- static const uint64_t modifiers[] = {
- DRM_MIPI_DBI_PLANE_FORMAT_MODIFIERS,
- };
- struct drm_device *drm = &dbidev->drm;
- int ret;
-
- ret = drm_mipi_dbi_dev_init(dbidev, mode, formats[0], rotation, tx_buf_size);
- if (ret)
- return ret;
-
- ret = drmm_mode_config_init(drm);
- if (ret)
- return ret;
-
- drm_connector_helper_add(&dbidev->connector, &mipi_dbi_connector_hfuncs);
- ret = drm_connector_init(drm, &dbidev->connector, &mipi_dbi_connector_funcs,
- DRM_MODE_CONNECTOR_SPI);
- if (ret)
- return ret;
-
- ret = drm_simple_display_pipe_init(drm, &dbidev->pipe, funcs, formats, format_count,
- modifiers, &dbidev->connector);
- if (ret)
- return ret;
-
- drm_plane_enable_fb_damage_clips(&dbidev->pipe.plane);
-
- drm->mode_config.funcs = &mipi_dbi_mode_config_funcs;
- drm->mode_config.min_width = dbidev->mode.hdisplay;
- drm->mode_config.max_width = dbidev->mode.hdisplay;
- drm->mode_config.min_height = dbidev->mode.vdisplay;
- drm->mode_config.max_height = dbidev->mode.vdisplay;
- drm->mode_config.helper_private = &mipi_dbi_mode_config_helper_funcs;
-
- return 0;
-}
-EXPORT_SYMBOL(mipi_dbi_dev_init_with_formats);
-
-/**
- * mipi_dbi_dev_init - MIPI DBI device initialization
- * @dbidev: MIPI DBI device structure to initialize
- * @funcs: Display pipe functions
- * @mode: Display mode
- * @rotation: Initial rotation in degrees Counter Clock Wise
- *
- * This function sets up a &drm_simple_display_pipe with a &drm_connector that
- * has one fixed &drm_display_mode which is rotated according to @rotation.
- * This mode is used to set the mode config min/max width/height properties.
- * Additionally &mipi_dbi.tx_buf is allocated.
- *
- * Supported formats: Native RGB565 and emulated XRGB8888.
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
- const struct drm_simple_display_pipe_funcs *funcs,
- const struct drm_display_mode *mode, unsigned int rotation)
-{
- dbidev->drm.mode_config.preferred_depth = 16;
-
- return mipi_dbi_dev_init_with_formats(dbidev, funcs, mipi_dbi_formats,
- ARRAY_SIZE(mipi_dbi_formats), mode,
- rotation, 0);
-}
-EXPORT_SYMBOL(mipi_dbi_dev_init);
-
/**
* mipi_dbi_hw_reset - Hardware reset of controller
* @dbi: MIPI DBI structure
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index ae92a5e8d13b..07374eb5d88e 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -15,7 +15,6 @@
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
struct drm_format_conv_state;
struct drm_rect;
@@ -91,16 +90,6 @@ struct mipi_dbi_dev {
*/
struct drm_device drm;
- /**
- * @pipe: Display pipe structure
- */
- struct drm_simple_display_pipe pipe;
-
- /**
- * @connector: Connector
- */
- struct drm_connector connector;
-
/**
* @mode: Fixed display mode
*/
@@ -173,28 +162,6 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *dbi,
int drm_mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, const struct drm_display_mode *mode,
u32 format, unsigned int rotation, size_t tx_buf_size);
-int mipi_dbi_dev_init_with_formats(struct mipi_dbi_dev *dbidev,
- const struct drm_simple_display_pipe_funcs *funcs,
- const uint32_t *formats, unsigned int format_count,
- const struct drm_display_mode *mode,
- unsigned int rotation, size_t tx_buf_size);
-int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
- const struct drm_simple_display_pipe_funcs *funcs,
- const struct drm_display_mode *mode, unsigned int rotation);
-enum drm_mode_status mipi_dbi_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
- const struct drm_display_mode *mode);
-void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *old_state);
-void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
-int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state);
-void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state);
-void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe);
-struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe);
-void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
- struct drm_plane_state *plane_state);
-
void mipi_dbi_hw_reset(struct mipi_dbi *dbi);
bool mipi_dbi_display_is_on(struct mipi_dbi *dbi);
int mipi_dbi_poweron_reset(struct mipi_dbi_dev *dbidev);
@@ -322,25 +289,4 @@ void mipi_dbi_debugfs_init(struct drm_minor *minor);
static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
#endif
-/**
- * DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS - Initializes struct drm_simple_display_pipe_funcs
- * for MIPI-DBI devices
- * @enable_: Enable-callback implementation
- *
- * This macro initializes struct drm_simple_display_pipe_funcs with default
- * values for MIPI-DBI-based devices. The only callback that depends on the
- * hardware is @enable, for which the driver has to provide an implementation.
- * MIPI-based drivers are encouraged to use this macro for initialization.
- */
-#define DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(enable_) \
- .mode_valid = mipi_dbi_pipe_mode_valid, \
- .enable = (enable_), \
- .disable = mipi_dbi_pipe_disable, \
- .update = mipi_dbi_pipe_update, \
- .begin_fb_access = mipi_dbi_pipe_begin_fb_access, \
- .end_fb_access = mipi_dbi_pipe_end_fb_access, \
- .reset_plane = mipi_dbi_pipe_reset_plane, \
- .duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state, \
- .destroy_plane_state = mipi_dbi_pipe_destroy_plane_state
-
#endif /* __LINUX_MIPI_DBI_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v3 16/16] drm/simple-kms: Deprecate simple-kms helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (14 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
@ 2026-03-19 15:59 ` Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-21 18:15 ` Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Claude Code Review Bot
16 siblings, 1 reply; 35+ messages in thread
From: Thomas Zimmermann @ 2026-03-19 15:59 UTC (permalink / raw)
To: david, javierm, lanzano.alex, kamlesh.gurudasani, architanant5,
wens, mripard, maarten.lankhorst, simona, airlied
Cc: dri-devel, Thomas Zimmermann
Deprecate simple-encoder and simple-display-pipe helpers in favor of
regular atomic helpers. Remove the related documentation. Add TODO
item for converting the remaining drivers.
These helpers have been deprecated for years and many drivers have
been updated to not use them. Still there are a few left and we
occasionally receive new drivers build upon them. Marking them as
deprecated will hopefully resolve these problems. The TODO items
should be easy enough for getting new voluteers started on DRM driver
development.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: David Lechner <david@lechnology.com>
---
Documentation/gpu/drm-kms-helpers.rst | 12 --
Documentation/gpu/introduction.rst | 6 -
Documentation/gpu/todo.rst | 32 ++++
drivers/gpu/drm/drm_crtc.c | 6 +-
drivers/gpu/drm/drm_gem_atomic_helper.c | 22 ---
drivers/gpu/drm/drm_modeset_helper.c | 3 -
drivers/gpu/drm/drm_simple_kms_helper.c | 83 ---------
include/drm/drm_simple_kms_helper.h | 216 +-----------------------
8 files changed, 39 insertions(+), 341 deletions(-)
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 781129f78b06..b4a9e5ae81f6 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -104,18 +104,6 @@ VBLANK Helper Reference
.. kernel-doc:: drivers/gpu/drm/drm_vblank_helper.c
:export:
-Simple KMS Helper Reference
-===========================
-
-.. kernel-doc:: drivers/gpu/drm/drm_simple_kms_helper.c
- :doc: overview
-
-.. kernel-doc:: include/drm/drm_simple_kms_helper.h
- :internal:
-
-.. kernel-doc:: drivers/gpu/drm/drm_simple_kms_helper.c
- :export:
-
fbdev Helper Functions Reference
================================
diff --git a/Documentation/gpu/introduction.rst b/Documentation/gpu/introduction.rst
index 3cd0c8860b94..d8f519693fc2 100644
--- a/Documentation/gpu/introduction.rst
+++ b/Documentation/gpu/introduction.rst
@@ -119,12 +119,6 @@ Simple DRM drivers to use as examples
The DRM subsystem contains a lot of helper functions to ease writing drivers for
simple graphic devices. For example, the `drivers/gpu/drm/tiny/` directory has a
set of drivers that are simple enough to be implemented in a single source file.
-
-These drivers make use of the `struct drm_simple_display_pipe_funcs`, that hides
-any complexity of the DRM subsystem and just requires drivers to implement a few
-functions needed to operate the device. This could be used for devices that just
-need a display pipeline with one full-screen scanout buffer feeding one output.
-
The tiny DRM drivers are good examples to understand how DRM drivers should look
like. Since are just a few hundreds lines of code, they are quite easy to read.
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 520da44a04a6..bc9f14c8a2ec 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -29,6 +29,38 @@ refactorings already and are an expert in the specific area
Subsystem-wide refactorings
===========================
+Open-code drm_simple_encoder_init()
+-----------------------------------
+
+The helper drm_simple_encoder_init() was supposed to simplify encoder
+initialization. Instead it only added an intermediate layer between atomic
+modesetting and the DRM driver.
+
+The task here is to remove drm_simple_encoder_init(). Search for a driver
+that calls drm_simple_encoder_init() and inline the helper. The driver will
+also need its own instance of drm_encoder_funcs.
+
+Contact: Thomas Zimmermann, respective driver maintainer
+
+Level: Easy
+
+Replace struct drm_simple_display_pipe with regular atomic helpers
+------------------------------------------------------------------
+
+The data type struct drm_simple_display_pipe and its helpers were supposed
+to simplify driver development. Instead they only added an intermediate layer
+between atomic modesetting and the DRM driver.
+
+There are still drivers that use drm_simple_display_pipe. The task here is to
+convert them to use regular atomic helpers. Search for a driver that calls
+drm_simple_display_pipe_init() and inline all helpers from drm_simple_kms_helper.c
+into the driver, such that no simple-KMS interfaces are required. Please also
+rename all inlined fucntions according to driver conventions.
+
+Contact: Thomas Zimmermann, respective driver maintainer
+
+Level: Easy
+
Remove custom dumb_map_offset implementations
---------------------------------------------
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 8d6f721c2c9a..63ead8ba6756 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -340,8 +340,7 @@ static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *
* Inits a new object created as base part of a driver crtc object. Drivers
* should use this function instead of drm_crtc_init(), which is only provided
* for backwards compatibility with drivers which do not yet support universal
- * planes). For really simple hardware which has only 1 plane look at
- * drm_simple_display_pipe_init() instead.
+ * planes).
* The &drm_crtc_funcs.destroy hook should call drm_crtc_cleanup() and kfree()
* the crtc structure. The crtc structure should not be allocated with
* devm_kzalloc().
@@ -424,8 +423,7 @@ static int __drmm_crtc_init_with_planes(struct drm_device *dev,
* Inits a new object created as base part of a driver crtc object. Drivers
* should use this function instead of drm_crtc_init(), which is only provided
* for backwards compatibility with drivers which do not yet support universal
- * planes). For really simple hardware which has only 1 plane look at
- * drm_simple_display_pipe_init() instead.
+ * planes).
*
* Cleanup is automatically handled through registering
* drmm_crtc_cleanup() with drmm_add_action(). The crtc structure should
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index 421c460ac972..abef865c5f2c 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -87,28 +87,6 @@
* A mapping address for each of the framebuffer's buffer object is stored in
* struct &drm_shadow_plane_state.map. The mappings are valid while the state
* is being used.
- *
- * Drivers that use struct drm_simple_display_pipe can use
- * %DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS to initialize the rsp
- * callbacks. Access to shadow-buffer mappings is similar to regular
- * atomic_update.
- *
- * .. code-block:: c
- *
- * struct drm_simple_display_pipe_funcs driver_pipe_funcs = {
- * ...,
- * DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS,
- * };
- *
- * void driver_pipe_enable(struct drm_simple_display_pipe *pipe,
- * struct drm_crtc_state *crtc_state,
- * struct drm_plane_state *plane_state)
- * {
- * struct drm_shadow_plane_state *shadow_plane_state =
- * to_drm_shadow_plane_state(plane_state);
- *
- * // access shadow buffer via shadow_plane_state->map
- * }
*/
/*
diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index a57f6a10ada4..d7721df744e7 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -135,9 +135,6 @@ static const struct drm_plane_funcs primary_plane_funcs = {
*
* This is purely a backwards compatibility helper for old drivers. Drivers
* should instead implement their own primary plane. Atomic drivers must do so.
- * Drivers with the above hardware restriction can look into using &struct
- * drm_simple_display_pipe, which encapsulates the above limitations into a nice
- * interface.
*
* Returns:
* Zero on success, error code on failure.
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index fcbcaaa36b5f..4d91513a1e34 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -16,55 +16,10 @@
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
-/**
- * DOC: overview
- *
- * This helper library provides helpers for drivers for simple display
- * hardware.
- *
- * drm_simple_display_pipe_init() initializes a simple display pipeline
- * which has only one full-screen scanout buffer feeding one output. The
- * pipeline is represented by &struct drm_simple_display_pipe and binds
- * together &drm_plane, &drm_crtc and &drm_encoder structures into one fixed
- * entity. Some flexibility for code reuse is provided through a separately
- * allocated &drm_connector object and supporting optional &drm_bridge
- * encoder drivers.
- *
- * Many drivers require only a very simple encoder that fulfills the minimum
- * requirements of the display pipeline and does not add additional
- * functionality. The function drm_simple_encoder_init() provides an
- * implementation of such an encoder.
- */
-
static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
.destroy = drm_encoder_cleanup,
};
-/**
- * drm_simple_encoder_init - Initialize a preallocated encoder with
- * basic functionality.
- * @dev: drm device
- * @encoder: the encoder to initialize
- * @encoder_type: user visible type of the encoder
- *
- * Initialises a preallocated encoder that has no further functionality.
- * Settings for possible CRTC and clones are left to their initial values.
- * The encoder will be cleaned up automatically as part of the mode-setting
- * cleanup.
- *
- * The caller of drm_simple_encoder_init() is responsible for freeing
- * the encoder's memory after the encoder has been cleaned up. At the
- * moment this only works reliably if the encoder data structure is
- * stored in the device structure. Free the encoder's memory as part of
- * the device release function.
- *
- * Note: consider using drmm_simple_encoder_alloc() instead of
- * drm_simple_encoder_init() to let the DRM managed resource infrastructure
- * take care of cleanup and deallocation.
- *
- * Returns:
- * Zero on success, error code on failure.
- */
int drm_simple_encoder_init(struct drm_device *dev,
struct drm_encoder *encoder,
int encoder_type)
@@ -370,20 +325,6 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
.format_mod_supported = drm_simple_kms_format_mod_supported,
};
-/**
- * drm_simple_display_pipe_attach_bridge - Attach a bridge to the display pipe
- * @pipe: simple display pipe object
- * @bridge: bridge to attach
- *
- * Makes it possible to still use the drm_simple_display_pipe helpers when
- * a DRM bridge has to be used.
- *
- * Note that you probably want to initialize the pipe by passing a NULL
- * connector to drm_simple_display_pipe_init().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe,
struct drm_bridge *bridge)
{
@@ -391,30 +332,6 @@ int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe,
}
EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge);
-/**
- * drm_simple_display_pipe_init - Initialize a simple display pipeline
- * @dev: DRM device
- * @pipe: simple display pipe object to initialize
- * @funcs: callbacks for the display pipe (optional)
- * @formats: array of supported formats (DRM_FORMAT\_\*)
- * @format_count: number of elements in @formats
- * @format_modifiers: array of formats modifiers
- * @connector: connector to attach and register (optional)
- *
- * Sets up a display pipeline which consist of a really simple
- * plane-crtc-encoder pipe.
- *
- * If a connector is supplied, the pipe will be coupled with the provided
- * connector. You may supply a NULL connector when using drm bridges, that
- * handle connectors themselves (see drm_simple_display_pipe_attach_bridge()).
- *
- * Teardown of a simple display pipe is all handled automatically by the drm
- * core through calling drm_mode_config_cleanup(). Drivers afterwards need to
- * release the memory for the structure themselves.
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
int drm_simple_display_pipe_init(struct drm_device *dev,
struct drm_simple_display_pipe *pipe,
const struct drm_simple_display_pipe_funcs *funcs,
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index b2486d073763..cb672ce0e856 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -3,6 +3,11 @@
* Copyright (C) 2016 Noralf Trønnes
*/
+/*
+ * Simple KMS helpers are deprected in favor of regular atomic helpers. Do not
+ * use the min new code.
+ */
+
#ifndef __LINUX_DRM_SIMPLE_KMS_HELPER_H
#define __LINUX_DRM_SIMPLE_KMS_HELPER_H
@@ -12,233 +17,38 @@
struct drm_simple_display_pipe;
-/**
- * struct drm_simple_display_pipe_funcs - helper operations for a simple
- * display pipeline
- */
struct drm_simple_display_pipe_funcs {
- /**
- * @mode_valid:
- *
- * This callback is used to check if a specific mode is valid in the
- * crtc used in this simple display pipe. This should be implemented
- * if the display pipe has some sort of restriction in the modes
- * it can display. For example, a given display pipe may be responsible
- * to set a clock value. If the clock can not produce all the values
- * for the available modes then this callback can be used to restrict
- * the number of modes to only the ones that can be displayed. Another
- * reason can be bandwidth mitigation: the memory port on the display
- * controller can have bandwidth limitations not allowing pixel data
- * to be fetched at any rate.
- *
- * This hook is used by the probe helpers to filter the mode list in
- * drm_helper_probe_single_connector_modes(), and it is used by the
- * atomic helpers to validate modes supplied by userspace in
- * drm_atomic_helper_check_modeset().
- *
- * This function is optional.
- *
- * NOTE:
- *
- * Since this function is both called from the check phase of an atomic
- * commit, and the mode validation in the probe paths it is not allowed
- * to look at anything else but the passed-in mode, and validate it
- * against configuration-invariant hardware constraints.
- *
- * RETURNS:
- *
- * drm_mode_status Enum
- */
enum drm_mode_status (*mode_valid)(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode);
-
- /**
- * @enable:
- *
- * This function should be used to enable the pipeline.
- * It is called when the underlying crtc is enabled.
- * This hook is optional.
- */
void (*enable)(struct drm_simple_display_pipe *pipe,
struct drm_crtc_state *crtc_state,
struct drm_plane_state *plane_state);
- /**
- * @disable:
- *
- * This function should be used to disable the pipeline.
- * It is called when the underlying crtc is disabled.
- * This hook is optional.
- */
void (*disable)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @check:
- *
- * This function is called in the check phase of an atomic update,
- * specifically when the underlying plane is checked.
- * The simple display pipeline helpers already check that the plane is
- * not scaled, fills the entire visible area and is always enabled
- * when the crtc is also enabled.
- * This hook is optional.
- *
- * RETURNS:
- *
- * 0 on success, -EINVAL if the state or the transition can't be
- * supported, -ENOMEM on memory allocation failure and -EDEADLK if an
- * attempt to obtain another state object ran into a &drm_modeset_lock
- * deadlock.
- */
int (*check)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state,
struct drm_crtc_state *crtc_state);
- /**
- * @update:
- *
- * This function is called when the underlying plane state is updated.
- * This hook is optional.
- *
- * This is the function drivers should submit the
- * &drm_pending_vblank_event from. Using either
- * drm_crtc_arm_vblank_event(), when the driver supports vblank
- * interrupt handling, or drm_crtc_send_vblank_event() for more
- * complex case. In case the hardware lacks vblank support entirely,
- * drivers can set &struct drm_crtc_state.no_vblank in
- * &struct drm_simple_display_pipe_funcs.check and let DRM's
- * atomic helper fake a vblank event.
- */
void (*update)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *old_plane_state);
-
- /**
- * @prepare_fb:
- *
- * Optional, called by &drm_plane_helper_funcs.prepare_fb. Please read
- * the documentation for the &drm_plane_helper_funcs.prepare_fb hook for
- * more details.
- *
- * For GEM drivers who neither have a @prepare_fb nor @cleanup_fb hook
- * set, drm_gem_plane_helper_prepare_fb() is called automatically
- * to implement this. Other drivers which need additional plane
- * processing can call drm_gem_plane_helper_prepare_fb() from
- * their @prepare_fb hook.
- */
int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
-
- /**
- * @cleanup_fb:
- *
- * Optional, called by &drm_plane_helper_funcs.cleanup_fb. Please read
- * the documentation for the &drm_plane_helper_funcs.cleanup_fb hook for
- * more details.
- */
void (*cleanup_fb)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
-
- /**
- * @begin_fb_access:
- *
- * Optional, called by &drm_plane_helper_funcs.begin_fb_access. Please read
- * the documentation for the &drm_plane_helper_funcs.begin_fb_access hook for
- * more details.
- */
int (*begin_fb_access)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *new_plane_state);
-
- /**
- * @end_fb_access:
- *
- * Optional, called by &drm_plane_helper_funcs.end_fb_access. Please read
- * the documentation for the &drm_plane_helper_funcs.end_fb_access hook for
- * more details.
- */
void (*end_fb_access)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
-
- /**
- * @enable_vblank:
- *
- * Optional, called by &drm_crtc_funcs.enable_vblank. Please read
- * the documentation for the &drm_crtc_funcs.enable_vblank hook for
- * more details.
- */
int (*enable_vblank)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @disable_vblank:
- *
- * Optional, called by &drm_crtc_funcs.disable_vblank. Please read
- * the documentation for the &drm_crtc_funcs.disable_vblank hook for
- * more details.
- */
void (*disable_vblank)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @reset_crtc:
- *
- * Optional, called by &drm_crtc_funcs.reset. Please read the
- * documentation for the &drm_crtc_funcs.reset hook for more details.
- */
void (*reset_crtc)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @duplicate_crtc_state:
- *
- * Optional, called by &drm_crtc_funcs.atomic_duplicate_state. Please
- * read the documentation for the &drm_crtc_funcs.atomic_duplicate_state
- * hook for more details.
- */
struct drm_crtc_state * (*duplicate_crtc_state)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @destroy_crtc_state:
- *
- * Optional, called by &drm_crtc_funcs.atomic_destroy_state. Please
- * read the documentation for the &drm_crtc_funcs.atomic_destroy_state
- * hook for more details.
- */
void (*destroy_crtc_state)(struct drm_simple_display_pipe *pipe,
struct drm_crtc_state *crtc_state);
-
- /**
- * @reset_plane:
- *
- * Optional, called by &drm_plane_funcs.reset. Please read the
- * documentation for the &drm_plane_funcs.reset hook for more details.
- */
void (*reset_plane)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @duplicate_plane_state:
- *
- * Optional, called by &drm_plane_funcs.atomic_duplicate_state. Please
- * read the documentation for the &drm_plane_funcs.atomic_duplicate_state
- * hook for more details.
- */
struct drm_plane_state * (*duplicate_plane_state)(struct drm_simple_display_pipe *pipe);
-
- /**
- * @destroy_plane_state:
- *
- * Optional, called by &drm_plane_funcs.atomic_destroy_state. Please
- * read the documentation for the &drm_plane_funcs.atomic_destroy_state
- * hook for more details.
- */
void (*destroy_plane_state)(struct drm_simple_display_pipe *pipe,
struct drm_plane_state *plane_state);
};
-/**
- * struct drm_simple_display_pipe - simple display pipeline
- * @crtc: CRTC control structure
- * @plane: Plane control structure
- * @encoder: Encoder control structure
- * @connector: Connector control structure
- * @funcs: Pipeline control functions (optional)
- *
- * Simple display pipeline with plane, crtc and encoder collapsed into one
- * entity. It should be initialized by calling drm_simple_display_pipe_init().
- */
struct drm_simple_display_pipe {
struct drm_crtc crtc;
struct drm_plane plane;
@@ -265,22 +75,6 @@ int drm_simple_encoder_init(struct drm_device *dev,
void *__drmm_simple_encoder_alloc(struct drm_device *dev, size_t size,
size_t offset, int encoder_type);
-/**
- * drmm_simple_encoder_alloc - Allocate and initialize an encoder with basic
- * functionality.
- * @dev: drm device
- * @type: the type of the struct which contains struct &drm_encoder
- * @member: the name of the &drm_encoder within @type.
- * @encoder_type: user visible type of the encoder
- *
- * Allocates and initializes an encoder that has no further functionality.
- * Settings for possible CRTC and clones are left to their initial values.
- * Cleanup is automatically handled through registering drm_encoder_cleanup()
- * with drmm_add_action().
- *
- * Returns:
- * Pointer to new encoder, or ERR_PTR on failure.
- */
#define drmm_simple_encoder_alloc(dev, type, member, encoder_type) \
((type *)__drmm_simple_encoder_alloc(dev, sizeof(type), \
offsetof(type, member), \
--
2.53.0
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (15 preceding siblings ...)
2026-03-19 15:59 ` [PATCH v3 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
16 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 17
Reviewed: 2026-03-22T04:15:32.729051
---
This is a well-structured 16-patch series by Thomas Zimmermann that converts all mipi-dbi based DRM drivers from the `drm_simple_display_pipe` abstraction to regular atomic modesetting helpers. The motivation is sound: simple-display-pipe helpers have been a source of confusion for new driver authors and add an unnecessary intermediate layer. The series is v3 with review from David Lechner (Acked-by on all patches).
The overall approach is clean:
1. Fix the commit-tail ordering so plane updates happen after CRTC enable (patch 1)
2. Factor out device init from pipeline init (patch 2)
3. Provide macro-based initializers for atomic helpers (patch 3)
4. Convert each driver individually (patches 4-14)
5. Remove the now-unused simple-display code from mipi-dbi (patch 15)
6. Deprecate simple-kms helpers project-wide (patch 16)
The conversion pattern is consistent and mechanical across all drivers. There are a few minor issues noted below, but nothing that would block merging. The series has already been acked by the relevant reviewer.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/mipi-dbi: Only modify planes on enabled CRTCs
2026-03-19 15:59 ` [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Sound change. Switching to `drm_atomic_helper_commit_tail_rpm()` ensures CRTC enable happens before plane update, eliminating the redundant plane flush from each driver's enable callback.
The removal of `mipi_dbi_enable_flush()` and inlining of `backlight_enable()` into each driver is correct. Each driver properly replaces the combined flush+backlight call with just `backlight_enable()`.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init()
2026-03-19 15:59 ` [PATCH v3 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Clean extraction of device initialization from pipeline setup. The new `drm_mipi_dbi_dev_init()` properly handles tx_buf allocation, mode rotation, pixel format, and write_memory_bpw setup.
The change from `DRM_ERROR` / `DRM_DEBUG_KMS` to `drm_err` / `drm_dbg` is a nice modernization done as part of the refactoring.
Passing `tx_buf_size = 0` to auto-calculate the buffer size is a good API design that simplifies callers.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/mipi-dbi: Provide callbacks for atomic interfaces
2026-03-19 15:59 ` [PATCH v3 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This is the core infrastructure patch. The new helper functions and macros are well-designed.
Minor issues:
1. **Documentation typos** in `drm_mipi_dbi_connector_helper_get_modes()`:
- `"Sets the connecto rmodes"` → should be `"Sets the connector modes"`
- `"get_mods callback"` → should be `"get_modes callback"`
- The cross-reference to `drm_gem_destroy_shadow_plane_state()` seems irrelevant to a connector get_modes function — this looks like a copy-paste error.
2. **Redundant return path** in `drm_mipi_dbi_plane_helper_atomic_check()`:
```c
if (ret)
return ret;
else if (!new_plane_state->visible)
return 0;
return 0;
```
The `else if` branch and the final `return 0` are equivalent. This could simply be `return ret;` after the check call, but it's harmless and may be intentional to leave a hook point for future additions.
3. **Documentation typo** in `drm_mipi_dbi_crtc_helper_atomic_check()`:
`"ensures that the primary plane as been set up"` → should be `"has been set up"`.
4. The `drm_mipi_dbi_plane_helper_atomic_update()` function drops the `WARN_ON(!fb)` that existed in the original `mipi_dbi_pipe_update()`. The new code silently returns on `!fb`, which is fine since the atomic framework guarantees `fb` is set when the plane is visible, but worth noting.
5. The `mipi_dbi_pipe_disable()` wrapper now calls `drm_mipi_dbi_crtc_helper_atomic_disable(&pipe->crtc, pipe->crtc.state->state)` — passing `pipe->crtc.state->state` is correct here since the simple-pipe disable callback is called from within the atomic commit, so state is valid. However, the new `drm_mipi_dbi_crtc_helper_atomic_disable()` doesn't actually use the `state` parameter, so this is fine regardless.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/hx8357d: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Clean mechanical conversion. The driver correctly:
- Introduces `struct hx8357d_device` wrapping `mipi_dbi_dev`
- Uses the provided macros for funcs initialization
- Sets up the full pipeline (plane → crtc → encoder → connector)
- Enables damage clips on the plane
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/ili9163: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 05/16] drm/ili9163: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same mechanical pattern as patch 4. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/ili9225: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 06/16] drm/ili9225: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This driver has a custom `ili9225_fb_dirty()` and custom disable, so it needs its own plane update and CRTC disable helpers. The conversion correctly preserves the custom dirty function and custom power-down sequence.
Note that the custom `ili9225_plane_helper_atomic_update()` doesn't match the pattern of the generic `drm_mipi_dbi_plane_helper_atomic_update()` — it calls `drm_dev_enter/exit` outside the damage check conditional rather than nesting them. This preserves the original behavior and is fine.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/ili9341: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 07/16] drm/ili9341: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same mechanical pattern as patch 4. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/ili9486: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 08/16] drm/ili9486: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same mechanical pattern as patch 4. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/mi0283qt: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 09/16] drm/mi0283qt: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same mechanical pattern as patch 4. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/panel-mipi-dbi: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same pattern. This driver has a slightly more complex enable function with firmware command loading, which is correctly preserved. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/st7586: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 11/16] drm/st7586: " Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Like ili9225, this driver has a custom dirty function (`st7586_fb_dirty`) and custom enable/disable. The conversion correctly preserves all custom behavior.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/st7735r: Rename struct st7735r_priv to struct st7735r_device
2026-03-19 15:59 ` [PATCH v3 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Pure rename prep patch. Clean and straightforward. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/st7735r: Rename priv variable to st7735r
2026-03-19 15:59 ` [PATCH v3 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Pure rename prep patch. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/st7735r: Use regular atomic helpers; drop simple-display helpers
2026-03-19 15:59 ` [PATCH v3 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Same mechanical pattern. The st7735r driver uses `cfg` data for different panel variants which is correctly preserved through the conversion. No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/mipi-dbi: Remove simple-display helpers from mipi-dbi
2026-03-19 15:59 ` [PATCH v3 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Clean removal of all now-unused simple-display pipe infrastructure from mipi-dbi: `mipi_dbi_pipe_mode_valid`, `mipi_dbi_pipe_update`, `mipi_dbi_pipe_disable`, and all the simple-pipe shadow plane helpers. Also removes `struct drm_simple_display_pipe pipe` and `struct drm_connector connector` from `struct mipi_dbi_dev`, and the `#include <drm/drm_simple_kms_helper.h>`.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
* Claude review: drm/simple-kms: Deprecate simple-kms helpers
2026-03-19 15:59 ` [PATCH v3 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
@ 2026-03-21 18:15 ` Claude Code Review Bot
0 siblings, 0 replies; 35+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:15 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Removes documentation for simple-kms helpers and adds TODO items for converting remaining users.
Issues:
1. **Two typos in the deprecation comment** added to `drm_simple_kms_helper.h`:
```c
* Simple KMS helpers are deprected in favor of regular atomic helpers. Do not
* use the min new code.
```
- `"deprected"` → `"deprecated"`
- `"use the min new code"` → `"use them in new code"` (or similar — `"the min"` doesn't make sense)
2. **Typo in TODO** at line 5222: `"inlined fucntions"` → `"inlined functions"`.
3. The approach of stripping all kernel-doc from `drm_simple_kms_helper.h` and `drm_simple_kms_helper.c` while keeping the code functional is reasonable for deprecation, though it's a bit aggressive — typically deprecated APIs retain their docs with a deprecation notice. However, since the goal is to strongly discourage use, this is defensible.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2026-03-21 18:15 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-19 15:59 [PATCH v3 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
2026-03-19 15:59 ` [PATCH v3 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 05/16] drm/ili9163: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 06/16] drm/ili9225: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 07/16] drm/ili9341: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 08/16] drm/ili9486: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 09/16] drm/mi0283qt: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 11/16] drm/st7586: " Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-19 15:59 ` [PATCH v3 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
2026-03-21 18:15 ` Claude review: " Claude Code Review Bot
2026-03-21 18:15 ` Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-11 10:10 [PATCH v2 00/16] " Thomas Zimmermann
2026-03-11 21:03 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox