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>

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 3bd3a218b6046008ca6c254170ead4d15fac0572..01bcccc585008a382a1e671d901a039ddb6a4a0c 100644 (file)
--- 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 49531434f9cf43788e93b7a4880eb26e5af5204a..3c03f690852cb60802ae56364ec80119e78d769a 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
@@ -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 d3201b0b3a09b7dc9c289a9d310d85892774c412..3ac1fec4bf53bd08b3256c7e25f18d2b99573657 100644 (file)
--- 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 3d0adf8838cacb551117aed9969269cb028ac030..8dd65a95d84b6629e69dfdcf19e5ef97d506c47b 100644 (file)
--- 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 ed9141a67db37484098e768b78684961fc727b63..29d1f150846a4e77ec33054c603ced23b850044c 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
@@ -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 */