From mboxrd@z Thu Jan 1 00:00:00 1970 From: Damien Lespiau Subject: Re: [PATCH 6/9] drm: Add a DRM_CAP_STEREO_3D capability for SET_CAP ioctl Date: Sun, 8 Sep 2013 15:49:51 +0100 Message-ID: <20130908144951.GA1175@strange.config> References: <1378493845-476-1-git-send-email-damien.lespiau@intel.com> <1378493845-476-7-git-send-email-damien.lespiau@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 40850E5C2F for ; Sun, 8 Sep 2013 07:49:54 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: David Herrmann Cc: "dri-devel@lists.freedesktop.org" List-Id: dri-devel@lists.freedesktop.org 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 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 > > --- > > 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