[PATCH] drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[PATCH] drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[Souradipto Das]

The helper drm_simple_encoder_init() is a trivial wrapper around
drm_encoder_init() that only provides a static drm_encoder_funcs with
.destroy set to drm_encoder_cleanup(). Open-code the initialization
with a driver-specific instance of drm_encoder_funcs and remove the
dependency on drm_simple_kms_helper.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Souradipto Das <souradiptodas6@gmail.com>
---
 drivers/gpu/drm/tegra/hdmi.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0adcd4244a42..a45c19d1631b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -25,12 +25,13 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_debugfs.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
 #include <drm/drm_eld.h>
 #include <drm/drm_file.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
+
 
 #include "hda.h"
 #include "hdmi.h"
@@ -371,6 +372,9 @@ static const struct tmds_config tegra124_tmds_config[] = {
 			PEAK_CURRENT_LANE3(PEAK_CURRENT_0_800_mA),
 	},
 };
+static const struct drm_encoder_funcs tegra_hdmi_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
 
 static void tegra_hdmi_audio_lock(struct tegra_hdmi *hdmi)
 {
@@ -1555,8 +1559,8 @@ static int tegra_hdmi_init(struct host1x_client *client)
 
 	hdmi->output.dev = client->dev;
 
-	drm_simple_encoder_init(drm, &hdmi->output.encoder,
-				DRM_MODE_ENCODER_TMDS);
+	drm_encoder_init(drm, &hdmi->output.encoder, &tegra_hdmi_encoder_funcs,
+				DRM_MODE_ENCODER_TMDS, NULL);
 	drm_encoder_helper_add(&hdmi->output.encoder,
 			       &tegra_hdmi_encoder_helper_funcs);
 
-- 
2.43.0

[PATCH v2] drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[Souradipto Das]

The helper drm_simple_encoder_init() is a trivial wrapper around
drm_encoder_init() that only provides a static drm_encoder_funcs with
.destroy set to drm_encoder_cleanup(). Open-code the initialization
with a driver-specific instance of drm_encoder_funcs and remove the
dependency on drm_simple_kms_helper.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Souradipto Das <souradiptodas6@gmail.com>
---
v2:
  - Remove stray blank line
  - Patch is compile-tested only 
  - No functional changes.

 drivers/gpu/drm/tegra/hdmi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0adcd4244a42..068a32be0c13 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -25,12 +25,12 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_debugfs.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
 #include <drm/drm_eld.h>
 #include <drm/drm_file.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
 
 #include "hda.h"
 #include "hdmi.h"
@@ -371,6 +371,9 @@ static const struct tmds_config tegra124_tmds_config[] = {
 			PEAK_CURRENT_LANE3(PEAK_CURRENT_0_800_mA),
 	},
 };
+static const struct drm_encoder_funcs tegra_hdmi_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
 
 static void tegra_hdmi_audio_lock(struct tegra_hdmi *hdmi)
 {
@@ -1555,8 +1558,8 @@ static int tegra_hdmi_init(struct host1x_client *client)
 
 	hdmi->output.dev = client->dev;
 
-	drm_simple_encoder_init(drm, &hdmi->output.encoder,
-				DRM_MODE_ENCODER_TMDS);
+	drm_encoder_init(drm, &hdmi->output.encoder, &tegra_hdmi_encoder_funcs,
+				DRM_MODE_ENCODER_TMDS, NULL);
 	drm_encoder_helper_add(&hdmi->output.encoder,
 			       &tegra_hdmi_encoder_helper_funcs);
 
-- 
2.43.0

Claude review: drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Overall Series Review

Subject: drm/tegra: hdmi: Open-code drm_simple_encoder_init()
Author: Souradipto Das <souradiptodas6@gmail.com>
Patches: 2
Reviewed: 2026-05-16T12:11:37.822669

---

This mbox contains two versions of the same patch — v1 and v2 — by Souradipto Das, which open-codes `drm_simple_encoder_init()` in the tegra HDMI driver. The change is straightforward and mechanically correct: it replaces a call to a trivial helper with the underlying `drm_encoder_init()` call and a driver-local `drm_encoder_funcs` struct, removing the dependency on `drm_simple_kms_helper.h`. This is part of a broader cleanup effort suggested by Thomas Zimmermann.

The v2 patch fixes the stray blank line from v1. One minor style issue remains (missing blank line before the new struct), and the unchecked return value of `drm_encoder_init()` is carried over from the original code. The other tegra encoder drivers (rgb, dsi, sor) still use `drm_simple_encoder_init()` and have the same unchecked-return pattern, so this patch is consistent with the rest of the driver.

**Verdict:** The v2 patch is correct and suitable for merge, with one minor style nit.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Patch Review

This is the initial version. It is functionally correct but has a minor style issue:

**Stray blank line in the include block:**
```diff
-#include <drm/drm_simple_kms_helper.h>
+
```
The removal of `drm_simple_kms_helper.h` leaves a double blank line (one from the diff `+` line plus the existing blank line before `#include "hda.h"`). This was identified and fixed in v2.

**Superseded by v2** — no further review needed.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/tegra: hdmi: Open-code drm_simple_encoder_init()

[Claude Code Review Bot]

Patch Review

This is the version that should be considered for merge.

**Include changes (correct):**
```c
+#include <drm/drm_encoder.h>
...
-#include <drm/drm_simple_kms_helper.h>
```
The new include for `drm_encoder.h` is needed to provide `drm_encoder_init()` and `drm_encoder_cleanup()` declarations directly, and the alphabetical sort order is maintained. The stray blank line from v1 is fixed.

**New encoder funcs struct:**
```c
+static const struct drm_encoder_funcs tegra_hdmi_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
```
This correctly replicates what `drm_simple_encoder_init()` provided internally. The struct is `static const`, which is the standard pattern.

**Minor style nit:** The struct is placed immediately after the closing `};` of `tegra124_tmds_config[]` on line 373 with no blank line separator. Kernel style typically has a blank line between top-level definitions. This was present in v1 too and wasn't addressed in v2's changelog.

**Call site change (correct):**
```c
-	drm_simple_encoder_init(drm, &hdmi->output.encoder,
-				DRM_MODE_ENCODER_TMDS);
+	drm_encoder_init(drm, &hdmi->output.encoder, &tegra_hdmi_encoder_funcs,
+				DRM_MODE_ENCODER_TMDS, NULL);
```
The `NULL` name parameter matches what `drm_simple_encoder_init()` passes internally. The return value of `drm_encoder_init()` is not checked, but this is consistent with the existing code — `drm_simple_encoder_init()`'s return was also being silently discarded. This is a pre-existing issue across all tegra encoder init paths (`rgb.c:308`, `dsi.c:1058`, `sor.c:3084`), and fixing it is outside the scope of this cleanup.

**Overall for v2:** Correct, minimal, no functional change. One optional nit about a missing blank line before the new struct definition. Suitable for merge as-is.

---
Generated by Claude Code Patch Reviewer