[Intel-gfx] [PATCH 1/3] drm/i915: add wait_for_vblank argument to intel_enable_pipe

[Jesse Barnes]

On Thu, 19 Dec 2013 19:12:29 -0200
Paulo Zanoni <przanoni at gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> Depending on the HW gen and the connector type, the pipe won't start
> running right after we call intel_enable_pipe, so that
> intel_wait_for_vblank call we currently have will just sit there for
> the full 50ms timeout. So this patch adds an argument that will allow
> us to avoid the vblank wait in case we want. Currently all the callers
> still request for the vblank wait, so the behavior should still be the
> same.
> 
> We also added a POSTING_READ on the register: previously
> intel_wait_for_vblank was acting as a POSTING_READ, but now if
> wait_for_vblank is false we'll stkip it, so we need an explicit
> POSTING_READ.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 869be78..6865fa2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1753,7 +1753,7 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
>   * returning.
>   */
>  static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
> -			      bool pch_port, bool dsi)
> +			      bool pch_port, bool dsi, bool wait_for_vblank)
>  {
>  	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
>  								      pipe);
> @@ -1796,7 +1796,9 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
>  		return;
>  
>  	I915_WRITE(reg, val | PIPECONF_ENABLE);
> -	intel_wait_for_vblank(dev_priv->dev, pipe);
> +	POSTING_READ(reg);
> +	if (wait_for_vblank)
> +		intel_wait_for_vblank(dev_priv->dev, pipe);
>  }
>  
>  /**
> @@ -3558,7 +3560,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_update_watermarks(crtc);
>  	intel_enable_pipe(dev_priv, pipe,
> -			  intel_crtc->config.has_pch_encoder, false);
> +			  intel_crtc->config.has_pch_encoder, false, true);
>  	intel_enable_primary_plane(dev_priv, plane, pipe);
>  	intel_enable_planes(crtc);
>  	intel_crtc_update_cursor(crtc, true);
> @@ -3704,7 +3706,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_update_watermarks(crtc);
>  	intel_enable_pipe(dev_priv, pipe,
> -			  intel_crtc->config.has_pch_encoder, false);
> +			  intel_crtc->config.has_pch_encoder, false, true);
>  
>  	if (intel_crtc->config.has_pch_encoder)
>  		lpt_pch_enable(crtc);
> @@ -4145,7 +4147,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  	intel_crtc_load_lut(crtc);
>  
>  	intel_update_watermarks(crtc);
> -	intel_enable_pipe(dev_priv, pipe, false, is_dsi);
> +	intel_enable_pipe(dev_priv, pipe, false, is_dsi, true);
>  	intel_enable_primary_plane(dev_priv, plane, pipe);
>  	intel_enable_planes(crtc);
>  	intel_crtc_update_cursor(crtc, true);
> @@ -4183,7 +4185,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  	intel_crtc_load_lut(crtc);
>  
>  	intel_update_watermarks(crtc);
> -	intel_enable_pipe(dev_priv, pipe, false, false);
> +	intel_enable_pipe(dev_priv, pipe, false, false, true);
>  	intel_enable_primary_plane(dev_priv, plane, pipe);
>  	intel_enable_planes(crtc);
>  	/* The fixup needs to happen before cursor is enabled */

Looks fine, though I echo Jani's comment.  Might be better to have an
explicitly named intel_enable_pipe_wait variant to make the code more
readable (likewise for DSI and PCH probably, if we don't need every
combination), but that can be a separate cleanup.

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center