Re: [PATCH 11/17] drm/i915: Make sure DPLL is enabled when kicking the power sequencer on VLV/CHV

Daniel Vetter
2014-10-28T08:22:01Z

On Thu, Oct 16, 2014 at 09:29:45PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The power seqeuencer kick procedure requires the DPLL to be running
> in order to complete succesfully. In case the DPLL isn't currently
> running when we need to kick the power seqeuncer enable it
> temporarily. This can happen eg. during ->detect() when the pipe is
> not already active.
>
> To avoid needlessly duplicating the DPLL programming re-use the already
> existing functions by passing a temporary pipe config to them instead
> of having them consult the current pipe config at crtc->config.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

[snip]

> + /*
> + * The DPLL for the pipe must be enabled for this to work.
> + * So enable temporarily it if it's not already enabled.
> + */
> + if (!pll_enabled) {
> + struct intel_crtc *crtc =
> + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + struct intel_crtc_config pipe_config = {
> + .pixel_multiplier = 1,
> + .dpll = IS_CHERRYVIEW(dev) ?
> + chv_dpll[0].dpll : vlv_dpll[0].dpll,
> + .port_clock = 162000,
> + .clock_set = true,
> + .has_dp_encoder = true,
> + };
> +
> + if (IS_CHERRYVIEW(dev)) {
> + chv_update_pll(crtc, &pipe_config);
> + chv_prepare_pll(crtc, &pipe_config);
> + chv_enable_pll(crtc, &pipe_config);
> + } else {
> + vlv_update_pll(crtc, &pipe_config);
> + vlv_prepare_pll(crtc, &pipe_config);
> + vlv_enable_pll(crtc, &pipe_config);

I'm not terribly happy with the massive list of non-static platform
functions this exposes. And it's also fairly leaky since the minimal pipe
config depends upon what exactly the vlv/chv pll functions need.

Could we instead have a wrapper function for both the enable and disable
which sits right next to the vlv/chv dpll code in intel_display.c? E.g.
valleyview_force_pll_on/off(dev, pipe) or something like that.
-Daniel

> + }
> + }
> +
> /*
> * Similar magic as in intel_dp_enable_port().
> * We _must_ do this port enable + disable trick
> @@ -365,6 +395,13 @@ vlv_power_sequencer_kick(struct intel_dp *intel_dp)
>
> I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
> POSTING_READ(intel_dp->output_reg);
> +
> + if (!pll_enabled) {
> + if (IS_CHERRYVIEW(dev))
> + chv_disable_pll(dev_priv, pipe);
> + else
> + vlv_disable_pll(dev_priv, pipe);
> + }
> }
>
> static enum pipe
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f7ba1fc..69c8c5f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -911,6 +911,21 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
> void intel_put_shared_dpll(struct intel_crtc *crtc);
>
> +void vlv_update_pll(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config);
> +void chv_update_pll(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config);
> +void vlv_prepare_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_config *pipe_config);
> +void chv_prepare_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_config *pipe_config);
> +void vlv_enable_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_config *pipe_config);
> +void chv_enable_pll(struct intel_crtc *crtc,
> + const struct intel_crtc_config *pipe_config);
> +void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe);
> +void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe);
> +
> /* modesetting asserts */
> void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> enum pipe pipe);
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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