All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] drm/i915: Clean up the port pipe select bits
@ 2018-03-02  9:54 Ville Syrjala
  2018-03-02  9:54 ` [PATCH 01/14] drm/i915: Clean up ADPA " Ville Syrjala
                   ` (16 more replies)
  0 siblings, 17 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:54 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bit of an effort to rid outselves of PORT_TO_PIPE() & co. The idea
with those macros was to share them between all the port registers,
but since not all port registers follow the same bit layout they're
kinda just making it harder to see what goes where. So I decided to
just define the pipe select bits individually for each register.

There's also a bit of cleanup on the DDI vswing/pre-emphasis stuff,
and elimination of the pre-atomic intel_trans_dp_port_sel().

I've given this the standard smoke test on ILK, IVB and CHV.

Entire pile available here:
git://github.com/vsyrjala/linux.git port_sel_cleanup

Ville Syrjälä (14):
  drm/i915: Clean up ADPA pipe select bits
  drm/i915: Clean up LVDS pipe select bits
  drm/i915: Clean up SDVO pipe select bits
  drm/i915: Clean up TV pipe select bits
  drm/i915: Clean up DVO pipe select bits
  drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too
  drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on
    SKL+
  drm/i915: Check for IVB instead of gen7 when we think about IVB CPU
    eDP
  drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
  drm/i915: Parametrize TRANS_DP_PORT_SEL
  drm/i915: Nuke intel_trans_dp_port_sel()
  drm/i915: Clean up DP pipe select bits
  drm/i915: Allow eDP on port C in theory
  drm/i915: Implement the missing bits of assert_panel_unlocked()

 drivers/gpu/drm/i915/i915_reg.h      |  64 +++++-----
 drivers/gpu/drm/i915/intel_crt.c     |  40 +++---
 drivers/gpu/drm/i915/intel_ddi.c     |  49 +++-----
 drivers/gpu/drm/i915/intel_display.c | 229 ++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_dp.c      | 170 +++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |  13 +-
 drivers/gpu/drm/i915/intel_dvo.c     |  13 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |  25 +---
 drivers/gpu/drm/i915/intel_lvds.c    |  54 +++++----
 drivers/gpu/drm/i915/intel_sdvo.c    |  38 +++---
 drivers/gpu/drm/i915/intel_tv.c      |  18 +--
 11 files changed, 327 insertions(+), 386 deletions(-)

-- 
2.13.6

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

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

* [PATCH 01/14] drm/i915: Clean up ADPA pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
@ 2018-03-02  9:54 ` Ville Syrjala
  2018-03-20  6:27   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 02/14] drm/i915: Clean up LVDS " Ville Syrjala
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:54 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the ADPA pipe select bits. To make the whole situation a bit
less ugly we'll start to share the same code between .get_hw_state()
and the port state asserts.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 11 +++++-----
 drivers/gpu/drm/i915/intel_crt.c     | 40 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c | 24 +++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 4 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95a2e51ecbb0..f573095d60c2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4199,11 +4199,12 @@ enum {
 
 #define   ADPA_DAC_ENABLE	(1<<31)
 #define   ADPA_DAC_DISABLE	0
-#define   ADPA_PIPE_SELECT_MASK	(1<<30)
-#define   ADPA_PIPE_A_SELECT	0
-#define   ADPA_PIPE_B_SELECT	(1<<30)
-#define   ADPA_PIPE_SELECT(pipe) ((pipe) << 30)
-/* CPT uses bits 29:30 for pch transcoder select */
+#define   ADPA_PIPE_SEL_MASK		(1<<30)
+#define   ADPA_PIPE_SEL_SHIFT		30
+#define   ADPA_PIPE_SEL(pipe)		((pipe) << 30)
+#define   ADPA_PIPE_SEL_MASK_CPT	(3<<29)
+#define   ADPA_PIPE_SEL_SHIFT_CPT	29
+#define   ADPA_PIPE_SEL_CPT(pipe)	((pipe) << 29)
 #define   ADPA_CRT_HOTPLUG_MASK  0x03ff0000 /* bit 25-16 */
 #define   ADPA_CRT_HOTPLUG_MONITOR_NONE  (0<<24)
 #define   ADPA_CRT_HOTPLUG_MONITOR_MASK  (3<<24)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 391dd69ae0a4..88889af44608 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -63,33 +63,35 @@ static struct intel_crt *intel_attached_crt(struct drm_connector *connector)
 	return intel_encoder_to_crt(intel_attached_encoder(connector));
 }
 
+bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
+			    i915_reg_t adpa_reg, enum pipe *pipe)
+{
+	u32 val;
+
+	val = I915_READ(adpa_reg);
+
+	/* asserts want to know the pipe even if the port is disabled */
+	if (HAS_PCH_CPT(dev_priv))
+		*pipe = (val & ADPA_PIPE_SEL_MASK_CPT) >> ADPA_PIPE_SEL_SHIFT_CPT;
+	else
+		*pipe = (val & ADPA_PIPE_SEL_MASK) >> ADPA_PIPE_SEL_SHIFT;
+
+	return val & ADPA_DAC_ENABLE;
+}
+
 static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 				   enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
-	u32 tmp;
 	bool ret;
 
 	if (!intel_display_power_get_if_enabled(dev_priv,
 						encoder->power_domain))
 		return false;
 
-	ret = false;
-
-	tmp = I915_READ(crt->adpa_reg);
-
-	if (!(tmp & ADPA_DAC_ENABLE))
-		goto out;
+	ret = intel_crt_port_enabled(dev_priv, crt->adpa_reg, pipe);
 
-	if (HAS_PCH_CPT(dev_priv))
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	else
-		*pipe = PORT_TO_PIPE(tmp);
-
-	ret = true;
-out:
 	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
@@ -168,11 +170,9 @@ static void intel_crt_set_dpms(struct intel_encoder *encoder,
 	if (HAS_PCH_LPT(dev_priv))
 		; /* Those bits don't exist here */
 	else if (HAS_PCH_CPT(dev_priv))
-		adpa |= PORT_TRANS_SEL_CPT(crtc->pipe);
-	else if (crtc->pipe == 0)
-		adpa |= ADPA_PIPE_A_SELECT;
+		adpa |= ADPA_PIPE_SEL_CPT(crtc->pipe);
 	else
-		adpa |= ADPA_PIPE_B_SELECT;
+		adpa |= ADPA_PIPE_SEL(crtc->pipe);
 
 	if (!HAS_PCH_SPLIT(dev_priv))
 		I915_WRITE(BCLRPAT(crtc->pipe), 0);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 740a918ee578..545d89152e9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1317,21 +1317,6 @@ static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-static bool adpa_pipe_enabled(struct drm_i915_private *dev_priv,
-			      enum pipe pipe, u32 val)
-{
-	if ((val & ADPA_DAC_ENABLE) == 0)
-		return false;
-	if (HAS_PCH_CPT(dev_priv)) {
-		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
-			return false;
-	} else {
-		if ((val & ADPA_PIPE_SELECT_MASK) != ADPA_PIPE_SELECT(pipe))
-			return false;
-	}
-	return true;
-}
-
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe, i915_reg_t reg,
 				   u32 port_sel)
@@ -1362,16 +1347,17 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
 static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 				      enum pipe pipe)
 {
+	enum pipe port_pipe;
 	u32 val;
 
 	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
 	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
 	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
 
-	val = I915_READ(PCH_ADPA);
-	I915_STATE_WARN(adpa_pipe_enabled(dev_priv, pipe, val),
-	     "PCH VGA enabled on transcoder %c, should be disabled\n",
-	     pipe_name(pipe));
+	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
+			port_pipe == pipe,
+			"PCH VGA enabled on transcoder %c, should be disabled\n",
+			pipe_name(pipe));
 
 	val = I915_READ(PCH_LVDS);
 	I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val),
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 652b11e788cc..79e741845e16 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1355,6 +1355,8 @@ void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv);
 void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
 
 /* intel_crt.c */
+bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
+			    i915_reg_t adpa_reg, enum pipe *pipe);
 void intel_crt_init(struct drm_i915_private *dev_priv);
 void intel_crt_reset(struct drm_encoder *encoder);
 
-- 
2.13.6

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

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

* [PATCH 02/14] drm/i915: Clean up LVDS pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
  2018-03-02  9:54 ` [PATCH 01/14] drm/i915: Clean up ADPA " Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  6:39   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 03/14] drm/i915: Clean up SDVO " Ville Syrjala
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the LVDS pipe select bits. To make the whole situation a bit
less ugly we'll start to share the same code between .get_hw_state()
and the port state asserts.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  9 ++++--
 drivers/gpu/drm/i915/intel_display.c | 35 ++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_lvds.c    | 54 +++++++++++++++++++-----------------
 4 files changed, 44 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f573095d60c2..e993eec97c98 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4458,9 +4458,12 @@ enum {
  */
 #define   LVDS_PORT_EN			(1 << 31)
 /* Selects pipe B for LVDS data.  Must be set on pre-965. */
-#define   LVDS_PIPEB_SELECT		(1 << 30)
-#define   LVDS_PIPE_MASK		(1 << 30)
-#define   LVDS_PIPE(pipe)		((pipe) << 30)
+#define   LVDS_PIPE_SEL(pipe)		((pipe) << 30)
+#define   LVDS_PIPE_SEL_MASK		(1 << 30)
+#define   LVDS_PIPE_SEL_SHIFT		30
+#define   LVDS_PIPE_SEL_CPT(pipe)	((pipe) << 29)
+#define   LVDS_PIPE_SEL_MASK_CPT	(3 << 30)
+#define   LVDS_PIPE_SEL_SHIFT_CPT	29
 /* LVDS dithering flag on 965/g4x platform */
 #define   LVDS_ENABLE_DITHER		(1 << 25)
 /* LVDS sync polarity flags. Set to invert (i.e. negative) */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 545d89152e9b..f1f164a20b3f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1171,9 +1171,9 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 		pp_reg = PP_CONTROL(0);
 		port_sel = I915_READ(PP_ON_DELAYS(0)) & PANEL_PORT_SELECT_MASK;
 
-		if (port_sel == PANEL_PORT_SELECT_LVDS &&
-		    I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
-			panel_pipe = PIPE_B;
+		if (port_sel == PANEL_PORT_SELECT_LVDS) {
+			intel_lvds_port_enabled(dev_priv, PCH_LVDS, &panel_pipe);
+		}
 		/* XXX: else fix for eDP */
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		/* presumably write lock depends on pipe, not port select */
@@ -1181,8 +1181,7 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 		panel_pipe = pipe;
 	} else {
 		pp_reg = PP_CONTROL(0);
-		if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
-			panel_pipe = PIPE_B;
+		intel_lvds_port_enabled(dev_priv, LVDS, &panel_pipe);
 	}
 
 	val = I915_READ(pp_reg);
@@ -1301,22 +1300,6 @@ static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
-			      enum pipe pipe, u32 val)
-{
-	if ((val & LVDS_PORT_EN) == 0)
-		return false;
-
-	if (HAS_PCH_CPT(dev_priv)) {
-		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
-			return false;
-	} else {
-		if ((val & LVDS_PIPE_MASK) != LVDS_PIPE(pipe))
-			return false;
-	}
-	return true;
-}
-
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe, i915_reg_t reg,
 				   u32 port_sel)
@@ -1348,7 +1331,6 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 				      enum pipe pipe)
 {
 	enum pipe port_pipe;
-	u32 val;
 
 	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
 	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
@@ -1359,10 +1341,10 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 			"PCH VGA enabled on transcoder %c, should be disabled\n",
 			pipe_name(pipe));
 
-	val = I915_READ(PCH_LVDS);
-	I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val),
-	     "PCH LVDS enabled on transcoder %c, should be disabled\n",
-	     pipe_name(pipe));
+	I915_STATE_WARN(intel_lvds_port_enabled(dev_priv, PCH_LVDS, &port_pipe) &&
+			port_pipe == pipe,
+			"PCH LVDS enabled on transcoder %c, should be disabled\n",
+			pipe_name(pipe));
 
 	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
 	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
@@ -1405,7 +1387,6 @@ static void vlv_enable_pll(struct intel_crtc *crtc,
 	POSTING_READ(DPLL_MD(pipe));
 }
 
-
 static void _chv_enable_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 79e741845e16..16b7adc7b0c9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1789,6 +1789,8 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
 
 
 /* intel_lvds.c */
+bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
+			     i915_reg_t lvds_reg, enum pipe *pipe);
 void intel_lvds_init(struct drm_i915_private *dev_priv);
 struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev);
 bool intel_is_dual_link_lvds(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d35d2d50f595..571b69f79eda 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -85,34 +85,35 @@ static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *conn
 	return container_of(connector, struct intel_lvds_connector, base.base);
 }
 
+bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
+			     i915_reg_t lvds_reg, enum pipe *pipe)
+{
+	u32 val;
+
+	val = I915_READ(lvds_reg);
+
+	/* asserts want to know the pipe even if the port is disabled */
+	if (HAS_PCH_CPT(dev_priv))
+		*pipe = (val & LVDS_PIPE_SEL_MASK_CPT) >> LVDS_PIPE_SEL_SHIFT_CPT;
+	else
+		*pipe = (val & LVDS_PIPE_SEL_MASK) >> LVDS_PIPE_SEL_SHIFT;
+
+	return val & LVDS_PORT_EN;
+}
+
 static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 				    enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
-	u32 tmp;
 	bool ret;
 
 	if (!intel_display_power_get_if_enabled(dev_priv,
 						encoder->power_domain))
 		return false;
 
-	ret = false;
+	ret = intel_lvds_port_enabled(dev_priv, lvds_encoder->reg, pipe);
 
-	tmp = I915_READ(lvds_encoder->reg);
-
-	if (!(tmp & LVDS_PORT_EN))
-		goto out;
-
-	if (HAS_PCH_CPT(dev_priv))
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	else
-		*pipe = PORT_TO_PIPE(tmp);
-
-	ret = true;
-
-out:
 	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
@@ -255,14 +256,11 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder,
 	temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
 
 	if (HAS_PCH_CPT(dev_priv)) {
-		temp &= ~PORT_TRANS_SEL_MASK;
-		temp |= PORT_TRANS_SEL_CPT(pipe);
+		temp &= ~LVDS_PIPE_SEL_MASK_CPT;
+		temp |= LVDS_PIPE_SEL_CPT(pipe);
 	} else {
-		if (pipe == 1) {
-			temp |= LVDS_PIPEB_SELECT;
-		} else {
-			temp &= ~LVDS_PIPEB_SELECT;
-		}
+		temp &= ~LVDS_PIPE_SEL_MASK;
+		temp |= LVDS_PIPE_SEL(pipe);
 	}
 
 	/* set the corresponsding LVDS_BORDER bit */
@@ -906,8 +904,12 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder)
 	 * we need to check "the value to be set" in VBT when LVDS
 	 * register is uninitialized.
 	 */
-	val = I915_READ(lvds_encoder->reg);
-	if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
+	val = I915_READ(lvds_encoder->reg) & ~LVDS_DETECTED;
+	if (HAS_PCH_CPT(dev_priv))
+		val &= ~LVDS_PIPE_SEL_MASK_CPT;
+	else
+		val &= ~LVDS_PIPE_SEL_MASK;
+	if (val == 0)
 		val = dev_priv->vbt.bios_lvds_val;
 
 	return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;
-- 
2.13.6

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

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

* [PATCH 03/14] drm/i915: Clean up SDVO pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
  2018-03-02  9:54 ` [PATCH 01/14] drm/i915: Clean up ADPA " Ville Syrjala
  2018-03-02  9:55 ` [PATCH 02/14] drm/i915: Clean up LVDS " Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  6:50   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 04/14] drm/i915: Clean up TV " Ville Syrjala
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the SDVO pipe select bits. To make the whole situation a bit
less ugly we'll start to share the same code between .get_hw_state()
and the port state asserts.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 +++-
 drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c    | 25 ++++----------------
 drivers/gpu/drm/i915/intel_sdvo.c    | 38 ++++++++++++++++++-----------
 5 files changed, 49 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e993eec97c98..0917fcbd618d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4365,7 +4365,7 @@ enum {
 #define   SDVO_ENABLE				(1 << 31)
 #define   SDVO_PIPE_SEL(pipe)			((pipe) << 30)
 #define   SDVO_PIPE_SEL_MASK			(1 << 30)
-#define   SDVO_PIPE_B_SELECT			(1 << 30)
+#define   SDVO_PIPE_SEL_SHIFT			30
 #define   SDVO_STALL_SELECT			(1 << 29)
 #define   SDVO_INTERRUPT_ENABLE			(1 << 26)
 /*
@@ -4407,10 +4407,12 @@ enum {
 /* Gen 6 (CPT) SDVO/HDMI bits: */
 #define   SDVO_PIPE_SEL_CPT(pipe)		((pipe) << 29)
 #define   SDVO_PIPE_SEL_MASK_CPT		(3 << 29)
+#define   SDVO_PIPE_SEL_SHIFT_CPT		29
 
 /* CHV SDVO/HDMI bits: */
 #define   SDVO_PIPE_SEL_CHV(pipe)		((pipe) << 24)
 #define   SDVO_PIPE_SEL_MASK_CHV		(3 << 24)
+#define   SDVO_PIPE_SEL_SHIFT_CHV		24
 
 
 /* DVO port control */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1f164a20b3f..26f8f1e741f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1281,25 +1281,6 @@ static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
 	return true;
 }
 
-static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
-			      enum pipe pipe, u32 val)
-{
-	if ((val & SDVO_ENABLE) == 0)
-		return false;
-
-	if (HAS_PCH_CPT(dev_priv)) {
-		if ((val & SDVO_PIPE_SEL_MASK_CPT) != SDVO_PIPE_SEL_CPT(pipe))
-			return false;
-	} else if (IS_CHERRYVIEW(dev_priv)) {
-		if ((val & SDVO_PIPE_SEL_MASK_CHV) != SDVO_PIPE_SEL_CHV(pipe))
-			return false;
-	} else {
-		if ((val & SDVO_PIPE_SEL_MASK) != SDVO_PIPE_SEL(pipe))
-			return false;
-	}
-	return true;
-}
-
 static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 				   enum pipe pipe, i915_reg_t reg,
 				   u32 port_sel)
@@ -1315,16 +1296,21 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
 }
 
 static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
-				     enum pipe pipe, i915_reg_t reg)
+				     enum pipe pipe, enum port port,
+				     i915_reg_t hdmi_reg)
 {
-	u32 val = I915_READ(reg);
-	I915_STATE_WARN(hdmi_pipe_enabled(dev_priv, pipe, val),
-	     "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n",
-	     i915_mmio_reg_offset(reg), pipe_name(pipe));
+	enum pipe port_pipe;
+	bool state;
+
+	state = intel_sdvo_port_enabled(dev_priv, hdmi_reg, &port_pipe);
+
+	I915_STATE_WARN(state && port_pipe == pipe,
+			"PCH HDMI %c enabled on transcoder %c, should be disabled\n",
+			port_name(port), pipe_name(pipe));
 
-	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & SDVO_ENABLE) == 0
-	     && (val & SDVO_PIPE_B_SELECT),
-	     "IBX PCH hdmi port still using transcoder B\n");
+	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B,
+			"IBX PCH HDMI %c still using transcoder B\n",
+			port_name(port));
 }
 
 static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
@@ -1346,9 +1332,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 			"PCH LVDS enabled on transcoder %c, should be disabled\n",
 			pipe_name(pipe));
 
-	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
-	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
-	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_B, PCH_HDMIB);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_C, PCH_HDMIC);
+	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_D, PCH_HDMID);
 }
 
 static void _vlv_enable_pll(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 16b7adc7b0c9..09a1b968ac9c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2024,6 +2024,8 @@ void intel_init_ipc(struct drm_i915_private *dev_priv);
 void intel_enable_ipc(struct drm_i915_private *dev_priv);
 
 /* intel_sdvo.c */
+bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv,
+			     i915_reg_t sdvo_reg, enum pipe *pipe);
 bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 		     i915_reg_t reg, enum port port);
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f5d7bfb43006..bf6facec4efb 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1161,33 +1161,16 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder,
 static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 				    enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
-	u32 tmp;
 	bool ret;
 
 	if (!intel_display_power_get_if_enabled(dev_priv,
 						encoder->power_domain))
 		return false;
 
-	ret = false;
-
-	tmp = I915_READ(intel_hdmi->hdmi_reg);
-
-	if (!(tmp & SDVO_ENABLE))
-		goto out;
-
-	if (HAS_PCH_CPT(dev_priv))
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	else if (IS_CHERRYVIEW(dev_priv))
-		*pipe = SDVO_PORT_TO_PIPE_CHV(tmp);
-	else
-		*pipe = PORT_TO_PIPE(tmp);
-
-	ret = true;
+	ret = intel_sdvo_port_enabled(dev_priv, intel_hdmi->hdmi_reg, pipe);
 
-out:
 	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
@@ -1421,8 +1404,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
-		temp &= ~SDVO_PIPE_B_SELECT;
-		temp |= SDVO_ENABLE;
+		temp &= ~SDVO_PIPE_SEL_MASK;
+		temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A);
 		/*
 		 * HW workaround, need to write this twice for issue
 		 * that may result in first write getting masked.
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 0c14d1c04cbd..3a90bcfbbedb 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1403,27 +1403,37 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector)
 		return false;
 }
 
+bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv,
+			     i915_reg_t sdvo_reg, enum pipe *pipe)
+{
+	u32 val;
+
+	val = I915_READ(sdvo_reg);
+
+	/* asserts want to know the pipe even if the port is disabled */
+	if (HAS_PCH_CPT(dev_priv))
+		*pipe = (val & SDVO_PIPE_SEL_MASK_CPT) >> SDVO_PIPE_SEL_SHIFT_CPT;
+	else if (IS_CHERRYVIEW(dev_priv))
+		*pipe = (val & SDVO_PIPE_SEL_MASK_CHV) >> SDVO_PIPE_SEL_SHIFT_CHV;
+	else
+		*pipe = (val & SDVO_PIPE_SEL_MASK) >> SDVO_PIPE_SEL_SHIFT;
+
+	return val & SDVO_ENABLE;
+}
+
 static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
 				    enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 	u16 active_outputs = 0;
-	u32 tmp;
+	bool ret;
 
-	tmp = I915_READ(intel_sdvo->sdvo_reg);
 	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
 
-	if (!(tmp & SDVO_ENABLE) && (active_outputs == 0))
-		return false;
-
-	if (HAS_PCH_CPT(dev_priv))
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	else
-		*pipe = PORT_TO_PIPE(tmp);
+	ret = intel_sdvo_port_enabled(dev_priv, intel_sdvo->sdvo_reg, pipe);
 
-	return true;
+	return ret || active_outputs;
 }
 
 static void intel_sdvo_get_config(struct intel_encoder *encoder,
@@ -1550,8 +1560,8 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
-		temp &= ~SDVO_PIPE_B_SELECT;
-		temp |= SDVO_ENABLE;
+		temp &= ~SDVO_PIPE_SEL_MASK;
+		temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A);
 		intel_sdvo_write_sdvox(intel_sdvo, temp);
 
 		temp &= ~SDVO_ENABLE;
-- 
2.13.6

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

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

* [PATCH 04/14] drm/i915: Clean up TV pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (2 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 03/14] drm/i915: Clean up SDVO " Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  6:55   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 05/14] drm/i915: Clean up DVO " Ville Syrjala
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Parametrize the TV pipe select bits.

For consistency with the new way of doing things, let's read out the
pipe select bits even when the port is disable, even though we don't
need that behaviour for asserts in this case.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  4 +++-
 drivers/gpu/drm/i915/intel_tv.c | 18 +++++-------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0917fcbd618d..fadd0a285efa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4762,7 +4762,9 @@ enum {
 /* Enables the TV encoder */
 # define TV_ENC_ENABLE			(1 << 31)
 /* Sources the TV encoder input from pipe B instead of A. */
-# define TV_ENC_PIPEB_SELECT		(1 << 30)
+# define TV_ENC_PIPE_SEL(pipe)		((pipe) << 30)
+# define TV_ENC_PIPE_SEL_MASK		(1 << 30)
+# define TV_ENC_PIPE_SEL_SHIFT		30
 /* Outputs composite video (DAC A only) */
 # define TV_ENC_OUTPUT_COMPOSITE	(0 << 28)
 /* Outputs SVideo video (DAC B/C) */
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 885fc3809f7f..24caf340a7a5 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -798,16 +798,12 @@ static struct intel_tv *intel_attached_tv(struct drm_connector *connector)
 static bool
 intel_tv_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 tmp = I915_READ(TV_CTL);
 
-	if (!(tmp & TV_ENC_ENABLE))
-		return false;
+	*pipe = (tmp & TV_ENC_PIPE_SEL_MASK) >> TV_ENC_PIPE_SEL_SHIFT;
 
-	*pipe = PORT_TO_PIPE(tmp);
-
-	return true;
+	return tmp & TV_ENC_ENABLE;
 }
 
 static void
@@ -1024,8 +1020,7 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder,
 		break;
 	}
 
-	if (intel_crtc->pipe == 1)
-		tv_ctl |= TV_ENC_PIPEB_SELECT;
+	tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe);
 	tv_ctl |= tv_mode->oversample;
 
 	if (tv_mode->progressive)
@@ -1151,10 +1146,7 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 	/* Poll for TV detection */
 	tv_ctl &= ~(TV_ENC_ENABLE | TV_TEST_MODE_MASK);
 	tv_ctl |= TV_TEST_MODE_MONITOR_DETECT;
-	if (intel_crtc->pipe == 1)
-		tv_ctl |= TV_ENC_PIPEB_SELECT;
-	else
-		tv_ctl &= ~TV_ENC_PIPEB_SELECT;
+	tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe);
 
 	tv_dac &= ~(TVDAC_SENSE_MASK | DAC_A_MASK | DAC_B_MASK | DAC_C_MASK);
 	tv_dac |= (TVDAC_STATE_CHG_EN |
-- 
2.13.6

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

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

* [PATCH 05/14] drm/i915: Clean up DVO pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (3 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 04/14] drm/i915: Clean up TV " Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  7:00   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 06/14] drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too Ville Syrjala
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Parametrize the DVO pipe select bits.

For consistency with the new way of doing things, let's read out the
pipe select bits even when the port is disable, even though we don't
need that behaviour for asserts in this case.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 +++-
 drivers/gpu/drm/i915/intel_dvo.c | 13 ++++---------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fadd0a285efa..d7dc03bd0b4f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4423,7 +4423,9 @@ enum {
 #define _DVOC			0x61160
 #define DVOC			_MMIO(_DVOC)
 #define   DVO_ENABLE			(1 << 31)
-#define   DVO_PIPE_B_SELECT		(1 << 30)
+#define   DVO_PIPE_SEL(pipe)		((pipe) << 30)
+#define   DVO_PIPE_SEL_MASK		(1 << 30)
+#define   DVO_PIPE_SEL_SHIFT		30
 #define   DVO_PIPE_STALL_UNUSED		(0 << 28)
 #define   DVO_PIPE_STALL		(1 << 28)
 #define   DVO_PIPE_STALL_TV		(2 << 28)
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index eb0c559b2715..a86f0398570f 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -137,19 +137,15 @@ static bool intel_dvo_connector_get_hw_state(struct intel_connector *connector)
 static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
 				   enum pipe *pipe)
 {
-	struct drm_device *dev = encoder->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
 	u32 tmp;
 
 	tmp = I915_READ(intel_dvo->dev.dvo_reg);
 
-	if (!(tmp & DVO_ENABLE))
-		return false;
-
-	*pipe = PORT_TO_PIPE(tmp);
+	*pipe = (tmp & DVO_PIPE_SEL_MASK) >> DVO_PIPE_SEL_SHIFT;
 
-	return true;
+	return tmp & DVO_ENABLE;
 }
 
 static void intel_dvo_get_config(struct intel_encoder *encoder,
@@ -276,8 +272,7 @@ static void intel_dvo_pre_enable(struct intel_encoder *encoder,
 	dvo_val |= DVO_DATA_ORDER_FP | DVO_BORDER_ENABLE |
 		   DVO_BLANK_ACTIVE_HIGH;
 
-	if (pipe == 1)
-		dvo_val |= DVO_PIPE_B_SELECT;
+	dvo_val |= DVO_PIPE_SEL(pipe);
 	dvo_val |= DVO_PIPE_STALL;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 		dvo_val |= DVO_HSYNC_ACTIVE_HIGH;
-- 
2.13.6

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

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

* [PATCH 06/14] drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (4 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 05/14] drm/i915: Clean up DVO " Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02  9:55 ` [PATCH 07/14] drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+ Ville Syrjala
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use intel_ddi_dp_voltage_max() for HSW/BDW too instead of letting these
fall through the if ladder in a weird way. This function will look at
the actual buf trans tables we have for HSW/BDW to determine the max
vswing level.

It looks to me like the current code leads HSW port A down the IVB port
A path, HSW port B+ and BDW fall through to the very end. Both cases do
result in the correct max vswing level 2, but it's very hard to see that
from the code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aba2f45819d8..f1fab5555d5d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3206,12 +3206,12 @@ uint8_t
 intel_dp_voltage_max(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
-	enum port port = dp_to_dig_port(intel_dp)->base.port;
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	enum port port = encoder->port;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	if (HAS_DDI(dev_priv))
 		return intel_ddi_dp_voltage_max(encoder);
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
 	else if (IS_GEN7(dev_priv) && port == PORT_A)
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
-- 
2.13.6

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

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

* [PATCH 07/14] drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (5 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 06/14] drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02  9:55 ` [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP Ville Syrjala
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

All DDI platforms support the full set of preemph settings for each
supported vswing, so let's use the same code for them. We'll also move
the code into intel_ddi.c so that it sits closer to the actual buf trans
tables.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 30 ++++--------------------------
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8ca376aca8bd..a56a4db6dc38 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1931,6 +1931,26 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 		DP_TRAIN_VOLTAGE_SWING_MASK;
 }
 
+/*
+ * We assume that the full set of pre-emphasis values can be
+ * used on all DDI platforms. Should that change we need to
+ * rethink this code.
+ */
+u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder, u8 voltage_swing)
+{
+	switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+	case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+		return DP_TRAIN_PRE_EMPH_LEVEL_3;
+	case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+		return DP_TRAIN_PRE_EMPH_LEVEL_2;
+	case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+		return DP_TRAIN_PRE_EMPH_LEVEL_1;
+	case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
+	default:
+		return DP_TRAIN_PRE_EMPH_LEVEL_0;
+	}
+}
+
 static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
 				   int level, enum intel_output_type type)
 {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1fab5555d5d..642ae298df07 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3225,33 +3225,11 @@ uint8_t
 intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
-	enum port port = dp_to_dig_port(intel_dp)->base.port;
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	enum port port = encoder->port;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_3;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
-	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
-		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
-			return DP_TRAIN_PRE_EMPH_LEVEL_3;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
-			return DP_TRAIN_PRE_EMPH_LEVEL_2;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
-			return DP_TRAIN_PRE_EMPH_LEVEL_1;
-		case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
-		default:
-			return DP_TRAIN_PRE_EMPH_LEVEL_0;
-		}
+	if (HAS_DDI(dev_priv)) {
+		return intel_ddi_dp_pre_emphasis_max(encoder, voltage_swing);
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
 		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 09a1b968ac9c..d783ecfef46d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1388,6 +1388,8 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
 u32 bxt_signal_levels(struct intel_dp *intel_dp);
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
 u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder);
+u8 intel_ddi_dp_pre_emphasis_max(struct intel_encoder *encoder,
+				 u8 voltage_swing);
 int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder,
 				     bool enable);
 
-- 
2.13.6

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

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

* [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (6 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 07/14] drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+ Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  7:11   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code Ville Syrjala
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Almost all of the GEN7 checks in the DP code are actually looking for
IVB. HSW doesn't even take these codepaths, and VLV is excluded on
account of not having port A. So let's change the checks to IS_IVB to
make the code less confusing.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 642ae298df07..2a82eccffe54 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1970,7 +1970,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 
 	/* Split out the IBX/CPU vs CPT settings */
 
-	if (IS_GEN7(dev_priv) && port == PORT_A) {
+	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
 			intel_dp->DP |= DP_SYNC_HS_HIGH;
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
@@ -2650,7 +2650,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	if (!(tmp & DP_PORT_EN))
 		goto out;
 
-	if (IS_GEN7(dev_priv) && port == PORT_A) {
+	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
 		*pipe = PORT_TO_PIPE_CPT(tmp);
 	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
 		enum pipe p;
@@ -2887,7 +2887,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
 		}
 		I915_WRITE(DP_TP_CTL(port), temp);
 
-	} else if ((IS_GEN7(dev_priv) && port == PORT_A) ||
+	} else if ((IS_IVYBRIDGE(dev_priv) && port == PORT_A) ||
 		   (HAS_PCH_CPT(dev_priv) && port != PORT_A)) {
 		*DP &= ~DP_LINK_TRAIN_MASK_CPT;
 
@@ -3213,7 +3213,7 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
 		return intel_ddi_dp_voltage_max(encoder);
 	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-	else if (IS_GEN7(dev_priv) && port == PORT_A)
+	else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A)
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
 	else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
 		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
@@ -3242,7 +3242,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
 		default:
 			return DP_TRAIN_PRE_EMPH_LEVEL_0;
 		}
-	} else if (IS_GEN7(dev_priv) && port == PORT_A) {
+	} else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
 		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
 		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
 			return DP_TRAIN_PRE_EMPH_LEVEL_2;
@@ -3551,7 +3551,7 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp)
 		signal_levels = chv_signal_levels(intel_dp);
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		signal_levels = vlv_signal_levels(intel_dp);
-	} else if (IS_GEN7(dev_priv) && port == PORT_A) {
+	} else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
 		signal_levels = gen7_edp_signal_levels(train_set);
 		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_IVB;
 	} else if (IS_GEN6(dev_priv) && port == PORT_A) {
@@ -3641,7 +3641,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
-	if ((IS_GEN7(dev_priv) && port == PORT_A) ||
+	if ((IS_IVYBRIDGE(dev_priv) && port == PORT_A) ||
 	    (HAS_PCH_CPT(dev_priv) && port != PORT_A)) {
 		DP &= ~DP_LINK_TRAIN_MASK_CPT;
 		DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
-- 
2.13.6

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

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

* [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (7 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  7:19   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL Ville Syrjala
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The ddi code no longer uses intel_ddi_get_crtc_new_encoder(). Move it
elsewhere where we have some users left.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 29 -----------------------------
 drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 3 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a56a4db6dc38..ab084170ea38 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1065,35 +1065,6 @@ intel_ddi_get_crtc_encoder(struct intel_crtc *crtc)
 	return ret;
 }
 
-/* Finds the only possible encoder associated with the given CRTC. */
-struct intel_encoder *
-intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct intel_encoder *ret = NULL;
-	struct drm_atomic_state *state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int num_encoders = 0;
-	int i;
-
-	state = crtc_state->base.state;
-
-	for_each_new_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc_state->base.crtc)
-			continue;
-
-		ret = to_intel_encoder(connector_state->best_encoder);
-		num_encoders++;
-	}
-
-	WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n", num_encoders,
-	     pipe_name(crtc->pipe));
-
-	BUG_ON(ret == NULL);
-	return ret;
-}
-
 #define LC_FREQ 2700
 
 static int hsw_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 26f8f1e741f7..239059493243 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4491,6 +4491,35 @@ static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
 	}
 }
 
+/*
+ * Finds the encoder associated with the given CRTC. This can only be
+ * used when we know that the CRTC isn't feeding multiple encoders!
+ */
+static struct intel_encoder *
+intel_get_crtc_new_encoder(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_atomic_state *state = crtc_state->base.state;
+	const struct drm_connector_state *connector_state;
+	const struct drm_connector *connector;
+	struct intel_encoder *encoder = NULL;
+	int num_encoders = 0;
+	int i;
+
+	for_each_new_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc != &crtc->base)
+			continue;
+
+		encoder = to_intel_encoder(connector_state->best_encoder);
+		num_encoders++;
+	}
+
+	WARN(num_encoders != 1, "%d encoders for pipe %c\n",
+	     num_encoders, pipe_name(crtc->pipe));
+
+	return encoder;
+}
+
 /* Return which DP Port should be selected for Transcoder DP control */
 static enum port
 intel_trans_dp_port_sel(struct intel_crtc *crtc)
@@ -8953,7 +8982,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 {
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
 		struct intel_encoder *encoder =
-			intel_ddi_get_crtc_new_encoder(crtc_state);
+			intel_get_crtc_new_encoder(crtc_state);
 
 		if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
 			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d783ecfef46d..9f6e68d192b6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1373,8 +1373,6 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
 				       enum transcoder cpu_transcoder);
 void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state);
 void intel_ddi_disable_pipe_clock(const  struct intel_crtc_state *crtc_state);
-struct intel_encoder *
-intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state);
 void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
-- 
2.13.6

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

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

* [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (8 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-20  8:36   ` Jani Nikula
  2018-03-02  9:55 ` [PATCH 11/14] drm/i915: Nuke intel_trans_dp_port_sel() Ville Syrjala
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Parametrize the TRANS_DP_PORT_SEL macros.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  8 +++-----
 drivers/gpu/drm/i915/intel_display.c | 23 +++++++----------------
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d7dc03bd0b4f..f1460fd27881 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7914,11 +7914,9 @@ enum {
 #define _TRANS_DP_CTL_C		0xe2300
 #define TRANS_DP_CTL(pipe)	_MMIO_PIPE(pipe, _TRANS_DP_CTL_A, _TRANS_DP_CTL_B)
 #define  TRANS_DP_OUTPUT_ENABLE	(1<<31)
-#define  TRANS_DP_PORT_SEL_B	(0<<29)
-#define  TRANS_DP_PORT_SEL_C	(1<<29)
-#define  TRANS_DP_PORT_SEL_D	(2<<29)
-#define  TRANS_DP_PORT_SEL_NONE	(3<<29)
-#define  TRANS_DP_PORT_SEL_MASK	(3<<29)
+#define  TRANS_DP_PORT_SEL(port)	(((port) - PORT_B) << 29)
+#define  TRANS_DP_PORT_SEL_NONE		(3 << 29)
+#define  TRANS_DP_PORT_SEL_MASK		(3 << 29)
 #define  TRANS_DP_PIPE_TO_PORT(val)	((((val) & TRANS_DP_PORT_SEL_MASK) >> 29) + PORT_B)
 #define  TRANS_DP_AUDIO_ONLY	(1<<26)
 #define  TRANS_DP_ENH_FRAMING	(1<<18)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 239059493243..e8bedf09d892 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1318,9 +1318,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 {
 	enum pipe port_pipe;
 
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL(PORT_B));
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL(PORT_C));
+	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL(PORT_D));
 
 	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
 			port_pipe == pipe,
@@ -4603,6 +4603,8 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
 			&crtc_state->base.adjusted_mode;
 		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) >> 5;
 		i915_reg_t reg = TRANS_DP_CTL(pipe);
+		enum port port;
+
 		temp = I915_READ(reg);
 		temp &= ~(TRANS_DP_PORT_SEL_MASK |
 			  TRANS_DP_SYNC_MASK |
@@ -4615,19 +4617,8 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 			temp |= TRANS_DP_VSYNC_ACTIVE_HIGH;
 
-		switch (intel_trans_dp_port_sel(crtc)) {
-		case PORT_B:
-			temp |= TRANS_DP_PORT_SEL_B;
-			break;
-		case PORT_C:
-			temp |= TRANS_DP_PORT_SEL_C;
-			break;
-		case PORT_D:
-			temp |= TRANS_DP_PORT_SEL_D;
-			break;
-		default:
-			BUG();
-		}
+		port = intel_trans_dp_port_sel(crtc);
+		temp |= TRANS_DP_PORT_SEL(port);
 
 		I915_WRITE(reg, temp);
 	}
-- 
2.13.6

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

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

* [PATCH 11/14] drm/i915: Nuke intel_trans_dp_port_sel()
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (9 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02  9:55 ` [PATCH 12/14] drm/i915: Clean up DP pipe select bits Ville Syrjala
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

for_each_encoder_on_crtc() is legacy and shouldn't be used by atomic
drivers. Let's throw out intel_trans_dp_port_sel() and replace it
with intel_get_crtc_new_encoder() which looks the atomic state instead.

Since we now have to call intel_get_crtc_new_encoder() during the commit
phase we'll need to plumb in the top level atomic state. The
crtc_state->state pointers are no longer valid at that point.

We'll also parametrize TRANS_DP_PORT_SEL() while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++-----------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e8bedf09d892..52bcb13c1a00 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4496,17 +4496,17 @@ static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
  * used when we know that the CRTC isn't feeding multiple encoders!
  */
 static struct intel_encoder *
-intel_get_crtc_new_encoder(const struct intel_crtc_state *crtc_state)
+intel_get_crtc_new_encoder(const struct intel_atomic_state *state,
+			   const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	const struct drm_atomic_state *state = crtc_state->base.state;
 	const struct drm_connector_state *connector_state;
 	const struct drm_connector *connector;
 	struct intel_encoder *encoder = NULL;
 	int num_encoders = 0;
 	int i;
 
-	for_each_new_connector_in_state(state, connector, connector_state, i) {
+	for_each_new_connector_in_state(&state->base, connector, connector_state, i) {
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
@@ -4520,22 +4520,6 @@ intel_get_crtc_new_encoder(const struct intel_crtc_state *crtc_state)
 	return encoder;
 }
 
-/* Return which DP Port should be selected for Transcoder DP control */
-static enum port
-intel_trans_dp_port_sel(struct intel_crtc *crtc)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_encoder *encoder;
-
-	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
-		if (encoder->type == INTEL_OUTPUT_DP ||
-		    encoder->type == INTEL_OUTPUT_EDP)
-			return encoder->port;
-	}
-
-	return -1;
-}
-
 /*
  * Enable PCH resources required for PCH ports:
  *   - PCH PLLs
@@ -4544,7 +4528,8 @@ intel_trans_dp_port_sel(struct intel_crtc *crtc)
  *   - DP transcoding bits
  *   - transcoder
  */
-static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
+static void ironlake_pch_enable(const struct intel_atomic_state *state,
+				const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
@@ -4617,7 +4602,7 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 			temp |= TRANS_DP_VSYNC_ACTIVE_HIGH;
 
-		port = intel_trans_dp_port_sel(crtc);
+		port = intel_get_crtc_new_encoder(state, crtc_state)->port;
 		temp |= TRANS_DP_PORT_SEL(port);
 
 		I915_WRITE(reg, temp);
@@ -4626,7 +4611,8 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
 	ironlake_enable_pch_transcoder(dev_priv, pipe);
 }
 
-static void lpt_pch_enable(const struct intel_crtc_state *crtc_state)
+static void lpt_pch_enable(const struct intel_atomic_state *state,
+			   const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -5384,7 +5370,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_enable_pipe(pipe_config);
 
 	if (intel_crtc->config->has_pch_encoder)
-		ironlake_pch_enable(pipe_config);
+		ironlake_pch_enable(old_intel_state, pipe_config);
 
 	assert_vblank_disabled(crtc);
 	drm_crtc_vblank_on(crtc);
@@ -5520,7 +5506,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_enable_pipe(pipe_config);
 
 	if (intel_crtc->config->has_pch_encoder)
-		lpt_pch_enable(pipe_config);
+		lpt_pch_enable(old_intel_state, pipe_config);
 
 	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST))
 		intel_ddi_set_vc_payload_alloc(pipe_config, true);
@@ -8971,9 +8957,12 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+
 	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
 		struct intel_encoder *encoder =
-			intel_get_crtc_new_encoder(crtc_state);
+			intel_get_crtc_new_encoder(state, crtc_state);
 
 		if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
 			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
-- 
2.13.6

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

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

* [PATCH 12/14] drm/i915: Clean up DP pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (10 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 11/14] drm/i915: Nuke intel_trans_dp_port_sel() Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02 18:08   ` [PATCH v2 " Ville Syrjala
  2018-03-02  9:55 ` [PATCH 13/14] drm/i915: Allow eDP on port C in theory Ville Syrjala
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the DP pipe select bits. To make the whole situation a bit
less ugly we'll start to share the same code between .get_hw_state(),
the port state asserts, and the VLV power sequencer code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  24 +++-----
 drivers/gpu/drm/i915/intel_display.c |  46 +++++----------
 drivers/gpu/drm/i915/intel_dp.c      | 106 +++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 4 files changed, 85 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f1460fd27881..2c1e33b63cf3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5248,10 +5248,15 @@ enum {
 #define CHV_DP_D		_MMIO(VLV_DISPLAY_BASE + 0x64300)
 
 #define   DP_PORT_EN			(1 << 31)
-#define   DP_PIPEB_SELECT		(1 << 30)
-#define   DP_PIPE_MASK			(1 << 30)
-#define   DP_PIPE_SELECT_CHV(pipe)	((pipe) << 16)
-#define   DP_PIPE_MASK_CHV		(3 << 16)
+#define   DP_PIPE_SEL(pipe)		((pipe) << 30)
+#define   DP_PIPE_SEL_MASK		(1 << 30)
+#define   DP_PIPE_SEL_SHIFT		30
+#define   DP_PIPE_SEL_IVB(pipe)		((pipe) << 29)
+#define   DP_PIPE_SEL_MASK_IVB		(3 << 29)
+#define   DP_PIPE_SEL_SHIFT_IVB		29
+#define   DP_PIPE_SEL_CHV(pipe)		((pipe) << 16)
+#define   DP_PIPE_SEL_MASK_CHV		(3 << 16)
+#define   DP_PIPE_SEL_SHIFT_CHV		16
 
 /* Link training mode - select a suitable mode for each stage */
 #define   DP_LINK_TRAIN_PAT_1		(0 << 28)
@@ -7899,16 +7904,6 @@ enum {
 #define PCH_DP_AUX_CH_DATA(aux_ch, i)	_MMIO(_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
 
 /* CPT */
-#define  PORT_TRANS_A_SEL_CPT	0
-#define  PORT_TRANS_B_SEL_CPT	(1<<29)
-#define  PORT_TRANS_C_SEL_CPT	(2<<29)
-#define  PORT_TRANS_SEL_MASK	(3<<29)
-#define  PORT_TRANS_SEL_CPT(pipe)	((pipe) << 29)
-#define  PORT_TO_PIPE(val)	(((val) & (1<<30)) >> 30)
-#define  PORT_TO_PIPE_CPT(val)	(((val) & PORT_TRANS_SEL_MASK) >> 29)
-#define  SDVO_PORT_TO_PIPE_CHV(val)	(((val) & (3<<24)) >> 24)
-#define  DP_PORT_TO_PIPE_CHV(val)	(((val) & (3<<16)) >> 16)
-
 #define _TRANS_DP_CTL_A		0xe0300
 #define _TRANS_DP_CTL_B		0xe1300
 #define _TRANS_DP_CTL_C		0xe2300
@@ -7917,7 +7912,6 @@ enum {
 #define  TRANS_DP_PORT_SEL(port)	(((port) - PORT_B) << 29)
 #define  TRANS_DP_PORT_SEL_NONE		(3 << 29)
 #define  TRANS_DP_PORT_SEL_MASK		(3 << 29)
-#define  TRANS_DP_PIPE_TO_PORT(val)	((((val) & TRANS_DP_PORT_SEL_MASK) >> 29) + PORT_B)
 #define  TRANS_DP_AUDIO_ONLY	(1<<26)
 #define  TRANS_DP_ENH_FRAMING	(1<<18)
 #define  TRANS_DP_8BPC		(0<<9)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 52bcb13c1a00..cb056283830c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1261,38 +1261,22 @@ void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
 	     pipe_name(pipe));
 }
 
-static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
-			    enum pipe pipe, u32 port_sel, u32 val)
+static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
+				   enum pipe pipe, enum port port,
+				   i915_reg_t dp_reg)
 {
-	if ((val & DP_PORT_EN) == 0)
-		return false;
+	enum pipe port_pipe;
+	bool state;
 
-	if (HAS_PCH_CPT(dev_priv)) {
-		u32 trans_dp_ctl = I915_READ(TRANS_DP_CTL(pipe));
-		if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
-			return false;
-	} else if (IS_CHERRYVIEW(dev_priv)) {
-		if ((val & DP_PIPE_MASK_CHV) != DP_PIPE_SELECT_CHV(pipe))
-			return false;
-	} else {
-		if ((val & DP_PIPE_MASK) != (pipe << 30))
-			return false;
-	}
-	return true;
-}
+	state = intel_dp_port_enabled(dev_priv, dp_reg, port, &port_pipe);
 
-static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe, i915_reg_t reg,
-				   u32 port_sel)
-{
-	u32 val = I915_READ(reg);
-	I915_STATE_WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val),
-	     "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
-	     i915_mmio_reg_offset(reg), pipe_name(pipe));
+	I915_STATE_WARN(state && port_pipe == pipe,
+			"PCH DP %c enabled on transcoder %c, should be disabled\n",
+			port_name(port), pipe_name(pipe));
 
-	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & DP_PORT_EN) == 0
-	     && (val & DP_PIPEB_SELECT),
-	     "IBX PCH dp port still using transcoder B\n");
+	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B,
+			"IBX PCH DP %c still using transcoder B\n",
+			port_name(port));
 }
 
 static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
@@ -1318,9 +1302,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 {
 	enum pipe port_pipe;
 
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL(PORT_B));
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL(PORT_C));
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL(PORT_D));
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_B, PCH_DP_B);
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_C, PCH_DP_C);
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_D, PCH_DP_D);
 
 	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
 			port_pipe == pipe,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2a82eccffe54..3732a4e22ef2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -532,9 +532,9 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
 	DP |= DP_LINK_TRAIN_PAT_1;
 
 	if (IS_CHERRYVIEW(dev_priv))
-		DP |= DP_PIPE_SELECT_CHV(pipe);
-	else if (pipe == PIPE_B)
-		DP |= DP_PIPEB_SELECT;
+		DP |= DP_PIPE_SEL_CHV(pipe);
+	else
+		DP |= DP_PIPE_SEL(pipe);
 
 	pll_enabled = I915_READ(DPLL(pipe)) & DPLL_VCO_ENABLE;
 
@@ -1980,7 +1980,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
-		intel_dp->DP |= crtc->pipe << 29;
+		intel_dp->DP |= DP_PIPE_SEL_IVB(crtc->pipe);
 	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
 		u32 trans_dp;
 
@@ -2006,9 +2006,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
 		if (IS_CHERRYVIEW(dev_priv))
-			intel_dp->DP |= DP_PIPE_SELECT_CHV(crtc->pipe);
-		else if (crtc->pipe == PIPE_B)
-			intel_dp->DP |= DP_PIPEB_SELECT;
+			intel_dp->DP |= DP_PIPE_SEL_CHV(crtc->pipe);
+		else
+			intel_dp->DP |= DP_PIPE_SEL(crtc->pipe);
 	}
 }
 
@@ -2630,52 +2630,61 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
 			      mode == DRM_MODE_DPMS_ON ? "enable" : "disable");
 }
 
-static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
-				  enum pipe *pipe)
+static enum pipe cpt_dp_port_pipe(struct drm_i915_private *dev_priv,
+				  enum port port)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = encoder->port;
-	u32 tmp;
-	bool ret;
-
-	if (!intel_display_power_get_if_enabled(dev_priv,
-						encoder->power_domain))
-		return false;
+	enum pipe pipe;
 
-	ret = false;
+	for_each_pipe(dev_priv, pipe) {
+		u32 val = I915_READ(TRANS_DP_CTL(pipe));
 
-	tmp = I915_READ(intel_dp->output_reg);
+		if ((val & TRANS_DP_PORT_SEL_MASK) == TRANS_DP_PORT_SEL(port))
+			return pipe;
+	}
 
-	if (!(tmp & DP_PORT_EN))
-		goto out;
+	DRM_DEBUG_KMS("No pipe for DP port %c found\n", port_name(port));
 
-	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
-		enum pipe p;
+	return INVALID_PIPE;
+}
 
-		for_each_pipe(dev_priv, p) {
-			u32 trans_dp = I915_READ(TRANS_DP_CTL(p));
-			if (TRANS_DP_PIPE_TO_PORT(trans_dp) == port) {
-				*pipe = p;
-				ret = true;
+bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
+			   i915_reg_t dp_reg, enum port port,
+			   enum pipe *pipe)
+{
+	u32 val;
 
-				goto out;
-			}
-		}
+	val = I915_READ(dp_reg);
 
-		DRM_DEBUG_KMS("No pipe for dp port 0x%x found\n",
-			      i915_mmio_reg_offset(intel_dp->output_reg));
+	/* asserts want to know the pipe even if the port is disabled */
+	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
+		*pipe = (val & DP_PIPE_SEL_MASK_IVB) >> DP_PIPE_SEL_SHIFT_IVB;
+	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
+		*pipe = cpt_dp_port_pipe(dev_priv, port);
+		if (*pipe == INVALID_PIPE)
+			return false;
 	} else if (IS_CHERRYVIEW(dev_priv)) {
-		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
+		*pipe = (val & DP_PIPE_SEL_MASK_CHV) >> DP_PIPE_SEL_SHIFT_CHV;
 	} else {
-		*pipe = PORT_TO_PIPE(tmp);
+		*pipe = (val & DP_PIPE_SEL_MASK) >> DP_PIPE_SEL_SHIFT;
 	}
 
-	ret = true;
+	return val & DP_PORT_EN;
+}
+
+static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
+				  enum pipe *pipe)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	bool ret;
+
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
+		return false;
+
+	ret = intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
+				    encoder->port, pipe);
 
-out:
 	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
@@ -3673,8 +3682,9 @@ intel_dp_link_down(struct intel_encoder *encoder,
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 		/* always enable with pattern 1 (as per spec) */
-		DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
-		DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
+		DP &= ~(DP_PIPE_SEL_MASK | DP_LINK_TRAIN_MASK);
+		DP |= DP_PORT_EN | DP_PIPE_SEL(PIPE_A) |
+			DP_LINK_TRAIN_PAT_1;
 		I915_WRITE(intel_dp->output_reg, DP);
 		POSTING_READ(intel_dp->output_reg);
 
@@ -5248,14 +5258,14 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 static enum pipe vlv_active_pipe(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	enum pipe pipe;
 
-	if ((intel_dp->DP & DP_PORT_EN) == 0)
-		return INVALID_PIPE;
+	if (intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
+				  encoder->port, &pipe))
+		return pipe;
 
-	if (IS_CHERRYVIEW(dev_priv))
-		return DP_PORT_TO_PIPE_CHV(intel_dp->DP);
-	else
-		return PORT_TO_PIPE(intel_dp->DP);
+	return INVALID_PIPE;
 }
 
 void intel_dp_encoder_reset(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9f6e68d192b6..36366665c1ed 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1616,6 +1616,9 @@ void intel_csr_ucode_suspend(struct drm_i915_private *);
 void intel_csr_ucode_resume(struct drm_i915_private *);
 
 /* intel_dp.c */
+bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
+			   i915_reg_t dp_reg, enum port port,
+			   enum pipe *pipe);
 bool intel_dp_init(struct drm_i915_private *dev_priv, i915_reg_t output_reg,
 		   enum port port);
 bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
-- 
2.13.6

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

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

* [PATCH 13/14] drm/i915: Allow eDP on port C in theory
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (11 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 12/14] drm/i915: Clean up DP pipe select bits Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02  9:55 ` [PATCH 14/14] drm/i915: Implement the missing bits of assert_panel_unlocked() Ville Syrjala
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The power sequencer has bits to allow DP C to be used for eDP.
Currently we assume this will never happen, but I guess it could
theoretically be a thing. MAke the code do the right thing in that
case, and toss in a MISSING_CASE() for any other port.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3732a4e22ef2..c7d0c21da6fe 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5687,10 +5687,20 @@ intel_dp_init_panel_power_sequencer_registers(struct intel_dp *intel_dp,
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		port_sel = PANEL_PORT_SELECT_VLV(port);
 	} else if (HAS_PCH_IBX(dev_priv) || HAS_PCH_CPT(dev_priv)) {
-		if (port == PORT_A)
+		switch (port) {
+		case PORT_A:
 			port_sel = PANEL_PORT_SELECT_DPA;
-		else
+			break;
+		case PORT_C:
+			port_sel = PANEL_PORT_SELECT_DPC;
+			break;
+		case PORT_D:
 			port_sel = PANEL_PORT_SELECT_DPD;
+			break;
+		default:
+			MISSING_CASE(port);
+			break;
+		}
 	}
 
 	pp_on |= port_sel;
-- 
2.13.6

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

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

* [PATCH 14/14] drm/i915: Implement the missing bits of assert_panel_unlocked()
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (12 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 13/14] drm/i915: Allow eDP on port C in theory Ville Syrjala
@ 2018-03-02  9:55 ` Ville Syrjala
  2018-03-02 11:02 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits Patchwork
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02  9:55 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add the missing eDP port handling into assert_panel_unlocked(). We now
have intel_dp_port_enabled() which makes this trivial.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb056283830c..02fd00634fe7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1171,10 +1171,23 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 		pp_reg = PP_CONTROL(0);
 		port_sel = I915_READ(PP_ON_DELAYS(0)) & PANEL_PORT_SELECT_MASK;
 
-		if (port_sel == PANEL_PORT_SELECT_LVDS) {
+		switch (port_sel) {
+		case PANEL_PORT_SELECT_LVDS:
 			intel_lvds_port_enabled(dev_priv, PCH_LVDS, &panel_pipe);
+			break;
+		case PANEL_PORT_SELECT_DPA:
+			intel_dp_port_enabled(dev_priv, DP_A, PORT_A, &panel_pipe);
+			break;
+		case PANEL_PORT_SELECT_DPC:
+			intel_dp_port_enabled(dev_priv, PCH_DP_C, PORT_C, &panel_pipe);
+			break;
+		case PANEL_PORT_SELECT_DPD:
+			intel_dp_port_enabled(dev_priv, PCH_DP_D, PORT_D, &panel_pipe);
+			break;
+		default:
+			MISSING_CASE(port_sel);
+			break;
 		}
-		/* XXX: else fix for eDP */
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		/* presumably write lock depends on pipe, not port select */
 		pp_reg = PP_CONTROL(pipe);
-- 
2.13.6

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (13 preceding siblings ...)
  2018-03-02  9:55 ` [PATCH 14/14] drm/i915: Implement the missing bits of assert_panel_unlocked() Ville Syrjala
@ 2018-03-02 11:02 ` Patchwork
  2018-03-02 13:09 ` Patchwork
  2018-03-02 18:40 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits (rev2) Patchwork
  16 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-03-02 11:02 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Clean up the port pipe select bits
URL   : https://patchwork.freedesktop.org/series/39259/
State : failure

== Summary ==

Series 39259v1 drm/i915: Clean up the port pipe select bits
https://patchwork.freedesktop.org/api/1.0/series/39259/revisions/1/mbox/

---- Possible new issues:

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-snb-2520m)

---- Known issues:

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6700k2) fdo#103191
Test prime_vgem:
        Subgroup basic-fence-flip:
                fail       -> PASS       (fi-byt-n2820) fdo#104008

fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:416s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:370s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:479s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:476s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:465s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:454s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:391s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:415s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:288s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:391s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:407s
fi-ivb-3520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:411s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:449s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:492s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:492s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:421s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:500s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:518s
fi-skl-6700k2    total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:485s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:474s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:405s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:388s
Blacklisted hosts:
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:494s

b2e10fd5e8b2cd72b0e1eba46c1221dc3d4b70bc drm-tip: 2018y-03m-02d-09h-36m-59s UTC integration manifest
a7f316764d99 drm/i915: Implement the missing bits of assert_panel_unlocked()
4f7d9bb94974 drm/i915: Allow eDP on port C in theory
a89c6281509b drm/i915: Clean up DP pipe select bits
9a0df9a20aa0 drm/i915: Nuke intel_trans_dp_port_sel()
c220a8c7df1b drm/i915: Parametrize TRANS_DP_PORT_SEL
b98263dcfc35 drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
99dedddd9590 drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP
f17399fdd4ff drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+
5bb0a7d136c6 drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too
bc1529c6bff2 drm/i915: Clean up DVO pipe select bits
3d9c866f3898 drm/i915: Clean up TV pipe select bits
fe22569aa14d drm/i915: Clean up SDVO pipe select bits
da99394e9b3f drm/i915: Clean up LVDS pipe select bits
1ff16b8483f3 drm/i915: Clean up ADPA pipe select bits

== Logs ==

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (14 preceding siblings ...)
  2018-03-02 11:02 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits Patchwork
@ 2018-03-02 13:09 ` Patchwork
  2018-03-02 18:40 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits (rev2) Patchwork
  16 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-03-02 13:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Clean up the port pipe select bits
URL   : https://patchwork.freedesktop.org/series/39259/
State : failure

== Summary ==

Series 39259v1 drm/i915: Clean up the port pipe select bits
https://patchwork.freedesktop.org/api/1.0/series/39259/revisions/1/mbox/

---- Possible new issues:

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-snb-2520m)

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:414s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:422s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:370s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:486s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:475s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:463s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:453s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:397s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:491s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:408s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:291s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:505s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:388s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:406s
fi-ivb-3520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:450s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:489s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:491s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:422s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:497s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:517s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:486s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:407s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
fi-snb-2520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:387s

519e1b37fcc70054cc3064b811bdf68fbd4ff699 drm-tip: 2018y-03m-02d-11h-56m-58s UTC integration manifest
950164172c86 drm/i915: Implement the missing bits of assert_panel_unlocked()
314966553211 drm/i915: Allow eDP on port C in theory
34744df0f6ca drm/i915: Clean up DP pipe select bits
a760b515dcb8 drm/i915: Nuke intel_trans_dp_port_sel()
cf688310aae9 drm/i915: Parametrize TRANS_DP_PORT_SEL
4897c0985bc7 drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
1c0d7dc03383 drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP
399bbd665a74 drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+
d3e8f01d950c drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too
7ab6b990997e drm/i915: Clean up DVO pipe select bits
e3d61fda6189 drm/i915: Clean up TV pipe select bits
c93cce952eca drm/i915: Clean up SDVO pipe select bits
30893edbb0e6 drm/i915: Clean up LVDS pipe select bits
450b0ac22c5c drm/i915: Clean up ADPA pipe select bits

== Logs ==

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

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

* [PATCH v2 12/14] drm/i915: Clean up DP pipe select bits
  2018-03-02  9:55 ` [PATCH 12/14] drm/i915: Clean up DP pipe select bits Ville Syrjala
@ 2018-03-02 18:08   ` Ville Syrjala
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2018-03-02 18:08 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clean up the DP pipe select bits. To make the whole situation a bit
less ugly we'll start to share the same code between .get_hw_state(),
the port state asserts, and the VLV power sequencer code.

v2: Return PIPE_A for cpt/ppt when the port isn't selected by
    any transcoder. Returning INVALID_PIPE explodes *somewhere*
    on some machines (can't immediately see where though). This
    now matches the old behaviour.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  24 +++-----
 drivers/gpu/drm/i915/intel_display.c |  46 +++++----------
 drivers/gpu/drm/i915/intel_dp.c      | 110 ++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 4 files changed, 90 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc9fc6220509..9548e0bee7db 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5248,10 +5248,15 @@ enum {
 #define CHV_DP_D		_MMIO(VLV_DISPLAY_BASE + 0x64300)
 
 #define   DP_PORT_EN			(1 << 31)
-#define   DP_PIPEB_SELECT		(1 << 30)
-#define   DP_PIPE_MASK			(1 << 30)
-#define   DP_PIPE_SELECT_CHV(pipe)	((pipe) << 16)
-#define   DP_PIPE_MASK_CHV		(3 << 16)
+#define   DP_PIPE_SEL(pipe)		((pipe) << 30)
+#define   DP_PIPE_SEL_MASK		(1 << 30)
+#define   DP_PIPE_SEL_SHIFT		30
+#define   DP_PIPE_SEL_IVB(pipe)		((pipe) << 29)
+#define   DP_PIPE_SEL_MASK_IVB		(3 << 29)
+#define   DP_PIPE_SEL_SHIFT_IVB		29
+#define   DP_PIPE_SEL_CHV(pipe)		((pipe) << 16)
+#define   DP_PIPE_SEL_MASK_CHV		(3 << 16)
+#define   DP_PIPE_SEL_SHIFT_CHV		16
 
 /* Link training mode - select a suitable mode for each stage */
 #define   DP_LINK_TRAIN_PAT_1		(0 << 28)
@@ -7921,16 +7926,6 @@ enum {
 #define PCH_DP_AUX_CH_DATA(aux_ch, i)	_MMIO(_PORT((aux_ch) - AUX_CH_B, _PCH_DPB_AUX_CH_DATA1, _PCH_DPC_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
 
 /* CPT */
-#define  PORT_TRANS_A_SEL_CPT	0
-#define  PORT_TRANS_B_SEL_CPT	(1<<29)
-#define  PORT_TRANS_C_SEL_CPT	(2<<29)
-#define  PORT_TRANS_SEL_MASK	(3<<29)
-#define  PORT_TRANS_SEL_CPT(pipe)	((pipe) << 29)
-#define  PORT_TO_PIPE(val)	(((val) & (1<<30)) >> 30)
-#define  PORT_TO_PIPE_CPT(val)	(((val) & PORT_TRANS_SEL_MASK) >> 29)
-#define  SDVO_PORT_TO_PIPE_CHV(val)	(((val) & (3<<24)) >> 24)
-#define  DP_PORT_TO_PIPE_CHV(val)	(((val) & (3<<16)) >> 16)
-
 #define _TRANS_DP_CTL_A		0xe0300
 #define _TRANS_DP_CTL_B		0xe1300
 #define _TRANS_DP_CTL_C		0xe2300
@@ -7939,7 +7934,6 @@ enum {
 #define  TRANS_DP_PORT_SEL(port)	(((port) - PORT_B) << 29)
 #define  TRANS_DP_PORT_SEL_NONE		(3 << 29)
 #define  TRANS_DP_PORT_SEL_MASK		(3 << 29)
-#define  TRANS_DP_PIPE_TO_PORT(val)	((((val) & TRANS_DP_PORT_SEL_MASK) >> 29) + PORT_B)
 #define  TRANS_DP_AUDIO_ONLY	(1<<26)
 #define  TRANS_DP_ENH_FRAMING	(1<<18)
 #define  TRANS_DP_8BPC		(0<<9)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 43b589dfe71f..dfe3a17b86d1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1261,38 +1261,22 @@ void assert_pch_transcoder_disabled(struct drm_i915_private *dev_priv,
 	     pipe_name(pipe));
 }
 
-static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
-			    enum pipe pipe, u32 port_sel, u32 val)
+static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
+				   enum pipe pipe, enum port port,
+				   i915_reg_t dp_reg)
 {
-	if ((val & DP_PORT_EN) == 0)
-		return false;
+	enum pipe port_pipe;
+	bool state;
 
-	if (HAS_PCH_CPT(dev_priv)) {
-		u32 trans_dp_ctl = I915_READ(TRANS_DP_CTL(pipe));
-		if ((trans_dp_ctl & TRANS_DP_PORT_SEL_MASK) != port_sel)
-			return false;
-	} else if (IS_CHERRYVIEW(dev_priv)) {
-		if ((val & DP_PIPE_MASK_CHV) != DP_PIPE_SELECT_CHV(pipe))
-			return false;
-	} else {
-		if ((val & DP_PIPE_MASK) != (pipe << 30))
-			return false;
-	}
-	return true;
-}
+	state = intel_dp_port_enabled(dev_priv, dp_reg, port, &port_pipe);
 
-static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
-				   enum pipe pipe, i915_reg_t reg,
-				   u32 port_sel)
-{
-	u32 val = I915_READ(reg);
-	I915_STATE_WARN(dp_pipe_enabled(dev_priv, pipe, port_sel, val),
-	     "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n",
-	     i915_mmio_reg_offset(reg), pipe_name(pipe));
+	I915_STATE_WARN(state && port_pipe == pipe,
+			"PCH DP %c enabled on transcoder %c, should be disabled\n",
+			port_name(port), pipe_name(pipe));
 
-	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & DP_PORT_EN) == 0
-	     && (val & DP_PIPEB_SELECT),
-	     "IBX PCH dp port still using transcoder B\n");
+	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B,
+			"IBX PCH DP %c still using transcoder B\n",
+			port_name(port));
 }
 
 static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
@@ -1318,9 +1302,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 {
 	enum pipe port_pipe;
 
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL(PORT_B));
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL(PORT_C));
-	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL(PORT_D));
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_B, PCH_DP_B);
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_C, PCH_DP_C);
+	assert_pch_dp_disabled(dev_priv, pipe, PORT_D, PCH_DP_D);
 
 	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
 			port_pipe == pipe,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2a82eccffe54..638a9dd05592 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -532,9 +532,9 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
 	DP |= DP_LINK_TRAIN_PAT_1;
 
 	if (IS_CHERRYVIEW(dev_priv))
-		DP |= DP_PIPE_SELECT_CHV(pipe);
-	else if (pipe == PIPE_B)
-		DP |= DP_PIPEB_SELECT;
+		DP |= DP_PIPE_SEL_CHV(pipe);
+	else
+		DP |= DP_PIPE_SEL(pipe);
 
 	pll_enabled = I915_READ(DPLL(pipe)) & DPLL_VCO_ENABLE;
 
@@ -1980,7 +1980,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 		if (drm_dp_enhanced_frame_cap(intel_dp->dpcd))
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
-		intel_dp->DP |= crtc->pipe << 29;
+		intel_dp->DP |= DP_PIPE_SEL_IVB(crtc->pipe);
 	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
 		u32 trans_dp;
 
@@ -2006,9 +2006,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
 		if (IS_CHERRYVIEW(dev_priv))
-			intel_dp->DP |= DP_PIPE_SELECT_CHV(crtc->pipe);
-		else if (crtc->pipe == PIPE_B)
-			intel_dp->DP |= DP_PIPEB_SELECT;
+			intel_dp->DP |= DP_PIPE_SEL_CHV(crtc->pipe);
+		else
+			intel_dp->DP |= DP_PIPE_SEL(crtc->pipe);
 	}
 }
 
@@ -2630,52 +2630,67 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
 			      mode == DRM_MODE_DPMS_ON ? "enable" : "disable");
 }
 
-static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
-				  enum pipe *pipe)
+static bool cpt_dp_port_selected(struct drm_i915_private *dev_priv,
+				 enum port port, enum pipe *pipe)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = encoder->port;
-	u32 tmp;
-	bool ret;
+	enum pipe p;
 
-	if (!intel_display_power_get_if_enabled(dev_priv,
-						encoder->power_domain))
-		return false;
+	for_each_pipe(dev_priv, p) {
+		u32 val = I915_READ(TRANS_DP_CTL(p));
+
+		if ((val & TRANS_DP_PORT_SEL_MASK) == TRANS_DP_PORT_SEL(port)) {
+			*pipe = p;
+			return true;
+		}
+	}
 
-	ret = false;
+	DRM_DEBUG_KMS("No pipe for DP port %c found\n", port_name(port));
 
-	tmp = I915_READ(intel_dp->output_reg);
+	/* must initialize pipe to something for the asserts */
+	*pipe = PIPE_A;
 
-	if (!(tmp & DP_PORT_EN))
-		goto out;
+	return false;
+}
 
-	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
-		*pipe = PORT_TO_PIPE_CPT(tmp);
-	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
-		enum pipe p;
+bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
+			   i915_reg_t dp_reg, enum port port,
+			   enum pipe *pipe)
+{
+	bool ret;
+	u32 val;
 
-		for_each_pipe(dev_priv, p) {
-			u32 trans_dp = I915_READ(TRANS_DP_CTL(p));
-			if (TRANS_DP_PIPE_TO_PORT(trans_dp) == port) {
-				*pipe = p;
-				ret = true;
+	val = I915_READ(dp_reg);
 
-				goto out;
-			}
-		}
+	ret = val & DP_PORT_EN;
 
-		DRM_DEBUG_KMS("No pipe for dp port 0x%x found\n",
-			      i915_mmio_reg_offset(intel_dp->output_reg));
+	/* asserts want to know the pipe even if the port is disabled */
+	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
+		*pipe = (val & DP_PIPE_SEL_MASK_IVB) >> DP_PIPE_SEL_SHIFT_IVB;
+	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
+		ret &= cpt_dp_port_selected(dev_priv, port, pipe);
 	} else if (IS_CHERRYVIEW(dev_priv)) {
-		*pipe = DP_PORT_TO_PIPE_CHV(tmp);
+		*pipe = (val & DP_PIPE_SEL_MASK_CHV) >> DP_PIPE_SEL_SHIFT_CHV;
 	} else {
-		*pipe = PORT_TO_PIPE(tmp);
+		*pipe = (val & DP_PIPE_SEL_MASK) >> DP_PIPE_SEL_SHIFT;
 	}
 
-	ret = true;
+	return ret;
+}
+
+static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
+				  enum pipe *pipe)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	bool ret;
+
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
+		return false;
+
+	ret = intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
+				    encoder->port, pipe);
 
-out:
 	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
