On Thu, Apr 18, 2013 at 11:51 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> +static void valleyview_crtc_enable(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_encoder *encoder;
> + int pipe = intel_crtc->pipe;
> + int plane = intel_crtc->plane;
> +
> + WARN_ON(!crtc->enabled);
> +
> + if (intel_crtc->active)
> + return;
> +
> + intel_crtc->active = true;
> + intel_update_watermarks(dev);
> +
> + mutex_lock(&dev_priv->dpio_lock);
> +
> + for_each_encoder_on_crtc(dev, crtc, encoder)
> + if (encoder->pre_pll_enable)
> + encoder->pre_pll_enable(encoder);
> +
> + intel_enable_pll(dev_priv, pipe);
> +
> + for_each_encoder_on_crtc(dev, crtc, encoder)
> + if (encoder->pre_enable)
> + encoder->pre_enable(encoder);
> +
> + for_each_encoder_on_crtc(dev, crtc, encoder)
> + encoder->enable(encoder);
So I remember complaining about this and that calling both
->pre_enable and ->enable at the same spot is ugly. But you've
strongarmed away my proposal to do the same we're doing for ilk-ivb
cpu-edp ports (or something more cute withe special cpu port enable
functions). Anyway, my subconsciousness finally told me why it's a bad
idea:
eDP backlight is now turned on too early, and we don't have a hook any
more at the end of the modeset sequence to do that.
Whoever touches this next (or related code in intel_dp/hdmi.c) will be
volunteer to fix it all up.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
