[PATCH 6/9] drm: Add a DRM_CAP_STEREO_3D capability for SET_CAP ioctl

[David Herrmann]

Hi

On Fri, Sep 6, 2013 at 8:57 PM, Damien Lespiau <damien.lespiau at intel.com> wrote:
> This capability allows user space to control the delivery of modes with
> the 3D flags set. This is to not play games with current user space
> users not knowing anything about stereo 3D flags and that could try
> to set a mode with one or several of those bits set.
>
> So, the plan is to remove the stereo 3D flags from the user mode
> modeinfo structure by default, and let them through if we are being told
> otherwise.
>
> stereo_allowed is bound to the drm_file structure to make it a
> per-client setting, not a global one.
>
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c  | 16 +++++++++++++---
>  drivers/gpu/drm/drm_ioctl.c | 14 +++++++++++++-
>  include/drm/drmP.h          |  3 +++
>  include/uapi/drm/drm.h      |  9 +++++++++
>  4 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index a691764..ff9646f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1257,12 +1257,14 @@ EXPORT_SYMBOL(drm_mode_group_init_legacy_group);
>   * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo
>   * @out: drm_mode_modeinfo struct to return to the user
>   * @in: drm_display_mode to use
> + * @file_priv: drm file from the ioctl call
>   *
>   * Convert a drm_display_mode into a drm_mode_modeinfo structure to return to
>   * the user.
>   */
>  static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
> -                                     const struct drm_display_mode *in)
> +                                     const struct drm_display_mode *in,
> +                                     const struct drm_file *file_priv)
>  {
>         WARN(in->hdisplay > USHRT_MAX || in->hsync_start > USHRT_MAX ||
>              in->hsync_end > USHRT_MAX || in->htotal > USHRT_MAX ||
> @@ -1287,6 +1289,13 @@ static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
>         out->type = in->type;
>         strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>         out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
> +
> +       /*
> +        * If user-space hasn't configured the driver to expose the stereo 3D
> +        * flags, clear them.
> +        */
> +       if (!file_priv->stereo_allowed)
> +               out->flags &= ~DRM_MODE_FLAG_3D_MASK;

So just to be clear: Whenever a mode is present with 3D flags, it is
also a valid non-3D mode? Is this guaranteed? Don't you want to add a
mode twice, once without 3D flags and once with? You could then just
skip all 3D modes for clients that don't support it.

I have no idea how the 3D flags are specified, just want to go sure
this doesn't break. So whenever a mode with 3D flags is present on a
device, it can be set by a client dropping the 3D flags and it will be
a valid mono-mode?

Cheers
David

>  }
>
>  /**
> @@ -1556,7 +1565,8 @@ int drm_mode_getcrtc(struct drm_device *dev,
>
>         if (crtc->enabled) {
>
> -               drm_crtc_convert_to_umode(&crtc_resp->mode, &crtc->mode);
> +               drm_crtc_convert_to_umode(&crtc_resp->mode, &crtc->mode,
> +                                         file_priv);
>                 crtc_resp->mode_valid = 1;
>
>         } else {
> @@ -1655,7 +1665,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>                 copied = 0;
>                 mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr;
>                 list_for_each_entry(mode, &connector->modes, head) {
> -                       drm_crtc_convert_to_umode(&u_mode, mode);
> +                       drm_crtc_convert_to_umode(&u_mode, mode, file_priv);
>                         if (copy_to_user(mode_ptr + copied,
>                                          &u_mode, sizeof(u_mode))) {
>                                 ret = -EFAULT;
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index e471cd9..a716641 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -304,7 +304,19 @@ int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>   */
>  int drm_setcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  {
> -       return -EINVAL;
> +       struct drm_set_cap *req = data;
> +
> +       switch (req->capability) {
> +       case DRM_CAP_STEREO_3D:
> +               if (req->value > 1)
> +                       return -EINVAL;
> +               file_priv->stereo_allowed = req->value;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
>  }
>
>  /**
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index b9c321b..0df654c 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -431,6 +431,9 @@ struct drm_file {
>         struct drm_master *master; /* master this node is currently associated with
>                                       N.B. not always minor->master */
>
> +       /* true when the client has asked us to expose stereo 3D mode flags */
> +       bool stereo_allowed;
> +
>         /**
>          * fbs - List of framebuffers associated with this file.
>          *
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index d400e6f..23922b4 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -627,6 +627,15 @@ struct drm_get_cap {
>         __u64 value;
>  };
>
> +/**
> + * DRM_CAP_STEREO_3D
> + *
> + * if set to 1, the DRM core will expose the stereo 3D capabilities of the
> + * monitor by advertising the supported 3D layouts in the flags of struct
> + * drm_mode_modeinfo.
> + */
> +#define DRM_CAP_STEREO_3D      1
> +
>  /** DRM_IOCTL_SET_CAP ioctl argument type */
>  struct drm_set_cap {
>         __u64 capability;
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel