Blob Blame History Raw
From 452575c5703cd7d0cf285ef7a97de52af9a3d70d Mon Sep 17 00:00:00 2001
From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Date: Tue, 20 Aug 2019 13:15:25 -0400
Subject: drm/amd/display: Lock the CRTC when setting CRC source
Git-commit: 452575c5703cd7d0cf285ef7a97de52af9a3d70d
Patch-mainline: v5.4-rc1
References: bsc#1152489

[Why]
We need to ensure that we're holding the lock on the CRTC when setting
the CRC source since we're modifying the CRTC state directly.

We also need to wait for any outstanding non-blocking commits to finish
so they aren't reading state that's potentially being modified -
non-blocking commits don't hold the CRTC lock while doing commit tail
work.

[How]
Lock the CRTC using its mutex. While holding the lock check if there's
any commit active on the CRTC - if there is, it's non-blocking and
we should wait until it's finished by waiting for hw_done to be
signaled since that's the last point where we touch CRTC state.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: David Francis <David.Francis@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 63 +++++++++++++++----
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
index d2cdf3002365..a549c7c717dd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
@@ -136,13 +136,13 @@ int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc,
 
 int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 {
-	struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state);
+	enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name);
+	struct drm_crtc_commit *commit;
+	struct dm_crtc_state *crtc_state;
 	struct drm_dp_aux *aux = NULL;
 	bool enable = false;
 	bool enabled = false;
-	int ret;
-
-	enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name);
+	int ret = 0;
 
 	if (source < 0) {
 		DRM_DEBUG_DRIVER("Unknown CRC source %s for CRTC%d\n",
@@ -150,7 +150,33 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 		return -EINVAL;
 	}
 
+	ret = drm_modeset_lock(&crtc->mutex, NULL);
+	if (ret)
+		return ret;
+
+	spin_lock(&crtc->commit_lock);
+	commit = list_first_entry_or_null(&crtc->commit_list,
+					  struct drm_crtc_commit, commit_entry);
+	if (commit)
+		drm_crtc_commit_get(commit);
+	spin_unlock(&crtc->commit_lock);
+
+	if (commit) {
+		/*
+		 * Need to wait for all outstanding programming to complete
+		 * in commit tail since it can modify CRC related fields and
+		 * hardware state. Since we're holding the CRTC lock we're
+		 * guaranteed that no other commit work can be queued off
+		 * before we modify the state below.
+		 */
+		ret = wait_for_completion_interruptible_timeout(
+			&commit->hw_done, 10 * HZ);
+		if (ret)
+			goto cleanup;
+	}
+
 	enable = amdgpu_dm_is_valid_crc_source(source);
+	crtc_state = to_dm_crtc_state(crtc->state);
 
 	/*
 	 * USER REQ SRC | CURRENT SRC | BEHAVIOR
@@ -184,19 +210,23 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 
 		if (!aconn) {
 			DRM_DEBUG_DRIVER("No amd connector matching CRTC-%d\n", crtc->index);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto cleanup;
 		}
 
 		aux = &aconn->dm_dp_aux.aux;
 
 		if (!aux) {
 			DRM_DEBUG_DRIVER("No dp aux for amd connector\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto cleanup;
 		}
 	}
 
-	if (amdgpu_dm_crtc_configure_crc_source(crtc, crtc_state, source))
-		return -EINVAL;
+	if (amdgpu_dm_crtc_configure_crc_source(crtc, crtc_state, source)) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
 
 	/*
 	 * Reading the CRC requires the vblank interrupt handler to be
@@ -206,12 +236,13 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 	if (!enabled && enable) {
 		ret = drm_crtc_vblank_get(crtc);
 		if (ret)
-			return ret;
+			goto cleanup;
 
 		if (dm_is_crc_source_dprx(source)) {
 			if (drm_dp_start_crc(aux, crtc)) {
 				DRM_DEBUG_DRIVER("dp start crc failed\n");
-				return -EINVAL;
+				ret = -EINVAL;
+				goto cleanup;
 			}
 		}
 	} else if (enabled && !enable) {
@@ -219,7 +250,8 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 		if (dm_is_crc_source_dprx(source)) {
 			if (drm_dp_stop_crc(aux)) {
 				DRM_DEBUG_DRIVER("dp stop crc failed\n");
-				return -EINVAL;
+				ret = -EINVAL;
+				goto cleanup;
 			}
 		}
 	}
@@ -228,7 +260,14 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 
 	/* Reset crc_skipped on dm state */
 	crtc_state->crc_skip_count = 0;
-	return 0;
+
+cleanup:
+	if (commit)
+		drm_crtc_commit_put(commit);
+
+	drm_modeset_unlock(&crtc->mutex);
+
+	return ret;
 }
 
 /**
-- 
2.28.0