[PATCH 0/6] drm: Open-code drm_simple_encoder_init() in several drivers

[PATCH 0/6] drm: Open-code drm_simple_encoder_init() in several drivers

[Naman Arora]

drm_simple_encoder_init() is a thin wrapper around drm_encoder_init()
that provides a simple destroy-only encoder funcs struct. This series
removes the dependency on drm_simple_kms_helper in six drivers by
open-coding the encoder initialization directly.

Each patch adds a static drm_encoder_funcs struct with a destroy
callback and replaces drm_simple_encoder_init() with drm_encoder_init().
The drm_simple_kms_helper.h include is removed where it is no longer
needed.

Drivers converted in this series:
- fsl-dcu
- tidss
- virtio
- meson (encoder_cvbs, encoder_hdmi, encoder_dsi)

Naman Arora (6):
  drm/fsl-dcu: Open-code drm_simple_encoder_init()
  drm/tidss: Open-code drm_simple_encoder_init()
  drm/virtio: Open-code drm_simple_encoder_init()
  drm/meson: Open-code drm_simple_encoder_init() in encoder_cvbs
  drm/meson: Open-code drm_simple_encoder_init() in encoder_hdmi
  drm/meson: Open-code drm_simple_encoder_init() in encoder_dsi

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c  | 10 +++++++---
 drivers/gpu/drm/meson/meson_encoder_cvbs.c | 10 +++++++---
 drivers/gpu/drm/meson/meson_encoder_dsi.c  | 10 +++++++---
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 10 +++++++---
 drivers/gpu/drm/tidss/tidss_encoder.c      | 10 +++++++---
 drivers/gpu/drm/virtio/virtgpu_display.c   |  8 ++++++--
 6 files changed, 41 insertions(+), 17 deletions(-)

-- 
2.20.1

[PATCH 1/6] drm/fsl-dcu: Open-code drm_simple_encoder_init()

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
index 84eff7519..a16c6013e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c
@@ -14,11 +14,14 @@
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include "fsl_dcu_drm_drv.h"
 #include "fsl_tcon.h"
 
+static const struct drm_encoder_funcs fsl_dcu_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev,
 			       struct drm_crtc *crtc)
 {
@@ -31,8 +34,9 @@ int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev,
 	if (fsl_dev->tcon)
 		fsl_tcon_bypass_enable(fsl_dev->tcon);
 
-	ret = drm_simple_encoder_init(fsl_dev->drm, encoder,
-				      DRM_MODE_ENCODER_LVDS);
+	ret = drm_encoder_init(fsl_dev->drm, encoder,
+			       &fsl_dcu_drm_encoder_funcs,
+			       DRM_MODE_ENCODER_LVDS, NULL);
 	if (ret < 0)
 		return ret;
 
-- 
2.20.1

Claude review: drm/fsl-dcu: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Patch Review

**Correctness: OK**

The transformation is correct. The new struct:
```c
static const struct drm_encoder_funcs fsl_dcu_drm_encoder_funcs = {
	.destroy = drm_encoder_cleanup,
};
```
is placed at file scope before its first use in `fsl_dcu_drm_encoder_create()`, and the call is correctly changed from:
```c
drm_simple_encoder_init(fsl_dev->drm, encoder, DRM_MODE_ENCODER_LVDS);
```
to:
```c
drm_encoder_init(fsl_dev->drm, encoder, &fsl_dcu_drm_encoder_funcs,
                 DRM_MODE_ENCODER_LVDS, NULL);
```

The `NULL` name argument matches what `drm_simple_encoder_init()` passed. The include removal is safe since no other symbols from `drm_simple_kms_helper.h` are used.

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

[PATCH 2/6] drm/tidss: Open-code drm_simple_encoder_init()

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/tidss/tidss_encoder.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c b/drivers/gpu/drm/tidss/tidss_encoder.c
index 81a04f767..4d73a271c 100644
--- a/drivers/gpu/drm/tidss/tidss_encoder.c
+++ b/drivers/gpu/drm/tidss/tidss_encoder.c
@@ -13,7 +13,6 @@
 #include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_of.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include "tidss_crtc.h"
 #include "tidss_drv.h"
@@ -81,6 +80,10 @@ static const struct drm_bridge_funcs tidss_bridge_funcs = {
 	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
 };
 
+static const struct drm_encoder_funcs tidss_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 int tidss_encoder_create(struct tidss_device *tidss,
 			 struct drm_bridge *next_bridge,
 			 u32 encoder_type, u32 possible_crtcs)
@@ -95,8 +98,9 @@ int tidss_encoder_create(struct tidss_device *tidss,
 	if (IS_ERR(t_enc))
 		return PTR_ERR(t_enc);
 
-	ret = drm_simple_encoder_init(&tidss->ddev, &t_enc->encoder,
-				      encoder_type);
+	ret = drm_encoder_init(&tidss->ddev, &t_enc->encoder,
+			       &tidss_drm_encoder_funcs,
+			       encoder_type, NULL);
 	if (ret)
 		return ret;
 
-- 
2.20.1

Claude review: drm/tidss: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Patch Review

**Correctness: OK**

Same mechanical transformation. The new struct `tidss_drm_encoder_funcs` is placed after `tidss_bridge_funcs` and before `tidss_encoder_create()`, which is the correct placement. The `encoder_type` parameter is correctly forwarded from the function argument.

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

[PATCH 3/6] drm/virtio: Open-code drm_simple_encoder_init()

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/virtio/virtgpu_display.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index f1dae9569..5b99cce17 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -32,7 +32,6 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 #include <drm/drm_vblank.h>
 #include <drm/drm_vblank_helper.h>
 
@@ -271,6 +270,10 @@ static const struct drm_connector_funcs virtio_gpu_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
+static const struct drm_encoder_funcs virtio_gpu_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index)
 {
 	struct drm_device *dev = vgdev->ddev;
@@ -306,7 +309,8 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index)
 	if (vgdev->has_edid)
 		drm_connector_attach_edid_property(connector);
 
-	drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_VIRTUAL);
+	drm_encoder_init(dev, encoder, &virtio_gpu_drm_encoder_funcs,
+			 DRM_MODE_ENCODER_VIRTUAL, NULL);
 	drm_encoder_helper_add(encoder, &virtio_gpu_enc_helper_funcs);
 	encoder->possible_crtcs = 1 << index;
 
