[Intel-gfx] [PATCHv5] drm/i915: use waitqueue in wait for vblank

[Chris Wilson]

On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 56edff3..f97f0fe 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
>  		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
>  		for_each_pipe(pipe) {
> -			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
> +			if (pipe_stats[pipe] &
> +					PIPE_START_VBLANK_INTERRUPT_STATUS) {
>  				drm_handle_vblank(dev, pipe);
> +				wake_up_interruptible(&dev_priv->wait_vblank);

We now have intel_handle_vblank() so these chunks can be simplified.

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4d4a0d9..e1eb564 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
>  static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
> +	u32 vblank_cnt;
>  
> -	frame = I915_READ(frame_reg);
> +	vblank_cnt = drm_vblank_count(dev, pipe);
>  
> -	if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
> -		DRM_DEBUG_KMS("vblank wait timed out\n");
> +	/* TODO: get the vblank time dynamically or from platform data */
> +	wait_event_interruptible_timeout(dev_priv->wait_vblank,
> +			(vblank_cnt != drm_vblank_count(dev, pipe)),
> +			msecs_to_jiffies(16));

Keep the ultimate timeout at 50 until you have evidence you can reduce
it. And then it should be 2x vrefresh interval to be safe. However, you
are likely still hitting the timeout as the vblank irq is not guaranteed
to be enabled here. How safe calling drm_vblank_get() is during modeset
I defer to Ville since he has just taken a pass over the whole mess.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre