From f7fb81a18f026b70e42ccc95c7d4485b0cdbfbbf Mon Sep 17 00:00:00 2001 From: Aric Cyr Date: Fri, 18 Sep 2020 11:45:04 -0400 Subject: [PATCH] drm/amd/display: Check for flip pending before locking pipes [Why] When running games or benchmarking with v-sync disabled, disabling a plane (which is v-sync) can cause underflow. This is caused by flips pending before pipe locking being applied after locks are released and pipes could have been re-arranged or disconnected. This could potentially apply a flip on incorrect pipe. Also, previous logic of always locking pipes was unnecessary. [How] Only lock the pipes when there is a pipe being disabled to increase efficiency. Before the pipes are locked, check that any pending flips are cleared to ensure the flips are applied to the correct pipe. Signed-off-by: Aric Cyr Acked-by: Eryk Brol Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/display/dc/core/dc.c | 11 +- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 156 +++++++++++---------- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h | 2 +- drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h | 2 +- 4 files changed, 88 insertions(+), 83 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 2a725a5..014d757 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -2383,7 +2383,6 @@ static void commit_planes_for_stream(struct dc *dc, enum surface_update_type update_type, struct dc_state *context) { - bool mpcc_disconnected = false; int i, j; struct pipe_ctx *top_pipe_to_program = NULL; @@ -2414,14 +2413,8 @@ static void commit_planes_for_stream(struct dc *dc, context_clock_trace(dc, context); } - if (update_type != UPDATE_TYPE_FAST && dc->hwss.interdependent_update_lock && - dc->hwss.disconnect_pipes && dc->hwss.wait_for_pending_cleared){ - dc->hwss.interdependent_update_lock(dc, context, true); - mpcc_disconnected = dc->hwss.disconnect_pipes(dc, context); - dc->hwss.interdependent_update_lock(dc, context, false); - if (mpcc_disconnected) - dc->hwss.wait_for_pending_cleared(dc, context); - } + if (update_type != UPDATE_TYPE_FAST && dc->hwss.interdependent_update_lock && dc->hwss.wait_for_pending_cleared) + dc->hwss.disconnect_pipes(dc, context); for (j = 0; j < dc->res_pool->pipe_count; j++) { struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[j]; diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index d0f3bf9..256185a 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -2761,120 +2761,132 @@ static struct pipe_ctx *dcn10_find_top_pipe_for_stream( return NULL; } -bool dcn10_disconnect_pipes( +void dcn10_disconnect_pipes( struct dc *dc, struct dc_state *context) { - bool found_pipe = false; - int i, j; - struct dce_hwseq *hws = dc->hwseq; - struct dc_state *old_ctx = dc->current_state; - bool mpcc_disconnected = false; - struct pipe_ctx *old_pipe; - struct pipe_ctx *new_pipe; - DC_LOGGER_INIT(dc->ctx->logger); + bool disconnect_pipes[MAX_PIPES] = {0}; + bool found_pipe = false; + int i, j; + struct dce_hwseq *hws = dc->hwseq; + struct dc_state *old_ctx = dc->current_state; + bool plane_disabled = false; + struct pipe_ctx *old_pipe; + struct pipe_ctx *new_pipe; - /* Set pipe update flags and lock pipes */ - for (i = 0; i < dc->res_pool->pipe_count; i++) { - old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; - new_pipe = &context->res_ctx.pipe_ctx[i]; - new_pipe->update_flags.raw = 0; + DC_LOGGER_INIT(dc->ctx->logger); - if (!old_pipe->plane_state && !new_pipe->plane_state) - continue; + /* Set pipe update flags and lock pipes */ + for (i = 0; i < dc->res_pool->pipe_count; i++) { + old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; + new_pipe = &context->res_ctx.pipe_ctx[i]; + new_pipe->update_flags.raw = 0; + + if (!old_pipe->plane_state && !new_pipe->plane_state) + continue; - if (old_pipe->plane_state && !new_pipe->plane_state) - new_pipe->update_flags.bits.disable = 1; + if (old_pipe->plane_state && !new_pipe->plane_state) + new_pipe->update_flags.bits.disable = 1; - /* Check for scl update */ - if (memcmp(&old_pipe->plane_res.scl_data, &new_pipe->plane_res.scl_data, sizeof(struct scaler_data))) - new_pipe->update_flags.bits.scaler = 1; + /* Check for scl update */ + if (memcmp(&old_pipe->plane_res.scl_data, &new_pipe->plane_res.scl_data, sizeof(struct scaler_data))) + new_pipe->update_flags.bits.scaler = 1; - /* Check for vp update */ - if (memcmp(&old_pipe->plane_res.scl_data.viewport, &new_pipe->plane_res.scl_data.viewport, sizeof(struct rect)) - || memcmp(&old_pipe->plane_res.scl_data.viewport_c, + /* Check for vp update */ + if (memcmp(&old_pipe->plane_res.scl_data.viewport, &new_pipe->plane_res.scl_data.viewport, sizeof(struct rect)) + || memcmp(&old_pipe->plane_res.scl_data.viewport_c, &new_pipe->plane_res.scl_data.viewport_c, sizeof(struct rect))) - new_pipe->update_flags.bits.viewport = 1; + new_pipe->update_flags.bits.viewport = 1; - } + } - if (!IS_DIAG_DC(dc->ctx->dce_environment)) { - /* Disconnect mpcc here only if losing pipe split*/ - for (i = 0; i < dc->res_pool->pipe_count; i++) { - if (context->res_ctx.pipe_ctx[i].update_flags.bits.disable && + if (!IS_DIAG_DC(dc->ctx->dce_environment)) { + /* Disconnect mpcc here only if losing pipe split*/ + for (i = 0; i < dc->res_pool->pipe_count; i++) { + if (context->res_ctx.pipe_ctx[i].update_flags.bits.disable && old_ctx->res_ctx.pipe_ctx[i].top_pipe) { - /* Find the top pipe in the new ctx for the bottom pipe that we - * want to remove by comparing the streams and planes. If both - * pipes are being disabled then do it in the regular pipe - * programming sequence - */ - for (j = 0; j < dc->res_pool->pipe_count; j++) { - if (old_ctx->res_ctx.pipe_ctx[i].top_pipe->stream == context->res_ctx.pipe_ctx[j].stream && + /* Find the top pipe in the new ctx for the bottom pipe that we + * want to remove by comparing the streams and planes. If both + * pipes are being disabled then do it in the regular pipe + * programming sequence + */ + for (j = 0; j < dc->res_pool->pipe_count; j++) { + if (old_ctx->res_ctx.pipe_ctx[i].top_pipe->stream == context->res_ctx.pipe_ctx[j].stream && old_ctx->res_ctx.pipe_ctx[i].top_pipe->plane_state == context->res_ctx.pipe_ctx[j].plane_state && !context->res_ctx.pipe_ctx[j].top_pipe && !context->res_ctx.pipe_ctx[j].update_flags.bits.disable) { - found_pipe = true; - break; - } + found_pipe = true; + break; } + } - // Disconnect if the top pipe lost it's pipe split - if (found_pipe && !context->res_ctx.pipe_ctx[j].bottom_pipe) { - hws->funcs.plane_atomic_disconnect(dc, &dc->current_state->res_ctx.pipe_ctx[i]); - DC_LOG_DC("Reset mpcc for pipe %d\n", dc->current_state->res_ctx.pipe_ctx[i].pipe_idx); - mpcc_disconnected = true; - } + plane_disabled = true; + + // Disconnect if the top pipe lost it's pipe split + if (found_pipe && !context->res_ctx.pipe_ctx[j].bottom_pipe) { + disconnect_pipes[i] = true; } - found_pipe = false; } + found_pipe = false; } + } - if (mpcc_disconnected) { - for (i = 0; i < dc->res_pool->pipe_count; i++) { - struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i]; - struct pipe_ctx *old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; - struct dc_plane_state *plane_state = pipe_ctx->plane_state; - struct hubp *hubp = pipe_ctx->plane_res.hubp; + if (plane_disabled) { + dc->hwss.wait_for_pending_cleared(dc, context); + dc->hwss.interdependent_update_lock(dc, context, true); + + for (i = 0; i < dc->res_pool->pipe_count; i++) { + struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i]; + struct pipe_ctx *old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; + struct dc_plane_state *plane_state = pipe_ctx->plane_state; + struct hubp *hubp = pipe_ctx->plane_res.hubp; + + if (!pipe_ctx || !plane_state || !pipe_ctx->stream) + continue; - if (!pipe_ctx || !plane_state || !pipe_ctx->stream) - continue; + if (disconnect_pipes[i]) { + hws->funcs.plane_atomic_disconnect(dc, &dc->current_state->res_ctx.pipe_ctx[i]); + DC_LOG_DC("Reset mpcc for pipe %d\n", dc->current_state->res_ctx.pipe_ctx[i].pipe_idx); + } - // Only update scaler and viewport here if we lose a pipe split. - // This is to prevent half the screen from being black when we - // unlock after disconnecting MPCC. - if (!(old_pipe && !pipe_ctx->top_pipe && + // Only update scaler and viewport here if we lose a pipe split. + // This is to prevent half the screen from being black when we + // unlock after disconnecting MPCC. + if (!(old_pipe && !pipe_ctx->top_pipe && !pipe_ctx->bottom_pipe && old_pipe->bottom_pipe)) - continue; + continue; - if (pipe_ctx->update_flags.raw || pipe_ctx->plane_state->update_flags.raw || pipe_ctx->stream->update_flags.raw) { - if (pipe_ctx->update_flags.bits.scaler || + if (pipe_ctx->update_flags.raw || pipe_ctx->plane_state->update_flags.raw || pipe_ctx->stream->update_flags.raw) { + if (pipe_ctx->update_flags.bits.scaler || plane_state->update_flags.bits.scaling_change || plane_state->update_flags.bits.position_change || plane_state->update_flags.bits.per_pixel_alpha_change || pipe_ctx->stream->update_flags.bits.scaling) { - pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->plane_state->per_pixel_alpha; - ASSERT(pipe_ctx->plane_res.scl_data.lb_params.depth == LB_PIXEL_DEPTH_30BPP); - /* scaler configuration */ - pipe_ctx->plane_res.dpp->funcs->dpp_set_scaler( - pipe_ctx->plane_res.dpp, &pipe_ctx->plane_res.scl_data); - } + pipe_ctx->plane_res.scl_data.lb_params.alpha_en = pipe_ctx->plane_state->per_pixel_alpha; + ASSERT(pipe_ctx->plane_res.scl_data.lb_params.depth == LB_PIXEL_DEPTH_30BPP); + /* scaler configuration */ + pipe_ctx->plane_res.dpp->funcs->dpp_set_scaler( + pipe_ctx->plane_res.dpp, &pipe_ctx->plane_res.scl_data); + } - if (pipe_ctx->update_flags.bits.viewport || + if (pipe_ctx->update_flags.bits.viewport || (context == dc->current_state && plane_state->update_flags.bits.position_change) || (context == dc->current_state && plane_state->update_flags.bits.scaling_change) || (context == dc->current_state && pipe_ctx->stream->update_flags.bits.scaling)) { - hubp->funcs->mem_program_viewport( + hubp->funcs->mem_program_viewport( hubp, &pipe_ctx->plane_res.scl_data.viewport, &pipe_ctx->plane_res.scl_data.viewport_c); - } } } } - return mpcc_disconnected; + + dc->hwss.interdependent_update_lock(dc, context, false); + dc->hwss.wait_for_pending_cleared(dc, context); + } } void dcn10_wait_for_pending_cleared(struct dc *dc, diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h index e5691e4..9a0f7a8 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h @@ -194,7 +194,7 @@ void dcn10_get_surface_visual_confirm_color( void dcn10_get_hdr_visual_confirm_color( struct pipe_ctx *pipe_ctx, struct tg_color *color); -bool dcn10_disconnect_pipes( +void dcn10_disconnect_pipes( struct dc *dc, struct dc_state *context); diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h index 64c1be8..f48ee24 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h @@ -67,7 +67,7 @@ struct hw_sequencer_funcs { int num_planes, struct dc_state *context); void (*program_front_end_for_ctx)(struct dc *dc, struct dc_state *context); - bool (*disconnect_pipes)(struct dc *dc, + void (*disconnect_pipes)(struct dc *dc, struct dc_state *context); void (*wait_for_pending_cleared)(struct dc *dc, struct dc_state *context); -- 2.7.4