Re: [Intel-gfx] [PATCH 10/12] drm: Don't allow multiple buffers fb with stereo modes

Daniel Vetter
2013-09-17T10:37:48Z

On Tue, Sep 17, 2013 at 12:54:09PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 17, 2013 at 10:03:12AM +0100, Damien Lespiau wrote:
> > On Tue, Sep 17, 2013 at 11:20:46AM +0300, Ville Syrjälä wrote:
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > > @@ -2131,6 +2131,17 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
> > > > goto out;
> > > > }
> > > >
> > > > + /*
> > > > + * Do not allow the use of framebuffers consisting of multiple
> > > > + * buffers with stereo modes until all the details API details
> > > > + * are fleshed out (eg. interaction with drm_planes, switch
> > > > + * between a 1 buffers and a 2 buffers fb, ...)
> > > > + */
> > > > + if (fb->num_buffers > 1 && drm_mode_is_stereo(mode)) {
> > > > + ret = -EINVAL;
> > > > + goto out;
> > > > + }
> > >
> > > This would prevent planar buffers in stereo modes. I'm think we just
> > > ignore the matter for now and let drivers deal with it. We don't have
> > > enough handles anyway for planar stereo, so maybe we even want to add
> > > separate left/right fb attachment properties to the planes instead of
> > > tying it up in inside a single fb. Or we cook up addfb3 when we hit
> > > this problem for real. I think we'd anyway need some kind of flag for
> > > the fb if it contains both left and right buffers.
> >
> > I'm quite happy to ignore 3 planes YUV stereo fbs for now :) (2 planes
> > YUV stereo fbs still fit!).
> >
> > Are you fine with this test though, or do you mean ignore the whole
> > matter of forbidding this case (or just the multiplane stereo fb case)?
> > I was just thinking that I missed the addition of this check in the page
> > flip ioctl.
>
> Yeah, I was thinking we that we can ignore this issue for now, and so we
> wouldn't need the check. Currently for non-stereo cases the only thing
> we check is that there is a valid handle for each plane. If userspace
> passes more handles, we simply ignore the extra ones.

I guess we should start to check that. For 3d framebuffers with 2
separate buffer handles for each plane I think we need to add another flag
to addfb2, e.g.

#define DRM_MODE_FB_3D_2_FRAMES (1<<1) /* separate left/right buffers, doubles plane count */

and then also throw in the respective check code into the core that
userspace supplies sufficient amounts of buffers in framebuffer_check()
by adjusting drM_format_num_planes and drm_format_plane_cpp.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch