[PATCH 1/4] drm/i915: stop for_each_intel_crtc_masked macro from leaking

[PATCH 1/4] drm/i915: stop for_each_intel_crtc_masked macro from leaking

[Daniel Vetter]

Spotted while changing related code.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6d35ccd..27a9ef6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7924,7 +7924,7 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 	list_for_each_entry((intel_crtc), \
 			    &(dev)->mode_config.crtc_list, \
 			    base.head) \
-		if (mask & (1 <<(intel_crtc)->pipe)) \
+		if (mask & (1 <<(intel_crtc)->pipe))
 
 static bool
 intel_pipe_config_compare(struct intel_crtc_config *current_config,
-- 
1.7.11.7

[PATCH 2/4] drm/i915: introduce macros to check pipe config properties

[Daniel Vetter]

This code will get _really_ repetive, and we'll end up with tons more
of this kind. So extract the common patterns.

This should also help when we add a lazy pipe_config compare mode for
fastboot.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 27a9ef6..7216ba5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7930,21 +7930,19 @@ static bool
 intel_pipe_config_compare(struct intel_crtc_config *current_config,
 			  struct intel_crtc_config *pipe_config)
 {
-	if (current_config->has_pch_encoder != pipe_config->has_pch_encoder) {
-		DRM_ERROR("mismatch in has_pch_encoder "
-			  "(expected %i, found %i)\n",
-			  current_config->has_pch_encoder,
-			  pipe_config->has_pch_encoder);
-		return false;
+#define PIPE_CONF_CHECK_I(name)	\
+	if (current_config->name != pipe_config->name) { \
+		DRM_ERROR("mismatch in " #name " " \
+			  "(expected %i, found %i)\n", \
+			  current_config->name, \
+			  pipe_config->name); \
+		return false; \
 	}
 
-	if (current_config->fdi_lanes != pipe_config->fdi_lanes) {
-		DRM_ERROR("mismatch in fdi_lanes "
-			  "(expected %i, found %i)\n",
-			  current_config->fdi_lanes,
-			  pipe_config->fdi_lanes);
-		return false;
-	}
+	PIPE_CONF_CHECK_I(has_pch_encoder);
+	PIPE_CONF_CHECK_I(fdi_lanes);
+
+#undef PIPE_CONF_CHECK_I
 
 	return true;
 }
-- 
1.7.11.7

[PATCH 3/4] drm/i915: hw state readout support for fdi m/n

[Daniel Vetter]

We want to use the fdi m/n values to easily compute the adjusted mode
dotclock on pch ports. Hence make sure the values stored in the pipe
config are always reliable.

v2: Fixup FDI TU readout.

v3: Rebase on top of moved cpu_transcoder.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9093d66..a017120 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3524,6 +3524,7 @@
 #define _PIPEA_DATA_M1           (dev_priv->info->display_mmio_offset + 0x60030)
 #define  TU_SIZE(x)             (((x)-1) << 25) /* default size 64 */
 #define  TU_SIZE_MASK           0x7e000000
+#define  TU_SIZE_SHIFT		25
 #define  PIPE_DATA_M1_OFFSET    0
 #define _PIPEA_DATA_N1           (dev_priv->info->display_mmio_offset + 0x60034)
 #define  PIPE_DATA_N1_OFFSET    0
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7216ba5..a023dc2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5799,6 +5799,22 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return ret;
 }
 
+static void ironlake_get_fdi_m_n_config(struct intel_crtc *crtc,
+					struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder transcoder = pipe_config->cpu_transcoder;
+
+	pipe_config->fdi_m_n.link_m = I915_READ(PIPE_LINK_M1(transcoder));
+	pipe_config->fdi_m_n.link_n = I915_READ(PIPE_LINK_N1(transcoder));
+	pipe_config->fdi_m_n.gmch_m = I915_READ(PIPE_DATA_M1(transcoder))
+					& ~TU_SIZE_MASK;
+	pipe_config->fdi_m_n.gmch_n = I915_READ(PIPE_DATA_N1(transcoder));
+	pipe_config->fdi_m_n.tu = ((I915_READ(PIPE_DATA_M1(transcoder))
+				   & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
+}
+
 static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 				     struct intel_crtc_config *pipe_config)
 {
@@ -5815,6 +5831,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 
 		tmp = I915_READ(FDI_RX_CTL(crtc->pipe));
 		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
+
+		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
 	return true;
@@ -5960,6 +5978,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 
 		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
 		pipe_config->fdi_lanes = ((tmp >> 19) & 0x3) + 1;
+
+		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
 	return true;
@@ -7941,6 +7961,11 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 
 	PIPE_CONF_CHECK_I(has_pch_encoder);
 	PIPE_CONF_CHECK_I(fdi_lanes);
+	PIPE_CONF_CHECK_I(fdi_m_n.gmch_m);
+	PIPE_CONF_CHECK_I(fdi_m_n.gmch_n);
+	PIPE_CONF_CHECK_I(fdi_m_n.link_m);
+	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
+	PIPE_CONF_CHECK_I(fdi_m_n.tu);
 
 #undef PIPE_CONF_CHECK_I
 
-- 
1.7.11.7

[PATCH 4/4] drm/i915: hw state readout support for pipe timings

[Daniel Vetter]

This does duplicate the logic in intel_crtc_mode_get a bit, but the
issue is that we also should handle interlace modes and other insanity
correctly.

Hence I've opted for a sligthly more elaborate route where we first
read out the crtc timings for the adjusted mode, and then optionally
(not sure if we really need it) compute the modeline from that.

v2: Also read out the pipe source dimensions into the requested mode.

v3: Rebase on top of the moved cpu_transcoder.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 74 ++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a017120..b569e17 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2837,6 +2837,7 @@
 #define   PIPECONF_INTERLACED_ILK		(3 << 21)
 #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
 #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
+#define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
 #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
 #define   PIPECONF_BPC_MASK	(0x7 << 5)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a023dc2..d226440 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4704,6 +4704,45 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
 		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
 }
 
+static void intel_get_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	uint32_t tmp;
+
+	tmp = I915_READ(HTOTAL(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HBLANK(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_hblank_end = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HSYNC(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_hsync_end = ((tmp >> 16) & 0xffff) + 1;
+
+	tmp = I915_READ(VTOTAL(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vdisplay = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VBLANK(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vblank_end = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VSYNC(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
+
+	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MODE_MASK) {
+		pipe_config->adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE;
+		pipe_config->adjusted_mode.crtc_vtotal += 1;
+		pipe_config->adjusted_mode.crtc_vblank_end += 1;
+	}
+
+	tmp = I915_READ(PIPESRC(crtc->pipe));
+	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
+	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
+}
+
 static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -4918,6 +4957,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -5835,6 +5876,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -5982,6 +6025,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -7959,6 +8004,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 		return false; \
 	}
 
+#define PIPE_CONF_CHECK_FLAGS(name, mask)	\
+	if ((current_config->name & (mask)) != (pipe_config->name & (mask))) { \
+		DRM_ERROR("mismatch in " #name " " \
+			  "(expected %i, found %i)\n", \
+			  current_config->name & (mask), \
+			  pipe_config->name & (mask)); \
+		return false; \
+	}
+
 	PIPE_CONF_CHECK_I(has_pch_encoder);
 	PIPE_CONF_CHECK_I(fdi_lanes);
 	PIPE_CONF_CHECK_I(fdi_m_n.gmch_m);
@@ -7967,6 +8021,26 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
 	PIPE_CONF_CHECK_I(fdi_m_n.tu);
 
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_end);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_end);
+
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vdisplay);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vtotal);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_end);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_end);
+
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_INTERLACE);
+
+	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
+	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
+
 #undef PIPE_CONF_CHECK_I
 
 	return true;
-- 
1.7.11.7

[PATCH] drm/i915: hw state readout support for pipe timings

[Daniel Vetter]

This does duplicate the logic in intel_crtc_mode_get a bit, but the
issue is that we also should handle interlace modes and other insanity
correctly.

Hence I've opted for a sligthly more elaborate route where we first
read out the crtc timings for the adjusted mode, and then optionally
(not sure if we really need it) compute the modeline from that.

v2: Also read out the pipe source dimensions into the requested mode.

v3: Rebase on top of the moved cpu_transcoder.

v4: Simplify CHECK_FLAGS logic as suggested by Chris Wilson. Also
properly #undef that macro again.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a017120..b569e17 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2837,6 +2837,7 @@
 #define   PIPECONF_INTERLACED_ILK		(3 << 21)
 #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
 #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
+#define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
 #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
 #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
 #define   PIPECONF_BPC_MASK	(0x7 << 5)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a023dc2..cde2cdd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4704,6 +4704,45 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
 		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
 }
 
+static void intel_get_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	uint32_t tmp;
+
+	tmp = I915_READ(HTOTAL(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HBLANK(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_hblank_end = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HSYNC(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_hsync_end = ((tmp >> 16) & 0xffff) + 1;
+
+	tmp = I915_READ(VTOTAL(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vdisplay = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VBLANK(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vblank_end = ((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VSYNC(cpu_transcoder));
+	pipe_config->adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
+	pipe_config->adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
+
+	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MODE_MASK) {
+		pipe_config->adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE;
+		pipe_config->adjusted_mode.crtc_vtotal += 1;
+		pipe_config->adjusted_mode.crtc_vblank_end += 1;
+	}
+
+	tmp = I915_READ(PIPESRC(crtc->pipe));
+	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
+	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
+}
+
 static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -4918,6 +4957,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -5835,6 +5876,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -5982,6 +6025,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 	}
 
+	intel_get_pipe_timings(crtc, pipe_config);
+
 	return true;
 }
 
@@ -7959,6 +8004,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 		return false; \
 	}
 
+#define PIPE_CONF_CHECK_FLAGS(name, mask)	\
+	if ((current_config->name ^ pipe_config->name) & (mask)) { \
+		DRM_ERROR("mismatch in " #name " " \
+			  "(expected %i, found %i)\n", \
+			  current_config->name & (mask), \
+			  pipe_config->name & (mask)); \
+		return false; \
+	}
+
 	PIPE_CONF_CHECK_I(has_pch_encoder);
 	PIPE_CONF_CHECK_I(fdi_lanes);
 	PIPE_CONF_CHECK_I(fdi_m_n.gmch_m);
@@ -7967,7 +8021,28 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
 	PIPE_CONF_CHECK_I(fdi_m_n.tu);
 
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_end);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_end);
+
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vdisplay);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vtotal);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_end);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_start);
+	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_end);
+
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_INTERLACE);
+
+	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
+	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
+
 #undef PIPE_CONF_CHECK_I
+#undef PIPE_CONF_CHECK_FLAGS
 
 	return true;
 }
-- 
1.7.11.7

Re: [PATCH] drm/i915: hw state readout support for pipe timings

[Mika Kuoppala]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> This does duplicate the logic in intel_crtc_mode_get a bit, but the
> issue is that we also should handle interlace modes and other insanity
> correctly.
>
> Hence I've opted for a sligthly more elaborate route where we first
> read out the crtc timings for the adjusted mode, and then optionally
> (not sure if we really need it) compute the modeline from that.
>
> v2: Also read out the pipe source dimensions into the requested mode.
>
> v3: Rebase on top of the moved cpu_transcoder.
>
> v4: Simplify CHECK_FLAGS logic as suggested by Chris Wilson. Also
> properly #undef that macro again.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a017120..b569e17 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2837,6 +2837,7 @@
>  #define   PIPECONF_INTERLACED_ILK		(3 << 21)
>  #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
>  #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
> +#define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)

You can use PIPECONF_INTERLACE_MODE.
With that fixed on the series:

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
>  #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
>  #define   PIPECONF_BPC_MASK	(0x7 << 5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a023dc2..cde2cdd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4704,6 +4704,45 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
>  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
>  }
>  
> +static void intel_get_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HBLANK(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_hblank_end = ((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HSYNC(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_hsync_end = ((tmp >> 16) & 0xffff) + 1;
> +
> +	tmp = I915_READ(VTOTAL(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_vdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VBLANK(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_vblank_end = ((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VSYNC(cpu_transcoder));
> +	pipe_config->adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
> +
> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MODE_MASK) {
> +		pipe_config->adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE;
> +		pipe_config->adjusted_mode.crtc_vtotal += 1;
> +		pipe_config->adjusted_mode.crtc_vblank_end += 1;
> +	}
> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
> +}
> +
>  static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
> @@ -4918,6 +4957,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>  	if (!(tmp & PIPECONF_ENABLE))
>  		return false;
>  
> +	intel_get_pipe_timings(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -5835,6 +5876,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
>  	}
>  
> +	intel_get_pipe_timings(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -5982,6 +6025,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
>  	}
>  
> +	intel_get_pipe_timings(crtc, pipe_config);
> +
>  	return true;
>  }
>  
> @@ -7959,6 +8004,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  		return false; \
>  	}
>  
> +#define PIPE_CONF_CHECK_FLAGS(name, mask)	\
> +	if ((current_config->name ^ pipe_config->name) & (mask)) { \
> +		DRM_ERROR("mismatch in " #name " " \
> +			  "(expected %i, found %i)\n", \
> +			  current_config->name & (mask), \
> +			  pipe_config->name & (mask)); \
> +		return false; \
> +	}
> +
>  	PIPE_CONF_CHECK_I(has_pch_encoder);
>  	PIPE_CONF_CHECK_I(fdi_lanes);
>  	PIPE_CONF_CHECK_I(fdi_m_n.gmch_m);
> @@ -7967,7 +8021,28 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
>  	PIPE_CONF_CHECK_I(fdi_m_n.tu);
>  
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_end);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_start);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_end);
> +
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vdisplay);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vtotal);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_start);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_end);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_start);
> +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_end);
> +
> +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> +			      DRM_MODE_FLAG_INTERLACE);
> +
> +	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
> +	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
> +
>  #undef PIPE_CONF_CHECK_I
> +#undef PIPE_CONF_CHECK_FLAGS
>  
>  	return true;
>  }
> -- 
> 1.7.11.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: hw state readout support for pipe timings

[Daniel Vetter]

