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

[Daniel Vetter <daniel@ffwll.ch>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx