drm/amd/display: Separate setting and programming of cursor

commit f63f86b5affcc2abd1162f11824b9386fc06ac94 upstream.

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>
Cc: Kevin Holm <kevin@holm.dev>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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 09b66c839b1c2eb1fcb9b2234b343d92448b1169..adfe653783df9d55c18e685ccede739f33c53d58 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8947,7 +8947,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 8a4c40b4c27e4f293be7bc1f789b9e124847370e..311c62d2d1ebbf3da7ad4468447f6c16b6161fa3 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1254,7 +1254,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);
                }
@@ -1284,11 +1284,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 5c7e4884cac2c5322cad156eb6ae0aefa791c85b..108b12e742c2cf59ee621a4742e4969b411c0965 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -266,7 +266,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");
@@ -297,20 +296,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(
@@ -355,9 +370,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;
@@ -368,24 +380,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 e5dbbc6089a5e4b1c1eee3272aa92a79ba1045b0..1039dfb0b071a7adc718fa4a3d4e8d9e530d030c 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -470,10 +470,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 5b09d95cc5b8fcb0df4dcac0454a597ccb7fbf52..4c4706153305097e36945a880c54fba60d3bd554 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c
@@ -1041,7 +1041,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 */