| Message ID | 20170817122741.708-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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(-)