On Thu, Aug 29, 2019 at 09:45:17AM -0700, Rob Clark wrote:
> From: Rob Clark <robdcl...@chromium.org>
>
> In addition, moving to kms->flush_commit() lets us drop the only user
> of kms->commit().
>
> Signed-off-by: Rob Clark <robdcl...@chromium.org>
Reviewed-by: Sean Paul <s...@poorly.run>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 ------
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 5 +++
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 46 +++++++++++----------
> drivers/gpu/drm/msm/msm_atomic.c | 5 +--
> drivers/gpu/drm/msm/msm_kms.h | 3 --
> 6 files changed, 34 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 31debd31ab8c..f38a7d27a1c0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -606,7 +606,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> - int ret;
>
> /*
> * If no mixers has been allocated in dpu_crtc_atomic_check(),
> @@ -626,17 +625,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> crtc->state->encoder_mask)
> dpu_encoder_prepare_for_kickoff(encoder);
>
> - /* 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);
> @@ -650,7 +638,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
> dpu_encoder_kickoff(encoder);
>
> -end:
> reinit_completion(&dpu_crtc->frame_done_comp);
> DPU_ATRACE_END("crtc_commit");
> }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ac2d534bf59e..3a69b93d8fb6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1678,8 +1678,7 @@ static u32 _dpu_encoder_calculate_linetime(struct
> dpu_encoder_virt *dpu_enc,
> return line_time;
> }
>
> -static int _dpu_encoder_wakeup_time(struct drm_encoder *drm_enc,
> - ktime_t *wakeup_time)
> +int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t *wakeup_time)
> {
> struct drm_display_mode *mode;
> struct dpu_encoder_virt *dpu_enc;
> @@ -1766,7 +1765,7 @@ static void dpu_encoder_vsync_event_work_handler(struct
> kthread_work *work)
> return;
> }
>
> - if (_dpu_encoder_wakeup_time(&dpu_enc->base, &wakeup_time))
> + if (dpu_encoder_vsync_time(&dpu_enc->base, &wakeup_time))
> return;
>
> trace_dpu_enc_vsync_event_work(DRMID(&dpu_enc->base), wakeup_time);
> @@ -1840,7 +1839,7 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
> }
>
> if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
> - !_dpu_encoder_wakeup_time(drm_enc, &wakeup_time)) {
> + !dpu_encoder_vsync_time(drm_enc, &wakeup_time)) {
> trace_dpu_enc_early_kickoff(DRMID(drm_enc),
> ktime_to_ms(wakeup_time));
> mod_timer(&dpu_enc->vsync_event_timer,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 8465b37adf3b..b4913465e602 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -85,6 +85,11 @@ void dpu_encoder_trigger_kickoff_pending(struct
> drm_encoder *encoder);
> */
> void dpu_encoder_kickoff(struct drm_encoder *encoder);
>
> +/**
> + * dpu_encoder_wakeup_time - get the time of the next vsync
> + */
> +int dpu_encoder_vsync_time(struct drm_encoder *drm_enc, ktime_t
> *wakeup_time);
> +
> /**
> * dpu_encoder_wait_for_event - Waits for encoder events
> * @encoder: encoder pointer
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index d54741f3ad9f..af41af1731c2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -260,6 +260,20 @@ static void dpu_kms_disable_commit(struct msm_kms *kms)
> pm_runtime_put_sync(&dpu_kms->pdev->dev);
> }
>
> +static ktime_t dpu_kms_vsync_time(struct msm_kms *kms, struct drm_crtc *crtc)
> +{
> + struct drm_encoder *encoder;
> +
> + drm_for_each_encoder_mask(encoder, crtc->dev,
> crtc->state->encoder_mask) {
> + ktime_t vsync_time;
> +
> + if (dpu_encoder_vsync_time(encoder, &vsync_time) == 0)
> + return vsync_time;
> + }
> +
> + return ktime_get();
> +}
> +
> static void dpu_kms_prepare_commit(struct msm_kms *kms,
> struct drm_atomic_state *state)
> {
> @@ -291,7 +305,16 @@ static void dpu_kms_prepare_commit(struct msm_kms *kms,
>
> static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
> {
> - /* TODO */
> + struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> + struct drm_crtc *crtc;
> +
> + for_each_crtc_mask(dpu_kms->dev, crtc, crtc_mask) {
> + if (!crtc->state->active)
> + continue;
> +
> + trace_dpu_kms_commit(DRMID(crtc));
> + dpu_crtc_commit_kickoff(crtc);
> + }
> }
>
> /*
> @@ -314,25 +337,6 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder)
> continue;
>
> trace_dpu_kms_enc_enable(DRMID(crtc));
> - dpu_crtc_commit_kickoff(crtc);
> - }
> -}
> -
> -static void dpu_kms_commit(struct msm_kms *kms, struct drm_atomic_state
> *state)
> -{
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *crtc_state;
> - int i;
> -
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> - /* If modeset is required, kickoff is run in encoder_enable */
> - if (drm_atomic_crtc_needs_modeset(crtc_state))
> - continue;
> -
> - if (crtc->state->active) {
> - trace_dpu_kms_commit(DRMID(crtc));
> - dpu_crtc_commit_kickoff(crtc);
> - }
> }
> }
>
> @@ -693,9 +697,9 @@ static const struct msm_kms_funcs kms_funcs = {
> .irq = dpu_irq,
> .enable_commit = dpu_kms_enable_commit,
> .disable_commit = dpu_kms_disable_commit,
> + .vsync_time = dpu_kms_vsync_time,
> .prepare_commit = dpu_kms_prepare_commit,
> .flush_commit = dpu_kms_flush_commit,
> - .commit = dpu_kms_commit,
> .wait_flush = dpu_kms_wait_flush,
> .complete_commit = dpu_kms_complete_commit,
> .enable_vblank = dpu_kms_enable_vblank,
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c
> b/drivers/gpu/drm/msm/msm_atomic.c
> index 8f8f74337cb4..80536538967b 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -213,10 +213,7 @@ void msm_atomic_commit_tail(struct drm_atomic_state
> *state)
> /*
> * Flush hardware updates:
> */
> - if (kms->funcs->commit) {
> - DRM_DEBUG_ATOMIC("triggering commit\n");
> - kms->funcs->commit(kms, state);
> - }
> + DRM_DEBUG_ATOMIC("triggering commit\n");
> kms->funcs->flush_commit(kms, crtc_mask);
> mutex_unlock(&kms->commit_lock);
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 5eafc9686d29..32ff2e070ea2 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -80,9 +80,6 @@ struct msm_kms_funcs {
> */
> void (*flush_commit)(struct msm_kms *kms, unsigned crtc_mask);
>
> - /* TODO remove ->commit(), use ->flush_commit() instead: */
> - void (*commit)(struct msm_kms *kms, struct drm_atomic_state *state);
> -
> /**
> * Wait for any in-progress flush to complete on the specified
> * crtcs. This should not block if there is no in-progress
> --
> 2.21.0
>
--
Sean Paul, Software Engineer, Google / Chromium OS
