[Intel-gfx] [PATCH v10 10/11] drm/i915: Ensure correct master/slave enable/disable sequence

Manasi Navare manasi.d.navare at intel.com
Thu Oct 8 21:45:34 UTC 2020


From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

Enabling is done in a special sequence and so should plane updates
be. Ideally the end user never notices the second pipe is used.

This way ideally everything will be tear free, and updates are
really atomic as userspace expects it.

This uses generic modeset_enables() calls like trans port sync
but still has special handling for disable since for slave we
should not disable things like encoder, plls that are not enabled
for  slave.

v3:
* Fixes in enable and disable sequence from testing (Manasi)
v2:
* Manual Rebase (Manasi)
* Refactoring on intel_update_crtc and enable_crtc and removing
special trans_port_sync_update (Manasi)

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 55 ++++++++++++++------
 drivers/gpu/drm/i915/display/intel_sprite.c  |  5 +-
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 357cc2bce300..101ddd0b48ab 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -15878,6 +15878,9 @@ static void intel_enable_crtc(struct intel_atomic_state *state,
 
 	dev_priv->display.crtc_enable(state, crtc);
 
+	if (new_crtc_state->bigjoiner_slave)
+		return;
+
 	/* vblanks work again, re-enable pipe CRC. */
 	intel_crtc_enable_pipe_crc(crtc);
 }
@@ -15914,9 +15917,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 
 	commit_pipe_config(state, crtc);
 
-	if (new_crtc_state->bigjoiner) {
-	/* Not supported yet */
-	} else if (INTEL_GEN(dev_priv) >= 9)
+	if (INTEL_GEN(dev_priv) >= 9)
 		skl_update_planes_on_crtc(state, crtc);
 	else
 		i9xx_update_planes_on_crtc(state, crtc);
@@ -15945,9 +15946,17 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 	drm_WARN_ON(&dev_priv->drm, old_crtc_state->bigjoiner_slave);
 
 	intel_crtc_disable_planes(state, crtc);
-	if (old_crtc_state->bigjoiner)
+
+	/*
+	 * We still need special handling for disabling bigjoiner master
+	 * and slaves since for slave we do not have encoder or plls
+	 * so we dont need to disable those.
+	 */
+	if (old_crtc_state->bigjoiner) {
 		intel_crtc_disable_planes(state,
 					  old_crtc_state->bigjoiner_linked_crtc);
+		old_crtc_state->bigjoiner_linked_crtc->active = false;
+	}
 
 	/*
 	 * We need to disable pipe CRC before disabling the pipe,
@@ -15977,7 +15986,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 	/* Only disable port sync and MST slaves */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (!needs_modeset(new_crtc_state) || old_crtc_state->bigjoiner_slave)
+		if (!needs_modeset(new_crtc_state) || old_crtc_state->bigjoiner)
 			continue;
 
 		if (!old_crtc_state->hw.active)
@@ -16040,6 +16049,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
 	struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
+	struct skl_ddb_entry new_entries[I915_MAX_PIPES] = {};
 	u8 update_pipes = 0, modeset_pipes = 0;
 	int i;
 
@@ -16056,6 +16066,14 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		} else {
 			modeset_pipes |= BIT(pipe);
 		}
+
+		new_entries[i] = new_crtc_state->wm.skl.ddb;
+
+		/* ignore allocations for crtc's that have been turned off during modeset. */
+		if (needs_modeset(new_crtc_state))
+			continue;
+
+		entries[i] = old_crtc_state->wm.skl.ddb;
 	}
 
 	/*
@@ -16071,28 +16089,28 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 						    new_crtc_state, i) {
 			enum pipe pipe = crtc->pipe;
+			bool ddb_changed;
 
 			if ((update_pipes & BIT(pipe)) == 0)
 				continue;
 
-			if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+			if (skl_ddb_allocation_overlaps(&new_entries[pipe],
 							entries, I915_MAX_PIPES, pipe))
 				continue;
 
-			entries[pipe] = new_crtc_state->wm.skl.ddb;
+			ddb_changed = !skl_ddb_entry_equal(&new_entries[pipe], &entries[pipe]);
+			entries[pipe] = new_entries[pipe];
 			update_pipes &= ~BIT(pipe);
 
-			intel_update_crtc(state, crtc);
-
 			/*
 			 * 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 (!skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
-						 &old_crtc_state->wm.skl.ddb) &&
-			    (update_pipes | modeset_pipes))
+			intel_update_crtc(state, crtc);
+
+			if (ddb_changed && (update_pipes | modeset_pipes))
 				intel_wait_for_vblank(dev_priv, pipe);
 		}
 	}
@@ -16110,7 +16128,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 			continue;
 
 		if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
-		    is_trans_port_sync_master(new_crtc_state))
+		    is_trans_port_sync_master(new_crtc_state) ||
+		    (new_crtc_state->bigjoiner && !new_crtc_state->bigjoiner_slave))
 			continue;
 
 		modeset_pipes &= ~BIT(pipe);
@@ -16120,7 +16139,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 
 	/*
 	 * Then we enable all remaining pipes that depend on other
-	 * pipes: MST slaves and port sync masters.
+	 * pipes: MST slaves and port sync masters, big joiner master
 	 */
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		enum pipe pipe = crtc->pipe;
@@ -16128,6 +16147,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		if ((modeset_pipes & BIT(pipe)) == 0)
 			continue;
 
+		WARN_ON(skl_ddb_allocation_overlaps(&new_entries[pipe],
+						    entries, I915_MAX_PIPES, pipe));
+
+		entries[pipe] = new_entries[pipe];
 		modeset_pipes &= ~BIT(pipe);
 
 		intel_enable_crtc(state, crtc);
@@ -16142,10 +16165,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		if ((update_pipes & BIT(pipe)) == 0)
 			continue;
 
-		drm_WARN_ON(&dev_priv->drm, skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
+		drm_WARN_ON(&dev_priv->drm, skl_ddb_allocation_overlaps(&new_entries[pipe],
 									entries, I915_MAX_PIPES, pipe));
 
-		entries[pipe] = new_crtc_state->wm.skl.ddb;
+		entries[pipe] = new_entries[pipe];
 		update_pipes &= ~BIT(pipe);
 
 		intel_update_crtc(state, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 9e235210adc7..1c740a22a8d7 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -103,6 +103,8 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
 
 	/* FIXME needs to be calibrated sensibly */
 	min = vblank_start - intel_usecs_to_scanlines(adjusted_mode,
+						      new_crtc_state->bigjoiner ?
+						      2 * VBLANK_EVASION_TIME_US :
 						      VBLANK_EVASION_TIME_US);
 	max = vblank_start - 1;
 
@@ -227,7 +229,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 
 		spin_lock(&crtc->base.dev->event_lock);
 		drm_crtc_arm_vblank_event(&crtc->base,
-				          new_crtc_state->uapi.event);
+					  new_crtc_state->uapi.event);
+
 		spin_unlock(&crtc->base.dev->event_lock);
 
 		new_crtc_state->uapi.event = NULL;
-- 
2.19.1



More information about the Intel-gfx mailing list