[Intel-gfx] [PATCH 2/4] drm/i915/display: Always enables MST master pipe first

["José Roberto de Souza" <jose.souza@intel.com>]

Due to DDB overlaps the pipe enabling sequence is not always crescent.
As the previous patch selects the first pipe/transcoder in the MST
stream to the MST master and it needs to be enabled first this
changes were needed to guarantee that.

So here it will first loop through all the MST masters and other
pipes that do not have pipe dependencies and enabling then, as when
the master is being enabled all the slaves are also going to a full
modeset they will not overlap with each other.
Then on the second loop it will enable all the MST slaves.

I have tried to put port sync pipes into those two loops but
intel_update_trans_port_sync_crtcs() is doing way more than just
enable pipes, reading spec I guess it could be accomplish but I will
leave it to people working on port sync.
At least now the port sync pipes are enabled by last so the slave DDB
allocation will not overlap with other pipes.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 120 ++++++++++++++++---
 1 file changed, 105 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f89494c849ce..2f74c0bfb2a8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14576,6 +14576,39 @@ static void intel_update_trans_port_sync_crtcs(struct intel_crtc *crtc,
 				       state);
 }
 
+static void
+skl_commit_modeset_enable_pipe(struct intel_crtc *crtc,
+			       struct intel_crtc_state *old_crtc_state,
+			       struct intel_crtc_state *new_crtc_state,
+			       unsigned int *updated, bool *progress,
+			       struct skl_ddb_entry *entry)
+{
+	struct intel_atomic_state *state = to_intel_atomic_state(old_crtc_state->uapi.state);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	bool vbl_wait = false;
+
+	*updated = *updated | BIT(crtc->pipe);
+	*progress = true;
+	*entry = new_crtc_state->wm.skl.ddb;
+
+	/*
+	 * If this is an already active pipe, it's DDB changed,
+	 * and this isn't the last pipe that needs updating
+	 * then we need to wait for a vblank to pass for the
+	 * new ddb allocation to take effect.
+	 */
+	if (!needs_modeset(new_crtc_state) &&
+	    state->wm_results.dirty_pipes != *updated &&
+	    !skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
+				 &old_crtc_state->wm.skl.ddb))
+		vbl_wait = true;
+
+	intel_update_crtc(crtc, state, old_crtc_state, new_crtc_state);
+
+	if (vbl_wait)
+		intel_wait_for_vblank(dev_priv, crtc->pipe);
+}
+
 static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
@@ -14600,18 +14633,84 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 	/*
 	 * Whenever the number of active pipes changes, we need to make sure we
 	 * update the pipes in the right order so that their ddb allocations
-	 * never overlap with eachother inbetween CRTC updates. Otherwise we'll
+	 * never overlap with each other between CRTC updates. Otherwise we'll
 	 * cause pipe underruns and other bad stuff.
+	 *
+	 * First enable all the pipes that do not depends on other pipes while
+	 * respecting the DDB allocation overlaps.
+	 *
+	 * TODO: integrate port sync to the loops bellow.
+	 * Port sync is not respecting the DDB allocation overlaps as it
+	 * was not checking for the slave port overlaps and there is more than
+	 * just a pipe enable in intel_update_trans_port_sync_crtcs()
 	 */
 	do {
 		progress = false;
 
-		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+						    new_crtc_state, i) {
+			if (updated & BIT(crtc->pipe) ||
+			    !new_crtc_state->hw.active)
+				continue;
+
+			if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
+			    is_trans_port_sync_mode(new_crtc_state))
+				continue;
+
+			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+							entries,
+							INTEL_NUM_PIPES(dev_priv), i))
+				continue;
+
+			skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
+						       new_crtc_state, &updated,
+						       &progress, &entries[i]);
+		}
+	} while (progress);
+
+	/*
+	 * Now enable all the pipes that depends on other pipe aka MST slaves
+	 */
+	do {
+		progress = false;
+
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+						    new_crtc_state, i) {
+			if (updated & BIT(crtc->pipe) ||
+			    !new_crtc_state->hw.active)
+				continue;
+
+			if (is_trans_port_sync_mode(new_crtc_state))
+				continue;
+
+			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+							entries,
+							INTEL_NUM_PIPES(dev_priv), i))
+				continue;
+
+			skl_commit_modeset_enable_pipe(crtc, old_crtc_state,
+						       new_crtc_state, &updated,
+						       &progress, &entries[i]);
+		}
+	} while (progress);
+
+	/* Port sync loop */
+	do {
+		progress = false;
+
+		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+						    new_crtc_state, i) {
 			enum pipe pipe = crtc->pipe;
 			bool vbl_wait = false;
 			bool modeset = needs_modeset(new_crtc_state);
 
-			if (updated & BIT(crtc->pipe) || !new_crtc_state->hw.active)
+			if (updated & BIT(pipe) || !new_crtc_state->hw.active)
+				continue;
+
+			if (!is_trans_port_sync_master(new_crtc_state))
+				continue;
+
+			if (!modeset)
 				continue;
 
 			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
@@ -14634,18 +14733,9 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 			    state->wm_results.dirty_pipes != updated)
 				vbl_wait = true;
 
-			if (modeset && is_trans_port_sync_mode(new_crtc_state)) {
-				if (is_trans_port_sync_master(new_crtc_state))
-					intel_update_trans_port_sync_crtcs(crtc,
-									   state,
-									   old_crtc_state,
-									   new_crtc_state);
-				else
-					continue;
-			} else {
-				intel_update_crtc(crtc, state, old_crtc_state,
-						  new_crtc_state);
-			}
+			intel_update_trans_port_sync_crtcs(crtc, state,
+							   old_crtc_state,
+							   new_crtc_state);
 
 			if (vbl_wait)
 				intel_wait_for_vblank(dev_priv, pipe);
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx