[PATCH 6/6] drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags
Daniel Vetter
daniel at ffwll.ch
Thu Aug 4 10:56:09 UTC 2016
On Thu, Aug 04, 2016 at 12:39:41PM +0900, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> These flags allow userspace to explicitly specify the target vertical
> blank period when a flip should take effect.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>
> Note that the previous patches in this series can avoid delaying page
> flips in some cases even without this patch or any corresponding
> userspace changes. Here are examples of how userspace can take advantage
> of this patch to avoid delaying page flips in even more cases:
>
> https://cgit.freedesktop.org/~daenzer/xf86-video-ati/commit/?id=fc884a8af25345c32bd4104c864ecfeb9bb3db9b
>
> https://cgit.freedesktop.org/~daenzer/xf86-video-amdgpu/commit/?id=b8631a9ba49c0d0ebe5dcd1dbfb68fcfe907296f
>
> drivers/gpu/drm/drm_crtc.c | 46 +++++++++++++++++++++++++++++++++++++++------
> drivers/gpu/drm/drm_ioctl.c | 8 ++++++++
> include/uapi/drm/drm.h | 1 +
> include/uapi/drm/drm_mode.h | 24 +++++++++++++++++++----
> 4 files changed, 69 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 15ad7e2..b2fd860 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5421,11 +5421,19 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> struct drm_crtc *crtc;
> struct drm_framebuffer *fb = NULL;
> struct drm_pending_vblank_event *e = NULL;
> - u32 target_vblank = 0;
> + u32 target_vblank = page_flip->sequence;
> int ret = -EINVAL;
>
> - if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
> - page_flip->reserved != 0)
> + if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS)
> + return -EINVAL;
> +
> + if (page_flip->sequence != 0 && !(page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET))
> + return -EINVAL;
> +
> + /* Only one of the DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags
> + * can be specified
> + */
> + if ((page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) == DRM_MODE_PAGE_FLIP_TARGET)
> return -EINVAL;
>
> if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip)
> @@ -5436,15 +5444,41 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> return -ENOENT;
>
> if (crtc->funcs->page_flip_target) {
> + u32 current_vblank;
> int r;
>
> r = drm_crtc_vblank_get(crtc);
> if (r)
> return r;
>
> - target_vblank = drm_crtc_vblank_count(crtc) +
> - !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
> - } else if (crtc->funcs->page_flip == NULL)
> + current_vblank = drm_crtc_vblank_count(crtc);
> +
> + switch (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET) {
> + case DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE:
> + if ((int)(target_vblank - current_vblank) > 1) {
> + DRM_DEBUG("Invalid absolute flip target %u, "
> + "must be <= %u\n", target_vblank,
> + current_vblank + 1);
> + drm_crtc_vblank_put(crtc);
> + return -EINVAL;
> + }
> + break;
> + case DRM_MODE_PAGE_FLIP_TARGET_RELATIVE:
> + if (target_vblank != 0 && target_vblank != 1) {
> + DRM_DEBUG("Invalid relative flip target %u, "
> + "must be 0 or 1\n", target_vblank);
> + drm_crtc_vblank_put(crtc);
> + return -EINVAL;
> + }
> + target_vblank += current_vblank;
> + break;
> + default:
> + target_vblank = current_vblank +
> + !(page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC);
> + break;
> + }
> + } else if (crtc->funcs->page_flip == NULL ||
> + (page_flip->flags & DRM_MODE_PAGE_FLIP_TARGET))
> return -EINVAL;
>
> drm_modeset_lock_crtc(crtc, crtc->primary);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 33af4a5..0099d2a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -228,6 +228,7 @@ static int drm_getstats(struct drm_device *dev, void *data,
> static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
> {
> struct drm_get_cap *req = data;
> + struct drm_crtc *crtc;
>
> req->value = 0;
> switch (req->capability) {
> @@ -254,6 +255,13 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
> case DRM_CAP_ASYNC_PAGE_FLIP:
> req->value = dev->mode_config.async_page_flip;
> break;
> + case DRM_CAP_PAGE_FLIP_TARGET:
> + req->value = 1;
> + drm_for_each_crtc(crtc, dev) {
> + if (!crtc->funcs->page_flip_target)
> + req->value = 0;
> + }
> + break;
> case DRM_CAP_CURSOR_WIDTH:
> if (dev->mode_config.cursor_width)
> req->value = dev->mode_config.cursor_width;
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 452675f..b2c5284 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -646,6 +646,7 @@ struct drm_gem_open {
> #define DRM_CAP_CURSOR_WIDTH 0x8
> #define DRM_CAP_CURSOR_HEIGHT 0x9
> #define DRM_CAP_ADDFB2_MODIFIERS 0x10
> +#define DRM_CAP_PAGE_FLIP_TARGET 0x11
>
> /** DRM_IOCTL_GET_CAP ioctl argument type */
> struct drm_get_cap {
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 49a7265..175aa1f 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -520,7 +520,13 @@ struct drm_color_lut {
>
> #define DRM_MODE_PAGE_FLIP_EVENT 0x01
> #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
> -#define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
> +#define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> +#define DRM_MODE_PAGE_FLIP_TARGET_RELATIVE 0x8
> +#define DRM_MODE_PAGE_FLIP_TARGET (DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE | \
> + DRM_MODE_PAGE_FLIP_TARGET_RELATIVE)
> +#define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT | \
> + DRM_MODE_PAGE_FLIP_ASYNC | \
> + DRM_MODE_PAGE_FLIP_TARGET)
>
> /*
> * Request a page flip on the specified crtc.
> @@ -543,15 +549,25 @@ struct drm_color_lut {
> * 'as soon as possible', meaning that it not delay waiting for vblank.
> * This may cause tearing on the screen.
> *
> - * The reserved field must be zero until we figure out something
> - * clever to use it for.
> + * The sequence field must be zero unless either of the
> + * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is specified. When
> + * the ABSOLUTE flag is specified, the sequence field denotes the absolute
> + * vblank sequence when the flip should take effect. When the RELATIVE
> + * flag is specified, the sequence field denotes the relative (to the
> + * current one when the ioctl is called) vblank sequence when the flip
> + * should take effect. NOTE: DRM_IOCTL_WAIT_VBLANK must still be used to
> + * make sure the vblank sequence before the target one has passed before
> + * calling this ioctl. The purpose of the
> + * DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags is merely to clarify
> + * the target for when code dealing with a page flip runs during a
> + * vertical blank period.
> */
>
> struct drm_mode_crtc_page_flip {
> __u32 crtc_id;
> __u32 fb_id;
> __u32 flags;
> - __u32 reserved;
> + __u32 sequence;
Might break abi somewhere. I think it'd be better to create a
struct drm_mode_crtc_page_flip2 with the renamed field. Otherwise this
looks great, and the only other quibble I have is that we extend the old
page_flip path here instead of atomic. But given all the fun we have
trying to port i915 to atomic I fully understand that you can't simply
first port amdgpu over ;-)
With or without the above (but strongingly in support of a new struct):
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> __u64 user_data;
> };
>
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the amd-gfx
mailing list