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/mipi-dsi: add RGB101010 pixel format Date: Sun, 22 Mar 2026 03:12:28 +1000 Message-ID: In-Reply-To: <20260321-dsi-rgb101010-support-v4-1-eb28ecebbfe8@pm.me> References: <20260321-dsi-rgb101010-support-v4-0-eb28ecebbfe8@pm.me> <20260321-dsi-rgb101010-support-v4-1-eb28ecebbfe8@pm.me> 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 new enum member is added before `MIPI_DSI_FMT_RGB888`: ```c enum mipi_dsi_pixel_format { MIPI_DSI_FMT_RGB101010, MIPI_DSI_FMT_RGB888, ``` This shifts the numeric values of all existing members (RGB888 goes from 0 = to 1, etc.). While all in-tree usage is by symbolic name, **adding new enum= values at the end is conventional practice** in the kernel to minimize ris= k from out-of-tree consumers or any code that might serialize/deserialize t= hese values. Consider placing it after `MIPI_DSI_FMT_RGB565` instead. The `mipi_dsi_pixel_format_to_bpp` addition returning 30 is correct per MIP= I DSI v1.3 Section 8.8.17. Note: `drm_mipi_dsi_get_input_bus_fmt()` in `drm_mipi_dsi.c` has a `default= ` case returning 0 for the new format =E2=80=94 this means any bridge drive= r calling that function for RGB101010 will silently get a 0 bus format. Tha= t's acceptable for now since no bridge currently supports RGB101010 on the = input side, but a `MEDIA_BUS_FMT_RGB101010_1X30` mapping may eventually be = needed. Also: several drivers do `mipi_dsi_pixel_format_to_bpp(...) / 8` to get byt= es-per-pixel (e.g., `sun6i_mipi_dsi.c`, `mcde_dsi.c`). For 30bpp, `30 / 8 = =3D 3` (integer truncation), which is wrong =E2=80=94 but those drivers don= 't support RGB101010, so this is fine in practice. **Looks good** with the minor suggestion about enum ordering. --- --- Generated by Claude Code Patch Reviewer