[Intel-gfx] [PATCH v2 1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes

[Intel-gfx] [PATCH v2 1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes

[José Roberto de Souza]

It should not care about DDB allocations of pipes going through
a fullmodeset, as at this point those pipes are disabled.
The comment in the code also points to that but that was not what
was being executed.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index eb0505a66ea8..68575457d40e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14603,7 +14603,7 @@ 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)
 		/* ignore allocations for crtc's that have been turned off. */
-		if (new_crtc_state->hw.active)
+		if (!needs_modeset(new_crtc_state) && new_crtc_state->hw.active)
 			entries[i] = old_crtc_state->wm.skl.ddb;
 
 	/* If 2nd DBuf slice required, enable it here */
-- 
2.24.0

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

[Intel-gfx] [PATCH v2 2/3] drm/i915/display/tgl: Fix the order of the step to turn transcoder clock off

[José Roberto de Souza]

For TGL the step to turn off the transcoder clock was moved to after
the complete shutdown of DDI. Only the MST slave transcoders should
disable the clock before that.

v2:
- Adding last_mst_stream to intel_mst_post_disable_dp, make code more
easy to read and is similar to first_mst_stream in
intel_mst_pre_enable_dp()(Ville's idea)
- Calling intel_ddi_disable_pipe_clock() for GEN12+ right
intel_disable_ddi_buf() as stated in BSpec(Ville)

BSpec: 49190
Cc: Lucas De Marchi <lucas.demarchi@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_ddi.c    | 10 +++++++++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 20 +++++++++++++++-----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ebcc7302706b..3cacb1e279c1 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3807,11 +3807,19 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	 */
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 
-	if (!is_mst)
+	if (INTEL_GEN(dev_priv) < 12 && !is_mst)
 		intel_ddi_disable_pipe_clock(old_crtc_state);
 
 	intel_disable_ddi_buf(encoder, old_crtc_state);
 
+	/*
+	 * From TGL spec: "If single stream or multi-stream master transcoder:
+	 * Configure Transcoder Clock select to direct no clock to the
+	 * transcoder"
+	 */
+	if (INTEL_GEN(dev_priv) >= 12)
+		intel_ddi_disable_pipe_clock(old_crtc_state);
+
 	intel_edp_panel_vdd_on(intel_dp);
 	intel_edp_panel_off(intel_dp);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index a1e4f4197a67..926e49f449a6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -235,8 +235,21 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
 	struct intel_connector *connector =
 		to_intel_connector(old_conn_state->connector);
+	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+	bool last_mst_stream;
 
-	intel_ddi_disable_pipe_clock(old_crtc_state);
+	intel_dp->active_mst_links--;
+	last_mst_stream = intel_dp->active_mst_links == 0;
+
+	/*
+	 * From TGL spec: "If multi-stream slave transcoder: Configure
+	 * Transcoder Clock Select to direct no clock to the transcoder"
+	 *
+	 * From older GENs spec: "Configure Transcoder Clock Select to direct
+	 * no clock to the transcoder"
+	 */
+	if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
+		intel_ddi_disable_pipe_clock(old_crtc_state);
 
 	/* this can fail */
 	drm_dp_check_act_status(&intel_dp->mst_mgr);
@@ -252,13 +265,10 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
 				     false);
 
-	intel_dp->active_mst_links--;
-
 	intel_mst->connector = NULL;
-	if (intel_dp->active_mst_links == 0) {
+	if (last_mst_stream)
 		intel_dig_port->base.post_disable(&intel_dig_port->base,
 						  old_crtc_state, NULL);
-	}
 
 	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
-- 
2.24.0

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

[Intel-gfx] [PATCH v2 3/3] drm/i915/display: Refactor intel_commit_modeset_disables()

[José Roberto de Souza]

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.

v2:
Not using crtc->active as it is deprecated

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
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 | 86 +++++++-------------
 1 file changed, 30 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 68575457d40e..a9f5aaf8df9b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14393,77 +14393,51 @@ 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(state, slave_crtc);
-	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(state, crtc);
-	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;
+	u32 handled = 0;
 	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) {
+	/* 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))
 			continue;
 
+		/* If it wasn't active no need to check the special cases */
+		if (!old_crtc_state->hw.active)
+			continue;
+
 		/* In case of Transcoder port Sync master slave CRTCs can be
 		 * assigned in any order and we need to make sure that
 		 * slave CRTCs are disabled first and then master CRTC since
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
-		if (is_trans_port_sync_mode(old_crtc_state)) {
-			if (is_trans_port_sync_master(old_crtc_state))
-				intel_trans_port_sync_modeset_disables(state,
-								       crtc,
-								       old_crtc_state,
-								       new_crtc_state);
-			else
-				continue;
-		} else {
-			intel_pre_plane_update(state, crtc);
+		if (!is_trans_port_sync_mode(old_crtc_state))
+			continue;
 
-			if (old_crtc_state->hw.active)
-				intel_old_crtc_state_disables(state,
-							      old_crtc_state,
-							      new_crtc_state,
-							      crtc);
-		}
+		if (is_trans_port_sync_master(old_crtc_state))
+			continue;
+
+		intel_pre_plane_update(state, crtc);
+		intel_old_crtc_state_disables(state, old_crtc_state,
+					      new_crtc_state, crtc);
+		handled |= BIT(crtc->pipe);
+	}
+
+	/* 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) ||
+		    (handled & BIT(crtc->pipe)))
+			continue;
+
+		intel_pre_plane_update(state, crtc);
+		if (old_crtc_state->hw.active)
+			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

Re: [Intel-gfx] [PATCH v2 1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes

[Ville Syrjälä]

On Wed, Dec 04, 2019 at 12:55:08PM -0800, José Roberto de Souza wrote:
> It should not care about DDB allocations of pipes going through
> a fullmodeset, as at this point those pipes are disabled.
> The comment in the code also points to that but that was not what
> was being executed.
> 
> 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 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index eb0505a66ea8..68575457d40e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14603,7 +14603,7 @@ 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)
>  		/* ignore allocations for crtc's that have been turned off. */
> -		if (new_crtc_state->hw.active)
> +		if (!needs_modeset(new_crtc_state) && new_crtc_state->hw.active)
>  			entries[i] = old_crtc_state->wm.skl.ddb;

Makes sense to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	/* If 2nd DBuf slice required, enable it here */
> -- 
> 2.24.0

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

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/display/tgl: Fix the order of the step to turn transcoder clock off

[Ville Syrjälä]

On Wed, Dec 04, 2019 at 12:55:09PM -0800, José Roberto de Souza wrote:
> For TGL the step to turn off the transcoder clock was moved to after
> the complete shutdown of DDI. Only the MST slave transcoders should
> disable the clock before that.
> 
> v2:
> - Adding last_mst_stream to intel_mst_post_disable_dp, make code more
> easy to read and is similar to first_mst_stream in
> intel_mst_pre_enable_dp()(Ville's idea)
> - Calling intel_ddi_disable_pipe_clock() for GEN12+ right
> intel_disable_ddi_buf() as stated in BSpec(Ville)
> 
> BSpec: 49190
> Cc: Lucas De Marchi <lucas.demarchi@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_ddi.c    | 10 +++++++++-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 20 +++++++++++++++-----
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ebcc7302706b..3cacb1e279c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3807,11 +3807,19 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>  	 */
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  
> -	if (!is_mst)
> +	if (INTEL_GEN(dev_priv) < 12 && !is_mst)
>  		intel_ddi_disable_pipe_clock(old_crtc_state);
>  
>  	intel_disable_ddi_buf(encoder, old_crtc_state);
>  
> +	/*
> +	 * From TGL spec: "If single stream or multi-stream master transcoder:
> +	 * Configure Transcoder Clock select to direct no clock to the
> +	 * transcoder"
> +	 */
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
> +

Memory is already a bit hazy, but yeah this seems like the right spot.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	intel_edp_panel_vdd_on(intel_dp);
>  	intel_edp_panel_off(intel_dp);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index a1e4f4197a67..926e49f449a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -235,8 +235,21 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
> +	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> +	bool last_mst_stream;
>  
> -	intel_ddi_disable_pipe_clock(old_crtc_state);
> +	intel_dp->active_mst_links--;
> +	last_mst_stream = intel_dp->active_mst_links == 0;
> +
> +	/*
> +	 * From TGL spec: "If multi-stream slave transcoder: Configure
> +	 * Transcoder Clock Select to direct no clock to the transcoder"
> +	 *
> +	 * From older GENs spec: "Configure Transcoder Clock Select to direct
> +	 * no clock to the transcoder"
> +	 */
> +	if (INTEL_GEN(dev_priv) < 12 || !last_mst_stream)
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
>  
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
> @@ -252,13 +265,10 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port,
>  				     false);
>  
> -	intel_dp->active_mst_links--;
> -
>  	intel_mst->connector = NULL;
> -	if (intel_dp->active_mst_links == 0) {
> +	if (last_mst_stream)
>  		intel_dig_port->base.post_disable(&intel_dig_port->base,
>  						  old_crtc_state, NULL);
> -	}
>  
>  	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
>  }
> -- 
> 2.24.0

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes
URL   : https://patchwork.freedesktop.org/series/70462/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7486 -> Patchwork_15591
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/index.html

