* [PATCH v2 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 21:03 ` Claude review: " Claude Code Review Bot
2026-03-11 10:10 ` [PATCH v2 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
` (15 subsequent siblings)
16 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- ili9225: remove unused variables (David)
- st7586: remove unused variables (David)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_mipi_dbi.c | 44 ++++-----------------------
drivers/gpu/drm/sitronix/st7586.c | 10 ------
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(+), 69 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..4aa6399a7856 100644
--- a/drivers/gpu/drm/sitronix/st7586.c
+++ b/drivers/gpu/drm/sitronix/st7586.c
@@ -174,15 +174,8 @@ 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 +235,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] 21+ messages in thread* Claude review: drm/mipi-dbi: Only modify planes on enabled CRTCs
2026-03-11 10:10 ` [PATCH v2 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
@ 2026-03-11 21:03 ` Claude Code Review Bot
0 siblings, 0 replies; 21+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 21:03 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Switches from `drm_atomic_helper_commit_tail()` to `drm_atomic_helper_commit_tail_rpm()`, which enables the CRTC before updating planes. This allows removing the redundant framebuffer flush from `mipi_dbi_enable_flush()` (and custom equivalents in ili9225/st7586). The remaining backlight enable is inlined into each driver.
The change is well-motivated and correct. The new mode config helper funcs:
```c
static const struct drm_mode_config_helper_funcs mipi_dbi_mode_config_helper_funcs = {
.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
};
```
No issues found. The ili9225 driver correctly drops its `ili9225_fb_dirty()` call without adding `backlight_enable()` since it doesn't use a backlight. The st7586 custom dirty function is still correctly invoked through the plane update path.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init()
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 21:03 ` Claude review: " Claude Code Review Bot
2026-03-11 10:10 ` [PATCH v2 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
` (14 subsequent siblings)
16 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_mipi_dbi.c | 76 ++++++++++++++++++++++++----------
include/drm/drm_mipi_dbi.h | 4 ++
2 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index bb6cebc583be..86f38d59c6e9 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -554,6 +554,55 @@ 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 this size.
+ *
+ * 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 +639,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 +667,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 +692,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] 21+ messages in thread* Claude review: drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init()
2026-03-11 10:10 ` [PATCH v2 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
@ 2026-03-11 21:03 ` Claude Code Review Bot
0 siblings, 0 replies; 21+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 21:03 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Extracts device initialization (tx_buf allocation, mode/rotation/format setup) from `mipi_dbi_dev_init_with_formats()` into a new `drm_mipi_dbi_dev_init()` that does not create any KMS objects. This allows drivers to use the common init and then set up their own pipeline.
The naming (`drm_mipi_dbi_dev_init` vs existing `mipi_dbi_dev_init`) is potentially confusing but the commit message explains the `drm_` prefix convention. The auto-calculation of `tx_buf_size` when zero is correct.
No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 01/16] drm/mipi-dbi: Only modify planes on enabled CRTCs Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 02/16] drm/mipi-dbi: Support custom pipelines with drm_mipi_dbi_dev_init() Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 21:03 ` Claude review: " Claude Code Review Bot
2026-03-11 10:10 ` [PATCH v2 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
` (13 subsequent siblings)
16 siblings, 1 reply; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
As the new helpers are DRM functions, add the drm_ prefix. Mipi-dbi
interfaces currently lack this.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_mipi_dbi.c | 167 +++++++++++++++++++++++----------
include/drm/drm_mipi_dbi.h | 86 +++++++++++++++++
2 files changed, 206 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 86f38d59c6e9..6846cb73ea87 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,75 @@ 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);
+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;
+ 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);
+
/**
- * mipi_dbi_pipe_update - Display pipe update helper
- * @pipe: Simple display pipe
- * @old_state: Old plane state
+ * 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)
- return;
-
- if (WARN_ON(!fb))
- return;
-
- if (!drm_dev_enter(fb->dev, &idx))
+ if (!fb)
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);
@@ -393,19 +441,36 @@ static void mipi_dbi_blank(struct mipi_dbi_dev *dbidev)
drm_dev_exit(idx);
}
+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);
+
/**
- * mipi_dbi_pipe_disable - MIPI DBI pipe disable helper
- * @pipe: Display pipe
+ * 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 +482,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 +582,21 @@ 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)
+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 +617,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,
};
/**
@@ -633,8 +707,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] 21+ messages in thread* Claude review: drm/mipi-dbi: Provide callbacks for atomic interfaces
2026-03-11 10:10 ` [PATCH v2 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
@ 2026-03-11 21:03 ` Claude Code Review Bot
0 siblings, 0 replies; 21+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 21:03 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Provides new standalone atomic helper functions (`drm_mipi_dbi_*` prefix) and initializer macros (`DRM_MIPI_DBI_PLANE_FUNCS`, `DRM_MIPI_DBI_CRTC_FUNCS`, etc.) for drivers to use with regular atomic modesetting.
**Bug — uninitialized variable:** In `drm_mipi_dbi_plane_helper_atomic_check()` (mbox line ~1106):
```c
struct drm_crtc_state *new_crtc_state;
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, ...);
```
When `new_plane_state->crtc` is NULL (plane being disabled), `new_crtc_state` is **used uninitialized**. While `drm_atomic_helper_check_plane_state()` does handle `crtc_state == NULL`, it will receive stack garbage rather than NULL. This must be:
```c
struct drm_crtc_state *new_crtc_state = NULL;
```
**Minor:** The tail of the same function is redundant:
```c
if (ret)
return ret;
else if (!new_plane_state->visible)
return 0;
return 0;
```
This could simply be `return ret;`. The `else if` branch suggests future checks were intended but not yet added.
**Minor:** Moving `drm_gem_atomic_helper.h`, `drm_gem_framebuffer_helper.h`, `drm_probe_helper.h`, and `drm_atomic_state_helper.h` into `drm_mipi_dbi.h` (to support the initializer macros) adds transitive header weight to all includers. This is a common trade-off for macro-based initializers.
### PATCH 4 (04/16): drm/hx8357d: Use regular atomic helpers
Standard mechanical conversion. Introduces `struct hx8357d_device` wrapping `mipi_dbi_dev` with embedded plane/crtc/encoder/connector. Converts `yx240qv29_enable` to `hx8357d_crtc_helper_atomic_enable`. Probe open-codes the pipeline setup using the new `DRM_MIPI_DBI_*` macros.
No issues found.
### PATCH 5 (05/16): drm/ili9163: Use regular atomic helpers
Identical structural conversion as hx8357d.
No issues found.
### PATCH 6 (06/16): drm/ili9225: Use regular atomic helpers
More complex conversion since ili9225 has custom update and disable callbacks. Uses `DRM_GEM_SHADOW_PLANE_HELPER_FUNCS` instead of `DRM_MIPI_DBI_PLANE_HELPER_FUNCS` since it provides its own `atomic_update`. The old `!pipe->crtc.state->active` guard is correctly replaced with `!plane_state->fb` (standard pattern for regular atomic helpers where the framework manages CRTC-active gating).
No issues found.
### PATCH 7 (07/16): drm/ili9341: Use regular atomic helpers
Standard mechanical conversion, same pattern as hx8357d/ili9163.
No issues found.
### PATCH 8 (08/16): drm/ili9486: Use regular atomic helpers
Standard mechanical conversion. Minor improvement: renames old `waveshare_enable` to `ili9486_crtc_helper_atomic_enable` for consistency.
No issues found.
### PATCH 9 (09/16): drm/mi0283qt: Use regular atomic helpers
Standard mechanical conversion.
No issues found.
### PATCH 10 (10/16): drm/panel-mipi-dbi: Use regular atomic helpers
Slightly different from others because the pixel format is determined dynamically at probe time. Correctly passes the runtime `formats[]` array to `drm_universal_plane_init()` while using `formats[0]` (native format) for `drm_mipi_dbi_dev_init()` buffer sizing.
No issues found.
### PATCH 11 (11/16): drm/st7586: Use regular atomic helpers
Complex conversion like ili9225 — custom update, enable, and disable. Uses `DRM_FORMAT_XRGB8888` only (this driver does internal 2bpp grayscale conversion). Sets `preferred_depth = 24` to match XRGB8888.
**Minor style inconsistency:** `st7586_plane_helper_atomic_update` uses `if (drm_dev_enter(...)) { ... }` while ili9225's equivalent uses the early-return pattern. Both are correct but inconsistent.
### PATCH 12 (12/16): drm/st7735r: Rename struct st7735r_priv to struct st7735r_device
Purely cosmetic rename to follow DRM naming conventions. Adds `to_st7735r_device()` upcast helper.
No issues found.
### PATCH 13 (13/16): drm/st7735r: Rename priv variable to st7735r
Purely cosmetic rename of local variable `priv` → `st7735r`.
No issues found.
### PATCH 14 (14/16): drm/st7735r: Use regular atomic helpers
Standard mechanical conversion for st7735r. Uses the `DRM_MIPI_DBI_*` macros. Only custom callback is `st7735r_crtc_helper_atomic_enable`.
No issues found.
### PATCH 15 (15/16): drm/mipi-dbi: Remove simple-display helpers from mipi-dbi
Large deletion patch removing all now-unused simple-display-pipe wrapper functions (`mipi_dbi_pipe_update`, `mipi_dbi_pipe_disable`, `mipi_dbi_enable_flush`, `mipi_dbi_dev_init_with_formats`, `mipi_dbi_dev_init`, etc.), the `DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS` macro, and the `pipe`/`connector` fields from `struct mipi_dbi_dev`.
No issues found. Clean removal of dead code.
### PATCH 16 (16/16): drm/simple-kms: Deprecate simple-kms helpers
Strips all kerneldoc from `drm_simple_kms_helper.c`/`.h`, removes the "Simple KMS Helper Reference" from the GPU documentation, and adds TODO items for converting remaining users.
**Three typos:**
- Commit message (line 5016): `"voluteers"` → `"volunteers"`
- TODO item (line 5105): `"fucntions"` → `"functions"`
- Header deprecation comment (lines 5306-5307):
```
Simple KMS helpers are deprected in favor of regular atomic helpers. Do not
use the min new code.
```
Should be: `"deprecated"` and `"Do not use them in new code."`
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (2 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 03/16] drm/mipi-dbi: Provide callbacks for atomic interfaces Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 05/16] drm/ili9163: " Thomas Zimmermann
` (12 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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] 21+ messages in thread* [PATCH v2 05/16] drm/ili9163: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (3 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 04/16] drm/hx8357d: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 06/16] drm/ili9225: " Thomas Zimmermann
` (11 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/ili9163.c | 136 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index c616f56af5b5..01fa7e78ac15 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,19 +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);
-
- dbi = &dbidev->dbi;
+ ili9163 = devm_drm_dev_alloc(dev, &ili9163_driver, struct ili9163_device, dbidev.drm);
+ if (IS_ERR(ili9163))
+ return PTR_ERR(ili9163);
+ dbidev = &ili9163->dbidev;
drm = &dbidev->drm;
spi_set_drvdata(spi, drm);
@@ -178,7 +240,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] 21+ messages in thread* [PATCH v2 06/16] drm/ili9225: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (4 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 05/16] drm/ili9163: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 07/16] drm/ili9341: " Thomas Zimmermann
` (10 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/ili9225.c | 182 ++++++++++++++++++++++++++-------
1 file changed, 145 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 3eaf6b3a055a..bbf2d09330be 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,19 +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);
-
- dbi = &dbidev->dbi;
+ ili9225 = devm_drm_dev_alloc(dev, &ili9225_driver, struct ili9225_device, dbidev.drm);
+ if (IS_ERR(ili9225))
+ return PTR_ERR(ili9225);
+ dbidev = &ili9225->dbidev;
drm = &dbidev->drm;
dbi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
@@ -405,7 +467,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] 21+ messages in thread* [PATCH v2 07/16] drm/ili9341: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (5 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 06/16] drm/ili9225: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 08/16] drm/ili9486: " Thomas Zimmermann
` (9 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/ili9341.c | 136 +++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 972811564d6a..fc1b4c8f7912 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,19 +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);
-
- dbi = &dbidev->dbi;
+ ili9341 = devm_drm_dev_alloc(dev, &ili9341_driver, struct ili9341_device, dbidev.drm);
+ if (IS_ERR(ili9341))
+ return PTR_ERR(ili9341);
+ dbidev = &ili9341->dbidev;
drm = &dbidev->drm;
dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
@@ -209,7 +271,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] 21+ messages in thread* [PATCH v2 08/16] drm/ili9486: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (6 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 07/16] drm/ili9341: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 09/16] drm/mi0283qt: " Thomas Zimmermann
` (8 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/ili9486.c | 127 ++++++++++++++++++++++++++++++---
1 file changed, 118 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index 52b14f2cb0e1..582153fe35e7 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,10 +260,15 @@ 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;
@@ -237,8 +301,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] 21+ messages in thread* [PATCH v2 09/16] drm/mi0283qt: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (7 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 08/16] drm/ili9486: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
` (7 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
- fix coding style
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/mi0283qt.c | 136 ++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index f121e1a8a303..6655fd6f74d4 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,19 +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);
-
- dbi = &dbidev->dbi;
+ mi0283qt = devm_drm_dev_alloc(dev, &mi0283qt_driver, struct mi0283qt_device, dbidev.drm);
+ if (IS_ERR(mi0283qt))
+ return PTR_ERR(mi0283qt);
+ dbidev = &mi0283qt->dbidev;
drm = &dbidev->drm;
dbi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
@@ -217,7 +279,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] 21+ messages in thread* [PATCH v2 10/16] drm/panel-mipi-dbi: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (8 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 09/16] drm/mi0283qt: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 11/16] drm/st7586: " Thomas Zimmermann
` (6 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
drivers/gpu/drm/tiny/panel-mipi-dbi.c | 140 +++++++++++++++++++++++---
1 file changed, 124 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..1d4289418216 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,22 +372,30 @@ 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;
+ drm = &dbidev->drm;
dbi = &dbidev->dbi;
- drm = &dbidev->drm;
ret = panel_mipi_dbi_get_mode(dbidev, &mode);
if (ret)
@@ -377,9 +440,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] 21+ messages in thread* [PATCH v2 11/16] drm/st7586: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (9 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 10/16] drm/panel-mipi-dbi: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
` (5 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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.
v2:
- fix connector initialization
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sitronix/st7586.c | 184 +++++++++++++++++++++++-------
1 file changed, 143 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7586.c b/drivers/gpu/drm/sitronix/st7586.c
index 4aa6399a7856..3006828a08ea 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,39 +162,57 @@ 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))
- return;
+ if (drm_dev_enter(plane->dev, &idx)) {
+ 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);
+ }
+}
- if (drm_atomic_helper_damage_merged(old_state, state, &rect))
- st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect,
- &shadow_plane_state->fmtcnv_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
+};
- drm_dev_exit(idx);
-}
+static const struct drm_plane_funcs st7586_plane_funcs = {
+ DRM_MIPI_DBI_PLANE_FUNCS,
+ .destroy = drm_plane_cleanup,
+};
-static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
- struct drm_crtc_state *crtc_state,
- struct drm_plane_state *plane_state)
+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_framebuffer *fb = plane_state->fb;
+ 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");
@@ -240,9 +273,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
@@ -256,20 +292,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 = {
@@ -305,21 +358,26 @@ 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;
+ drm = &dbidev->drm;
dbi = &dbidev->dbi;
- drm = &dbidev->drm;
bufsize = (st7586_mode.vdisplay + 2) / 3 * st7586_mode.hdisplay;
@@ -346,9 +404,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] 21+ messages in thread* [PATCH v2 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (10 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 11/16] drm/st7586: " Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
` (4 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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] 21+ messages in thread* [PATCH v2 13/16] drm/st7735r: Rename priv variable to st7735r
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (11 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 12/16] drm/st7735r: Rename struct st7735r_priv to struct st7735r_device Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
` (3 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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] 21+ messages in thread* [PATCH v2 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (12 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 13/16] drm/st7735r: Rename priv variable to st7735r Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
` (2 subsequent siblings)
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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] 21+ messages in thread* [PATCH v2 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (13 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 14/16] drm/st7735r: Use regular atomic helpers; drop simple-display helpers Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 10:10 ` [PATCH v2 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
2026-03-11 21:03 ` Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Claude Code Review Bot
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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 6846cb73ea87..09c3208a81ea 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);
-
int drm_mipi_dbi_plane_helper_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state)
{
@@ -401,21 +384,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;
@@ -484,104 +452,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);
-
int drm_mipi_dbi_connector_helper_get_modes(struct drm_connector *connector)
{
struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
@@ -590,15 +460,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)
{
@@ -616,18 +477,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
@@ -677,102 +526,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] 21+ messages in thread* [PATCH v2 16/16] drm/simple-kms: Deprecate simple-kms helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (14 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 15/16] drm/mipi-dbi: Remove simple-display helpers from mipi-dbi Thomas Zimmermann
@ 2026-03-11 10:10 ` Thomas Zimmermann
2026-03-11 21:03 ` Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Claude Code Review Bot
16 siblings, 0 replies; 21+ messages in thread
From: Thomas Zimmermann @ 2026-03-11 10:10 UTC (permalink / raw)
To: javierm, lanzano.alex, kamlesh.gurudasani, david, 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>
---
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] 21+ messages in thread* Claude review: drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers
2026-03-11 10:10 [PATCH v2 00/16] drm/mipi-dbi: Replace simple-display helpers with regular atomic helpers Thomas Zimmermann
` (15 preceding siblings ...)
2026-03-11 10:10 ` [PATCH v2 16/16] drm/simple-kms: Deprecate simple-kms helpers Thomas Zimmermann
@ 2026-03-11 21:03 ` Claude Code Review Bot
16 siblings, 0 replies; 21+ 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] 21+ messages in thread