On Fri, Oct 31, 2014 at 6:54 PM, Steve Longerbeam <slongerbeam@xxxxxxxxx> wrote:
> Planes like crtcs would benefit from having page flip event support.
> With planes page flip it is now possible to synchronize changing a
> framebuffer used by an overlay (or even cursor) plane with vertical
> sync events.
>
> A page flip in the primary plane is equivalent to a crtc page flip,
> which suggests this ioctl could deprecate the crtc page flip ioctl
> at some point in the future.
jfyi, a plane_page_flip ioctl would be useful, except it is about to
be obsolete by the atomic stuff that Daniel is working on
BR,
-R
> Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
> Signed-off-by: Dmitry Eremin-Solenikov <dmitry_eremin@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_crtc.c | 241 +++++++++++++++++++++++++------------------
> drivers/gpu/drm/drm_ioctl.c | 1 +
> include/drm/drm_crtc.h | 11 ++
> include/uapi/drm/drm.h | 1 +
> include/uapi/drm/drm_mode.h | 8 ++
> 5 files changed, 164 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e79c8d3..53b5f87 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2445,6 +2445,142 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
> plane_req->src_w, plane_req->src_h);
> }
>
> +int drm_mode_plane_page_flip_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_plane_page_flip *page_flip = data;
> + struct drm_plane *plane;
> + struct drm_crtc *crtc;
> + struct drm_framebuffer *fb = NULL;
> + struct drm_pending_vblank_event *e = NULL;
> + unsigned long flags;
> + bool is_primary;
> + int ret;
> +
> + if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
> + page_flip->reserved != 0)
> + return -EINVAL;
> +
> + if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) &&
> + !dev->mode_config.async_page_flip)
> + return -EINVAL;
> +
> + plane = drm_plane_find(dev, page_flip->plane_id);
> + if (!plane)
> + return -ENOENT;
> +
> + is_primary = (plane->type == DRM_PLANE_TYPE_PRIMARY);
> +
> + drm_modeset_lock_all(dev);
> +
> + if (plane->crtc == NULL || plane->fb == NULL) {
> + /* The crtc and/or framebuffer is currently unbound,
> + * presumably due to a hotplug event, that userspace
> + * has not yet discovered.
> + */
> + ret = -EBUSY;
> + goto out;
> + }
> + crtc = plane->crtc;
> +
> + if ((is_primary && crtc->funcs->page_flip == NULL) ||
> + plane->funcs->page_flip == NULL) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
> + if (!fb) {
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + /*
> + * If this is the primary plane (equivalent to a crtc page flip),
> + * verify that the fb is valid for the bound crtc's viewport.
> + * For overlay or cursor planes, the plane's page_flip op should
> + * check that the fb is valid.
> + */
> + if (is_primary) {
> + ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y,
> + &crtc->mode, fb);
> + if (ret)
> + goto out;
> + }
> +
> + if (plane->fb->pixel_format != fb->pixel_format) {
> + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> + ret = -ENOMEM;
> + spin_lock_irqsave(&dev->event_lock, flags);
> + if (file_priv->event_space < sizeof(e->event)) {
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + goto out;
> + }
> + file_priv->event_space -= sizeof(e->event);
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> + e = kzalloc(sizeof(*e), GFP_KERNEL);
> + if (e == NULL) {
> + spin_lock_irqsave(&dev->event_lock, flags);
> + file_priv->event_space += sizeof(e->event);
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + goto out;
> + }
> +
> + e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
> + e->event.base.length = sizeof(e->event);
> + e->event.user_data = page_flip->user_data;
> + e->base.event = &e->event.base;
> + e->base.file_priv = file_priv;
> + e->base.destroy =
> + (void (*) (struct drm_pending_event *)) kfree;
> + }
> +
> + plane->old_fb = plane->fb;
> +
> + /* for now use crtc page_flip op, if this is the primary plane */
> + if (is_primary)
> + ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
> + else
> + ret = plane->funcs->page_flip(plane, fb, e, page_flip->flags);
> +
> + if (ret) {
> + if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> + spin_lock_irqsave(&dev->event_lock, flags);
> + file_priv->event_space += sizeof(e->event);
> + spin_unlock_irqrestore(&dev->event_lock, flags);
> + kfree(e);
> + }
> + /* Keep the old fb, don't unref it. */
> + plane->old_fb = NULL;
> + } else {
> + /*
> + * Warn if the driver hasn't properly updated the plane->fb
> + * field to reflect that the new framebuffer is now used.
> + * Failing to do so will screw with the reference counting
> + * on framebuffers.
> + */
> + WARN_ON(plane->fb != fb);
> + /* Unref only the old framebuffer. */
> + fb = NULL;
> + }
> +
> +out:
> + if (fb)
> + drm_framebuffer_unreference(fb);
> + if (plane->old_fb)
> + drm_framebuffer_unreference(plane->old_fb);
> + plane->old_fb = NULL;
> + drm_modeset_unlock_all(dev);
> +
> + return ret;
> +}
> +
> /**
> * drm_mode_set_config_internal - helper to call ->set_config
> * @set: modeset config to set
> @@ -4582,111 +4718,20 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> struct drm_mode_crtc_page_flip *page_flip = data;
> + struct drm_mode_plane_page_flip primary_pf = {0};
> struct drm_crtc *crtc;
> - struct drm_framebuffer *fb = NULL;
> - struct drm_pending_vblank_event *e = NULL;
> - unsigned long flags;
> - int ret = -EINVAL;
> -
> - if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ||
> - page_flip->reserved != 0)
> - return -EINVAL;
> -
> - if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip)
> - return -EINVAL;
>
> crtc = drm_crtc_find(dev, page_flip->crtc_id);
> if (!crtc)
> return -ENOENT;
>
> - drm_modeset_lock_crtc(crtc);
> - if (crtc->primary->fb == NULL) {
> - /* The framebuffer is currently unbound, presumably
> - * due to a hotplug event, that userspace has not
> - * yet discovered.
> - */
> - ret = -EBUSY;
> - goto out;
> - }
> -
> - if (crtc->funcs->page_flip == NULL)
> - goto out;
> -
> - fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
> - if (!fb) {
> - ret = -ENOENT;
> - goto out;
> - }
> -
> - ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, &crtc->mode, fb);
> - if (ret)
> - goto out;
> -
> - if (crtc->primary->fb->pixel_format != fb->pixel_format) {
> - DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> - ret = -ENOMEM;
> - spin_lock_irqsave(&dev->event_lock, flags);
> - if (file_priv->event_space < sizeof e->event) {
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - goto out;
> - }
> - file_priv->event_space -= sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> -
> - e = kzalloc(sizeof *e, GFP_KERNEL);
> - if (e == NULL) {
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - goto out;
> - }
> -
> - e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
> - e->event.base.length = sizeof e->event;
> - e->event.user_data = page_flip->user_data;
> - e->base.event = &e->event.base;
> - e->base.file_priv = file_priv;
> - e->base.destroy =
> - (void (*) (struct drm_pending_event *)) kfree;
> - }
> -
> - crtc->primary->old_fb = crtc->primary->fb;
> - ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
> - if (ret) {
> - if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> - spin_lock_irqsave(&dev->event_lock, flags);
> - file_priv->event_space += sizeof e->event;
> - spin_unlock_irqrestore(&dev->event_lock, flags);
> - kfree(e);
> - }
> - /* Keep the old fb, don't unref it. */
> - crtc->primary->old_fb = NULL;
> - } else {
> - /*
> - * Warn if the driver hasn't properly updated the crtc->fb
> - * field to reflect that the new framebuffer is now used.
> - * Failing to do so will screw with the reference counting
> - * on framebuffers.
> - */
> - WARN_ON(crtc->primary->fb != fb);
> - /* Unref only the old framebuffer. */
> - fb = NULL;
> - }
> + primary_pf.plane_id = crtc->primary->base.id;
> + primary_pf.fb_id = page_flip->fb_id;
> + primary_pf.flags = page_flip->flags;
> + primary_pf.user_data = page_flip->user_data;
>
> -out:
> - if (fb)
> - drm_framebuffer_unreference(fb);
> - if (crtc->primary->old_fb)
> - drm_framebuffer_unreference(crtc->primary->old_fb);
> - crtc->primary->old_fb = NULL;
> - drm_modeset_unlock_crtc(crtc);
> -
> - return ret;
> + return drm_mode_plane_page_flip_ioctl(dev, (void *)&primary_pf,
> + file_priv);
> }
>
> /**
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 00587a1..2e60ee3 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -620,6 +620,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_PLANE_PAGE_FLIP, drm_mode_plane_page_flip_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c40070a..26d859b 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -571,6 +571,7 @@ struct drm_connector {
> * @disable_plane: shut down the plane
> * @destroy: clean up plane resources
> * @set_property: called when a property is changed
> + * @page_flip: initiate a page flip
> */
> struct drm_plane_funcs {
> int (*update_plane)(struct drm_plane *plane,
> @@ -585,6 +586,14 @@ struct drm_plane_funcs {
>
> int (*set_property)(struct drm_plane *plane,
> struct drm_property *property, uint64_t val);
> +
> + /*
> + * See comment at drm_crtc_funcs.page_flip
> + */
> + int (*page_flip)(struct drm_plane *plane,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *event,
> + uint32_t flags);
> };
>
> enum drm_plane_type {
> @@ -1105,6 +1114,8 @@ extern bool drm_detect_monitor_audio(struct edid *edid);
> extern bool drm_rgb_quant_range_selectable(struct edid *edid);
> extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
> +extern int drm_mode_plane_page_flip_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> extern int drm_add_modes_noedid(struct drm_connector *connector,
> int hdisplay, int vdisplay);
> extern void drm_set_preferred_mode(struct drm_connector *connector,
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b0b8556..48d9da6 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -777,6 +777,7 @@ struct drm_prime_handle {
> #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
> #define DRM_IOCTL_MODE_OBJ_SETPROPERTY DRM_IOWR(0xBA, struct drm_mode_obj_set_property)
> #define DRM_IOCTL_MODE_CURSOR2 DRM_IOWR(0xBB, struct drm_mode_cursor2)
> +#define DRM_IOCTL_MODE_PLANE_PAGE_FLIP DRM_IOWR(0xBC, struct drm_mode_plane_page_flip)
>
> /**
> * Device specific ioctls should only be in their respective headers
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index a0db2d4a..8d98cd0 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -488,6 +488,14 @@ struct drm_mode_crtc_page_flip {
> __u64 user_data;
> };
>
> +struct drm_mode_plane_page_flip {
> + __u32 plane_id;
> + __u32 fb_id;
> + __u32 flags;
> + __u32 reserved;
> + __u64 user_data;
> +};
> +
> /* create a dumb scanout buffer */
> struct drm_mode_create_dumb {
> uint32_t height;
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
