[PATCH] drm/i915/dp: Fix link retrain loop on pre-gen8 platforms with DP adapters

[PATCH] drm/i915/dp: Fix link retrain loop on pre-gen8 platforms with DP adapters

[bitey]

On Ivy Bridge (and other pre-gen8 platforms), active DP-to-HDMI protocol
converters such as the Chrontel CH73111 trigger an infinite link retrain
loop on kernels 6.x+. The monitor connected through the adapter goes
black and recovers approximately once per second, eventually causing
system instability.

Three changes introduced in the i915 DP stack combine to cause this:

1. intel_dp_stop_link_train() now automatically schedules a post-training
   link check via intel_encoder_link_check_queue_work(). The Chrontel
   adapter reports non-standard DPCD link status, which causes the check
   to fail and trigger a retrain, looping indefinitely.

2. intel_dp_needs_link_retrain() unconditionally returns true when
   seq_train_failures > 0, bypassing the actual DPCD link status read.
   A single marginal training attempt permanently marks the link as
   needing retrain.

3. intel_dp_short_pulse() forces LINK_STATUS_CHANGED on every short HPD
   for all platforms, causing unnecessary link checks that initiate the
   retrain cycle on adapters that generate frequent short HPDs.

Fix all three by gating the new behavior behind DISPLAY_VER(display) >= 8,
restoring the 5.15-era behavior for Ivy Bridge/Sandy Bridge and earlier.
These platforms use the older g4x DP path and do not benefit from the
aggressive post-training link monitoring.

Tested on Intel HD 2500 (Ivy Bridge, device ID 0152) with a Chrontel
CH73111 DP-to-HDMI adapter driving a 1920x1080 display over 2 lanes
at HBR (270 MHz).

Signed-off-by: bitey <s.mavlenkov@mail.ru>
---
 drivers/gpu/drm/i915/display/intel_dp.c              | 8 +++++++-
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index ORIGINAL..FIXED 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.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;
 
 	memset(link_status, 0, DP_LINK_STATUS_SIZE);
@@ -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;
 
 	/* Retrain if link not ok */
@@ -5965,6 +5967,7 @@ static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
+	struct intel_display *display = to_intel_display(intel_dp);
 	bool reprobe_needed = false;
 	u8 esi[4] = {};
 
@@ -5988,7 +5991,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 	 * TODO: let the link status check depend on LINK_STATUS_CHANGED
 	 * or intel_dp->link.force_retrain for DPCD_REV >= 1.2
 	 */
-	esi[3] |= LINK_STATUS_CHANGED;
+	if (DISPLAY_VER(display) >= 8)
+		esi[3] |= LINK_STATUS_CHANGED;
 	if (intel_dp_handle_link_service_irq(intel_dp, esi[3]))
 		reprobe_needed = true;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index ORIGINAL..FIXED 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1154,7 +1154,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
 
 	intel_hpd_unblock(encoder);
 
-	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) {
 		int delay_ms = intel_dp->link.seq_train_failures ? 0 : 2000;
 
-- 
2.49.0

Re: [PATCH] drm/i915/dp: Fix link retrain loop on pre-gen8 platforms with DP adapters

[Jani Nikula]

On Thu, 30 Apr 2026, bitey <s.mavlenkov@mail.ru> wrote:
> On Ivy Bridge (and other pre-gen8 platforms), active DP-to-HDMI protocol
> converters such as the Chrontel CH73111 trigger an infinite link retrain
> loop on kernels 6.x+. The monitor connected through the adapter goes
> black and recovers approximately once per second, eventually causing
> system instability.
>
> Three changes introduced in the i915 DP stack combine to cause this:
>
> 1. intel_dp_stop_link_train() now automatically schedules a post-training
>    link check via intel_encoder_link_check_queue_work(). The Chrontel
>    adapter reports non-standard DPCD link status, which causes the check
>    to fail and trigger a retrain, looping indefinitely.
>
> 2. intel_dp_needs_link_retrain() unconditionally returns true when
>    seq_train_failures > 0, bypassing the actual DPCD link status read.
>    A single marginal training attempt permanently marks the link as
>    needing retrain.
>
> 3. intel_dp_short_pulse() forces LINK_STATUS_CHANGED on every short HPD
>    for all platforms, causing unnecessary link checks that initiate the
>    retrain cycle on adapters that generate frequent short HPDs.
>
> Fix all three by gating the new behavior behind DISPLAY_VER(display) >= 8,
> restoring the 5.15-era behavior for Ivy Bridge/Sandy Bridge and earlier.
> These platforms use the older g4x DP path and do not benefit from the
> aggressive post-training link monitoring.
>
> Tested on Intel HD 2500 (Ivy Bridge, device ID 0152) with a Chrontel
> CH73111 DP-to-HDMI adapter driving a 1920x1080 display over 2 lanes
> at HBR (270 MHz).

Please file a bug over at [1] with the full dmesg reproducing the
issues, and reply with the link to the bug.

Thanks,
Jani.


[1] https://drm.pages.freedesktop.org/intel-docs/how-to-file-i915-bugs.html


>
> Signed-off-by: bitey <s.mavlenkov@mail.ru>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c              | 8 +++++++-
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 3 ++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ORIGINAL..FIXED 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.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;
>  
>  	memset(link_status, 0, DP_LINK_STATUS_SIZE);
> @@ -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;
>  
>  	/* Retrain if link not ok */
> @@ -5965,6 +5967,7 @@ static bool
>  intel_dp_short_pulse(struct intel_dp *intel_dp)
>  {
> +	struct intel_display *display = to_intel_display(intel_dp);
>  	bool reprobe_needed = false;
>  	u8 esi[4] = {};
>  
> @@ -5988,7 +5991,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  	 * TODO: let the link status check depend on LINK_STATUS_CHANGED
>  	 * or intel_dp->link.force_retrain for DPCD_REV >= 1.2
>  	 */
> -	esi[3] |= LINK_STATUS_CHANGED;
> +	if (DISPLAY_VER(display) >= 8)
> +		esi[3] |= LINK_STATUS_CHANGED;
>  	if (intel_dp_handle_link_service_irq(intel_dp, esi[3]))
>  		reprobe_needed = true;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index ORIGINAL..FIXED 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1154,7 +1154,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
>  
>  	intel_hpd_unblock(encoder);
>  
> -	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) {
>  		int delay_ms = intel_dp->link.seq_train_failures ? 0 : 2000;

-- 
Jani Nikula, Intel

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

[Claude Code Review Bot]

Overall Series Review

Subject: drm/i915/dp: Fix link retrain loop on pre-gen8 platforms with DP adapters
Author: bitey <s.mavlenkov@mail.ru>
Patches: 2
Reviewed: 2026-05-05T10:51:24.766213

---

This is a single patch attempting to fix an infinite link retrain loop on pre-gen8 i915 platforms when using DP-to-HDMI protocol converters (specifically a Chrontel CH73111). The commit message is well-written and clearly identifies three contributing root causes in the modern DP link training path. However, the patch has a **critical build-breaking error** and several design-level concerns.

**Severity: The patch will not compile as submitted.**

The approach of gating all three code paths behind `DISPLAY_VER(display) >= 8` is a broad workaround rather than a targeted fix. It effectively disables post-training link monitoring and short-HPD link checks for all pre-gen8 platforms, not just the ones affected by misbehaving adapters. A more surgical fix (e.g., quirking the specific adapter, or using DPCD revision for the `short_pulse` change) would be preferable.

Additionally, the `Signed-off-by` uses the handle "bitey" rather than a real name, which does not satisfy the DCO (Developer Certificate of Origin) requirements for kernel contributions.

---

---
Generated by Claude Code Patch Reviewer

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

[Claude Code Review Bot]

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