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

[Daniel Vetter]

On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
> Instead of using sleep functions in wait for vblank, which wakes up on
> sleep timeout aften, thereby scheduler provoking scheduler. Hence use
> waitqueue in wait for vblank.
> 
> Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>

You're rebuilding infrastructure that drm_irq.c already has. In latest
nightly all you really need should be

	drm_crtc_vblank_get();
	wait_event_timeout(dev->vblank[drm_crtc_index()].queue, ...);
	drm_crtc_vblank_put();

If you fancy it you could wrap this little sequence up into a new helper
in drm_irq.c called drm_crtc_wait_one_vblank(). Please add kerneldoc for
that, too.

Also latest upstream WARNs if the vblank wait times out, we want that
here, too. And there's no need to optimize the timeout since it's an
exceptional condition which should never happen. If it does it's a driver
bug and needs to be addressed.

Note that you might need to shuffle around the drm_crtc_vblank_on/off
calls a bit to make sure the vblank support is enabled again.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |    2 ++
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/i915_irq.c      |   17 +++++++++++++----
>  drivers/gpu/drm/i915/intel_display.c |   10 ++++++----
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 258b1be..896620c 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1506,6 +1506,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	mutex_init(&dev_priv->dpio_lock);
>  	mutex_init(&dev_priv->modeset_restore_lock);
>  
> +	init_waitqueue_head(&dev_priv->wait_vblank);
> +
>  	intel_pm_setup(dev);
>  
>  	intel_display_crc_init(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 728b9c3..449bfef 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1590,6 +1590,7 @@ typedef struct drm_i915_private {
>  	struct i915_dri1_state dri1;
>  	/* Old ums support infrastructure, same warning applies. */
>  	struct i915_ums_state ums;
> +	wait_queue_head_t wait_vblank;
>  } drm_i915_private_t;
>  
>  static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> 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);
> +			}
>  
>  			if (pipe_stats[pipe] & PLANE_FLIPDONE_INT_STATUS_VLV) {
>  				intel_prepare_page_flip(dev, pipe);
> @@ -3269,8 +3272,10 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  				plane = !plane;
>  
>  			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
> -			    i8xx_handle_vblank(dev, plane, pipe, iir))
> +			    i8xx_handle_vblank(dev, plane, pipe, iir)) {
>  				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
> +				wake_up_interruptible(&dev_priv->wait_vblank);
> +			}
>  
>  			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>  				i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -3464,8 +3469,10 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>  				plane = !plane;
>  
>  			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
> -			    i915_handle_vblank(dev, plane, pipe, iir))
> +			    i915_handle_vblank(dev, plane, pipe, iir)) {
>  				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
> +				wake_up_interruptible(&dev_priv->wait_vblank);
> +			}
>  
>  			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
>  				blc_event = true;
> @@ -3709,8 +3716,10 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  
>  		for_each_pipe(pipe) {
>  			if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
> -			    i915_handle_vblank(dev, pipe, pipe, iir))
> +			    i915_handle_vblank(dev, pipe, pipe, iir)) {
>  				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe);
> +				wake_up_interruptible(&dev_priv->wait_vblank);
> +			}
>  
>  			if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
>  				blc_event = true;
> 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));
>  }
>  
>  /**
> -- 
> 1.7.9.5
> 

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