All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support
@ 2020-02-04 11:29 Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 1/7] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

This is update to last RFC series after fixing the review comment
provided by Ville, and fixed couple of broken vblank API, which
were passing the intel_pipe to drm core.

Anshuman Gupta (7):
  drm/i915: Iterate over pipe and skip the disabled one
  drm/i915: Remove (pipe == crtc->index) assumption
  drm/i915: Fix broken transcoder err state
  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  | 29 ++++++++++----
 drivers/gpu/drm/i915/display/intel_display.h  |  5 ++-
 .../drm/i915/display/intel_display_types.h    | 12 +++++-
 drivers/gpu/drm/i915/display/intel_sprite.c   |  2 -
 drivers/gpu/drm/i915/i915_irq.c               | 14 +++++--
 drivers/gpu/drm/i915/intel_device_info.c      | 38 ++++++++++++++-----
 6 files changed, 74 insertions(+), 26 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] 21+ messages in thread

* [Intel-gfx] [PATCH 1/7] drm/i915: Iterate over pipe and skip the disabled one
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

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.

Below compilation error need to be handle due to change in
for_each_pipe() macro.
"suggest explicit braces to avoid ambiguous ‘else’ [-Werror=dangling-else]"

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] 21+ messages in thread

* [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 1/7] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 11:52   ` Jani Nikula
  2020-02-04 14:36   ` Ville Syrjälä
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state Anshuman Gupta
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cc : Jani Nikula

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

FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
till we won't fix all such assumption.

changes since RFC:
- Added again removed (pipe == crtc->index) WARN_ON.
- Pass drm_crtc_index instead of intel pipe in order to
  call drm_handle_vblank() from gen8_de_irq_handler(),
  other legacy irq handlers also calls drm_handle_vblank()
  with intel pipe but those doesn't require this change.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 878d331b9e8c..5709e672151a 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));
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 33ba93863488..80a6460da852 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1618,7 +1618,9 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
 static inline void
 intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
-	drm_wait_one_vblank(&dev_priv->drm, pipe);
+	const struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+
+	drm_wait_one_vblank(&dev_priv->drm, drm_crtc_index(&crtc->base));
 }
 static inline void
 intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 22ecd5bc407e..9f8b2566166a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2311,6 +2311,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 
 	for_each_pipe(dev_priv, pipe) {
 		u32 fault_errors;
+		struct intel_crtc *crtc =
+			intel_get_crtc_for_pipe(dev_priv, pipe);
 
 		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
 			continue;
@@ -2324,8 +2326,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		ret = IRQ_HANDLED;
 		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
 
-		if (iir & GEN8_PIPE_VBLANK)
-			drm_handle_vblank(&dev_priv->drm, pipe);
+		if (iir & GEN8_PIPE_VBLANK) {
+			drm_handle_vblank(&dev_priv->drm,
+					  drm_crtc_index(&crtc->base));
+		}
 
 		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
 			hsw_pipe_crc_irq_handler(dev_priv, 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] 21+ messages in thread

* [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 1/7] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 14:28   ` Ville Syrjälä
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

Skip the trascoder whose pipe is disabled while
initializing trascoder error state in 3 display
pipe syatem.

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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5709e672151a..cf36c3d0f8fc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -18607,8 +18607,10 @@ intel_display_capture_error_state(struct drm_i915_private *dev_priv)
 
 	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
 		enum transcoder cpu_transcoder = transcoders[i];
+		enum pipe pipe = (enum pipe)cpu_transcoder;
 
-		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder])
+		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder] &&
+		    !(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)))
 			continue;
 
 		error->transcoder[i].available = true;
-- 
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] 21+ messages in thread

* [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
                   ` (2 preceding siblings ...)
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 14:30   ` Ville Syrjälä
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride Anshuman Gupta
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

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.

changes since RFC:
- Simplify the possible_crtcs initialization. [Ville]

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 | 12 ++++++++++++
 drivers/gpu/drm/i915/display/intel_sprite.c  |  2 --
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cf36c3d0f8fc..7c51eb3faeb3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16407,6 +16407,17 @@ static void intel_crtc_free(struct intel_crtc *crtc)
 	kfree(crtc);
 }
 
+static void intel_plane_possible_crtc_init(struct drm_i915_private *dev_priv)
+{
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
+
+	for_each_intel_plane(&dev_priv->drm, plane) {
+		crtc = intel_get_crtc_for_pipe(dev_priv, plane->pipe);
+		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;
@@ -17546,6 +17557,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
 		}
 	}
 
+	intel_plane_possible_crtc_init(i915);
 	intel_shared_dpll_init(dev);
 	intel_update_fdi_pll_freq(i915);
 
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index fca77ec1e0dd..4a5b192678bf 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -3023,8 +3023,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
 	else
 		plane_type = DRM_PLANE_TYPE_OVERLAY;
 
-	possible_crtcs = BIT(pipe);
-
 	ret = drm_universal_plane_init(&dev_priv->drm, &plane->base,
 				       possible_crtcs, plane_funcs,
 				       formats, num_formats, modifiers,
-- 
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] 21+ messages in thread

* [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
                   ` (3 preceding siblings ...)
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 14:30   ` Ville Syrjälä
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 6/7] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

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

changes since RFC:
- Introduced a helper to get first intel_crtc intel_get_first_crtc. [Ville]

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       | 5 +++--
 drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7c51eb3faeb3..0dcf400f6954 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2706,9 +2706,10 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
 
 	/*
 	 * 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);
+	crtc = intel_get_first_crtc(dev_priv);
 	if (!crtc)
 		return 0;
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 80a6460da852..1f295c89061a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1423,6 +1423,12 @@ vlv_pipe_to_channel(enum pipe pipe)
 	}
 }
 
+static inline struct intel_crtc *
+intel_get_first_crtc(struct drm_i915_private *dev_priv)
+{
+	return to_intel_crtc(drm_crtc_from_index(&dev_priv->drm, 0));
+}
+
 static inline struct intel_crtc *
 intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe 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] 21+ messages in thread

* [Intel-gfx] [PATCH 6/7] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe()
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
                   ` (4 preceding siblings ...)
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support Anshuman Gupta
  2020-02-05  1:36 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for 3 display pipes combination system support (rev2) Patchwork
  7 siblings, 0 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

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>
Reviewed-by: 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 1f295c89061a..ecbdf4d2ab0a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1432,6 +1432,8 @@ intel_get_first_crtc(struct drm_i915_private *dev_priv)
 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] 21+ messages in thread

* [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
                   ` (5 preceding siblings ...)
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 6/7] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
@ 2020-02-04 11:29 ` Anshuman Gupta
  2020-02-04 14:35   ` Ville Syrjälä
  2020-02-05  1:36 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for 3 display pipes combination system support (rev2) Patchwork
  7 siblings, 1 reply; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-04 11:29 UTC (permalink / raw)
  To: intel-gfx

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

changes since RFC:
- using intel_pipe_mask_is_valid() function to check integrity of
  pipe_mask. [Ville]

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 | 38 +++++++++++++++++-------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index fcdacd6d4aa5..caf93a68a056 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -896,6 +896,30 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
 }
 
+static bool
+intel_pipe_mask_is_valid(struct drm_i915_private *dev_priv, u8 pipe_mask)
+{
+	/*
+	 * At least one pipe should be enabled.
+	 */
+	if (pipe_mask == 0)
+		return false;
+	/*
+	 * if there are disabled pipes they should be the last ones,
+	 * with no holses in the mask for Dispaly Gen<=12.
+	 */
+	if (!is_power_of_2(pipe_mask + 1)) {
+		if (INTEL_GEN(dev_priv) <= 11)
+			return false;
+		else if (IS_TIGERLAKE(dev_priv))
+			return false;
+		else if (IS_GEN(dev_priv, 12))
+			return true;
+	}
+
+	return true;
+}
+
 /**
  * intel_device_info_runtime_init - initialize runtime info
  * @dev_priv: the i915 device
@@ -995,17 +1019,11 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		    (dfsm & TGL_DFSM_PIPE_D_DISABLE))
 			enabled_mask &= ~BIT(PIPE_D);
 
-		/*
-		 * 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.
-		 */
-		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
-			drm_err(&dev_priv->drm,
-				"invalid pipe fuse configuration: enabled_mask=0x%x\n",
-				enabled_mask);
-		else
+		if (intel_pipe_mask_is_valid(dev_priv, enabled_mask))
 			info->pipe_mask = enabled_mask;
+		else
+			drm_err(&dev_priv->drm, "invalid pipe fuse configuration: enabled_mask=0x%x\n",
+				enabled_mask);
 
 		if (dfsm & SKL_DFSM_DISPLAY_HDCP_DISABLE)
 			info->display.has_hdcp = 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
@ 2020-02-04 11:52   ` Jani Nikula
  2020-02-04 14:36   ` Ville Syrjälä
  1 sibling, 0 replies; 21+ messages in thread
From: Jani Nikula @ 2020-02-04 11:52 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx

On Tue, 04 Feb 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.
>
> FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
> till we won't fix all such assumption.
>
> changes since RFC:
> - Added again removed (pipe == crtc->index) WARN_ON.
> - Pass drm_crtc_index instead of intel pipe in order to
>   call drm_handle_vblank() from gen8_de_irq_handler(),
>   other legacy irq handlers also calls drm_handle_vblank()
>   with intel pipe but those doesn't require this change.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c       | 8 ++++----
>  drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++-
>  drivers/gpu/drm/i915/i915_irq.c                    | 8 ++++++--
>  3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 878d331b9e8c..5709e672151a 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));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 33ba93863488..80a6460da852 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1618,7 +1618,9 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
>  static inline void
>  intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> -	drm_wait_one_vblank(&dev_priv->drm, pipe);
> +	const struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +
> +	drm_wait_one_vblank(&dev_priv->drm, drm_crtc_index(&crtc->base));

Please use drm_crtc_wait_one_vblank() instead.

>  }
>  static inline void
>  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 22ecd5bc407e..9f8b2566166a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2311,6 +2311,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		u32 fault_errors;
> +		struct intel_crtc *crtc =
> +			intel_get_crtc_for_pipe(dev_priv, pipe);
>  
>  		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
>  			continue;
> @@ -2324,8 +2326,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  		ret = IRQ_HANDLED;
>  		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
>  
> -		if (iir & GEN8_PIPE_VBLANK)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +		if (iir & GEN8_PIPE_VBLANK) {
> +			drm_handle_vblank(&dev_priv->drm,
> +					  drm_crtc_index(&crtc->base));
> +		}

drm_crtc_handle_vblank().

>  
>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>  			hsw_pipe_crc_irq_handler(dev_priv, pipe);

-- 
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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state Anshuman Gupta
@ 2020-02-04 14:28   ` Ville Syrjälä
  2020-02-04 16:37     ` Jani Nikula
  0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 14:28 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Tue, Feb 04, 2020 at 04:59:23PM +0530, Anshuman Gupta wrote:
> Skip the trascoder whose pipe is disabled while
> initializing trascoder error state in 3 display
> pipe syatem.
> 
> 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5709e672151a..cf36c3d0f8fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -18607,8 +18607,10 @@ intel_display_capture_error_state(struct drm_i915_private *dev_priv)
>  
>  	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
>  		enum transcoder cpu_transcoder = transcoders[i];
> +		enum pipe pipe = (enum pipe)cpu_transcoder;

Not correct for EDP transcoder.

>  
> -		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder])
> +		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder] &&
> +		    !(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)))
>  			continue;
>  
>  		error->transcoder[i].available = true;
> -- 
> 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
@ 2020-02-04 14:30   ` Ville Syrjälä
  2020-02-04 16:44     ` Ville Syrjälä
  0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 14:30 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Tue, Feb 04, 2020 at 04:59:24PM +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.
> 
> changes since RFC:
> - Simplify the possible_crtcs initialization. [Ville]
> 
> 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 | 12 ++++++++++++
>  drivers/gpu/drm/i915/display/intel_sprite.c  |  2 --
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cf36c3d0f8fc..7c51eb3faeb3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16407,6 +16407,17 @@ static void intel_crtc_free(struct intel_crtc *crtc)
>  	kfree(crtc);
>  }
>  
> +static void intel_plane_possible_crtc_init(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_crtc *crtc;

Move this declaration into the loop body.

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

> +	struct intel_plane *plane;
> +
> +	for_each_intel_plane(&dev_priv->drm, plane) {
> +		crtc = intel_get_crtc_for_pipe(dev_priv, plane->pipe);
> +		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;
> @@ -17546,6 +17557,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
>  		}
>  	}
>  
> +	intel_plane_possible_crtc_init(i915);
>  	intel_shared_dpll_init(dev);
>  	intel_update_fdi_pll_freq(i915);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index fca77ec1e0dd..4a5b192678bf 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -3023,8 +3023,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  	else
>  		plane_type = DRM_PLANE_TYPE_OVERLAY;
>  
> -	possible_crtcs = BIT(pipe);
> -
>  	ret = drm_universal_plane_init(&dev_priv->drm, &plane->base,
>  				       possible_crtcs, plane_funcs,
>  				       formats, num_formats, modifiers,
> -- 
> 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride Anshuman Gupta
@ 2020-02-04 14:30   ` Ville Syrjälä
  0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 14:30 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Tue, Feb 04, 2020 at 04:59:25PM +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.
> 
> changes since RFC:
> - Introduced a helper to get first intel_crtc intel_get_first_crtc. [Ville]
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/intel_display.c       | 5 +++--
>  drivers/gpu/drm/i915/display/intel_display_types.h | 6 ++++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7c51eb3faeb3..0dcf400f6954 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2706,9 +2706,10 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
>  
>  	/*
>  	 * 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);
> +	crtc = intel_get_first_crtc(dev_priv);
>  	if (!crtc)
>  		return 0;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 80a6460da852..1f295c89061a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1423,6 +1423,12 @@ vlv_pipe_to_channel(enum pipe pipe)
>  	}
>  }
>  
> +static inline struct intel_crtc *
> +intel_get_first_crtc(struct drm_i915_private *dev_priv)
> +{
> +	return to_intel_crtc(drm_crtc_from_index(&dev_priv->drm, 0));
> +}
> +
>  static inline struct intel_crtc *
>  intel_get_crtc_for_pipe(struct drm_i915_private *dev_priv, enum pipe 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support Anshuman Gupta
@ 2020-02-04 14:35   ` Ville Syrjälä
  0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 14:35 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Tue, Feb 04, 2020 at 04:59:27PM +0530, Anshuman Gupta wrote:
> Allow 3-display pipes SKU system with any combination
> in INTEL_INFO pipe mask.
> B.Spec:50075
> 
> changes since RFC:
> - using intel_pipe_mask_is_valid() function to check integrity of
>   pipe_mask. [Ville]
> 
> 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 | 38 +++++++++++++++++-------
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index fcdacd6d4aa5..caf93a68a056 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -896,6 +896,30 @@ void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>  	RUNTIME_INFO(i915)->platform_mask[pi] |= mask;
>  }
>  
> +static bool
> +intel_pipe_mask_is_valid(struct drm_i915_private *dev_priv, u8 pipe_mask)
> +{
> +	/*
> +	 * At least one pipe should be enabled.
> +	 */
> +	if (pipe_mask == 0)
> +		return false;

Doesn't that just mean the entire display engine is fused off?

> +	/*
> +	 * if there are disabled pipes they should be the last ones,
> +	 * with no holses in the mask for Dispaly Gen<=12.

"holes"

> +	 */
> +	if (!is_power_of_2(pipe_mask + 1)) {
> +		if (INTEL_GEN(dev_priv) <= 11)
> +			return false;
> +		else if (IS_TIGERLAKE(dev_priv))
> +			return false;
> +		else if (IS_GEN(dev_priv, 12))
> +			return true;

Why is tgl and rest of gen12 treated differently? I thought this
flexible fusing thing was next-gen stuff.

The structure of this function is a bit wonky. Simpler:

intel_pipe_mask_is_valid()
{
	if (is_whatever_supports_holes)
		return true;

	return is_power_of_2();
}


> +	}
> +
> +	return true;
> +}
> +
>  /**
>   * intel_device_info_runtime_init - initialize runtime info
>   * @dev_priv: the i915 device
> @@ -995,17 +1019,11 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  		    (dfsm & TGL_DFSM_PIPE_D_DISABLE))
>  			enabled_mask &= ~BIT(PIPE_D);
>  
> -		/*
> -		 * 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.
> -		 */
> -		if (enabled_mask == 0 || !is_power_of_2(enabled_mask + 1))
> -			drm_err(&dev_priv->drm,
> -				"invalid pipe fuse configuration: enabled_mask=0x%x\n",
> -				enabled_mask);
> -		else
> +		if (intel_pipe_mask_is_valid(dev_priv, enabled_mask))
>  			info->pipe_mask = enabled_mask;
> +		else
> +			drm_err(&dev_priv->drm, "invalid pipe fuse configuration: enabled_mask=0x%x\n",
> +				enabled_mask);
>  
>  		if (dfsm & SKL_DFSM_DISPLAY_HDCP_DISABLE)
>  			info->display.has_hdcp = 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
  2020-02-04 11:52   ` Jani Nikula
@ 2020-02-04 14:36   ` Ville Syrjälä
  2020-02-05  8:02     ` Anshuman Gupta
  1 sibling, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 14:36 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: Cc : Jani Nikula, intel-gfx

On Tue, Feb 04, 2020 at 04:59:22PM +0530, Anshuman Gupta wrote:
> we can't have (pipe == crtc->index) assumption in
> driver in order to support 3 non-contiguous
> display pipe system.
> 
> FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
> till we won't fix all such assumption.
> 
> changes since RFC:
> - Added again removed (pipe == crtc->index) WARN_ON.
> - Pass drm_crtc_index instead of intel pipe in order to
>   call drm_handle_vblank() from gen8_de_irq_handler(),
>   other legacy irq handlers also calls drm_handle_vblank()
>   with intel pipe but those doesn't require this change.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c       | 8 ++++----
>  drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++-
>  drivers/gpu/drm/i915/i915_irq.c                    | 8 ++++++--
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 878d331b9e8c..5709e672151a 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));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 33ba93863488..80a6460da852 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1618,7 +1618,9 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
>  static inline void
>  intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> -	drm_wait_one_vblank(&dev_priv->drm, pipe);
> +	const struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +
> +	drm_wait_one_vblank(&dev_priv->drm, drm_crtc_index(&crtc->base));
>  }
>  static inline void
>  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 22ecd5bc407e..9f8b2566166a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2311,6 +2311,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		u32 fault_errors;
> +		struct intel_crtc *crtc =
> +			intel_get_crtc_for_pipe(dev_priv, pipe);
>  
>  		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
>  			continue;
> @@ -2324,8 +2326,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  		ret = IRQ_HANDLED;
>  		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
>  
> -		if (iir & GEN8_PIPE_VBLANK)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +		if (iir & GEN8_PIPE_VBLANK) {
> +			drm_handle_vblank(&dev_priv->drm,
> +					  drm_crtc_index(&crtc->base));

Missed all the other places.

Please just add intel_handle_vblank() which wraps the
intel_get_crtc_for_pipe()+drm_handle_vblank().

> +		}
>  
>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>  			hsw_pipe_crc_irq_handler(dev_priv, 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state
  2020-02-04 14:28   ` Ville Syrjälä
