[PATCH RFC] drm: introduce DRM_MODE_FB_PERSIST

[Daniel Vetter]

On Wed, Oct 6, 2021 at 5:19 PM Simon Ser <contact at emersion.fr> wrote:
> This new ADDFB2 flag allows callers to mark a framebuffer as
> "persistent", and no longer have RMFB semantics when the DRM
> file is closed.
>
> [1]: https://lore.kernel.org/dri-devel/YTJypepF1Hpc2YYT@reader/
>
> Signed-off-by: Simon Ser <contact at emersion.fr>
> Cc: Hans de Goede <hdegoede at redhat.com>
> Cc: Dennis Filder <d.filder at web.de>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen at gmail.com>
> ---
>
> I'm not sure this is enough, but posting this to get initial
> feedback and allow to start e.g. Plymouth experiments. I'll
> follow up with an IGT test soon.
>
>  drivers/gpu/drm/drm_framebuffer.c |  6 ++++--
>  include/uapi/drm/drm_mode.h       | 15 +++++++++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 07f5abc875e9..9b398838e1f4 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -292,7 +292,8 @@ drm_internal_framebuffer_create(struct drm_device *dev,
>         struct drm_framebuffer *fb;
>         int ret;
>
> -       if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
> +       if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS |
> +                        DRM_MODE_FB_PERSIST)) {
>                 DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
>                 return ERR_PTR(-EINVAL);
>         }
> @@ -789,7 +790,8 @@ void drm_fb_release(struct drm_file *priv)
>          * at it any more.
>          */
>         list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
> -               if (drm_framebuffer_read_refcount(fb) > 1) {
> +               if (drm_framebuffer_read_refcount(fb) > 1 &&
> +                   !(fb->flags & DRM_MODE_FB_PERSIST)) {
>                         list_move_tail(&fb->filp_head, &arg.fbs);
>                 } else {
>                         list_del_init(&fb->filp_head);
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index e1e351682872..c7a7089ec31e 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -662,6 +662,21 @@ struct drm_mode_fb_cmd {
>
>  #define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */
>  #define DRM_MODE_FB_MODIFIERS  (1<<1) /* enables ->modifer[] */
> +/**
> + * DRM_MODE_FB_PERSIST
> + *
> + * DRM framebuffers are normally implicitly removed when their owner closes the
> + * DRM FD. Passing this flag will make the framebuffer persistent: it will not
> + * be implicitly removed. This is useful to implement flicker-free transitions
> + * between two processes.
> + *
> + * This flag doesn't change the behavior of &DRM_IOCTL_MODE_RMFB.
> + *
> + * User-space should ensure the framebuffer doesn't expose any sensitive user
> + * information: persistent framebuffers can be read back by the next DRM
> + * master.

Should probably explain here that the persistent fb stays around for
as long as it's still in use by a plane, but will disappear
automatically when it's no longer in active use.

Also I guess there was some discussion I've missed on why we exclude
rmfb from this (unlike the CLOSEFB thing robclark proposed ages ago).
The nice thing about closefb is that you can give it persistent
semantics retroactively, so don't need to re-wrap an gem_bo and do a
page flip when you quit.
-Daniel

> + */
> +#define DRM_MODE_FB_PERSIST (1 << 2)
>
>  struct drm_mode_fb_cmd2 {
>         __u32 fb_id;
> --
> 2.33.0
>
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch