[PATCH 0/5] drm/i915: push more stuff down to encoders on crtc enable/disable

[PATCH 0/5] drm/i915: push more stuff down to encoders on crtc enable/disable

[Jani Nikula]

Update of http://mid.mail-archive.com/cover.1499868357.git.jani.nikula@intel.com

BR,
Jani.

Jani Nikula (5):
  drm/i915: push DDI CRT underrun reporting on enable to encoder
  drm/i915: push DDI CRT underrun reporting on disable to encoder
  drm/i915: push DDI and DSI underrun reporting on enable to encoder
  drm/i915: push DDI FDI link training on enable to CRT encoder
  drm/i915/crt: clean up encoder hook assignment

 drivers/gpu/drm/i915/intel_crt.c     | 86 ++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_ddi.c     |  8 ++++
 drivers/gpu/drm/i915/intel_display.c | 24 ----------
 drivers/gpu/drm/i915/intel_dsi.c     |  7 ++-
 4 files changed, 92 insertions(+), 33 deletions(-)

-- 
2.11.0

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

[PATCH 1/5] drm/i915: push DDI CRT underrun reporting on enable to encoder

[Jani Nikula]

CRT being the only PCH encoder, we can simplify the crtc enable by
pushing the PCH encoder specific parts to CRT encoder.

v2: add separate hsw_enable_crt (Daniel), rebase

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     | 51 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c | 14 +---------
 2 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 954070255b4d..6d57c92ac999 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -227,6 +227,52 @@ static void hsw_post_disable_crt(struct intel_encoder *encoder,
 	intel_ddi_fdi_post_disable(encoder, old_crtc_state, old_conn_state);
 }
 
+static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
+				   const struct intel_crtc_state *pipe_config,
+				   const struct drm_connector_state *conn_state)
+{
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	WARN_ON(!intel_crtc->config->has_pch_encoder);
+
+	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+}
+
+static void hsw_pre_enable_crt(struct intel_encoder *encoder,
+			       const struct intel_crtc_state *pipe_config,
+			       const struct drm_connector_state *conn_state)
+{
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+
+	WARN_ON(!intel_crtc->config->has_pch_encoder);
+
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+}
+
+static void hsw_enable_crt(struct intel_encoder *encoder,
+			   const struct intel_crtc_state *pipe_config,
+			   const struct drm_connector_state *conn_state)
+{
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+
+	WARN_ON(!intel_crtc->config->has_pch_encoder);
+
+	intel_crt_set_dpms(encoder, pipe_config, DRM_MODE_DPMS_ON);
+
+	intel_wait_for_vblank(dev_priv, pipe);
+	intel_wait_for_vblank(dev_priv, pipe);
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
+}
+
 static void intel_enable_crt(struct intel_encoder *encoder,
 			     const struct intel_crtc_state *pipe_config,
 			     const struct drm_connector_state *conn_state)
@@ -897,7 +943,6 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 	} else {
 		crt->base.disable = intel_disable_crt;
 	}
-	crt->base.enable = intel_enable_crt;
 	if (I915_HAS_HOTPLUG(dev_priv) &&
 	    !dmi_check_system(intel_spurious_crt_detect))
 		crt->base.hpd_pin = HPD_CRT;
@@ -905,11 +950,15 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 		crt->base.port = PORT_E;
 		crt->base.get_config = hsw_crt_get_config;
 		crt->base.get_hw_state = intel_ddi_get_hw_state;
+		crt->base.pre_pll_enable = hsw_pre_pll_enable_crt;
+		crt->base.pre_enable = hsw_pre_enable_crt;
+		crt->base.enable = hsw_enable_crt;
 		crt->base.post_disable = hsw_post_disable_crt;
 	} else {
 		crt->base.port = PORT_NONE;
 		crt->base.get_config = intel_crt_get_config;
 		crt->base.get_hw_state = intel_crt_get_hw_state;
+		crt->base.enable = intel_enable_crt;
 	}
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cdb2e25a577c..6f275c930fd5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5488,9 +5488,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (WARN_ON(intel_crtc->active))
 		return;
 
-	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
-
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
 	if (intel_crtc->config->shared_dpll)
@@ -5524,9 +5521,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_crtc->active = true;
 
-	if (intel_crtc->config->has_pch_encoder)
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
-	else
+	if (!intel_crtc->config->has_pch_encoder)
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
@@ -5581,13 +5576,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false);
 	}
 
-	if (intel_crtc->config->has_pch_encoder) {
-		intel_wait_for_vblank(dev_priv, pipe);
-		intel_wait_for_vblank(dev_priv, pipe);
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
-	}
-
 	/* If we change the relative order between pipe/planes enabling, we need
 	 * to change the workaround. */
 	hsw_workaround_pipe = pipe_config->hsw_workaround_pipe;
-- 
2.11.0

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

[PATCH 2/5] drm/i915: push DDI CRT underrun reporting on disable to encoder

[Jani Nikula]

CRT being the only PCH encoder, we can simplify the crtc disable by
pushing the PCH encoder specific parts to CRT encoder.

v2: add hsw_disable_crt (Daniel), rebase

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  6 ------
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 6d57c92ac999..b43e8e16da37 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -213,6 +213,19 @@ static void pch_post_disable_crt(struct intel_encoder *encoder,
 	intel_disable_crt(encoder, old_crtc_state, old_conn_state);
 }
 
+static void hsw_disable_crt(struct intel_encoder *encoder,
+			    const struct intel_crtc_state *old_crtc_state,
+			    const struct drm_connector_state *old_conn_state)
+{
+	struct drm_crtc *crtc = old_crtc_state->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	WARN_ON(!intel_crtc->config->has_pch_encoder);
+
+	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+}
+
 static void hsw_post_disable_crt(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *old_crtc_state,
 				 const struct drm_connector_state *old_conn_state)
@@ -225,6 +238,10 @@ static void hsw_post_disable_crt(struct intel_encoder *encoder,
 	lpt_disable_iclkip(dev_priv);
 
 	intel_ddi_fdi_post_disable(encoder, old_crtc_state, old_conn_state);
+
+	WARN_ON(!old_crtc_state->has_pch_encoder);
+
+	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 }
 
 static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
@@ -953,6 +970,7 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 		crt->base.pre_pll_enable = hsw_pre_pll_enable_crt;
 		crt->base.pre_enable = hsw_pre_enable_crt;
 		crt->base.enable = hsw_enable_crt;
