From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/tegra: hdmi: Open-code drm_simple_encoder_init()
Date: Sat, 16 May 2026 12:11:38 +1000
Message-ID: <review-patch2-20260513100501.6468-1-souradiptodas6@gmail.com>
In-Reply-To: <20260513100501.6468-1-souradiptodas6@gmail.com>
References: <20260502143420.7850-1-souradiptodas6@gmail.com> <20260513100501.6468-1-souradiptodas6@gmail.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

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 =3D {
+	.destroy =3D drm_encoder_cleanup,
+};
```
This correctly replicates what `drm_simple_encoder_init()` provided interna=
lly. 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. Ker=
nel style typically has a blank line between top-level definitions. This wa=
s 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 i=
nternally. The return value of `drm_encoder_init()` is not checked, but thi=
s is consistent with the existing code =E2=80=94 `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 ni=
t about a missing blank line before the new struct definition. Suitable for=
 merge as-is.

---
Generated by Claude Code Patch Reviewer