On Thu, Jul 18, 2019 at 09:47:09PM +0200, Sam Ravnborg wrote: > From e6f70cb90e0c3c90d45017a8257353652b7e0dcc Mon Sep 17 00:00:00 2001 > From: Sam Ravnborg <[email protected]> > Date: Thu, 30 May 2019 09:38:47 +0200 > Subject: [PATCH] drm/vblank: drop use of DRM_WAIT_ON() > > 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 and to allow > different debug prints depending on the result. > > Signed-off-by: Sam Ravnborg <[email protected]> > Reviewed-by: Sean Paul <[email protected]>
A bit bold to keep the r-b for this substantial bugfix to the patch. Also not per-patch changelog here. Plus doesn't seem to be on dri-devel somehow. > Cc: Maarten Lankhorst <[email protected]> > Cc: Maxime Ripard <[email protected]> > Cc: David Airlie <[email protected]> > Cc: Daniel Vetter <[email protected]> > --- > > To verify if it works with the fix. > (Added wat variable to handle the situation where we never wait) > > Sam > > drivers/gpu/drm/drm_vblank.c | 32 ++++++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 603ab105125d..4e83f1dfd446 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -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" > @@ -1576,7 +1575,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void > *data, > struct drm_crtc *crtc; > struct drm_vblank_crtc *vblank; > union drm_wait_vblank *vblwait = data; > - int ret; > + int ret, wait; > u64 req_seq, seq; > unsigned int pipe_index; > unsigned int flags, pipe, high_pipe; > @@ -1669,22 +1668,35 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, > void *data, > return drm_queue_vblank_event(dev, pipe, req_seq, vblwait, > file_priv); > } > > + wait = 1; Instead of this hack I think better to remap the ret value in the if condition. This here is really hard to read. -Daniel > if (req_seq != seq) { > 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)); > } > > - if (ret != -EINTR) { > + switch (wait) { > + case 0: > + /* timeout */ > + ret = -EBUSY; > drm_wait_vblank_reply(dev, pipe, &vblwait->reply); > - > - DRM_DEBUG("crtc %d returning %u to client\n", > + DRM_DEBUG("timeout waiting for vblank. crtc %d returning %u to > client\n", > pipe, vblwait->reply.sequence); > - } else { > + break; > + case -ERESTARTSYS: > + /* interrupted by signal */ > + ret = -EINTR; > DRM_DEBUG("crtc %d vblank wait interrupted by signal\n", pipe); > + break; > + default: > + ret = 0; > + drm_wait_vblank_reply(dev, pipe, &vblwait->reply); > + DRM_DEBUG("crtc %d returning %u to client\n", > + pipe, vblwait->reply.sequence); > + break; > } > > done: > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
