Blob Blame History Raw
From 47a959d337d07d5a76f9a5a430be7a495620f2a7 Mon Sep 17 00:00:00 2001
From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Date: Mon, 28 Feb 2022 10:48:56 -0500
Subject: drm/amd/display: Fix double free during GPU reset on DC streams
Git-commit: 32685b32d825ca08c5dec826477332df886c4743
Patch-mainline: v5.18-rc1
References: jsc#PED-1166 jsc#PED-1168 jsc#PED-1170 jsc#PED-1218 jsc#PED-1220 jsc#PED-1222 jsc#PED-1223 jsc#PED-1225

[Why]
The issue only occurs during the GPU reset code path.

We first backup the current state prior to commiting 0 streams
internally from DM to DC. This state backup contains valid link
encoder assignments.

DC will clear the link encoder assignments as part of current state
(but not the backup, since it was a copied before the commit) and
free the extra stream reference it held.

DC requires that the link encoder assignments remain cleared/invalid
prior to commiting. Since the backup still has valid assignments we
call the interface post reset to clear them. This routine also
releases the extra reference that the link encoder interface held -
resulting in a double free (and eventually a NULL pointer dereference).

[How]
We'll have to do a full DC commit anyway after GPU reset because
the stream count previously went to 0.

We don't need to retain the assignment that we had backed up, so
just copy off of the now clean current state assignment after the
reset has occcurred with the new link_enc_cfg_copy() interface.

Fixes: 6d63fcc2a334 ("drm/amd/display: Reset link encoder assignments for GPU reset")

Reviewed-by: Jimmy Kizito <Jimmy.Kizito@amd.com>
Acked-by: Alan Liu <HaoPing.Liu@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c     | 9 ++++++---
 drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c | 7 +++++++
 drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h     | 5 +++++
 3 files changed, 18 insertions(+), 3 deletions(-)

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 e1d3db3fe8de..a47370058088 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2633,10 +2633,13 @@ static int dm_resume(void *handle)
 		 * before the 0 streams commit.
 		 *
 		 * DC expects that link encoder assignments are *not* valid
-		 * when committing a state, so as a workaround it needs to be
-		 * cleared here.
+		 * when committing a state, so as a workaround we can copy
+		 * off of the current state.
+		 *
+		 * We lose the previous assignments, but we had already
+		 * commit 0 streams anyway.
 		 */
-		link_enc_cfg_init(dm->dc, dc_state);
+		link_enc_cfg_copy(adev->dm.dc->current_state, dc_state);
 
 		if (dc_enable_dmub_notifications(adev->dm.dc))
 			amdgpu_dm_outbox_init(adev);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
index 047c626a4a34..42da7f430113 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_enc_cfg.c
@@ -272,6 +272,13 @@ void link_enc_cfg_init(
 	state->res_ctx.link_enc_cfg_ctx.mode = LINK_ENC_CFG_STEADY;
 }
 
+void link_enc_cfg_copy(const struct dc_state *src_ctx, struct dc_state *dst_ctx)
+{
+	memcpy(&dst_ctx->res_ctx.link_enc_cfg_ctx,
+	       &src_ctx->res_ctx.link_enc_cfg_ctx,
+	       sizeof(dst_ctx->res_ctx.link_enc_cfg_ctx));
+}
+
 void link_enc_cfg_link_encs_assign(
 		struct dc *dc,
 		struct dc_state *state,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h b/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
index c2f08514a1d9..c6f6baa6e677 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_enc_cfg.h
@@ -39,6 +39,11 @@ void link_enc_cfg_init(
 		const struct dc *dc,
 		struct dc_state *state);
 
+/*
+ * Copies a link encoder assignment from another state.
+ */
+void link_enc_cfg_copy(const struct dc_state *src_ctx, struct dc_state *dst_ctx);
+
 /*
  * Algorithm for assigning available DIG link encoders to streams.
  *
-- 
2.38.1