drm/vblank: drop use of DRM_WAIT_ON()
authorSam Ravnborg <sam@ravnborg.org>
Fri, 26 Jul 2019 21:06:58 +0000 (23:06 +0200)
committerSam Ravnborg <sam@ravnborg.org>
Sat, 3 Aug 2019 14:07:39 +0000 (16:07 +0200)
DRM_WAIT_ON() is from the deprecated drm_os_linux header and
the modern replacement is the wait_event_*.

The return values differ, so a conversion is needed to
keep the original interface towards userspace.
Introduced a switch/case to make code obvious.

Analysis from Michel Dänzer:

The waiting condition rely on all relevant places where vblank_count
is modified calls wake_up(&vblank->queue).

drm_handle_vblank():
- Calls wake_up(&vblank->queue)

drm_vblank_enable():
- There is no need here because there can be no sleeping waiters
  in the queue, because vblank->enabled == false immediately
  terminates any waits.

drm_crtc_accurate_vblank_count():
- This is called from interrupt handlers, at least from
  amdgpu_dm.c:dm_pflip_high_irq(). Not sure it needs to wake up
  the queue though, the driver should call
  drm_(crtc_)_handle_vblank anyway.

drm_vblank_disable_and_save():
- It can be called from an interrupt, via drm_handle_vblank ->
  vblank_disable_fn. However, the only place where
  drm_vblank_disable_and_save can be called with sleeping waiters
  in the queue is in drm_crtc_vblank_off, which wakes up the queue
  afterwards (which terminates all waits, because
  vblank->enabled == false at this point).

v3:
- Added analysis to changelog from Michel Dänzer
- Moved return result handling inside if (req_seq != seq) (Daniel V)
- Reused more of the former logic - resulting in simpler code
- Dropped Reviewed-by from Sean Paul as this is a new implementation

v2:
- Fix so the case where req_seq equals seq was handled properly
- quick hack to check if IGT became happy
- Only sent to igt, not to dri-devel

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190726210658.GA6299@ravnborg.org
drivers/gpu/drm/drm_vblank.c

index 603ab105125d4112aada8f72bb79a9b766f38c38..fd1fbc77871f1eefce920f49e6e2939cb96cdf18 100644 (file)
@@ -31,7 +31,6 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_print.h>
-#include <drm/drm_os_linux.h>
 #include <drm/drm_vblank.h>
 
 #include "drm_internal.h"
@@ -1670,12 +1669,28 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
        }
 
        if (req_seq != seq) {
+               int wait;
+
                DRM_DEBUG("waiting on vblank count %llu, crtc %u\n",
                          req_seq, pipe);
-               DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
-                           vblank_passed(drm_vblank_count(dev, pipe),
-                                         req_seq) ||
-                           !READ_ONCE(vblank->enabled));
+               wait = wait_event_interruptible_timeout(vblank->queue,
+                       vblank_passed(drm_vblank_count(dev, pipe), req_seq) ||
+                                     !READ_ONCE(vblank->enabled),
+                       msecs_to_jiffies(3000));
+
+               switch (wait) {
+               case 0:
+                       /* timeout */
+                       ret = -EBUSY;
+                       break;
+               case -ERESTARTSYS:
+                       /* interrupted by signal */
+                       ret = -EINTR;
+                       break;
+               default:
+                       ret = 0;
+                       break;
+               }
        }
 
        if (ret != -EINTR) {