[Intel-gfx] [PATCH] drm/i915: move infoframe setting to after pll enable v2

[Paulo Zanoni]

2014-04-04 18:38 GMT-03:00 Jesse Barnes <jbarnes at 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 at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni