Claude review: drm/i915/edp: Check supported link rates DPCD read

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

Patch Review

**Correctness: Good**

The switch from `drm_dp_dpcd_read()` (returns bytes read or negative error) to `drm_dp_dpcd_read_data()` (returns 0 on success, negative on error, also handles short reads as `-EPROTO`) is the right call. It catches both outright failures *and* short reads, which the original code did not handle at all.

The `memset` fallback is elegant — it reuses the existing `rate == 0 → break` logic at line 4828 of the current tree:

```c
rate = le16_to_cpu(sink_rates[i]) * 200 / 10;
if (rate == 0)
    break;
```

This means `num_sink_rates` will be set to 0, which is the correct fallback.

**Minor observation:**

The `memset` on failure is technically redundant given `drm_dp_dpcd_read_data()`'s implementation. Looking at the function (drm_dp_helper.h:588-617), on the primary `drm_dp_dpcd_read()` failure path it falls through to byte-by-byte retry. If both paths fail, the buffer contents are indeterminate (partially written by the failed `drm_dp_dpcd_read()` call). So the `memset` is actually **necessary** for correctness — the buffer could contain partial/garbage data from a failed read. This is fine as-is.

**One nit:** The debug message says `"Unable to read eDP supported link rates, using default rates"` but whether "default rates" are actually used depends on what happens after `num_sink_rates` ends up as 0. Looking at the broader function (line 4845 in the tree), `num_sink_rates` is set to `i` which will be 0 after the zeroed array causes immediate break. The caller path does handle `num_sink_rates == 0` by falling back to DPCD-based rate calculation. So the message is accurate enough, though "falling back to DPCD-based rates" would be more precise. This is not worth a respin.

**Summary: Patch is correct, addresses a real (if unlikely) bug, and the implementation is clean. No blocking issues.**

---
Generated by Claude Code Patch Reviewer