On Sat, Apr 1, 2017 at 3:11 PM, Rob Clark <robdclark@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
