[PATCH] drm: Wake up all waiters for the vblank

[PATCH] drm: Wake up all waiters for the vblank

[Chris Wilson]

As we have a single list for vblank waiters (not sorted by the vblank
they are waiting for), make sure we wake up all waiters rather than just
the first as we may have multiple consumers for this vblank event.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_vblank.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 70f2b9593edc..930e3ed5234b 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1118,7 +1118,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 	if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
 		drm_vblank_disable_and_save(dev, pipe);
 
-	wake_up(&vblank->queue);
+	wake_up_all(&vblank->queue);
 
 	/*
 	 * Prevent subsequent drm_vblank_get() from re-enabling
@@ -1572,7 +1572,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
 
 	spin_unlock(&dev->vblank_time_lock);
 
-	wake_up(&vblank->queue);
+	wake_up_all(&vblank->queue);
 
 	/* With instant-off, we defer disabling the interrupt until after
 	 * we finish processing the following vblank after all events have
-- 
2.14.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Wake up all waiters for the vblank

[Ville Syrjälä]

On Thu, Aug 17, 2017 at 01:27:41PM +0100, Chris Wilson wrote:
> As we have a single list for vblank waiters (not sorted by the vblank
> they are waiting for), make sure we wake up all waiters rather than just
> the first as we may have multiple consumers for this vblank event.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_vblank.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 70f2b9593edc..930e3ed5234b 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1118,7 +1118,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>  	if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
>  		drm_vblank_disable_and_save(dev, pipe);
>  
> -	wake_up(&vblank->queue);
> +	wake_up_all(&vblank->queue);

I don't think we have any exclusive waiters currently so this shouldn't
actually change anything. I guess given the unordered nature of vblank
waiters exclusive waiters wouldn't really work anyway.

I guess it might be a reasonable measure to avoid bugs if someone
accidentally adds eclusive waiters so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	/*
>  	 * Prevent subsequent drm_vblank_get() from re-enabling
> @@ -1572,7 +1572,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
>  
>  	spin_unlock(&dev->vblank_time_lock);
>  
> -	wake_up(&vblank->queue);
> +	wake_up_all(&vblank->queue);
>  
>  	/* With instant-off, we defer disabling the interrupt until after
>  	 * we finish processing the following vblank after all events have
> -- 
> 2.14.1

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

Re: [PATCH] drm: Wake up all waiters for the vblank

[Chris Wilson]

Quoting Ville Syrjälä (2017-08-17 15:56:19)
> On Thu, Aug 17, 2017 at 01:27:41PM +0100, Chris Wilson wrote:
> > As we have a single list for vblank waiters (not sorted by the vblank
> > they are waiting for), make sure we wake up all waiters rather than just
> > the first as we may have multiple consumers for this vblank event.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_vblank.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 70f2b9593edc..930e3ed5234b 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -1118,7 +1118,7 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> >       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
> >               drm_vblank_disable_and_save(dev, pipe);
> >  
> > -     wake_up(&vblank->queue);
> > +     wake_up_all(&vblank->queue);
> 
> I don't think we have any exclusive waiters currently so this shouldn't
> actually change anything. I guess given the unordered nature of vblank
> waiters exclusive waiters wouldn't really work anyway.

Gah, I had it in my head as they were two separate modes of working -
one for the waiter as to whether it should be treated exclusively (for
an exclusive wakeup), and the other for the waker to control how many it
should wake up. My fingers were crossed that we had some small bugs given
the multiple drmWaitVblank clients (plus more users in the kernel)
springing up that would miraculously be resolved. I hope we don't find
ourselves in a position where we do need to kill a thundering herd of
vblank waiters.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel