diff mbox

drm/i915: move infoframe setting to after pll enable v2

Message ID 1396647521-9250-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes April 4, 2014, 9:38 p.m. UTC
Needs to happen after clock is running or it doesn't behave correctly.

v2: fix subject (Ville)
    make it clearer that this occurs in pre_enable (Paulo)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Paulo Zanoni April 4, 2014, 9:59 p.m. UTC | #1
2014-04-04 18:38 GMT-03:00 Jesse Barnes <jbarnes@virtuousgeek.org>:
> Needs to happen after clock is running or it doesn't behave correctly.
>
> v2: fix subject (Ville)
>     make it clearer that this occurs in pre_enable (Paulo)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c |   18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index fb9839b..05055f6 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -669,8 +669,6 @@ static void intel_hdmi_mode_set(struct intel_encoder *encoder)
>
>         I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
>         POSTING_READ(intel_hdmi->hdmi_reg);
> -
> -       intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
>  }
>
>  static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> @@ -1115,13 +1113,26 @@ done:
>         return 0;
>  }
>
> +static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
> +{
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> +       struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +       struct drm_display_mode *adjusted_mode =
> +               &intel_crtc->config.adjusted_mode;
> +
> +       intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
> +}
> +
>  static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>  {
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>         struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);

<bikeshed><OCD>
struct intel_hdmi *intel_hdmi = &dport->hdmi;

Because enc_to_intel_hdmi() just calls enc_to_dig_port() again, which
you already called on this funciton.
</OCD></bikeshed>


>         struct drm_device *dev = encoder->base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_crtc *intel_crtc =
>                 to_intel_crtc(encoder->base.crtc);
> +       struct drm_display_mode *adjusted_mode =
> +               &intel_crtc->config.adjusted_mode;
>         enum dpio_channel port = vlv_dport_to_channel(dport);
>         int pipe = intel_crtc->pipe;
>         u32 val;
> @@ -1155,6 +1166,8 @@ static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
>         vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW23(port), 0x00400888);
>         mutex_unlock(&dev_priv->dpio_lock);
>
> +       intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
> +
>         intel_enable_hdmi(encoder);
>
>         vlv_wait_port_ready(dev_priv, dport);
> @@ -1344,6 +1357,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>         intel_encoder->disable = intel_disable_hdmi;
>         intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
>         intel_encoder->get_config = intel_hdmi_get_config;
> +       intel_encoder->pre_enable = intel_hdmi_pre_enable;

<bikeshed>
Put the assignment above on the "else" case of the "if" below, to
reduce the probability that someone will quickly spot this assignment
and think it is also valid for VLV, because they will fail to notice
that pre_enable is replaced below.
</bikeshed>

We still didn't change HSW+, but I'll pretend I didn't see that :)

With or without the above: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>. But I still think it needs testing :)

>         if (IS_VALLEYVIEW(dev)) {
>                 intel_encoder->pre_pll_enable = vlv_hdmi_pre_pll_enable;
>                 intel_encoder->pre_enable = vlv_hdmi_pre_enable;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index fb9839b..05055f6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -669,8 +669,6 @@  static void intel_hdmi_mode_set(struct intel_encoder *encoder)
 
 	I915_WRITE(intel_hdmi->hdmi_reg, hdmi_val);
 	POSTING_READ(intel_hdmi->hdmi_reg);
-
-	intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
 }
 
 static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
@@ -1115,13 +1113,26 @@  done:
 	return 0;
 }
 
+static void intel_hdmi_pre_enable(struct intel_encoder *encoder)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config.adjusted_mode;
+
+	intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
+}
+
 static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
 {
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct intel_digital_port *dport = enc_to_dig_port(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc =
 		to_intel_crtc(encoder->base.crtc);
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config.adjusted_mode;
 	enum dpio_channel port = vlv_dport_to_channel(dport);
 	int pipe = intel_crtc->pipe;
 	u32 val;
@@ -1155,6 +1166,8 @@  static void vlv_hdmi_pre_enable(struct intel_encoder *encoder)
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW23(port), 0x00400888);
 	mutex_unlock(&dev_priv->dpio_lock);
 
+	intel_hdmi->set_infoframes(&encoder->base, adjusted_mode);
+
 	intel_enable_hdmi(encoder);
 
 	vlv_wait_port_ready(dev_priv, dport);
@@ -1344,6 +1357,7 @@  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 	intel_encoder->disable = intel_disable_hdmi;
 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
 	intel_encoder->get_config = intel_hdmi_get_config;
+	intel_encoder->pre_enable = intel_hdmi_pre_enable;
 	if (IS_VALLEYVIEW(dev)) {
 		intel_encoder->pre_pll_enable = vlv_hdmi_pre_pll_enable;
 		intel_encoder->pre_enable = vlv_hdmi_pre_enable;