From: Harry Wentland <harry.wentland@xxxxxxx> Cursors are always programmed independently of updates on other planes. When atomic commits program cursor and surface updates together the cursor update might be locked out by the surface update and not take effect. To combat this program cursor and surface updates together via dc_update_planes_and_stream to ensure they can be applied atomically. When cursor updates come on their own use the old method to program the cursor as dc_update_planes_and_stream isn't handling this case correctly (yet), leading to a flickering screen. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2186 Reviewed-by: Agustin Gutierrez <agustin.gutierrez@xxxxxxx> Acked-by: Wayne Lin <wayne.lin@xxxxxxx> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++++++++++++++++++- .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 4 +- .../amd/display/amdgpu_dm/amdgpu_dm_plane.h | 3 + drivers/gpu/drm/amd/display/dc/core/dc.c | 5 ++ .../gpu/drm/amd/display/dc/core/dc_stream.c | 14 ++-- drivers/gpu/drm/amd/display/dc/dc_stream.h | 12 +++ 6 files changed, 105 insertions(+), 12 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 7481440ab124..75b65b243f1e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -8365,6 +8365,77 @@ static inline uint32_t get_mem_type(struct drm_framebuffer *fb) return abo->tbo.resource ? abo->tbo.resource->mem_type : 0; } +static void amdgpu_dm_update_cursor(struct drm_plane *plane, + struct drm_plane_state *old_plane_state, + struct dc_stream_update *update) +{ + struct amdgpu_device *adev = drm_to_adev(plane->dev); + struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(plane->state->fb); + struct drm_crtc *crtc = afb ? plane->state->crtc : old_plane_state->crtc; + struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL; + struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); + uint64_t address = afb ? afb->address : 0; + struct dc_cursor_position position = {0}; + struct dc_cursor_attributes attributes; + int ret; + + if (!plane->state->fb && !old_plane_state->fb) + return; + + drm_dbg_atomic(plane->dev, "crtc_id=%d with size %d to %d\n", + amdgpu_crtc->crtc_id, plane->state->crtc_w, + plane->state->crtc_h); + + ret = amdgpu_dm_plane_get_cursor_position(plane, crtc, &position); + if (ret) + return; + + if (!position.enable) { + /* turn off cursor */ + if (crtc_state && crtc_state->stream) { + dc_stream_set_cursor_position(crtc_state->stream, + &position); + update->cursor_position = &crtc_state->stream->cursor_position; + } + return; + } + + amdgpu_crtc->cursor_width = plane->state->crtc_w; + amdgpu_crtc->cursor_height = plane->state->crtc_h; + + memset(&attributes, 0, sizeof(attributes)); + attributes.address.high_part = upper_32_bits(address); + attributes.address.low_part = lower_32_bits(address); + attributes.width = plane->state->crtc_w; + attributes.height = plane->state->crtc_h; + attributes.color_format = CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA; + attributes.rotation_angle = 0; + attributes.attribute_flags.value = 0; + + /* Enable cursor degamma ROM on DCN3+ for implicit sRGB degamma in DRM + * legacy gamma setup. + */ + if (crtc_state->cm_is_degamma_srgb && + adev->dm.dc->caps.color.dpp.gamma_corr) + attributes.attribute_flags.bits.ENABLE_CURSOR_DEGAMMA = 1; + + attributes.pitch = afb->base.pitches[0] / afb->base.format->cpp[0]; + + if (crtc_state->stream) { + if (!dc_stream_set_cursor_attributes(crtc_state->stream, + &attributes)) + DRM_ERROR("DC failed to set cursor attributes\n"); + + update->cursor_attributes = &crtc_state->stream->cursor_attributes; + + if (!dc_stream_set_cursor_position(crtc_state->stream, + &position)) + DRM_ERROR("DC failed to set cursor position\n"); + + update->cursor_position = &crtc_state->stream->cursor_position; + } +} + static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct drm_device *dev, struct amdgpu_display_manager *dm, @@ -8388,6 +8459,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, bool cursor_update = false; bool pflip_present = false; bool dirty_rects_changed = false; + bool updated_planes_and_streams = false; struct { struct dc_surface_update surface_updates[MAX_SURFACES]; struct dc_plane_info plane_infos[MAX_SURFACES]; @@ -8424,8 +8496,10 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, /* Cursor plane is handled after stream updates */ if (plane->type == DRM_PLANE_TYPE_CURSOR) { if ((fb && crtc == pcrtc) || - (old_plane_state->fb && old_plane_state->crtc == pcrtc)) + (old_plane_state->fb && old_plane_state->crtc == pcrtc)) { cursor_update = true; + amdgpu_dm_update_cursor(plane, old_plane_state, &bundle->stream_update); + } continue; } @@ -8698,6 +8772,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, acrtc_state->stream, &bundle->stream_update, bundle->surface_updates); + updated_planes_and_streams = true; /** * Enable or disable the interrupts on the backend. @@ -8775,7 +8850,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, * This avoids redundant programming in the case where we're going * to be disabling a single plane - those pipes are being disabled. */ - if (acrtc_state->active_planes) + if (acrtc_state->active_planes && !updated_planes_and_streams) amdgpu_dm_commit_cursors(state); cleanup: 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 3c03f690852c..a64f20fcddaa 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 @@ -1197,8 +1197,8 @@ static int amdgpu_dm_plane_atomic_async_check(struct drm_plane *plane, return 0; } -static int amdgpu_dm_plane_get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc, - struct dc_cursor_position *position) +int amdgpu_dm_plane_get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc, + struct dc_cursor_position *position) { struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); int x, y; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h index b51a6b57bd9b..6498359bff6f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h @@ -29,6 +29,9 @@ #include "dc.h" +int amdgpu_dm_plane_get_cursor_position(struct drm_plane *plane, struct drm_crtc *crtc, + struct dc_cursor_position *position); + void amdgpu_dm_plane_handle_cursor_update(struct drm_plane *plane, struct drm_plane_state *old_plane_state); diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 3e16041bf4f9..e955c97697ff 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -3340,6 +3340,11 @@ static void commit_planes_do_stream_update(struct dc *dc, } } + if (stream_update->cursor_attributes) + program_cursor_attributes(dc, stream); + + if (stream_update->cursor_position) + program_cursor_position(dc, stream); /* Full fe update*/ if (update_type == UPDATE_TYPE_FAST) 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 3ac1fec4bf53..b5a89b587d86 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -219,10 +219,9 @@ struct dc_stream_status *dc_stream_get_status( return dc_state_get_stream_status(dc->current_state, stream); } -static void program_cursor_attributes( +void program_cursor_attributes( struct dc *dc, - struct dc_stream_state *stream, - const struct dc_cursor_attributes *attributes) + struct dc_stream_state *stream) { int i; struct resource_context *res_ctx; @@ -318,7 +317,7 @@ bool dc_stream_program_cursor_attributes( reset_idle_optimizations = true; } - program_cursor_attributes(dc, stream, attributes); + program_cursor_attributes(dc, stream); /* re-enable idle optimizations if necessary */ if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle) @@ -330,10 +329,9 @@ bool dc_stream_program_cursor_attributes( return false; } -static void program_cursor_position( +void program_cursor_position( struct dc *dc, - struct dc_stream_state *stream, - const struct dc_cursor_position *position) + struct dc_stream_state *stream) { int i; struct resource_context *res_ctx; @@ -410,7 +408,7 @@ bool dc_stream_program_cursor_position( reset_idle_optimizations = true; } - program_cursor_position(dc, stream, position); + program_cursor_position(dc, stream); /* re-enable idle optimizations if necessary */ if (reset_idle_optimizations && !dc->debug.disable_dmub_reallow_idle) dc_allow_idle_optimizations(dc, true); diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h index 8dd65a95d84b..1469a20f2511 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h @@ -341,6 +341,9 @@ struct dc_stream_update { struct test_pattern *pending_test_pattern; struct dc_crtc_timing_adjust *crtc_timing_adjust; + + struct dc_cursor_attributes *cursor_attributes; + struct dc_cursor_position *cursor_position; }; bool dc_is_stream_unchanged( @@ -480,6 +483,15 @@ struct dc_stream_status *dc_stream_get_status( * Cursor interfaces - To manages the cursor within a stream ******************************************************************************/ /* TODO: Deprecated once we switch to dc_set_cursor_position */ + +void program_cursor_attributes( + struct dc *dc, + struct dc_stream_state *stream); + +void program_cursor_position( + struct dc *dc, + struct dc_stream_state *stream); + bool dc_stream_set_cursor_attributes( struct dc_stream_state *stream, const struct dc_cursor_attributes *attributes); -- 2.37.3
