[PATCH 53/72] drm: implement page flipping support for planes

Rob Clark robdclark at gmail.com
Sat Nov 1 06:25:29 PDT 2014


On Fri, Oct 31, 2014 at 6:54 PM, Steve Longerbeam <slongerbeam at gmail.com> 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 at mentor.com>
> Signed-off-by: Dmitry Eremin-Solenikov <dmitry_eremin at mentor.com>
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list