-- 
2.20.1

Claude review: drm/virtio: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Patch Review

**Correctness: OK, with a pre-existing nit**

The transformation itself is correct. However, note that both the original and patched code ignore the return value:
```c
drm_encoder_init(dev, encoder, &virtio_gpu_drm_encoder_funcs,
                 DRM_MODE_ENCODER_VIRTUAL, NULL);
```
`drm_encoder_init()` returns `int`, and `vgdev_output_init()` checks errors from other init calls. The original `drm_simple_encoder_init()` also returned `int` and was similarly ignored. This is a **pre-existing issue**, not introduced by this patch, but could be mentioned as a follow-up cleanup opportunity.

Otherwise reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

[PATCH 4/6] drm/meson: Open-code drm_simple_encoder_init() in encoder_cvbs

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/meson/meson_encoder_cvbs.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 41071d6e0..496100ba2 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -18,7 +18,6 @@
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include "meson_registers.h"
 #include "meson_vclk.h"
@@ -218,6 +217,10 @@ static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
+static const struct drm_encoder_funcs meson_encoder_cvbs_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 int meson_encoder_cvbs_probe(struct meson_drm *priv)
 {
 	struct drm_device *drm = priv->drm;
@@ -257,8 +260,9 @@ int meson_encoder_cvbs_probe(struct meson_drm *priv)
 	meson_encoder_cvbs->priv = priv;
 
 	/* Encoder */
-	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
-				      DRM_MODE_ENCODER_TVDAC);
+	ret = drm_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
+			       &meson_encoder_cvbs_drm_encoder_funcs,
+			       DRM_MODE_ENCODER_TVDAC, NULL);
 	if (ret)
 		return dev_err_probe(priv->dev, ret,
 				     "Failed to init CVBS encoder\n");
-- 
2.20.1

[PATCH 5/6] drm/meson: Open-code drm_simple_encoder_init() in encoder_hdmi

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 1abb0572b..0a0ec34e3 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -24,7 +24,6 @@
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include <linux/media-bus-format.h>
 #include <linux/videodev2.h>
@@ -369,6 +368,10 @@ static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
+static const struct drm_encoder_funcs meson_encoder_hdmi_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 int meson_encoder_hdmi_probe(struct meson_drm *priv)
 {
 	struct meson_encoder_hdmi *meson_encoder_hdmi;
@@ -407,8 +410,9 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
 	meson_encoder_hdmi->priv = priv;
 
 	/* Encoder */
-	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
-				      DRM_MODE_ENCODER_TMDS);
+	ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
+			       &meson_encoder_hdmi_drm_encoder_funcs,
+			       DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret) {
 		dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
 		goto err_put_node;
-- 
2.20.1

Claude review: drm/meson: Open-code drm_simple_encoder_init() in encoder_hdmi

[Claude Code Review Bot]

Patch Review

**Correctness: OK**

Same pattern. `meson_encoder_hdmi_drm_encoder_funcs` is correctly defined. Encoder type `DRM_MODE_ENCODER_TMDS` is preserved. Error handling via `dev_err_probe` + `goto err_put_node` is maintained.

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

[PATCH 6/6] drm/meson: Open-code drm_simple_encoder_init() in encoder_dsi

[Naman Arora]

The helper drm_simple_encoder_init() is a thin wrapper around
drm_encoder_init() with a simple destroy-only encoder funcs struct.
Remove the dependency on drm_simple_kms_helper by open-coding the
encoder initialization directly in the driver.

Signed-off-by: Naman Arora <namanarora029@gmail.com>
---
 drivers/gpu/drm/meson/meson_encoder_dsi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_dsi.c b/drivers/gpu/drm/meson/meson_encoder_dsi.c
index eba246791..e14da1f71 100644
--- a/drivers/gpu/drm/meson/meson_encoder_dsi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_dsi.c
@@ -10,7 +10,6 @@
 #include <linux/of_graph.h>
 
 #include <drm/drm_atomic_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_bridge_connector.h>
 #include <drm/drm_device.h>
@@ -99,6 +98,10 @@ static const struct drm_bridge_funcs meson_encoder_dsi_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 };
 
