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: Implement "color format" DRM property Date: Mon, 25 May 2026 17:26:12 +1000 Message-ID: In-Reply-To: <20260523-color-format-v16-20-24340c5e4732@collabora.com> References: <20260523-color-format-v16-0-24340c5e4732@collabora.com> <20260523-color-format-v16-20-24340c5e4732@collabora.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 The most complex rockchip patch. A few observations: **The `dw_hdmi_qp_rockchip_get_vop_format` function** correctly uses `__fre= e(drm_bridge_put)` for cleanup. The fallback from `input_bus_cfg.format` to= `output_bus_cfg.format` when input is FIXED is the right pattern. **The register value function** `dw_hdmi_qp_rockchip_bus_fmt_to_reg` correc= tly unifies the old `RK3576_RGB`/`RK3588_RGB` etc. constants into a single = set (`RK_COLOR_FMT_*`) since the values were the same (the old code had a t= ypo in the definitions). **Minor:** In `dw_hdmi_qp_rk3576_enc_init` and `dw_hdmi_qp_rk3588_enc_init`= , `likely(color >=3D 0)` guards the register write. If `color` is negative = (unknown bus format), the color format field is simply not written, meaning= whatever was previously in the register persists. This is a reasonable def= ensive behavior, but it silently accepts an error condition. **Missing UYVY10_1X20 in atomic_check:** The switch in `dw_hdmi_qp_rockchip= _encoder_atomic_check` handles `MEDIA_BUS_FMT_UYVY8_1X16` for YUV422 but no= t `MEDIA_BUS_FMT_UYVY10_1X20` (10-bit YUV422). If the bridge chain selects = 10-bit YUV422, the atomic_check would return `-EINVAL`. This seems like a p= otential gap =E2=80=94 the `dw_hdmi_qp_rockchip_bus_fmt_to_reg` function *d= oes* handle `UYVY10_1X20`, suggesting the encoder should too. --- Generated by Claude Code Patch Reviewer