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/colorop: Add DRM_COLOROP_CSC_FF Date: Mon, 09 Mar 2026 08:26:52 +1000 Message-ID: In-Reply-To: <20260306165307.3233194-2-chaitanya.kumar.borah@intel.com> References: <20260306165307.3233194-1-chaitanya.kumar.borah@intel.com> <20260306165307.3233194-2-chaitanya.kumar.borah@intel.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 Good addition to the DRM colorop framework. The implementation follows exis= ting patterns well. Minor observations: - The `drm_get_colorop_csc_ff_type_name` doc comment says "return a string = for interpolation type" =E2=80=94 this is a copy-paste error from the LUT3D= interpolation function. It should say "return a string for CSC FF type". ```c /** * drm_get_colorop_csc_ff_type_name: return a string for interpolation type * @type: csc ff type to compute name of ``` - In `drm_plane_colorop_csc_ff_init`, if the `drm_plane_colorop_init` call = succeeds but `drm_property_create_enum` returns NULL, the function returns = `-ENOMEM` without cleaning up the partially initialized colorop. The existi= ng patterns in other `*_init` functions have the same issue, so this is pre= -existing, but worth noting. - The `WARN_ON(len <=3D 0)` check after the loop is dead code since `suppor= ted_csc_ff` was already validated non-zero above. It's a reasonable defensi= ve check though. --- Generated by Claude Code Patch Reviewer