On Thu, Apr 25, 2013 at 05:24:19PM +0300, Mika Kuoppala wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> 
> > This does duplicate the logic in intel_crtc_mode_get a bit, but the
> > issue is that we also should handle interlace modes and other insanity
> > correctly.
> >
> > Hence I've opted for a sligthly more elaborate route where we first
> > read out the crtc timings for the adjusted mode, and then optionally
> > (not sure if we really need it) compute the modeline from that.
> >
> > v2: Also read out the pipe source dimensions into the requested mode.
> >
> > v3: Rebase on top of the moved cpu_transcoder.
> >
> > v4: Simplify CHECK_FLAGS logic as suggested by Chris Wilson. Also
> > properly #undef that macro again.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >  drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 76 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a017120..b569e17 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2837,6 +2837,7 @@
> >  #define   PIPECONF_INTERLACED_ILK		(3 << 21)
> >  #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
> >  #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
> > +#define   PIPECONF_INTERLACE_MODE_MASK		(7 << 21)
> 
> You can use PIPECONF_INTERLACE_MODE.
> With that fixed on the series:

Fixed and the entire series merged, thanks for the review.
-Daniel

> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> 
> >  #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
> >  #define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
> >  #define   PIPECONF_BPC_MASK	(0x7 << 5)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a023dc2..cde2cdd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4704,6 +4704,45 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
> >  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
> >  }
> >  
> > +static void intel_get_pipe_timings(struct intel_crtc *crtc,
> > +				   struct intel_crtc_config *pipe_config)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	uint32_t tmp;
> > +
> > +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(HBLANK(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_hblank_end = ((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(HSYNC(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_hsync_end = ((tmp >> 16) & 0xffff) + 1;
> > +
> > +	tmp = I915_READ(VTOTAL(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_vdisplay = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(VBLANK(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_vblank_end = ((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(VSYNC(cpu_transcoder));
> > +	pipe_config->adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> > +	pipe_config->adjusted_mode.crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
> > +
> > +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MODE_MASK) {
> > +		pipe_config->adjusted_mode.flags |= DRM_MODE_FLAG_INTERLACE;
> > +		pipe_config->adjusted_mode.crtc_vtotal += 1;
> > +		pipe_config->adjusted_mode.crtc_vblank_end += 1;
> > +	}
> > +
> > +	tmp = I915_READ(PIPESRC(crtc->pipe));
> > +	pipe_config->requested_mode.vdisplay = (tmp & 0xffff) + 1;
> > +	pipe_config->requested_mode.hdisplay = ((tmp >> 16) & 0xffff) + 1;
> > +}
> > +
> >  static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
> >  {
> >  	struct drm_device *dev = intel_crtc->base.dev;
> > @@ -4918,6 +4957,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> >  	if (!(tmp & PIPECONF_ENABLE))
> >  		return false;
> >  
> > +	intel_get_pipe_timings(crtc, pipe_config);
> > +
> >  	return true;
> >  }
> >  
> > @@ -5835,6 +5876,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
> >  	}
> >  
> > +	intel_get_pipe_timings(crtc, pipe_config);
> > +
> >  	return true;
> >  }
> >  
> > @@ -5982,6 +6025,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
> >  	}
> >  
> > +	intel_get_pipe_timings(crtc, pipe_config);
> > +
> >  	return true;
> >  }
> >  
> > @@ -7959,6 +8004,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
> >  		return false; \
> >  	}
> >  
> > +#define PIPE_CONF_CHECK_FLAGS(name, mask)	\
> > +	if ((current_config->name ^ pipe_config->name) & (mask)) { \
> > +		DRM_ERROR("mismatch in " #name " " \
> > +			  "(expected %i, found %i)\n", \
> > +			  current_config->name & (mask), \
> > +			  pipe_config->name & (mask)); \
> > +		return false; \
> > +	}
> > +
> >  	PIPE_CONF_CHECK_I(has_pch_encoder);
> >  	PIPE_CONF_CHECK_I(fdi_lanes);
> >  	PIPE_CONF_CHECK_I(fdi_m_n.gmch_m);
> > @@ -7967,7 +8021,28 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
> >  	PIPE_CONF_CHECK_I(fdi_m_n.link_n);
> >  	PIPE_CONF_CHECK_I(fdi_m_n.tu);
> >  
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hdisplay);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_htotal);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_start);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hblank_end);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_start);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_hsync_end);
> > +
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vdisplay);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vtotal);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_start);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vblank_end);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_start);
> > +	PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_end);
> > +
> > +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> > +			      DRM_MODE_FLAG_INTERLACE);
> > +
> > +	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
> > +	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
> > +
> >  #undef PIPE_CONF_CHECK_I
> > +#undef PIPE_CONF_CHECK_FLAGS
> >  
> >  	return true;
> >  }
> > -- 
> > 1.7.11.7
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch