[DPU PATCH 02/11] drm/msm: Don't duplicate modeset_enables atomic helper

[Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

Instead, shuffle things around so we kickoff crtc after enabling encoder
during modesets. Also moves the vblank wait to after the frame.

Change-Id: I16c7b7f9390d04f6050aa20e17a5335fbf49eba3
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_encoder.c |   5 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  31 ++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     |   2 +
 drivers/gpu/drm/msm/msm_atomic.c            | 132 +-------------------
 5 files changed, 48 insertions(+), 131 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_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 28ceb589ee40..4d1e3652dbf4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -3693,8 +3693,11 @@ static void dpu_encoder_frame_done_timeout(struct timer_list *t)
 static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = {
 	.mode_set = dpu_encoder_virt_mode_set,
 	.disable = dpu_encoder_virt_disable,
-	.enable = dpu_encoder_virt_enable,
+	.enable = dpu_kms_encoder_enable,
 	.atomic_check = dpu_encoder_virt_atomic_check,
+
+	/* This is called by dpu_kms_encoder_enable */
+	.commit = dpu_encoder_virt_enable,
 };
 
 static const struct drm_encoder_funcs dpu_encoder_funcs = {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 81fd3a429e9f..3d83037e8305 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -425,14 +425,37 @@ 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)
+/*
+ * Override the encoder enable since we need to setup the inline rotator and do
+ * some crtc magic before enabling any bridge that might be present.
+ */
+void dpu_kms_encoder_enable(struct drm_encoder *encoder)
+{
+	const struct drm_encoder_helper_funcs *funcs = encoder->helper_private;
+	struct drm_crtc *crtc = encoder->crtc;
+
+	/* Forward this enable call to the commit hook */
+	if (funcs && funcs->commit)
+		funcs->commit(encoder);
+
+	if (crtc && crtc->state->active) {
+		DPU_EVT32(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 *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 modeset is required, kickoff is run in encoder_enable */
+		if (drm_atomic_crtc_needs_modeset(crtc_state))
+			continue;
+
 		if (crtc->state->active) {
 			DPU_EVT32(DRMID(crtc));
 			dpu_crtc_commit_kickoff(crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 8cadd29a48b1..42c809ed9467 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -529,4 +529,6 @@ int dpu_kms_fbo_reference(struct dpu_kms_fbo *fbo);
  */
 void dpu_kms_fbo_unreference(struct dpu_kms_fbo *fbo);
 
+void dpu_kms_encoder_enable(struct drm_encoder *encoder);
+
 #endif /* __dpu_kms_H__ */
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
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno