[RFC] drm: atomic-rmfb semantics

[Rob Clark]

On Sat, Apr 1, 2017 at 3:11 PM, Rob Clark <robdclark at gmail.com> wrote:
> We possibly missed the boat on redefining rmfb semantics for atomic
> userspace to something more sane, unless perhaps the few existing atomic
> userspaces (CrOS?) could confirm that this change won't cause problems
> (in which case we could just call this a bug-fix, drop the cap, and
> delete some code?).
>
> The old semantics of rmfb shutting down the display is kind of pointless
> in an atomic world, and it is more awkward for userspace that creates
> and destroys the fb every frame, since they need to defer the rmfb.
> Since we have better ways for userspace to shut down the display pipe
> and the legacy behaviour of rmfb is awkward, provide a way for atomic
> userspace to simply make rmfb an unref.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  drivers/gpu/drm/drm_framebuffer.c | 2 +-
>  drivers/gpu/drm/drm_ioctl.c       | 9 +++++++++
>  include/drm/drm_file.h            | 8 ++++++++
>  include/uapi/drm/drm.h            | 7 +++++++
>  4 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index e4909ae..c5127dd0 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -383,7 +383,7 @@ int drm_mode_rmfb(struct drm_device *dev,
>          * so run this in a separate stack as there's no way to correctly
>          * handle this after the fb is already removed from the lookup table.
>          */
> -       if (drm_framebuffer_read_refcount(fb) > 1) {
> +       if (drm_framebuffer_read_refcount(fb) > 1 && !file_priv->atomic_rmfb) {

oh, ignore the fact that this patch as-is would leak.. RFC cobbled
together after a red-eye flight, the fact that it compiles is an
accomplishment ;-)

(but I still want to hear what folks think of the idea)

BR,
-R

>                 struct drm_mode_rmfb_work arg;
>
>                 INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn);
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index a7c61c2..b42348f 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -318,6 +318,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>                 file_priv->atomic = req->value;
>                 file_priv->universal_planes = req->value;
>                 break;
> +       case DRM_CLIENT_CAP_ATOMIC_RMFB:
> +               if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
> +                       return -EINVAL;
> +               if (req->value > 1)
> +                       return -EINVAL;
> +               file_priv->atomic = req->value;
> +               file_priv->universal_planes = req->value;
> +               file_priv->atomic_rmfb = req->value;
> +               break;
>         default:
>                 return -EINVAL;
>         }
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 5dd27ae..2a41c29 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -181,6 +181,14 @@ struct drm_file {
>         unsigned atomic:1;
>
>         /**
> +        * @atomic_rmfb:
> +        *
> +        * True if client wants new semantics for rmfb, ie. simply dropping
> +        * refcnt without tearing down the display.
> +        */
> +       unsigned atomic_rmfb:1;
> +
> +       /**
>          * @is_master:
>          *
>          * This client is the creator of @master. Protected by struct
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b2c5284..4063cc8 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -678,6 +678,13 @@ struct drm_get_cap {
>   */
>  #define DRM_CLIENT_CAP_ATOMIC  3
>
> +/**
> + * DRM_CLIENT_CAP_ATOMIC_RMFB
> + *
> + * If set to 1, the DRM core will not shutdown display pipe on rmfb ioctl.
> + */
> +#define DRM_CLIENT_CAP_ATOMIC_RMFB     4
> +
>  /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
>  struct drm_set_client_cap {
>         __u64 capability;
> --
> 2.9.3
>