+		crt->base.disable = hsw_disable_crt;
 		crt->base.post_disable = hsw_post_disable_crt;
 	} else {
 		crt->base.port = PORT_NONE;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6f275c930fd5..9058cdfb0649 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5669,9 +5669,6 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
-	if (intel_crtc->config->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
-
 	intel_encoders_disable(crtc, old_crtc_state, old_state);
 
 	drm_crtc_vblank_off(crtc);
@@ -5696,9 +5693,6 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 		intel_ddi_disable_pipe_clock(intel_crtc->config);
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
-
-	if (old_crtc_state->has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
-- 
2.11.0

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

[PATCH 3/5] drm/i915: push DDI and DSI underrun reporting on enable to encoder

[Jani Nikula]

Simplify CRTC enable.

v2: Don't forget DSI (Daniel)

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 8 ++++++++
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 drivers/gpu/drm/i915/intel_dsi.c     | 7 ++++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 511aa60e0176..f1adc2544ab9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2207,8 +2207,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *pipe_config,
 				 const struct drm_connector_state *conn_state)
 {
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
 	int type = encoder->type;
 
+	WARN_ON(intel_crtc->config->has_pch_encoder);
+
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+
 	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
 		intel_ddi_pre_enable_dp(encoder,
 					pipe_config->port_clock,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9058cdfb0649..b55944d8149b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5521,9 +5521,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_crtc->active = true;
 
-	if (!intel_crtc->config->has_pch_encoder)
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
 	if (intel_crtc->config->has_pch_encoder)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 20a7b004ffd7..66bbedc5fa01 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -790,14 +790,19 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *pipe_config,
 				 const struct drm_connector_state *conn_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
 	enum port port;
 	u32 val;
 	bool glk_cold_boot = false;
 
 	DRM_DEBUG_KMS("\n");
 
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+
 	/*
 	 * The BIOS may leave the PLL in a wonky state where it doesn't
 	 * lock. It needs to be fully powered down to fix it.
-- 
2.11.0

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

[PATCH 4/5] drm/i915: push DDI FDI link training on enable to CRT encoder

[Jani Nikula]

Reduce encoder specific checks from CRTC code.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     | 2 ++
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index b43e8e16da37..2abe556ccaf5 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -269,6 +269,8 @@ static void hsw_pre_enable_crt(struct intel_encoder *encoder,
 	WARN_ON(!intel_crtc->config->has_pch_encoder);
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
+	dev_priv->display.fdi_link_train(intel_crtc, pipe_config);
 }
 
 static void hsw_enable_crt(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b55944d8149b..9f2bf3b3f759 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5523,9 +5523,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc->config->has_pch_encoder)
-		dev_priv->display.fdi_link_train(intel_crtc, pipe_config);
-
 	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_ddi_enable_pipe_clock(pipe_config);
 
-- 
2.11.0

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

[PATCH 5/5] drm/i915/crt: clean up encoder hook assignment

[Jani Nikula]

Only assign the hooks once instead of overwriting for DDI.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 2abe556ccaf5..668e8c3e791d 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -955,16 +955,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 
 	crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
 
-	crt->base.compute_config = intel_crt_compute_config;
-	if (HAS_PCH_SPLIT(dev_priv)) {
-		crt->base.disable = pch_disable_crt;
-		crt->base.post_disable = pch_post_disable_crt;
-	} else {
-		crt->base.disable = intel_disable_crt;
-	}
 	if (I915_HAS_HOTPLUG(dev_priv) &&
 	    !dmi_check_system(intel_spurious_crt_detect))
 		crt->base.hpd_pin = HPD_CRT;
+
+	crt->base.compute_config = intel_crt_compute_config;
 	if (HAS_DDI(dev_priv)) {
 		crt->base.port = PORT_E;
 		crt->base.get_config = hsw_crt_get_config;
@@ -975,6 +970,12 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 		crt->base.disable = hsw_disable_crt;
 		crt->base.post_disable = hsw_post_disable_crt;
 	} else {
+		if (HAS_PCH_SPLIT(dev_priv)) {
+			crt->base.disable = pch_disable_crt;
+			crt->base.post_disable = pch_post_disable_crt;
+		} else {
+			crt->base.disable = intel_disable_crt;
+		}
 		crt->base.port = PORT_NONE;
 		crt->base.get_config = intel_crt_get_config;
 		crt->base.get_hw_state = intel_crt_get_hw_state;
-- 
2.11.0

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

✓ Fi.CI.BAT: success for drm/i915: push more stuff down to encoders on crtc enable/disable (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: push more stuff down to encoders on crtc enable/disable (rev2)
URL   : https://patchwork.freedesktop.org/series/27186/
State : success

== Summary ==

Series 27186v2 drm/i915: push more stuff down to encoders on crtc enable/disable
https://patchwork.freedesktop.org/api/1.0/series/27186/revisions/2/mbox/

Test drv_module_reload:
        Subgroup basic-no-display:
                incomplete -> PASS       (fi-cfl-s) k.org#196765

k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:457s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:469s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:394s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:569s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:287s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:523s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:531s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:538s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:540s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:552s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:621s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:439s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:606s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:437s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:416s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:463s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:474s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:504s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:585s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:491s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:597s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:654s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:469s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:660s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:533s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:529s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:470s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:585s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:433s

247cb84af034b8e90ecd22cd69adb13a7a305350 drm-tip: 2017y-10m-05d-06h-44m-09s UTC integration manifest
ce11ed928f6e drm/i915/crt: clean up encoder hook assignment
7a7b82006d36 drm/i915: push DDI FDI link training on enable to CRT encoder
ecb89fd5dab9 drm/i915: push DDI and DSI underrun reporting on enable to encoder
d7a8ccfb4913 drm/i915: push DDI CRT underrun reporting on disable to encoder
c37536c25a71 drm/i915: push DDI CRT underrun reporting on enable to encoder

== Logs ==

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

[RFC PATCH] drm/i915: push shared dpll enable to encoders on DDI platforms

[Jani Nikula]

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  3 +++
 drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  3 ---
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 668e8c3e791d..b3094d606329 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -255,6 +255,9 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
 	WARN_ON(!intel_crtc->config->has_pch_encoder);
 
 	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+
+	if (intel_crtc->config->shared_dpll)
+		intel_enable_shared_dpll(intel_crtc);
 }
 
 static void hsw_pre_enable_crt(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f1adc2544ab9..c1152c602f08 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2407,13 +2407,29 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
 	}
 }
 
+static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
+				     const struct intel_crtc_state *pipe_config,
+				     const struct drm_connector_state *conn_state)
+{
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (intel_crtc->config->shared_dpll)
+		intel_enable_shared_dpll(intel_crtc);
+}
+
 static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
 				   const struct intel_crtc_state *pipe_config,
 				   const struct drm_connector_state *conn_state)
 {
+	struct drm_crtc *crtc = pipe_config->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint8_t mask = pipe_config->lane_lat_optim_mask;
 
 	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
+
+	if (intel_crtc->config->shared_dpll)
+		intel_enable_shared_dpll(intel_crtc);
 }
 
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
@@ -2714,6 +2730,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
 		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
+	else
+		intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
 	intel_encoder->disable = intel_disable_ddi;
 	intel_encoder->post_disable = intel_ddi_post_disable;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f2bf3b3f759..6d0573350786 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5490,9 +5490,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
-	if (intel_crtc->config->shared_dpll)
-		intel_enable_shared_dpll(intel_crtc);
-
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
-- 
2.11.0

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

✓ Fi.CI.IGT: success for drm/i915: push shared dpll enable to encoders on DDI platforms (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: push shared dpll enable to encoders on DDI platforms (rev2)
URL   : https://patchwork.freedesktop.org/series/27186/
State : success

== Summary ==

Test kms_cursor_legacy:
        Subgroup cursorA-vs-flipA-atomic-transitions:
                fail       -> PASS       (shard-hsw) fdo#102723

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

shard-hsw        total:2430 pass:1331 dwarn:7   dfail:0   fail:8   skip:1084 time:10190s

== Logs ==

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

Re: [PATCH 3/5] drm/i915: push DDI and DSI underrun reporting on enable to encoder

[Daniel Vetter]

On Thu, Oct 05, 2017 at 01:52:12PM +0300, Jani Nikula wrote:
> Simplify CRTC enable.
> 
> v2: Don't forget DSI (Daniel)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c     | 8 ++++++++
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  drivers/gpu/drm/i915/intel_dsi.c     | 7 ++++++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 511aa60e0176..f1adc2544ab9 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2207,8 +2207,16 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
>  				 const struct intel_crtc_state *pipe_config,
>  				 const struct drm_connector_state *conn_state)
>  {
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int pipe = intel_crtc->pipe;
>  	int type = encoder->type;
>  
> +	WARN_ON(intel_crtc->config->has_pch_encoder);
> +
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> +
>  	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
>  		intel_ddi_pre_enable_dp(encoder,
>  					pipe_config->port_clock,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9058cdfb0649..b55944d8149b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5521,9 +5521,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_crtc->active = true;
>  
> -	if (!intel_crtc->config->has_pch_encoder)
> -		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> -
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
>  	if (intel_crtc->config->has_pch_encoder)
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 20a7b004ffd7..66bbedc5fa01 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -790,14 +790,19 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
>  				 const struct intel_crtc_state *pipe_config,
>  				 const struct drm_connector_state *conn_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int pipe = intel_crtc->pipe;
>  	enum port port;
>  	u32 val;
>  	bool glk_cold_boot = false;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> +
>  	/*
>  	 * The BIOS may leave the PLL in a wonky state where it doesn't
>  	 * lock. It needs to be fully powered down to fix it.
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915/crt: clean up encoder hook assignment

[Daniel Vetter]

On Thu, Oct 05, 2017 at 01:52:14PM +0300, Jani Nikula wrote:
> Only assign the hooks once instead of overwriting for DDI.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_crt.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 2abe556ccaf5..668e8c3e791d 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -955,16 +955,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>  
>  	crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
>  
> -	crt->base.compute_config = intel_crt_compute_config;
> -	if (HAS_PCH_SPLIT(dev_priv)) {
> -		crt->base.disable = pch_disable_crt;
> -		crt->base.post_disable = pch_post_disable_crt;
> -	} else {
> -		crt->base.disable = intel_disable_crt;
> -	}
>  	if (I915_HAS_HOTPLUG(dev_priv) &&
>  	    !dmi_check_system(intel_spurious_crt_detect))
>  		crt->base.hpd_pin = HPD_CRT;
> +
> +	crt->base.compute_config = intel_crt_compute_config;
>  	if (HAS_DDI(dev_priv)) {
>  		crt->base.port = PORT_E;
>  		crt->base.get_config = hsw_crt_get_config;
> @@ -975,6 +970,12 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>  		crt->base.disable = hsw_disable_crt;
>  		crt->base.post_disable = hsw_post_disable_crt;
>  	} else {
> +		if (HAS_PCH_SPLIT(dev_priv)) {
> +			crt->base.disable = pch_disable_crt;
> +			crt->base.post_disable = pch_post_disable_crt;
> +		} else {
> +			crt->base.disable = intel_disable_crt;
> +		}
>  		crt->base.port = PORT_NONE;
>  		crt->base.get_config = intel_crt_get_config;
>  		crt->base.get_hw_state = intel_crt_get_hw_state;
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH] drm/i915: push shared dpll enable to encoders on DDI platforms

[Daniel Vetter]

On Thu, Oct 05, 2017 at 02:50:13PM +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Not yet seeing the point of this one here ... If it would get rid of the
conditional it would make sense (and I think we should know statically
whether we need a shared dpll or not).
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  3 +++
>  drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  3 ---
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 668e8c3e791d..b3094d606329 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -255,6 +255,9 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
>  	WARN_ON(!intel_crtc->config->has_pch_encoder);
>  
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
>  }
>  
>  static void hsw_pre_enable_crt(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f1adc2544ab9..c1152c602f08 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2407,13 +2407,29 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	}
>  }
>  
> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> +				     const struct intel_crtc_state *pipe_config,
> +				     const struct drm_connector_state *conn_state)
> +{
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
> +}
> +
>  static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
>  				   const struct intel_crtc_state *pipe_config,
>  				   const struct drm_connector_state *conn_state)
>  {
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint8_t mask = pipe_config->lane_lat_optim_mask;
>  
>  	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
>  }
>  
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> @@ -2714,6 +2730,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->enable = intel_enable_ddi;
>  	if (IS_GEN9_LP(dev_priv))
>  		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> +	else
> +		intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
>  	intel_encoder->pre_enable = intel_ddi_pre_enable;
>  	intel_encoder->disable = intel_disable_ddi;
>  	intel_encoder->post_disable = intel_ddi_post_disable;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9f2bf3b3f759..6d0573350786 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5490,9 +5490,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc->config->shared_dpll)
> -		intel_enable_shared_dpll(intel_crtc);
> -
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH] drm/i915: push shared dpll enable to encoders on DDI platforms

[Daniel Vetter]

On Thu, Oct 05, 2017 at 02:50:13PM +0300, Jani Nikula wrote:
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  3 +++
>  drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  3 ---
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 668e8c3e791d..b3094d606329 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -255,6 +255,9 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
>  	WARN_ON(!intel_crtc->config->has_pch_encoder);
>  
>  	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
>  }

Looking at haswell_crtc_compute_clock all outputs on hsw+ need a shared
dpll, except dsi. Here's my proposal:

- Drop the if condition when pushing into encoder callbacks, we statically
  know which one it is. With that I think this patch here looks good.

- Push the clock computation into encoders to get rid of the silly DSI
  special in haswell_crtc_compute_clock. Which then again will force you
  to get rid of all the encoder special casing in dpll_mgr->get_dpll, plus
  passing the encoder argument around.

  I think the prettier way to do this is to pre-fill the clock we want in
  the encoder compute_config callback, and then also call
  intel_find_shared_dpll from there.

There's a lot more that should be untangled imo in intel_ddi.c and
computed in compute_config of the right encoder type instead of if ladders
all over, but we're slowly getting there I think.

Cheers, Daniel

>  
>  static void hsw_pre_enable_crt(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f1adc2544ab9..c1152c602f08 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2407,13 +2407,29 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>  	}
>  }
>  
> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> +				     const struct intel_crtc_state *pipe_config,
> +				     const struct drm_connector_state *conn_state)
> +{
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
> +}
> +
>  static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
>  				   const struct intel_crtc_state *pipe_config,
>  				   const struct drm_connector_state *conn_state)
>  {
> +	struct drm_crtc *crtc = pipe_config->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint8_t mask = pipe_config->lane_lat_optim_mask;
>  
>  	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> +
> +	if (intel_crtc->config->shared_dpll)
> +		intel_enable_shared_dpll(intel_crtc);
>  }
>  
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> @@ -2714,6 +2730,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->enable = intel_enable_ddi;
>  	if (IS_GEN9_LP(dev_priv))
>  		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> +	else
> +		intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
>  	intel_encoder->pre_enable = intel_ddi_pre_enable;
>  	intel_encoder->disable = intel_disable_ddi;
>  	intel_encoder->post_disable = intel_ddi_post_disable;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9f2bf3b3f759..6d0573350786 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5490,9 +5490,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
> -	if (intel_crtc->config->shared_dpll)
> -		intel_enable_shared_dpll(intel_crtc);
> -
>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>  
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH] drm/i915: push shared dpll enable to encoders on DDI platforms

[Daniel Vetter]

On Fri, Oct 06, 2017 at 10:07:45AM +0200, Daniel Vetter wrote:
> On Thu, Oct 05, 2017 at 02:50:13PM +0300, Jani Nikula wrote:
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_crt.c     |  3 +++
> >  drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_display.c |  3 ---
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index 668e8c3e791d..b3094d606329 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -255,6 +255,9 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
> >  	WARN_ON(!intel_crtc->config->has_pch_encoder);
> >  
> >  	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> > +	if (intel_crtc->config->shared_dpll)
> > +		intel_enable_shared_dpll(intel_crtc);
> >  }
> 
> Looking at haswell_crtc_compute_clock all outputs on hsw+ need a shared
> dpll, except dsi. Here's my proposal:
> 
> - Drop the if condition when pushing into encoder callbacks, we statically
>   know which one it is. With that I think this patch here looks good.

Note that you don't need to add a WARN_ON to check this if you feel
paranoid, intel_enable_shared_dpll already has you covered (including safe
bail-out if the dpll isn't assigned).
-Daniel

> 
> - Push the clock computation into encoders to get rid of the silly DSI
>   special in haswell_crtc_compute_clock. Which then again will force you
>   to get rid of all the encoder special casing in dpll_mgr->get_dpll, plus
>   passing the encoder argument around.
> 
>   I think the prettier way to do this is to pre-fill the clock we want in
>   the encoder compute_config callback, and then also call
>   intel_find_shared_dpll from there.
> 
> There's a lot more that should be untangled imo in intel_ddi.c and
> computed in compute_config of the right encoder type instead of if ladders
> all over, but we're slowly getting there I think.
> 
> Cheers, Daniel
> 
> >  
> >  static void hsw_pre_enable_crt(struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f1adc2544ab9..c1152c602f08 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2407,13 +2407,29 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
> >  	}
> >  }
> >  
> > +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
> > +				     const struct intel_crtc_state *pipe_config,
> > +				     const struct drm_connector_state *conn_state)
> > +{
> > +	struct drm_crtc *crtc = pipe_config->base.crtc;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > +	if (intel_crtc->config->shared_dpll)
> > +		intel_enable_shared_dpll(intel_crtc);
> > +}
> > +
> >  static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
> >  				   const struct intel_crtc_state *pipe_config,
> >  				   const struct drm_connector_state *conn_state)
> >  {
> > +	struct drm_crtc *crtc = pipe_config->base.crtc;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	uint8_t mask = pipe_config->lane_lat_optim_mask;
> >  
> >  	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
> > +
> > +	if (intel_crtc->config->shared_dpll)
> > +		intel_enable_shared_dpll(intel_crtc);
> >  }
> >  
> >  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
> > @@ -2714,6 +2730,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_encoder->enable = intel_enable_ddi;
> >  	if (IS_GEN9_LP(dev_priv))
> >  		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
> > +	else
> > +		intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
> >  	intel_encoder->pre_enable = intel_ddi_pre_enable;
> >  	intel_encoder->disable = intel_disable_ddi;
> >  	intel_encoder->post_disable = intel_ddi_post_disable;
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9f2bf3b3f759..6d0573350786 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5490,9 +5490,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  
> >  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> >  
> > -	if (intel_crtc->config->shared_dpll)
> > -		intel_enable_shared_dpll(intel_crtc);
> > -
> >  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >  		intel_dp_set_m_n(intel_crtc, M1_N1);
> >  
> > -- 
> > 2.11.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915/crt: clean up encoder hook assignment

[Jani Nikula]

On Thu, 05 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 05, 2017 at 01:52:14PM +0300, Jani Nikula wrote:
>> Only assign the hooks once instead of overwriting for DDI.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for the review, pushed patches 1-5 to dinq.

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/intel_crt.c | 15 ++++++++-------
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index 2abe556ccaf5..668e8c3e791d 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -955,16 +955,11 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>>  
>>  	crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
>>  
>> -	crt->base.compute_config = intel_crt_compute_config;
>> -	if (HAS_PCH_SPLIT(dev_priv)) {
>> -		crt->base.disable = pch_disable_crt;
>> -		crt->base.post_disable = pch_post_disable_crt;
>> -	} else {
>> -		crt->base.disable = intel_disable_crt;
>> -	}
>>  	if (I915_HAS_HOTPLUG(dev_priv) &&
>>  	    !dmi_check_system(intel_spurious_crt_detect))
>>  		crt->base.hpd_pin = HPD_CRT;
>> +
>> +	crt->base.compute_config = intel_crt_compute_config;
>>  	if (HAS_DDI(dev_priv)) {
>>  		crt->base.port = PORT_E;
>>  		crt->base.get_config = hsw_crt_get_config;
>> @@ -975,6 +970,12 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>>  		crt->base.disable = hsw_disable_crt;
>>  		crt->base.post_disable = hsw_post_disable_crt;
>>  	} else {
>> +		if (HAS_PCH_SPLIT(dev_priv)) {
>> +			crt->base.disable = pch_disable_crt;
>> +			crt->base.post_disable = pch_post_disable_crt;
>> +		} else {
>> +			crt->base.disable = intel_disable_crt;
>> +		}
>>  		crt->base.port = PORT_NONE;
>>  		crt->base.get_config = intel_crt_get_config;
>>  		crt->base.get_hw_state = intel_crt_get_hw_state;
>> -- 
>> 2.11.0
>> 

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

Re: [RFC PATCH] drm/i915: push shared dpll enable to encoders on DDI platforms

[Jani Nikula]

On Fri, 06 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 05, 2017 at 02:50:13PM +0300, Jani Nikula wrote:
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_crt.c     |  3 +++
>>  drivers/gpu/drm/i915/intel_ddi.c     | 18 ++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c |  3 ---
>>  3 files changed, 21 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index 668e8c3e791d..b3094d606329 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -255,6 +255,9 @@ static void hsw_pre_pll_enable_crt(struct intel_encoder *encoder,
>>  	WARN_ON(!intel_crtc->config->has_pch_encoder);
>>  
>>  	intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
>> +
>> +	if (intel_crtc->config->shared_dpll)
>> +		intel_enable_shared_dpll(intel_crtc);
>>  }
>
> Looking at haswell_crtc_compute_clock all outputs on hsw+ need a shared
> dpll, except dsi. Here's my proposal:
>
> - Drop the if condition when pushing into encoder callbacks, we statically
>   know which one it is. With that I think this patch here looks good.

Agreed. You think the patch at hand is fine after that as the first
step?

> - Push the clock computation into encoders to get rid of the silly DSI
>   special in haswell_crtc_compute_clock. Which then again will force you
>   to get rid of all the encoder special casing in dpll_mgr->get_dpll, plus
>   passing the encoder argument around.
>
>   I think the prettier way to do this is to pre-fill the clock we want in
>   the encoder compute_config callback, and then also call
>   intel_find_shared_dpll from there.

Are you proposing to do this for all platforms? Or just hsw/ddi+?

Do we still call ->crtc_compute_clock() in intel_crtc_atomic_check(),
and the subsequent call chain will just use the information pre-filled
in compute config?

BR,
Jani.

>
> There's a lot more that should be untangled imo in intel_ddi.c and
> computed in compute_config of the right encoder type instead of if ladders
> all over, but we're slowly getting there I think.
>
> Cheers, Daniel
>
>>  
>>  static void hsw_pre_enable_crt(struct intel_encoder *encoder,
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index f1adc2544ab9..c1152c602f08 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2407,13 +2407,29 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder,
>>  	}
>>  }
>>  
>> +static void intel_ddi_pre_pll_enable(struct intel_encoder *encoder,
>> +				     const struct intel_crtc_state *pipe_config,
>> +				     const struct drm_connector_state *conn_state)
>> +{
>> +	struct drm_crtc *crtc = pipe_config->base.crtc;
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +
>> +	if (intel_crtc->config->shared_dpll)
>> +		intel_enable_shared_dpll(intel_crtc);
>> +}
>> +
>>  static void bxt_ddi_pre_pll_enable(struct intel_encoder *encoder,
>>  				   const struct intel_crtc_state *pipe_config,
>>  				   const struct drm_connector_state *conn_state)
>>  {
>> +	struct drm_crtc *crtc = pipe_config->base.crtc;
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	uint8_t mask = pipe_config->lane_lat_optim_mask;
>>  
>>  	bxt_ddi_phy_set_lane_optim_mask(encoder, mask);
>> +
>> +	if (intel_crtc->config->shared_dpll)
>> +		intel_enable_shared_dpll(intel_crtc);
>>  }
>>  
>>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
>> @@ -2714,6 +2730,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>  	intel_encoder->enable = intel_enable_ddi;
>>  	if (IS_GEN9_LP(dev_priv))
>>  		intel_encoder->pre_pll_enable = bxt_ddi_pre_pll_enable;
>> +	else
>> +		intel_encoder->pre_pll_enable = intel_ddi_pre_pll_enable;
>>  	intel_encoder->pre_enable = intel_ddi_pre_enable;
>>  	intel_encoder->disable = intel_disable_ddi;
>>  	intel_encoder->post_disable = intel_ddi_post_disable;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 9f2bf3b3f759..6d0573350786 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5490,9 +5490,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>>  
>>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>>  
>> -	if (intel_crtc->config->shared_dpll)
>> -		intel_enable_shared_dpll(intel_crtc);
>> -
>>  	if (intel_crtc_has_dp_encoder(intel_crtc->config))
>>  		intel_dp_set_m_n(intel_crtc, M1_N1);
>>  
>> -- 
>> 2.11.0
>> 

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