@@ -3673,8 +3688,9 @@ intel_dp_link_down(struct intel_encoder *encoder,
 		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
 
 		/* always enable with pattern 1 (as per spec) */
-		DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
-		DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
+		DP &= ~(DP_PIPE_SEL_MASK | DP_LINK_TRAIN_MASK);
+		DP |= DP_PORT_EN | DP_PIPE_SEL(PIPE_A) |
+			DP_LINK_TRAIN_PAT_1;
 		I915_WRITE(intel_dp->output_reg, DP);
 		POSTING_READ(intel_dp->output_reg);
 
@@ -5248,14 +5264,14 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 static enum pipe vlv_active_pipe(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	enum pipe pipe;
 
-	if ((intel_dp->DP & DP_PORT_EN) == 0)
-		return INVALID_PIPE;
+	if (intel_dp_port_enabled(dev_priv, intel_dp->output_reg,
+				  encoder->port, &pipe))
+		return pipe;
 
-	if (IS_CHERRYVIEW(dev_priv))
-		return DP_PORT_TO_PIPE_CHV(intel_dp->DP);
-	else
-		return PORT_TO_PIPE(intel_dp->DP);
+	return INVALID_PIPE;
 }
 
 void intel_dp_encoder_reset(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 11c0f3785f59..fd64016476a4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1617,6 +1617,9 @@ void intel_csr_ucode_suspend(struct drm_i915_private *);
 void intel_csr_ucode_resume(struct drm_i915_private *);
 
 /* intel_dp.c */
+bool intel_dp_port_enabled(struct drm_i915_private *dev_priv,
+			   i915_reg_t dp_reg, enum port port,
+			   enum pipe *pipe);
 bool intel_dp_init(struct drm_i915_private *dev_priv, i915_reg_t output_reg,
 		   enum port port);
 bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
-- 
2.16.1

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits (rev2)
  2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
                   ` (15 preceding siblings ...)
  2018-03-02 13:09 ` Patchwork
@ 2018-03-02 18:40 ` Patchwork
  16 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-03-02 18:40 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Clean up the port pipe select bits (rev2)
URL   : https://patchwork.freedesktop.org/series/39259/
State : failure

== Summary ==

Series 39259v2 drm/i915: Clean up the port pipe select bits
https://patchwork.freedesktop.org/api/1.0/series/39259/revisions/2/mbox/

---- Possible new issues:

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-snb-2520m)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> INCOMPLETE (fi-skl-6700k2)
Test kms_chamelium:
        Subgroup common-hpd-after-suspend:
                pass       -> DMESG-WARN (fi-skl-6700k2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> FAIL       (fi-skl-6700k2)
        Subgroup basic-rte:
                pass       -> FAIL       (fi-skl-6700k2)

---- Known issues:

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6700k2) fdo#104108

fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:420s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:421s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:485s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:479s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:481s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:472s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:459s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:392s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:496s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:416s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:292s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:507s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:386s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:414s
fi-ivb-3520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:411s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:449s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:492s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:492s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:586s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:426s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:518s
fi-skl-6700k2    total:285  pass:257  dwarn:2   dfail:0   fail:2   skip:23 
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:408s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:432s
fi-snb-2520m     total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:392s

4f4e4dd52a3054b01591c3444ab1a91889c46172 drm-tip: 2018y-03m-02d-16h-28m-21s UTC integration manifest
9541206e7d05 drm/i915: Implement the missing bits of assert_panel_unlocked()
06e6449c17cb drm/i915: Allow eDP on port C in theory
2d0716ba5ea1 drm/i915: Clean up DP pipe select bits
cd255a42ae79 drm/i915: Nuke intel_trans_dp_port_sel()
4009bd6467a8 drm/i915: Parametrize TRANS_DP_PORT_SEL
7747b69cd6fd drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
88ab721b4937 drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP
4566679f9e68 drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+
690beefecf42 drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too
e4f2dca9a68c drm/i915: Clean up DVO pipe select bits
446caedf8adc drm/i915: Clean up TV pipe select bits
3eb206142cb5 drm/i915: Clean up SDVO pipe select bits
84ec7de4ae0c drm/i915: Clean up LVDS pipe select bits
4a222a76addc drm/i915: Clean up ADPA pipe select bits

== Logs ==

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

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

* Re: [PATCH 01/14] drm/i915: Clean up ADPA pipe select bits
  2018-03-02  9:54 ` [PATCH 01/14] drm/i915: Clean up ADPA " Ville Syrjala
@ 2018-03-20  6:27   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  6:27 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Clean up the ADPA pipe select bits. To make the whole situation a bit
> less ugly we'll start to share the same code between .get_hw_state()
> and the port state asserts.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 11 +++++-----
>  drivers/gpu/drm/i915/intel_crt.c     | 40 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_display.c | 24 +++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  4 files changed, 33 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95a2e51ecbb0..f573095d60c2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4199,11 +4199,12 @@ enum {
>  
>  #define   ADPA_DAC_ENABLE	(1<<31)
>  #define   ADPA_DAC_DISABLE	0
> -#define   ADPA_PIPE_SELECT_MASK	(1<<30)
> -#define   ADPA_PIPE_A_SELECT	0
> -#define   ADPA_PIPE_B_SELECT	(1<<30)
> -#define   ADPA_PIPE_SELECT(pipe) ((pipe) << 30)
> -/* CPT uses bits 29:30 for pch transcoder select */
> +#define   ADPA_PIPE_SEL_MASK		(1<<30)
> +#define   ADPA_PIPE_SEL_SHIFT		30
> +#define   ADPA_PIPE_SEL(pipe)		((pipe) << 30)
> +#define   ADPA_PIPE_SEL_MASK_CPT	(3<<29)
> +#define   ADPA_PIPE_SEL_SHIFT_CPT	29
> +#define   ADPA_PIPE_SEL_CPT(pipe)	((pipe) << 29)
>  #define   ADPA_CRT_HOTPLUG_MASK  0x03ff0000 /* bit 25-16 */
>  #define   ADPA_CRT_HOTPLUG_MONITOR_NONE  (0<<24)
>  #define   ADPA_CRT_HOTPLUG_MONITOR_MASK  (3<<24)
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 391dd69ae0a4..88889af44608 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -63,33 +63,35 @@ static struct intel_crt *intel_attached_crt(struct drm_connector *connector)
>  	return intel_encoder_to_crt(intel_attached_encoder(connector));
>  }
>  
> +bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
> +			    i915_reg_t adpa_reg, enum pipe *pipe)
> +{
> +	u32 val;
> +
> +	val = I915_READ(adpa_reg);
> +
> +	/* asserts want to know the pipe even if the port is disabled */
> +	if (HAS_PCH_CPT(dev_priv))
> +		*pipe = (val & ADPA_PIPE_SEL_MASK_CPT) >> ADPA_PIPE_SEL_SHIFT_CPT;
> +	else
> +		*pipe = (val & ADPA_PIPE_SEL_MASK) >> ADPA_PIPE_SEL_SHIFT;
> +
> +	return val & ADPA_DAC_ENABLE;
> +}
> +
>  static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
>  				   enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crt *crt = intel_encoder_to_crt(encoder);
> -	u32 tmp;
>  	bool ret;
>  
>  	if (!intel_display_power_get_if_enabled(dev_priv,
>  						encoder->power_domain))
>  		return false;
>  
> -	ret = false;
> -
> -	tmp = I915_READ(crt->adpa_reg);
> -
> -	if (!(tmp & ADPA_DAC_ENABLE))
> -		goto out;
> +	ret = intel_crt_port_enabled(dev_priv, crt->adpa_reg, pipe);
>  
> -	if (HAS_PCH_CPT(dev_priv))
> -		*pipe = PORT_TO_PIPE_CPT(tmp);
> -	else
> -		*pipe = PORT_TO_PIPE(tmp);
> -
> -	ret = true;
> -out:
>  	intel_display_power_put(dev_priv, encoder->power_domain);
>  
>  	return ret;
> @@ -168,11 +170,9 @@ static void intel_crt_set_dpms(struct intel_encoder *encoder,
>  	if (HAS_PCH_LPT(dev_priv))
>  		; /* Those bits don't exist here */
>  	else if (HAS_PCH_CPT(dev_priv))
> -		adpa |= PORT_TRANS_SEL_CPT(crtc->pipe);
> -	else if (crtc->pipe == 0)
> -		adpa |= ADPA_PIPE_A_SELECT;
> +		adpa |= ADPA_PIPE_SEL_CPT(crtc->pipe);
>  	else
> -		adpa |= ADPA_PIPE_B_SELECT;
> +		adpa |= ADPA_PIPE_SEL(crtc->pipe);
>  
>  	if (!HAS_PCH_SPLIT(dev_priv))
>  		I915_WRITE(BCLRPAT(crtc->pipe), 0);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 740a918ee578..545d89152e9b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1317,21 +1317,6 @@ static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> -static bool adpa_pipe_enabled(struct drm_i915_private *dev_priv,
> -			      enum pipe pipe, u32 val)
> -{
> -	if ((val & ADPA_DAC_ENABLE) == 0)
> -		return false;
> -	if (HAS_PCH_CPT(dev_priv)) {
> -		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
> -			return false;
> -	} else {
> -		if ((val & ADPA_PIPE_SELECT_MASK) != ADPA_PIPE_SELECT(pipe))
> -			return false;
> -	}
> -	return true;
> -}
> -
>  static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
>  				   enum pipe pipe, i915_reg_t reg,
>  				   u32 port_sel)
> @@ -1362,16 +1347,17 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
>  static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  				      enum pipe pipe)
>  {
> +	enum pipe port_pipe;
>  	u32 val;
>  
>  	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
>  	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
>  	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
>  
> -	val = I915_READ(PCH_ADPA);
> -	I915_STATE_WARN(adpa_pipe_enabled(dev_priv, pipe, val),
> -	     "PCH VGA enabled on transcoder %c, should be disabled\n",
> -	     pipe_name(pipe));
> +	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
> +			port_pipe == pipe,
> +			"PCH VGA enabled on transcoder %c, should be disabled\n",
> +			pipe_name(pipe));
>  
>  	val = I915_READ(PCH_LVDS);
>  	I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val),
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 652b11e788cc..79e741845e16 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1355,6 +1355,8 @@ void gen9_enable_guc_interrupts(struct drm_i915_private *dev_priv);
>  void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>  
>  /* intel_crt.c */
> +bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
> +			    i915_reg_t adpa_reg, enum pipe *pipe);
>  void intel_crt_init(struct drm_i915_private *dev_priv);
>  void intel_crt_reset(struct drm_encoder *encoder);

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

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

* Re: [PATCH 02/14] drm/i915: Clean up LVDS pipe select bits
  2018-03-02  9:55 ` [PATCH 02/14] drm/i915: Clean up LVDS " Ville Syrjala
@ 2018-03-20  6:39   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  6:39 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Clean up the LVDS pipe select bits. To make the whole situation a bit
> less ugly we'll start to share the same code between .get_hw_state()
> and the port state asserts.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Couple of nitpicks below, but regardless,

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  9 ++++--
>  drivers/gpu/drm/i915/intel_display.c | 35 ++++++-----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_lvds.c    | 54 +++++++++++++++++++-----------------
>  4 files changed, 44 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f573095d60c2..e993eec97c98 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4458,9 +4458,12 @@ enum {
>   */
>  #define   LVDS_PORT_EN			(1 << 31)
>  /* Selects pipe B for LVDS data.  Must be set on pre-965. */
> -#define   LVDS_PIPEB_SELECT		(1 << 30)
> -#define   LVDS_PIPE_MASK		(1 << 30)
> -#define   LVDS_PIPE(pipe)		((pipe) << 30)
> +#define   LVDS_PIPE_SEL(pipe)		((pipe) << 30)
> +#define   LVDS_PIPE_SEL_MASK		(1 << 30)
> +#define   LVDS_PIPE_SEL_SHIFT		30
> +#define   LVDS_PIPE_SEL_CPT(pipe)	((pipe) << 29)
> +#define   LVDS_PIPE_SEL_MASK_CPT	(3 << 30)
> +#define   LVDS_PIPE_SEL_SHIFT_CPT	29

I'd prefer masks and shifts before values.

>  /* LVDS dithering flag on 965/g4x platform */
>  #define   LVDS_ENABLE_DITHER		(1 << 25)
>  /* LVDS sync polarity flags. Set to invert (i.e. negative) */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 545d89152e9b..f1f164a20b3f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1171,9 +1171,9 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
>  		pp_reg = PP_CONTROL(0);
>  		port_sel = I915_READ(PP_ON_DELAYS(0)) & PANEL_PORT_SELECT_MASK;
>  
> -		if (port_sel == PANEL_PORT_SELECT_LVDS &&
> -		    I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
> -			panel_pipe = PIPE_B;
> +		if (port_sel == PANEL_PORT_SELECT_LVDS) {
> +			intel_lvds_port_enabled(dev_priv, PCH_LVDS, &panel_pipe);
> +		}

Superfluous braces.

>  		/* XXX: else fix for eDP */
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		/* presumably write lock depends on pipe, not port select */
> @@ -1181,8 +1181,7 @@ void assert_panel_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
>  		panel_pipe = pipe;
>  	} else {
>  		pp_reg = PP_CONTROL(0);
> -		if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
> -			panel_pipe = PIPE_B;
> +		intel_lvds_port_enabled(dev_priv, LVDS, &panel_pipe);
>  	}
>  
>  	val = I915_READ(pp_reg);
> @@ -1301,22 +1300,6 @@ static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> -static bool lvds_pipe_enabled(struct drm_i915_private *dev_priv,
> -			      enum pipe pipe, u32 val)
> -{
> -	if ((val & LVDS_PORT_EN) == 0)
> -		return false;
> -
> -	if (HAS_PCH_CPT(dev_priv)) {
> -		if ((val & PORT_TRANS_SEL_MASK) != PORT_TRANS_SEL_CPT(pipe))
> -			return false;
> -	} else {
> -		if ((val & LVDS_PIPE_MASK) != LVDS_PIPE(pipe))
> -			return false;
> -	}
> -	return true;
> -}
> -
>  static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
>  				   enum pipe pipe, i915_reg_t reg,
>  				   u32 port_sel)
> @@ -1348,7 +1331,6 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  				      enum pipe pipe)
>  {
>  	enum pipe port_pipe;
> -	u32 val;
>  
>  	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
>  	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
> @@ -1359,10 +1341,10 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  			"PCH VGA enabled on transcoder %c, should be disabled\n",
>  			pipe_name(pipe));
>  
> -	val = I915_READ(PCH_LVDS);
> -	I915_STATE_WARN(lvds_pipe_enabled(dev_priv, pipe, val),
> -	     "PCH LVDS enabled on transcoder %c, should be disabled\n",
> -	     pipe_name(pipe));
> +	I915_STATE_WARN(intel_lvds_port_enabled(dev_priv, PCH_LVDS, &port_pipe) &&
> +			port_pipe == pipe,
> +			"PCH LVDS enabled on transcoder %c, should be disabled\n",
> +			pipe_name(pipe));
>  
>  	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
>  	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
> @@ -1405,7 +1387,6 @@ static void vlv_enable_pll(struct intel_crtc *crtc,
>  	POSTING_READ(DPLL_MD(pipe));
>  }
>  
> -

Superfluous whitespace change.

>  static void _chv_enable_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 79e741845e16..16b7adc7b0c9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1789,6 +1789,8 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
>  
>  
>  /* intel_lvds.c */
> +bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
> +			     i915_reg_t lvds_reg, enum pipe *pipe);
>  void intel_lvds_init(struct drm_i915_private *dev_priv);
>  struct intel_encoder *intel_get_lvds_encoder(struct drm_device *dev);
>  bool intel_is_dual_link_lvds(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d35d2d50f595..571b69f79eda 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -85,34 +85,35 @@ static struct intel_lvds_connector *to_lvds_connector(struct drm_connector *conn
>  	return container_of(connector, struct intel_lvds_connector, base.base);
>  }
>  
> +bool intel_lvds_port_enabled(struct drm_i915_private *dev_priv,
> +			     i915_reg_t lvds_reg, enum pipe *pipe)
> +{
> +	u32 val;
> +
> +	val = I915_READ(lvds_reg);
> +
> +	/* asserts want to know the pipe even if the port is disabled */
> +	if (HAS_PCH_CPT(dev_priv))
> +		*pipe = (val & LVDS_PIPE_SEL_MASK_CPT) >> LVDS_PIPE_SEL_SHIFT_CPT;
> +	else
> +		*pipe = (val & LVDS_PIPE_SEL_MASK) >> LVDS_PIPE_SEL_SHIFT;
> +
> +	return val & LVDS_PORT_EN;
> +}
> +
>  static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
>  				    enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> -	u32 tmp;
>  	bool ret;
>  
>  	if (!intel_display_power_get_if_enabled(dev_priv,
>  						encoder->power_domain))
>  		return false;
>  
> -	ret = false;
> +	ret = intel_lvds_port_enabled(dev_priv, lvds_encoder->reg, pipe);
>  
> -	tmp = I915_READ(lvds_encoder->reg);
> -
> -	if (!(tmp & LVDS_PORT_EN))
> -		goto out;
> -
> -	if (HAS_PCH_CPT(dev_priv))
> -		*pipe = PORT_TO_PIPE_CPT(tmp);
> -	else
> -		*pipe = PORT_TO_PIPE(tmp);
> -
> -	ret = true;
> -
> -out:
>  	intel_display_power_put(dev_priv, encoder->power_domain);
>  
>  	return ret;
> @@ -255,14 +256,11 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder,
>  	temp |= LVDS_PORT_EN | LVDS_A0A2_CLKA_POWER_UP;
>  
>  	if (HAS_PCH_CPT(dev_priv)) {
> -		temp &= ~PORT_TRANS_SEL_MASK;
> -		temp |= PORT_TRANS_SEL_CPT(pipe);
> +		temp &= ~LVDS_PIPE_SEL_MASK_CPT;
> +		temp |= LVDS_PIPE_SEL_CPT(pipe);
>  	} else {
> -		if (pipe == 1) {
> -			temp |= LVDS_PIPEB_SELECT;
> -		} else {
> -			temp &= ~LVDS_PIPEB_SELECT;
> -		}
> +		temp &= ~LVDS_PIPE_SEL_MASK;
> +		temp |= LVDS_PIPE_SEL(pipe);
>  	}
>  
>  	/* set the corresponsding LVDS_BORDER bit */
> @@ -906,8 +904,12 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder)
>  	 * we need to check "the value to be set" in VBT when LVDS
>  	 * register is uninitialized.
>  	 */
> -	val = I915_READ(lvds_encoder->reg);
> -	if (!(val & ~(LVDS_PIPE_MASK | LVDS_DETECTED)))
> +	val = I915_READ(lvds_encoder->reg) & ~LVDS_DETECTED;
> +	if (HAS_PCH_CPT(dev_priv))
> +		val &= ~LVDS_PIPE_SEL_MASK_CPT;
> +	else
> +		val &= ~LVDS_PIPE_SEL_MASK;

Matter of taste, I'd duplicate the LVDS_DETECTED in the masking instead
of combining it in the register read line.

> +	if (val == 0)
>  		val = dev_priv->vbt.bios_lvds_val;
>  
>  	return (val & LVDS_CLKB_POWER_MASK) == LVDS_CLKB_POWER_UP;

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

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

* Re: [PATCH 03/14] drm/i915: Clean up SDVO pipe select bits
  2018-03-02  9:55 ` [PATCH 03/14] drm/i915: Clean up SDVO " Ville Syrjala
@ 2018-03-20  6:50   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  6:50 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Clean up the SDVO pipe select bits. To make the whole situation a bit
> less ugly we'll start to share the same code between .get_hw_state()
> and the port state asserts.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Good stuff. Same nitpicks about masks and shifts before values, but it's
already there before this patch,

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


> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  4 +++-
>  drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++-----------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 25 ++++----------------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 38 ++++++++++++++++++-----------
>  5 files changed, 49 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e993eec97c98..0917fcbd618d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4365,7 +4365,7 @@ enum {
>  #define   SDVO_ENABLE				(1 << 31)
>  #define   SDVO_PIPE_SEL(pipe)			((pipe) << 30)
>  #define   SDVO_PIPE_SEL_MASK			(1 << 30)
> -#define   SDVO_PIPE_B_SELECT			(1 << 30)
> +#define   SDVO_PIPE_SEL_SHIFT			30
>  #define   SDVO_STALL_SELECT			(1 << 29)
>  #define   SDVO_INTERRUPT_ENABLE			(1 << 26)
>  /*
> @@ -4407,10 +4407,12 @@ enum {
>  /* Gen 6 (CPT) SDVO/HDMI bits: */
>  #define   SDVO_PIPE_SEL_CPT(pipe)		((pipe) << 29)
>  #define   SDVO_PIPE_SEL_MASK_CPT		(3 << 29)
> +#define   SDVO_PIPE_SEL_SHIFT_CPT		29
>  
>  /* CHV SDVO/HDMI bits: */
>  #define   SDVO_PIPE_SEL_CHV(pipe)		((pipe) << 24)
>  #define   SDVO_PIPE_SEL_MASK_CHV		(3 << 24)
> +#define   SDVO_PIPE_SEL_SHIFT_CHV		24
>  
>  
>  /* DVO port control */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f1f164a20b3f..26f8f1e741f7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1281,25 +1281,6 @@ static bool dp_pipe_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> -static bool hdmi_pipe_enabled(struct drm_i915_private *dev_priv,
> -			      enum pipe pipe, u32 val)
> -{
> -	if ((val & SDVO_ENABLE) == 0)
> -		return false;
> -
> -	if (HAS_PCH_CPT(dev_priv)) {
> -		if ((val & SDVO_PIPE_SEL_MASK_CPT) != SDVO_PIPE_SEL_CPT(pipe))
> -			return false;
> -	} else if (IS_CHERRYVIEW(dev_priv)) {
> -		if ((val & SDVO_PIPE_SEL_MASK_CHV) != SDVO_PIPE_SEL_CHV(pipe))
> -			return false;
> -	} else {
> -		if ((val & SDVO_PIPE_SEL_MASK) != SDVO_PIPE_SEL(pipe))
> -			return false;
> -	}
> -	return true;
> -}
> -
>  static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
>  				   enum pipe pipe, i915_reg_t reg,
>  				   u32 port_sel)
> @@ -1315,16 +1296,21 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
>  }
>  
>  static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv,
> -				     enum pipe pipe, i915_reg_t reg)
> +				     enum pipe pipe, enum port port,
> +				     i915_reg_t hdmi_reg)
>  {
> -	u32 val = I915_READ(reg);
> -	I915_STATE_WARN(hdmi_pipe_enabled(dev_priv, pipe, val),
> -	     "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n",
> -	     i915_mmio_reg_offset(reg), pipe_name(pipe));
> +	enum pipe port_pipe;
> +	bool state;
> +
> +	state = intel_sdvo_port_enabled(dev_priv, hdmi_reg, &port_pipe);
> +
> +	I915_STATE_WARN(state && port_pipe == pipe,
> +			"PCH HDMI %c enabled on transcoder %c, should be disabled\n",
> +			port_name(port), pipe_name(pipe));
>  
> -	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && (val & SDVO_ENABLE) == 0
> -	     && (val & SDVO_PIPE_B_SELECT),
> -	     "IBX PCH hdmi port still using transcoder B\n");
> +	I915_STATE_WARN(HAS_PCH_IBX(dev_priv) && !state && port_pipe == PIPE_B,
> +			"IBX PCH HDMI %c still using transcoder B\n",
> +			port_name(port));
>  }
>  
>  static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
> @@ -1346,9 +1332,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  			"PCH LVDS enabled on transcoder %c, should be disabled\n",
>  			pipe_name(pipe));
>  
> -	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIB);
> -	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMIC);
> -	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_B, PCH_HDMIB);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_C, PCH_HDMIC);
> +	assert_pch_hdmi_disabled(dev_priv, pipe, PORT_D, PCH_HDMID);
>  }
>  
>  static void _vlv_enable_pll(struct intel_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 16b7adc7b0c9..09a1b968ac9c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2024,6 +2024,8 @@ void intel_init_ipc(struct drm_i915_private *dev_priv);
>  void intel_enable_ipc(struct drm_i915_private *dev_priv);
>  
>  /* intel_sdvo.c */
> +bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv,
> +			     i915_reg_t sdvo_reg, enum pipe *pipe);
>  bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>  		     i915_reg_t reg, enum port port);
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f5d7bfb43006..bf6facec4efb 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1161,33 +1161,16 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder,
>  static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
>  				    enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> -	u32 tmp;
>  	bool ret;
>  
>  	if (!intel_display_power_get_if_enabled(dev_priv,
>  						encoder->power_domain))
>  		return false;
>  
> -	ret = false;
> -
> -	tmp = I915_READ(intel_hdmi->hdmi_reg);
> -
> -	if (!(tmp & SDVO_ENABLE))
> -		goto out;
> -
> -	if (HAS_PCH_CPT(dev_priv))
> -		*pipe = PORT_TO_PIPE_CPT(tmp);
> -	else if (IS_CHERRYVIEW(dev_priv))
> -		*pipe = SDVO_PORT_TO_PIPE_CHV(tmp);
> -	else
> -		*pipe = PORT_TO_PIPE(tmp);
> -
> -	ret = true;
> +	ret = intel_sdvo_port_enabled(dev_priv, intel_hdmi->hdmi_reg, pipe);
>  
> -out:
>  	intel_display_power_put(dev_priv, encoder->power_domain);
>  
>  	return ret;
> @@ -1421,8 +1404,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  
> -		temp &= ~SDVO_PIPE_B_SELECT;
> -		temp |= SDVO_ENABLE;
> +		temp &= ~SDVO_PIPE_SEL_MASK;
> +		temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A);
>  		/*
>  		 * HW workaround, need to write this twice for issue
>  		 * that may result in first write getting masked.
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 0c14d1c04cbd..3a90bcfbbedb 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1403,27 +1403,37 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector)
>  		return false;
>  }
>  
> +bool intel_sdvo_port_enabled(struct drm_i915_private *dev_priv,
> +			     i915_reg_t sdvo_reg, enum pipe *pipe)
> +{
> +	u32 val;
> +
> +	val = I915_READ(sdvo_reg);
> +
> +	/* asserts want to know the pipe even if the port is disabled */
> +	if (HAS_PCH_CPT(dev_priv))
> +		*pipe = (val & SDVO_PIPE_SEL_MASK_CPT) >> SDVO_PIPE_SEL_SHIFT_CPT;
> +	else if (IS_CHERRYVIEW(dev_priv))
> +		*pipe = (val & SDVO_PIPE_SEL_MASK_CHV) >> SDVO_PIPE_SEL_SHIFT_CHV;
> +	else
> +		*pipe = (val & SDVO_PIPE_SEL_MASK) >> SDVO_PIPE_SEL_SHIFT;
> +
> +	return val & SDVO_ENABLE;
> +}
> +
>  static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
>  				    enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
>  	u16 active_outputs = 0;
> -	u32 tmp;
> +	bool ret;
>  
> -	tmp = I915_READ(intel_sdvo->sdvo_reg);
>  	intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs);
>  
> -	if (!(tmp & SDVO_ENABLE) && (active_outputs == 0))
> -		return false;
> -
> -	if (HAS_PCH_CPT(dev_priv))
> -		*pipe = PORT_TO_PIPE_CPT(tmp);
> -	else
> -		*pipe = PORT_TO_PIPE(tmp);
> +	ret = intel_sdvo_port_enabled(dev_priv, intel_sdvo->sdvo_reg, pipe);
>  
> -	return true;
> +	return ret || active_outputs;
>  }
>  
>  static void intel_sdvo_get_config(struct intel_encoder *encoder,
> @@ -1550,8 +1560,8 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>  
> -		temp &= ~SDVO_PIPE_B_SELECT;
> -		temp |= SDVO_ENABLE;
> +		temp &= ~SDVO_PIPE_SEL_MASK;
> +		temp |= SDVO_ENABLE | SDVO_PIPE_SEL(PIPE_A);
>  		intel_sdvo_write_sdvox(intel_sdvo, temp);
>  
>  		temp &= ~SDVO_ENABLE;

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

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

* Re: [PATCH 04/14] drm/i915: Clean up TV pipe select bits
  2018-03-02  9:55 ` [PATCH 04/14] drm/i915: Clean up TV " Ville Syrjala
@ 2018-03-20  6:55   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  6:55 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Parametrize the TV pipe select bits.
>
> For consistency with the new way of doing things, let's read out the
> pipe select bits even when the port is disable, even though we don't
> need that behaviour for asserts in this case.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  4 +++-
>  drivers/gpu/drm/i915/intel_tv.c | 18 +++++-------------
>  2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0917fcbd618d..fadd0a285efa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4762,7 +4762,9 @@ enum {
>  /* Enables the TV encoder */
>  # define TV_ENC_ENABLE			(1 << 31)
>  /* Sources the TV encoder input from pipe B instead of A. */
> -# define TV_ENC_PIPEB_SELECT		(1 << 30)
> +# define TV_ENC_PIPE_SEL(pipe)		((pipe) << 30)
> +# define TV_ENC_PIPE_SEL_MASK		(1 << 30)
> +# define TV_ENC_PIPE_SEL_SHIFT		30

Same old nitpick about masks and shifts before values.

>  /* Outputs composite video (DAC A only) */
>  # define TV_ENC_OUTPUT_COMPOSITE	(0 << 28)
>  /* Outputs SVideo video (DAC B/C) */
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 885fc3809f7f..24caf340a7a5 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -798,16 +798,12 @@ static struct intel_tv *intel_attached_tv(struct drm_connector *connector)
>  static bool
>  intel_tv_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 tmp = I915_READ(TV_CTL);
>  
> -	if (!(tmp & TV_ENC_ENABLE))
> -		return false;
> +	*pipe = (tmp & TV_ENC_PIPE_SEL_MASK) >> TV_ENC_PIPE_SEL_SHIFT;
>  
> -	*pipe = PORT_TO_PIPE(tmp);
> -
> -	return true;
> +	return tmp & TV_ENC_ENABLE;
>  }
>  
>  static void
> @@ -1024,8 +1020,7 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder,
>  		break;
>  	}
>  
> -	if (intel_crtc->pipe == 1)
> -		tv_ctl |= TV_ENC_PIPEB_SELECT;
> +	tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe);
>  	tv_ctl |= tv_mode->oversample;
>  
>  	if (tv_mode->progressive)
> @@ -1151,10 +1146,7 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
>  	/* Poll for TV detection */
>  	tv_ctl &= ~(TV_ENC_ENABLE | TV_TEST_MODE_MASK);

You need to add TV_ENC_PIPE_SEL_MASK here.

With that fixed,

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

>  	tv_ctl |= TV_TEST_MODE_MONITOR_DETECT;
> -	if (intel_crtc->pipe == 1)
> -		tv_ctl |= TV_ENC_PIPEB_SELECT;
> -	else
> -		tv_ctl &= ~TV_ENC_PIPEB_SELECT;
> +	tv_ctl |= TV_ENC_PIPE_SEL(intel_crtc->pipe);
>  
>  	tv_dac &= ~(TVDAC_SENSE_MASK | DAC_A_MASK | DAC_B_MASK | DAC_C_MASK);
>  	tv_dac |= (TVDAC_STATE_CHG_EN |

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

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

* Re: [PATCH 05/14] drm/i915: Clean up DVO pipe select bits
  2018-03-02  9:55 ` [PATCH 05/14] drm/i915: Clean up DVO " Ville Syrjala
@ 2018-03-20  7:00   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  7:00 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Parametrize the DVO pipe select bits.
>
> For consistency with the new way of doing things, let's read out the
> pipe select bits even when the port is disable, even though we don't
> need that behaviour for asserts in this case.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  4 +++-
>  drivers/gpu/drm/i915/intel_dvo.c | 13 ++++---------
>  2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fadd0a285efa..d7dc03bd0b4f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4423,7 +4423,9 @@ enum {
>  #define _DVOC			0x61160
>  #define DVOC			_MMIO(_DVOC)
>  #define   DVO_ENABLE			(1 << 31)
> -#define   DVO_PIPE_B_SELECT		(1 << 30)
> +#define   DVO_PIPE_SEL(pipe)		((pipe) << 30)
> +#define   DVO_PIPE_SEL_MASK		(1 << 30)
> +#define   DVO_PIPE_SEL_SHIFT		30
>  #define   DVO_PIPE_STALL_UNUSED		(0 << 28)
>  #define   DVO_PIPE_STALL		(1 << 28)
>  #define   DVO_PIPE_STALL_TV		(2 << 28)
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index eb0c559b2715..a86f0398570f 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -137,19 +137,15 @@ static bool intel_dvo_connector_get_hw_state(struct intel_connector *connector)
>  static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
>  				   enum pipe *pipe)
>  {
> -	struct drm_device *dev = encoder->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
>  	u32 tmp;
>  
>  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
>  
> -	if (!(tmp & DVO_ENABLE))
> -		return false;
> -
> -	*pipe = PORT_TO_PIPE(tmp);
> +	*pipe = (tmp & DVO_PIPE_SEL_MASK) >> DVO_PIPE_SEL_SHIFT;
>  
> -	return true;
> +	return tmp & DVO_ENABLE;
>  }
>  
>  static void intel_dvo_get_config(struct intel_encoder *encoder,
> @@ -276,8 +272,7 @@ static void intel_dvo_pre_enable(struct intel_encoder *encoder,
>  	dvo_val |= DVO_DATA_ORDER_FP | DVO_BORDER_ENABLE |
>  		   DVO_BLANK_ACTIVE_HIGH;
>  
> -	if (pipe == 1)
> -		dvo_val |= DVO_PIPE_B_SELECT;
> +	dvo_val |= DVO_PIPE_SEL(pipe);
>  	dvo_val |= DVO_PIPE_STALL;
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
>  		dvo_val |= DVO_HSYNC_ACTIVE_HIGH;

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

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

* Re: [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP
  2018-03-02  9:55 ` [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP Ville Syrjala
@ 2018-03-20  7:11   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  7:11 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Almost all of the GEN7 checks in the DP code are actually looking for
> IVB. HSW doesn't even take these codepaths, and VLV is excluded on
> account of not having port A. So let's change the checks to IS_IVB to
> make the code less confusing.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 642ae298df07..2a82eccffe54 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1970,7 +1970,7 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
>  
>  	/* Split out the IBX/CPU vs CPT settings */
>  
> -	if (IS_GEN7(dev_priv) && port == PORT_A) {
> +	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
>  		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
>  			intel_dp->DP |= DP_SYNC_HS_HIGH;
>  		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
> @@ -2650,7 +2650,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
>  	if (!(tmp & DP_PORT_EN))
>  		goto out;
>  
> -	if (IS_GEN7(dev_priv) && port == PORT_A) {
> +	if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
>  		*pipe = PORT_TO_PIPE_CPT(tmp);
>  	} else if (HAS_PCH_CPT(dev_priv) && port != PORT_A) {
>  		enum pipe p;
> @@ -2887,7 +2887,7 @@ _intel_dp_set_link_train(struct intel_dp *intel_dp,
>  		}
>  		I915_WRITE(DP_TP_CTL(port), temp);
>  
> -	} else if ((IS_GEN7(dev_priv) && port == PORT_A) ||
> +	} else if ((IS_IVYBRIDGE(dev_priv) && port == PORT_A) ||
>  		   (HAS_PCH_CPT(dev_priv) && port != PORT_A)) {
>  		*DP &= ~DP_LINK_TRAIN_MASK_CPT;
>  
> @@ -3213,7 +3213,7 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
>  		return intel_ddi_dp_voltage_max(encoder);
>  	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else if (IS_GEN7(dev_priv) && port == PORT_A)
> +	else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A)
>  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
>  	else if (HAS_PCH_CPT(dev_priv) && port != PORT_A)
>  		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> @@ -3242,7 +3242,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
>  		default:
>  			return DP_TRAIN_PRE_EMPH_LEVEL_0;
>  		}
> -	} else if (IS_GEN7(dev_priv) && port == PORT_A) {
> +	} else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
>  		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
>  		case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
>  			return DP_TRAIN_PRE_EMPH_LEVEL_2;
> @@ -3551,7 +3551,7 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp)
>  		signal_levels = chv_signal_levels(intel_dp);
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		signal_levels = vlv_signal_levels(intel_dp);
> -	} else if (IS_GEN7(dev_priv) && port == PORT_A) {
> +	} else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) {
>  		signal_levels = gen7_edp_signal_levels(train_set);

I guess a follow-up could rename the gen7_ function to ivb_ too.

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

>  		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_IVB;
>  	} else if (IS_GEN6(dev_priv) && port == PORT_A) {
> @@ -3641,7 +3641,7 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  
>  	DRM_DEBUG_KMS("\n");
>  
> -	if ((IS_GEN7(dev_priv) && port == PORT_A) ||
> +	if ((IS_IVYBRIDGE(dev_priv) && port == PORT_A) ||
>  	    (HAS_PCH_CPT(dev_priv) && port != PORT_A)) {
>  		DP &= ~DP_LINK_TRAIN_MASK_CPT;
>  		DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;

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

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

* Re: [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code
  2018-03-02  9:55 ` [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code Ville Syrjala
@ 2018-03-20  7:19   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  7:19 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The ddi code no longer uses intel_ddi_get_crtc_new_encoder(). Move it
> elsewhere where we have some users left.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bit sad for adding more stuff to intel_display.c, but seems like a
better match here.

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


> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 29 -----------------------------
>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 --
>  3 files changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index a56a4db6dc38..ab084170ea38 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1065,35 +1065,6 @@ intel_ddi_get_crtc_encoder(struct intel_crtc *crtc)
>  	return ret;
>  }
>  
> -/* Finds the only possible encoder associated with the given CRTC. */
> -struct intel_encoder *
> -intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
> -{
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> -	struct intel_encoder *ret = NULL;
> -	struct drm_atomic_state *state;
> -	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> -	int num_encoders = 0;
> -	int i;
> -
> -	state = crtc_state->base.state;
> -
> -	for_each_new_connector_in_state(state, connector, connector_state, i) {
> -		if (connector_state->crtc != crtc_state->base.crtc)
> -			continue;
> -
> -		ret = to_intel_encoder(connector_state->best_encoder);
> -		num_encoders++;
> -	}
> -
> -	WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n", num_encoders,
> -	     pipe_name(crtc->pipe));
> -
> -	BUG_ON(ret == NULL);
> -	return ret;
> -}
> -
>  #define LC_FREQ 2700
>  
>  static int hsw_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 26f8f1e741f7..239059493243 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4491,6 +4491,35 @@ static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
>  	}
>  }
>  
> +/*
> + * Finds the encoder associated with the given CRTC. This can only be
> + * used when we know that the CRTC isn't feeding multiple encoders!
> + */
> +static struct intel_encoder *
> +intel_get_crtc_new_encoder(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	const struct drm_atomic_state *state = crtc_state->base.state;
> +	const struct drm_connector_state *connector_state;
> +	const struct drm_connector *connector;
> +	struct intel_encoder *encoder = NULL;
> +	int num_encoders = 0;
> +	int i;
> +
> +	for_each_new_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != &crtc->base)
> +			continue;
> +
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +		num_encoders++;
> +	}
> +
> +	WARN(num_encoders != 1, "%d encoders for pipe %c\n",
> +	     num_encoders, pipe_name(crtc->pipe));
> +
> +	return encoder;
> +}
> +
>  /* Return which DP Port should be selected for Transcoder DP control */
>  static enum port
>  intel_trans_dp_port_sel(struct intel_crtc *crtc)
> @@ -8953,7 +8982,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  {
>  	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
>  		struct intel_encoder *encoder =
> -			intel_ddi_get_crtc_new_encoder(crtc_state);
> +			intel_get_crtc_new_encoder(crtc_state);
>  
>  		if (!intel_get_shared_dpll(crtc, crtc_state, encoder)) {
>  			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d783ecfef46d..9f6e68d192b6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1373,8 +1373,6 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
>  				       enum transcoder cpu_transcoder);
>  void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state);
>  void intel_ddi_disable_pipe_clock(const  struct intel_crtc_state *crtc_state);
> -struct intel_encoder *
> -intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state);
>  void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state);
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
>  bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);

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

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

* Re: [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL
  2018-03-02  9:55 ` [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL Ville Syrjala
@ 2018-03-20  8:36   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-03-20  8:36 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Fri, 02 Mar 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Parametrize the TRANS_DP_PORT_SEL macros.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  8 +++-----
>  drivers/gpu/drm/i915/intel_display.c | 23 +++++++----------------
>  2 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d7dc03bd0b4f..f1460fd27881 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7914,11 +7914,9 @@ enum {
>  #define _TRANS_DP_CTL_C		0xe2300
>  #define TRANS_DP_CTL(pipe)	_MMIO_PIPE(pipe, _TRANS_DP_CTL_A, _TRANS_DP_CTL_B)
>  #define  TRANS_DP_OUTPUT_ENABLE	(1<<31)
> -#define  TRANS_DP_PORT_SEL_B	(0<<29)
> -#define  TRANS_DP_PORT_SEL_C	(1<<29)
> -#define  TRANS_DP_PORT_SEL_D	(2<<29)
> -#define  TRANS_DP_PORT_SEL_NONE	(3<<29)
> -#define  TRANS_DP_PORT_SEL_MASK	(3<<29)
> +#define  TRANS_DP_PORT_SEL(port)	(((port) - PORT_B) << 29)
> +#define  TRANS_DP_PORT_SEL_NONE		(3 << 29)
> +#define  TRANS_DP_PORT_SEL_MASK		(3 << 29)
>  #define  TRANS_DP_PIPE_TO_PORT(val)	((((val) & TRANS_DP_PORT_SEL_MASK) >> 29) + PORT_B)
>  #define  TRANS_DP_AUDIO_ONLY	(1<<26)
>  #define  TRANS_DP_ENH_FRAMING	(1<<18)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 239059493243..e8bedf09d892 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1318,9 +1318,9 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  {
>  	enum pipe port_pipe;
>  
> -	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
> -	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
> -	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
> +	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL(PORT_B));
> +	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL(PORT_C));
> +	assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL(PORT_D));
>  
>  	I915_STATE_WARN(intel_crt_port_enabled(dev_priv, PCH_ADPA, &port_pipe) &&
>  			port_pipe == pipe,
> @@ -4603,6 +4603,8 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
>  			&crtc_state->base.adjusted_mode;
>  		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) >> 5;
>  		i915_reg_t reg = TRANS_DP_CTL(pipe);
> +		enum port port;
> +
>  		temp = I915_READ(reg);
>  		temp &= ~(TRANS_DP_PORT_SEL_MASK |
>  			  TRANS_DP_SYNC_MASK |
> @@ -4615,19 +4617,8 @@ static void ironlake_pch_enable(const struct intel_crtc_state *crtc_state)
>  		if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
>  			temp |= TRANS_DP_VSYNC_ACTIVE_HIGH;
>  
> -		switch (intel_trans_dp_port_sel(crtc)) {
> -		case PORT_B:
> -			temp |= TRANS_DP_PORT_SEL_B;
> -			break;
> -		case PORT_C:
> -			temp |= TRANS_DP_PORT_SEL_C;
> -			break;
> -		case PORT_D:
> -			temp |= TRANS_DP_PORT_SEL_D;
> -			break;
> -		default:
> -			BUG();
> -		}
> +		port = intel_trans_dp_port_sel(crtc);

I'd be happier to retain something like this here:

	WARN_ON(port < PORT_B || port > PORT_D);

With that,

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

> +		temp |= TRANS_DP_PORT_SEL(port);
>  
>  		I915_WRITE(reg, temp);
>  	}

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

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

end of thread, other threads:[~2018-03-20  8:35 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02  9:54 [PATCH 00/14] drm/i915: Clean up the port pipe select bits Ville Syrjala
2018-03-02  9:54 ` [PATCH 01/14] drm/i915: Clean up ADPA " Ville Syrjala
2018-03-20  6:27   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 02/14] drm/i915: Clean up LVDS " Ville Syrjala
2018-03-20  6:39   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 03/14] drm/i915: Clean up SDVO " Ville Syrjala
2018-03-20  6:50   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 04/14] drm/i915: Clean up TV " Ville Syrjala
2018-03-20  6:55   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 05/14] drm/i915: Clean up DVO " Ville Syrjala
2018-03-20  7:00   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 06/14] drm/i915: Use intel_ddi_dp_voltage_max() for HSW/BDW too Ville Syrjala
2018-03-02  9:55 ` [PATCH 07/14] drm/i915: Use the same vswing->max_preemph mapping on HSW/BDW as on SKL+ Ville Syrjala
2018-03-02  9:55 ` [PATCH 08/14] drm/i915: Check for IVB instead of gen7 when we think about IVB CPU eDP Ville Syrjala
2018-03-20  7:11   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 09/14] drm/i915: Move intel_ddi_get_crtc_new_encoder() out from ddi code Ville Syrjala
2018-03-20  7:19   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 10/14] drm/i915: Parametrize TRANS_DP_PORT_SEL Ville Syrjala
2018-03-20  8:36   ` Jani Nikula
2018-03-02  9:55 ` [PATCH 11/14] drm/i915: Nuke intel_trans_dp_port_sel() Ville Syrjala
2018-03-02  9:55 ` [PATCH 12/14] drm/i915: Clean up DP pipe select bits Ville Syrjala
2018-03-02 18:08   ` [PATCH v2 " Ville Syrjala
2018-03-02  9:55 ` [PATCH 13/14] drm/i915: Allow eDP on port C in theory Ville Syrjala
2018-03-02  9:55 ` [PATCH 14/14] drm/i915: Implement the missing bits of assert_panel_unlocked() Ville Syrjala
2018-03-02 11:02 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits Patchwork
2018-03-02 13:09 ` Patchwork
2018-03-02 18:40 ` ✗ Fi.CI.BAT: failure for drm/i915: Clean up the port pipe select bits (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.