Re: [BUG] thunderbolt: NULL pointer dereference in tb_dp_dprx_work after device disconnect (7.2.6)

[Mario Limonciello <mario.limonciello@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]



On 10/19/26 12:43, Paweł Frelek wrote:
> pon., 19 paź 2026 o 08:11 Mika Westerberg
> <mika.westerberg@linux.intel.com> napisał(a):
>>
>> Hi,
> Hi
>> I suggest to take the just released v7.3 and see if the problem still
>> persist.
> 
> Tested v7.3 (Arch config, localmodconfig). The recipe that hung 3 out
> of 3 times on 7.2.9 (monitors off via DPMS, back on, reboot ~10 s
> later) now passes 3 out of 3: no hang in nhi_pci_remove, nothing in
> pstore. So it looks fixed by 7.3, thanks!
> 
> The amdgpu side still logs this on every monitor wake through the
> dock, but that’s a separate thing for the AMD folks:
> 
> WARNING: drivers/gpu/drm/amd/amdgpu/../display/dc/link/hwss/link_hwss_dpia.c:52
> at update_dpia_stream_allocation_table+0x120/0x130 [amdgpu]
> amdgpu 0000:73:00.0: [drm] ERROR Sending link address failed with -5

I have a few patches I was going to propose for this area.  Can you see 
if these help?


[-- Attachment #2: 0001-drm-amd-display-Release-the-IRQ-owned-stream-during-.patch --]
[-- Type: text/x-patch, Size: 5249 bytes --]

From 59addbdc42d111e8a1e9b5b241900ebdebce25b2 Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@amd.com>
Date: Wed, 14 Oct 2026 11:23:48 -0500
Subject: [PATCH 1/3] drm/amd/display: Release the IRQ-owned stream during
 teardown

amdgpu_dm_commit_streams() drops dm_old_crtc_state->stream when disabling
interrupts to balance the reference retained in
acrtc->dm_irq_params.stream.

Those pointers normally alias, but a seamless-boot CRTC can be active
before the IRQ stream reference has been acquired. If the link disappears
during early boot, teardown releases an unowned reference. Releasing the
remaining DC state later then underflows the stream refcount and corrupts
the slab.

Release the stream actually stored in dm_irq_params instead, and clear the
pointer so it cannot remain stale. Add tests for both populated and absent
IRQ stream ownership.

Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5981
Fixes: 585d450c76d1 ("drm/amd/display: Refactor to prevent crtc state access in DM IRQ handler")
Cc: stable@vger.kernel.org
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  1 +
 .../display/amdgpu_dm/tests/amdgpu_dm_test.c  | 39 +++++++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c4fee73a02f91..1b7ac5070ad9d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3488,6 +3488,16 @@ STATIC_IFN_KUNIT void manage_dm_interrupts(struct amdgpu_device *adev,
 }
 EXPORT_IF_KUNIT(manage_dm_interrupts);
 
+STATIC_IFN_KUNIT void amdgpu_dm_release_irq_stream(struct amdgpu_crtc *acrtc)
+{
+	struct dc_stream_state *stream = acrtc->dm_irq_params.stream;
+
+	acrtc->dm_irq_params.stream = NULL;
+	dc_stream_release(stream);
+}
+
+EXPORT_IF_KUNIT(amdgpu_dm_release_irq_stream);
+
 STATIC_IFN_KUNIT void dm_update_pflip_irq_state(struct amdgpu_device *adev,
 						struct amdgpu_crtc *acrtc)
 {
@@ -4632,7 +4642,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
 			 * stream is released.
 			 */
 			amdgpu_dm_ism_flush(&acrtc->ism);
-			dc_stream_release(dm_old_crtc_state->stream);
+			amdgpu_dm_release_irq_stream(acrtc);
 		}
 	}
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 594d275918d5e..569c782c866fa 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -1353,6 +1353,7 @@ int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_commit *state, struct dc_
 				     struct dsc_mst_fairness_vars *vars);
 void manage_dm_interrupts(struct amdgpu_device *adev, struct amdgpu_crtc *acrtc,
 			  struct dm_crtc_state *acrtc_state);
+void amdgpu_dm_release_irq_stream(struct amdgpu_crtc *acrtc);
 void amdgpu_dm_enable_self_refresh(struct amdgpu_display_manager *dm,
 				   struct amdgpu_crtc *acrtc_attach,
 				   const struct dm_crtc_state *acrtc_state, const u64 current_ts);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
index a497cce332bb3..ff61093443392 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
@@ -4298,6 +4298,43 @@ static void dm_test_manage_interrupts_disable(struct kunit *test)
 	KUNIT_EXPECT_FALSE(test, ctx->adev->ddev.vblank[0].enabled);
 }
 
+/**
+ * dm_test_release_irq_stream - Test the IRQ-owned stream reference is released
+ * @test: KUnit test context
+ */
+static void dm_test_release_irq_stream(struct kunit *test)
+{
+	struct dc_stream_state *stream = dm_kunit_alloc_stream(test, NULL);
+	struct amdgpu_crtc *acrtc;
+
+	acrtc = kunit_kzalloc(test, sizeof(*acrtc), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, acrtc);
+
+	kref_get(&stream->refcount);
+	acrtc->dm_irq_params.stream = stream;
+
+	amdgpu_dm_release_irq_stream(acrtc);
+
+	KUNIT_EXPECT_NULL(test, acrtc->dm_irq_params.stream);
+	KUNIT_EXPECT_EQ(test, kref_read(&stream->refcount), 1);
+}
+
+/**
+ * dm_test_release_irq_stream_unset - Test release without IRQ stream ownership
+ * @test: KUnit test context
+ */
+static void dm_test_release_irq_stream_unset(struct kunit *test)
+{
+	struct amdgpu_crtc *acrtc;
+
+	acrtc = kunit_kzalloc(test, sizeof(*acrtc), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, acrtc);
+
+	amdgpu_dm_release_irq_stream(acrtc);
+
+	KUNIT_EXPECT_NULL(test, acrtc->dm_irq_params.stream);
+}
+
 /* Tests for dm_early_init() */
 
 #define DM_TEST_ATOM_BIOS_SIZE	512
@@ -5409,6 +5446,8 @@ static struct kunit_case amdgpu_dm_tests[] = {
 	KUNIT_CASE(dm_test_manage_interrupts_offdelay_fallback),
 	KUNIT_CASE(dm_test_manage_interrupts_apu_instant_off),
 	KUNIT_CASE(dm_test_manage_interrupts_disable),
+	KUNIT_CASE(dm_test_release_irq_stream),
+	KUNIT_CASE(dm_test_release_irq_stream_unset),
 	/* dm_early_init */
 	KUNIT_CASE(dm_test_early_init_no_object_header),
 	KUNIT_CASE(dm_test_early_init_legacy_asics),
-- 
2.43.0


[-- Attachment #3: 0002-drm-amd-display-Skip-stale-DPIA-slot-updates-after-d.patch --]
[-- Type: text/x-patch, Size: 2680 bytes --]

From 25141b19306cf22f85ae359250027ccf58cef821 Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@amd.com>
Date: Wed, 14 Oct 2026 15:46:51 -0500
Subject: [PATCH 2/3] drm/amd/display: Skip stale DPIA slot updates after
 disconnect

DMUB discards a DPIA tunnel allocation when HPD goes low, before
the hotplug atomic commit disables the old MST streams. DC removes
those streams one at a time, so the first removal can send an
intermediate non-zero slot total for a topology that no longer exists.
DMUB rejects that total because its slot state is already zero.

Do not send non-zero slot updates once the link is no longer an MST
branch. Keep the final zero update so firmware is released even when it
did not clear the allocation, and continue programming the link encoder
table for every stream removal.

Fixes: 139a33112f17 ("drm/amd/display: MST support for DPIA")

Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5981

Cc: stable@vger.kernel.org
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../amd/display/dc/link/hwss/link_hwss_dpia.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dpia.c b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dpia.c
index 2a87b23582f34..3645f638f6b8b 100644
--- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dpia.c
+++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dpia.c
@@ -47,11 +47,20 @@ static void update_dpia_stream_allocation_table(struct dc_link *link,
 	for (i = 0; i < table->stream_count; i++)
 		mst_alloc_slots += table->stream_allocations[i].slot_count;
 
-	status = dc_process_dmub_set_mst_slots(link->dc, link->link_index,
-			mst_alloc_slots, &prev_mst_slots_in_use);
-	ASSERT(status == DC_OK);
-	DC_LOG_MST("dpia : status[%d]: alloc_slots[%d]: used_slots[%d]\n",
-			status, mst_alloc_slots, prev_mst_slots_in_use);
+	/*
+	 * Once the MST topology is gone, DMUB no longer has the old stream
+	 * allocations. Avoid sending intermediate non-zero totals while DC
+	 * removes those streams one at a time, but still send the final zero
+	 * in case firmware has not cleared the allocation yet.
+	 */
+	if (link->type == dc_connection_mst_branch || !mst_alloc_slots) {
+		status = dc_process_dmub_set_mst_slots(link->dc, link->link_index,
+						       mst_alloc_slots,
+						       &prev_mst_slots_in_use);
+		ASSERT(status == DC_OK);
+		DC_LOG_MST("dpia : status[%d]: alloc_slots[%d]: used_slots[%d]\n",
+			   status, mst_alloc_slots, prev_mst_slots_in_use);
+	}
 
 	if (link_enc)
 		link_enc->funcs->update_mst_stream_allocation_table(link_enc, table);
-- 
2.43.0


[-- Attachment #4: 0003-drm-amd-display-Reject-unallocated-MST-payloads.patch --]
[-- Type: text/x-patch, Size: 8904 bytes --]

From 16dd1c243b5581f6377c72a58cb52232296503df Mon Sep 17 00:00:00 2001
From: Mario Limonciello <mario.limonciello@amd.com>
Date: Wed, 14 Oct 2026 15:51:32 -0500
Subject: [PATCH 3/3] drm/amd/display: Reject unallocated MST payloads

A connector left behind by MST topology churn can reach commit-tail
with an atomic payload that has neither a VCPI nor time slots. The
payload helper currently treats it like an allocated payload, producing
an empty source allocation table and triggering the stream-count
assertion before attempting further hardware programming.

Reject unallocated payloads before calling the DRM payload helpers.
Propagate that failure out of allocate_mst_payload() so source-table
programming, ACT polling, and part 2 are skipped, while completing
post-enable programming with a failed-handshake result to leave DCN
consistent.

Keep tolerating drm_dp_add_payload_part1() communication failures as
required by its commit-tail contract.

Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state")

Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5981

Cc: stable@vger.kernel.org
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  3 ++
 .../amdgpu_dm/tests/amdgpu_dm_helpers_test.c  | 45 ++++++++++++++-----
 .../gpu/drm/amd/display/dc/link/link_dpms.c   | 31 ++++++-------
 3 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 3217aa82bea24..70f211a8775d4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -397,6 +397,9 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
 	new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
 
 	if (enable) {
+		if (!new_payload->vcpi || !new_payload->time_slots)
+			return false;
+
 		target_payload = new_payload;
 
 		/* It's OK for this to fail */
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c
index d37fe26eecea4..3cf7c2eb8da66 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c
@@ -3369,21 +3369,23 @@ static void dm_test_construct_old_payload_intervening(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, old_payload->pbn, 15);
 }
 
-/* Tests for dm_helpers_dp_mst_write_payload_allocation_table() success path */
+/* Tests for dm_helpers_dp_mst_write_payload_allocation_table() */
 
 /**
- * dm_test_write_payload_alloc_table_success - Exercise the MST success path
+ * dm_test_write_payload_alloc_table - Exercise MST payload table updates
  * @test: The KUnit test context
  * @enable: true for the add-payload path, false for the remove-payload path
+ * @valid_payload: true when the payload has an allocated VCPI and slots
  *
  * Builds a minimal MST topology-state fixture so the helper traverses past the
  * early NULL checks and runs the real DRM MST payload helpers
  * (drm_dp_add_payload_part1 / drm_dp_remove_payload_part1). With
- * mgr->mst_primary == NULL the topology walk fails gracefully, so no remote
- * DPCD/AUX traffic is generated, and the helper still returns true.
+ * mgr->mst_primary == NULL the topology walk fails without remote DPCD/AUX
+ * traffic. Such communication failures are tolerated for valid payloads so
+ * commit-tail can finish local hardware programming.
  */
-static void dm_test_write_payload_alloc_table_success(struct kunit *test,
-						      bool enable)
+static void dm_test_write_payload_alloc_table(struct kunit *test, bool enable,
+					      bool valid_payload)
 {
 	struct amdgpu_device *adev;
 	struct amdgpu_dm_connector *aconnector;
@@ -3427,9 +3429,9 @@ static void dm_test_write_payload_alloc_table_success(struct kunit *test,
 
 	/* payload found by drm_atomic_get_mst_payload_state via matching port */
 	payload->port = port;
-	payload->vcpi = 1;
+	payload->vcpi = valid_payload ? 1 : 0;
 	payload->vc_start_slot = 1;
-	payload->time_slots = 2;
+	payload->time_slots = valid_payload ? 2 : 0;
 	list_add_tail(&payload->next, &mst_state->payloads);
 
 	/* pre-existing HW allocation so the disable path finds a VCPI to clear */
@@ -3443,6 +3445,17 @@ static void dm_test_write_payload_alloc_table_success(struct kunit *test,
 	ret = dm_helpers_dp_mst_write_payload_allocation_table(NULL, stream,
 							       &table, enable);
 
+	if (enable && !valid_payload) {
+		KUNIT_EXPECT_FALSE(test, ret);
+		KUNIT_EXPECT_EQ(test, table.stream_count, 0);
+		KUNIT_EXPECT_EQ(test, mgr->payload_count, 0);
+		KUNIT_EXPECT_EQ(test, mgr->next_start_slot, 0);
+		KUNIT_EXPECT_EQ(test, payload->vc_start_slot, 1);
+		KUNIT_EXPECT_EQ(test, payload->payload_allocation_status,
+				DRM_DP_MST_PAYLOAD_ALLOCATION_NONE);
+		return;
+	}
+
 	KUNIT_EXPECT_TRUE(test, ret);
 
 	if (enable)
@@ -3459,7 +3472,16 @@ static void dm_test_write_payload_alloc_table_success(struct kunit *test,
  */
 static void dm_test_write_payload_alloc_table_enable(struct kunit *test)
 {
-	dm_test_write_payload_alloc_table_success(test, true);
+	dm_test_write_payload_alloc_table(test, true, true);
+}
+
+/**
+ * dm_test_write_payload_alloc_table_enable_unallocated - Reject unallocated payload
+ * @test: The KUnit test context
+ */
+static void dm_test_write_payload_alloc_table_enable_unallocated(struct kunit *test)
+{
+	dm_test_write_payload_alloc_table(test, true, false);
 }
 
 /**
@@ -3468,7 +3490,7 @@ static void dm_test_write_payload_alloc_table_enable(struct kunit *test)
  */
 static void dm_test_write_payload_alloc_table_disable(struct kunit *test)
 {
-	dm_test_write_payload_alloc_table_success(test, false);
+	dm_test_write_payload_alloc_table(test, false, true);
 }
 
 /* Tests for dm_helpers_dp_mst_poll_for_allocation_change_trigger() */
@@ -4946,8 +4968,9 @@ static struct kunit_case amdgpu_dm_helpers_test_cases[] = {
 	KUNIT_CASE(dm_test_smu_timeout_no_crash),
 	KUNIT_CASE(dm_test_set_phyd32clk_no_crash),
 	KUNIT_CASE(dm_test_mst_update_branch_bandwidth_no_crash),
-	/* dm_helpers_dp_mst_write_payload_allocation_table success path */
+	/* dm_helpers_dp_mst_write_payload_allocation_table */
 	KUNIT_CASE(dm_test_write_payload_alloc_table_enable),
+	KUNIT_CASE(dm_test_write_payload_alloc_table_enable_unallocated),
 	KUNIT_CASE(dm_test_write_payload_alloc_table_disable),
 	/* MST null-connector early returns */
 	KUNIT_CASE(dm_test_mst_write_payload_alloc_table_null_ctx),
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index 42a47d12d1f38..652645619d3d9 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -1462,19 +1462,17 @@ static enum dc_status allocate_mst_payload(struct pipe_ctx *pipe_ctx)
 	 */
 
 	/* get calculate VC payload for stream: stream_alloc */
-	if (dm_helpers_dp_mst_write_payload_allocation_table(
-		stream->ctx,
-		stream,
-		&proposed_table,
-		true))
-		update_mst_stream_alloc_table(
-					link,
-					pipe_ctx->stream_res.stream_enc,
-					pipe_ctx->stream_res.hpo_dp_stream_enc,
-					&proposed_table);
-	else
+	if (!dm_helpers_dp_mst_write_payload_allocation_table(stream->ctx, stream,
+							      &proposed_table,
+							      true)) {
 		DC_LOG_WARNING("Failed to update MST allocation table for idx %d\n",
 				pipe_ctx->pipe_idx);
+		return DC_FAIL_DP_PAYLOAD_ALLOCATION;
+	}
+
+	update_mst_stream_alloc_table(link, pipe_ctx->stream_res.stream_enc,
+				      pipe_ctx->stream_res.hpo_dp_stream_enc,
+				      &proposed_table);
 
 	print_mst_streams(link);
 
@@ -2754,6 +2752,8 @@ static enum dc_status link_set_dpms_on_post_enable_link(
 		struct pipe_ctx *pipe_ctx
 )
 {
+	enum dc_status result = DC_DPMS_SUCCESS;
+
 	// Used conditionally in ifdef'ed diagnostic builds
 	(void) state;
 
@@ -2803,9 +2803,10 @@ static enum dc_status link_set_dpms_on_post_enable_link(
 	if (pipe_ctx->link_config.dp_tunnel_settings.should_use_dp_bw_allocation)
 		allocate_usb4_bandwidth(stream);
 
-	if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
-		allocate_mst_payload(pipe_ctx);
-	else if (dc_is_dp_sst_signal(stream->signal) &&
+	if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
+		if (allocate_mst_payload(pipe_ctx) != DC_OK)
+			result = DC_DPMS_FAILED_HANDSHAKE;
+	} else if (dc_is_dp_sst_signal(stream->signal) &&
 			dp_is_128b_132b_signal(pipe_ctx))
 		update_sst_payload(pipe_ctx, true);
 
@@ -2833,7 +2834,7 @@ static enum dc_status link_set_dpms_on_post_enable_link(
 	if (dc_is_hdmi_signal(stream->signal))
 		set_avmute(pipe_ctx, false);
 
-	return DC_DPMS_SUCCESS;
+	return result;
 }
 
 enum dc_status link_set_dpms_on(
-- 
2.43.0