All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link
@ 2026-08-17  3:16 rh_king
  2026-08-17  3:21 ` sashiko-bot
  2026-08-18  2:42 ` [PATCH v2] " rh_king
  0 siblings, 2 replies; 4+ messages in thread
From: rh_king @ 2026-08-17  3:16 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona
  Cc: siqueira, dillon.varone, gaghik.khachatrian, pinglei.lin,
	zhikai.zhai, robin.chen, alex.hung, Matthew.Stewart2, jack.chang,
	clayking, chuntao.tso, PeiChen.Huang, Derek.Lai, amd-gfx,
	dri-devel, linux-kernel, Kean Ren

From: Kean Ren <rh_king@163.com>

When an eDP panel is still recovering right after resume (e.g. lid open
or AC return on a ThinkPad that has been in s2idle for hours), AUX/DPCD
writes may transiently fail. Two distinct code paths compound this into
a kernel WARN at dce_aux_transfer_raw():

  1. dpcd_set_link_settings() reuses a single `status` variable for
     four core_link_write_dpcd() calls and only emits DC_LOG_ERROR on
     failure. The first failing write gets overwritten by the next
     call, so the function can return DC_OK even when every DPCD write
     failed. Callers therefore cannot tell that the link is unhealthy
     and continue with PSR setup on a dead AUX channel.

  2. edp_setup_psr() does not consult link->link_status.link_active
     before pushing the PSR enable DPCD writes. When the link training
     failed, the sink is not actually there to ACK, so
     dm_helpers_dp_write_dpcd() -> dce_aux_transfer_raw() hangs until
     AUX_SW_DONE times out and triggers ASSERT_CRITICAL().

Observed on a Lenovo ThinkPad 21XHZDY2CN (BIOS R3HET22W 1.08) running
Ubuntu 24.04 with 6.17.0-1030-oem. The user-visible trigger is usually
a network event right after resume (unplug/replug the r8169 Ethernet
cable, NetworkManager roaming to wlan, or a lid-close -> lid-open
cycle). The dbus signal from those events causes a Wayland compositor
(gnome-shell) or an X11 client running under XWayland to issue
DRM_IOCTL_MODE_SETCRTC, which reaches amdgpu_dm_enable_self_refresh()
and then edp_setup_psr(). The "Xorg" comm name in the WARN trace is
XWayland, since this box boots into a GNOME Wayland session.

```
amdgpu 0000:c6:00.0: [drm] enabling link 0 failed: 15
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1122: core_link_write_dpcd (DP_DOWNSPREAD_CTRL) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1127: core_link_write_dpcd (DP_LANE_COUNT_SET) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1144: core_link_write_dpcd (DP_LINK_BW_SET) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1149: core_link_write_dpcd (DP_LINK_RATE_SET) failed
[- cut here -]
WARNING: CPU: 0 PID: 2615 at drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:393 dce_aux_transfer_raw+0x296/0x2e0 [amdgpu]
CPU: 0 UID: 1000 PID: 2615 Comm: Xorg Tainted: G           O        6.17.0-1030-oem #30-Ubuntu PREEMPT(voluntary)
Tainted: [O]=OOT_MODULE
Hardware name: LENOVO 21XHZDY2CN/21XHZDY2CN, BIOS R3HET22W (1.08 ) 06/24/2026
RIP: 0010:dce_aux_transfer_raw+0x296/0x2e0 [amdgpu]
Code: ff e9 49 ff ff ff 41 c7 04 24 04 00 00 00 eb eb 3c 01 0f 87 4c f4 34 00 83 e0 01 3c 01 19 c0 83 e0 c0 83 c0 50 e9 3f fe ff ff <0f> 0b 41 c7 04 24 03 00 00 00 eb c5 41 c7 04 24 03 00 00 00 eb bb
RSP: 0018:ffffcdd6c53272f8 EFLAGS: 00010246
RAX: 0000000062000000 RBX: ffff8d353020fc80 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffcdd6c5327358 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffcdd6c53273ac
R13: ffffcdd6c53273b0 R14: 0000000000000001 R15: ffff8d3562690000
FS:  00007b3cc831aac0(0000) GS:ffff8d4c6d669000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000700f940020f8 CR3: 000000012fe37000 CR4: 0000000000f50ef0
PKRU: 55555554
Call stack:
 <TASK>
 link_aux_transfer_raw+0x48/0x80 [amdgpu]
 dc_link_aux_transfer_raw+0x24/0x40 [amdgpu]
 dm_dp_aux_transfer+0xee/0x2b0 [amdgpu]
 drm_dp_dpcd_access+0xbe/0x160 [drm_display_helper]
 drm_dp_dpcd_write+0xc4/0x120 [drm_display_helper]
 dm_helpers_dp_write_dpcd+0x29/0x60 [amdgpu]
 edp_setup_psr+0x156/0x5a0 [amdgpu]
 dc_link_setup_psr+0x20/0x40 [amdgpu]
 amdgpu_dm_link_setup_psr+0x155/0x1a0 [amdgpu]
 ? dm_write_reg_func+0x47/0xc0 [amdgpu]
 amdgpu_dm_enable_self_refresh+0xaa/0x240 [amdgpu]
 amdgpu_dm_commit_planes+0x636/0x1740 [amdgpu]
 ? manage_dm_interrupts+0xa5/0x280 [amdgpu]
 amdgpu_dm_atomic_commit_tail+0xb04/0x1270 [amdgpu]
 ? __set_output_tf.constprop.0+0xfd/0x1a0 [amdgpu]
 commit_tail+0xc6/0x1b0
 drm_atomic_helper_commit+0x132/0x160
 drm_atomic_commit+0xac/0xf0
 ? __pfx___drm_printfn_info+0x10/0x10
 drm_atomic_helper_set_config+0x82/0xd0
 drm_mode_setcrtc+0x3ff/0x9e0
 ? rmapiMapWithSecInfo+0x230/0x2b0 [nvidia]
 ? __pfx_drm_mode_setcrtc+0x10/0x10
 drm_ioctl_kernel+0xb4/0x110
 drm_ioctl+0x2ec/0x5b0
 ? __pfx_drm_mode_setcrtc+0x10/0x10
 amdgpu_drm_ioctl+0x4b/0xa0 [amdgpu]
 __x64_sys_ioctl+0xa2/0x100
 x64_sys_call+0x1226/0x2680
 do_syscall_64+0x80/0x8b0
 ? check_heap_object+0x17f/0x1c0
 ? nvidia_unlocked_ioctl+0x175/0x9a0 [nvidia]
 ? __x64_sys_ioctl+0xbf/0x100
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7b3cc8724f1d
Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00
RSP: 002b:00007ffe65c05b80 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000617240d63e10 RCX: 00007b3cc8724f1d
RDX: 00007ffe65c05c10 RSI: 00000000c06864a2 RDI: 0000000000000010
RBP: 00007ffe65c05bd0 R08: 0000000000000000 R09: 0000617240c4b700
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe65c05c10
R13: 00000000c06864a2 R14: 0000000000000010 R15: 000061723f332b30
 </TASK>
[- end trace 0000000000000000 -]
```

Fix both issues:

  - dpcd_set_link_settings(): use a separate `result` variable for each
    core_link_write_dpcd() call and return immediately on the first
    failure, so callers see the real AUX/DPCD state.

  - edp_setup_psr(): short-circuit when link->link_status.link_active
    is false, so we never push PSR configuration over a dead AUX
    channel.

Signed-off-by: Kean Ren <rh_king@163.com>
---
 drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c       | 33 ++++++++++++++++----
 drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c | 11 +++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
