On Thu, Aug 04, 2016 at 12:39:41PM +0900, Michel Dänzer wrote: > From: Michel Dänzer <michel.daenzer@xxxxxxx> > > 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@xxxxxxx> > --- > > 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@xxxxxxxx> > __u64 user_data; > }; > > -- > 2.8.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
