[PATCH 00/10] drm/i915: Remove low hanging crtc->config fruit, part 2.

[PATCH 00/10] drm/i915: Remove low hanging crtc->config fruit, part 2.

[Maarten Lankhorst]

More users of crtc->config are converted to using the correct crtc_state.

Maarten Lankhorst (10):
  drm/i915: Remove crtc->config dereference from drrs_ctl
  drm/i915: Make intel_dp_set_m_n take crtc_state
  drm/i915: Remove crtc->config references in vlv_prepare_pll
  drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n
  drm/i915: Pass crtc_state to update_scanline_offset
  drm/i915: Remove crtc->config dereferences in intel_sanitize_crtc
  drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state
  drm/i915: Pass crtc_state to lpt_program_iclkip
  drm/i915: Pass crtc_state to ivybridge_update_fdi_bc_bifurcation
  drm/i915: Remove crtc->active from crtc_enable callbacks

 drivers/gpu/drm/i915/i915_debugfs.c  |  54 +++++++--
 drivers/gpu/drm/i915/intel_display.c | 157 ++++++++++++---------------
 drivers/gpu/drm/i915/intel_dp.c      |   4 +-
 drivers/gpu/drm/i915/intel_drv.h     |   3 +-
 4 files changed, 118 insertions(+), 100 deletions(-)

-- 
2.19.0

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

[PATCH 01/10] drm/i915: Remove crtc->config dereference from drrs_ctl

[Maarten Lankhorst]

Wait for idle, and iterate over connectors instead of encoders.
With this information we know crtc->state is the actual state,
and we can enable/disable drrs safely.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 54 ++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f42e93b71e67..b04d5ade5a15 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4661,20 +4661,45 @@ static int i915_drrs_ctl_set(void *data, u64 val)
 {
 	struct drm_i915_private *dev_priv = data;
 	struct drm_device *dev = &dev_priv->drm;
-	struct intel_crtc *intel_crtc;
-	struct intel_encoder *encoder;
-	struct intel_dp *intel_dp;
+	struct intel_crtc *crtc;
 
 	if (INTEL_GEN(dev_priv) < 7)
 		return -ENODEV;
 
-	drm_modeset_lock_all(dev);
-	for_each_intel_crtc(dev, intel_crtc) {
-		if (!intel_crtc->base.state->active ||
-					!intel_crtc->config->has_drrs)
-			continue;
+	for_each_intel_crtc(dev, crtc) {
+		struct drm_connector_list_iter conn_iter;
+		struct intel_crtc_state *crtc_state;
+		struct drm_connector *connector;
+		struct drm_crtc_commit *commit;
+		int ret;
+
+		ret = drm_modeset_lock_single_interruptible(&crtc->base.mutex);
+		if (ret)
+			return ret;
+
+		crtc_state = to_intel_crtc_state(crtc->base.state);
+
+		if (!crtc_state->base.active ||
+		    !crtc_state->has_drrs)
+			goto out;
 
-		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
+		commit = crtc_state->base.commit;
+		if (commit) {
+			ret = wait_for_completion_interruptible(&commit->hw_done);
+			if (ret)
+				goto out;
+		}
+
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			struct intel_encoder *encoder;
+			struct intel_dp *intel_dp;
+
+			if (!(crtc_state->base.connector_mask &
+			      drm_connector_mask(connector)))
+				continue;
+
+			encoder = intel_attached_encoder(connector);
 			if (encoder->type != INTEL_OUTPUT_EDP)
 				continue;
 
@@ -4684,13 +4709,18 @@ static int i915_drrs_ctl_set(void *data, u64 val)
 			intel_dp = enc_to_intel_dp(&encoder->base);
 			if (val)
 				intel_edp_drrs_enable(intel_dp,
-							intel_crtc->config);
+						      crtc_state);
 			else
 				intel_edp_drrs_disable(intel_dp,
-							intel_crtc->config);
+						       crtc_state);
 		}
+		drm_connector_list_iter_end(&conn_iter);
+
+out:
+		drm_modeset_unlock(&crtc->base.mutex);
+		if (ret)
+			return ret;
 	}
-	drm_modeset_unlock_all(dev);
 
 	return 0;
 }
-- 
2.19.0

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

[PATCH 02/10] drm/i915: Make intel_dp_set_m_n take crtc_state

[Maarten Lankhorst]

Another user of crtc->config gone. The functions it calls also
needed crtc->config, so convert those as well.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dp.c      |  4 +-
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 3 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3fd37a9fd49..3d3eefa6ec65 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -143,9 +143,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *ifb,
 				  struct drm_mode_fb_cmd2 *mode_cmd);
 static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state);
 static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
-static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
-					 struct intel_link_m_n *m_n,
-					 struct intel_link_m_n *m2_n2);
+static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
+					 const struct intel_link_m_n *m_n,
+					 const struct intel_link_m_n *m2_n2);
 static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state);
 static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state);
 static void haswell_set_pipeconf(const struct intel_crtc_state *crtc_state);
@@ -5604,14 +5604,14 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_prepare_shared_dpll(pipe_config);
 
 	if (intel_crtc_has_dp_encoder(pipe_config))
-		intel_dp_set_m_n(intel_crtc, M1_N1);
+		intel_dp_set_m_n(pipe_config, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
 	intel_set_pipe_src_size(pipe_config);
 
 	if (pipe_config->has_pch_encoder) {
-		intel_cpu_transcoder_set_m_n(intel_crtc,
-				     &pipe_config->fdi_m_n, NULL);
+		intel_cpu_transcoder_set_m_n(pipe_config,
+					     &pipe_config->fdi_m_n, NULL);
 	}
 
 	ironlake_set_pipeconf(pipe_config);
@@ -5728,7 +5728,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
 	if (intel_crtc_has_dp_encoder(pipe_config))
-		intel_dp_set_m_n(intel_crtc, M1_N1);
+		intel_dp_set_m_n(pipe_config, M1_N1);
 
 	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_set_pipe_timings(pipe_config);
@@ -5742,8 +5742,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	}
 
 	if (pipe_config->has_pch_encoder) {
-		intel_cpu_transcoder_set_m_n(intel_crtc,
-				     &pipe_config->fdi_m_n, NULL);
+		intel_cpu_transcoder_set_m_n(pipe_config,
+					     &pipe_config->fdi_m_n, NULL);
 	}
 
 	if (!transcoder_is_dsi(cpu_transcoder))
@@ -6070,7 +6070,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 		return;
 
 	if (intel_crtc_has_dp_encoder(pipe_config))
-		intel_dp_set_m_n(intel_crtc, M1_N1);
+		intel_dp_set_m_n(pipe_config, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
 	intel_set_pipe_src_size(pipe_config);
@@ -6140,7 +6140,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 	i9xx_set_pll_dividers(pipe_config);
 
 	if (intel_crtc_has_dp_encoder(pipe_config))
-		intel_dp_set_m_n(intel_crtc, M1_N1);
+		intel_dp_set_m_n(pipe_config, M1_N1);
 
 	intel_set_pipe_timings(pipe_config);
 	intel_set_pipe_src_size(pipe_config);
@@ -6865,12 +6865,12 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv, enum pipe
 	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
 }
 
-static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
-					 struct intel_link_m_n *m_n)
+static void intel_pch_transcoder_set_m_n(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	int pipe = crtc->pipe;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct intel_link_m_n *m_n = &crtc_state->dp_m_n;
+	enum pipe pipe = crtc->pipe;
 
 	I915_WRITE(PCH_TRANS_DATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
 	I915_WRITE(PCH_TRANS_DATA_N1(pipe), m_n->gmch_n);
@@ -6878,13 +6878,14 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
 	I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
 }
 
-static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
-					 struct intel_link_m_n *m_n,
-					 struct intel_link_m_n *m2_n2)
+static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
+					 const struct intel_link_m_n *m_n,
+					 const struct intel_link_m_n *m2_n2)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	int pipe = crtc->pipe;
-	enum transcoder transcoder = crtc->config->cpu_transcoder;
+	enum pipe pipe = crtc->pipe;
+	enum transcoder transcoder = crtc_state->cpu_transcoder;
 
 	if (INTEL_GEN(dev_priv) >= 5) {
 		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
@@ -6896,7 +6897,7 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 		 * registers are not unnecessarily accessed).
 		 */
 		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
-		    INTEL_GEN(dev_priv) < 8) && crtc->config->has_drrs) {
+		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
 			I915_WRITE(PIPE_DATA_M2(transcoder),
 					TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
 			I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
@@ -6911,29 +6912,29 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 	}
 }
 
-void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
+void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, enum link_m_n_set m_n)
 {
-	struct intel_link_m_n *dp_m_n, *dp_m2_n2 = NULL;
+	const struct intel_link_m_n *dp_m_n, *dp_m2_n2 = NULL;
 
 	if (m_n == M1_N1) {
-		dp_m_n = &crtc->config->dp_m_n;
-		dp_m2_n2 = &crtc->config->dp_m2_n2;
+		dp_m_n = &crtc_state->dp_m_n;
+		dp_m2_n2 = &crtc_state->dp_m2_n2;
 	} else if (m_n == M2_N2) {
 
 		/*
 		 * M2_N2 registers are not supported. Hence m2_n2 divider value
 		 * needs to be programmed into M1_N1.
 		 */
-		dp_m_n = &crtc->config->dp_m2_n2;
+		dp_m_n = &crtc_state->dp_m2_n2;
 	} else {
 		DRM_ERROR("Unsupported divider value\n");
 		return;
 	}
 
-	if (crtc->config->has_pch_encoder)
-		intel_pch_transcoder_set_m_n(crtc, &crtc->config->dp_m_n);
+	if (crtc_state->has_pch_encoder)
+		intel_pch_transcoder_set_m_n(crtc_state);
 	else
-		intel_cpu_transcoder_set_m_n(crtc, dp_m_n, dp_m2_n2);
+		intel_cpu_transcoder_set_m_n(crtc_state, dp_m_n, dp_m2_n2);
 }
 
 static void vlv_compute_dpll(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 19f0c3f59cbe..21bb7c8cb721 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6082,10 +6082,10 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,
 	if (INTEL_GEN(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) {
 		switch (index) {
 		case DRRS_HIGH_RR:
-			intel_dp_set_m_n(intel_crtc, M1_N1);
+			intel_dp_set_m_n(crtc_state, M1_N1);
 			break;
 		case DRRS_LOW_RR:
-			intel_dp_set_m_n(intel_crtc, M2_N2);
+			intel_dp_set_m_n(crtc_state, M2_N2);
 			break;
 		case DRRS_MAX_RR:
 		default:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 43190c6e9ef2..9d29c414628b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1627,7 +1627,8 @@ void gen9_enable_dc5(struct drm_i915_private *dev_priv);
 unsigned int skl_cdclk_get_vco(unsigned int freq);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
 		      struct intel_crtc_state *pipe_config);
-void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
+void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
+		      enum link_m_n_set m_n);
 int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
 bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
 			struct dpll *best_clock);
-- 
2.19.0

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

[PATCH 03/10] drm/i915: Remove crtc->config references in vlv_prepare_pll

[Maarten Lankhorst]

We already have a perfectly nice pipe_config, use that instead.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d3eefa6ec65..bb16a9acf117 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7033,8 +7033,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	/* Set HBR and RBR LPF coefficients */
 	if (pipe_config->port_clock == 162000 ||
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
+	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_ANALOG) ||
+	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
 		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
 				 0x009f0003);
 	else
@@ -7061,7 +7061,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
 	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
-	if (intel_crtc_has_dp_encoder(crtc->config))
+	if (intel_crtc_has_dp_encoder(pipe_config))
 		coreclk |= 0x01000000;
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
 
-- 
2.19.0

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

[PATCH 04/10] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n

[Maarten Lankhorst]

has_drrs is a flag we can't read out. We set it when seamless DRRS is
enabled in pipe_config, so intel_dump_pipe_config() and
intel_pipe_config_compare() will continue to do the right thing when
has_drrs is set on the real state.

This removes one more dereference of crtc->config.
While at it, fixup the comment and also read out M2_N2 for CHV, since
we program it in the set_m_n function.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bb16a9acf117..7812fab31646 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6893,8 +6893,8 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
 		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
 		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
 		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
-		 * for gen < 8) and if DRRS is supported (to make sure the
-		 * registers are not unnecessarily accessed).
+		 * for gen < 8 and CHV) and if DRRS is supported (to make sure
+		 * the registers are not unnecessarily accessed).
 		 */
 		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
 		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
@@ -8747,11 +8747,10 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
 		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
 			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
 		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
-		 * gen < 8) and if DRRS is supported (to make sure the
-		 * registers are not unnecessarily read).
+		 * gen < 8 and CHV).
 		 */
-		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
-			crtc->config->has_drrs) {
+		if (m2_n2 && (INTEL_GEN(dev_priv) < 8 ||
+			      IS_CHERRYVIEW(dev_priv))) {
 			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
 			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
 			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
-- 
2.19.0

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

[PATCH 05/10] drm/i915: Pass crtc_state to update_scanline_offset

[Maarten Lankhorst]

No need to look at crtc->config when we have crtc_state in the caller.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7812fab31646..eaf522ef2927 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12127,8 +12127,9 @@ intel_modeset_verify_disabled(struct drm_device *dev,
 	verify_disabled_dpll_state(dev);
 }
 
-static void update_scanline_offset(struct intel_crtc *crtc)
+static void update_scanline_offset(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);
 
 	/*
@@ -12159,7 +12160,7 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 	 * answer that's slightly in the future.
 	 */
 	if (IS_GEN2(dev_priv)) {
-		const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
+		const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
 		int vtotal;
 
 		vtotal = adjusted_mode->crtc_vtotal;
@@ -12168,7 +12169,7 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 
 		crtc->scanline_offset = vtotal - 1;
 	} else if (HAS_DDI(dev_priv) &&
-		   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
+		   intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		crtc->scanline_offset = 2;
 	} else
 		crtc->scanline_offset = 1;
@@ -12522,7 +12523,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
 						 to_intel_plane(crtc->primary));
 
 	if (modeset) {
-		update_scanline_offset(intel_crtc);
+		update_scanline_offset(pipe_config);
 		dev_priv->display.crtc_enable(pipe_config, state);
 
 		/* vblanks work again, re-enable pipe CRC. */
@@ -15868,7 +15869,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 			drm_calc_timestamping_constants(&crtc->base,
 							&crtc_state->base.adjusted_mode);
-			update_scanline_offset(crtc);
+			update_scanline_offset(crtc_state);
 		}
 
 		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
-- 
2.19.0

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

[PATCH 06/10] drm/i915: Remove crtc->config dereferences in intel_sanitize_crtc

[Maarten Lankhorst]

We know the crtc is idle because we're at the beginning of sanitization,
so just dereference crtc->state instead.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eaf522ef2927..19d7714df021 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15565,7 +15565,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
 	/* Clear any frame start delays used for debugging left by the BIOS */
 	if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) {
@@ -15575,7 +15576,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
 	}
 
-	if (crtc->active) {
+	if (crtc_state->base.active) {
 		struct intel_plane *plane;
 
 		/* Disable everything but the primary plane */
@@ -15591,10 +15592,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 
 	/* Adjust the state of the output pipe according to whether we
 	 * have active connectors/encoders. */
-	if (crtc->active && !intel_crtc_has_encoders(crtc))
+	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
 		intel_crtc_disable_noatomic(&crtc->base, ctx);
 
-	if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) {
+	if (crtc_state->base.active || HAS_GMCH_DISPLAY(dev_priv)) {
 		/*
 		 * We start out with underrun reporting disabled to avoid races.
 		 * For correct bookkeeping mark this on active crtcs.
-- 
2.19.0

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

[PATCH 07/10] drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state

[Maarten Lankhorst]

The CRTC is idle at this point, so we can dereference crtc->state safely.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 19d7714df021..cbe70bc4d02d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15934,6 +15934,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
+	struct intel_crtc_state *crtc_state;
 	struct intel_encoder *encoder;
 	int i;
 
@@ -15952,7 +15953,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	for_each_intel_crtc(&dev_priv->drm, crtc) {
 		drm_crtc_vblank_reset(&crtc->base);
 
-		if (crtc->active)
+		if (crtc->base.state->active)
 			drm_crtc_vblank_on(&crtc->base);
 	}
 
@@ -15961,9 +15962,10 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	for_each_intel_encoder(dev, encoder)
 		intel_sanitize_encoder(encoder);
 
-	for_each_intel_crtc(&dev_priv->drm, crtc) {
+	for_each_intel_crtc(dev, crtc) {
+		crtc_state = to_intel_crtc_state(crtc->base.state);
 		intel_sanitize_crtc(crtc, ctx);
-		intel_dump_pipe_config(crtc, crtc->config,
+		intel_dump_pipe_config(crtc, crtc_state,
 				       "[setup_hw_state]");
 	}
 
@@ -15997,7 +15999,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
 	for_each_intel_crtc(dev, crtc) {
 		u64 put_domains;
 
-		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc->config);
+		crtc_state = to_intel_crtc_state(crtc->base.state);
+		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc_state);
 		if (WARN_ON(put_domains))
 			modeset_put_power_domains(dev_priv, put_domains);
 	}
-- 
2.19.0

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

[PATCH 08/10] drm/i915: Pass crtc_state to lpt_program_iclkip

[Maarten Lankhorst]

Instead of derferencing crtc->config, look at crtc_state.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cbe70bc4d02d..ad1694c4d947 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4494,10 +4494,11 @@ void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
 }
 
 /* Program iCLKIP clock to the desired frequency */
-static void lpt_program_iclkip(struct intel_crtc *crtc)
+static void lpt_program_iclkip(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);
-	int clock = crtc->config->base.adjusted_mode.crtc_clock;
+	int clock = crtc_state->base.adjusted_mode.crtc_clock;
 	u32 divsel, phaseinc, auxdiv, phasedir = 0;
 	u32 temp;
 
@@ -4806,7 +4807,7 @@ static void lpt_pch_enable(const struct intel_atomic_state *state,
 
 	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
 
-	lpt_program_iclkip(crtc);
+	lpt_program_iclkip(crtc_state);
 
 	/* Set transcoder timing. */
 	ironlake_pch_transcoder_set_timings(crtc_state, PIPE_A);
-- 
2.19.0

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

[PATCH 09/10] drm/i915: Pass crtc_state to ivybridge_update_fdi_bc_bifurcation

[Maarten Lankhorst]

We have to look at crtc_state, so pass that instead.
Also cleanup the use of dev vs dev_priv, we really want to pass along
dev_priv.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad1694c4d947..ad2c207d18bb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4633,9 +4633,8 @@ static void ironlake_pch_transcoder_set_timings(const struct intel_crtc_state *c
 		   I915_READ(VSYNCSHIFT(cpu_transcoder)));
 }
 
-static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
+static void cpt_set_fdi_bc_bifurcation(struct drm_i915_private *dev_priv, bool enable)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	uint32_t temp;
 
 	temp = I915_READ(SOUTH_CHICKEN1);
@@ -4654,22 +4653,23 @@ static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
 	POSTING_READ(SOUTH_CHICKEN1);
 }
 
-static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
+static void ivybridge_update_fdi_bc_bifurcation(const struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = intel_crtc->base.dev;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	switch (intel_crtc->pipe) {
+	switch (crtc->pipe) {
 	case PIPE_A:
 		break;
 	case PIPE_B:
-		if (intel_crtc->config->fdi_lanes > 2)
-			cpt_set_fdi_bc_bifurcation(dev, false);
+		if (crtc_state->fdi_lanes > 2)
+			cpt_set_fdi_bc_bifurcation(dev_priv, false);
 		else
-			cpt_set_fdi_bc_bifurcation(dev, true);
+			cpt_set_fdi_bc_bifurcation(dev_priv, true);
 
 		break;
 	case PIPE_C:
-		cpt_set_fdi_bc_bifurcation(dev, true);
+		cpt_set_fdi_bc_bifurcation(dev_priv, true);
 
 		break;
 	default:
@@ -4726,7 +4726,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
 	assert_pch_transcoder_disabled(dev_priv, pipe);
 
 	if (IS_IVYBRIDGE(dev_priv))
-		ivybridge_update_fdi_bc_bifurcation(crtc);
+		ivybridge_update_fdi_bc_bifurcation(crtc_state);
 
 	/* Write the TU size bits before fdi link training, so that error
 	 * detection works. */
-- 
2.19.0

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

[PATCH 10/10] drm/i915: Remove crtc->active from crtc_enable callbacks

[Maarten Lankhorst]

Now that most of the driver is atomic, we no longer need to worry
about setting crtc->active right before actually enabling the pipe.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ad2c207d18bb..0e4bdd5c337e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5585,9 +5585,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 	struct intel_atomic_state *old_intel_state =
 		to_intel_atomic_state(old_state);
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	/*
 	 * Sometimes spurious CPU pipe underruns happen during FDI
 	 * training, at least with VGA+HDMI cloning. Suppress them.
@@ -5617,8 +5614,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	ironlake_set_pipeconf(pipe_config);
 
-	intel_crtc->active = true;
-
 	intel_encoders_pre_enable(crtc, pipe_config, old_state);
 
 	if (pipe_config->has_pch_encoder) {
@@ -5715,9 +5710,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	bool psl_clkgate_wa;
 	u32 pipe_chicken;
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
 
 	if (pipe_config->shared_dpll)
@@ -5754,8 +5746,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_color_set_csc(&pipe_config->base);
 
-	intel_crtc->active = true;
-
 	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
 	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
 			 pipe_config->pch_pfit.enabled;
@@ -6067,9 +6057,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	if (intel_crtc_has_dp_encoder(pipe_config))
 		intel_dp_set_m_n(pipe_config, M1_N1);
 
@@ -6085,8 +6072,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	intel_color_set_csc(&pipe_config->base);
 
-	intel_crtc->active = true;
-
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
 	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
@@ -6135,9 +6120,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	i9xx_set_pll_dividers(pipe_config);
 
 	if (intel_crtc_has_dp_encoder(pipe_config))
@@ -6148,8 +6130,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
 
 	i9xx_set_pipeconf(pipe_config);
 
-	intel_crtc->active = true;
-
 	if (!IS_GEN2(dev_priv))
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
@@ -12525,6 +12505,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
 
 	if (modeset) {
 		update_scanline_offset(pipe_config);
+		intel_crtc->active = true;
 		dev_priv->display.crtc_enable(pipe_config, state);
 
 		/* vblanks work again, re-enable pipe CRC. */
-- 
2.19.0

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

✗ Fi.CI.BAT: failure for drm/i915: Remove low hanging crtc->config fruit, part 2.

[Patchwork]

== Series Details ==

Series: drm/i915: Remove low hanging crtc->config fruit, part 2.
URL   : https://patchwork.freedesktop.org/series/50856/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4969 -> Patchwork_10423 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50856/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@debugfs_test@read_all_entries:
      fi-hsw-4770r:       PASS -> DMESG-WARN +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#105128, fdo#107139)

    igt@kms_frontbuffer_tracking@basic:
      {fi-icl-u2}:        SKIP -> FAIL (fdo#103167)
      fi-glk-j4005:       PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       INCOMPLETE (fdo#107187, fdo#108126) -> PASS

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       WARN -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107187 https://bugs.freedesktop.org/show_bug.cgi?id=107187
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126


== Participating hosts (44 -> 41) ==

  Missing    (3): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_4969 -> Patchwork_10423

  CI_DRM_4969: 1121d2889e57dedacc0885deaaa9de614832e62f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4673: 54cb1aeb4e50dea9f3abae632e317875d147c4ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10423: 518326da7b388dc590bb100447a53ebd36766cdc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

518326da7b38 drm/i915: Remove crtc->active from crtc_enable callbacks
a294d3a692c3 drm/i915: Pass crtc_state to ivybridge_update_fdi_bc_bifurcation
8847505f3a0d drm/i915: Pass crtc_state to lpt_program_iclkip
dda83f80db38 drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state
5ec681b34f6c drm/i915: Remove crtc->config dereferences in intel_sanitize_crtc
1b3d07b4919a drm/i915: Pass crtc_state to update_scanline_offset
e8010cbd4f8e drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n
ebe23015acff drm/i915: Remove crtc->config references in vlv_prepare_pll
a508491f6269 drm/i915: Make intel_dp_set_m_n take crtc_state
a739110adcb3 drm/i915: Remove crtc->config dereference from drrs_ctl

== Logs ==

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

Re: [PATCH 02/10] drm/i915: Make intel_dp_set_m_n take crtc_state

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:49PM +0200, Maarten Lankhorst wrote:
> Another user of crtc->config gone. The functions it calls also
> needed crtc->config, so convert those as well.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_dp.c      |  4 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>  3 files changed, 35 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c3fd37a9fd49..3d3eefa6ec65 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -143,9 +143,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *ifb,
>  				  struct drm_mode_fb_cmd2 *mode_cmd);
>  static void intel_set_pipe_timings(const struct intel_crtc_state *crtc_state);
>  static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
> -static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> -					 struct intel_link_m_n *m_n,
> -					 struct intel_link_m_n *m2_n2);
> +static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
> +					 const struct intel_link_m_n *m_n,
> +					 const struct intel_link_m_n *m2_n2);
>  static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state);
>  static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state);
>  static void haswell_set_pipeconf(const struct intel_crtc_state *crtc_state);
> @@ -5604,14 +5604,14 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  		intel_prepare_shared_dpll(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> -		intel_dp_set_m_n(intel_crtc, M1_N1);
> +		intel_dp_set_m_n(pipe_config, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
>  	intel_set_pipe_src_size(pipe_config);
>  
>  	if (pipe_config->has_pch_encoder) {
> -		intel_cpu_transcoder_set_m_n(intel_crtc,
> -				     &pipe_config->fdi_m_n, NULL);
> +		intel_cpu_transcoder_set_m_n(pipe_config,
> +					     &pipe_config->fdi_m_n, NULL);
>  	}
>  
>  	ironlake_set_pipeconf(pipe_config);
> @@ -5728,7 +5728,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> -		intel_dp_set_m_n(intel_crtc, M1_N1);
> +		intel_dp_set_m_n(pipe_config, M1_N1);
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
>  		intel_set_pipe_timings(pipe_config);
> @@ -5742,8 +5742,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	}
>  
>  	if (pipe_config->has_pch_encoder) {
> -		intel_cpu_transcoder_set_m_n(intel_crtc,
> -				     &pipe_config->fdi_m_n, NULL);
> +		intel_cpu_transcoder_set_m_n(pipe_config,
> +					     &pipe_config->fdi_m_n, NULL);
>  	}
>  
>  	if (!transcoder_is_dsi(cpu_transcoder))
> @@ -6070,7 +6070,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  		return;
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> -		intel_dp_set_m_n(intel_crtc, M1_N1);
> +		intel_dp_set_m_n(pipe_config, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
>  	intel_set_pipe_src_size(pipe_config);
> @@ -6140,7 +6140,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	i9xx_set_pll_dividers(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> -		intel_dp_set_m_n(intel_crtc, M1_N1);
> +		intel_dp_set_m_n(pipe_config, M1_N1);
>  
>  	intel_set_pipe_timings(pipe_config);
>  	intel_set_pipe_src_size(pipe_config);
> @@ -6865,12 +6865,12 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv, enum pipe
>  	vlv_dpio_write(dev_priv, pipe, VLV_REF_DW13, reg_val);
>  }
>  
> -static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
> -					 struct intel_link_m_n *m_n)
> +static void intel_pch_transcoder_set_m_n(const struct intel_crtc_state *crtc_state)

I'd probably still pass the m_n struct in explicity here as there's
nothing in the function name that says "this is for dp only".

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

>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int pipe = crtc->pipe;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct intel_link_m_n *m_n = &crtc_state->dp_m_n;
> +	enum pipe pipe = crtc->pipe;
>  
>  	I915_WRITE(PCH_TRANS_DATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
>  	I915_WRITE(PCH_TRANS_DATA_N1(pipe), m_n->gmch_n);
> @@ -6878,13 +6878,14 @@ static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
>  	I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
>  }
>  
> -static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> -					 struct intel_link_m_n *m_n,
> -					 struct intel_link_m_n *m2_n2)
> +static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
> +					 const struct intel_link_m_n *m_n,
> +					 const struct intel_link_m_n *m2_n2)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	int pipe = crtc->pipe;
> -	enum transcoder transcoder = crtc->config->cpu_transcoder;
> +	enum pipe pipe = crtc->pipe;
> +	enum transcoder transcoder = crtc_state->cpu_transcoder;
>  
>  	if (INTEL_GEN(dev_priv) >= 5) {
>  		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
> @@ -6896,7 +6897,7 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  		 * registers are not unnecessarily accessed).
>  		 */
>  		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
> -		    INTEL_GEN(dev_priv) < 8) && crtc->config->has_drrs) {
> +		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
>  			I915_WRITE(PIPE_DATA_M2(transcoder),
>  					TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
>  			I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
> @@ -6911,29 +6912,29 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
>  	}
>  }
>  
> -void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n)
> +void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state, enum link_m_n_set m_n)
>  {
> -	struct intel_link_m_n *dp_m_n, *dp_m2_n2 = NULL;
> +	const struct intel_link_m_n *dp_m_n, *dp_m2_n2 = NULL;
>  
>  	if (m_n == M1_N1) {
> -		dp_m_n = &crtc->config->dp_m_n;
> -		dp_m2_n2 = &crtc->config->dp_m2_n2;
> +		dp_m_n = &crtc_state->dp_m_n;
> +		dp_m2_n2 = &crtc_state->dp_m2_n2;
>  	} else if (m_n == M2_N2) {
>  
>  		/*
>  		 * M2_N2 registers are not supported. Hence m2_n2 divider value
>  		 * needs to be programmed into M1_N1.
>  		 */
> -		dp_m_n = &crtc->config->dp_m2_n2;
> +		dp_m_n = &crtc_state->dp_m2_n2;
>  	} else {
>  		DRM_ERROR("Unsupported divider value\n");
>  		return;
>  	}
>  
> -	if (crtc->config->has_pch_encoder)
> -		intel_pch_transcoder_set_m_n(crtc, &crtc->config->dp_m_n);
> +	if (crtc_state->has_pch_encoder)
> +		intel_pch_transcoder_set_m_n(crtc_state);
>  	else
> -		intel_cpu_transcoder_set_m_n(crtc, dp_m_n, dp_m2_n2);
> +		intel_cpu_transcoder_set_m_n(crtc_state, dp_m_n, dp_m2_n2);
>  }
>  
>  static void vlv_compute_dpll(struct intel_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 19f0c3f59cbe..21bb7c8cb721 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6082,10 +6082,10 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,
>  	if (INTEL_GEN(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) {
>  		switch (index) {
>  		case DRRS_HIGH_RR:
> -			intel_dp_set_m_n(intel_crtc, M1_N1);
> +			intel_dp_set_m_n(crtc_state, M1_N1);
>  			break;
>  		case DRRS_LOW_RR:
> -			intel_dp_set_m_n(intel_crtc, M2_N2);
> +			intel_dp_set_m_n(crtc_state, M2_N2);
>  			break;
>  		case DRRS_MAX_RR:
>  		default:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 43190c6e9ef2..9d29c414628b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1627,7 +1627,8 @@ void gen9_enable_dc5(struct drm_i915_private *dev_priv);
>  unsigned int skl_cdclk_get_vco(unsigned int freq);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
>  		      struct intel_crtc_state *pipe_config);
> -void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> +void intel_dp_set_m_n(const struct intel_crtc_state *crtc_state,
> +		      enum link_m_n_set m_n);
>  int intel_dotclock_calculate(int link_freq, const struct intel_link_m_n *m_n);
>  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
>  			struct dpll *best_clock);
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 03/10] drm/i915: Remove crtc->config references in vlv_prepare_pll

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:50PM +0200, Maarten Lankhorst wrote:
> We already have a perfectly nice pipe_config, use that instead.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3d3eefa6ec65..bb16a9acf117 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7033,8 +7033,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>  
>  	/* Set HBR and RBR LPF coefficients */
>  	if (pipe_config->port_clock == 162000 ||
> -	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
> -	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
> +	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_ANALOG) ||
> +	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>  		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
>  				 0x009f0003);
>  	else
> @@ -7061,7 +7061,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>  
>  	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
>  	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
> -	if (intel_crtc_has_dp_encoder(crtc->config))
> +	if (intel_crtc_has_dp_encoder(pipe_config))
>  		coreclk |= 0x01000000;
>  	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
>  
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 04/10] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:51PM +0200, Maarten Lankhorst wrote:
> has_drrs is a flag we can't read out. We set it when seamless DRRS is
> enabled in pipe_config, so intel_dump_pipe_config() and
> intel_pipe_config_compare() will continue to do the right thing when
> has_drrs is set on the real state.
> 
> This removes one more dereference of crtc->config.
> While at it, fixup the comment and also read out M2_N2 for CHV, since
> we program it in the set_m_n function.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bb16a9acf117..7812fab31646 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6893,8 +6893,8 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
>  		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>  		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>  		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
> -		 * for gen < 8) and if DRRS is supported (to make sure the
> -		 * registers are not unnecessarily accessed).
> +		 * for gen < 8 and CHV) and if DRRS is supported (to make sure
> +		 * the registers are not unnecessarily accessed).
>  		 */
>  		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
>  		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {

I think what I'd really like to see is splitting the m/n and m2/n2 stuff
into two distinct pieces, and when we don't use drrs we could just
set m2_n2 = m_n, and program the everything exactly the same way as
when drrs is enabled.

Also we could nuke that "m/n vs. m2/n2" enum and just pass the correct
struct to foo_set_m_n() directly.

> @@ -8747,11 +8747,10 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
>  		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
>  			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
>  		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
> -		 * gen < 8) and if DRRS is supported (to make sure the
> -		 * registers are not unnecessarily read).
> +		 * gen < 8 and CHV).
>  		 */
> -		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
> -			crtc->config->has_drrs) {
> +		if (m2_n2 && (INTEL_GEN(dev_priv) < 8 ||
> +			      IS_CHERRYVIEW(dev_priv))) {

I think this is maybe the third installment of this check. Could
perhaps use a small has_m2_n2() helper to avoid the repetition?

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

>  			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
>  			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
>  			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 05/10] drm/i915: Pass crtc_state to update_scanline_offset

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:52PM +0200, Maarten Lankhorst wrote:
> No need to look at crtc->config when we have crtc_state in the caller.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7812fab31646..eaf522ef2927 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12127,8 +12127,9 @@ intel_modeset_verify_disabled(struct drm_device *dev,
>  	verify_disabled_dpll_state(dev);
>  }
>  
> -static void update_scanline_offset(struct intel_crtc *crtc)
> +static void update_scanline_offset(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);
>  
>  	/*
> @@ -12159,7 +12160,7 @@ static void update_scanline_offset(struct intel_crtc *crtc)
>  	 * answer that's slightly in the future.
>  	 */
>  	if (IS_GEN2(dev_priv)) {
> -		const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
> +		const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode;
>  		int vtotal;
>  
>  		vtotal = adjusted_mode->crtc_vtotal;
> @@ -12168,7 +12169,7 @@ static void update_scanline_offset(struct intel_crtc *crtc)
>  
>  		crtc->scanline_offset = vtotal - 1;
>  	} else if (HAS_DDI(dev_priv) &&
> -		   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
> +		   intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
>  		crtc->scanline_offset = 2;
>  	} else
>  		crtc->scanline_offset = 1;
> @@ -12522,7 +12523,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
>  						 to_intel_plane(crtc->primary));
>  
>  	if (modeset) {
> -		update_scanline_offset(intel_crtc);
> +		update_scanline_offset(pipe_config);
>  		dev_priv->display.crtc_enable(pipe_config, state);
>  
>  		/* vblanks work again, re-enable pipe CRC. */
> @@ -15868,7 +15869,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  			drm_calc_timestamping_constants(&crtc->base,
>  							&crtc_state->base.adjusted_mode);
> -			update_scanline_offset(crtc);
> +			update_scanline_offset(crtc_state);
>  		}
>  
>  		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 06/10] drm/i915: Remove crtc->config dereferences in intel_sanitize_crtc

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:53PM +0200, Maarten Lankhorst wrote:
> We know the crtc is idle because we're at the beginning of sanitization,
> so just dereference crtc->state instead.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eaf522ef2927..19d7714df021 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15565,7 +15565,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> +	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
>  	/* Clear any frame start delays used for debugging left by the BIOS */
>  	if (crtc->active && !transcoder_is_dsi(cpu_transcoder)) {
> @@ -15575,7 +15576,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>  	}
>  
> -	if (crtc->active) {
> +	if (crtc_state->base.active) {
>  		struct intel_plane *plane;
>  
>  		/* Disable everything but the primary plane */
> @@ -15591,10 +15592,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  
>  	/* Adjust the state of the output pipe according to whether we
>  	 * have active connectors/encoders. */
> -	if (crtc->active && !intel_crtc_has_encoders(crtc))
> +	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
>  		intel_crtc_disable_noatomic(&crtc->base, ctx);
>  
> -	if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) {
> +	if (crtc_state->base.active || HAS_GMCH_DISPLAY(dev_priv)) {
>  		/*
>  		 * We start out with underrun reporting disabled to avoid races.
>  		 * For correct bookkeeping mark this on active crtcs.
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 07/10] drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:54PM +0200, Maarten Lankhorst wrote:
> The CRTC is idle at this point, so we can dereference crtc->state safely.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 19d7714df021..cbe70bc4d02d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15934,6 +15934,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *crtc;
> +	struct intel_crtc_state *crtc_state;

Can be moved into narrower scope.

>  	struct intel_encoder *encoder;
>  	int i;
>  
> @@ -15952,7 +15953,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_crtc(&dev_priv->drm, crtc) {
>  		drm_crtc_vblank_reset(&crtc->base);
>  
> -		if (crtc->active)
> +		if (crtc->base.state->active)
>  			drm_crtc_vblank_on(&crtc->base);
>  	}
>  
> @@ -15961,9 +15962,10 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_encoder(dev, encoder)
>  		intel_sanitize_encoder(encoder);
>  
> -	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +	for_each_intel_crtc(dev, crtc) {

I'd keep the dev_priv->drm, because we should just change the function
to take the dev_priv directly.

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

> +		crtc_state = to_intel_crtc_state(crtc->base.state);
>  		intel_sanitize_crtc(crtc, ctx);
> -		intel_dump_pipe_config(crtc, crtc->config,
> +		intel_dump_pipe_config(crtc, crtc_state,
>  				       "[setup_hw_state]");
>  	}
>  
> @@ -15997,7 +15999,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for_each_intel_crtc(dev, crtc) {
>  		u64 put_domains;
>  
> -		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc->config);
> +		crtc_state = to_intel_crtc_state(crtc->base.state);
> +		put_domains = modeset_get_crtc_power_domains(&crtc->base, crtc_state);
>  		if (WARN_ON(put_domains))
>  			modeset_put_power_domains(dev_priv, put_domains);
>  	}
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 08/10] drm/i915: Pass crtc_state to lpt_program_iclkip

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:55PM +0200, Maarten Lankhorst wrote:
> Instead of derferencing crtc->config, look at crtc_state.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cbe70bc4d02d..ad1694c4d947 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4494,10 +4494,11 @@ void lpt_disable_iclkip(struct drm_i915_private *dev_priv)
>  }
>  
>  /* Program iCLKIP clock to the desired frequency */
> -static void lpt_program_iclkip(struct intel_crtc *crtc)
> +static void lpt_program_iclkip(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);
> -	int clock = crtc->config->base.adjusted_mode.crtc_clock;
> +	int clock = crtc_state->base.adjusted_mode.crtc_clock;
>  	u32 divsel, phaseinc, auxdiv, phasedir = 0;
>  	u32 temp;
>  
> @@ -4806,7 +4807,7 @@ static void lpt_pch_enable(const struct intel_atomic_state *state,
>  
>  	assert_pch_transcoder_disabled(dev_priv, PIPE_A);
>  
> -	lpt_program_iclkip(crtc);
> +	lpt_program_iclkip(crtc_state);
>  
>  	/* Set transcoder timing. */
>  	ironlake_pch_transcoder_set_timings(crtc_state, PIPE_A);
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 09/10] drm/i915: Pass crtc_state to ivybridge_update_fdi_bc_bifurcation

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:56PM +0200, Maarten Lankhorst wrote:
> We have to look at crtc_state, so pass that instead.
> Also cleanup the use of dev vs dev_priv, we really want to pass along
> dev_priv.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad1694c4d947..ad2c207d18bb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4633,9 +4633,8 @@ static void ironlake_pch_transcoder_set_timings(const struct intel_crtc_state *c
>  		   I915_READ(VSYNCSHIFT(cpu_transcoder)));
>  }
>  
> -static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
> +static void cpt_set_fdi_bc_bifurcation(struct drm_i915_private *dev_priv, bool enable)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	uint32_t temp;
>  
>  	temp = I915_READ(SOUTH_CHICKEN1);
> @@ -4654,22 +4653,23 @@ static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
>  	POSTING_READ(SOUTH_CHICKEN1);
>  }
>  
> -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc)
> +static void ivybridge_update_fdi_bc_bifurcation(const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = intel_crtc->base.dev;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  
> -	switch (intel_crtc->pipe) {
> +	switch (crtc->pipe) {
>  	case PIPE_A:
>  		break;
>  	case PIPE_B:
> -		if (intel_crtc->config->fdi_lanes > 2)
> -			cpt_set_fdi_bc_bifurcation(dev, false);
> +		if (crtc_state->fdi_lanes > 2)
> +			cpt_set_fdi_bc_bifurcation(dev_priv, false);
>  		else
> -			cpt_set_fdi_bc_bifurcation(dev, true);
> +			cpt_set_fdi_bc_bifurcation(dev_priv, true);
>  
>  		break;
>  	case PIPE_C:
> -		cpt_set_fdi_bc_bifurcation(dev, true);
> +		cpt_set_fdi_bc_bifurcation(dev_priv, true);
>  
>  		break;
>  	default:
> @@ -4726,7 +4726,7 @@ static void ironlake_pch_enable(const struct intel_atomic_state *state,
>  	assert_pch_transcoder_disabled(dev_priv, pipe);
>  
>  	if (IS_IVYBRIDGE(dev_priv))
> -		ivybridge_update_fdi_bc_bifurcation(crtc);
> +		ivybridge_update_fdi_bc_bifurcation(crtc_state);
>  
>  	/* Write the TU size bits before fdi link training, so that error
>  	 * detection works. */
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 10/10] drm/i915: Remove crtc->active from crtc_enable callbacks

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:57PM +0200, Maarten Lankhorst wrote:
> Now that most of the driver is atomic, we no longer need to worry
> about setting crtc->active right before actually enabling the pipe.

Hmm. I think we need at least that wait_for_vblank_if_active() change
we discussed. And maybe there's something in the watermark code that
depends on this being more or less accurate indication of thw hw state.
That probably needs some actual thought.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 21 +--------------------
>  1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad2c207d18bb..0e4bdd5c337e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5585,9 +5585,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_atomic_state *old_intel_state =
>  		to_intel_atomic_state(old_state);
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	/*
>  	 * Sometimes spurious CPU pipe underruns happen during FDI
>  	 * training, at least with VGA+HDMI cloning. Suppress them.
> @@ -5617,8 +5614,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	ironlake_set_pipeconf(pipe_config);
>  
> -	intel_crtc->active = true;
> -
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
>  	if (pipe_config->has_pch_encoder) {
> @@ -5715,9 +5710,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	bool psl_clkgate_wa;
>  	u32 pipe_chicken;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
>  	if (pipe_config->shared_dpll)
> @@ -5754,8 +5746,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> -	intel_crtc->active = true;
> -
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
>  	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
>  			 pipe_config->pch_pfit.enabled;
> @@ -6067,9 +6057,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(pipe_config, M1_N1);
>  
> @@ -6085,8 +6072,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> -	intel_crtc->active = true;
> -
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> @@ -6135,9 +6120,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	i9xx_set_pll_dividers(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> @@ -6148,8 +6130,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	i9xx_set_pipeconf(pipe_config);
>  
> -	intel_crtc->active = true;
> -
>  	if (!IS_GEN2(dev_priv))
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  
> @@ -12525,6 +12505,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
>  
>  	if (modeset) {
>  		update_scanline_offset(pipe_config);
> +		intel_crtc->active = true;
>  		dev_priv->display.crtc_enable(pipe_config, state);
>  
>  		/* vblanks work again, re-enable pipe CRC. */
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 04/10] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n

[Maarten Lankhorst]

Op 11-10-18 om 13:56 schreef Ville Syrjälä:
> On Thu, Oct 11, 2018 at 12:04:51PM +0200, Maarten Lankhorst wrote:
>> has_drrs is a flag we can't read out. We set it when seamless DRRS is
>> enabled in pipe_config, so intel_dump_pipe_config() and
>> intel_pipe_config_compare() will continue to do the right thing when
>> has_drrs is set on the real state.
>>
>> This removes one more dereference of crtc->config.
>> While at it, fixup the comment and also read out M2_N2 for CHV, since
>> we program it in the set_m_n function.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index bb16a9acf117..7812fab31646 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6893,8 +6893,8 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
>>  		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>>  		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>>  		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
>> -		 * for gen < 8) and if DRRS is supported (to make sure the
>> -		 * registers are not unnecessarily accessed).
>> +		 * for gen < 8 and CHV) and if DRRS is supported (to make sure
>> +		 * the registers are not unnecessarily accessed).
>>  		 */
>>  		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
>>  		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
> I think what I'd really like to see is splitting the m/n and m2/n2 stuff
> into two distinct pieces, and when we don't use drrs we could just
> set m2_n2 = m_n, and program the everything exactly the same way as
> when drrs is enabled.
>
> Also we could nuke that "m/n vs. m2/n2" enum and just pass the correct
> struct to foo_set_m_n() directly.
>
>> @@ -8747,11 +8747,10 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
>>  		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
>>  			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
>>  		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
>> -		 * gen < 8) and if DRRS is supported (to make sure the
>> -		 * registers are not unnecessarily read).
>> +		 * gen < 8 and CHV).
>>  		 */
>> -		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
>> -			crtc->config->has_drrs) {
>> +		if (m2_n2 && (INTEL_GEN(dev_priv) < 8 ||
>> +			      IS_CHERRYVIEW(dev_priv))) {
> I think this is maybe the third installment of this check. Could
> perhaps use a small has_m2_n2() helper to avoid the repetition?
>
> Either way
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Hm looks like this is causing a unclaimed register read in debugfs. Could we limit reads/writes to eDP trasncoder?
>>  			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
>>  			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
>>  			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
>> -- 
>> 2.19.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

Re: [PATCH 04/10] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 08:17:49PM +0200, Maarten Lankhorst wrote:
> Op 11-10-18 om 13:56 schreef Ville Syrjälä:
> > On Thu, Oct 11, 2018 at 12:04:51PM +0200, Maarten Lankhorst wrote:
> >> has_drrs is a flag we can't read out. We set it when seamless DRRS is
> >> enabled in pipe_config, so intel_dump_pipe_config() and
> >> intel_pipe_config_compare() will continue to do the right thing when
> >> has_drrs is set on the real state.
> >>
> >> This removes one more dereference of crtc->config.
> >> While at it, fixup the comment and also read out M2_N2 for CHV, since
> >> we program it in the set_m_n function.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> >>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index bb16a9acf117..7812fab31646 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -6893,8 +6893,8 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
> >>  		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
> >>  		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> >>  		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
> >> -		 * for gen < 8) and if DRRS is supported (to make sure the
> >> -		 * registers are not unnecessarily accessed).
> >> +		 * for gen < 8 and CHV) and if DRRS is supported (to make sure
> >> +		 * the registers are not unnecessarily accessed).
> >>  		 */
> >>  		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
> >>  		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
> > I think what I'd really like to see is splitting the m/n and m2/n2 stuff
> > into two distinct pieces, and when we don't use drrs we could just
> > set m2_n2 = m_n, and program the everything exactly the same way as
> > when drrs is enabled.
> >
> > Also we could nuke that "m/n vs. m2/n2" enum and just pass the correct
> > struct to foo_set_m_n() directly.
> >
> >> @@ -8747,11 +8747,10 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
> >>  		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
> >>  			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> >>  		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
> >> -		 * gen < 8) and if DRRS is supported (to make sure the
> >> -		 * registers are not unnecessarily read).
> >> +		 * gen < 8 and CHV).
> >>  		 */
> >> -		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
> >> -			crtc->config->has_drrs) {
> >> +		if (m2_n2 && (INTEL_GEN(dev_priv) < 8 ||
> >> +			      IS_CHERRYVIEW(dev_priv))) {
> > I think this is maybe the third installment of this check. Could
> > perhaps use a small has_m2_n2() helper to avoid the repetition?
> >
> > Either way
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Hm looks like this is causing a unclaimed register read in debugfs. Could we limit reads/writes to eDP trasncoder?

That's not a thing on most platforms.

transcoder_has_m2_n2()
{
	if (IS_HASWELL)
		return trans == EDP;
	return IS_ILK || IS_SNB || IS_IVB || IS_VLV || IS_CHV;
}

?

Though intel_dp_set_drrs_state() only claims to support ivb+, even
though ilk/snb should work with the same code as ivb. Hmm, oh right
the fdi bits are a bit busted on those platforms. So drrs won't work
at least on the pch ports with the current code. I guess it might
work on port A eDP.

> >>  			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
> >>  			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
> >>  			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
> >> -- 
> >> 2.19.0
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

[PATCH] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n, v2.

[Maarten Lankhorst]

has_drrs is a flag we can't read out. We set it when seamless DRRS is
enabled in pipe_config, so intel_dump_pipe_config() and
intel_pipe_config_compare() will continue to do the right thing when
has_drrs is set on the real state.

This removes one more dereference of crtc->config.
While at it, fixup the comment and also read out M2_N2 for CHV, since
we program it in the set_m_n function.

Changes since v1:
- Only read out M2/N2 on platforms that support DRRS.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 64c1c6f8e0f4..1ca93cb89842 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6769,6 +6769,19 @@ static void intel_pch_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
 	I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
 }
 
+static bool transcoder_has_m2_n2(struct drm_i915_private *dev_priv,
+				 enum transcoder transcoder)
+{
+	if (IS_HASWELL(dev_priv))
+		return transcoder == TRANSCODER_EDP;
+
+	/*
+	 * Strictly speaking some registers are available before
+	 * gen7, but we only support DRRS on gen7+
+	 */
+	return IS_GEN7(dev_priv) || IS_CHERRYVIEW(dev_priv);
+}
+
 static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
 					 const struct intel_link_m_n *m_n,
 					 const struct intel_link_m_n *m2_n2)
@@ -6783,12 +6796,12 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
 		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
 		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
 		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
-		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
-		 * for gen < 8) and if DRRS is supported (to make sure the
-		 * registers are not unnecessarily accessed).
+		/*
+		 *  M2_N2 registers are set only if DRRS is supported
+		 * (to make sure the registers are not unnecessarily accessed).
 		 */
-		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
-		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
+		if (m2_n2 && crtc_state->has_drrs &&
+		    transcoder_has_m2_n2(dev_priv, transcoder)) {
 			I915_WRITE(PIPE_DATA_M2(transcoder),
 					TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
 			I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
@@ -8637,12 +8650,8 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
 		m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
 		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
 			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
-		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
-		 * gen < 8) and if DRRS is supported (to make sure the
-		 * registers are not unnecessarily read).
-		 */
-		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
-			crtc->config->has_drrs) {
+
+		if (m2_n2 && transcoder_has_m2_n2(dev_priv, transcoder)) {
 			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
 			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
 			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
-- 
2.19.1

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

✓ Fi.CI.BAT: success for drm/i915: Remove low hanging crtc->config fruit, part 2. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Remove low hanging crtc->config fruit, part 2. (rev2)
URL   : https://patchwork.freedesktop.org/series/50856/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4979 -> Patchwork_10454 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50856/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> DMESG-WARN (fdo#106612)
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107774, fdo#107859, fdo#107556)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@pm_rpm@module-reload:
      fi-skl-6600u:       PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-glk-j4005:       DMESG-WARN (fdo#106725, fdo#106248) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-icl-u:           INCOMPLETE (fdo#108315) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    igt@kms_flip@basic-plain-flip:
      fi-ilk-650:         DMESG-WARN (fdo#106387) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS +1

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#106612 https://bugs.freedesktop.org/show_bug.cgi?id=106612
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315


== Participating hosts (44 -> 47) ==

  Additional (7): fi-kbl-soraka fi-snb-2520m fi-ivb-3770 fi-pnv-d510 fi-kbl-7560u fi-byt-n2820 fi-snb-2600 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-apl-guc 


== Build changes ==

    * Linux: CI_DRM_4979 -> Patchwork_10454

  CI_DRM_4979: 2c411746783a4db33844f298ee88f0301cf0453e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10454: 87c5cb1713a6793a7bb09dba1815b7c75819c27e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

87c5cb1713a6 drm/i915: Remove crtc->active from crtc_enable callbacks
3dacbe4ccb19 drm/i915: Pass crtc_state to ivybridge_update_fdi_bc_bifurcation
74844c5eae16 drm/i915: Pass crtc_state to lpt_program_iclkip
f755a9532085 drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state
658ac819df32 drm/i915: Remove crtc->config dereferences in intel_sanitize_crtc
98f30dc9e3ed drm/i915: Pass crtc_state to update_scanline_offset
ac9321aee5f9 drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n, v2.
102eaff867d3 drm/i915: Remove crtc->config references in vlv_prepare_pll
6abb673fb52e drm/i915: Make intel_dp_set_m_n take crtc_state
f73fcaa0f6d0 drm/i915: Remove crtc->config dereference from drrs_ctl

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: Remove low hanging crtc->config fruit, part 2. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Remove low hanging crtc->config fruit, part 2. (rev2)
URL   : https://patchwork.freedesktop.org/series/50856/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4979_full -> Patchwork_10454_full =

== Summary - WARNING ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@kms_plane@pixel-format-pipe-a-planes:
      shard-snb:          SKIP -> PASS

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_cpu_reloc@full:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108073)

    igt@gem_exec_schedule@pi-ringfull-render:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106023, fdo#103665)

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-apl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-128x128-sliding:
      shard-apl:          PASS -> FAIL (fdo#103232) +2

    igt@kms_cursor_crc@cursor-256x85-sliding:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-apl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-render:
      shard-skl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-shrfb-fliptrack:
      shard-skl:          PASS -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
      shard-skl:          NOTRUN -> FAIL (fdo#105683)

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@psr-rgb565-draw-render:
      shard-skl:          SKIP -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
      shard-skl:          NOTRUN -> FAIL (fdo#108146)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-skl:          NOTRUN -> FAIL (fdo#99912)

    igt@kms_sysfs_edid_timing:
      shard-skl:          NOTRUN -> FAIL (fdo#100047)

    igt@prime_vgem@basic-busy-default:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538) +1

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-apl:          INCOMPLETE (fdo#103927, fdo#106886) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#106023, fdo#103665) -> PASS

    igt@gem_userptr_blits@readonly-unsync:
      shard-skl:          INCOMPLETE (fdo#108074) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> PASS

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_fbcon_fbt@fbc-suspend:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@pm_rpm@drm-resources-equal:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    igt@prime_busy@hang-render:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4979 -> Patchwork_10454

  CI_DRM_4979: 2c411746783a4db33844f298ee88f0301cf0453e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4674: 93871c6fb3c25e5d350c9faf36ded917174214de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10454: 87c5cb1713a6793a7bb09dba1815b7c75819c27e @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915: Always read out M2_N2 in intel_cpu_transcoder_get_m_n, v2.

[Ville Syrjälä]

On Mon, Oct 15, 2018 at 11:40:23AM +0200, Maarten Lankhorst wrote:
> has_drrs is a flag we can't read out. We set it when seamless DRRS is
> enabled in pipe_config, so intel_dump_pipe_config() and
> intel_pipe_config_compare() will continue to do the right thing when
> has_drrs is set on the real state.
> 
> This removes one more dereference of crtc->config.
> While at it, fixup the comment and also read out M2_N2 for CHV, since
> we program it in the set_m_n function.
> 
> Changes since v1:
> - Only read out M2/N2 on platforms that support DRRS.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 64c1c6f8e0f4..1ca93cb89842 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6769,6 +6769,19 @@ static void intel_pch_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
>  	I915_WRITE(PCH_TRANS_LINK_N1(pipe), m_n->link_n);
>  }
>  
> +static bool transcoder_has_m2_n2(struct drm_i915_private *dev_priv,
> +				 enum transcoder transcoder)
> +{
> +	if (IS_HASWELL(dev_priv))
> +		return transcoder == TRANSCODER_EDP;
> +
> +	/*
> +	 * Strictly speaking some registers are available before
> +	 * gen7, but we only support DRRS on gen7+
> +	 */
> +	return IS_GEN7(dev_priv) || IS_CHERRYVIEW(dev_priv);
> +}
> +
>  static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_state,
>  					 const struct intel_link_m_n *m_n,
>  					 const struct intel_link_m_n *m2_n2)
> @@ -6783,12 +6796,12 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
>  		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
>  		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>  		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> -		/* M2_N2 registers to be set only for gen < 8 (M2_N2 available
> -		 * for gen < 8) and if DRRS is supported (to make sure the
> -		 * registers are not unnecessarily accessed).
> +		/*
> +		 *  M2_N2 registers are set only if DRRS is supported
> +		 * (to make sure the registers are not unnecessarily accessed).
>  		 */
> -		if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
> -		    INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {
> +		if (m2_n2 && crtc_state->has_drrs &&
> +		    transcoder_has_m2_n2(dev_priv, transcoder)) {
>  			I915_WRITE(PIPE_DATA_M2(transcoder),
>  					TU_SIZE(m2_n2->tu) | m2_n2->gmch_m);
>  			I915_WRITE(PIPE_DATA_N2(transcoder), m2_n2->gmch_n);
> @@ -8637,12 +8650,8 @@ static void intel_cpu_transcoder_get_m_n(struct intel_crtc *crtc,
>  		m_n->gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
>  		m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
>  			    & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
> -		/* Read M2_N2 registers only for gen < 8 (M2_N2 available for
> -		 * gen < 8) and if DRRS is supported (to make sure the
> -		 * registers are not unnecessarily read).
> -		 */
> -		if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
> -			crtc->config->has_drrs) {
> +
> +		if (m2_n2 && transcoder_has_m2_n2(dev_priv, transcoder)) {
>  			m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
>  			m2_n2->link_n =	I915_READ(PIPE_LINK_N2(transcoder));
>  			m2_n2->gmch_m =	I915_READ(PIPE_DATA_M2(transcoder))
> -- 
> 2.19.1

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

Re: [PATCH 07/10] drm/i915: Remove crtc->config dereferences in intel_modeset_setup_hw_state

[Maarten Lankhorst]

Op 11-10-18 om 13:59 schreef Ville Syrjälä:
> On Thu, Oct 11, 2018 at 12:04:54PM +0200, Maarten Lankhorst wrote:
>> The CRTC is idle at this point, so we can dereference crtc->state safely.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 19d7714df021..cbe70bc4d02d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -15934,6 +15934,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>  	struct intel_crtc *crtc;
>> +	struct intel_crtc_state *crtc_state;
> Can be moved into narrower scope.
2 places use it, the power domains and the dump_pipe_config, because of this
I kept it as top scope. :)

Thanks, pushed patch 2-9. Hoping to get a reviewer for 1, and will resubmit 10/10 with
a patch doing the proposed intel_wait_for_vblank_if_active() changes.

>>  	struct intel_encoder *encoder;
>>  	int i;
>>  
>> @@ -15952,7 +15953,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>>  	for_each_intel_crtc(&dev_priv->drm, crtc) {
>>  		drm_crtc_vblank_reset(&crtc->base);
>>  
>> -		if (crtc->active)
>> +		if (crtc->base.state->active)
>>  			drm_crtc_vblank_on(&crtc->base);
>>  	}
>>  
>> @@ -15961,9 +15962,10 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>>  	for_each_intel_encoder(dev, encoder)
>>  		intel_sanitize_encoder(encoder);
>>  
>> -	for_each_intel_crtc(&dev_priv->drm, crtc) {
>> +	for_each_intel_crtc(dev, crtc) {
> I'd keep the dev_priv->drm, because we should just change the function
> to take the dev_priv directly.
>
> Apart from those
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Re: [PATCH 01/10] drm/i915: Remove crtc->config dereference from drrs_ctl

[Ville Syrjälä]

On Thu, Oct 11, 2018 at 12:04:48PM +0200, Maarten Lankhorst wrote:
> Wait for idle, and iterate over connectors instead of encoders.
> With this information we know crtc->state is the actual state,
> and we can enable/disable drrs safely.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Looks sensible

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

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 54 ++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f42e93b71e67..b04d5ade5a15 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4661,20 +4661,45 @@ static int i915_drrs_ctl_set(void *data, u64 val)
>  {
>  	struct drm_i915_private *dev_priv = data;
>  	struct drm_device *dev = &dev_priv->drm;
> -	struct intel_crtc *intel_crtc;
> -	struct intel_encoder *encoder;
> -	struct intel_dp *intel_dp;
> +	struct intel_crtc *crtc;
>  
>  	if (INTEL_GEN(dev_priv) < 7)
>  		return -ENODEV;
>  
> -	drm_modeset_lock_all(dev);
> -	for_each_intel_crtc(dev, intel_crtc) {
> -		if (!intel_crtc->base.state->active ||
> -					!intel_crtc->config->has_drrs)
> -			continue;
> +	for_each_intel_crtc(dev, crtc) {
> +		struct drm_connector_list_iter conn_iter;
> +		struct intel_crtc_state *crtc_state;
> +		struct drm_connector *connector;
> +		struct drm_crtc_commit *commit;
> +		int ret;
> +
> +		ret = drm_modeset_lock_single_interruptible(&crtc->base.mutex);
> +		if (ret)
> +			return ret;
> +
> +		crtc_state = to_intel_crtc_state(crtc->base.state);
> +
> +		if (!crtc_state->base.active ||
> +		    !crtc_state->has_drrs)
> +			goto out;
>  
> -		for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> +		commit = crtc_state->base.commit;
> +		if (commit) {
> +			ret = wait_for_completion_interruptible(&commit->hw_done);
> +			if (ret)
> +				goto out;
> +		}
> +
> +		drm_connector_list_iter_begin(dev, &conn_iter);
> +		drm_for_each_connector_iter(connector, &conn_iter) {
> +			struct intel_encoder *encoder;
> +			struct intel_dp *intel_dp;
> +
> +			if (!(crtc_state->base.connector_mask &
> +			      drm_connector_mask(connector)))
> +				continue;
> +
> +			encoder = intel_attached_encoder(connector);
>  			if (encoder->type != INTEL_OUTPUT_EDP)
>  				continue;
>  
> @@ -4684,13 +4709,18 @@ static int i915_drrs_ctl_set(void *data, u64 val)
>  			intel_dp = enc_to_intel_dp(&encoder->base);
>  			if (val)
>  				intel_edp_drrs_enable(intel_dp,
> -							intel_crtc->config);
> +						      crtc_state);
>  			else
>  				intel_edp_drrs_disable(intel_dp,
> -							intel_crtc->config);
> +						       crtc_state);
>  		}
> +		drm_connector_list_iter_end(&conn_iter);
> +
> +out:
> +		drm_modeset_unlock(&crtc->base.mutex);
> +		if (ret)
> +			return ret;
>  	}
> -	drm_modeset_unlock_all(dev);
>  
>  	return 0;
>  }
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 01/10] drm/i915: Remove crtc->config dereference from drrs_ctl

[Maarten Lankhorst]

Op 16-10-18 om 22:13 schreef Ville Syrjälä:
> On Thu, Oct 11, 2018 at 12:04:48PM +0200, Maarten Lankhorst wrote:
>> Wait for idle, and iterate over connectors instead of encoders.
>> With this information we know crtc->state is the actual state,
>> and we can enable/disable drrs safely.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Looks sensible
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks, pushed.

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