Commit 452575c5 authored by Nicholas Kazlauskas's avatar Nicholas Kazlauskas Committed by Alex Deucher

drm/amd/display: Lock the CRTC when setting CRC source

[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: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: default avatarDavid Francis <David.Francis@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 57638021
...@@ -136,13 +136,13 @@ int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc, ...@@ -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) 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; struct drm_dp_aux *aux = NULL;
bool enable = false; bool enable = false;
bool enabled = false; bool enabled = false;
int ret; int ret = 0;
enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name);
if (source < 0) { if (source < 0) {
DRM_DEBUG_DRIVER("Unknown CRC source %s for CRTC%d\n", 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) ...@@ -150,7 +150,33 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
return -EINVAL; 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); enable = amdgpu_dm_is_valid_crc_source(source);
crtc_state = to_dm_crtc_state(crtc->state);
/* /*
* USER REQ SRC | CURRENT SRC | BEHAVIOR * 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) ...@@ -184,19 +210,23 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
if (!aconn) { if (!aconn) {
DRM_DEBUG_DRIVER("No amd connector matching CRTC-%d\n", crtc->index); DRM_DEBUG_DRIVER("No amd connector matching CRTC-%d\n", crtc->index);
return -EINVAL; ret = -EINVAL;
goto cleanup;
} }
aux = &aconn->dm_dp_aux.aux; aux = &aconn->dm_dp_aux.aux;
if (!aux) { if (!aux) {
DRM_DEBUG_DRIVER("No dp aux for amd connector\n"); 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)) if (amdgpu_dm_crtc_configure_crc_source(crtc, crtc_state, source)) {
return -EINVAL; ret = -EINVAL;
goto cleanup;
}
/* /*
* Reading the CRC requires the vblank interrupt handler to be * 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) ...@@ -206,12 +236,13 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
if (!enabled && enable) { if (!enabled && enable) {
ret = drm_crtc_vblank_get(crtc); ret = drm_crtc_vblank_get(crtc);
if (ret) if (ret)
return ret; goto cleanup;
if (dm_is_crc_source_dprx(source)) { if (dm_is_crc_source_dprx(source)) {
if (drm_dp_start_crc(aux, crtc)) { if (drm_dp_start_crc(aux, crtc)) {
DRM_DEBUG_DRIVER("dp start crc failed\n"); DRM_DEBUG_DRIVER("dp start crc failed\n");
return -EINVAL; ret = -EINVAL;
goto cleanup;
} }
} }
} else if (enabled && !enable) { } else if (enabled && !enable) {
...@@ -219,7 +250,8 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) ...@@ -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 (dm_is_crc_source_dprx(source)) {
if (drm_dp_stop_crc(aux)) { if (drm_dp_stop_crc(aux)) {
DRM_DEBUG_DRIVER("dp stop crc failed\n"); 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) ...@@ -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 */ /* Reset crc_skipped on dm state */
crtc_state->crc_skip_count = 0; crtc_state->crc_skip_count = 0;
return 0;
cleanup:
if (commit)
drm_crtc_commit_put(commit);
drm_modeset_unlock(&crtc->mutex);
return ret;
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment