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 <sam@xxxxxxxxxxxx>
> 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 <sam@xxxxxxxxxxxx>
> Reviewed-by: Sean Paul <sean@xxxxxxxxxx>
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 <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> ---
>
> 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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
