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

2013-09-16 Thread Damien Lespiau 
On Fri, Sep 13, 2013 at 04:04:02PM +, Joakim Plate wrote:
 David Herrmann dh.herrmann at gmail.com writes:
 
  
  So just to be clear: Whenever a mode is present with 3D flags, it is
  also a valid non-3D mode? Is this guaranteed? 
  
 
 Well.. Some HDTV's will when they receive a frame packed mode (1080*2+45=2205 
 pixels high) . Display just the top part. The bottom part of that is not on 
 screen.

I changed this part of the API so there's no mixing the 2d mode
structure with the 3d flags. Now (v4 of the series) userspace receives
one struct drm_mode_modeinfo for the 2D mode (like before) and one
struct drm_mode_modeinfo for each 3D layout (and the rest of the timings
are from the underlying 2D mode that userspace can use in conjonction
with the layout bit to compute the stereo framebufer layout)

HTH,

-- 
Damien
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-09-13 Thread Joakim Plate 
David Herrmann dh.herrmann at gmail.com writes:

 
 So just to be clear: Whenever a mode is present with 3D flags, it is
 also a valid non-3D mode? Is this guaranteed? 
 

Well.. Some HDTV's will when they receive a frame packed mode (1080*2+45=2205 
pixels high) . Display just the top part. The bottom part of that is not on 
screen.

So while it will not display it as 3d, it will discard half of the image.

/Joakim

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-09-08 Thread David Herrmann 
Hi

On Fri, Sep 6, 2013 at 8:57 PM, Damien Lespiau damien.lesp...@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.lesp...@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

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

2013-09-08 Thread Damien Lespiau 
On Sun, Sep 08, 2013 at 03:50:29PM +0200, David Herrmann wrote:
 Hi
 
 On Fri, Sep 6, 2013 at 8:57 PM, Damien Lespiau damien.lesp...@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.lesp...@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?

So yes, that's exactly what happens right now. I was actually thinking
about a v4 of the series with what you said in the first paragraph: just
add the 3D modes to the list, one mode per 3D layout, and drop those
modes from the list given back to userspace when the stereo_allowed
isn't set. That seems quite a bit better than the convoluted approach
here.

-- 
Damien
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2013-09-06 Thread Damien Lespiau 
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.lesp...@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;
 }
 
 /**
@@ -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