+static const struct drm_encoder_funcs meson_encoder_dsi_drm_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
 int meson_encoder_dsi_probe(struct meson_drm *priv)
 {
 	struct meson_encoder_dsi *meson_encoder_dsi;
@@ -133,8 +136,9 @@ int meson_encoder_dsi_probe(struct meson_drm *priv)
 	meson_encoder_dsi->priv = priv;
 
 	/* Encoder */
-	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_dsi->encoder,
-				      DRM_MODE_ENCODER_DSI);
+	ret = drm_encoder_init(priv->drm, &meson_encoder_dsi->encoder,
+			       &meson_encoder_dsi_drm_encoder_funcs,
+			       DRM_MODE_ENCODER_DSI, NULL);
 	if (ret)
 		return dev_err_probe(priv->dev, ret,
 				     "Failed to init DSI encoder\n");
-- 
2.20.1

Claude review: drm/meson: Open-code drm_simple_encoder_init() in encoder_dsi

[Claude Code Review Bot]

Patch Review

**Correctness: OK**

Same pattern. `meson_encoder_dsi_drm_encoder_funcs` is correctly defined. Encoder type `DRM_MODE_ENCODER_DSI` is preserved.

**Minor nit on include ordering:** After removing `drm_simple_kms_helper.h`, the remaining includes have `drm_atomic_helper.h` followed by `drm_bridge.h`. The original had `drm_simple_kms_helper.h` between `drm_atomic_helper.h` and `drm_bridge.h`. The resulting order is still alphabetical, so this is fine.

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/meson: Open-code drm_simple_encoder_init() in encoder_cvbs

[Claude Code Review Bot]

Patch Review

**Correctness: OK**

Same pattern. `meson_encoder_cvbs_drm_encoder_funcs` is correctly defined and used. The encoder type `DRM_MODE_ENCODER_TVDAC` is correctly preserved. Error handling path is maintained.

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer

Claude review: drm: Open-code drm_simple_encoder_init() in several drivers

[Claude Code Review Bot]

Overall Series Review

Subject: drm: Open-code drm_simple_encoder_init() in several drivers
Author: Naman Arora <namanarora029@gmail.com>
Patches: 7
Reviewed: 2026-06-04T15:05:00.305353

---

This is a straightforward, well-structured cleanup series that open-codes `drm_simple_encoder_init()` across six call sites in four drivers. The stated goal is to remove the dependency on `drm_simple_kms_helper` from drivers that only use it for this single trivial wrapper.

Each patch is mechanically identical: define a static `const struct drm_encoder_funcs` with `.destroy = drm_encoder_cleanup`, replace the `drm_simple_encoder_init()` call with `drm_encoder_init()`, and remove the `drm_simple_kms_helper.h` include.

I verified the original implementation in `drm_simple_kms_helper.c:18-29` — it is exactly a wrapper around `drm_encoder_init()` with a `{ .destroy = drm_encoder_cleanup }` funcs struct. All six patches replicate this correctly.

**Concerns:**

1. **Missing motivation for the larger goal.** The cover letter explains *what* but not *why* removing `drm_simple_kms_helper` dependency matters. Is this a precursor to removing `drm_simple_encoder_init()` entirely? Or changing the helper module? The series would benefit from stating the end goal. The `Documentation/gpu/todo.rst` file in the tree lists this cleanup, so it's a known TODO item, but the cover letter should reference that.

2. **Code duplication vs. consolidation.** This series creates six identical `{ .destroy = drm_encoder_cleanup }` structs across six files. An alternative approach would be to move `drm_simple_encoder_init()` into a more lightweight header/module, or just keep using it. However, the todo.rst likely endorses this approach, and the resulting code is trivial enough that duplication is acceptable.

3. **Incomplete series** — these are only 6 of the ~40+ callers of `drm_simple_encoder_init()` in the tree. That's fine as incremental progress, but the cover letter could mention this.

4. **Meson patches 4-6 sent as a sub-thread** — patches 4/6, 5/6, and 6/6 have a different Message-Id series (`20260531074627.8936-*`) vs the main series (`20260531073532.8609-*`). This is a minor submission issue where they were sent as a separate `git send-email` run with different In-Reply-To threading. Not a code issue but can confuse tooling.

Overall: **Low risk, correct transformation.** Series is suitable for merging with minor nits.

---
Generated by Claude Code Patch Reviewer