[DPU PATCH v2] drm/msm: Don't duplicate modeset_enables atomic helper
Jeykumar Sankaran
jsanka at codeaurora.org
Fri Mar 16 20:57:24 UTC 2018
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 at codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul at chromium.org>
Reviewed-by: Jeykumar Sankaran <jsanka at 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
--
Jeykumar S
More information about the dri-devel
mailing list