Claude review: drm/edid: parse panel type from DisplayID 2.x Display Parameters block

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Struct layout and minimum length check — looks correct.**

The `displayid_display_params_block` struct places `device_tech_byte` at payload offset 27 (absolute block offset 0x1E), matching the spec reference. The `DISPLAYID_DISPLAY_PARAMS_MIN_LEN` correctly computes 29 (the struct payload excluding the `displayid_block` header), ensuring `device_tech_byte` is accessible before it's read:

```c
if (block->num_bytes < DISPLAYID_DISPLAY_PARAMS_MIN_LEN) {
    ...
    continue;
}
```

**Private field access in `displayid_iter`.**

The patch accesses `iter.section` directly:

```c
const u8 *section = NULL;
...
if (section != iter.section) {
    ...
    section = iter.section;
}
```

The `displayid_iter` struct is annotated with `Do not access directly, this is private.` While `drm_edid.c` is core DRM code and uses the internal header, this still violates the stated API contract. Consider adding an accessor like `displayid_section()` alongside the existing `displayid_version()` and `displayid_primary_use()`. Alternatively, a boolean `displayid_is_v2_headmount()` helper that encapsulates the VR/AR check might be cleaner, so the section-tracking remains internal to the iterator implementation.

**Behavior change from removing the `break` — intentional but worth noting in the commit message.**

The old code:
```c
displayid_iter_for_each(block, &iter) {
    // log + VR/AR check
    break;  // only first block of first section
}
```

The new code iterates **all blocks across all sections**. This is required to find the Display Parameters block, and the VR/AR detection is correctly refactored to fire once per section via the `section` pointer guard. However, the commit message doesn't mention this change in iteration scope. It would be helpful to note that the function now processes all DisplayID sections/blocks, not just the first.

**Minor: magic numbers in the switch.**

```c
case 1: /* LCD */
    info->did_panel_type = DRM_MODE_PANEL_TYPE_LCD;
    break;
case 2: /* OLED */
    info->did_panel_type = DRM_MODE_PANEL_TYPE_OLED;
    break;
```

These match the spec (Display Device Technology: 001b = LCD, 010b = OLED). The comments are sufficient, but named constants in the header (e.g. `DISPLAYID_DEVICE_TECH_LCD = 1`, `DISPLAYID_DEVICE_TECH_OLED = 2`) would be slightly more self-documenting and consistent with the kernel style for spec-derived values.

**Missing update to `drm_panel_type_enum_list`.**

The new `DRM_MODE_PANEL_TYPE_LCD` value is added to `drm_mode.h` but the enum registration in `drm_connector.c` is not updated:

```c
static const struct drm_prop_enum_list drm_panel_type_enum_list[] = {
    { DRM_MODE_PANEL_TYPE_UNKNOWN, "unknown" },
    { DRM_MODE_PANEL_TYPE_OLED, "OLED" },
    // missing: { DRM_MODE_PANEL_TYPE_LCD, "LCD" },
};
```

Without this, `DRM_MODE_PANEL_TYPE_LCD` cannot actually be used as a property value. This should be part of this patch (since this is where the UAPI constant is introduced).

**Overall:** Structurally solid parsing code, but needs the enum list update and preferably an accessor for `iter.section`.

---

---
Generated by Claude Code Patch Reviewer