* [PATCH] drm/i915: add encoder get_config function v3 @ 2013-05-07 18:35 Jesse Barnes 2013-05-07 22:38 ` Jesse Barnes 0 siblings, 1 reply; 11+ messages in thread From: Jesse Barnes @ 2013-05-07 18:35 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] drm/i915: add encoder get_config function v3 2013-05-07 18:35 [PATCH] drm/i915: add encoder get_config function v3 Jesse Barnes @ 2013-05-07 22:38 ` Jesse Barnes 2013-05-08 10:39 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Jesse Barnes @ 2013-05-07 22:38 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: add encoder get_config function v3 2013-05-07 22:38 ` Jesse Barnes @ 2013-05-08 10:39 ` Ville Syrjälä 2013-05-08 21:01 ` [PATCH] drm/i915: add encoder get_config function v4 Jesse Barnes 2013-05-08 21:01 ` [PATCH] drm: check user mode flags for validity Jesse Barnes 0 siblings, 2 replies; 11+ messages in thread From: Ville Syrjälä @ 2013-05-08 10:39 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915: add encoder get_config function v4 2013-05-08 10:39 ` Ville Syrjälä @ 2013-05-08 21:01 ` Jesse Barnes 2013-05-08 22:28 ` Ville Syrjälä 2013-05-08 21:01 ` [PATCH] drm: check user mode flags for validity Jesse Barnes 1 sibling, 1 reply; 11+ messages in thread From: Jesse Barnes @ 2013-05-08 21:01 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: add encoder get_config function v4 2013-05-08 21:01 ` [PATCH] drm/i915: add encoder get_config function v4 Jesse Barnes @ 2013-05-08 22:28 ` Ville Syrjälä 2013-05-10 7:21 ` Daniel Vetter 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2013-05-08 22:28 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: add encoder get_config function v4 2013-05-08 22:28 ` Ville Syrjälä @ 2013-05-10 7:21 ` Daniel Vetter 2013-05-10 20:20 ` Daniel Vetter 0 siblings, 1 reply; 11+ messages in thread From: Daniel Vetter @ 2013-05-10 7:21 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: add encoder get_config function v4 2013-05-10 7:21 ` Daniel Vetter @ 2013-05-10 20:20 ` Daniel Vetter 0 siblings, 0 replies; 11+ messages in thread From: Daniel Vetter @ 2013-05-10 20:20 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm: check user mode flags for validity 2013-05-08 10:39 ` Ville Syrjälä 2013-05-08 21:01 ` [PATCH] drm/i915: add encoder get_config function v4 Jesse Barnes @ 2013-05-08 21:01 ` Jesse Barnes 2013-05-08 22:25 ` Ville Syrjälä 1 sibling, 1 reply; 11+ messages in thread From: Jesse Barnes @ 2013-05-08 21:01 UTC (permalink / raw) To: intel-gfx 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm: check user mode flags for validity 2013-05-08 21:01 ` [PATCH] drm: check user mode flags for validity Jesse Barnes @ 2013-05-08 22:25 ` Ville Syrjälä 2013-05-10 16:08 ` Jesse Barnes 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2013-05-08 22:25 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm: check user mode flags for validity 2013-05-08 22:25 ` Ville Syrjälä @ 2013-05-10 16:08 ` Jesse Barnes 2013-05-10 18:58 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Jesse Barnes @ 2013-05-10 16:08 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm: check user mode flags for validity 2013-05-10 16:08 ` Jesse Barnes @ 2013-05-10 18:58 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2013-05-10 18:58 UTC (permalink / raw) To: Jesse Barnes; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-05-10 20:20 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-07 18:35 [PATCH] drm/i915: add encoder get_config function v3 Jesse Barnes 2013-05-07 22:38 ` Jesse Barnes 2013-05-08 10:39 ` Ville Syrjälä 2013-05-08 21:01 ` [PATCH] drm/i915: add encoder get_config function v4 Jesse Barnes 2013-05-08 22:28 ` Ville Syrjälä 2013-05-10 7:21 ` Daniel Vetter 2013-05-10 20:20 ` Daniel Vetter 2013-05-08 21:01 ` [PATCH] drm: check user mode flags for validity Jesse Barnes 2013-05-08 22:25 ` Ville Syrjälä 2013-05-10 16:08 ` Jesse Barnes 2013-05-10 18:58 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.