All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC 0/6] 3 display pipes combination system support
@ 2020-01-23 13:26 Anshuman Gupta
  2020-01-23 13:26 ` [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

This is a proposed RFC solution for 3 display pipes combination
system support.

Anshuman Gupta (6):
  drm/i915: Iterate over pipe and skip the disabled one
  drm/i915: Remove (pipe == crtc->index) asummption
  drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  drm/i915: Get right max plane stride
  drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
  drm/i915: Enable 3 display pipes support

 drivers/gpu/drm/i915/display/intel_display.c  | 40 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_display.h  |  5 ++-
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/i915_irq.c               |  6 ++-
 drivers/gpu/drm/i915/intel_device_info.c      |  7 ++--
 5 files changed, 45 insertions(+), 15 deletions(-)

-- 
2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:48   ` Jani Nikula
  2020-01-23 13:26 ` [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption Anshuman Gupta
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

It should not be assumed that a disabled display pipe will be
always last the pipe.
for_each_pipe() should iterate over I915_MAX_PIPES and check
for the disabled pipe and skip that pipe so that it should not
initialize the intel crtc for any disabled pipes.

Few compilation error needed to handle accordingly due to
change in for_each_pipe() macro.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
 drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 028aab728514..47813a50add4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -312,10 +312,11 @@ enum phy_fia {
 };
 
 #define for_each_pipe(__dev_priv, __p) \
-	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
+	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \
+		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
 
 #define for_each_pipe_masked(__dev_priv, __p, __mask) \
-	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
+	for_each_pipe(__dev_priv, __p) \
 		for_each_if((__mask) & BIT(__p))
 
 #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 94cb25ac504d..22ecd5bc407e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 	if (pch_iir & SDE_POISON)
 		DRM_ERROR("PCH poison interrupt\n");
 
-	if (pch_iir & SDE_FDI_MASK)
+	if (pch_iir & SDE_FDI_MASK) {
 		for_each_pipe(dev_priv, pipe)
 			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
 					 pipe_name(pipe),
 					 I915_READ(FDI_RX_IIR(pipe)));
+	}
 
 	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
 		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
@@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
 	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
 		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
 
-	if (pch_iir & SDE_FDI_MASK_CPT)
+	if (pch_iir & SDE_FDI_MASK_CPT) {
 		for_each_pipe(dev_priv, pipe)
 			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
 					 pipe_name(pipe),
 					 I915_READ(FDI_RX_IIR(pipe)));
+	}
 
 	if (pch_iir & SDE_ERROR_CPT)
 		cpt_serr_int_handler(dev_priv);
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
  2020-01-23 13:26 ` [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:40   ` Ville Syrjälä
  2020-01-23 13:49   ` Jani Nikula
  2020-01-23 13:26 ` [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

we can't have (pipe == crtc->index) assumption in
driver in order to support 3 non-contiguous
display pipe system.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 878d331b9e8c..afd8d43160c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
 	if (new_crtc_state->hw.active)
 		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
 				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
-				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
+				pipe_name(crtc->pipe), pll->active_mask);
 	else
 		I915_STATE_WARN(pll->active_mask & crtc_mask,
 				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
-				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
+				pipe_name(crtc->pipe), pll->active_mask);
 
 	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
 			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
@@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
 
 		I915_STATE_WARN(pll->active_mask & crtc_mask,
 				"pll active mismatch (didn't expect pipe %c in active mask)\n",
-				pipe_name(drm_crtc_index(&crtc->base)));
+				pipe_name(crtc->pipe));
 		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
 				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
-				pipe_name(drm_crtc_index(&crtc->base)));
+				pipe_name(crtc->pipe));
 	}
 }
 
@@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 
 	intel_color_init(crtc);
 
-	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
-
 	return 0;
 
 fail:
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
  2020-01-23 13:26 ` [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
  2020-01-23 13:26 ` [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:47   ` Ville Syrjälä
  2020-01-23 13:26 ` [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride Anshuman Gupta
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

As a disabled pipe in pipe_mask is not having a valid intel crtc,
driver wrongly populates the possible_crtcs mask while initializing
the plane for a CRTC. Fixing up the plane possible_crtc mask.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index afd8d43160c6..b250b31f6000 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16407,6 +16407,28 @@ static void intel_crtc_free(struct intel_crtc *crtc)
 	kfree(crtc);
 }
 
+static void intel_plane_possible_crtc_fixup(struct drm_i915_private *dev_priv)
+{
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
+
+	/*
+	 * if in case the disabled fused pipe is not the last pipe,
+	 * it requires to fix the wrong populated possible_crtcs mask.
+	 */
+	if (is_power_of_2(INTEL_INFO(dev_priv)->pipe_mask + 1))
+		return;
+
+	for_each_intel_crtc(&dev_priv->drm, crtc) {
+		for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
+			if (WARN_ON(!(plane->base.possible_crtcs & BIT(crtc->pipe))))
+				return;
+			plane->base.possible_crtcs =
+					drm_crtc_mask(&crtc->base);
+		}
+	}
+}
+
 static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
 	struct intel_plane *primary, *cursor;
@@ -17544,6 +17566,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
 		}
 	}
 
+	intel_plane_possible_crtc_fixup(i915);
 	intel_shared_dpll_init(dev);
 	intel_update_fdi_pll_freq(i915);
 
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (2 preceding siblings ...)
  2020-01-23 13:26 ` [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:50   ` Ville Syrjälä
  2020-01-23 13:26 ` [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

intel_plane_fb_max_stride should return the max stride of
primary plane for first available pipe in intel device info
pipe_mask.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b250b31f6000..b9209b7e71d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2703,12 +2703,15 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
 {
 	struct intel_crtc *crtc;
 	struct intel_plane *plane;
+	enum pipe pipe;
 
 	/*
 	 * We assume the primary plane for pipe A has
-	 * the highest stride limits of them all.
+	 * the highest stride limits of them all,
+	 * if in case pipe A is disabled, use the first pipe from pipe_mask.
 	 */
-	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
+	pipe = ffs(INTEL_INFO(dev_priv)->pipe_mask) - 1;
+	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
 	if (!crtc)
 		return 0;
 
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (3 preceding siblings ...)
  2020-01-23 13:26 ` [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:52   ` Ville Syrjälä
  2020-01-23 13:26 ` [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support Anshuman Gupta
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Add a WARN_ON for a disabled pipe in pipe_mask at
intel_get_crtc_for_pipe() function.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 33ba93863488..ca8d1e17814e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1426,6 +1426,8 @@ vlv_pipe_to_channel(enum pipe pipe)
 static inline struct intel_crtc *
 intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
+	/* pipe_to_crtc_mapping may have hole on any of 3 display pipe system */
+	WARN_ON(!(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)));
 	return dev_priv->pipe_to_crtc_mapping[pipe];
 }
 
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (4 preceding siblings ...)
  2020-01-23 13:26 ` [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
@ 2020-01-23 13:26 ` Anshuman Gupta
  2020-01-23 13:53   ` Ville Syrjälä
  2020-01-23 21:38 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for 3 display pipes combination system support Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-23 13:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Allow 3-display pipes SKU system with any combination
in INTEL_INFO pipe mask.
B.Spec:50075

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index fcdacd6d4aa5..459047c98e82 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -997,10 +997,11 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 
 		/*
 		 * At least one pipe should be enabled and if there are
-		 * disabled pipes, they should be the last ones, with no holes
-		 * in the mask.
+		 * disabled pipes, up to Display Gen<=12, they should be the
+		 * last ones, with no holses in the mask.
 		 */
-		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
+		if (enabled_mask == 0 || (!is_power_of_2(enabled_mask + 1) &&
+		    (INTEL_GEN(dev_priv) <= 11 || IS_TIGERLAKE(dev_priv))))
 			drm_err(&dev_priv->drm,
 				"invalid pipe fuse configuration: enabled_mask=0x%x\n",
 				enabled_mask);
-- 
2.24.0

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-23 13:26 ` [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption Anshuman Gupta
@ 2020-01-23 13:40   ` Ville Syrjälä
  2020-01-30 12:02     ` Anshuman Gupta
  2020-01-23 13:49   ` Jani Nikula
  1 sibling, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:40 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 23, 2020 at 06:56:55PM +0530, Anshuman Gupta wrote:
> we can't have (pipe == crtc->index) assumption in
> driver in order to support 3 non-contiguous
> display pipe system.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 878d331b9e8c..afd8d43160c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
>  	if (new_crtc_state->hw.active)
>  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
>  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  	else
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  
>  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
>  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> @@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
>  
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
>  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  	}
>  }
>  
> @@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	intel_color_init(crtc);
>  
> -	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
> -

The first and second hunks don't really have anything to do with
each other. Also the WARN_ON() should not be removed until all the
assumptions are fixed.

>  	return 0;
>  
>  fail:
> -- 
> 2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-01-23 13:26 ` [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
@ 2020-01-23 13:47   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:47 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 23, 2020 at 06:56:56PM +0530, Anshuman Gupta wrote:
> As a disabled pipe in pipe_mask is not having a valid intel crtc,
> driver wrongly populates the possible_crtcs mask while initializing
> the plane for a CRTC. Fixing up the plane possible_crtc mask.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 23 ++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index afd8d43160c6..b250b31f6000 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16407,6 +16407,28 @@ static void intel_crtc_free(struct intel_crtc *crtc)
>  	kfree(crtc);
>  }
>  
> +static void intel_plane_possible_crtc_fixup(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
> +
> +	/*
> +	 * if in case the disabled fused pipe is not the last pipe,
> +	 * it requires to fix the wrong populated possible_crtcs mask.
> +	 */
> +	if (is_power_of_2(INTEL_INFO(dev_priv)->pipe_mask + 1))
> +		return;

I don't undestand what you're trying to do here. Looks totally
pointless.

> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) {
> +			if (WARN_ON(!(plane->base.possible_crtcs & BIT(crtc->pipe))))
> +				return;

Rather ugly abuse of possible_crtcs. I would remove the current
possible_crtcs assignments totally, and just do something simple like

for_each_intel_plane() {
	crtc = crtc_for_pipe(plane->pipe);
	plane->possible_crtcs = crtc_mask(&crtc->base);
}


> +			plane->base.possible_crtcs =
> +					drm_crtc_mask(&crtc->base);
> +		}
> +	}
> +}
> +
>  static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
>  	struct intel_plane *primary, *cursor;
> @@ -17544,6 +17566,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
>  		}
>  	}
>  
> +	intel_plane_possible_crtc_fixup(i915);
>  	intel_shared_dpll_init(dev);
>  	intel_update_fdi_pll_freq(i915);
>  
> -- 
> 2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-23 13:26 ` [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
@ 2020-01-23 13:48   ` Jani Nikula
  2020-01-24 11:59     ` Anshuman Gupta
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2020-01-23 13:48 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx

On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> It should not be assumed that a disabled display pipe will be
> always last the pipe.
> for_each_pipe() should iterate over I915_MAX_PIPES and check
> for the disabled pipe and skip that pipe so that it should not
> initialize the intel crtc for any disabled pipes.
>
> Few compilation error needed to handle accordingly due to
> change in for_each_pipe() macro.

Really? Please paste.

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
>  drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 028aab728514..47813a50add4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -312,10 +312,11 @@ enum phy_fia {
>  };
>  
>  #define for_each_pipe(__dev_priv, __p) \
> -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
> +	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \

Originally I was envisioning using for_each_set_bit() from bitops.h for
this. It's probably more efficient, however I'm not sure if efficiency
matters much here. The ugly part is that for_each_set_bit() requires an
explicit cast to unsigned long *.

Perhaps this is just as well, it's not wrong, and can always be updated
later.

> +		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
>  
>  #define for_each_pipe_masked(__dev_priv, __p, __mask) \
> -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
> +	for_each_pipe(__dev_priv, __p) \
>  		for_each_if((__mask) & BIT(__p))
>  
>  #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 94cb25ac504d..22ecd5bc407e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>  	if (pch_iir & SDE_POISON)
>  		DRM_ERROR("PCH poison interrupt\n");
>  
> -	if (pch_iir & SDE_FDI_MASK)
> +	if (pch_iir & SDE_FDI_MASK) {
>  		for_each_pipe(dev_priv, pipe)
>  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>  					 pipe_name(pipe),
>  					 I915_READ(FDI_RX_IIR(pipe)));
> +	}

Are the brace changes really needed? This is what the for_each_if hack
is supposed to tackle.

>  
>  	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
>  		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
> @@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>  	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
>  		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
>  
> -	if (pch_iir & SDE_FDI_MASK_CPT)
> +	if (pch_iir & SDE_FDI_MASK_CPT) {
>  		for_each_pipe(dev_priv, pipe)
>  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>  					 pipe_name(pipe),
>  					 I915_READ(FDI_RX_IIR(pipe)));
> +	}
>  
>  	if (pch_iir & SDE_ERROR_CPT)
>  		cpt_serr_int_handler(dev_priv);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-23 13:26 ` [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption Anshuman Gupta
  2020-01-23 13:40   ` Ville Syrjälä
@ 2020-01-23 13:49   ` Jani Nikula
  1 sibling, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2020-01-23 13:49 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx

On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> we can't have (pipe == crtc->index) assumption in
> driver in order to support 3 non-contiguous
> display pipe system.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 878d331b9e8c..afd8d43160c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
>  	if (new_crtc_state->hw.active)
>  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
>  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  	else
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  
>  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
>  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> @@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
>  
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
>  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  	}
>  }
>  
> @@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  
>  	intel_color_init(crtc);
>  
> -	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
> -
>  	return 0;
>  
>  fail:

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride
  2020-01-23 13:26 ` [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride Anshuman Gupta
@ 2020-01-23 13:50   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:50 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 23, 2020 at 06:56:57PM +0530, Anshuman Gupta wrote:
> intel_plane_fb_max_stride should return the max stride of
> primary plane for first available pipe in intel device info
> pipe_mask.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b250b31f6000..b9209b7e71d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2703,12 +2703,15 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_crtc *crtc;
>  	struct intel_plane *plane;
> +	enum pipe pipe;
>  
>  	/*
>  	 * We assume the primary plane for pipe A has
> -	 * the highest stride limits of them all.
> +	 * the highest stride limits of them all,
> +	 * if in case pipe A is disabled, use the first pipe from pipe_mask.
>  	 */
> -	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> +	pipe = ffs(INTEL_INFO(dev_priv)->pipe_mask) - 1;
> +	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);

I'd probably just add a small helper for this. Eg.:

intel_get_first_crtc()
{
	return to_intel_crtc(drm_crtc_from_index(0))
}

>  	if (!crtc)
>  		return 0;
>  
> -- 
> 2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
  2020-01-23 13:26 ` [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
@ 2020-01-23 13:52   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:52 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 23, 2020 at 06:56:58PM +0530, Anshuman Gupta wrote:
> Add a WARN_ON for a disabled pipe in pipe_mask at
> intel_get_crtc_for_pipe() function.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 33ba93863488..ca8d1e17814e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1426,6 +1426,8 @@ vlv_pipe_to_channel(enum pipe pipe)
>  static inline struct intel_crtc *
>  intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> +	/* pipe_to_crtc_mapping may have hole on any of 3 display pipe system */
> +	WARN_ON(!(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)));

Sure. Might help catch accidents where the caller does a NULL check and
thus doesn't oops.

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

>  	return dev_priv->pipe_to_crtc_mapping[pipe];
>  }
>  
> -- 
> 2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support
  2020-01-23 13:26 ` [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support Anshuman Gupta
@ 2020-01-23 13:53   ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-23 13:53 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 23, 2020 at 06:56:59PM +0530, Anshuman Gupta wrote:
> Allow 3-display pipes SKU system with any combination
> in INTEL_INFO pipe mask.
> B.Spec:50075
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index fcdacd6d4aa5..459047c98e82 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -997,10 +997,11 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  
>  		/*
>  		 * At least one pipe should be enabled and if there are
> -		 * disabled pipes, they should be the last ones, with no holes
> -		 * in the mask.
> +		 * disabled pipes, up to Display Gen<=12, they should be the
> +		 * last ones, with no holses in the mask.
>  		 */
> -		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
> +		if (enabled_mask == 0 || (!is_power_of_2(enabled_mask + 1) &&
> +		    (INTEL_GEN(dev_priv) <= 11 || IS_TIGERLAKE(dev_priv))))

intel_pipe_mask_is_valid() or something to avoid this horror show
in the if condition.

>  			drm_err(&dev_priv->drm,
>  				"invalid pipe fuse configuration: enabled_mask=0x%x\n",
>  				enabled_mask);
> -- 
> 2.24.0

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for 3 display pipes combination system support
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (5 preceding siblings ...)
  2020-01-23 13:26 ` [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support Anshuman Gupta
@ 2020-01-23 21:38 ` Patchwork
  2020-01-23 22:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-01-25 13:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2020-01-23 21:38 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: 3 display pipes combination system support
URL   : https://patchwork.freedesktop.org/series/72468/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ff6c602c84d0 drm/i915: Iterate over pipe and skip the disabled one
dbdf2193b56c drm/i915: Remove (pipe == crtc->index) asummption
d0cb3e6e2556 drm/i915: Fix wrongly populated plane possible_crtcs bit mask
3d9a940bfc7e drm/i915: Get right max plane stride
7c0eef4e0e3c drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
ca9423f88fb2 drm/i915: Enable 3 display pipes support
-:31: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#31: FILE: drivers/gpu/drm/i915/intel_device_info.c:1004:
+		if (enabled_mask == 0 || (!is_power_of_2(enabled_mask + 1) &&
+		    (INTEL_GEN(dev_priv) <= 11 || IS_TIGERLAKE(dev_priv))))

total: 0 errors, 0 warnings, 1 checks, 14 lines checked

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for 3 display pipes combination system support
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (6 preceding siblings ...)
  2020-01-23 21:38 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for 3 display pipes combination system support Patchwork
@ 2020-01-23 22:19 ` Patchwork
  2020-01-25 13:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2020-01-23 22:19 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: 3 display pipes combination system support
URL   : https://patchwork.freedesktop.org/series/72468/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7804 -> Patchwork_16235
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@fds:
    - fi-byt-n2820:       [PASS][1] -> [TIMEOUT][2] ([fdo#112271])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-byt-n2820/igt@gem_exec_parallel@fds.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-byt-n2820/igt@gem_exec_parallel@fds.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-cfl-guc:         [PASS][3] -> [INCOMPLETE][4] ([i915#505] / [i915#671])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-cfl-guc/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-n2820:       [PASS][5] -> [DMESG-FAIL][6] ([i915#722])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-cml-u2:          [PASS][7] -> [FAIL][8] ([i915#217])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html

  * igt@prime_vgem@basic-gtt:
    - fi-tgl-y:           [PASS][9] -> [DMESG-WARN][10] ([CI#94] / [i915#402]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-tgl-y/igt@prime_vgem@basic-gtt.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-tgl-y/igt@prime_vgem@basic-gtt.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-j1900:       [TIMEOUT][11] ([fdo#112271] / [i915#816]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-byt-j1900/igt@gem_close_race@basic-threads.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-byt-j1900/igt@gem_close_race@basic-threads.html

  * igt@gem_exec_parallel@contexts:
    - fi-byt-n2820:       [TIMEOUT][13] ([fdo#112271]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-byt-n2820/igt@gem_exec_parallel@contexts.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-byt-n2820/igt@gem_exec_parallel@contexts.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][15] ([i915#725]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  * igt@kms_addfb_basic@bad-pitch-0:
    - fi-tgl-y:           [DMESG-WARN][17] ([CI#94] / [i915#402]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-tgl-y/igt@kms_addfb_basic@bad-pitch-0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-tgl-y/igt@kms_addfb_basic@bad-pitch-0.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][19] ([fdo#111096] / [i915#323]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][21] ([i915#725]) -> [DMESG-FAIL][22] ([i915#553] / [i915#725])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/fi-hsw-4770/igt@i915_selftest@live_blt.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#505]: https://gitlab.freedesktop.org/drm/intel/issues/505
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#722]: https://gitlab.freedesktop.org/drm/intel/issues/722
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816


Participating hosts (49 -> 43)
------------------------------

  Additional (3): fi-hsw-peppy fi-kbl-7560u fi-snb-2600 
  Missing    (9): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bwr-2160 fi-ilk-650 fi-ctg-p8600 fi-bdw-samus fi-byt-clapper fi-skl-6700k2 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7804 -> Patchwork_16235

  CI-20190529: 20190529
  CI_DRM_7804: 74ed9d57007ab848a57ec6d785de4187b70acd9b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5382: 8dbe5ce61baa2d563d4dd7c56a018bb1e1077467 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16235: ca9423f88fb22b387a66c32a457a53f172388a1b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ca9423f88fb2 drm/i915: Enable 3 display pipes support
7c0eef4e0e3c drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
3d9a940bfc7e drm/i915: Get right max plane stride
d0cb3e6e2556 drm/i915: Fix wrongly populated plane possible_crtcs bit mask
dbdf2193b56c drm/i915: Remove (pipe == crtc->index) asummption
ff6c602c84d0 drm/i915: Iterate over pipe and skip the disabled one

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-23 13:48   ` Jani Nikula
@ 2020-01-24 11:59     ` Anshuman Gupta
  2020-01-24 12:15       ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-24 11:59 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On 2020-01-23 at 15:48:05 +0200, Jani Nikula wrote:
> On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > It should not be assumed that a disabled display pipe will be
> > always last the pipe.
> > for_each_pipe() should iterate over I915_MAX_PIPES and check
> > for the disabled pipe and skip that pipe so that it should not
> > initialize the intel crtc for any disabled pipes.
> >
> > Few compilation error needed to handle accordingly due to
> > change in for_each_pipe() macro.
> 
> Really? Please paste.
It is dangling-else warning at couple of places.
drivers/gpu/drm/i915/i915_irq.c:1861:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
 1861 |  if (pch_iir & SDE_FDI_MASK)
drivers/gpu/drm/i915/i915_irq.c:1944:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
1944 |  if (pch_iir & SDE_FDI_MASK_CPT)
> 
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
> >  drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index 028aab728514..47813a50add4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -312,10 +312,11 @@ enum phy_fia {
> >  };
> >  
> >  #define for_each_pipe(__dev_priv, __p) \
> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
> > +	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \
> 
> Originally I was envisioning using for_each_set_bit() from bitops.h for
> this. It's probably more efficient, however I'm not sure if efficiency
> matters much here. The ugly part is that for_each_set_bit() requires an
> explicit cast to unsigned long *.
> 
> Perhaps this is just as well, it's not wrong, and can always be updated
> later.
> 
> > +		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
> >  
> >  #define for_each_pipe_masked(__dev_priv, __p, __mask) \
> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
> > +	for_each_pipe(__dev_priv, __p) \
> >  		for_each_if((__mask) & BIT(__p))
> >  
> >  #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 94cb25ac504d..22ecd5bc407e 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> >  	if (pch_iir & SDE_POISON)
> >  		DRM_ERROR("PCH poison interrupt\n");
> >  
> > -	if (pch_iir & SDE_FDI_MASK)
> > +	if (pch_iir & SDE_FDI_MASK) {
> >  		for_each_pipe(dev_priv, pipe)
> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
> >  					 pipe_name(pipe),
> >  					 I915_READ(FDI_RX_IIR(pipe)));
> > +	}
> 
> Are the brace changes really needed? This is what the for_each_if hack
> is supposed to tackle.
IMHO it was dangling-else compilation, warning that requires braces.
please correct me if i am wrong.
Thanks,
Anshuman
> 
> >  
> >  	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
> >  		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
> > @@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> >  	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
> >  		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
> >  
> > -	if (pch_iir & SDE_FDI_MASK_CPT)
> > +	if (pch_iir & SDE_FDI_MASK_CPT) {
> >  		for_each_pipe(dev_priv, pipe)
> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
> >  					 pipe_name(pipe),
> >  					 I915_READ(FDI_RX_IIR(pipe)));
> > +	}
> >  
> >  	if (pch_iir & SDE_ERROR_CPT)
> >  		cpt_serr_int_handler(dev_priv);
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-24 11:59     ` Anshuman Gupta
@ 2020-01-24 12:15       ` Jani Nikula
  2020-01-24 12:19         ` Anshuman Gupta
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2020-01-24 12:15 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Fri, 24 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> On 2020-01-23 at 15:48:05 +0200, Jani Nikula wrote:
>> On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
>> > It should not be assumed that a disabled display pipe will be
>> > always last the pipe.
>> > for_each_pipe() should iterate over I915_MAX_PIPES and check
>> > for the disabled pipe and skip that pipe so that it should not
>> > initialize the intel crtc for any disabled pipes.
>> >
>> > Few compilation error needed to handle accordingly due to
>> > change in for_each_pipe() macro.
>> 
>> Really? Please paste.
> It is dangling-else warning at couple of places.
> drivers/gpu/drm/i915/i915_irq.c:1861:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
>  1861 |  if (pch_iir & SDE_FDI_MASK)
> drivers/gpu/drm/i915/i915_irq.c:1944:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
> 1944 |  if (pch_iir & SDE_FDI_MASK_CPT)

Right, I suppose this is caused by the nesting of the for loops with
if-else.

Perhaps the right course of action is to *not* reuse for_each_pipe() in
for_each_pipe_masked(). Just combine the conditions into one.

BR,
Jani.



>> 
>> >
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
>> >  drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
>> >  2 files changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> > index 028aab728514..47813a50add4 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> > @@ -312,10 +312,11 @@ enum phy_fia {
>> >  };
>> >  
>> >  #define for_each_pipe(__dev_priv, __p) \
>> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
>> > +	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \
>> 
>> Originally I was envisioning using for_each_set_bit() from bitops.h for
>> this. It's probably more efficient, however I'm not sure if efficiency
>> matters much here. The ugly part is that for_each_set_bit() requires an
>> explicit cast to unsigned long *.
>> 
>> Perhaps this is just as well, it's not wrong, and can always be updated
>> later.
>> 
>> > +		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
>> >  
>> >  #define for_each_pipe_masked(__dev_priv, __p, __mask) \
>> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
>> > +	for_each_pipe(__dev_priv, __p) \
>> >  		for_each_if((__mask) & BIT(__p))
>> >  
>> >  #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index 94cb25ac504d..22ecd5bc407e 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> >  	if (pch_iir & SDE_POISON)
>> >  		DRM_ERROR("PCH poison interrupt\n");
>> >  
>> > -	if (pch_iir & SDE_FDI_MASK)
>> > +	if (pch_iir & SDE_FDI_MASK) {
>> >  		for_each_pipe(dev_priv, pipe)
>> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>> >  					 pipe_name(pipe),
>> >  					 I915_READ(FDI_RX_IIR(pipe)));
>> > +	}
>> 
>> Are the brace changes really needed? This is what the for_each_if hack
>> is supposed to tackle.
> IMHO it was dangling-else compilation, warning that requires braces.
> please correct me if i am wrong.
> Thanks,
> Anshuman
>> 
>> >  
>> >  	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
>> >  		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
>> > @@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> >  	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
>> >  		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
>> >  
>> > -	if (pch_iir & SDE_FDI_MASK_CPT)
>> > +	if (pch_iir & SDE_FDI_MASK_CPT) {
>> >  		for_each_pipe(dev_priv, pipe)
>> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>> >  					 pipe_name(pipe),
>> >  					 I915_READ(FDI_RX_IIR(pipe)));
>> > +	}
>> >  
>> >  	if (pch_iir & SDE_ERROR_CPT)
>> >  		cpt_serr_int_handler(dev_priv);
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-24 12:15       ` Jani Nikula
@ 2020-01-24 12:19         ` Anshuman Gupta
  2020-01-24 13:34           ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-24 12:19 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On 2020-01-24 at 14:15:30 +0200, Jani Nikula wrote:
> On Fri, 24 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > On 2020-01-23 at 15:48:05 +0200, Jani Nikula wrote:
> >> On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> >> > It should not be assumed that a disabled display pipe will be
> >> > always last the pipe.
> >> > for_each_pipe() should iterate over I915_MAX_PIPES and check
> >> > for the disabled pipe and skip that pipe so that it should not
> >> > initialize the intel crtc for any disabled pipes.
> >> >
> >> > Few compilation error needed to handle accordingly due to
> >> > change in for_each_pipe() macro.
> >> 
> >> Really? Please paste.
> > It is dangling-else warning at couple of places.
> > drivers/gpu/drm/i915/i915_irq.c:1861:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
> >  1861 |  if (pch_iir & SDE_FDI_MASK)
> > drivers/gpu/drm/i915/i915_irq.c:1944:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
> > 1944 |  if (pch_iir & SDE_FDI_MASK_CPT)
> 
> Right, I suppose this is caused by the nesting of the for loops with
> if-else.
> 
> Perhaps the right course of action is to *not* reuse for_each_pipe() in
> for_each_pipe_masked(). Just combine the conditions into one.
This is not caused by for_each_pipe_masked, this is caused by for_each_pipe itself,
if (foo)
	/*for_each_pipe()*/
	for(;;)
		if (condition) {}
		else
> 
> BR,
> Jani.
> 
> 
> 
> >> 
> >> >
> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
> >> >  drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
> >> >  2 files changed, 7 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> >> > index 028aab728514..47813a50add4 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> >> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> >> > @@ -312,10 +312,11 @@ enum phy_fia {
> >> >  };
> >> >  
> >> >  #define for_each_pipe(__dev_priv, __p) \
> >> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
> >> > +	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \
> >> 
> >> Originally I was envisioning using for_each_set_bit() from bitops.h for
> >> this. It's probably more efficient, however I'm not sure if efficiency
> >> matters much here. The ugly part is that for_each_set_bit() requires an
> >> explicit cast to unsigned long *.
> >> 
> >> Perhaps this is just as well, it's not wrong, and can always be updated
> >> later.
> >> 
> >> > +		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
> >> >  
> >> >  #define for_each_pipe_masked(__dev_priv, __p, __mask) \
> >> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
> >> > +	for_each_pipe(__dev_priv, __p) \
> >> >  		for_each_if((__mask) & BIT(__p))
> >> >  
> >> >  #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> > index 94cb25ac504d..22ecd5bc407e 100644
> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> > @@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> >> >  	if (pch_iir & SDE_POISON)
> >> >  		DRM_ERROR("PCH poison interrupt\n");
> >> >  
> >> > -	if (pch_iir & SDE_FDI_MASK)
> >> > +	if (pch_iir & SDE_FDI_MASK) {
> >> >  		for_each_pipe(dev_priv, pipe)
> >> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
> >> >  					 pipe_name(pipe),
> >> >  					 I915_READ(FDI_RX_IIR(pipe)));
> >> > +	}
> >> 
> >> Are the brace changes really needed? This is what the for_each_if hack
> >> is supposed to tackle.
> > IMHO it was dangling-else compilation, warning that requires braces.
> > please correct me if i am wrong.
> > Thanks,
> > Anshuman
> >> 
> >> >  
> >> >  	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
> >> >  		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
> >> > @@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> >> >  	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
> >> >  		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
> >> >  
> >> > -	if (pch_iir & SDE_FDI_MASK_CPT)
> >> > +	if (pch_iir & SDE_FDI_MASK_CPT) {
> >> >  		for_each_pipe(dev_priv, pipe)
> >> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
> >> >  					 pipe_name(pipe),
> >> >  					 I915_READ(FDI_RX_IIR(pipe)));
> >> > +	}
> >> >  
> >> >  	if (pch_iir & SDE_ERROR_CPT)
> >> >  		cpt_serr_int_handler(dev_priv);
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one
  2020-01-24 12:19         ` Anshuman Gupta
@ 2020-01-24 13:34           ` Jani Nikula
  0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2020-01-24 13:34 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Fri, 24 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> On 2020-01-24 at 14:15:30 +0200, Jani Nikula wrote:
>> On Fri, 24 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
>> > On 2020-01-23 at 15:48:05 +0200, Jani Nikula wrote:
>> >> On Thu, 23 Jan 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
>> >> > It should not be assumed that a disabled display pipe will be
>> >> > always last the pipe.
>> >> > for_each_pipe() should iterate over I915_MAX_PIPES and check
>> >> > for the disabled pipe and skip that pipe so that it should not
>> >> > initialize the intel crtc for any disabled pipes.
>> >> >
>> >> > Few compilation error needed to handle accordingly due to
>> >> > change in for_each_pipe() macro.
>> >> 
>> >> Really? Please paste.
>> > It is dangling-else warning at couple of places.
>> > drivers/gpu/drm/i915/i915_irq.c:1861:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
>> >  1861 |  if (pch_iir & SDE_FDI_MASK)
>> > drivers/gpu/drm/i915/i915_irq.c:1944:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
>> > 1944 |  if (pch_iir & SDE_FDI_MASK_CPT)
>> 
>> Right, I suppose this is caused by the nesting of the for loops with
>> if-else.
>> 
>> Perhaps the right course of action is to *not* reuse for_each_pipe() in
>> for_each_pipe_masked(). Just combine the conditions into one.
> This is not caused by for_each_pipe_masked, this is caused by for_each_pipe itself,
> if (foo)
> 	/*for_each_pipe()*/
> 	for(;;)
> 		if (condition) {}
> 		else

