[PATCH] drm/i915: add encoder get_config function v3

[PATCH] drm/i915: add encoder get_config function v3

[Jesse Barnes]

We can use this for fetching encoder specific pipe_config state, like
mode flags, adjusted clock, etc.

Just used for mode flags atm, so we can check the pipe config state at
mode set time.

v2: get_config when checking hw state too
v3: fix DVO and LVDS mode flags (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_crt.c     |   23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++++++---
 drivers/gpu/drm/i915/intel_dp.c      |   23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    4 ++++
 drivers/gpu/drm/i915/intel_dvo.c     |   21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_hdmi.c    |   23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lvds.c    |   26 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |   24 ++++++++++++++++++++++++
 8 files changed, 161 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 991e530..1370870 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -81,6 +81,28 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_crt_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_crt *crt = intel_encoder_to_crt(encoder);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(crt->adpa_reg);
+
+	if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_crt(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -784,6 +806,7 @@ void intel_crt_init(struct drm_device *dev)
 	crt->base.compute_config = intel_crt_compute_config;
 	crt->base.disable = intel_disable_crt;
 	crt->base.enable = intel_enable_crt;
+	crt->base.get_config = intel_crt_get_config;
 	if (I915_HAS_HOTPLUG(dev))
 		crt->base.hpd_pin = HPD_CRT;
 	if (HAS_DDI(dev))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de8be75..924932f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8060,6 +8060,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
 			      DRM_MODE_FLAG_INTERLACE);
 
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PVSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NVSYNC);
+
 	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
 	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
 
@@ -8144,6 +8153,8 @@ intel_modeset_check_state(struct drm_device *dev)
 		bool enabled = false;
 		bool active = false;
 
+		memset(&pipe_config, 0, sizeof(pipe_config));
+
 		DRM_DEBUG_KMS("[CRTC:%d]\n",
 			      crtc->base.base.id);
 
@@ -8157,6 +8168,8 @@ intel_modeset_check_state(struct drm_device *dev)
 			enabled = true;
 			if (encoder->connectors_active)
 				active = true;
+			if (encoder->get_config)
+				encoder->get_config(encoder, &pipe_config);
 		}
 		WARN(active != crtc->active,
 		     "crtc's computed active state doesn't match tracked active state "
@@ -8165,7 +8178,6 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
-		memset(&pipe_config, 0, sizeof(pipe_config));
 		pipe_config.cpu_transcoder = crtc->config.cpu_transcoder;
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
@@ -9575,8 +9587,10 @@ setup_pipes:
 		pipe = 0;
 
 		if (encoder->get_hw_state(encoder, &pipe)) {
-			encoder->base.crtc =
-				dev_priv->pipe_to_crtc_mapping[pipe];
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+			if (encoder->get_config)
+				encoder->get_config(encoder, &crtc->config);
+			encoder->base.crtc = &crtc->base;
 		} else {
 			encoder->base.crtc = NULL;
 		}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a293523..889ce46 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1383,6 +1383,28 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dp_get_config(struct intel_encoder *encoder,
+				struct intel_crtc_config *pipe_config)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dp->output_reg);
+
+	if (tmp & DP_SYNC_HS_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & DP_SYNC_VS_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -3170,6 +3192,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
+	intel_encoder->get_config = intel_dp_get_config;
 	if (IS_VALLEYVIEW(dev))
 		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfcf546..abc4aa8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -140,6 +140,10 @@ struct intel_encoder {
 	 * the encoder is active. If the encoder is enabled it also set the pipe
 	 * it is connected to in the pipe parameter. */
 	bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
+	/* Reconstructs the equivalent mode flags for the current hardware
+	 * state. */
+	void (*get_config)(struct intel_encoder *,
+			   struct intel_crtc_config *pipe_config);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
 };
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 00e70db..91e9905 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -129,6 +129,26 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dvo_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dvo->dev.dvo_reg);
+	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	if (tmp & DVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -440,6 +460,7 @@ void intel_dvo_init(struct drm_device *dev)
 	intel_encoder->disable = intel_disable_dvo;
 	intel_encoder->enable = intel_enable_dvo;
 	intel_encoder->get_hw_state = intel_dvo_get_hw_state;
+	intel_encoder->get_config = intel_dvo_get_config;
 	intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
 
 	/* Now, try to find a controller */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 93de5ff..9091655 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -658,6 +658,28 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_hdmi_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_hdmi->hdmi_reg);
+
+	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_enable_hdmi(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -1208,6 +1230,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
+	intel_encoder->get_config = intel_hdmi_get_config;
 	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_enable = intel_hdmi_pre_enable;
 		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d256fe4..8d464a9 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -86,6 +86,31 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_lvds_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 lvds_reg, tmp, flags = 0;
+
+	if (HAS_PCH_SPLIT(dev))
+		lvds_reg = PCH_LVDS;
+	else
+		lvds_reg = LVDS;
+
+	tmp = I915_READ(lvds_reg);
+	if (tmp & LVDS_HSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	if (tmp & LVDS_VSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 /* The LVDS pin pair needs to be on before the DPLLs are enabled.
  * This is an exception to the general rule that mode_set doesn't turn
  * things on.
@@ -920,6 +945,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	intel_encoder->compute_config = intel_lvds_compute_config;
 	intel_encoder->disable = intel_disable_lvds;
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
+	intel_encoder->get_config = intel_lvds_get_config;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a618a6a..75284ae 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1264,6 +1264,28 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_sdvo_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_sdvo->sdvo_reg);
+
+	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_sdvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -2793,6 +2815,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 	intel_encoder->mode_set = intel_sdvo_mode_set;
 	intel_encoder->enable = intel_enable_sdvo;
 	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
+	if (INTEL_INFO(dev)->gen >= 4)
+		intel_encoder->get_config = intel_sdvo_get_config;
 
 	/* In default case sdvo lvds is false */
 	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
-- 
1.7.9.5

[PATCH] drm/i915: add encoder get_config function v3

[Jesse Barnes]

We can use this for fetching encoder specific pipe_config state, like
mode flags, adjusted clock, etc.

Just used for mode flags atm, so we can check the pipe config state at
mode set time.

v2: get_config when checking hw state too
v3: fix DVO and LVDS mode flags (Ville)
    get SDVO DTD for flag fetch (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_crt.c     |   23 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++---
 drivers/gpu/drm/i915/intel_dp.c      |   23 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    4 ++++
 drivers/gpu/drm/i915/intel_dvo.c     |   21 +++++++++++++++++
 drivers/gpu/drm/i915/intel_hdmi.c    |   23 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_lvds.c    |   26 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |   43 ++++++++++++++++++++++++++++++++++
 8 files changed, 180 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 991e530..1370870 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -81,6 +81,28 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_crt_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_crt *crt = intel_encoder_to_crt(encoder);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(crt->adpa_reg);
+
+	if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_crt(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -784,6 +806,7 @@ void intel_crt_init(struct drm_device *dev)
 	crt->base.compute_config = intel_crt_compute_config;
 	crt->base.disable = intel_disable_crt;
 	crt->base.enable = intel_enable_crt;
+	crt->base.get_config = intel_crt_get_config;
 	if (I915_HAS_HOTPLUG(dev))
 		crt->base.hpd_pin = HPD_CRT;
 	if (HAS_DDI(dev))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de8be75..924932f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8060,6 +8060,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
 			      DRM_MODE_FLAG_INTERLACE);
 
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PVSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NVSYNC);
+
 	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
 	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
 
@@ -8144,6 +8153,8 @@ intel_modeset_check_state(struct drm_device *dev)
 		bool enabled = false;
 		bool active = false;
 
+		memset(&pipe_config, 0, sizeof(pipe_config));
+
 		DRM_DEBUG_KMS("[CRTC:%d]\n",
 			      crtc->base.base.id);
 
@@ -8157,6 +8168,8 @@ intel_modeset_check_state(struct drm_device *dev)
 			enabled = true;
 			if (encoder->connectors_active)
 				active = true;
+			if (encoder->get_config)
+				encoder->get_config(encoder, &pipe_config);
 		}
 		WARN(active != crtc->active,
 		     "crtc's computed active state doesn't match tracked active state "
@@ -8165,7 +8178,6 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
-		memset(&pipe_config, 0, sizeof(pipe_config));
 		pipe_config.cpu_transcoder = crtc->config.cpu_transcoder;
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
@@ -9575,8 +9587,10 @@ setup_pipes:
 		pipe = 0;
 
 		if (encoder->get_hw_state(encoder, &pipe)) {
-			encoder->base.crtc =
-				dev_priv->pipe_to_crtc_mapping[pipe];
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+			if (encoder->get_config)
+				encoder->get_config(encoder, &crtc->config);
+			encoder->base.crtc = &crtc->base;
 		} else {
 			encoder->base.crtc = NULL;
 		}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a293523..889ce46 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1383,6 +1383,28 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dp_get_config(struct intel_encoder *encoder,
+				struct intel_crtc_config *pipe_config)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dp->output_reg);
+
+	if (tmp & DP_SYNC_HS_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & DP_SYNC_VS_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -3170,6 +3192,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
+	intel_encoder->get_config = intel_dp_get_config;
 	if (IS_VALLEYVIEW(dev))
 		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfcf546..abc4aa8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -140,6 +140,10 @@ struct intel_encoder {
 	 * the encoder is active. If the encoder is enabled it also set the pipe
 	 * it is connected to in the pipe parameter. */
 	bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
+	/* Reconstructs the equivalent mode flags for the current hardware
+	 * state. */
+	void (*get_config)(struct intel_encoder *,
+			   struct intel_crtc_config *pipe_config);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
 };
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 00e70db..91e9905 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -129,6 +129,26 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dvo_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dvo->dev.dvo_reg);
+	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	if (tmp & DVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -440,6 +460,7 @@ void intel_dvo_init(struct drm_device *dev)
 	intel_encoder->disable = intel_disable_dvo;
 	intel_encoder->enable = intel_enable_dvo;
 	intel_encoder->get_hw_state = intel_dvo_get_hw_state;
+	intel_encoder->get_config = intel_dvo_get_config;
 	intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
 
 	/* Now, try to find a controller */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 93de5ff..9091655 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -658,6 +658,28 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_hdmi_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_hdmi->hdmi_reg);
+
+	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_enable_hdmi(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -1208,6 +1230,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
+	intel_encoder->get_config = intel_hdmi_get_config;
 	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_enable = intel_hdmi_pre_enable;
 		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d256fe4..8d464a9 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -86,6 +86,31 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_lvds_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 lvds_reg, tmp, flags = 0;
+
+	if (HAS_PCH_SPLIT(dev))
+		lvds_reg = PCH_LVDS;
+	else
+		lvds_reg = LVDS;
+
+	tmp = I915_READ(lvds_reg);
+	if (tmp & LVDS_HSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	if (tmp & LVDS_VSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 /* The LVDS pin pair needs to be on before the DPLLs are enabled.
  * This is an exception to the general rule that mode_set doesn't turn
  * things on.
@@ -920,6 +945,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	intel_encoder->compute_config = intel_lvds_compute_config;
 	intel_encoder->disable = intel_disable_lvds;
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
+	intel_encoder->get_config = intel_lvds_get_config;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a618a6a..7630cac 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -712,6 +712,13 @@ static bool intel_sdvo_set_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
 		intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
 }
 
+static bool intel_sdvo_get_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
+				  struct intel_sdvo_dtd *dtd)
+{
+	return intel_sdvo_get_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1)) &&
+		intel_sdvo_get_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
+}
+
 static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo,
 					 struct intel_sdvo_dtd *dtd)
 {
@@ -726,6 +733,13 @@ static bool intel_sdvo_set_output_timing(struct intel_sdvo *intel_sdvo,
 				     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd);
 }
 
+static bool intel_sdvo_get_output_timing(struct intel_sdvo *intel_sdvo,
+					 struct intel_sdvo_dtd *dtd)
+{
+	return intel_sdvo_get_timing(intel_sdvo,
+				     SDVO_CMD_GET_OUTPUT_TIMINGS_PART2, dtd);
+}
+
 static bool
 intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
 					 uint16_t clock,
@@ -1264,6 +1278,33 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_sdvo_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
+	struct intel_sdvo_dtd dtd;
+	u32 flags = 0;
+	bool ret;
+
+	ret = intel_sdvo_get_output_timing(intel_sdvo, &dtd);
+	if (!ret) {
+		DRM_DEBUG_DRIVER("failed to retrieve SDVO DTD\n");
+		return;
+	}
+
+	if (dtd.part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (dtd.part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_sdvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -2793,6 +2834,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 	intel_encoder->mode_set = intel_sdvo_mode_set;
 	intel_encoder->enable = intel_enable_sdvo;
 	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
+	if (INTEL_INFO(dev)->gen >= 4)
+		intel_encoder->get_config = intel_sdvo_get_config;
 
 	/* In default case sdvo lvds is false */
 	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
-- 
1.7.9.5

Re: [PATCH] drm/i915: add encoder get_config function v3

[Ville Syrjälä]

On Tue, May 07, 2013 at 03:38:41PM -0700, Jesse Barnes wrote:
> We can use this for fetching encoder specific pipe_config state, like
> mode flags, adjusted clock, etc.
> 
> Just used for mode flags atm, so we can check the pipe config state at
> mode set time.
> 
> v2: get_config when checking hw state too
> v3: fix DVO and LVDS mode flags (Ville)
>     get SDVO DTD for flag fetch (Ville)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     |   23 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      |   23 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    4 ++++
>  drivers/gpu/drm/i915/intel_dvo.c     |   21 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    |   23 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lvds.c    |   26 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |   43 ++++++++++++++++++++++++++++++++++
>  8 files changed, 180 insertions(+), 3 deletions(-)
> 
<snip>
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -712,6 +712,13 @@ static bool intel_sdvo_set_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
>  		intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
>  }
>  
> +static bool intel_sdvo_get_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
> +				  struct intel_sdvo_dtd *dtd)
> +{
> +	return intel_sdvo_get_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1)) &&
> +		intel_sdvo_get_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
> +}
> +
>  static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo,
>  					 struct intel_sdvo_dtd *dtd)
>  {
> @@ -726,6 +733,13 @@ static bool intel_sdvo_set_output_timing(struct intel_sdvo *intel_sdvo,
>  				     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd);
>  }
>  
> +static bool intel_sdvo_get_output_timing(struct intel_sdvo *intel_sdvo,
> +					 struct intel_sdvo_dtd *dtd)
> +{
> +	return intel_sdvo_get_timing(intel_sdvo,
> +				     SDVO_CMD_GET_OUTPUT_TIMINGS_PART2, dtd);

PART1

Oh and it looks like we actually feed adjusted_mode into the input timings,
not the output timings. So I'm thinking we should use the input timings in
get_config() too.

> +}
> +
>  static bool
>  intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
>  					 uint16_t clock,
> @@ -1264,6 +1278,33 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_sdvo_get_config(struct intel_encoder *encoder,
> +				  struct intel_crtc_config *pipe_config)
> +{
> +	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> +	struct intel_sdvo_dtd dtd;
> +	u32 flags = 0;
> +	bool ret;
> +
> +	ret = intel_sdvo_get_output_timing(intel_sdvo, &dtd);
> +	if (!ret) {
> +		DRM_DEBUG_DRIVER("failed to retrieve SDVO DTD\n");
> +		return;
> +	}
> +
> +	if (dtd.part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (dtd.part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_disable_sdvo(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2793,6 +2834,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  	intel_encoder->mode_set = intel_sdvo_mode_set;
>  	intel_encoder->enable = intel_enable_sdvo;
>  	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> +	if (INTEL_INFO(dev)->gen >= 4)
> +		intel_encoder->get_config = intel_sdvo_get_config;

I'm assuming the gen4 check can be dropped now.

So, as we discussed on irc, the only issue left is that we don't sanitize
the adjusted_mode sync flags. I think they just get copied from the user
specified mode directly, so if the user specifies neither + or - flag,
or specifies both, intel_pipe_config_compare() will scream.

>  	/* In default case sdvo lvds is false */
>  	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: add encoder get_config function v4

[Jesse Barnes]

We can use this for fetching encoder specific pipe_config state, like
mode flags, adjusted clock, etc.

Just used for mode flags atm, so we can check the pipe config state at
mode set time.

v2: get_config when checking hw state too
v3: fix DVO and LVDS mode flags (Ville)
    get SDVO DTD for flag fetch (Ville)
v4: use input timings (Ville)
    correct command used (Ville)
    remove gen4 check (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_crt.c     |   23 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++---
 drivers/gpu/drm/i915/intel_dp.c      |   23 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    4 ++++
 drivers/gpu/drm/i915/intel_dvo.c     |   21 +++++++++++++++++
 drivers/gpu/drm/i915/intel_hdmi.c    |   23 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_lvds.c    |   26 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |   42 ++++++++++++++++++++++++++++++++++
 8 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 991e530..1370870 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -81,6 +81,28 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_crt_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_crt *crt = intel_encoder_to_crt(encoder);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(crt->adpa_reg);
+
+	if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_crt(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -784,6 +806,7 @@ void intel_crt_init(struct drm_device *dev)
 	crt->base.compute_config = intel_crt_compute_config;
 	crt->base.disable = intel_disable_crt;
 	crt->base.enable = intel_enable_crt;
+	crt->base.get_config = intel_crt_get_config;
 	if (I915_HAS_HOTPLUG(dev))
 		crt->base.hpd_pin = HPD_CRT;
 	if (HAS_DDI(dev))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de8be75..924932f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8060,6 +8060,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
 	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
 			      DRM_MODE_FLAG_INTERLACE);
 
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NHSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_PVSYNC);
+	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
+			      DRM_MODE_FLAG_NVSYNC);
+
 	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
 	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
 
@@ -8144,6 +8153,8 @@ intel_modeset_check_state(struct drm_device *dev)
 		bool enabled = false;
 		bool active = false;
 
+		memset(&pipe_config, 0, sizeof(pipe_config));
+
 		DRM_DEBUG_KMS("[CRTC:%d]\n",
 			      crtc->base.base.id);
 
@@ -8157,6 +8168,8 @@ intel_modeset_check_state(struct drm_device *dev)
 			enabled = true;
 			if (encoder->connectors_active)
 				active = true;
+			if (encoder->get_config)
+				encoder->get_config(encoder, &pipe_config);
 		}
 		WARN(active != crtc->active,
 		     "crtc's computed active state doesn't match tracked active state "
@@ -8165,7 +8178,6 @@ intel_modeset_check_state(struct drm_device *dev)
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
 
-		memset(&pipe_config, 0, sizeof(pipe_config));
 		pipe_config.cpu_transcoder = crtc->config.cpu_transcoder;
 		active = dev_priv->display.get_pipe_config(crtc,
 							   &pipe_config);
@@ -9575,8 +9587,10 @@ setup_pipes:
 		pipe = 0;
 
 		if (encoder->get_hw_state(encoder, &pipe)) {
-			encoder->base.crtc =
-				dev_priv->pipe_to_crtc_mapping[pipe];
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+			if (encoder->get_config)
+				encoder->get_config(encoder, &crtc->config);
+			encoder->base.crtc = &crtc->base;
 		} else {
 			encoder->base.crtc = NULL;
 		}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a293523..889ce46 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1383,6 +1383,28 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dp_get_config(struct intel_encoder *encoder,
+				struct intel_crtc_config *pipe_config)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dp->output_reg);
+
+	if (tmp & DP_SYNC_HS_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & DP_SYNC_VS_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -3170,6 +3192,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->disable = intel_disable_dp;
 	intel_encoder->post_disable = intel_post_disable_dp;
 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
+	intel_encoder->get_config = intel_dp_get_config;
 	if (IS_VALLEYVIEW(dev))
 		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dfcf546..abc4aa8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -140,6 +140,10 @@ struct intel_encoder {
 	 * the encoder is active. If the encoder is enabled it also set the pipe
 	 * it is connected to in the pipe parameter. */
 	bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
+	/* Reconstructs the equivalent mode flags for the current hardware
+	 * state. */
+	void (*get_config)(struct intel_encoder *,
+			   struct intel_crtc_config *pipe_config);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
 };
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 00e70db..91e9905 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -129,6 +129,26 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_dvo_get_config(struct intel_encoder *encoder,
+				 struct intel_crtc_config *pipe_config)
+{
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_dvo->dev.dvo_reg);
+	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	if (tmp & DVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_dvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -440,6 +460,7 @@ void intel_dvo_init(struct drm_device *dev)
 	intel_encoder->disable = intel_disable_dvo;
 	intel_encoder->enable = intel_enable_dvo;
 	intel_encoder->get_hw_state = intel_dvo_get_hw_state;
+	intel_encoder->get_config = intel_dvo_get_config;
 	intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
 
 	/* Now, try to find a controller */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 93de5ff..9091655 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -658,6 +658,28 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_hdmi_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
+	u32 tmp, flags = 0;
+
+	tmp = I915_READ(intel_hdmi->hdmi_reg);
+
+	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_enable_hdmi(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -1208,6 +1230,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 	intel_encoder->enable = intel_enable_hdmi;
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
+	intel_encoder->get_config = intel_hdmi_get_config;
 	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_enable = intel_hdmi_pre_enable;
 		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index d256fe4..8d464a9 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -86,6 +86,31 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_lvds_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 lvds_reg, tmp, flags = 0;
+
+	if (HAS_PCH_SPLIT(dev))
+		lvds_reg = PCH_LVDS;
+	else
+		lvds_reg = LVDS;
+
+	tmp = I915_READ(lvds_reg);
+	if (tmp & LVDS_HSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	if (tmp & LVDS_VSYNC_POLARITY)
+		flags |= DRM_MODE_FLAG_NVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_PVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 /* The LVDS pin pair needs to be on before the DPLLs are enabled.
  * This is an exception to the general rule that mode_set doesn't turn
  * things on.
@@ -920,6 +945,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	intel_encoder->compute_config = intel_lvds_compute_config;
 	intel_encoder->disable = intel_disable_lvds;
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
+	intel_encoder->get_config = intel_lvds_get_config;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a618a6a..271b0b4 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -712,6 +712,13 @@ static bool intel_sdvo_set_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
 		intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
 }
 
+static bool intel_sdvo_get_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
+				  struct intel_sdvo_dtd *dtd)
+{
+	return intel_sdvo_get_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1)) &&
+		intel_sdvo_get_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
+}
+
 static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo,
 					 struct intel_sdvo_dtd *dtd)
 {
@@ -726,6 +733,13 @@ static bool intel_sdvo_set_output_timing(struct intel_sdvo *intel_sdvo,
 				     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd);
 }
 
+static bool intel_sdvo_get_input_timing(struct intel_sdvo *intel_sdvo,
+					struct intel_sdvo_dtd *dtd)
+{
+	return intel_sdvo_get_timing(intel_sdvo,
+				     SDVO_CMD_GET_INPUT_TIMINGS_PART1, dtd);
+}
+
 static bool
 intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
 					 uint16_t clock,
@@ -1264,6 +1278,33 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
 	return true;
 }
 
+static void intel_sdvo_get_config(struct intel_encoder *encoder,
+				  struct intel_crtc_config *pipe_config)
+{
+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
+	struct intel_sdvo_dtd dtd;
+	u32 flags = 0;
+	bool ret;
+
+	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
+	if (!ret) {
+		DRM_DEBUG_DRIVER("failed to retrieve SDVO DTD\n");
+		return;
+	}
+
+	if (dtd.part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
+		flags |= DRM_MODE_FLAG_PHSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NHSYNC;
+
+	if (dtd.part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
+		flags |= DRM_MODE_FLAG_PVSYNC;
+	else
+		flags |= DRM_MODE_FLAG_NVSYNC;
+
+	pipe_config->adjusted_mode.flags |= flags;
+}
+
 static void intel_disable_sdvo(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
@@ -2793,6 +2834,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
 	intel_encoder->mode_set = intel_sdvo_mode_set;
 	intel_encoder->enable = intel_enable_sdvo;
 	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
+	intel_encoder->get_config = intel_sdvo_get_config;
 
 	/* In default case sdvo lvds is false */
 	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
-- 
1.7.9.5

[PATCH] drm: check user mode flags for validity

[Jesse Barnes]

Requested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_crtc.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 792c3e3..72ae33a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1318,6 +1318,18 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
 	if (in->clock > INT_MAX || in->vrefresh > INT_MAX)
 		return -ERANGE;
 
+	/* Reject modes with invalid h/vsync */
+	if (!(in->flags & (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC)))
+		return -EINVAL;
+	if ((in->flags & DRM_MODE_FLAG_PHSYNC) &&
+	    (in->flags & DRM_MODE_FLAG_NHSYNC))
+		return -EINVAL;
+	if (!(in->flags & (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
+		return -EINVAL;
+	if ((in->flags & DRM_MODE_FLAG_PVSYNC) &&
+	    (in->flags & DRM_MODE_FLAG_NVSYNC))
+		return -EINVAL;
+
 	out->clock = in->clock;
 	out->hdisplay = in->hdisplay;
 	out->hsync_start = in->hsync_start;
-- 
1.7.9.5

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

Re: [PATCH] drm: check user mode flags for validity

[Ville Syrjälä]

On Wed, May 08, 2013 at 02:01:25PM -0700, Jesse Barnes wrote:
> Requested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/drm_crtc.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 792c3e3..72ae33a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1318,6 +1318,18 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
>  	if (in->clock > INT_MAX || in->vrefresh > INT_MAX)
>  		return -ERANGE;
>  
> +	/* Reject modes with invalid h/vsync */
> +	if (!(in->flags & (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC)))
> +		return -EINVAL;
> +	if ((in->flags & DRM_MODE_FLAG_PHSYNC) &&
> +	    (in->flags & DRM_MODE_FLAG_NHSYNC))
> +		return -EINVAL;
> +	if (!(in->flags & (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
> +		return -EINVAL;
> +	if ((in->flags & DRM_MODE_FLAG_PVSYNC) &&
> +	    (in->flags & DRM_MODE_FLAG_NVSYNC))
> +		return -EINVAL;

That might be a bit too drastic. Well I suppose making sure that both
flags are not enabled at the same time could be OK. But having neither
flag set could be perfectly legal (the user could be asking for composite
sync instead for example).


So my less drastic suggestion would be doing something like this in i915
specific code:

 adjusted_mode->flags = 0;
 if (requested_mode->flags & NHSYNC)
 	adjusted_mode->flags |= NHSYNC;
 else
 	adjusted_mode->flags |= PHSYNC;

It would gurantee that we end up picking exactly one of the flags in
every case. If both are set, we pick -, of neither is set we pick +.

> +
>  	out->clock = in->clock;
>  	out->hdisplay = in->hdisplay;
>  	out->hsync_start = in->hsync_start;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: add encoder get_config function v4

[Ville Syrjälä]

On Wed, May 08, 2013 at 02:01:02PM -0700, Jesse Barnes wrote:
> We can use this for fetching encoder specific pipe_config state, like
> mode flags, adjusted clock, etc.
> 
> Just used for mode flags atm, so we can check the pipe config state at
> mode set time.
> 
> v2: get_config when checking hw state too
> v3: fix DVO and LVDS mode flags (Ville)
>     get SDVO DTD for flag fetch (Ville)
> v4: use input timings (Ville)
>     correct command used (Ville)
>     remove gen4 check (Ville)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

I believe all my complaints have been addressed.

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

> ---
>  drivers/gpu/drm/i915/intel_crt.c     |   23 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |   20 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      |   23 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    4 ++++
>  drivers/gpu/drm/i915/intel_dvo.c     |   21 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    |   23 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lvds.c    |   26 +++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |   42 ++++++++++++++++++++++++++++++++++
>  8 files changed, 179 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 991e530..1370870 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -81,6 +81,28 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_crt_get_config(struct intel_encoder *encoder,
> +				 struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_crt *crt = intel_encoder_to_crt(encoder);
> +	u32 tmp, flags = 0;
> +
> +	tmp = I915_READ(crt->adpa_reg);
> +
> +	if (tmp & ADPA_HSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (tmp & ADPA_VSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_disable_crt(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -784,6 +806,7 @@ void intel_crt_init(struct drm_device *dev)
>  	crt->base.compute_config = intel_crt_compute_config;
>  	crt->base.disable = intel_disable_crt;
>  	crt->base.enable = intel_enable_crt;
> +	crt->base.get_config = intel_crt_get_config;
>  	if (I915_HAS_HOTPLUG(dev))
>  		crt->base.hpd_pin = HPD_CRT;
>  	if (HAS_DDI(dev))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index de8be75..924932f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8060,6 +8060,15 @@ intel_pipe_config_compare(struct intel_crtc_config *current_config,
>  	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
>  			      DRM_MODE_FLAG_INTERLACE);
>  
> +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> +			      DRM_MODE_FLAG_PHSYNC);
> +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> +			      DRM_MODE_FLAG_NHSYNC);
> +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> +			      DRM_MODE_FLAG_PVSYNC);
> +	PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags,
> +			      DRM_MODE_FLAG_NVSYNC);
> +
>  	PIPE_CONF_CHECK_I(requested_mode.hdisplay);
>  	PIPE_CONF_CHECK_I(requested_mode.vdisplay);
>  
> @@ -8144,6 +8153,8 @@ intel_modeset_check_state(struct drm_device *dev)
>  		bool enabled = false;
>  		bool active = false;
>  
> +		memset(&pipe_config, 0, sizeof(pipe_config));
> +
>  		DRM_DEBUG_KMS("[CRTC:%d]\n",
>  			      crtc->base.base.id);
>  
> @@ -8157,6 +8168,8 @@ intel_modeset_check_state(struct drm_device *dev)
>  			enabled = true;
>  			if (encoder->connectors_active)
>  				active = true;
> +			if (encoder->get_config)
> +				encoder->get_config(encoder, &pipe_config);
>  		}
>  		WARN(active != crtc->active,
>  		     "crtc's computed active state doesn't match tracked active state "
> @@ -8165,7 +8178,6 @@ intel_modeset_check_state(struct drm_device *dev)
>  		     "crtc's computed enabled state doesn't match tracked enabled state "
>  		     "(expected %i, found %i)\n", enabled, crtc->base.enabled);
>  
> -		memset(&pipe_config, 0, sizeof(pipe_config));
>  		pipe_config.cpu_transcoder = crtc->config.cpu_transcoder;
>  		active = dev_priv->display.get_pipe_config(crtc,
>  							   &pipe_config);
> @@ -9575,8 +9587,10 @@ setup_pipes:
>  		pipe = 0;
>  
>  		if (encoder->get_hw_state(encoder, &pipe)) {
> -			encoder->base.crtc =
> -				dev_priv->pipe_to_crtc_mapping[pipe];
> +			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> +			if (encoder->get_config)
> +				encoder->get_config(encoder, &crtc->config);
> +			encoder->base.crtc = &crtc->base;
>  		} else {
>  			encoder->base.crtc = NULL;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a293523..889ce46 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1383,6 +1383,28 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_dp_get_config(struct intel_encoder *encoder,
> +				struct intel_crtc_config *pipe_config)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	u32 tmp, flags = 0;
> +
> +	tmp = I915_READ(intel_dp->output_reg);
> +
> +	if (tmp & DP_SYNC_HS_HIGH)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (tmp & DP_SYNC_VS_HIGH)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_disable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -3170,6 +3192,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_encoder->disable = intel_disable_dp;
>  	intel_encoder->post_disable = intel_post_disable_dp;
>  	intel_encoder->get_hw_state = intel_dp_get_hw_state;
> +	intel_encoder->get_config = intel_dp_get_config;
>  	if (IS_VALLEYVIEW(dev))
>  		intel_encoder->pre_pll_enable = intel_dp_pre_pll_enable;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index dfcf546..abc4aa8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -140,6 +140,10 @@ struct intel_encoder {
>  	 * the encoder is active. If the encoder is enabled it also set the pipe
>  	 * it is connected to in the pipe parameter. */
>  	bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
> +	/* Reconstructs the equivalent mode flags for the current hardware
> +	 * state. */
> +	void (*get_config)(struct intel_encoder *,
> +			   struct intel_crtc_config *pipe_config);
>  	int crtc_mask;
>  	enum hpd_pin hpd_pin;
>  };
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 00e70db..91e9905 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -129,6 +129,26 @@ static bool intel_dvo_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_dvo_get_config(struct intel_encoder *encoder,
> +				 struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	struct intel_dvo *intel_dvo = enc_to_intel_dvo(&encoder->base);
> +	u32 tmp, flags = 0;
> +
> +	tmp = I915_READ(intel_dvo->dev.dvo_reg);
> +	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +	if (tmp & DVO_VSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_disable_dvo(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -440,6 +460,7 @@ void intel_dvo_init(struct drm_device *dev)
>  	intel_encoder->disable = intel_disable_dvo;
>  	intel_encoder->enable = intel_enable_dvo;
>  	intel_encoder->get_hw_state = intel_dvo_get_hw_state;
> +	intel_encoder->get_config = intel_dvo_get_config;
>  	intel_connector->get_hw_state = intel_dvo_connector_get_hw_state;
>  
>  	/* Now, try to find a controller */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 93de5ff..9091655 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -658,6 +658,28 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_hdmi_get_config(struct intel_encoder *encoder,
> +				  struct intel_crtc_config *pipe_config)
> +{
> +	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> +	u32 tmp, flags = 0;
> +
> +	tmp = I915_READ(intel_hdmi->hdmi_reg);
> +
> +	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (tmp & SDVO_VSYNC_ACTIVE_HIGH)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_enable_hdmi(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> @@ -1208,6 +1230,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  	intel_encoder->enable = intel_enable_hdmi;
>  	intel_encoder->disable = intel_disable_hdmi;
>  	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> +	intel_encoder->get_config = intel_hdmi_get_config;
>  	if (IS_VALLEYVIEW(dev)) {
>  		intel_encoder->pre_enable = intel_hdmi_pre_enable;
>  		intel_encoder->pre_pll_enable = intel_hdmi_pre_pll_enable;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index d256fe4..8d464a9 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -86,6 +86,31 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_lvds_get_config(struct intel_encoder *encoder,
> +				  struct intel_crtc_config *pipe_config)
> +{
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 lvds_reg, tmp, flags = 0;
> +
> +	if (HAS_PCH_SPLIT(dev))
> +		lvds_reg = PCH_LVDS;
> +	else
> +		lvds_reg = LVDS;
> +
> +	tmp = I915_READ(lvds_reg);
> +	if (tmp & LVDS_HSYNC_POLARITY)
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	if (tmp & LVDS_VSYNC_POLARITY)
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  /* The LVDS pin pair needs to be on before the DPLLs are enabled.
>   * This is an exception to the general rule that mode_set doesn't turn
>   * things on.
> @@ -920,6 +945,7 @@ bool intel_lvds_init(struct drm_device *dev)
>  	intel_encoder->compute_config = intel_lvds_compute_config;
>  	intel_encoder->disable = intel_disable_lvds;
>  	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
> +	intel_encoder->get_config = intel_lvds_get_config;
>  	intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
>  	intel_connector_attach_encoder(intel_connector, intel_encoder);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index a618a6a..271b0b4 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -712,6 +712,13 @@ static bool intel_sdvo_set_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
>  		intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
>  }
>  
> +static bool intel_sdvo_get_timing(struct intel_sdvo *intel_sdvo, u8 cmd,
> +				  struct intel_sdvo_dtd *dtd)
> +{
> +	return intel_sdvo_get_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->part1)) &&
> +		intel_sdvo_get_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->part2));
> +}
> +
>  static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo,
>  					 struct intel_sdvo_dtd *dtd)
>  {
> @@ -726,6 +733,13 @@ static bool intel_sdvo_set_output_timing(struct intel_sdvo *intel_sdvo,
>  				     SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd);
>  }
>  
> +static bool intel_sdvo_get_input_timing(struct intel_sdvo *intel_sdvo,
> +					struct intel_sdvo_dtd *dtd)
> +{
> +	return intel_sdvo_get_timing(intel_sdvo,
> +				     SDVO_CMD_GET_INPUT_TIMINGS_PART1, dtd);
> +}
> +
>  static bool
>  intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
>  					 uint16_t clock,
> @@ -1264,6 +1278,33 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
>  	return true;
>  }
>  
> +static void intel_sdvo_get_config(struct intel_encoder *encoder,
> +				  struct intel_crtc_config *pipe_config)
> +{
> +	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
> +	struct intel_sdvo_dtd dtd;
> +	u32 flags = 0;
> +	bool ret;
> +
> +	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
> +	if (!ret) {
> +		DRM_DEBUG_DRIVER("failed to retrieve SDVO DTD\n");
> +		return;
> +	}
> +
> +	if (dtd.part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> +		flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (dtd.part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> +		flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	pipe_config->adjusted_mode.flags |= flags;
> +}
> +
>  static void intel_disable_sdvo(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -2793,6 +2834,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
>  	intel_encoder->mode_set = intel_sdvo_mode_set;
>  	intel_encoder->enable = intel_enable_sdvo;
>  	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
> +	intel_encoder->get_config = intel_sdvo_get_config;
>  
>  	/* In default case sdvo lvds is false */
>  	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: add encoder get_config function v4

[Daniel Vetter]

On Thu, May 09, 2013 at 01:28:12AM +0300, Ville Syrjälä wrote:
> On Wed, May 08, 2013 at 02:01:02PM -0700, Jesse Barnes wrote:
> > We can use this for fetching encoder specific pipe_config state, like
> > mode flags, adjusted clock, etc.
> > 
> > Just used for mode flags atm, so we can check the pipe config state at
> > mode set time.
> > 
> > v2: get_config when checking hw state too
> > v3: fix DVO and LVDS mode flags (Ville)
> >     get SDVO DTD for flag fetch (Ville)
> > v4: use input timings (Ville)
> >     correct command used (Ville)
> >     remove gen4 check (Ville)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> I believe all my complaints have been addressed.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Re: [PATCH] drm: check user mode flags for validity

[Jesse Barnes]

On Thu, 9 May 2013 01:25:59 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Wed, May 08, 2013 at 02:01:25PM -0700, Jesse Barnes wrote:
> > Requested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/drm_crtc.c |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 792c3e3..72ae33a 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -1318,6 +1318,18 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
> >  	if (in->clock > INT_MAX || in->vrefresh > INT_MAX)
> >  		return -ERANGE;
> >  
> > +	/* Reject modes with invalid h/vsync */
> > +	if (!(in->flags & (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC)))
> > +		return -EINVAL;
> > +	if ((in->flags & DRM_MODE_FLAG_PHSYNC) &&
> > +	    (in->flags & DRM_MODE_FLAG_NHSYNC))
> > +		return -EINVAL;
> > +	if (!(in->flags & (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
> > +		return -EINVAL;
> > +	if ((in->flags & DRM_MODE_FLAG_PVSYNC) &&
> > +	    (in->flags & DRM_MODE_FLAG_NVSYNC))
> > +		return -EINVAL;
> 
> That might be a bit too drastic. Well I suppose making sure that both
> flags are not enabled at the same time could be OK. But having neither
> flag set could be perfectly legal (the user could be asking for composite
> sync instead for example).
> 
> 
> So my less drastic suggestion would be doing something like this in i915
> specific code:
> 
>  adjusted_mode->flags = 0;
>  if (requested_mode->flags & NHSYNC)
>  	adjusted_mode->flags |= NHSYNC;
>  else
>  	adjusted_mode->flags |= PHSYNC;
> 
> It would gurantee that we end up picking exactly one of the flags in
> every case. If both are set, we pick -, of neither is set we pick +.

Ah yeah I knew I was forgetting something... I'll drop the checks for
no flags.

You really think it would be better to do this in i915?  I guess it's
probably safe, but it seems nicer to filter this out where it might
occur (the EDID quirks should already deal with bogus flags for kernel
generated mode lists).

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: check user mode flags for validity

[Ville Syrjälä]

On Fri, May 10, 2013 at 09:08:08AM -0700, Jesse Barnes wrote:
> On Thu, 9 May 2013 01:25:59 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 
> > On Wed, May 08, 2013 at 02:01:25PM -0700, Jesse Barnes wrote:
> > > Requested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/drm_crtc.c |   12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index 792c3e3..72ae33a 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -1318,6 +1318,18 @@ static int drm_crtc_convert_umode(struct drm_display_mode *out,
> > >  	if (in->clock > INT_MAX || in->vrefresh > INT_MAX)
> > >  		return -ERANGE;
> > >  
> > > +	/* Reject modes with invalid h/vsync */
> > > +	if (!(in->flags & (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC)))
> > > +		return -EINVAL;
> > > +	if ((in->flags & DRM_MODE_FLAG_PHSYNC) &&
> > > +	    (in->flags & DRM_MODE_FLAG_NHSYNC))
> > > +		return -EINVAL;
> > > +	if (!(in->flags & (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
> > > +		return -EINVAL;
> > > +	if ((in->flags & DRM_MODE_FLAG_PVSYNC) &&
> > > +	    (in->flags & DRM_MODE_FLAG_NVSYNC))
> > > +		return -EINVAL;
> > 
> > That might be a bit too drastic. Well I suppose making sure that both
> > flags are not enabled at the same time could be OK. But having neither
> > flag set could be perfectly legal (the user could be asking for composite
> > sync instead for example).
> > 
> > 
> > So my less drastic suggestion would be doing something like this in i915
> > specific code:
> > 
> >  adjusted_mode->flags = 0;
> >  if (requested_mode->flags & NHSYNC)
> >  	adjusted_mode->flags |= NHSYNC;
> >  else
> >  	adjusted_mode->flags |= PHSYNC;
> > 
> > It would gurantee that we end up picking exactly one of the flags in
> > every case. If both are set, we pick -, of neither is set we pick +.
> 
> Ah yeah I knew I was forgetting something... I'll drop the checks for
> no flags.
> 
> You really think it would be better to do this in i915?  I guess it's
> probably safe, but it seems nicer to filter this out where it might
> occur (the EDID quirks should already deal with bogus flags for kernel
> generated mode lists).

I was thinking that we're generally quite relaxed about modesetting. The
user may specify something, but we might not really honor it due to
various mode fixups. Heck, if we have an LVDS or some other fixed mode
display, we ignore everything but the hdisp/vdisp from the user mode.

The same goes with the sync flags. For example if we're doing composite
or component output, it's always going to be composite sync, no matter
what the user asked for. And if we're cloning it at the same time, we
may be outputting both composite sync and separate h/v syncs at the same
time. Also the actual sync polarities may be limited by the hardware, so
not all cloned outputs might be using the same polarities.

We have no spec for how these flags should work, and we've been very
relaxed about them thus far. I'm mostly worried that if we add strict
checks, we may break some setup that works. So my idea would be to treat
that stuff as just hints.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: add encoder get_config function v4

[Daniel Vetter]

On Fri, May 10, 2013 at 9:21 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, May 09, 2013 at 01:28:12AM +0300, Ville Syrjälä wrote:
>> On Wed, May 08, 2013 at 02:01:02PM -0700, Jesse Barnes wrote:
>> > We can use this for fetching encoder specific pipe_config state, like
>> > mode flags, adjusted clock, etc.
>> >
>> > Just used for mode flags atm, so we can check the pipe config state at
>> > mode set time.
>> >
>> > v2: get_config when checking hw state too
>> > v3: fix DVO and LVDS mode flags (Ville)
>> >     get SDVO DTD for flag fetch (Ville)
>> > v4: use input timings (Ville)
>> >     correct command used (Ville)
>> >     remove gen4 check (Ville)
>> >
>> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>>
>> I believe all my complaints have been addressed.
>>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Queued for -next, thanks for the patch.

Dumped again since the ddi_encoder->get_config was missing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch