Hi all.
On Fri, Jul 26, 2019 at 11:06:58PM +0200, Sam Ravnborg wrote:
> 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.
Patch passes igt testing, and Michel was happy with it.
It is now applied to drm-misc-next and pushed out.
With this we have one less user of drm_os_linux.h
(only very few remains)
And one less cryptic macro use.
Sam
>
> 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 implmentation
>
> 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@xxxxxxxxxxxx>
> Cc: "Michel Dänzer" <michel@xxxxxxxxxxx>
> Cc: Sean Paul <sean@xxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> ---
>
> This patch survives my testing here.
> vbltest spits out the same value as before, and I can see something on
> the screen.
> Added intel-gfx@xxxxxxxxxxxxxxxxxxxxx, as IGT have identified troubles
> with this in v1.
>
> Sam
>
> drivers/gpu/drm/drm_vblank.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 603ab105125d..fd1fbc77871f 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"
> @@ -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) {
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