index 605bf19dc4f2..881cba95427f 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
@@ -1097,6 +1097,7 @@ enum dc_status dpcd_set_link_settings(
 {
 	uint8_t rate;
 	enum dc_status status;
+	enum dc_status result;

 	union down_spread_ctrl downspread = {0};
 	union lane_count_set lane_count_set = {0};
@@ -1117,15 +1118,25 @@ enum dc_status dpcd_set_link_settings(
 				link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED;
 	}

-	status = core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
-		&downspread.raw, sizeof(downspread));
-	if (status != DC_OK)
+	/* Bail out on the first DPCD write failure so callers can react and
+	 * subsequent operations (e.g. PSR setup) do not keep poking an
+	 * unhealthy AUX channel. Without this, a transient AUX/HPD glitch
+	 * during resume leads to a cascade of DPCD errors and ultimately a
+	 * WARN at dce_aux_transfer_raw() because AUX_SW_DONE never asserts.
+	 */
+	result = core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
+				      &downspread.raw, sizeof(downspread));
+	if (result != DC_OK) {
 		DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_DOWNSPREAD_CTRL) failed\n", __func__, __LINE__);
+		return result;
+	}

-	status = core_link_write_dpcd(link, DP_LANE_COUNT_SET,
-		&lane_count_set.raw, 1);
-	if (status != DC_OK)
+	result = core_link_write_dpcd(link, DP_LANE_COUNT_SET,
+				      &lane_count_set.raw, 1);
+	if (result != DC_OK) {
 		DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LANE_COUNT_SET) failed\n", __func__, __LINE__);
+		return result;
+	}

 	if (link->dpcd_caps.dpcd_rev.raw >= DPCD_REV_13 &&
 			lt_settings->link_settings.use_link_rate_set == true) {
@@ -1140,20 +1151,26 @@ enum dc_status dpcd_set_link_settings(
 			core_link_read_dpcd(link, DP_SUPPORTED_LINK_RATES,
 					supported_link_rates, sizeof(supported_link_rates));
 		}
-		status = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
-		if (status != DC_OK)
+		result = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
+		if (result != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_BW_SET) failed\n", __func__, __LINE__);
+			return result;
+		}

-		status = core_link_write_dpcd(link, DP_LINK_RATE_SET,
-				&lt_settings->link_settings.link_rate_set, 1);
-		if (status != DC_OK)
+		result = core_link_write_dpcd(link, DP_LINK_RATE_SET,
+					      &lt_settings->link_settings.link_rate_set, 1);
+		if (result != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_RATE_SET) failed\n", __func__, __LINE__);
+			return result;
+		}
 	} else {
 		rate = get_dpcd_link_rate(&lt_settings->link_settings);

-		status = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
-		if (status != DC_OK)
+		result = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
+		if (result != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_BW_SET) failed\n", __func__, __LINE__);
+			return result;
+		}
 	}

 	if (rate) {
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index 80a372ceaa51..43a0facc8884 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -699,6 +699,17 @@ bool edp_setup_psr(struct dc_link *link,
 	if (!link)
 		return false;

+	/* Skip PSR setup when the eDP link is not active. When AUX/DPCD
+	 * writes are failing (e.g. after a resume where the panel has not
+	 * fully come back yet), edp_setup_psr() will still try to push
+	 * configuration over the AUX channel. That auxiliary transfer never
+	 * completes and triggers ASSERT_CRITICAL() in dce_aux_transfer_raw().
+	 * The DPCD read of the PSR cap below is also unsafe on a dead link,
+	 * so bail out early before touching the sink.
+	 */
+	if (!link->link_status.link_active)
+		return false;
+
 	/* This is a workaround: some vendors require the source to
 	 * read the PSR cap; otherwise, the vendor's PSR feature will
 	 * fall back to its default behavior, causing a misconfiguration
--
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link
  2026-08-17  3:16 [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link rh_king
@ 2026-08-17  3:21 ` sashiko-bot
  2026-08-18  2:42 ` [PATCH v2] " rh_king
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-17  3:21 UTC (permalink / raw)
  To: rh_king; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The function `dpcd_set_link_settings()` returns an uninitialized `status` variable on its success path.
--

--- Patch [1]: [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link ---
commit ea92b689001bf0ca86b430f7c8b3fa5b47810b12
Author: Kean Ren <rh_king@163.com>

drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link

This commit fixes PSR AUX WARNs on unhealthy eDP links by returning early
from dpcd_set_link_settings() on the first DPCD write failure, and by
short-circuiting edp_setup_psr() when the link is inactive.

> diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
> index 04eedec8a230c..501d2556e06c5 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
> @@ -1097,6 +1097,7 @@ enum dc_status dpcd_set_link_settings(
>  {
>  	uint8_t rate;
>  	enum dc_status status;
> +	enum dc_status result;

[Severity: High]
Since the assignments in this function were changed from status to result,
is status left uninitialized?

If all DPCD writes succeed, the function completes by returning status at the
end of the function:

dpcd_set_link_settings() {
    ...
    return status;
}

Does this cause dpcd_set_link_settings() to return random stack memory on the
success path, potentially causing callers to interpret a success as an error
and fail the display initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817031640.2097973-1-rh_king@163.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link
  2026-08-17  3:16 [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link rh_king
  2026-08-17  3:21 ` sashiko-bot
@ 2026-08-18  2:42 ` rh_king
  2026-08-18  3:03   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: rh_king @ 2026-08-18  2:42 UTC (permalink / raw)
  To: harry.wentland, sunpeng.li, alexander.deucher, christian.koenig,
	airlied, simona
  Cc: siqueira, dillon.varone, gaghik.khachatrian, pinglei.lin,
	zhikai.zhai, robin.chen, alex.hung, Matthew.Stewart2, jack.chang,
	clayking, chuntao.tso, PeiChen.Huang, Derek.Lai, amd-gfx,
	dri-devel, linux-kernel, Kean Ren

From: Kean Ren <rh_king@163.com>

v2: Keep the original `status` variable, just add `return status;` on
    each error path. The previous v1 attempt renamed `status` to a new
    `result` variable to avoid masking failures across the four
    core_link_write_dpcd() calls, but missed that the function's final
    `return status;` was then left returning uninitialised stack memory
    on the success path (caught by Sashiko AI review). Restoring the
    `status` name and only adding the early returns gives a smaller,
    more obvious diff.

When an eDP panel is still recovering right after resume (e.g. lid open
or AC return on a ThinkPad that has been in s2idle for hours), AUX/DPCD
writes may transiently fail. Two distinct code paths compound this into
a kernel WARN at dce_aux_transfer_raw():

  1. dpcd_set_link_settings() reuses a single `status` variable for
     four core_link_write_dpcd() calls and only emits DC_LOG_ERROR on
     failure. The first failing write gets overwritten by the next
     call, so the function can return DC_OK even when every DPCD write
     failed. Callers therefore cannot tell that the link is unhealthy
     and continue with PSR setup on a dead AUX channel.

  2. edp_setup_psr() does not consult link->link_status.link_active
     before pushing the PSR enable DPCD writes. When the link training
     failed, the sink is not actually there to ACK, so
     dm_helpers_dp_write_dpcd() -> dce_aux_transfer_raw() hangs until
     AUX_SW_DONE times out and triggers ASSERT_CRITICAL().

Observed on a Lenovo ThinkPad 21XHZDY2CN (BIOS R3HET22W 1.08) running
Ubuntu 24.04 with 6.17.0-1030-oem. The user-visible trigger is usually
a network event right after resume (unplug/replug the r8169 Ethernet
cable, NetworkManager roaming to wlan, or a lid-close -> lid-open
cycle). The dbus signal from those events causes a Wayland compositor
(gnome-shell) or an X11 client running under XWayland to issue
DRM_IOCTL_MODE_SETCRTC, which reaches amdgpu_dm_enable_self_refresh()
and then edp_setup_psr(). The "Xorg" comm name in the WARN trace is
XWayland, since this box boots into a GNOME Wayland session.

```
amdgpu 0000:c6:00.0: [drm] enabling link 0 failed: 15
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1122:
        core_link_write_dpcd (DP_DOWNSPREAD_CTRL) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1127:
        core_link_write_dpcd (DP_LANE_COUNT_SET) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1144:
        core_link_write_dpcd (DP_LINK_BW_SET) failed
amdgpu 0000:c6:00.0: [drm] *ERROR* dpcd_set_link_settings:1149:
        core_link_write_dpcd (DP_LINK_RATE_SET) failed
[- cut here -]
WARNING: CPU: 0 PID: 2615 at drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:393
        dce_aux_transfer_raw+0x296/0x2e0 [amdgpu]
CPU: 0 UID: 1000 PID: 2615 Comm: Xorg Tainted: G           O
        6.17.0-1030-oem #30-Ubuntu PREEMPT(voluntary)
Tainted: [O]=OOT_MODULE
Hardware name: LENOVO 21XHZDY2CN/21XHZDY2CN, BIOS R3HET22W (1.08 )
        06/24/2026
RIP: 0010:dce_aux_transfer_raw+0x296/0x2e0 [amdgpu]
Code: ff e9 49 ff ff ff 41 c7 04 24 04 00 00 00 eb eb 3c 01
       0f 87 4c f4 34 00 83 e0 01 3c 01 19 c0 83 e0 c0 83 c0 50
       e9 3f fe ff ff <0f> 0b 41 c7 04 24 03 00 00 00 eb c5
       41 c7 04 24 03 00 00 00 eb bb
RSP: 0018:ffffcdd6c53272f8 EFLAGS: 00010246
RAX: 0000000062000000 RBX: ffff8d353020fc80 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffcdd6c5327358 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffffcdd6c53273ac
R13: ffffcdd6c53273b0 R14: 0000000000000001 R15: ffff8d3562690000
FS:  00007b3cc831aac0(0000) GS:ffff8d4c6d669000(0000)
    knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000700f940020f8 CR3: 000000012fe37000 CR4: 0000000000f50ef0
PKRU: 55555554
Call stack:
 <TASK>
 link_aux_transfer_raw+0x48/0x80 [amdgpu]
 dc_link_aux_transfer_raw+0x24/0x40 [amdgpu]
 dm_dp_aux_transfer+0xee/0x2b0 [amdgpu]
 drm_dp_dpcd_access+0xbe/0x160 [drm_display_helper]
 drm_dp_dpcd_write+0xc4/0x120 [drm_display_helper]
 dm_helpers_dp_write_dpcd+0x29/0x60 [amdgpu]
 edp_setup_psr+0x156/0x5a0 [amdgpu]
 dc_link_setup_psr+0x20/0x40 [amdgpu]
 amdgpu_dm_link_setup_psr+0x155/0x1a0 [amdgpu]
 ? dm_write_reg_func+0x47/0xc0 [amdgpu]
 amdgpu_dm_enable_self_refresh+0xaa/0x240 [amdgpu]
 amdgpu_dm_commit_planes+0x636/0x1740 [amdgpu]
 ? manage_dm_interrupts+0xa5/0x280 [amdgpu]
 amdgpu_dm_atomic_commit_tail+0xb04/0x1270 [amdgpu]
 ? __set_output_tf.constprop.0+0xfd/0x1a0 [amdgpu]
 commit_tail+0xc6/0x1b0
 drm_atomic_helper_commit+0x132/0x160
 drm_atomic_commit+0xac/0xf0
 ? __pfx___drm_printfn_info+0x10/0x10
 drm_atomic_helper_set_config+0x82/0xd0
 drm_mode_setcrtc+0x3ff/0x9e0
 ? rmapiMapWithSecInfo+0x230/0x2b0 [nvidia]
 ? __pfx_drm_mode_setcrtc+0x10/0x10
 drm_ioctl_kernel+0xb4/0x110
 drm_ioctl+0x2ec/0x5b0
 ? __pfx_drm_mode_setcrtc+0x10/0x10
 amdgpu_drm_ioctl+0x4b/0xa0 [amdgpu]
 __x64_sys_ioctl+0xa2/0x100
 x64_sys_call+0x1226/0x2680
 do_syscall_64+0x80/0x8b0
 ? check_heap_object+0x17f/0x1c0
 ? nvidia_unlocked_ioctl+0x175/0x9a0 [nvidia]
 ? __x64_sys_ioctl+0xbf/0x100
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 ? arch_exit_to_user_mode_prepare.isra.0+0xd/0xe0
 ? do_syscall_64+0xb6/0x8b0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7b3cc8724f1d
Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8
 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8
 64 48 2b 04 25 28 00 00 00
RSP: 002b:00007ffe65c05b80 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000617240d63e10 RCX: 00007b3cc8724f1d
RDX: 00007ffe65c05c10 RSI: 00000000c06864a2 RDI: 0000000000000010
RBP: 00007ffe65c05bd0 R08: 0000000000000000 R09: 0000617240c4b700
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe65c05c10
R13: 00000000c06864a2 R14: 0000000000000010 R15: 000061723f332b30
 </TASK>
[- end trace 0000000000000000 -]
```

Fix both issues:

  - dpcd_set_link_settings(): add an early `return status;` after each
    failed core_link_write_dpcd() check, so callers see the real
    AUX/DPCD state instead of the last successful write.

  - edp_setup_psr(): short-circuit when link->link_status.link_active
    is false, so we never push PSR configuration over a dead AUX
    channel.

Signed-off-by: Kean Ren <rh_king@163.com>
---
 drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c       | 30 ++++++++++++++++----
 drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c | 11 +++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
index 605bf19dc4f2..f15a5e7ea43a 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
@@ -1117,15 +1117,25 @@ enum dc_status dpcd_set_link_settings(
 				link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED;
 	}

+	/* Bail out on the first DPCD write failure so callers can react and
+	 * subsequent operations (e.g. PSR setup) do not keep poking an
+	 * unhealthy AUX channel. Without this, a transient AUX/HPD glitch
+	 * during resume leads to a cascade of DPCD errors and ultimately a
+	 * WARN at dce_aux_transfer_raw() because AUX_SW_DONE never asserts.
+	 */
 	status = core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
-		&downspread.raw, sizeof(downspread));
-	if (status != DC_OK)
+				      &downspread.raw, sizeof(downspread));
+	if (status != DC_OK) {
 		DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_DOWNSPREAD_CTRL) failed\n", __func__, __LINE__);
+		return status;
+	}

 	status = core_link_write_dpcd(link, DP_LANE_COUNT_SET,
-		&lane_count_set.raw, 1);
-	if (status != DC_OK)
+				      &lane_count_set.raw, 1);
+	if (status != DC_OK) {
 		DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LANE_COUNT_SET) failed\n", __func__, __LINE__);
+		return status;
+	}

 	if (link->dpcd_caps.dpcd_rev.raw >= DPCD_REV_13 &&
 			lt_settings->link_settings.use_link_rate_set == true) {
@@ -1141,19 +1151,25 @@ enum dc_status dpcd_set_link_settings(
 					supported_link_rates, sizeof(supported_link_rates));
 		}
 		status = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
-		if (status != DC_OK)
+		if (status != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_BW_SET) failed\n", __func__, __LINE__);
+			return status;
+		}

 		status = core_link_write_dpcd(link, DP_LINK_RATE_SET,
-				&lt_settings->link_settings.link_rate_set, 1);
-		if (status != DC_OK)
+					      &lt_settings->link_settings.link_rate_set, 1);
+		if (status != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_RATE_SET) failed\n", __func__, __LINE__);
+			return status;
+		}
 	} else {
 		rate = get_dpcd_link_rate(&lt_settings->link_settings);

 		status = core_link_write_dpcd(link, DP_LINK_BW_SET, &rate, 1);
-		if (status != DC_OK)
+		if (status != DC_OK) {
 			DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_LINK_BW_SET) failed\n", __func__, __LINE__);
+			return status;
+		}
 	}

 	if (rate) {
diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
index 80a372ceaa51..43a0facc8884 100644
--- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
+++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_edp_panel_control.c
@@ -699,6 +699,17 @@ bool edp_setup_psr(struct dc_link *link,
 	if (!link)
 		return false;

+	/* Skip PSR setup when the eDP link is not active. When AUX/DPCD
+	 * writes are failing (e.g. after a resume where the panel has not
+	 * fully come back yet), edp_setup_psr() will still try to push
+	 * configuration over the AUX channel. That auxiliary transfer never
+	 * completes and triggers ASSERT_CRITICAL() in dce_aux_transfer_raw().
+	 * The DPCD read of the PSR cap below is also unsafe on a dead link,
+	 * so bail out early before touching the sink.
+	 */
+	if (!link->link_status.link_active)
+		return false;
+
 	/* This is a workaround: some vendors require the source to
 	 * read the PSR cap; otherwise, the vendor's PSR feature will
 	 * fall back to its default behavior, causing a misconfiguration
--
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link
  2026-08-18  2:42 ` [PATCH v2] " rh_king
@ 2026-08-18  3:03   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-18  3:03 UTC (permalink / raw)
  To: rh_king; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Callers of `dpcd_set_link_settings()` ignore its return value, turning the new early aborts into a logic flaw that causes link training to fail or fall back to lower bandwidths on transient AUX errors.
--

--- Patch [1]: [PATCH v2] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link ---
commit 3a9a75a9b46189406373e0f0c7bc3d7ed3668c23
Author: Kean Ren <rh_king@163.com>

drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link

This commit addresses a kernel warning caused by PSR AUX timeouts on an
unhealthy eDP link after resume. It adds early returns to
dpcd_set_link_settings() when DPCD writes fail, and adds a check for active
links in edp_setup_psr().

> diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
> --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c
[ ... ]
> @@ -1117,15 +1117,25 @@ enum dc_status dpcd_set_link_settings(
>  				link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED;
>  	}
>  
> +	/* Bail out on the first DPCD write failure so callers can react and
> +	 * subsequent operations (e.g. PSR setup) do not keep poking an
> +	 * unhealthy AUX channel. Without this, a transient AUX/HPD glitch
> +	 * during resume leads to a cascade of DPCD errors and ultimately a
> +	 * WARN at dce_aux_transfer_raw() because AUX_SW_DONE never asserts.
> +	 */
>  	status = core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
> -		&downspread.raw, sizeof(downspread));
> -	if (status != DC_OK)
> +				      &downspread.raw, sizeof(downspread));
> +	if (status != DC_OK) {
>  		DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_DOWNSPREAD_CTRL) failed\n", __func__, __LINE__);
> +		return status;
> +	}

[Severity: High]
By returning early here, do we cause link training to permanently fall back
to lower bandwidths if a transient AUX error occurs?

It looks like the callers of dpcd_set_link_settings() do not check its
return value. For example, in dp_perform_8b_10b_link_training():

    else
        dpcd_set_link_settings(link, lt_settings);
    
    if (lt_settings->lttpr_mode == LTTPR_MODE_NON_TRANSPARENT) {

And similarly in dp_perform_128b_132b_link_training():

    dpcd_set_link_settings(link, lt_settings);
    
    if (result == LINK_TRAINING_SUCCESS) {
        result = dp_perform_128b_132b_channel_eq_done_sequence(...);

If a transient error occurs on the first DPCD write, dpcd_set_link_settings()
now bails out early and skips critical configurations like DP_LANE_COUNT_SET
and DP_LINK_BW_SET. 

Because the callers ignore the return value, they will blindly proceed with
clock recovery on a sink that was not fully configured. Does this guarantee
a CR lock failure, which triggers the driver's fallback logic to permanently
downgrade the link bandwidth?

Should the callers be updated to handle the error return from
dpcd_set_link_settings() so they can abort or retry properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818024219.2921012-1-rh_king@163.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-18  3:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17  3:16 [PATCH] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link rh_king
2026-08-17  3:21 ` sashiko-bot
2026-08-18  2:42 ` [PATCH v2] " rh_king
2026-08-18  3:03   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.