Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC

[Daniel Vetter]

On Thu, Feb 23, 2017 at 10:02:26AM +0100, Stefan Lengfeld wrote:
> Hi Maxime, Hi Ville,
> 
> On Wed, Feb 22, 2017 at 04:52:33PM -0800, Maxime Ripard wrote:
> > Hi Ville, Stefan,
> > 
> > On Tue, Feb 21, 2017 at 12:55:01PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 21, 2017 at 11:00:59AM +0100, Stefan Lengfeld wrote:
> > > > 
> > > > Furthmore most exiting userspace code just passes the value "0" as the
> > > > argument. For example DirectFB:
> > > > 
> > > >      static const int zero = 0;
> > > >      [...]
> > > >      if (ioctl( dfb_fbdev->fd, FBIO_WAITFORVSYNC, &zero ))
> > > 
> > > Again the matrox driver is different. And looks like unichrome might have
> > > done something even more exotic with the argument.
> > > 
> > > But I do agree that using this with the kms fbdev implementation could
> > > be quite problematic as you can't actually know which crtcs the
> > > fb_helper has picked.
> > 
> > I'm not sure what a good solution might be then. Is just waiting for
> > always the first crtc acceptable? All the code I could find is passing
> > 0 as an argument anyway, so that's effectively the same behaviour
> > anyway here. And getting tearing might be a good argument in favour of
> > moving to KMS :)
> > 
> 
> I also vote for the first solution: just waiting for the first enabled
> crtc. It's deterministic and userspace can avoid tearing at least for
> one crtc. If the system has more active scanouts at the same time with
> different timings, userspace has to use the newer kms/drm API.
> 
> It is the same argument as already posted by Michel Dänzer:
> 
>      > 2/ Wait for a single vsync event on all active crtcs as Ville Syrjälä
>      >    described here [2] in the optimal implementation.
>      
>      FWIW, this seems like a bad idea, since with multiple active CRTCs it
>      would make it essentially random at which intervals the ioctl returns,
>      and which CRTCs are in vertical blank when it does. So it would be
>      useless for both timing and tearing prevention purposes, i.e. more or
>      less completely useless.
> 
> So I think the implementation (waiting only for the first crtc) of ioctl
> FBIO_WAITFORVSYNC is a good enough implementation and works the most
> already existing userspace code out there.

Also note that integrated panels /should/ be the first connectors in the
connector list, which means the first active crtc is most likely the
integrated panel (if there is one).

That's about as good a default choice as we can make I think, everything
else really just needs to get over to doing KMS natively.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch