From f63f86b5affcc2abd1162f11824b9386fc06ac94 Mon Sep 17 00:00:00 2001
From: Harry Wentland <harry.wentland@amd.com>
Date: Fri, 15 Mar 2024 11:19:15 -0400
Subject: [PATCH] drm/amd/display: Separate setting and programming of cursor
We're seeing issues when user-space tries to do an atomic update of
the primary surface, as well as the cursor. These two updates are
separate calls into DC and don't currently act as an atomic update.
This might lead to cursor updates being locked out and cursors
stuttering.
In order to solve this problem we want to separate the setting
and programming of cursor attributes and position. That's what
we're doing in this patch. The subsequent patch will then be
able to use the cursor setters in independent cursor updates,
as well as in atomic commits.
Reviewed-by: Agustin Gutierrez <agustin.gutierrez@amd.com>
Acked-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
.../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 6 +-
.../gpu/drm/amd/display/dc/core/dc_stream.c | 87 +++++++++++++------
drivers/gpu/drm/amd/display/dc/dc_stream.h | 8 ++
.../amd/display/dc/hwss/dcn30/dcn30_hwseq.c | 2 +-
5 files changed, 72 insertions(+), 33 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 3bd3a218b6046..01bcccc585008 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8963,7 +8963,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state,
memset(&position, 0, sizeof(position));
mutex_lock(&dm->dc_lock);
- dc_stream_set_cursor_position(dm_old_crtc_state->stream, &position);
+ dc_stream_program_cursor_position(dm_old_crtc_state->stream, &position);
mutex_unlock(&dm->dc_lock);
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 49531434f9cf4..3c03f690852cb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1269,7 +1269,7 @@ void amdgpu_dm_plane_handle_cursor_update(struct drm_plane *plane,
/* turn off cursor */
if (crtc_state && crtc_state->stream) {
mutex_lock(&adev->dm.dc_lock);
- dc_stream_set_cursor_position(crtc_state->stream,
+ dc_stream_program_cursor_position(crtc_state->stream,
&position);
mutex_unlock(&adev->dm.dc_lock);
}
@@ -1299,11 +1299,11 @@ void amdgpu_dm_plane_handle_cursor_update(struct drm_plane *plane,
if (crtc_state->stream) {
mutex_lock(&adev->dm.dc_lock);
- if (!dc_stream_set_cursor_attributes(crtc_state->stream,
+ if (!dc_stream_program_cursor_attributes(crtc_state->stream,
&attributes))
DRM_ERROR("DC failed to set cursor attributes\n");
- if (!dc_stream_set_cursor_position(crtc_state->stream,
+ if (!dc_stream_program_cursor_position(crtc_state->stream,
&position))
DRM_ERROR("DC failed to set cursor position\n");
mutex_unlock(&adev->dm.dc_lock);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index d3201b0b3a09b..3ac1fec4bf53b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -268,7 +268,6 @@ bool dc_stream_set_cursor_attributes(
const struct dc_cursor_attributes *attributes)
{
struct dc *dc;
- bool reset_idle_optimizations = false;
if (NULL == stream) {
dm_error("DC: dc_stream is NULL!\n");
@@ -299,20 +298,36 @@ bool dc_stream_set_cursor_attributes(
stream->cursor_attributes = *attributes;
- dc_z10_restore(dc);
- /* disable idle optimizations while updating cursor */
- if (dc->idle_optimizations_allowed) {
- dc_allow_idle_optimizations(dc, false);
- reset_idle_optimizations = true;
- }
+ return true;
+}
- program_cursor_attributes(dc, stream, attributes);
+bool dc_stream_program_cursor_attributes(
+ struct dc_stream_state *stream,
+ const struct dc_cursor_attributes *attributes)
+{
+ struct dc *dc;
+ bool reset_idle_optimizations = false;
- /* re-enable idle optimizations if necessary */
- if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle)
- dc_allow_idle_optimizations(dc, true);
+ dc = stream ? stream->ctx->dc : NULL;
- return true;
+ if (dc_stream_set_cursor_attributes(stream, attributes)) {
+ dc_z10_restore(dc);
+ /* disable idle optimizations while updating cursor */
+ if (dc->idle_optimizations_allowed) {
+ dc_allow_idle_optimizations(dc, false);
+ reset_idle_optimizations = true;
+ }
+
+ program_cursor_attributes(dc, stream, attributes);
+
+ /* re-enable idle optimizations if necessary */
+ if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle)
+ dc_allow_idle_optimizations(dc, true);
+
+ return true;
+ }
+
+ return false;
}
static void program_cursor_position(
@@ -357,9 +372,6 @@ bool dc_stream_set_cursor_position(
struct dc_stream_state *stream,
const struct dc_cursor_position *position)
{
- struct dc *dc;
- bool reset_idle_optimizations = false;
-
if (NULL == stream) {
dm_error("DC: dc_stream is NULL!\n");
return false;
@@ -370,24 +382,43 @@ bool dc_stream_set_cursor_position(
return false;
}
+ stream->cursor_position = *position;
+
+
+ return true;
+}
+
+bool dc_stream_program_cursor_position(
+ struct dc_stream_state *stream,
+ const struct dc_cursor_position *position)
+{
+ struct dc *dc;
+ bool reset_idle_optimizations = false;
+ const struct dc_cursor_position *old_position;
+
+ old_position = stream ? &stream->cursor_position : NULL;
dc = stream->ctx->dc;
- dc_z10_restore(dc);
- /* disable idle optimizations if enabling cursor */
- if (dc->idle_optimizations_allowed && (!stream->cursor_position.enable || dc->debug.exit_idle_opt_for_cursor_updates)
- && position->enable) {
- dc_allow_idle_optimizations(dc, false);
- reset_idle_optimizations = true;
- }
+ if (dc_stream_set_cursor_position(stream, position)) {
+ dc_z10_restore(dc);
- stream->cursor_position = *position;
+ /* disable idle optimizations if enabling cursor */
+ if (dc->idle_optimizations_allowed &&
+ (!old_position->enable || dc->debug.exit_idle_opt_for_cursor_updates) &&
+ position->enable) {
+ dc_allow_idle_optimizations(dc, false);
+ reset_idle_optimizations = true;
+ }
- program_cursor_position(dc, stream, position);
- /* re-enable idle optimizations if necessary */
- if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle)
- dc_allow_idle_optimizations(dc, true);
+ program_cursor_position(dc, stream, position);
+ /* re-enable idle optimizations if necessary */
+ if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle)
+ dc_allow_idle_optimizations(dc, true);
- return true;
+ return true;
+ }
+
+ return false;
}
bool dc_stream_add_writeback(struct dc *dc,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 3d0adf8838cac..8dd65a95d84b6 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -484,10 +484,18 @@ bool dc_stream_set_cursor_attributes(
struct dc_stream_state *stream,
const struct dc_cursor_attributes *attributes);
+bool dc_stream_program_cursor_attributes(
+ struct dc_stream_state *stream,
+ const struct dc_cursor_attributes *attributes);
+
bool dc_stream_set_cursor_position(
struct dc_stream_state *stream,
const struct dc_cursor_position *position);
+bool dc_stream_program_cursor_position(
+ struct dc_stream_state *stream,
+ const struct dc_cursor_position *position);
+
bool dc_stream_adjust_vmin_vmax(struct dc *dc,
struct dc_stream_state *stream,
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
index ed9141a67db37..29d1f150846a4 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
@@ -1038,7 +1038,7 @@ bool dcn30_apply_idle_power_optimizations(struct dc *dc, bool enable)
/* Use copied cursor, and it's okay to not switch back */
cursor_attr.address.quad_part = cmd.mall.cursor_copy_dst.quad_part;
- dc_stream_set_cursor_attributes(stream, &cursor_attr);
+ dc_stream_program_cursor_attributes(stream, &cursor_attr);
}
/* Enable MALL */
--
GitLab