From: Rob Clark <robdclark@xxxxxxxxxxxx> It attempted to avoid fps drops in the presence of cursor updates. But it is racing, and can result in hw updates after flush before vblank, which leads to underruns. Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx> Reviewed-by: Sean Paul <sean@xxxxxxxxxx> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 41 ++++++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++------------- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 3 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 +-- drivers/gpu/drm/msm/msm_atomic.c | 3 +- 6 files changed, 37 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index a52439e029c9..c3f7154017c4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -610,7 +610,7 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) return rc; } -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async) +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) { struct drm_encoder *encoder; struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); @@ -636,35 +636,32 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async) crtc->state->encoder_mask) dpu_encoder_prepare_for_kickoff(encoder); - if (!async) { - /* wait for previous frame_event_done completion */ - DPU_ATRACE_BEGIN("wait_for_frame_done_event"); - ret = _dpu_crtc_wait_for_frame_done(crtc); - DPU_ATRACE_END("wait_for_frame_done_event"); - if (ret) { - DPU_ERROR("crtc%d wait for frame done failed;frame_pending%d\n", - crtc->base.id, - atomic_read(&dpu_crtc->frame_pending)); - goto end; - } + /* wait for previous frame_event_done completion */ + DPU_ATRACE_BEGIN("wait_for_frame_done_event"); + ret = _dpu_crtc_wait_for_frame_done(crtc); + DPU_ATRACE_END("wait_for_frame_done_event"); + if (ret) { + DPU_ERROR("crtc%d wait for frame done failed;frame_pending%d\n", + crtc->base.id, + atomic_read(&dpu_crtc->frame_pending)); + goto end; + } - if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) { - /* acquire bandwidth and other resources */ - DPU_DEBUG("crtc%d first commit\n", crtc->base.id); - } else - DPU_DEBUG("crtc%d commit\n", crtc->base.id); + if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) { + /* acquire bandwidth and other resources */ + DPU_DEBUG("crtc%d first commit\n", crtc->base.id); + } else + DPU_DEBUG("crtc%d commit\n", crtc->base.id); - dpu_crtc->play_count++; - } + dpu_crtc->play_count++; dpu_vbif_clear_errors(dpu_kms); drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) - dpu_encoder_kickoff(encoder, async); + dpu_encoder_kickoff(encoder); end: - if (!async) - reinit_completion(&dpu_crtc->frame_done_comp); + reinit_completion(&dpu_crtc->frame_done_comp); DPU_ATRACE_END("crtc_commit"); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h index 5181f079a6a1..10f78459f6c2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -238,9 +238,8 @@ void dpu_crtc_vblank_callback(struct drm_crtc *crtc); /** * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc * @crtc: Pointer to drm crtc object - * @async: true if the commit is asynchronous, false otherwise */ -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async); +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc); /** * dpu_crtc_complete_commit - callback signalling completion of current commit diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 627c57594221..ac2d534bf59e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1421,8 +1421,7 @@ static void dpu_encoder_off_work(struct work_struct *work) * extra_flush_bits: Additional bit mask to include in flush trigger */ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc, - struct dpu_encoder_phys *phys, uint32_t extra_flush_bits, - bool async) + struct dpu_encoder_phys *phys, uint32_t extra_flush_bits) { struct dpu_hw_ctl *ctl; int pending_kickoff_cnt; @@ -1439,10 +1438,7 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc, return; } - if (!async) - pending_kickoff_cnt = dpu_encoder_phys_inc_pending(phys); - else - pending_kickoff_cnt = atomic_read(&phys->pending_kickoff_cnt); + pending_kickoff_cnt = dpu_encoder_phys_inc_pending(phys); if (extra_flush_bits && ctl->ops.update_pending_flush) ctl->ops.update_pending_flush(ctl, extra_flush_bits); @@ -1553,8 +1549,7 @@ static void dpu_encoder_helper_hw_reset(struct dpu_encoder_phys *phys_enc) * a time. * dpu_enc: Pointer to virtual encoder structure */ -static void _dpu_encoder_kickoff_phys(struct dpu_encoder_virt *dpu_enc, - bool async) +static void _dpu_encoder_kickoff_phys(struct dpu_encoder_virt *dpu_enc) { struct dpu_hw_ctl *ctl; uint32_t i, pending_flush; @@ -1581,13 +1576,12 @@ static void _dpu_encoder_kickoff_phys(struct dpu_encoder_virt *dpu_enc, * for async commits. So don't set this for async, since it'll * roll over to the next commit. */ - if (!async && phys->split_role != ENC_ROLE_SLAVE) + if (phys->split_role != ENC_ROLE_SLAVE) set_bit(i, dpu_enc->frame_busy_mask); if (!phys->ops.needs_single_flush || !phys->ops.needs_single_flush(phys)) - _dpu_encoder_trigger_flush(&dpu_enc->base, phys, 0x0, - async); + _dpu_encoder_trigger_flush(&dpu_enc->base, phys, 0x0); else if (ctl->ops.get_pending_flush) pending_flush |= ctl->ops.get_pending_flush(ctl); } @@ -1597,7 +1591,7 @@ static void _dpu_encoder_kickoff_phys(struct dpu_encoder_virt *dpu_enc, _dpu_encoder_trigger_flush( &dpu_enc->base, dpu_enc->cur_master, - pending_flush, async); + pending_flush); } _dpu_encoder_trigger_start(dpu_enc->cur_master); @@ -1815,11 +1809,12 @@ void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc) } } -void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async) +void dpu_encoder_kickoff(struct drm_encoder *drm_enc) { struct dpu_encoder_virt *dpu_enc; struct dpu_encoder_phys *phys; ktime_t wakeup_time; + unsigned long timeout_ms; unsigned int i; DPU_ATRACE_BEGIN("encoder_kickoff"); @@ -1827,23 +1822,15 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc, bool async) trace_dpu_enc_kickoff(DRMID(drm_enc)); - /* - * Asynchronous frames don't handle FRAME_DONE events. As such, they - * shouldn't enable the frame_done watchdog since it will always time - * out. - */ - if (!async) { - unsigned long timeout_ms; - timeout_ms = DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES * 1000 / + timeout_ms = DPU_ENCODER_FRAME_DONE_TIMEOUT_FRAMES * 1000 / drm_mode_vrefresh(&drm_enc->crtc->state->adjusted_mode); - atomic_set(&dpu_enc->frame_done_timeout_ms, timeout_ms); - mod_timer(&dpu_enc->frame_done_timer, - jiffies + msecs_to_jiffies(timeout_ms)); - } + atomic_set(&dpu_enc->frame_done_timeout_ms, timeout_ms); + mod_timer(&dpu_enc->frame_done_timer, + jiffies + msecs_to_jiffies(timeout_ms)); /* All phys encs are ready to go, trigger the kickoff */ - _dpu_encoder_kickoff_phys(dpu_enc, async); + _dpu_encoder_kickoff_phys(dpu_enc); /* allow phys encs to handle any post-kickoff business */ for (i = 0; i < dpu_enc->num_phys_encs; i++) { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 997d131c2440..8465b37adf3b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -82,9 +82,8 @@ void dpu_encoder_trigger_kickoff_pending(struct drm_encoder *encoder); * dpu_encoder_kickoff - trigger a double buffer flip of the ctl path * (i.e. ctl flush and start) immediately. * @encoder: encoder pointer - * @async: true if this is an asynchronous commit */ -void dpu_encoder_kickoff(struct drm_encoder *encoder, bool async); +void dpu_encoder_kickoff(struct drm_encoder *encoder); /** * dpu_encoder_wait_for_event - Waits for encoder events diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index b8d264bd15df..31454cc5d8c5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -298,7 +298,7 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder) continue; trace_dpu_kms_enc_enable(DRMID(crtc)); - dpu_crtc_commit_kickoff(crtc, false); + dpu_crtc_commit_kickoff(crtc); } } @@ -315,8 +315,7 @@ static void dpu_kms_commit(struct msm_kms *kms, struct drm_atomic_state *state) if (crtc->state->active) { trace_dpu_kms_commit(DRMID(crtc)); - dpu_crtc_commit_kickoff(crtc, - state->legacy_cursor_update); + dpu_crtc_commit_kickoff(crtc); } } } diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index dd16babdd8c0..e5aae1645933 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -70,8 +70,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state *state) kms->funcs->commit(kms, state); } - if (!state->legacy_cursor_update) - msm_atomic_wait_for_commit_done(dev, state); + msm_atomic_wait_for_commit_done(dev, state); kms->funcs->complete_commit(kms, state); -- 2.21.0 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel