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/edid: parse panel type from DisplayID 2.x Display Parameters Date: Wed, 27 May 2026 15:16:43 +1000 Message-ID: In-Reply-To: <20260526030254.1460480-3-chen-yu.chen@amd.com> References: <20260526030254.1460480-1-chen-yu.chen@amd.com> <20260526030254.1460480-3-chen-yu.chen@amd.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 **Status: Looks good with minor notes.** The struct definition for `displayid_display_params_block` properly lays ou= t all fields per DisplayID 2.1a spec Table 4-7. The payload size calculates= correctly: ``` sizeof(displayid_display_params_block) - sizeof(displayid_block) =3D (3 + 2+2+2+2+1+9+3+2+2+2+1+1) - 3 =3D 29 bytes ``` This matches the expected payload for the Display Parameters block. The length validation is correct: ```c if (block->num_bytes < sizeof(*params) - sizeof(params->base)) ``` The field extraction `FIELD_GET(DISPLAYID_DISPLAY_PARAMS_DEVICE_TECH, param= s->color_depth_and_tech)` with `GENMASK(6, 4)` correctly extracts bits [6:4= ] for the device technology field. The version guard in `update_displayid_info()`: ```c if (displayid_version(&iter) =3D=3D DISPLAY_ID_STRUCTURE_VER_20 && block->tag =3D=3D DATA_BLOCK_2_DISPLAY_PARAMETERS) ``` correctly restricts parsing to DisplayID 2.x only, since the tag value 0x21= has different semantics in DisplayID 1.x. **Minor notes:** - The `panel_type` field added to `drm_display_info` stores `DRM_MODE_PANEL= _TYPE_*` UAPI values directly. This is consistent with how other UAPI-facin= g properties are handled in this struct, but it creates a coupling worth no= ting =E2=80=94 if the UAPI enum ever changes numbering, this field must be = updated too. In practice this won't happen since UAPI is stable. - If a DisplayID contains multiple Display Parameters Data Blocks (theoreti= cally possible per spec, though unusual), the last one wins. This seems acc= eptable. - The UAPI addition of `DRM_MODE_PANEL_TYPE_LCD =3D 2` is backwards-compati= ble. Existing userspace seeing an unknown enum value would already need to = handle it gracefully, and the immutable property documentation is updated a= ccordingly. --- Generated by Claude Code Patch Reviewer