Known issues
------------

  Here are the changes found in Patchwork_15591 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-ilk-650:         [PASS][1] -> [DMESG-WARN][2] ([i915#116])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-ilk-650/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-ilk-650/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-lmem:        [PASS][3] -> [DMESG-WARN][4] ([i915#592])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-hsw-peppy:       [PASS][5] -> [INCOMPLETE][6] ([i915#694])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-guc:         [PASS][7] -> [FAIL][8] ([i915#49])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-icl-guc/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][9] ([i915#683]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-hsw-4770/igt@i915_selftest@live_blt.html
    - fi-bsw-nick:        [DMESG-FAIL][11] ([i915#563]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-bsw-nick/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-bsw-nick/igt@i915_selftest@live_blt.html
    - fi-hsw-4770r:       [DMESG-FAIL][13] ([i915#683]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [INCOMPLETE][15] ([i915#45]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([fdo#107139] / [i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][18] ([fdo#107139] / [i915#62] / [i915#92])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-kbl-x1275/igt@gem_exec_suspend@basic-s4-devices.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-kbl-x1275/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@kms_busy@basic-flip-pipe-b:
    - fi-kbl-x1275:       [DMESG-WARN][19] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][20] ([i915#62] / [i915#92]) +6 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-kbl-x1275/igt@kms_busy@basic-flip-pipe-b.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-kbl-x1275/igt@kms_busy@basic-flip-pipe-b.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][21] ([i915#62] / [i915#92]) -> [DMESG-WARN][22] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7486/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [i915#116]: https://gitlab.freedesktop.org/drm/intel/issues/116
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#563]: https://gitlab.freedesktop.org/drm/intel/issues/563
  [i915#592]: https://gitlab.freedesktop.org/drm/intel/issues/592
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#683]: https://gitlab.freedesktop.org/drm/intel/issues/683
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (53 -> 46)
------------------------------

  Additional (1): fi-cfl-8109u 
  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7486 -> Patchwork_15591

  CI-20190529: 20190529
  CI_DRM_7486: 8bd05bdd30fdd1b56307b780ab0f0e8de878c75a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5331: dad473697b2e6bb5c45d7fec533b20d5dbe4fa17 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15591: 1302551b35de701b9185458a5ae959b85736048c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1302551b35de drm/i915/display: Refactor intel_commit_modeset_disables()
aad423b6e066 drm/i915/display/tgl: Fix the order of the step to turn transcoder clock off
0155230f77e3 drm/i915/display: Do not check for the ddb allocations of turned off pipes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15591/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Refactor intel_commit_modeset_disables()

[Ville Syrjälä]

On Wed, Dec 04, 2019 at 12:55:10PM -0800, José Roberto de Souza wrote:
> 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.
> 
> v2:
> Not using crtc->active as it is deprecated
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> 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 | 86 +++++++-------------
>  1 file changed, 30 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 68575457d40e..a9f5aaf8df9b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14393,77 +14393,51 @@ 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(state, slave_crtc);
> -	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(state, crtc);
> -	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;
> +	u32 handled = 0;
>  	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) {
> +	/* 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))
>  			continue;
>  
> +		/* If it wasn't active no need to check the special cases */

This comment seems a bit redundant.

> +		if (!old_crtc_state->hw.active)
> +			continue;
> +
>  		/* In case of Transcoder port Sync master slave CRTCs can be
>  		 * assigned in any order and we need to make sure that
>  		 * slave CRTCs are disabled first and then master CRTC since
>  		 * Slave vblanks are masked till Master Vblanks.
>  		 */
> -		if (is_trans_port_sync_mode(old_crtc_state)) {
> -			if (is_trans_port_sync_master(old_crtc_state))
> -				intel_trans_port_sync_modeset_disables(state,
> -								       crtc,
> -								       old_crtc_state,
> -								       new_crtc_state);
> -			else
> -				continue;
> -		} else {
> -			intel_pre_plane_update(state, crtc);
> +		if (!is_trans_port_sync_mode(old_crtc_state))
> +			continue;
>  
> -			if (old_crtc_state->hw.active)
> -				intel_old_crtc_state_disables(state,
> -							      old_crtc_state,
> -							      new_crtc_state,
> -							      crtc);
> -		}
> +		if (is_trans_port_sync_master(old_crtc_state))
> +			continue;

I'd still like to see is_trans_port_sync_slave() so we could eliminate
the double check here.

> +
> +		intel_pre_plane_update(state, crtc);
> +		intel_old_crtc_state_disables(state, old_crtc_state,
> +					      new_crtc_state, crtc);
> +		handled |= BIT(crtc->pipe);
> +	}
> +
> +	/* 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) ||
> +		    (handled & BIT(crtc->pipe)))
> +			continue;
> +
> +		intel_pre_plane_update(state, crtc);
> +		if (old_crtc_state->hw.active)
> +			intel_old_crtc_state_disables(state, old_crtc_state,
> +						      new_crtc_state, crtc);
>  	}

The only concern left is the new ordering when we have multiple
sets of genlocked pipes. Assuming the hardware is sane I don't
think anything *should* break. But who knows.

Apart from that:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  }
>  
> -- 
> 2.24.0

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

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes (rev2)
URL   : https://patchwork.freedesktop.org/series/70462/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7491 -> Patchwork_15610
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/index.html

Known issues
------------

  Here are the changes found in Patchwork_15610 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [PASS][1] -> [DMESG-FAIL][2] ([i915#683])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-hsw-4770/igt@i915_selftest@live_blt.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-icl-dsi:         [PASS][3] -> [DMESG-WARN][4] ([i915#109]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-icl-dsi/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-icl-dsi/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - {fi-tgl-guc}:       [INCOMPLETE][5] ([i915#435]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-tgl-guc/igt@gem_close_race@basic-threads.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-tgl-guc/igt@gem_close_race@basic-threads.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-lmem:        [DMESG-WARN][7] ([i915#592]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-skl-lmem/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-byt-j1900:       [DMESG-FAIL][9] ([i915#725]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-byt-j1900/igt@i915_selftest@live_blt.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-byt-j1900/igt@i915_selftest@live_blt.html
    - fi-hsw-4770r:       [DMESG-FAIL][11] ([i915#683]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#62] / [i915#92] / [i915#95]) +6 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][15] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][16] ([i915#62] / [i915#92]) +6 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
  [i915#592]: https://gitlab.freedesktop.org/drm/intel/issues/592
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#683]: https://gitlab.freedesktop.org/drm/intel/issues/683
  [i915#710]: https://gitlab.freedesktop.org/drm/intel/issues/710
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (53 -> 47)
------------------------------

  Additional (1): fi-bdw-5557u 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7491 -> Patchwork_15610

  CI-20190529: 20190529
  CI_DRM_7491: ac5e57322614f1c08b00ed9f52153ca12603c637 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5333: e08522bb09ff1b9720359b3867da7e4aca0bd5f1 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15610: 37b90176460c723b8d66a0d4ae9d553bed353ffc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

37b90176460c drm/i915/display: Refactor intel_commit_modeset_disables()
08fdd58952a0 drm/i915/display/tgl: Fix the order of the step to turn transcoder clock off
02f6f4d716cd drm/i915/display: Do not check for the ddb allocations of turned off pipes

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Refactor intel_commit_modeset_disables()

[Souza, Jose]

On Thu, 2019-12-05 at 12:38 +0200, Ville Syrjälä wrote:
> On Wed, Dec 04, 2019 at 12:55:10PM -0800, José Roberto de Souza
> wrote:
> > 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.
> > 
> > v2:
> > Not using crtc->active as it is deprecated
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > 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 | 86 +++++++---------
> > ----
> >  1 file changed, 30 insertions(+), 56 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 68575457d40e..a9f5aaf8df9b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14393,77 +14393,51 @@ 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(state, slave_crtc);
> > -	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(state, crtc);
> > -	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;
> > +	u32 handled = 0;
> >  	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)
> > {
> > +	/* 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))
> >  			continue;
> >  
> > +		/* If it wasn't active no need to check the special
> > cases */
> 
> This comment seems a bit redundant.

Okay, removing it.

> 
> > +		if (!old_crtc_state->hw.active)
> > +			continue;
> > +
> >  		/* In case of Transcoder port Sync master slave CRTCs
> > can be
> >  		 * assigned in any order and we need to make sure that
> >  		 * slave CRTCs are disabled first and then master CRTC
> > since
> >  		 * Slave vblanks are masked till Master Vblanks.
> >  		 */
> > -		if (is_trans_port_sync_mode(old_crtc_state)) {
> > -			if (is_trans_port_sync_master(old_crtc_state))
> > -				intel_trans_port_sync_modeset_disables(
> > state,
> > -								       
> > crtc,
> > -								       
> > old_crtc_state,
> > -								       
> > new_crtc_state);
> > -			else
> > -				continue;
> > -		} else {
> > -			intel_pre_plane_update(state, crtc);
> > +		if (!is_trans_port_sync_mode(old_crtc_state))
> > +			continue;
> >  
> > -			if (old_crtc_state->hw.active)
> > -				intel_old_crtc_state_disables(state,
> > -							      old_crtc_
> > state,
> > -							      new_crtc_
> > state,
> > -							      crtc);
> > -		}
> > +		if (is_trans_port_sync_master(old_crtc_state))
> > +			continue;
> 
> I'd still like to see is_trans_port_sync_slave() so we could
> eliminate
> the double check here.

Thinking again, we only need

if (is_trans_port_sync_master(old_crtc_state))
	continue;

here, no is_trans_port_sync_master() required.

Adding the is_trans_port_sync_slave() would leave use with
if (!is_trans_port_sync_master(old_crtc_state))
	continue;

or with a indented block.

Fixing it...

> 
> > +
> > +		intel_pre_plane_update(state, crtc);
> > +		intel_old_crtc_state_disables(state, old_crtc_state,
> > +					      new_crtc_state, crtc);
> > +		handled |= BIT(crtc->pipe);
> > +	}
> > +
> > +	/* 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) ||
> > +		    (handled & BIT(crtc->pipe)))
> > +			continue;
> > +
> > +		intel_pre_plane_update(state, crtc);
> > +		if (old_crtc_state->hw.active)
> > +			intel_old_crtc_state_disables(state,
> > old_crtc_state,
> > +						      new_crtc_state,
> > crtc);
> >  	}
> 
> The only concern left is the new ordering when we have multiple
> sets of genlocked pipes. Assuming the hardware is sane I don't
> think anything *should* break. But who knows.
> 
> Apart from that:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 

Thanks, will send a new version with the small changes above and keep
your rvb.

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

[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/display: Do not check for the ddb allocations of turned off pipes (rev2)
URL   : https://patchwork.freedesktop.org/series/70462/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7491_full -> Patchwork_15610_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15610_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15610_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15610_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_persistence@bcs0-mixed-process:
    - shard-skl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl2/igt@gem_ctx_persistence@bcs0-mixed-process.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl2/igt@gem_ctx_persistence@bcs0-mixed-process.html

  * igt@gem_exec_parse_blt@allowed-single:
    - shard-skl:          [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl1/igt@gem_exec_parse_blt@allowed-single.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl8/igt@gem_exec_parse_blt@allowed-single.html

  
Known issues
------------

  Here are the changes found in Patchwork_15610_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-kbl:          [PASS][5] -> [DMESG-WARN][6] ([i915#180]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl7/igt@gem_ctx_isolation@rcs0-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl3/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_persistence@vcs1-mixed-process:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#109276] / [fdo#112080]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb1/igt@gem_ctx_persistence@vcs1-mixed-process.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb6/igt@gem_ctx_persistence@vcs1-mixed-process.html

  * igt@gem_exec_nop@basic-series:
    - shard-tglb:         [PASS][9] -> [INCOMPLETE][10] ([i915#435])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb7/igt@gem_exec_nop@basic-series.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb6/igt@gem_exec_nop@basic-series.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112080]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@out-order-bsd1:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109276]) +8 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@gem_exec_schedule@out-order-bsd1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@gem_exec_schedule@out-order-bsd1.html

  * igt@gem_exec_suspend@basic-s3-devices:
    - shard-tglb:         [PASS][15] -> [INCOMPLETE][16] ([i915#456] / [i915#460])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb9/igt@gem_exec_suspend@basic-s3-devices.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb1/igt@gem_exec_suspend@basic-s3-devices.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-iclb:         [PASS][17] -> [INCOMPLETE][18] ([i915#140] / [i915#530])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-snb:          [PASS][19] -> [DMESG-WARN][20] ([fdo#111870])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-snb2/igt@gem_userptr_blits@sync-unmap-after-close.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-snb1/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@kms_big_fb@y-tiled-16bpp-rotate-180:
    - shard-kbl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#103665]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl1/igt@kms_big_fb@y-tiled-16bpp-rotate-180.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl6/igt@kms_big_fb@y-tiled-16bpp-rotate-180.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-kbl:          [PASS][23] -> [DMESG-WARN][24] ([i915#728]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl7/igt@kms_ccs@pipe-a-crc-sprite-planes-basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl6/igt@kms_ccs@pipe-a-crc-sprite-planes-basic.html

  * igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque:
    - shard-kbl:          [PASS][25] -> [FAIL][26] ([i915#54])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl6/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([i915#54]) +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-apl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [PASS][31] -> [FAIL][32] ([i915#96])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [PASS][33] -> [INCOMPLETE][34] ([i915#69])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl8/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([i915#79])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
    - shard-tglb:         [PASS][37] -> [DMESG-WARN][38] ([i915#728]) +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-iclb:         [PASS][39] -> [INCOMPLETE][40] ([i915#140])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-rte.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-rte.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][41] -> [INCOMPLETE][42] ([fdo#103927])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-apl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-apl1/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [PASS][43] -> [FAIL][44] ([i915#49]) +6 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-skl:          [PASS][45] -> [DMESG-WARN][46] ([i915#728]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][47] -> [SKIP][48] ([fdo#109642] / [fdo#111068]) +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][49] -> [SKIP][50] ([fdo#109441])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb4/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][51] -> [FAIL][52] ([i915#31])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-apl1/igt@kms_setmode@basic.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-apl2/igt@kms_setmode@basic.html

  * igt@perf_pmu@multi-client-rcs0:
    - shard-iclb:         [PASS][53] -> [DMESG-WARN][54] ([i915#728]) +2 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb6/igt@perf_pmu@multi-client-rcs0.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb6/igt@perf_pmu@multi-client-rcs0.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-skl:          [INCOMPLETE][55] ([i915#69]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl2/igt@gem_ctx_isolation@vcs0-s3.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl1/igt@gem_ctx_isolation@vcs0-s3.html

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [SKIP][57] ([fdo#109276] / [fdo#112080]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@gem_ctx_persistence@vcs1-queued.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_shared@q-smoketest-vebox:
    - shard-tglb:         [INCOMPLETE][59] ([fdo#111735]) -> [PASS][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb6/igt@gem_ctx_shared@q-smoketest-vebox.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb5/igt@gem_ctx_shared@q-smoketest-vebox.html

  * igt@gem_eio@in-flight-contexts-1us:
    - shard-snb:          [INCOMPLETE][61] ([i915#82]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-snb7/igt@gem_eio@in-flight-contexts-1us.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-snb1/igt@gem_eio@in-flight-contexts-1us.html

  * igt@gem_exec_nop@basic-sequential:
    - shard-tglb:         [INCOMPLETE][63] ([i915#435]) -> [PASS][64] +1 similar issue
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb6/igt@gem_exec_nop@basic-sequential.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb4/igt@gem_exec_nop@basic-sequential.html

  * igt@gem_exec_parallel@rcs0-fds:
    - shard-hsw:          [DMESG-WARN][65] -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-hsw7/igt@gem_exec_parallel@rcs0-fds.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-hsw6/igt@gem_exec_parallel@rcs0-fds.html

  * igt@gem_exec_schedule@in-order-bsd2:
    - shard-iclb:         [SKIP][67] ([fdo#109276]) -> [PASS][68] +2 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@gem_exec_schedule@in-order-bsd2.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@gem_exec_schedule@in-order-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][69] ([fdo#112146]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_exec_schedule@smoketest-vebox:
    - shard-tglb:         [INCOMPLETE][71] ([i915#707]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb3/igt@gem_exec_schedule@smoketest-vebox.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb9/igt@gem_exec_schedule@smoketest-vebox.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-kbl:          [INCOMPLETE][73] ([fdo#103665] / [i915#530]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl6/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl1/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive:
    - shard-hsw:          [TIMEOUT][75] ([i915#530]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-hsw2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-hsw4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-snb:          [TIMEOUT][77] ([i915#530]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-snb4/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-snb:          [FAIL][79] ([i915#520]) -> [PASS][80]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-snb7/igt@gem_persistent_relocs@forked-thrashing.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-snb2/igt@gem_persistent_relocs@forked-thrashing.html

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-tglb:         [INCOMPLETE][81] ([i915#470] / [i915#475]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb6/igt@gem_ppgtt@blt-vs-render-ctxn.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb5/igt@gem_ppgtt@blt-vs-render-ctxn.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [DMESG-WARN][83] ([fdo#111870]) -> [PASS][84] +2 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-snb7/igt@gem_userptr_blits@sync-unmap-cycles.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-snb7/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [DMESG-WARN][85] ([i915#180]) -> [PASS][86] +3 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-apl4/igt@gem_workarounds@suspend-resume-context.html

  * igt@i915_hangman@error-state-capture-vcs1:
    - shard-iclb:         [SKIP][87] ([fdo#112080]) -> [PASS][88] +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@i915_hangman@error-state-capture-vcs1.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@i915_hangman@error-state-capture-vcs1.html

  * igt@kms_big_fb@x-tiled-8bpp-rotate-180:
    - shard-kbl:          [INCOMPLETE][89] ([fdo#103665]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl4/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl1/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][91] ([i915#180]) -> [PASS][92] +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding:
    - shard-skl:          [FAIL][93] ([i915#54]) -> [PASS][94]
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled:
    - shard-skl:          [INCOMPLETE][95] -> [PASS][96]
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl4/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl8/igt@kms_draw_crc@draw-method-xrgb8888-blt-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][97] ([i915#79]) -> [PASS][98]
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [INCOMPLETE][99] ([i915#61]) -> [PASS][100]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-hsw4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-hsw6/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - shard-kbl:          [DMESG-WARN][101] ([i915#728]) -> [PASS][102]
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][103] ([i915#49]) -> [PASS][104] +4 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt:
    - shard-tglb:         [FAIL][105] ([i915#49]) -> [PASS][106]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-tglb:         [INCOMPLETE][107] ([i915#456] / [i915#460] / [i915#474]) -> [PASS][108]
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         [DMESG-WARN][109] ([i915#728]) -> [PASS][110] +3 similar issues
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [DMESG-FAIL][111] ([i915#49]) -> [PASS][112]
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-tglb:         [INCOMPLETE][113] ([i915#456] / [i915#460]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb1/igt@kms_frontbuffer_tracking@psr-suspend.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb6/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-tglb:         [FAIL][115] ([fdo#111842]) -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb6/igt@kms_psr2_su@frontbuffer.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb5/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [SKIP][117] ([fdo#109441]) -> [PASS][118]
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@kms_psr@psr2_dpms.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@kms_psr@psr2_dpms.html

  * igt@perf_pmu@busy-no-semaphores-rcs0:
    - shard-skl:          [DMESG-WARN][119] ([i915#728]) -> [PASS][120] +3 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl2/igt@perf_pmu@busy-no-semaphores-rcs0.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl2/igt@perf_pmu@busy-no-semaphores-rcs0.html

  * igt@perf_pmu@render-node-busy-vcs1:
    - shard-tglb:         [DMESG-WARN][121] ([i915#728]) -> [PASS][122] +1 similar issue
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-tglb2/igt@perf_pmu@render-node-busy-vcs1.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-tglb2/igt@perf_pmu@render-node-busy-vcs1.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-iclb:         [DMESG-FAIL][123] ([i915#49]) -> [DMESG-WARN][124] ([i915#728])
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [DMESG-WARN][125] ([i915#728]) -> [FAIL][126] ([i915#49])
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-iclb:         [INCOMPLETE][127] ([i915#140]) -> [DMESG-WARN][128] ([i915#728])
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt:
    - shard-kbl:          [DMESG-WARN][129] ([i915#728]) -> [INCOMPLETE][130] ([fdo#103665] / [i915#634])
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-kbl:          [DMESG-WARN][131] ([i915#728]) -> [INCOMPLETE][132] ([fdo#103665])
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl3/igt@kms_plane@pixel-format-pipe-a-planes.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl1/igt@kms_plane@pixel-format-pipe-a-planes.html
    - shard-skl:          [INCOMPLETE][133] ([fdo#112391] / [i915#648]) -> [INCOMPLETE][134] ([fdo#112347] / [i915#648])
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl8/igt@kms_plane@pixel-format-pipe-a-planes.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl10/igt@kms_plane@pixel-format-pipe-a-planes.html

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-kbl:          [INCOMPLETE][135] ([fdo#103665]) -> [DMESG-WARN][136] ([i915#728])
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl3/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl2/igt@kms_plane@pixel-format-pipe-a-planes-source-clamping.html

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          [INCOMPLETE][137] ([fdo#112347] / [i915#648]) -> [INCOMPLETE][138] ([i915#648])
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-skl6/igt@kms_plane@pixel-format-pipe-b-planes.html
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-skl9/igt@kms_plane@pixel-format-pipe-b-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [INCOMPLETE][139] ([fdo#103665]) -> [DMESG-WARN][140] ([i915#180])
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7491/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
  [fdo#111842]: https://bugs.freedesktop.org/show_bug.cgi?id=111842
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112347]: https://bugs.freedesktop.org/show_bug.cgi?id=112347
  [fdo#112391]: https://bugs.freedesktop.org/show_bug.cgi?id=112391
  [i915#140]: https://gitlab.freedesktop.org/drm/intel/issues/140
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
  [i915#456]: https://gitlab.freedesktop.org/drm/intel/issues/456
  [i915#460]: https://gitlab.freedesktop.org/drm/intel/issues/460
  [i915#470]: https://gitlab.freedesktop.org/drm/intel/issues/470
  [i915#474]: https://gitlab.freedesktop.org/drm/intel/issues/474
  [i915#475]: https://gitlab.freedesktop.org/drm/intel/issues/475
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#520]: https://gitlab.freedesktop.org/drm/intel/issues/520
  [i915#530]: https://gitlab.freedesktop.org/drm/intel/issues/530
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#634]: https://gitlab.freedesktop.org/drm/intel/issues/634
  [i915#648]: https://gitlab.freedesktop.org/drm/intel/issues/648
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#707]: https://gitlab.freedesktop.org/drm/intel/issues/707
  [i915#728]: https://gitlab.freedesktop.org/drm/intel/issues/728
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15610/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/display: Refactor intel_commit_modeset_disables()

[Ville Syrjälä]

On Thu, Dec 05, 2019 at 08:28:51PM +0000, Souza, Jose wrote:
> On Thu, 2019-12-05 at 12:38 +0200, Ville Syrjälä wrote:
> > On Wed, Dec 04, 2019 at 12:55:10PM -0800, José Roberto de Souza
> > wrote:
> > > 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.
> > > 
> > > v2:
> > > Not using crtc->active as it is deprecated
> > > 
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > 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 | 86 +++++++---------
> > > ----
> > >  1 file changed, 30 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 68575457d40e..a9f5aaf8df9b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -14393,77 +14393,51 @@ 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(state, slave_crtc);
> > > -	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(state, crtc);
> > > -	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;
> > > +	u32 handled = 0;
> > >  	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)
> > > {
> > > +	/* 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))
> > >  			continue;
> > >  
> > > +		/* If it wasn't active no need to check the special
> > > cases */
> > 
> > This comment seems a bit redundant.
> 
> Okay, removing it.
> 
> > 
> > > +		if (!old_crtc_state->hw.active)
> > > +			continue;
> > > +
> > >  		/* In case of Transcoder port Sync master slave CRTCs
> > > can be
> > >  		 * assigned in any order and we need to make sure that
> > >  		 * slave CRTCs are disabled first and then master CRTC
> > > since
> > >  		 * Slave vblanks are masked till Master Vblanks.
> > >  		 */
> > > -		if (is_trans_port_sync_mode(old_crtc_state)) {
> > > -			if (is_trans_port_sync_master(old_crtc_state))
> > > -				intel_trans_port_sync_modeset_disables(
> > > state,
> > > -								       
> > > crtc,
> > > -								       
> > > old_crtc_state,
> > > -								       
> > > new_crtc_state);
> > > -			else
> > > -				continue;
> > > -		} else {
> > > -			intel_pre_plane_update(state, crtc);
> > > +		if (!is_trans_port_sync_mode(old_crtc_state))
> > > +			continue;
> > >  
> > > -			if (old_crtc_state->hw.active)
> > > -				intel_old_crtc_state_disables(state,
> > > -							      old_crtc_
> > > state,
> > > -							      new_crtc_
> > > state,
> > > -							      crtc);
> > > -		}
> > > +		if (is_trans_port_sync_master(old_crtc_state))
> > > +			continue;
> > 
> > I'd still like to see is_trans_port_sync_slave() so we could
> > eliminate
> > the double check here.
> 
> Thinking again, we only need
> 
> if (is_trans_port_sync_master(old_crtc_state))
> 	continue;
> 
> here, no is_trans_port_sync_master() required.

Only if we want to also handle the non-sync pipes here. Which I don't
think we want.

> 
> Adding the is_trans_port_sync_slave() would leave use with
> if (!is_trans_port_sync_master(old_crtc_state))
> 	continue;
> 
> or with a indented block.
> 
> Fixing it...
> 
> > 
> > > +
> > > +		intel_pre_plane_update(state, crtc);
> > > +		intel_old_crtc_state_disables(state, old_crtc_state,
> > > +					      new_crtc_state, crtc);
> > > +		handled |= BIT(crtc->pipe);
> > > +	}
> > > +
> > > +	/* 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) ||
> > > +		    (handled & BIT(crtc->pipe)))
> > > +			continue;
> > > +
> > > +		intel_pre_plane_update(state, crtc);
> > > +		if (old_crtc_state->hw.active)
> > > +			intel_old_crtc_state_disables(state,
> > > old_crtc_state,
> > > +						      new_crtc_state,
> > > crtc);
> > >  	}
> > 
> > The only concern left is the new ordering when we have multiple
> > sets of genlocked pipes. Assuming the hardware is sane I don't
> > think anything *should* break. But who knows.
> > 
> > Apart from that:
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> 
> Thanks, will send a new version with the small changes above and keep
> your rvb.
> 
> > >  }
> > >  
> > > -- 
> > > 2.24.0

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