All of lore.kernel.org
 help / color / mirror / Atom feed
From: "José Roberto de Souza" <jose.souza@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 1/3] drm/i915/display: Refactor intel_commit_modeset_disables()
Date: Wed, 20 Nov 2019 15:48:51 -0800	[thread overview]
Message-ID: <20191120234853.348731-1-jose.souza@intel.com> (raw)

Commit 9c722e17c1b9 ("drm/i915: Disable pipes in reverse order")
reverted the order that pipes gets disabled because of TGL
master/slave relationship between transcoders in MST mode.

But as stated in a comment in skl_commit_modeset_enables() the
enabling order is not always crescent, possibly causing previously
selected slave transcoder being enabled before master so another
approach will be needed to select a transcoder to master in MST mode.
It will be similar to the approach taken in port sync.

But instead of implement something like
intel_trans_port_sync_modeset_disables() to MST lets simply it and
iterate over all pipes 2 times, the first one disabling any slave and
then disabling everything else.
The MST bits will be added in another patch.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 53dc310a5f6d..1b1fbb6d8acc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14443,53 +14443,16 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 		dev_priv->display.initial_watermarks(state, crtc);
 }
 
-static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *state,
-						   struct intel_crtc *crtc,
-						   struct intel_crtc_state *old_crtc_state,
-						   struct intel_crtc_state *new_crtc_state)
-{
-	struct intel_crtc *slave_crtc = intel_get_slave_crtc(new_crtc_state);
-	struct intel_crtc_state *new_slave_crtc_state =
-		intel_atomic_get_new_crtc_state(state, slave_crtc);
-	struct intel_crtc_state *old_slave_crtc_state =
-		intel_atomic_get_old_crtc_state(state, slave_crtc);
-
-	WARN_ON(!slave_crtc || !new_slave_crtc_state ||
-		!old_slave_crtc_state);
-
-	/* Disable Slave first */
-	intel_pre_plane_update(old_slave_crtc_state, new_slave_crtc_state);
-	if (old_slave_crtc_state->hw.active)
-		intel_old_crtc_state_disables(state,
-					      old_slave_crtc_state,
-					      new_slave_crtc_state,
-					      slave_crtc);
-
-	/* Disable Master */
-	intel_pre_plane_update(old_crtc_state, new_crtc_state);
-	if (old_crtc_state->hw.active)
-		intel_old_crtc_state_disables(state,
-					      old_crtc_state,
-					      new_crtc_state,
-					      crtc);
-}
-
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 {
 	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	int i;
 
-	/*
-	 * Disable CRTC/pipes in reverse order because some features(MST in
-	 * TGL+) requires master and slave relationship between pipes, so it
-	 * should always pick the lowest pipe as master as it will be enabled
-	 * first and disable in the reverse order so the master will be the
-	 * last one to be disabled.
-	 */
-	for_each_oldnew_intel_crtc_in_state_reverse(state, crtc, old_crtc_state,
-						    new_crtc_state, i) {
-		if (!needs_modeset(new_crtc_state))
+	/* Only disable port sync slaves */
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (!needs_modeset(new_crtc_state) || !crtc->active)
 			continue;
 
 		/* In case of Transcoder port Sync master slave CRTCs can be
@@ -14497,23 +14460,25 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		 * slave CRTCs are disabled first and then master CRTC since
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
-		if (is_trans_port_sync_mode(new_crtc_state)) {
-			if (is_trans_port_sync_master(new_crtc_state))
-				intel_trans_port_sync_modeset_disables(state,
-								       crtc,
-								       old_crtc_state,
-								       new_crtc_state);
-			else
-				continue;
-		} else {
-			intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		if (!is_trans_port_sync_mode(new_crtc_state))
+			continue;
+		if (is_trans_port_sync_master(new_crtc_state))
+			continue;
 
-			if (old_crtc_state->hw.active)
-				intel_old_crtc_state_disables(state,
-							      old_crtc_state,
-							      new_crtc_state,
-							      crtc);
-		}
+		intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		intel_old_crtc_state_disables(state, old_crtc_state,
+					      new_crtc_state, crtc);
+	}
+
+	/* Disable everything else left on */
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (!needs_modeset(new_crtc_state) || !crtc->active)
+			continue;
+
+		intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		intel_old_crtc_state_disables(state, old_crtc_state,
+					      new_crtc_state, crtc);
 	}
 }
 
-- 
2.24.0

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

WARNING: multiple messages have this Message-ID (diff)
From: "José Roberto de Souza" <jose.souza@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 1/3] drm/i915/display: Refactor intel_commit_modeset_disables()
Date: Wed, 20 Nov 2019 15:48:51 -0800	[thread overview]
Message-ID: <20191120234853.348731-1-jose.souza@intel.com> (raw)
Message-ID: <20191120234851.AvWkkXgyhHkeAvGjyRQPIADja7iL3niQBGRaNJavu2M@z> (raw)

Commit 9c722e17c1b9 ("drm/i915: Disable pipes in reverse order")
reverted the order that pipes gets disabled because of TGL
master/slave relationship between transcoders in MST mode.

But as stated in a comment in skl_commit_modeset_enables() the
enabling order is not always crescent, possibly causing previously
selected slave transcoder being enabled before master so another
approach will be needed to select a transcoder to master in MST mode.
It will be similar to the approach taken in port sync.

But instead of implement something like
intel_trans_port_sync_modeset_disables() to MST lets simply it and
iterate over all pipes 2 times, the first one disabling any slave and
then disabling everything else.
The MST bits will be added in another patch.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 53dc310a5f6d..1b1fbb6d8acc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14443,53 +14443,16 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 		dev_priv->display.initial_watermarks(state, crtc);
 }
 
-static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *state,
-						   struct intel_crtc *crtc,
-						   struct intel_crtc_state *old_crtc_state,
-						   struct intel_crtc_state *new_crtc_state)
-{
-	struct intel_crtc *slave_crtc = intel_get_slave_crtc(new_crtc_state);
-	struct intel_crtc_state *new_slave_crtc_state =
-		intel_atomic_get_new_crtc_state(state, slave_crtc);
-	struct intel_crtc_state *old_slave_crtc_state =
-		intel_atomic_get_old_crtc_state(state, slave_crtc);
-
-	WARN_ON(!slave_crtc || !new_slave_crtc_state ||
-		!old_slave_crtc_state);
-
-	/* Disable Slave first */
-	intel_pre_plane_update(old_slave_crtc_state, new_slave_crtc_state);
-	if (old_slave_crtc_state->hw.active)
-		intel_old_crtc_state_disables(state,
-					      old_slave_crtc_state,
-					      new_slave_crtc_state,
-					      slave_crtc);
-
-	/* Disable Master */
-	intel_pre_plane_update(old_crtc_state, new_crtc_state);
-	if (old_crtc_state->hw.active)
-		intel_old_crtc_state_disables(state,
-					      old_crtc_state,
-					      new_crtc_state,
-					      crtc);
-}
-
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 {
 	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	int i;
 
-	/*
-	 * Disable CRTC/pipes in reverse order because some features(MST in
-	 * TGL+) requires master and slave relationship between pipes, so it
-	 * should always pick the lowest pipe as master as it will be enabled
-	 * first and disable in the reverse order so the master will be the
-	 * last one to be disabled.
-	 */
-	for_each_oldnew_intel_crtc_in_state_reverse(state, crtc, old_crtc_state,
-						    new_crtc_state, i) {
-		if (!needs_modeset(new_crtc_state))
+	/* Only disable port sync slaves */
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (!needs_modeset(new_crtc_state) || !crtc->active)
 			continue;
 
 		/* In case of Transcoder port Sync master slave CRTCs can be
@@ -14497,23 +14460,25 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		 * slave CRTCs are disabled first and then master CRTC since
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
-		if (is_trans_port_sync_mode(new_crtc_state)) {
-			if (is_trans_port_sync_master(new_crtc_state))
-				intel_trans_port_sync_modeset_disables(state,
-								       crtc,
-								       old_crtc_state,
-								       new_crtc_state);
-			else
-				continue;
-		} else {
-			intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		if (!is_trans_port_sync_mode(new_crtc_state))
+			continue;
+		if (is_trans_port_sync_master(new_crtc_state))
+			continue;
 
-			if (old_crtc_state->hw.active)
-				intel_old_crtc_state_disables(state,
-							      old_crtc_state,
-							      new_crtc_state,
-							      crtc);
-		}
+		intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		intel_old_crtc_state_disables(state, old_crtc_state,
+					      new_crtc_state, crtc);
+	}
+
+	/* Disable everything else left on */
+	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
+					    new_crtc_state, i) {
+		if (!needs_modeset(new_crtc_state) || !crtc->active)
+			continue;
+
+		intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		intel_old_crtc_state_disables(state, old_crtc_state,
+					      new_crtc_state, crtc);
 	}
 }
 
-- 
2.24.0

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

             reply	other threads:[~2019-11-20 23:48 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-20 23:48 José Roberto de Souza [this message]
2019-11-20 23:48 ` [Intel-gfx] [PATCH 1/3] drm/i915/display: Refactor intel_commit_modeset_disables() José Roberto de Souza
2019-11-20 23:48 ` [PATCH 2/3] drm/i915/display: Check the old state to find port sync slave José Roberto de Souza
2019-11-20 23:48   ` [Intel-gfx] " José Roberto de Souza
2019-11-20 23:48 ` [PATCH 3/3] drm/i915/tgl: Select master trasconder for MST stream José Roberto de Souza
2019-11-20 23:48   ` [Intel-gfx] " José Roberto de Souza
2019-11-21  1:53   ` Souza, Jose
2019-11-21  1:53     ` [Intel-gfx] " Souza, Jose
2019-11-21 21:20   ` [PATCH v2 " José Roberto de Souza
2019-11-21 21:20     ` [Intel-gfx] " José Roberto de Souza
2019-11-21  2:46 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/display: Refactor intel_commit_modeset_disables() Patchwork
2019-11-21  2:46   ` [Intel-gfx] " Patchwork
2019-11-21 21:59 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/display: Refactor intel_commit_modeset_disables() (rev2) Patchwork
2019-11-21 21:59   ` [Intel-gfx] " Patchwork
2019-11-22  2:59 ` ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/display: Refactor intel_commit_modeset_disables() Patchwork
2019-11-22  2:59   ` [Intel-gfx] " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191120234853.348731-1-jose.souza@intel.com \
    --to=jose.souza@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.