| Message ID | 20180316194541.23514-1-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-03-16 12:45, Sean Paul wrote: > Instead, shuffle things around so we kickoff crtc after enabling > encoder > during modesets. Also moves the vblank wait to after the frame. > > Changes in v2: > - Remove the encoder.commit hack, it's not required (Jeykumar) > > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 ++ > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 +- > drivers/gpu/drm/msm/msm_atomic.c | 132 ++--------------------- > 3 files changed, 19 insertions(+), 130 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index a3ab6ed2bf1d..94fab2dcca5b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -3525,6 +3525,12 @@ static void dpu_crtc_enable(struct drm_crtc > *crtc, > DPU_EVT32_VERBOSE(DRMID(crtc)); > dpu_crtc = to_dpu_crtc(crtc); > > + if (msm_is_mode_seamless(&crtc->state->adjusted_mode) || > + msm_is_mode_seamless_vrr(&crtc->state->adjusted_mode)) { > + DPU_DEBUG("Skipping crtc enable, seamless mode\n"); > + return; > + } > + > pm_runtime_get_sync(crtc->dev->dev); > > drm_for_each_encoder(encoder, crtc->dev) { > @@ -3572,6 +3578,9 @@ static void dpu_crtc_enable(struct drm_crtc > *crtc, > DPU_POWER_EVENT_POST_ENABLE | DPU_POWER_EVENT_POST_DISABLE > | > DPU_POWER_EVENT_PRE_DISABLE, > dpu_crtc_handle_power_event, crtc, dpu_crtc->name); > + > + if (msm_needs_vblank_pre_modeset(&crtc->state->adjusted_mode)) > + drm_crtc_wait_one_vblank(crtc); > } > > struct plane_state { > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 5ba345395b82..2c4c7fe1affe 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -422,14 +422,14 @@ static void dpu_kms_prepare_commit(struct msm_kms > *kms, > dpu_encoder_prepare_commit(encoder); > } > > -static void dpu_kms_commit(struct msm_kms *kms, > - struct drm_atomic_state *old_state) > +static void dpu_kms_commit(struct msm_kms *kms, struct > drm_atomic_state > *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > + struct drm_crtc_state *crtc_state; > + struct dpu_crtc_state *cstate; > int i; > > - for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > if (crtc->state->active) { > DPU_EVT32(DRMID(crtc)); > dpu_crtc_commit_kickoff(crtc); > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > b/drivers/gpu/drm/msm/msm_atomic.c > index 5cfb80345052..f5794dce25dd 100644 > --- a/drivers/gpu/drm/msm/msm_atomic.c > +++ b/drivers/gpu/drm/msm/msm_atomic.c > @@ -84,131 +84,6 @@ static void msm_atomic_wait_for_commit_done( > } > } > > -/** > - * msm_atomic_helper_commit_modeset_enables - modeset commit to enable > outputs > - * @dev: DRM device > - * @old_state: atomic state object with old state structures > - * > - * This function enables all the outputs with the new configuration > which > had to > - * be turned off for the update. > - * > - * For compatibility with legacy crtc helpers this should be called > after > - * drm_atomic_helper_commit_planes(), which is what the default commit > function > - * does. But drivers with different needs can group the modeset > commits > together > - * and do the plane commits at the end. This is useful for drivers > doing > runtime > - * PM since planes updates then only happen when the CRTC is actually > enabled. > - */ > -static void msm_atomic_helper_commit_modeset_enables(struct drm_device > *dev, > - struct drm_atomic_state *old_state) > -{ > - struct drm_crtc *crtc; > - struct drm_crtc_state *old_crtc_state; > - struct drm_crtc_state *new_crtc_state; > - struct drm_connector *connector; > - struct drm_connector_state *new_conn_state; > - struct msm_drm_private *priv = dev->dev_private; > - struct msm_kms *kms = priv->kms; > - int bridge_enable_count = 0; > - int i; > - > - for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, > - new_crtc_state, i) { > - const struct drm_crtc_helper_funcs *funcs; > - > - /* Need to filter out CRTCs where only planes change. */ > - if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) > - continue; > - > - if (!new_crtc_state->active) > - continue; > - > - if (msm_is_mode_seamless(&new_crtc_state->mode) || > - msm_is_mode_seamless_vrr( > - &new_crtc_state->adjusted_mode)) > - continue; > - > - funcs = crtc->helper_private; > - > - if (crtc->state->enable) { > - DRM_DEBUG_ATOMIC("enabling [CRTC:%d]\n", > - crtc->base.id); > - > - if (funcs->atomic_enable) > - funcs->atomic_enable(crtc, > old_crtc_state); > - else > - funcs->commit(crtc); > - } > - > - if (msm_needs_vblank_pre_modeset( > - &new_crtc_state->adjusted_mode)) > - drm_crtc_wait_one_vblank(crtc); > - } > - > - /* ensure bridge/encoder updates happen on same vblank */ > - msm_atomic_wait_for_commit_done(dev, old_state); > - > - for_each_new_connector_in_state(old_state, connector, > - new_conn_state, i) { > - const struct drm_encoder_helper_funcs *funcs; > - struct drm_encoder *encoder; > - > - if (!new_conn_state->best_encoder) > - continue; > - > - if (!new_conn_state->crtc->state->active || > - !drm_atomic_crtc_needs_modeset( > - new_conn_state->crtc->state)) > - continue; > - > - encoder = new_conn_state->best_encoder; > - funcs = encoder->helper_private; > - > - DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", > - encoder->base.id, encoder->name); > - > - /* > - * Each encoder has at most one connector (since we always > steal > - * it away), so we won't call enable hooks twice. > - */ > - drm_bridge_pre_enable(encoder->bridge); > - ++bridge_enable_count; > - > - if (funcs->enable) > - funcs->enable(encoder); > - else > - funcs->commit(encoder); > - } > - > - if (kms->funcs->commit) { > - DRM_DEBUG_ATOMIC("triggering commit\n"); > - kms->funcs->commit(kms, old_state); > - } > - > - /* If no bridges were pre_enabled, skip iterating over them again > */ > - if (bridge_enable_count == 0) > - return; > - > - for_each_new_connector_in_state(old_state, connector, > - new_conn_state, i) { > - struct drm_encoder *encoder; > - > - if (!new_conn_state->best_encoder) > - continue; > - > - if (!new_conn_state->crtc->state->active || > - !drm_atomic_crtc_needs_modeset( > - new_conn_state->crtc->state)) > - continue; > - > - encoder = new_conn_state->best_encoder; > - > - DRM_DEBUG_ATOMIC("bridge enable enabling > [ENCODER:%d:%s]\n", > - encoder->base.id, encoder->name); > - > - drm_bridge_enable(encoder->bridge); > - } > -} > - > /* The (potentially) asynchronous part of the commit. At this point > * nothing can fail short of armageddon. > */ > @@ -227,7 +102,12 @@ static void complete_commit(struct msm_commit *c) > > drm_atomic_helper_commit_planes(dev, state, 0); > > - msm_atomic_helper_commit_modeset_enables(dev, state); > + drm_atomic_helper_commit_modeset_enables(dev, state); > + > + if (kms->funcs->commit) { > + DRM_DEBUG_ATOMIC("triggering commit\n"); > + kms->funcs->commit(kms, state); > + } > > /* NOTE: _wait_for_vblanks() only waits for vblank on > * enabled CRTCs. So we end up faulting when disabling
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index a3ab6ed2bf1d..94fab2dcca5b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -3525,6 +3525,12 @@ static void dpu_crtc_enable(struct drm_crtc *crtc, DPU_EVT32_VERBOSE(DRMID(crtc)); dpu_crtc = to_dpu_crtc(crtc); + if (msm_is_mode_seamless(&crtc->state->adjusted_mode) || + msm_is_mode_seamless_vrr(&crtc->state->adjusted_mode)) { + DPU_DEBUG("Skipping crtc enable, seamless mode\n"); + return; + } + pm_runtime_get_sync(crtc->dev->dev); drm_for_each_encoder(encoder, crtc->dev) { @@ -3572,6 +3578,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc, DPU_POWER_EVENT_POST_ENABLE | DPU_POWER_EVENT_POST_DISABLE | DPU_POWER_EVENT_PRE_DISABLE, dpu_crtc_handle_power_event, crtc, dpu_crtc->name); + + if (msm_needs_vblank_pre_modeset(&crtc->state->adjusted_mode)) + drm_crtc_wait_one_vblank(crtc); } struct plane_state { diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5ba345395b82..2c4c7fe1affe 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -422,14 +422,14 @@ static void dpu_kms_prepare_commit(struct msm_kms *kms, dpu_encoder_prepare_commit(encoder); } -static void dpu_kms_commit(struct msm_kms *kms, - struct drm_atomic_state *old_state) +static void dpu_kms_commit(struct msm_kms *kms, struct drm_atomic_state *state) { struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; + struct drm_crtc_state *crtc_state; + struct dpu_crtc_state *cstate; int i; - for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { if (crtc->state->active) { DPU_EVT32(DRMID(crtc)); dpu_crtc_commit_kickoff(crtc); diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 5cfb80345052..f5794dce25dd 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -84,131 +84,6 @@ static void msm_atomic_wait_for_commit_done( } } -/** - * msm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs - * @dev: DRM device - * @old_state: atomic state object with old state structures - * - * This function enables all the outputs with the new configuration which had to - * be turned off for the update. - * - * For compatibility with legacy crtc helpers this should be called after - * drm_atomic_helper_commit_planes(), which is what the default commit function - * does. But drivers with different needs can group the modeset commits together - * and do the plane commits at the end. This is useful for drivers doing runtime - * PM since planes updates then only happen when the CRTC is actually enabled. - */ -static void msm_atomic_helper_commit_modeset_enables(struct drm_device *dev, - struct drm_atomic_state *old_state) -{ - struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state; - struct drm_crtc_state *new_crtc_state; - struct drm_connector *connector; - struct drm_connector_state *new_conn_state; - struct msm_drm_private *priv = dev->dev_private; - struct msm_kms *kms = priv->kms; - int bridge_enable_count = 0; - int i; - - for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, - new_crtc_state, i) { - const struct drm_crtc_helper_funcs *funcs; - - /* Need to filter out CRTCs where only planes change. */ - if (!drm_atomic_crtc_needs_modeset(new_crtc_state)) - continue; - - if (!new_crtc_state->active) - continue; - - if (msm_is_mode_seamless(&new_crtc_state->mode) || - msm_is_mode_seamless_vrr( - &new_crtc_state->adjusted_mode)) - continue; - - funcs = crtc->helper_private; - - if (crtc->state->enable) { - DRM_DEBUG_ATOMIC("enabling [CRTC:%d]\n", - crtc->base.id); - - if (funcs->atomic_enable) - funcs->atomic_enable(crtc, old_crtc_state); - else - funcs->commit(crtc); - } - - if (msm_needs_vblank_pre_modeset( - &new_crtc_state->adjusted_mode)) - drm_crtc_wait_one_vblank(crtc); - } - - /* ensure bridge/encoder updates happen on same vblank */ - msm_atomic_wait_for_commit_done(dev, old_state); - - for_each_new_connector_in_state(old_state, connector, - new_conn_state, i) { - const struct drm_encoder_helper_funcs *funcs; - struct drm_encoder *encoder; - - if (!new_conn_state->best_encoder) - continue; - - if (!new_conn_state->crtc->state->active || - !drm_atomic_crtc_needs_modeset( - new_conn_state->crtc->state)) - continue; - - encoder = new_conn_state->best_encoder; - funcs = encoder->helper_private; - - DRM_DEBUG_ATOMIC("enabling [ENCODER:%d:%s]\n", - encoder->base.id, encoder->name); - - /* - * Each encoder has at most one connector (since we always steal - * it away), so we won't call enable hooks twice. - */ - drm_bridge_pre_enable(encoder->bridge); - ++bridge_enable_count; - - if (funcs->enable) - funcs->enable(encoder); - else - funcs->commit(encoder); - } - - if (kms->funcs->commit) { - DRM_DEBUG_ATOMIC("triggering commit\n"); - kms->funcs->commit(kms, old_state); - } - - /* If no bridges were pre_enabled, skip iterating over them again */ - if (bridge_enable_count == 0) - return; - - for_each_new_connector_in_state(old_state, connector, - new_conn_state, i) { - struct drm_encoder *encoder; - - if (!new_conn_state->best_encoder) - continue; - - if (!new_conn_state->crtc->state->active || - !drm_atomic_crtc_needs_modeset( - new_conn_state->crtc->state)) - continue; - - encoder = new_conn_state->best_encoder; - - DRM_DEBUG_ATOMIC("bridge enable enabling [ENCODER:%d:%s]\n", - encoder->base.id, encoder->name); - - drm_bridge_enable(encoder->bridge); - } -} - /* The (potentially) asynchronous part of the commit. At this point * nothing can fail short of armageddon. */ @@ -227,7 +102,12 @@ static void complete_commit(struct msm_commit *c) drm_atomic_helper_commit_planes(dev, state, 0); - msm_atomic_helper_commit_modeset_enables(dev, state); + drm_atomic_helper_commit_modeset_enables(dev, state); + + if (kms->funcs->commit) { + DRM_DEBUG_ATOMIC("triggering commit\n"); + kms->funcs->commit(kms, state); + } /* NOTE: _wait_for_vblanks() only waits for vblank on * enabled CRTCs. So we end up faulting when disabling
Instead, shuffle things around so we kickoff crtc after enabling encoder during modesets. Also moves the vblank wait to after the frame. Changes in v2: - Remove the encoder.commit hack, it's not required (Jeykumar) Cc: Jeykumar Sankaran <jsanka@codeaurora.org> Signed-off-by: Sean Paul <seanpaul@chromium.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 +- drivers/gpu/drm/msm/msm_atomic.c | 132 ++--------------------- 3 files changed, 19 insertions(+), 130 deletions(-)