From: Harry Wentland <harry.wentland@xxxxxxx>
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@xxxxxxx>
Acked-by: Aurabindo Pillai <aurabindo.pillai@xxxxxxx>
Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
Tested-by: Daniel Wheeler <daniel.wheeler@xxxxxxx>
---
.../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 a5dc0081d4b6..7481440ab124 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8964,7 +8964,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 49531434f9cf..3c03f690852c 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 d3201b0b3a09..3ac1fec4bf53 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 3d0adf8838ca..8dd65a95d84b 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 40391dd16944..701001ae99be 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 */
--
2.44.0
