Re: [PATCH v2] drm: Make the decision to keep vblank irq enabled earlier

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 24, 2017 at 05:30:58PM +0000, Chris Wilson wrote:
> We want to provide the vblank irq shadow for pageflip events as well as
> vblank queries. Such events are completed within the vblank interrupt
> handler, and so the current check for disabling the irq will disable it
> from with the same interrupt as the last pageflip event. If we move the
> decision on whether to disable the irq (based on there no being no
> remaining vblank events, i.e. vblank->refcount == 0) to before we signal
> the events, we will only disable the irq on the interrupt after the last
> event was signaled. In the normal course of events, this will keep the
> vblank irq enabled for the entire flip sequence whereas before it would
> flip-flop around every interrupt.
> 
> v2: Move the disable_fn() call outside of the vblank_event_lock.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Michel Dänzer <michel@daenzer.net>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dave Airlie <airlied@redhat.com>,
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> #v1
> Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com> #v1

Pushed to drm-misc-next. Thanks for the patch and review.

> ---
>  drivers/gpu/drm/drm_irq.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 5b77057e91ca..a511597580d8 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1741,6 +1741,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>  	unsigned long irqflags;
> +	bool disable_irq;
>  
>  	if (WARN_ON_ONCE(!dev->num_crtcs))
>  		return false;
> @@ -1768,20 +1769,23 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
>  	spin_unlock(&dev->vblank_time_lock);
>  
>  	wake_up(&vblank->queue);
> -	drm_handle_vblank_events(dev, pipe);
>  
>  	/* With instant-off, we defer disabling the interrupt until after
> -	 * we finish processing the following vblank. The disable has to
> -	 * be last (after drm_handle_vblank_events) so that the timestamp
> -	 * is always accurate.
> +	 * we finish processing the following vblank after all events have
> +	 * been signaled. The disable has to be last (after
> +	 * drm_handle_vblank_events) so that the timestamp is always accurate.
>  	 */
> -	if (dev->vblank_disable_immediate &&
> -	    drm_vblank_offdelay > 0 &&
> -	    !atomic_read(&vblank->refcount))
> -		vblank_disable_fn((unsigned long)vblank);
> +	disable_irq = (dev->vblank_disable_immediate &&
> +		       drm_vblank_offdelay > 0 &&
> +		       !atomic_read(&vblank->refcount));
> +
> +	drm_handle_vblank_events(dev, pipe);
>  
>  	spin_unlock_irqrestore(&dev->event_lock, irqflags);
>  
> +	if (disable_irq)
> +		vblank_disable_fn((unsigned long)vblank);
> +
>  	return true;
>  }
>  EXPORT_SYMBOL(drm_handle_vblank);
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel