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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170223090226.GF14871@sill.h.stcim.de>
Date:   Thu, 23 Feb 2017 10:02:26 +0100
From:   Stefan Lengfeld <contact@...fanchrist.eu>
To:     Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc:     Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Daniel Vetter <daniel.vetter@...el.com>, michel@...nzer.net
Subject: Re: [PATCH v3 2/2] drm/fb-helper: implement ioctl FBIO_WAITFORVSYNC

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.

Kind regards,
    Stefan Lengfeld