@ 2020-02-04 16:37     ` Jani Nikula
  2020-02-06 10:23       ` Anshuman Gupta
  0 siblings, 1 reply; 21+ messages in thread
From: Jani Nikula @ 2020-02-04 16:37 UTC (permalink / raw)
  To: Ville Syrjälä, Anshuman Gupta; +Cc: intel-gfx

On Tue, 04 Feb 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Feb 04, 2020 at 04:59:23PM +0530, Anshuman Gupta wrote:
>> Skip the trascoder whose pipe is disabled while
>> initializing trascoder error state in 3 display
>> pipe syatem.
>> 
>> 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 | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 5709e672151a..cf36c3d0f8fc 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -18607,8 +18607,10 @@ intel_display_capture_error_state(struct drm_i915_private *dev_priv)
>>  
>>  	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
>>  		enum transcoder cpu_transcoder = transcoders[i];
>> +		enum pipe pipe = (enum pipe)cpu_transcoder;
>
> Not correct for EDP transcoder.

Nor DSI?

BR,
Jani.

>
>>  
>> -		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder])
>> +		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder] &&
>> +		    !(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)))
>>  			continue;
>>  
>>  		error->transcoder[i].available = true;
>> -- 
>> 2.24.0

-- 
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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-02-04 14:30   ` Ville Syrjälä
@ 2020-02-04 16:44     ` Ville Syrjälä
  2020-02-06 10:15       ` Anshuman Gupta
  0 siblings, 1 reply; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-04 16:44 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

On Tue, Feb 04, 2020 at 04:30:16PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 04, 2020 at 04:59:24PM +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.
> > 
> > changes since RFC:
> > - Simplify the possible_crtcs initialization. [Ville]
> > 
> > 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 | 12 ++++++++++++
> >  drivers/gpu/drm/i915/display/intel_sprite.c  |  2 --
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index cf36c3d0f8fc..7c51eb3faeb3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -16407,6 +16407,17 @@ static void intel_crtc_free(struct intel_crtc *crtc)
> >  	kfree(crtc);
> >  }
> >  
> > +static void intel_plane_possible_crtc_init(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_crtc *crtc;
> 
> Move this declaration into the loop body.
> 
> With that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +	struct intel_plane *plane;
> > +
> > +	for_each_intel_plane(&dev_priv->drm, plane) {
> > +		crtc = intel_get_crtc_for_pipe(dev_priv, plane->pipe);
> > +		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;
> > @@ -17546,6 +17557,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
> >  		}
> >  	}
> >  
> > +	intel_plane_possible_crtc_init(i915);
> >  	intel_shared_dpll_init(dev);
> >  	intel_update_fdi_pll_freq(i915);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index fca77ec1e0dd..4a5b192678bf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -3023,8 +3023,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> >  	else
> >  		plane_type = DRM_PLANE_TYPE_OVERLAY;
> >  
> > -	possible_crtcs = BIT(pipe);
> > -

Actually that now contains stack garbage. Pls remove the variable
entire and just pass zero to the thing.

> >  	ret = drm_universal_plane_init(&dev_priv->drm, &plane->base,
> >  				       possible_crtcs, plane_funcs,
> >  				       formats, num_formats, modifiers,
> > -- 
> > 2.24.0
> 
> -- 
> Ville Syrjälä
> Intel

-- 
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] 21+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for 3 display pipes combination system support (rev2)
  2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
                   ` (6 preceding siblings ...)
  2020-02-04 11:29 ` [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support Anshuman Gupta
@ 2020-02-05  1:36 ` Patchwork
  7 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2020-02-05  1:36 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx

== Series Details ==

Series: 3 display pipes combination system support (rev2)
URL   : https://patchwork.freedesktop.org/series/72468/
State : failure

== Summary ==

Applying: drm/i915: Iterate over pipe and skip the disabled one
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display.h
M	drivers/gpu/drm/i915/i915_irq.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_irq.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_irq.c
Auto-merging drivers/gpu/drm/i915/display/intel_display.h
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915: Iterate over pipe and skip the disabled one
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

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

* Re: [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption
  2020-02-04 14:36   ` Ville Syrjälä
@ 2020-02-05  8:02     ` Anshuman Gupta
  2020-02-05 11:07       ` Ville Syrjälä
  0 siblings, 1 reply; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-05  8:02 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Cc : Jani Nikula, intel-gfx

On 2020-02-04 at 16:36:17 +0200, Ville Syrjälä wrote:
> On Tue, Feb 04, 2020 at 04:59:22PM +0530, Anshuman Gupta wrote:
> > we can't have (pipe == crtc->index) assumption in
> > driver in order to support 3 non-contiguous
> > display pipe system.
> > 
> > FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
> > till we won't fix all such assumption.
> > 
> > changes since RFC:
> > - Added again removed (pipe == crtc->index) WARN_ON.
> > - Pass drm_crtc_index instead of intel pipe in order to
> >   call drm_handle_vblank() from gen8_de_irq_handler(),
> >   other legacy irq handlers also calls drm_handle_vblank()
> >   with intel pipe but those doesn't require this change.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c       | 8 ++++----
> >  drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++-
> >  drivers/gpu/drm/i915/i915_irq.c                    | 8 ++++++--
> >  3 files changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 878d331b9e8c..5709e672151a 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));
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 33ba93863488..80a6460da852 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1618,7 +1618,9 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
> >  static inline void
> >  intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
> >  {
> > -	drm_wait_one_vblank(&dev_priv->drm, pipe);
> > +	const struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > +
> > +	drm_wait_one_vblank(&dev_priv->drm, drm_crtc_index(&crtc->base));
> >  }
> >  static inline void
> >  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 22ecd5bc407e..9f8b2566166a 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2311,6 +2311,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >  
> >  	for_each_pipe(dev_priv, pipe) {
> >  		u32 fault_errors;
> > +		struct intel_crtc *crtc =
> > +			intel_get_crtc_for_pipe(dev_priv, pipe);
> >  
> >  		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
> >  			continue;
> > @@ -2324,8 +2326,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> >  		ret = IRQ_HANDLED;
> >  		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
> >  
> > -		if (iir & GEN8_PIPE_VBLANK)
> > -			drm_handle_vblank(&dev_priv->drm, pipe);
> > +		if (iir & GEN8_PIPE_VBLANK) {
> > +			drm_handle_vblank(&dev_priv->drm,
> > +					  drm_crtc_index(&crtc->base));
> 
> Missed all the other places.
All other places were legcay handlers so i thought we don't require this change,
is it require do this change all places to keep same pattern.
Please correct me if i am wrong here.
> 
> Please just add intel_handle_vblank() which wraps the
> intel_get_crtc_for_pipe()+drm_handle_vblank().
jani has suggested to use drm_crtc_handle_vblank(), i think 
that would be simpler to replace it at all places instead of
intel_handle_vblank(), what is your opinion on that ?
Thanks,
Anshuman Gupta.
> 
> > +		}
> >  
> >  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
> >  			hsw_pipe_crc_irq_handler(dev_priv, 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption
  2020-02-05  8:02     ` Anshuman Gupta
@ 2020-02-05 11:07       ` Ville Syrjälä
  0 siblings, 0 replies; 21+ messages in thread
From: Ville Syrjälä @ 2020-02-05 11:07 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: Cc : Jani Nikula, intel-gfx

On Wed, Feb 05, 2020 at 01:32:54PM +0530, Anshuman Gupta wrote:
> On 2020-02-04 at 16:36:17 +0200, Ville Syrjälä wrote:
> > On Tue, Feb 04, 2020 at 04:59:22PM +0530, Anshuman Gupta wrote:
> > > we can't have (pipe == crtc->index) assumption in
> > > driver in order to support 3 non-contiguous
> > > display pipe system.
> > > 
> > > FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
> > > till we won't fix all such assumption.
> > > 
> > > changes since RFC:
> > > - Added again removed (pipe == crtc->index) WARN_ON.
> > > - Pass drm_crtc_index instead of intel pipe in order to
> > >   call drm_handle_vblank() from gen8_de_irq_handler(),
> > >   other legacy irq handlers also calls drm_handle_vblank()
> > >   with intel pipe but those doesn't require this change.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c       | 8 ++++----
> > >  drivers/gpu/drm/i915/display/intel_display_types.h | 4 +++-
> > >  drivers/gpu/drm/i915/i915_irq.c                    | 8 ++++++--
> > >  3 files changed, 13 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 878d331b9e8c..5709e672151a 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));
> > >  	}
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 33ba93863488..80a6460da852 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1618,7 +1618,9 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
> > >  static inline void
> > >  intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >  {
> > > -	drm_wait_one_vblank(&dev_priv->drm, pipe);
> > > +	const struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > > +
> > > +	drm_wait_one_vblank(&dev_priv->drm, drm_crtc_index(&crtc->base));
> > >  }
> > >  static inline void
> > >  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 22ecd5bc407e..9f8b2566166a 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2311,6 +2311,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> > >  
> > >  	for_each_pipe(dev_priv, pipe) {
> > >  		u32 fault_errors;
> > > +		struct intel_crtc *crtc =
> > > +			intel_get_crtc_for_pipe(dev_priv, pipe);
> > >  
> > >  		if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
> > >  			continue;
> > > @@ -2324,8 +2326,10 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> > >  		ret = IRQ_HANDLED;
> > >  		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
> > >  
> > > -		if (iir & GEN8_PIPE_VBLANK)
> > > -			drm_handle_vblank(&dev_priv->drm, pipe);
> > > +		if (iir & GEN8_PIPE_VBLANK) {
> > > +			drm_handle_vblank(&dev_priv->drm,
> > > +					  drm_crtc_index(&crtc->base));
> > 
> > Missed all the other places.
> All other places were legcay handlers so i thought we don't require this change,
> is it require do this change all places to keep same pattern.

Yes.

> Please correct me if i am wrong here.
> > 
> > Please just add intel_handle_vblank() which wraps the
> > intel_get_crtc_for_pipe()+drm_handle_vblank().
> jani has suggested to use drm_crtc_handle_vblank(), i think 
> that would be simpler to replace it at all places instead of
> intel_handle_vblank(), what is your opinion on that ?

That's orthogonal.

> Thanks,
> Anshuman Gupta.
> > 
> > > +		}
> > >  
> > >  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
> > >  			hsw_pipe_crc_irq_handler(dev_priv, pipe);
> > > -- 
> > > 2.24.0
> > 
> > -- 
> > Ville Syrjälä
> > Intel

-- 
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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask
  2020-02-04 16:44     ` Ville Syrjälä
@ 2020-02-06 10:15       ` Anshuman Gupta
  0 siblings, 0 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-06 10:15 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On 2020-02-04 at 18:44:32 +0200, Ville Syrjälä wrote:
> On Tue, Feb 04, 2020 at 04:30:16PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 04, 2020 at 04:59:24PM +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.
> > > 
> > > changes since RFC:
> > > - Simplify the possible_crtcs initialization. [Ville]
> > > 
> > > 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 | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/display/intel_sprite.c  |  2 --
> > >  2 files changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index cf36c3d0f8fc..7c51eb3faeb3 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -16407,6 +16407,17 @@ static void intel_crtc_free(struct intel_crtc *crtc)
> > >  	kfree(crtc);
> > >  }
> > >  
> > > +static void intel_plane_possible_crtc_init(struct drm_i915_private *dev_priv)
> > > +{
> > > +	struct intel_crtc *crtc;
> > 
> > Move this declaration into the loop body.
> > 
> > With that
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks Ville for review, I will move it to loop body.
Thanks,
Anshuman
> > 
> > > +	struct intel_plane *plane;
> > > +
> > > +	for_each_intel_plane(&dev_priv->drm, plane) {
> > > +		crtc = intel_get_crtc_for_pipe(dev_priv, plane->pipe);
> > > +		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;
> > > @@ -17546,6 +17557,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
> > >  		}
> > >  	}
> > >  
> > > +	intel_plane_possible_crtc_init(i915);
> > >  	intel_shared_dpll_init(dev);
> > >  	intel_update_fdi_pll_freq(i915);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > index fca77ec1e0dd..4a5b192678bf 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > @@ -3023,8 +3023,6 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> > >  	else
> > >  		plane_type = DRM_PLANE_TYPE_OVERLAY;
> > >  
> > > -	possible_crtcs = BIT(pipe);
> > > -
> 
> Actually that now contains stack garbage. Pls remove the variable
> entire and just pass zero to the thing.
> 
> > >  	ret = drm_universal_plane_init(&dev_priv->drm, &plane->base,
> > >  				       possible_crtcs, plane_funcs,
> > >  				       formats, num_formats, modifiers,
> > > -- 
> > > 2.24.0
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> 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] 21+ messages in thread

* Re: [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state
  2020-02-04 16:37     ` Jani Nikula
@ 2020-02-06 10:23       ` Anshuman Gupta
  0 siblings, 0 replies; 21+ messages in thread
From: Anshuman Gupta @ 2020-02-06 10:23 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On 2020-02-04 at 18:37:26 +0200, Jani Nikula wrote:
> On Tue, 04 Feb 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Feb 04, 2020 at 04:59:23PM +0530, Anshuman Gupta wrote:
> >> Skip the trascoder whose pipe is disabled while
> >> initializing trascoder error state in 3 display
> >> pipe syatem.
> >> 
> >> 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 | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 5709e672151a..cf36c3d0f8fc 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -18607,8 +18607,10 @@ intel_display_capture_error_state(struct drm_i915_private *dev_priv)
> >>  
> >>  	for (i = 0; i < ARRAY_SIZE(error->transcoder); i++) {
> >>  		enum transcoder cpu_transcoder = transcoders[i];
> >> +		enum pipe pipe = (enum pipe)cpu_transcoder;
> >
> > Not correct for EDP transcoder.
> 
> Nor DSI?
> 
> BR,
> Jani.
> 
> >
> >>  
> >> -		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder])
Is it ok to make trans_offsets[] NULL for disabled pipe as an another approach?
Thanks,
Anshuman Gupta.
> >> +		if (!INTEL_INFO(dev_priv)->trans_offsets[cpu_transcoder] &&
> >> +		    !(INTEL_INFO(dev_priv)->pipe_mask & BIT(pipe)))
> >>  			continue;
> >>  
> >>  		error->transcoder[i].available = true;
> >> -- 
> >> 2.24.0
> 
> -- 
> 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] 21+ messages in thread

end of thread, other threads:[~2020-02-06 10:31 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-04 11:29 [Intel-gfx] [PATCH 0/7] 3 display pipes combination system support Anshuman Gupta
2020-02-04 11:29 ` [Intel-gfx] [PATCH 1/7] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
2020-02-04 11:29 ` [Intel-gfx] [PATCH 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
2020-02-04 11:52   ` Jani Nikula
2020-02-04 14:36   ` Ville Syrjälä
2020-02-05  8:02     ` Anshuman Gupta
2020-02-05 11:07       ` Ville Syrjälä
2020-02-04 11:29 ` [Intel-gfx] [PATCH 3/7] drm/i915: Fix broken transcoder err state Anshuman Gupta
2020-02-04 14:28   ` Ville Syrjälä
2020-02-04 16:37     ` Jani Nikula
2020-02-06 10:23       ` Anshuman Gupta
2020-02-04 11:29 ` [Intel-gfx] [PATCH 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
2020-02-04 14:30   ` Ville Syrjälä
2020-02-04 16:44     ` Ville Syrjälä
2020-02-06 10:15       ` Anshuman Gupta
2020-02-04 11:29 ` [Intel-gfx] [PATCH 5/7] drm/i915: Get right max plane stride Anshuman Gupta
2020-02-04 14:30   ` Ville Syrjälä
2020-02-04 11:29 ` [Intel-gfx] [PATCH 6/7] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
2020-02-04 11:29 ` [Intel-gfx] [PATCH 7/7] drm/i915: Enable 3 display pipes support Anshuman Gupta
2020-02-04 14:35   ` Ville Syrjälä
2020-02-05  1:36 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for 3 display pipes combination system support (rev2) 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.