Claude review: drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

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

Patch Review

This patch extends the HDMI 2.0 code for HDMI 2.1 FRL (Fixed Rate Link) support. Good approach of not requiring a version check since HDMI 2.0 sinks have reserved fields defined as 0.

**Error counter checksum with 4 lanes:** For 4-lane mode, the code reads 9 bytes (0x50-0x58) and checksums all of them:

```c
case 4:
    buf_sz = 9;
    break;
...
for (i = 0; i < buf_sz; i++)
    sum = wrapping_add(u8, sum, buf[i]);
```

The checksum register is at 0x56 (buf[6]). This assumes the HDMI 2.1 checksum at 0x56 covers registers 0x50-0x55 *and* 0x57-0x58 (all error detection registers). Please confirm this matches the CTA-861-H spec. If the checksum only covers 0x50-0x55 (the HDMI 2.0 range), the validation would fail when lane 3 has non-zero data, as bytes 0x57-0x58 would be summed but not included in the checksum calculation by the sink.

**Enum bitfield:**

```c
enum drm_scdc_frl_rate rate : 4;
u8 ffe_levels : 4;
```

Enum bitfields are a GCC/Clang extension, not standard C. This works in the kernel build environment but is worth noting. The 4-bit width is sufficient for the 0-15 value range since GCC treats this enum's underlying type as unsigned (all values are non-negative).

**`scdc_err_cnt_buf_idx` function:** This works correctly but is more complex than needed. Since the registers are at known offsets, a simpler computation would be `lane * 2` for lanes 0-2 and 7 for lane 3. The switch statement is clearer about the non-contiguous register layout though (checksum sits between lane 2 and lane 3), so it's a reasonable choice.

**`drm_scdc_frl_rate_str` has a `default: return NULL` case:**

```c
default:
    return NULL;
```

All 16 possible 4-bit values are covered by the switch cases (0-6 named, 7-15 reserved), so the `default` is unreachable. Returning `NULL` from a function whose result is passed directly to `seq_printf` via `scdc_print_str` would cause a NULL dereference if it were ever reached. Consider returning `"(Unknown)"` instead, or removing the default case entirely.

**FRL rate 0 → 3 lanes fallback:** The handling in `drm_scdc_read_state` is correct:

```c
num_lanes = drm_scdc_num_frl_lanes(state->rate);
if (num_lanes < 0)
    return num_lanes;
if (!num_lanes)
    num_lanes = 3;
```

When FRL is off (TMDS mode), defaulting to 3 lanes for the HDMI 2.0 error counters makes sense.

**Status register 1 bitfield name:** The field is called `flags` but now contains 4-bit training pattern values rather than individual boolean flags:

```c
struct {
    u8 ln0_training_pattern : 4;
    u8 ln1_training_pattern : 4;
} flags __packed;
```

Consider renaming it to something like `fields` to better reflect the content, since these are multi-bit values, not flags.

---
Generated by Claude Code Patch Reviewer