linux-kernel - Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 26 Feb 2017 22:11:10 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     Stefan Lengfeld <contact@...fanchrist.eu>
Cc:     Maxime Ripard <maxime.ripard@...e-electrons.com>,
        michel@...nzer.net, Daniel Vetter <daniel.vetter@...el.com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC

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