[PATCH 1/3] RFC: drm: Restrict vblank ioctl to master

[Daniel Vetter]

On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 14.06.2016 18:03, Daniel Vetter wrote:
>> Somehow this escaped us, this is a KMS ioctl which should only be used
>> by the master (which is the thing that's also in control of kms
>> resources). Everything else is bound to result in fail.
>>
>> Clients shouldn't have a trouble coping with this, since a pile of
>> drivers don't support vblank waits (or just randomly fall over when
>> using them). Note that the big motivation for abusing this like mad
>> seems to be that EGL doesn't have OML_sync, but somehow it didn't
>> cross anyone's mind that adding OML_sync to EGL would be useful.
>
> That may be one motivation, but it's certainly not the only one.
> DRM_IOCTL_WAIT_VBLANK is used by apps which don't use EGL or any similar
> API at all.

Hm, what else? Quick irc discussion turned up a lot of users of this,
but they sem to all have cargo-culted this from the same source, and
all because OML_sync doesn't exist on EGL. Kodi for example uses a 2nd
hidden window and glx oml_sync as a fallback when the vblank ioctl
isn't there. So at least from that pov the reason really seems to be
lack of oml_sync on egl, and not anything more fundamental. At least
on X, where DRI2/Present already fully support everything you need for
oml_sync, and it's just a question of typing the egl variant of that
extension and implementing it in mesa.

Note that I _only_ mean clients here, anything doing bare kms+egl is
of course perfectly fine to use all the kms apis we have directly.

> Seems like you really want to throw out a baby with the bathwater. :(

This is another baby and bathtub ;-) I didn't realize at all that
clients of compositors where doing this. Which is an entirely new can
of worms, since they don't really know on which screen they are, when
the compositor is going to turn it of or do something else. Note that
I only try to restrict it here to DRM_MASTER. And I guess on the
master file there's indeed a pile more use-cases besides just timing
page_flip.

>> -     DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
>> +     DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
>
> If the DRM_IOCTL_MODESET_CTL change is intended, it should be documented
> in the commit log.

Yeah that's an accident, I forgot to take out this hunk. MODESET_CTL
is an ums-only ioctl, so really doesn't matter what we do with it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch