From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/rockchip: dw_hdmi_qp: Switch to drmm_encoder_init() Date: Tue, 10 Mar 2026 11:47:02 +1000 Message-ID: In-Reply-To: <20260310-drm-rk-fixes-v2-6-645ecfb43f49@collabora.com> References: <20260310-drm-rk-fixes-v2-0-645ecfb43f49@collabora.com> <20260310-drm-rk-fixes-v2-6-645ecfb43f49@collabora.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Verdict: Good, one ordering note** This patch does two things: (1) switches the struct allocation to `drmm_kzalloc()` (same fix as patches 1/2), and (2) replaces `drm_simple_encoder_init()` with `drmm_encoder_init()`, which also allows removing the explicit `drm_encoder_cleanup()` call from `unbind`. One observation: In the patched code, `drm_encoder_helper_add()` is called *before* `drmm_encoder_init()`: ```c drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs); ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL); ``` This is technically fine since `drm_encoder_helper_add()` just sets `encoder->helper_private` which doesn't require the encoder to be registered with the DRM core. However, the existing `dw_dp-rockchip.c` does it in the opposite order (init first, helper_add second), which is the more conventional ordering. Consider swapping these two lines for consistency, though it's not a correctness issue. --- Generated by Claude Code Patch Reviewer