Sorry for the noise, please carry on with the patch.

*blush*

BR,
Jani.



>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>> >> 
>> >> >
>> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/display/intel_display.h | 5 +++--
>> >> >  drivers/gpu/drm/i915/i915_irq.c              | 6 ++++--
>> >> >  2 files changed, 7 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> >> > index 028aab728514..47813a50add4 100644
>> >> > --- a/drivers/gpu/drm/i915/display/intel_display.h
>> >> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> >> > @@ -312,10 +312,11 @@ enum phy_fia {
>> >> >  };
>> >> >  
>> >> >  #define for_each_pipe(__dev_priv, __p) \
>> >> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++)
>> >> > +	for ((__p) = 0; (__p) < I915_MAX_PIPES; (__p)++) \
>> >> 
>> >> Originally I was envisioning using for_each_set_bit() from bitops.h for
>> >> this. It's probably more efficient, however I'm not sure if efficiency
>> >> matters much here. The ugly part is that for_each_set_bit() requires an
>> >> explicit cast to unsigned long *.
>> >> 
>> >> Perhaps this is just as well, it's not wrong, and can always be updated
>> >> later.
>> >> 
>> >> > +		for_each_if((INTEL_INFO(__dev_priv)->pipe_mask) & BIT(__p))
>> >> >  
>> >> >  #define for_each_pipe_masked(__dev_priv, __p, __mask) \
>> >> > -	for ((__p) = 0; (__p) < INTEL_NUM_PIPES(__dev_priv); (__p)++) \
>> >> > +	for_each_pipe(__dev_priv, __p) \
>> >> >  		for_each_if((__mask) & BIT(__p))
>> >> >  
>> >> >  #define for_each_cpu_transcoder_masked(__dev_priv, __t, __mask) \
>> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> >> > index 94cb25ac504d..22ecd5bc407e 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> >> > @@ -1735,11 +1735,12 @@ static void ibx_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> >> >  	if (pch_iir & SDE_POISON)
>> >> >  		DRM_ERROR("PCH poison interrupt\n");
>> >> >  
>> >> > -	if (pch_iir & SDE_FDI_MASK)
>> >> > +	if (pch_iir & SDE_FDI_MASK) {
>> >> >  		for_each_pipe(dev_priv, pipe)
>> >> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>> >> >  					 pipe_name(pipe),
>> >> >  					 I915_READ(FDI_RX_IIR(pipe)));
>> >> > +	}
>> >> 
>> >> Are the brace changes really needed? This is what the for_each_if hack
>> >> is supposed to tackle.
>> > IMHO it was dangling-else compilation, warning that requires braces.
>> > please correct me if i am wrong.
>> > Thanks,
>> > Anshuman
>> >> 
>> >> >  
>> >> >  	if (pch_iir & (SDE_TRANSB_CRC_DONE | SDE_TRANSA_CRC_DONE))
>> >> >  		DRM_DEBUG_DRIVER("PCH transcoder CRC done interrupt\n");
>> >> > @@ -1818,11 +1819,12 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> >> >  	if (pch_iir & SDE_AUDIO_CP_CHG_CPT)
>> >> >  		DRM_DEBUG_DRIVER("Audio CP change interrupt\n");
>> >> >  
>> >> > -	if (pch_iir & SDE_FDI_MASK_CPT)
>> >> > +	if (pch_iir & SDE_FDI_MASK_CPT) {
>> >> >  		for_each_pipe(dev_priv, pipe)
>> >> >  			DRM_DEBUG_DRIVER("  pipe %c FDI IIR: 0x%08x\n",
>> >> >  					 pipe_name(pipe),
>> >> >  					 I915_READ(FDI_RX_IIR(pipe)));
>> >> > +	}
>> >> >  
>> >> >  	if (pch_iir & SDE_ERROR_CPT)
>> >> >  		cpt_serr_int_handler(dev_priv);
>> >> 
>> >> -- 
>> >> Jani Nikula, Intel Open Source Graphics Center
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for 3 display pipes combination system support
  2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
                   ` (7 preceding siblings ...)
  2020-01-23 22:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-01-25 13:08 ` Patchwork
  8 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2020-01-25 13:08 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: 3 display pipes combination system support
