Claude review: drm/i915/dp: Fix link retrain loop on pre-gen8 platforms with DP adapters

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

Patch Review

#### Build error: `display` variable declared in wrong function

The first hunk adds `display` to `intel_dp_read_link_status()`:

```c
@@ -5676,6 +5676,7 @@ static int
 intel_dp_read_link_status(struct intel_dp *intel_dp,
 			  u8 link_status[DP_LINK_STATUS_SIZE])
 {
+	struct intel_display *display = to_intel_display(intel_dp);
 	int err;
```

But the second hunk uses `display` inside `intel_dp_needs_link_retrain()`, which is a **separate function** starting at line 5675:

```c
@@ -5712,7 +5713,8 @@ static bool intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
 	if (intel_dp->link.retrain_disabled)
 		return false;
 
-	if (intel_dp->link.seq_train_failures)
+	if (DISPLAY_VER(display) >= 8 &&
+	    intel_dp->link.seq_train_failures)
 		return true;
```

This will cause two compilation errors:
1. **Unused variable warning/error** in `intel_dp_read_link_status()` (where `display` is declared but never referenced).
2. **Undeclared identifier error** in `intel_dp_needs_link_retrain()` (where `display` is used but never declared).

The `display` variable declaration should be moved from `intel_dp_read_link_status()` to `intel_dp_needs_link_retrain()`.

#### Design concern: `intel_dp_short_pulse()` change disables link checking too broadly

```c
-	esi[3] |= LINK_STATUS_CHANGED;
+	if (DISPLAY_VER(display) >= 8)
+		esi[3] |= LINK_STATUS_CHANGED;
```

The existing code force-sets `LINK_STATUS_CHANGED` specifically to cover sinks with `DPCD_REV < 1.2` that don't report this bit themselves. The existing TODO comment right above this line says:

```c
	 * TODO: let the link status check depend on LINK_STATUS_CHANGED
	 * or intel_dp->link.force_retrain for DPCD_REV >= 1.2
```

Gating this on `DISPLAY_VER >= 8` means pre-gen8 platforms with DPCD 1.1 sinks will never have their link status checked on short HPD — even for genuine link degradation, not just the Chrontel adapter problem. A more targeted guard would be to check `intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_12` (which is what the TODO itself suggests), or to detect/quirk the specific misbehaving adapter via its OUI.

#### Design concern: `intel_dp_needs_link_retrain()` change

```c
-	if (intel_dp->link.seq_train_failures)
+	if (DISPLAY_VER(display) >= 8 &&
+	    intel_dp->link.seq_train_failures)
 		return true;
```

On pre-gen8, `seq_train_failures > 0` will no longer trigger a retrain. The code will fall through to the `intel_dp_link_ok()` check, which reads DPCD link status. If the adapter reports non-standard DPCD (as noted in the commit message), this may still trigger a retrain via that path. Whether this fully breaks the loop depends on what the Chrontel adapter reports — worth verifying in testing.

#### `intel_dp_stop_link_train()` change looks reasonable

```c
-	if (!display->hotplug.ignore_long_hpd &&
+	if (DISPLAY_VER(display) >= 8 &&
+	    !display->hotplug.ignore_long_hpd &&
 	    intel_dp->link.seq_train_failures < MAX_SEQ_TRAIN_FAILURES) {
```

This is the most reasonable of the three changes. The post-training link check queue was added for modern platforms and is the primary entry point into the retrain loop. Skipping it on pre-gen8 platforms is a sensible and low-risk change. The `display` variable is already declared in this function, so no scoping issue here.

#### Signed-off-by issue

```
Signed-off-by: bitey <s.mavlenkov@mail.ru>
```

The kernel DCO requires a legal name in the `Signed-off-by` line, not a handle/nickname.

#### Summary

The patch must be resubmitted with at minimum:
1. The `display` variable declaration moved to the correct function (`intel_dp_needs_link_retrain`, not `intel_dp_read_link_status`).
2. A real name in the `Signed-off-by` tag.

Ideally, the `intel_dp_short_pulse()` change should be reconsidered — either guarding on `DPCD_REV` instead of `DISPLAY_VER`, or quirking the specific adapter, to avoid disabling legitimate link checks on all pre-gen8 platforms.

---
Generated by Claude Code Patch Reviewer