URL   : https://patchwork.freedesktop.org/series/72468/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7804_full -> Patchwork_16235_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@busy-vcs1:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#112080]) +16 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb4/igt@gem_busy@busy-vcs1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb5/igt@gem_busy@busy-vcs1.html

  * igt@gem_ctx_persistence@vcs1-mixed:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276] / [fdo#112080]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb4/igt@gem_ctx_persistence@vcs1-mixed.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb8/igt@gem_ctx_persistence@vcs1-mixed.html

  * igt@gem_ctx_persistence@vecs0-mixed-process:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([i915#679])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-glk1/igt@gem_ctx_persistence@vecs0-mixed-process.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-glk1/igt@gem_ctx_persistence@vecs0-mixed-process.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#110841])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb7/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_eio@kms:
    - shard-snb:          [PASS][9] -> [INCOMPLETE][10] ([i915#82])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-snb1/igt@gem_eio@kms.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-snb4/igt@gem_eio@kms.html

  * igt@gem_exec_reloc@basic-gtt-active:
    - shard-skl:          [PASS][11] -> [DMESG-WARN][12] ([i915#109]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl7/igt@gem_exec_reloc@basic-gtt-active.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl7/igt@gem_exec_reloc@basic-gtt-active.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#112146]) +9 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb3/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-kbl:          [PASS][15] -> [INCOMPLETE][16] ([fdo#103665])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-kbl1/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-kbl3/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing:
    - shard-glk:          [PASS][17] -> [INCOMPLETE][18] ([i915#58] / [i915#970] / [k.org#198133])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-glk5/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-glk2/igt@gem_persistent_relocs@forked-interruptible-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-hsw:          [PASS][19] -> [INCOMPLETE][20] ([i915#61]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-hsw5/igt@gem_persistent_relocs@forked-thrashing.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-hsw6/igt@gem_persistent_relocs@forked-thrashing.html

  * igt@i915_pm_rps@waitboost:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([i915#413])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb4/igt@i915_pm_rps@waitboost.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb8/igt@i915_pm_rps@waitboost.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-kbl:          [PASS][23] -> [DMESG-WARN][24] ([i915#180]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-kbl7/igt@i915_suspend@fence-restore-untiled.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-kbl4/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_color@pipe-b-ctm-green-to-red:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#129])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl7/igt@kms_color@pipe-b-ctm-green-to-red.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl7/igt@kms_color@pipe-b-ctm-green-to-red.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][27] -> [FAIL][28] ([i915#79])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([i915#79])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt:
    - shard-tglb:         [PASS][31] -> [FAIL][32] ([i915#49]) +4 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-apl:          [PASS][33] -> [DMESG-WARN][34] ([i915#180])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-apl6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-apl2/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][35] -> [FAIL][36] ([fdo#108145])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#109642] / [fdo#111068])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb1/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [PASS][39] -> [SKIP][40] ([fdo#109441])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb7/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][41] -> [FAIL][42] ([i915#31])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-apl4/igt@kms_setmode@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-apl2/igt@kms_setmode@basic.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [PASS][43] -> [SKIP][44] ([fdo#109276]) +26 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb1/igt@prime_busy@hang-bsd2.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb6/igt@prime_busy@hang-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][45] ([fdo#109276] / [fdo#112080]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb5/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb2/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_ctx_persistence@rcs0-mixed-process:
    - shard-glk:          [FAIL][47] ([i915#679]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-glk3/igt@gem_ctx_persistence@rcs0-mixed-process.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-glk1/igt@gem_ctx_persistence@rcs0-mixed-process.html

  * igt@gem_ctx_persistence@vecs0-mixed-process:
    - shard-tglb:         [FAIL][49] ([i915#679]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-tglb5/igt@gem_ctx_persistence@vecs0-mixed-process.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-tglb3/igt@gem_ctx_persistence@vecs0-mixed-process.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [SKIP][51] ([fdo#112146]) -> [PASS][52] +6 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb8/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_exec_schedule@promotion-bsd1:
    - shard-iclb:         [SKIP][53] ([fdo#109276]) -> [PASS][54] +13 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb3/igt@gem_exec_schedule@promotion-bsd1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb2/igt@gem_exec_schedule@promotion-bsd1.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-hsw:          [INCOMPLETE][55] ([i915#61]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-hsw1/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-hsw2/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-glk:          [INCOMPLETE][57] ([i915#58] / [k.org#198133]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-glk7/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-glk6/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-kbl:          [INCOMPLETE][59] ([fdo#103665]) -> [PASS][60] +3 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-kbl6/igt@gem_persistent_relocs@forked-thrashing.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-kbl7/igt@gem_persistent_relocs@forked-thrashing.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-apl:          [FAIL][61] ([i915#644]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-apl7/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-apl8/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_selftest@mock_requests:
    - shard-tglb:         [INCOMPLETE][63] ([i915#472]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-tglb4/igt@i915_selftest@mock_requests.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-tglb8/igt@i915_selftest@mock_requests.html
    - shard-apl:          [INCOMPLETE][65] ([fdo#103927]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-apl7/igt@i915_selftest@mock_requests.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-apl1/igt@i915_selftest@mock_requests.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][67] ([i915#180]) -> [PASS][68] +3 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-apl7/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_color@pipe-b-ctm-0-75:
    - shard-skl:          [DMESG-WARN][69] ([i915#109]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl2/igt@kms_color@pipe-b-ctm-0-75.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl1/igt@kms_color@pipe-b-ctm-0-75.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [INCOMPLETE][71] ([i915#221]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-skl6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-skl10/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt:
    - shard-tglb:         [FAIL][73] ([i915#49]) -> [PASS][74] +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [DMESG-WARN][75] ([i915#180]) -> [PASS][76] +2 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][77] ([fdo#109642] / [fdo#111068]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb3/igt@kms_psr2_su@page_flip.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][79] ([fdo#109441]) -> [PASS][80] +2 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb5/igt@kms_psr@psr2_cursor_render.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@perf_pmu@init-busy-vcs1:
    - shard-iclb:         [SKIP][81] ([fdo#112080]) -> [PASS][82] +9 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-iclb8/igt@perf_pmu@init-busy-vcs1.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-iclb1/igt@perf_pmu@init-busy-vcs1.html

  
#### Warnings ####

  * igt@gem_tiled_blits@interruptible:
    - shard-hsw:          [FAIL][83] ([i915#818]) -> [FAIL][84] ([i915#694])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-hsw7/igt@gem_tiled_blits@interruptible.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-hsw2/igt@gem_tiled_blits@interruptible.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - shard-snb:          [SKIP][85] ([fdo#109271]) -> [INCOMPLETE][86] ([i915#82])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-snb5/igt@i915_pm_rpm@basic-pci-d3-state.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-snb2/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_selftest@live_blt:
    - shard-hsw:          [DMESG-FAIL][87] ([i915#770]) -> [DMESG-FAIL][88] ([i915#725])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-hsw8/igt@i915_selftest@live_blt.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-hsw8/igt@i915_selftest@live_blt.html

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-hsw:          [SKIP][89] ([fdo#109271] / [i915#439]) -> [SKIP][90] ([fdo#109271])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7804/shard-hsw2/igt@kms_atomic_transition@3x-modeset-transitions.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16235/shard-hsw1/igt@kms_atomic_transition@3x-modeset-transitions.html

  
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [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#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#129]: https://gitlab.freedesktop.org/drm/intel/issues/129
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#221]: https://gitlab.freedesktop.org/drm/intel/issues/221
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#439]: https://gitlab.freedesktop.org/drm/intel/issues/439
  [i915#472]: https://gitlab.freedesktop.org/drm/intel/issues/472
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#58]: https://gitlab.freedesktop.org/drm/intel/issues/58
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#770]: https://gitlab.freedesktop.org/drm/intel/issues/770
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#970]: https://gitlab.freedesktop.org/drm/intel/issues/970
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7804 -> Patchwork_16235

  CI-20190529: 20190529
  CI_DRM_7804: 74ed9d57007ab848a57ec6d785de4187b70acd9b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5382: 8dbe5ce61baa2d563d4dd7c56a018bb1e1077467 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16235: ca9423f88fb22b387a66c32a457a53f172388a1b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-23 13:40   ` Ville Syrjälä
@ 2020-01-30 12:02     ` Anshuman Gupta
  2020-01-30 13:35       ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Anshuman Gupta @ 2020-01-30 12:02 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: jani.nikula, intel-gfx

On 2020-01-23 at 15:40:57 +0200, Ville Syrjälä wrote:
> On Thu, Jan 23, 2020 at 06:56:55PM +0530, Anshuman Gupta wrote:
> > we can't have (pipe == crtc->index) assumption in
> > driver in order to support 3 non-contiguous
> > display pipe system.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 878d331b9e8c..afd8d43160c6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
> >  	if (new_crtc_state->hw.active)
> >  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
> >  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > +				pipe_name(crtc->pipe), pll->active_mask);
> >  	else
> >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> >  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > +				pipe_name(crtc->pipe), pll->active_mask);
> >  
> >  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
> >  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> > @@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
> >  
> >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> >  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> > -				pipe_name(drm_crtc_index(&crtc->base)));
> > +				pipe_name(crtc->pipe));
> >  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
> >  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> > -				pipe_name(drm_crtc_index(&crtc->base)));
> > +				pipe_name(crtc->pipe));
> >  	}
> >  }
> >  
> > @@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  
> >  	intel_color_init(crtc);
> >  
> > -	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
> > -
> 
> The first and second hunks don't really have anything to do with
> each other. Also the WARN_ON() should not be removed until all the
> assumptions are fixed.
True there can be other assumptions as well, there are few, i have come to know
drm_handle_vblank(&dev_priv->drm, pipe) in gen8_de_irq_handler()
drm_wait_one_vblank(&dev_priv->drm, pipe) in intel_wait_for_vblank(),
i will fix these assumptions is next update, are there any other similar kind of
assumption on which u can throw some light to look for?
I am not sure how does above WARN_ON helps to know all such kind of 
assumptions, but it make sense to have it with FIXME.
Thanks,
Anshuman Gupta.
> 
> >  	return 0;
> >  
> >  fail:
> > -- 
> > 2.24.0
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-30 12:02     ` Anshuman Gupta
@ 2020-01-30 13:35       ` Ville Syrjälä
  2020-01-30 15:27         ` Ville Syrjälä
  0 siblings, 1 reply; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-30 13:35 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 30, 2020 at 05:32:01PM +0530, Anshuman Gupta wrote:
> On 2020-01-23 at 15:40:57 +0200, Ville Syrjälä wrote:
> > On Thu, Jan 23, 2020 at 06:56:55PM +0530, Anshuman Gupta wrote:
> > > we can't have (pipe == crtc->index) assumption in
> > > driver in order to support 3 non-contiguous
> > > display pipe system.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 878d331b9e8c..afd8d43160c6 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
> > >  	if (new_crtc_state->hw.active)
> > >  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
> > >  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> > > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > > +				pipe_name(crtc->pipe), pll->active_mask);
> > >  	else
> > >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> > >  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> > > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > > +				pipe_name(crtc->pipe), pll->active_mask);
> > >  
> > >  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
> > >  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> > > @@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
> > >  
> > >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> > >  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> > > -				pipe_name(drm_crtc_index(&crtc->base)));
> > > +				pipe_name(crtc->pipe));
> > >  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
> > >  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> > > -				pipe_name(drm_crtc_index(&crtc->base)));
> > > +				pipe_name(crtc->pipe));
> > >  	}
> > >  }
> > >  
> > > @@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >  
> > >  	intel_color_init(crtc);
> > >  
> > > -	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
> > > -
> > 
> > The first and second hunks don't really have anything to do with
> > each other. Also the WARN_ON() should not be removed until all the
> > assumptions are fixed.
> True there can be other assumptions as well, there are few, i have come to know
> drm_handle_vblank(&dev_priv->drm, pipe) in gen8_de_irq_handler()

In fact it's in all irq handlers.

> drm_wait_one_vblank(&dev_priv->drm, pipe) in intel_wait_for_vblank(),

Good catch. Totally forgot about these.

> i will fix these assumptions is next update, are there any other similar kind of
> assumption on which u can throw some light to look for?
> I am not sure how does above WARN_ON helps to know all such kind of 
> assumptions, but it make sense to have it with FIXME.

It doesn't help finding them, what it does is make people realize
that they're running a driver which is known to be broken.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption
  2020-01-30 13:35       ` Ville Syrjälä
@ 2020-01-30 15:27         ` Ville Syrjälä
  0 siblings, 0 replies; 24+ messages in thread
From: Ville Syrjälä @ 2020-01-30 15:27 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: jani.nikula, intel-gfx

On Thu, Jan 30, 2020 at 03:35:20PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 30, 2020 at 05:32:01PM +0530, Anshuman Gupta wrote:
> > On 2020-01-23 at 15:40:57 +0200, Ville Syrjälä wrote:
> > > On Thu, Jan 23, 2020 at 06:56:55PM +0530, Anshuman Gupta wrote:
> > > > we can't have (pipe == crtc->index) assumption in
> > > > driver in order to support 3 non-contiguous
> > > > display pipe system.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 10 ++++------
> > > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 878d331b9e8c..afd8d43160c6 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -14070,11 +14070,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
> > > >  	if (new_crtc_state->hw.active)
> > > >  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
> > > >  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> > > > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > > > +				pipe_name(crtc->pipe), pll->active_mask);
> > > >  	else
> > > >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> > > >  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> > > > -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> > > > +				pipe_name(crtc->pipe), pll->active_mask);
> > > >  
> > > >  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
> > > >  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> > > > @@ -14103,10 +14103,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
> > > >  
> > > >  		I915_STATE_WARN(pll->active_mask & crtc_mask,
> > > >  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> > > > -				pipe_name(drm_crtc_index(&crtc->base)));
> > > > +				pipe_name(crtc->pipe));
> > > >  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
> > > >  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> > > > -				pipe_name(drm_crtc_index(&crtc->base)));
> > > > +				pipe_name(crtc->pipe));
> > > >  	}
> > > >  }
> > > >  
> > > > @@ -16485,8 +16485,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > >  
> > > >  	intel_color_init(crtc);
> > > >  
> > > > -	WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe);
> > > > -
> > > 
> > > The first and second hunks don't really have anything to do with
> > > each other. Also the WARN_ON() should not be removed until all the
> > > assumptions are fixed.
> > True there can be other assumptions as well, there are few, i have come to know
> > drm_handle_vblank(&dev_priv->drm, pipe) in gen8_de_irq_handler()
> 
> In fact it's in all irq handlers.
> 
> > drm_wait_one_vblank(&dev_priv->drm, pipe) in intel_wait_for_vblank(),
> 
> Good catch. Totally forgot about these.
> 
> > i will fix these assumptions is next update, are there any other similar kind of
> > assumption on which u can throw some light to look for?
> > I am not sure how does above WARN_ON helps to know all such kind of 
> > assumptions, but it make sense to have it with FIXME.
> 
> It doesn't help finding them, what it does is make people realize
> that they're running a driver which is known to be broken.

Just remembered another borked thing: trans_offsets[]. Some places use
that to check if the transcoder is present, and we don't take fusing
into account when filling that. Though looks like
intel_display_capture_error_state() is the only place where can
actually do the wrong thing (assuming EDP/DSI transcoders are never
fused off).

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-01-30 15:27 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 13:26 [Intel-gfx] [RFC 0/6] 3 display pipes combination system support Anshuman Gupta
2020-01-23 13:26 ` [Intel-gfx] [RFC 1/6] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
2020-01-23 13:48   ` Jani Nikula
2020-01-24 11:59     ` Anshuman Gupta
2020-01-24 12:15       ` Jani Nikula
2020-01-24 12:19         ` Anshuman Gupta
2020-01-24 13:34           ` Jani Nikula
2020-01-23 13:26 ` [Intel-gfx] [RFC 2/6] drm/i915: Remove (pipe == crtc->index) asummption Anshuman Gupta
2020-01-23 13:40   ` Ville Syrjälä
2020-01-30 12:02     ` Anshuman Gupta
2020-01-30 13:35       ` Ville Syrjälä
2020-01-30 15:27         ` Ville Syrjälä
2020-01-23 13:49   ` Jani Nikula
2020-01-23 13:26 ` [Intel-gfx] [RFC 3/6] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
2020-01-23 13:47   ` Ville Syrjälä
2020-01-23 13:26 ` [Intel-gfx] [RFC 4/6] drm/i915: Get right max plane stride Anshuman Gupta
2020-01-23 13:50   ` Ville Syrjälä
2020-01-23 13:26 ` [Intel-gfx] [RFC 5/6] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
2020-01-23 13:52   ` Ville Syrjälä
2020-01-23 13:26 ` [Intel-gfx] [RFC 6/6] drm/i915: Enable 3 display pipes support Anshuman Gupta
2020-01-23 13:53   ` Ville Syrjälä
2020-01-23 21:38 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for 3 display pipes combination system support Patchwork
2020-01-23 22:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-01-25 13:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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.