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

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

[Daniel Vetter]

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. This
patch is meant to essentially start kicking that can from the back
end.

Cc: fritsch@kodi.tv
Cc: fernetmenta@kodi.tv
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 0510675eec5d..6cc78d648393 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -529,9 +529,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_MASTER|DRM_UNLOCKED),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm: Mark authmagic ioctls as unlocked

[Daniel Vetter]

All protected by dev->master_mutex. And there's no driver callbacks,
which means no need to sync with old dri1 horror show drivers at all.
Hence safe to drop the drm legacy BKL from these paths.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6cc78d648393..288b047b2de5 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -467,7 +467,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version,
 		      DRM_UNLOCKED|DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
-	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
+	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_legacy_getmap_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
@@ -479,7 +479,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_AUTH|DRM_MASTER),
+	DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_AUTH|DRM_UNLOCKED|DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_ADD_MAP, drm_legacy_addmap_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_RM_MAP, drm_legacy_rmmap_ioctl, DRM_AUTH),
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm: Mark set/drop master ioctl as unlocked.

[Daniel Vetter]

Again this is neatly protected by the dev->master_mutex now. There is
a driver callback both for set and drop, but it's only used by vmwgfx.
And vmwgfx has it's own solid locking for shared resources (besides
dev->master_mutex), hence is all safe. Let's drop another place where
the drm legacy bkl is used.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 288b047b2de5..08c75630f1dc 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -487,8 +487,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, DRM_UNLOCKED|DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, DRM_UNLOCKED|DRM_ROOT_ONLY),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for series starting with [1/3] RFC: drm: Restrict vblank ioctl to master

[Patchwork]

== Series Details ==

Series: series starting with [1/3] RFC: drm: Restrict vblank ioctl to master
URL   : https://patchwork.freedesktop.org/series/8674/
State : failure

== Summary ==

Applying: RFC: drm: Restrict vblank ioctl to master
Applying: drm: Mark authmagic ioctls as unlocked
fatal: sha1 information is lacking or useless (drivers/gpu/drm/drm_ioctl.c).
error: could not build fake ancestor
Patch failed at 0002 drm: Mark authmagic ioctls as unlocked
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rainer Hochecker]

Finally we removed this code from Kodi.

Regards,
Rainer

On Tue, Jun 14, 2016 at 11:02 AM, Daniel Vetter <daniel.vetter@ffwll.ch> 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. This
> patch is meant to essentially start kicking that can from the back
> end.
>
> Cc: fritsch@kodi.tv
> Cc: fernetmenta@kodi.tv
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 0510675eec5d..6cc78d648393 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -529,9 +529,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>         DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>         DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
> +       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_MASTER|DRM_UNLOCKED),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
> +       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
>
>         DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
> --
> 2.8.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

Hi Rainer,

Awesome, thanks a lot for doing this! Any idea for when this will ship
in a release, and for how long your users are generally using older
releases? Just to have a rough indication for when we could attempt to
merge this patch here.

Cheers, Daniel

On Wed, Aug 8, 2018 at 5:35 PM, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> Finally we removed this code from Kodi.
>
> Regards,
> Rainer
>
> On Tue, Jun 14, 2016 at 11:02 AM, Daniel Vetter <daniel.vetter@ffwll.ch> 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. This
>> patch is meant to essentially start kicking that can from the back
>> end.
>>
>> Cc: fritsch@kodi.tv
>> Cc: fernetmenta@kodi.tv
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 0510675eec5d..6cc78d648393 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -529,9 +529,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>>         DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>         DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>
>> -       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
>> +       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_MASTER|DRM_UNLOCKED),
>>
>> -       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
>> +       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
>>
>>         DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>
>> --
>> 2.8.1
>>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rainer Hochecker]

Hi Daniel,

We are in beta for v18. I expect release in September.
I don't think we have to consider users who run older Kodi versions on
systems with latest kernels. The feature the patch would disable won't
make Kodi completely unusable when absent. I'd say you can merge
the patch short after v18 has been released.

Cheers,
Rainer

On Wed, Aug 8, 2018 at 6:59 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi Rainer,
>
> Awesome, thanks a lot for doing this! Any idea for when this will ship
> in a release, and for how long your users are generally using older
> releases? Just to have a rough indication for when we could attempt to
> merge this patch here.
>
> Cheers, Daniel
>
> On Wed, Aug 8, 2018 at 5:35 PM, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
>> Finally we removed this code from Kodi.
>>
>> Regards,
>> Rainer
>>
>> On Tue, Jun 14, 2016 at 11:02 AM, Daniel Vetter <daniel.vetter@ffwll.ch> 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. This
>>> patch is meant to essentially start kicking that can from the back
>>> end.
>>>
>>> Cc: fritsch@kodi.tv
>>> Cc: fernetmenta@kodi.tv
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>>  drivers/gpu/drm/drm_ioctl.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>>> index 0510675eec5d..6cc78d648393 100644
>>> --- a/drivers/gpu/drm/drm_ioctl.c
>>> +++ b/drivers/gpu/drm/drm_ioctl.c
>>> @@ -529,9 +529,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>>>         DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>>         DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>>
>>> -       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
>>> +       DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_MASTER|DRM_UNLOCKED),
>>>
>>> -       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
>>> +       DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
>>>
>>>         DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>>>
>>> --
>>> 2.8.1
>>>
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rainer Hochecker]

>>
>
> Yep. The weird thing about that extension is that it only implements the
> query vblank count/timestamp part of GLX_OML_sync_control. Sounds like the
> least useful bit of all in isolation, although the extension as a whole
> could be useful, just as on GLX. Also note this comment on that commit
> referenced by Daniel from just 2 hours ago:
>
> "marcheu 2 hours, 35 minutes ago (2017-05-05 18:52:45 UTC) #10
> lgtm
>
> By the way, there is discussion on dropping this extension from mesa. You
> should let Chad know if you need it in the long run. Chrome OS stopped
> requiring it when we switched to freon."
>
> Cc'ing Chad Versace on the off-chance he is the mentioned Chad.
>
> Also, the VDPAU api for video playback implements functions for precise
> timing of video frame presentation. I don't have any practical experience
> with them, but iirc Mesa's VDPAU video presentation api implements those via
> DRI3/Present extension, so hooks into drmWaitVblank et al. If Kodi uses
> VDPAU for playback, it could use those conveniently for timing.
>
> -mario

Kodi needs interoperability between vpdau and GL, it does not use vdpau's
presentation queue. Currently vdpau is bound to GLX and there don't seem
to be any activity to change this, it will most likely die with X11.

Kodi will get Wayland support soon, a GSOC project was just acceptend wich
I will mentor. We already have support for Mir.
That means Kodi is fine if you go ahead with this RFC.

Rainer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Mario Kleiner]

On 05/05/2017 09:35 PM, Daniel Vetter wrote:
> On Mon, Jul 25, 2016 at 1:15 PM, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
>> Am 25.07.2016 08:38 schrieb "Michel Dänzer" <michel@daenzer.net>:
>>>
>>> On 13.07.2016 18:49, Rainer Hochecker wrote:
>>>> We have been using this for years now and did not observe issues you
>>>> mentioned. I would be surprised if a child window refreshes in a
>>>> different rate than the main window
>>>
>>> The Xorg driver decides which CRTC to synchronize with based on the
>>> window geometry. For a window with no visible geometry, it may choose a
>>> different CRTC than for the visible output window. In the case of DRI3,
>>> the Xorg Present extension code may even fall back to the fake CRTC in
>>> that case, which only generates a fake vblank event once every second.
>>>
>>>
>>> --
>>> Earthling Michel Dänzer               |               http://www.amd.com
>>> Libre software enthusiast             |             Mesa and X developer
>>>
>>
>> That means that we won't have a solution for X11 with EGL for Intel systems
>> anymore.
>
> I didn't know about this, but a proper EGL extension, implemented on
> X11 exists already:
>
> https://codereview.chromium.org/2867513002/
>
> It's not in the khronos registry, but mesa implements it, and
> semantics match oml_sync (and wayland present), so we already have the
> proper extension to implement this without digging around in device
> files and making a guess about which pipe you might be running on.
>
> Cheers, Daniel
>

Yep. The weird thing about that extension is that it only implements the 
query vblank count/timestamp part of GLX_OML_sync_control. Sounds like 
the least useful bit of all in isolation, although the extension as a 
whole could be useful, just as on GLX. Also note this comment on that 
commit referenced by Daniel from just 2 hours ago:

"marcheu 2 hours, 35 minutes ago (2017-05-05 18:52:45 UTC) #10
lgtm

By the way, there is discussion on dropping this extension from mesa. 
You should let Chad know if you need it in the long run. Chrome OS 
stopped requiring it when we switched to freon."

Cc'ing Chad Versace on the off-chance he is the mentioned Chad.

Also, the VDPAU api for video playback implements functions for precise 
timing of video frame presentation. I don't have any practical 
experience with them, but iirc Mesa's VDPAU video presentation api 
implements those via DRI3/Present extension, so hooks into drmWaitVblank 
et al. If Kodi uses VDPAU for playback, it could use those conveniently 
for timing.

-mario
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Mon, Jul 25, 2016 at 1:15 PM, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> Am 25.07.2016 08:38 schrieb "Michel Dänzer" <michel@daenzer.net>:
>>
>> On 13.07.2016 18:49, Rainer Hochecker wrote:
>> > We have been using this for years now and did not observe issues you
>> > mentioned. I would be surprised if a child window refreshes in a
>> > different rate than the main window
>>
>> The Xorg driver decides which CRTC to synchronize with based on the
>> window geometry. For a window with no visible geometry, it may choose a
>> different CRTC than for the visible output window. In the case of DRI3,
>> the Xorg Present extension code may even fall back to the fake CRTC in
>> that case, which only generates a fake vblank event once every second.
>>
>>
>> --
>> Earthling Michel Dänzer               |               http://www.amd.com
>> Libre software enthusiast             |             Mesa and X developer
>>
>
> That means that we won't have a solution for X11 with EGL for Intel systems
> anymore.

I didn't know about this, but a proper EGL extension, implemented on
X11 exists already:

https://codereview.chromium.org/2867513002/

It's not in the khronos registry, but mesa implements it, and
semantics match oml_sync (and wayland present), so we already have the
proper extension to implement this without digging around in device
files and making a guess about which pipe you might be running on.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 933 bytes --]

Am 25.07.2016 08:38 schrieb "Michel Dänzer" <michel@daenzer.net>:
>
> On 13.07.2016 18:49, Rainer Hochecker wrote:
> > We have been using this for years now and did not observe issues you
> > mentioned. I would be surprised if a child window refreshes in a
> > different rate than the main window
>
> The Xorg driver decides which CRTC to synchronize with based on the
> window geometry. For a window with no visible geometry, it may choose a
> different CRTC than for the visible output window. In the case of DRI3,
> the Xorg Present extension code may even fall back to the fake CRTC in
> that case, which only generates a fake vblank event once every second.
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
>

That means that we won't have a solution for X11 with EGL for Intel systems
anymore.

[-- Attachment #1.2: Type: text/html, Size: 1221 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 13.07.2016 18:49, Rainer Hochecker wrote:
> We have been using this for years now and did not observe issues you
> mentioned. I would be surprised if a child window refreshes in a
> different rate than the main window

The Xorg driver decides which CRTC to synchronize with based on the
window geometry. For a window with no visible geometry, it may choose a
different CRTC than for the visible output window. In the case of DRI3,
the Xorg Present extension code may even fall back to the fake CRTC in
that case, which only generates a fake vblank event once every second.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 914 bytes --]

GLX can't interop with libva in a reasonable way. That was the reason why
we switched to EGL
Am 13.07.2016 11:48 schrieb "Daniel Vetter" <daniel@ffwll.ch>:

On Wed, Jul 13, 2016 at 06:43:37PM +0900, Michel Dänzer wrote:
> On 13.07.2016 15:50, Rainer Hochecker wrote:
> > Whatever action is taken, it is fine for Kodi. GLX+OML_sync_control is
> > not an option anymore because we need EGL for vaapi. But we can fall
> > back to the invisible window for getting vsync. I never tried using EGL
> > and GLX in the same application, different windows. Any reason why this
> > should not work?
>
> An invisible window may not synchronize with the same output refresh
> cycle as your output window.

Why do you need EGL for libva? Besides that noob question from me, no idea
how well EGL/GLX can interop at all ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[-- Attachment #1.2: Type: text/html, Size: 1392 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 864 bytes --]

We have been using this for years now and did not observe issues you
mentioned. I would be surprised if a child window refreshes in a different
rate than the main window
Am 13.07.2016 11:44 schrieb "Michel Dänzer" <michel@daenzer.net>:

On 13.07.2016 15:50, Rainer Hochecker wrote:
> Whatever action is taken, it is fine for Kodi. GLX+OML_sync_control is
> not an option anymore because we need EGL for vaapi. But we can fall
> back to the invisible window for getting vsync. I never tried using EGL
> and GLX in the same application, different windows. Any reason why this
> should not work?

An invisible window may not synchronize with the same output refresh
cycle as your output window.


--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

[-- Attachment #1.2: Type: text/html, Size: 1325 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Jul 13, 2016 at 06:43:37PM +0900, Michel Dänzer wrote:
> On 13.07.2016 15:50, Rainer Hochecker wrote:
> > Whatever action is taken, it is fine for Kodi. GLX+OML_sync_control is
> > not an option anymore because we need EGL for vaapi. But we can fall
> > back to the invisible window for getting vsync. I never tried using EGL
> > and GLX in the same application, different windows. Any reason why this
> > should not work?
> 
> An invisible window may not synchronize with the same output refresh
> cycle as your output window.

Why do you need EGL for libva? Besides that noob question from me, no idea
how well EGL/GLX can interop at all ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 13.07.2016 15:50, Rainer Hochecker wrote:
> Whatever action is taken, it is fine for Kodi. GLX+OML_sync_control is
> not an option anymore because we need EGL for vaapi. But we can fall
> back to the invisible window for getting vsync. I never tried using EGL
> and GLX in the same application, different windows. Any reason why this
> should not work?

An invisible window may not synchronize with the same output refresh
cycle as your output window.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 1728 bytes --]

Whatever action is taken, it is fine for Kodi. GLX+OML_sync_control is not
an option anymore because we need EGL for vaapi. But we can fall back to
the invisible window for getting vsync. I never tried using EGL and GLX in
the same application, different windows. Any reason why this should not
work?

Rainer

On Tue, Jul 12, 2016 at 12:29 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Jun 24, 2016 at 06:55:55AM +1000, Daniel Stone wrote:
> > Hi Rainer,
> >
> > On 24 June 2016 at 05:54, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> > > I spent some time reading and investigating on this. Bear with me, I am
> > > doing Kodi development in my spare time and may not be up-to-date on
> all
> > > platforms. Seems Wayland is much better suited to serve as reference
> > > platform as X11 does. Is that correct? If so I don't request
> > > OML_sync_control for EGL. Don't waste resources and let the old crap
> die.
> >
> > I certainly think so, for a number of reasons. I don't believe X11
> > will ever be as accurate or as efficient as Wayland can be.
>
> Seconded. I think GLX+OML_sync_control for X11 and Wayland with EGL and
> the frame timing Daniel Stone laid out (already should work in both cases)
> seems like the perfect solution.
>
> What kind of transition plan would be reasonable? Should we start with a
> printk_once to inform userspace developers that they should change their
> code, and then eventually (after a few years or so) remove that ioctl?
> Maybe first behind a module option?
>
> Who should all be on cc for such a change?
>
> I'd like to get this started, it'll take years no matter what ...
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 2472 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Fri, Jun 24, 2016 at 06:55:55AM +1000, Daniel Stone wrote:
> Hi Rainer,
> 
> On 24 June 2016 at 05:54, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> > I spent some time reading and investigating on this. Bear with me, I am
> > doing Kodi development in my spare time and may not be up-to-date on all
> > platforms. Seems Wayland is much better suited to serve as reference
> > platform as X11 does. Is that correct? If so I don't request
> > OML_sync_control for EGL. Don't waste resources and let the old crap die.
> 
> I certainly think so, for a number of reasons. I don't believe X11
> will ever be as accurate or as efficient as Wayland can be.

Seconded. I think GLX+OML_sync_control for X11 and Wayland with EGL and
the frame timing Daniel Stone laid out (already should work in both cases)
seems like the perfect solution.

What kind of transition plan would be reasonable? Should we start with a
printk_once to inform userspace developers that they should change their
code, and then eventually (after a few years or so) remove that ioctl?
Maybe first behind a module option?

Who should all be on cc for such a change?

I'd like to get this started, it'll take years no matter what ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Stone]

Hi Rainer,

On 24 June 2016 at 05:54, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> I spent some time reading and investigating on this. Bear with me, I am
> doing Kodi development in my spare time and may not be up-to-date on all
> platforms. Seems Wayland is much better suited to serve as reference
> platform as X11 does. Is that correct? If so I don't request
> OML_sync_control for EGL. Don't waste resources and let the old crap die.

I certainly think so, for a number of reasons. I don't believe X11
will ever be as accurate or as efficient as Wayland can be.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 1581 bytes --]

I spent some time reading and investigating on this. Bear with me, I am
doing Kodi development in my spare time and may not be up-to-date on all
platforms. Seems Wayland is much better suited to serve as reference
platform as X11 does. Is that correct? If so I don't request OML_sync_control
for EGL. Don't waste resources and let the old crap die.

Rainer

On Tue, Jun 21, 2016 at 7:15 AM, Rainer Hochecker <fernetmenta@kodi.tv>
wrote:

> Thanks for clarification. That changes my view on Wayland.
>
> Cheers,
> Rainer
>
> On Tue, Jun 21, 2016 at 7:01 AM, Daniel Stone <daniel@fooishbar.org>
> wrote:
>
>> Hi,
>>
>> On 21 June 2016 at 14:57, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
>> > Are you saying that this is outdated:
>> > https://wayland.freedesktop.org/faq.html#heading_toc_j_12
>> >
>> > A more subtle point is that libGL.so includes the GLX symbols, so
>> linking to
>> > that library will pull in all the X dependencies. This means that we
>> can't
>> > link to full GL without pulling in the client side of X, so we're using
>> > GLES2 for now. Longer term, we'll need a way to use full GL under
>> Wayland.
>>
>> Badly worded, really.
>>
>> libGL.so includes the GLX API entrypoints, so your libGL will link to
>> X11. For that reason - and because there's no need for it to use full
>> GL - Weston uses GLES2 for its own composition. For clients, if you
>> don't care about this, then you can use libGL + EGL (this has always
>> worked), or there's also libglvnd's libOpenGL (this is new). Given
>> that, it should be reworded.
>>
>> Cheers,
>> Daniel
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 2711 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 1100 bytes --]

Thanks for clarification. That changes my view on Wayland.

Cheers,
Rainer

On Tue, Jun 21, 2016 at 7:01 AM, Daniel Stone <daniel@fooishbar.org> wrote:

> Hi,
>
> On 21 June 2016 at 14:57, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> > Are you saying that this is outdated:
> > https://wayland.freedesktop.org/faq.html#heading_toc_j_12
> >
> > A more subtle point is that libGL.so includes the GLX symbols, so
> linking to
> > that library will pull in all the X dependencies. This means that we
> can't
> > link to full GL without pulling in the client side of X, so we're using
> > GLES2 for now. Longer term, we'll need a way to use full GL under
> Wayland.
>
> Badly worded, really.
>
> libGL.so includes the GLX API entrypoints, so your libGL will link to
> X11. For that reason - and because there's no need for it to use full
> GL - Weston uses GLES2 for its own composition. For clients, if you
> don't care about this, then you can use libGL + EGL (this has always
> worked), or there's also libglvnd's libOpenGL (this is new). Given
> that, it should be reworded.
>
> Cheers,
> Daniel
>

[-- Attachment #1.2: Type: text/html, Size: 1714 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Stone]

Hi,

On 21 June 2016 at 14:57, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> Are you saying that this is outdated:
> https://wayland.freedesktop.org/faq.html#heading_toc_j_12
>
> A more subtle point is that libGL.so includes the GLX symbols, so linking to
> that library will pull in all the X dependencies. This means that we can't
> link to full GL without pulling in the client side of X, so we're using
> GLES2 for now. Longer term, we'll need a way to use full GL under Wayland.

Badly worded, really.

libGL.so includes the GLX API entrypoints, so your libGL will link to
X11. For that reason - and because there's no need for it to use full
GL - Weston uses GLES2 for its own composition. For clients, if you
don't care about this, then you can use libGL + EGL (this has always
worked), or there's also libglvnd's libOpenGL (this is new). Given
that, it should be reworded.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 832 bytes --]

Are you saying that this is outdated:
https://wayland.freedesktop.org/faq.html#heading_toc_j_12

*A more subtle point is that libGL.so includes the GLX symbols, so linking
to that library will pull in all the X dependencies. This means that we
can't link to full GL without pulling in the client side of X, so we're
using GLES2 for now. Longer term, we'll need a way to use full GL under
Wayland.*

On Tue, Jun 21, 2016 at 1:27 AM, Daniel Stone <daniel@fooishbar.org> wrote:

> Hi,
>
> On 21 June 2016 at 04:24, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> > Thanks a lot.
> > Would you know if/when Wayland will support OpenGL?
>
> Er, it always has ... ?
>
> It will never support GLX (as the name implies, that's X-specific),
> but EGL is perfectly capable of creating OpenGL contexts. It works
> fine.
>
> Cheers,
> Daniel
>

[-- Attachment #1.2: Type: text/html, Size: 1441 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Stone]

Hi,

On 21 June 2016 at 04:24, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> Thanks a lot.
> Would you know if/when Wayland will support OpenGL?

Er, it always has ... ?

It will never support GLX (as the name implies, that's X-specific),
but EGL is perfectly capable of creating OpenGL contexts. It works
fine.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 1705 bytes --]

Thanks a lot.
Would you know if/when Wayland will support OpenGL?

Rainer

On Mon, Jun 20, 2016 at 4:46 PM, Daniel Stone <daniel@fooishbar.org> wrote:

> Hi Rainer,
>
> On 17 June 2016 at 22:00, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> > I agree. GLX_OML_sync_control fulfils all our requirements apart from
> being
> > available for EGL. It would be great to have it available for EGL. In
> regard
> > to Wayland this is really important. For the time being Kodi stopped
> > supporting Wayland because a/v sync is not possible on that platform.
>
> This isn't true. For Wayland, you can use the wp_presentation_time
> extension:
>
> https://cgit.freedesktop.org/wayland/wayland-protocols/tree/stable/presentation-time/presentation-time.xml
>
> If you're using the zwp_linux_dmabuf extension directly, then you can
> easily and obviously use the extension to get feedback, and there are
> examples of using this. But even if you're using EGL, Wayland's EGL
> client platform has the property that a wl_surface::commit request
> must be issued from inside eglSwapBuffers. So you can make use of this
> to issue a presentation_time feedback request immediately before
> calling eglSwapBuffers, which will return a unique event giving you
> the exact time that the buffer swap was actually displayed on screen.
> This is far better than vblank requests which only give you hardware
> clock ticks, and tell you nothing about actual display time.
>
> I do discuss this with Peter occasionally, but please get in touch if
> you feel there is anything else missing, because we have gone to great
> lengths to make Wayland as good as it possibly can be for exactly
> these usecases.
>
> Cheers,
> Daniel
>

[-- Attachment #1.2: Type: text/html, Size: 2367 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Stone]

Hi Rainer,

On 17 June 2016 at 22:00, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> I agree. GLX_OML_sync_control fulfils all our requirements apart from being
> available for EGL. It would be great to have it available for EGL. In regard
> to Wayland this is really important. For the time being Kodi stopped
> supporting Wayland because a/v sync is not possible on that platform.

This isn't true. For Wayland, you can use the wp_presentation_time extension:
https://cgit.freedesktop.org/wayland/wayland-protocols/tree/stable/presentation-time/presentation-time.xml

If you're using the zwp_linux_dmabuf extension directly, then you can
easily and obviously use the extension to get feedback, and there are
examples of using this. But even if you're using EGL, Wayland's EGL
client platform has the property that a wl_surface::commit request
must be issued from inside eglSwapBuffers. So you can make use of this
to issue a presentation_time feedback request immediately before
calling eglSwapBuffers, which will return a unique event giving you
the exact time that the buffer swap was actually displayed on screen.
This is far better than vblank requests which only give you hardware
clock ticks, and tell you nothing about actual display time.

I do discuss this with Peter occasionally, but please get in touch if
you feel there is anything else missing, because we have gone to great
lengths to make Wayland as good as it possibly can be for exactly
these usecases.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 5575 bytes --]

Daniel,

I agree. GLX_OML_sync_control fulfils all our requirements apart from being
available for EGL. It would be great to have it available for EGL. In
regard to Wayland this is really important. For the time being Kodi stopped
supporting Wayland because a/v sync is not possible on that platform.

Rainer

On Thu, Jun 16, 2016 at 6:17 PM, Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> On Thu, Jun 16, 2016 at 3:02 PM, Rainer Hochecker <fernetmenta@kodi.tv>
> wrote:
> > Daniel,
> >
> > Peter posted me some snippets about your discussion around vblank that
> > confused me. Our use case is as follows:
> > We synchronise our video player clock to the pace of the display. Assume
> we
> > play some 23.976 content and the actual refresh rate is 24hz. We
> increment
> > the clock every vblank, that makes the clock run faster. Audio is
> resampled
> > to the faster running clock.
> > For this we just need an event fired for vblank.
> >
> > Another requirement not directly related to 1)
> > We would like to know when a frame in actually presented on screen. Now
> this
> > is only guessing.
>
>
> If you do vblank support right (which means GLX_OML_sync_control, it
> should work on mesa/glx but not anywhere else unfortunately) you get
> both. That's what I mean with calling the vblank ioctl directly is a
> hack: It's only really one half of correctly done a/v sync, you get
> the clock source but still have no idea when the frames actually show
> up. GLX_OML_sync_control (and a port to EGL/wayland, if someone
> bothers with that) gives you that control, plus ofc still a clock
> source which is as accurate as the current one (since it's the same).
>
> Note that DRI2/DRI3 and wayland all support this already, it's just a
> matter of wiring it up as an EGL extension. And of course it won't
> work on nvidia blobs because they just suck at this and don't want to
> let apps control all the double/triple buffering and whatever else
> they have in their driver.
>
> Note that the only reason this works on mesa is because Mario Kleiner
> really cares about it, and fixed up all the bugs across the entire
> stack (meas, X, kernel drivers). Mario might be interested in an EGL
> version of all this, not sure. Adding him.
>
> Oh and: Please feel free to add other video players and folks
> interested in this, I don't know them really at all.
>
> Cheers, Daniel
>
> >
> > Rainer
> >
> > On Wed, Jun 15, 2016 at 10:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch>
> > wrote:
> >>
> >> On Wed, Jun 15, 2016 at 9:29 AM, Michel Dänzer <michel@daenzer.net>
> wrote:
> >> > On 14.06.2016 18:35, Daniel Vetter wrote:
> >> >> On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel@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?
> >> >
> >> > Off the top of my head, I know specifically about compton and xfwm4.
> >> > There's certainly more.
> >>
> >> But do they anything more fancy than what could be achieved with
> >> OML_sync too? Or is the issue that they don't want to use EGL/GLX? In
> >> that case I think it's reasonable to ask them to use bare-metal
> >> Present, since that's not any less portable than using the vblank
> >> ioctl.
> >>
> >> > Note that as I mentioned before in the other thread, I absolutely
> agree
> >> > that DRM_IOCTL_WAIT_VBLANK isn't supposed to be used directly. But the
> >> > fact of the matter is that it is being used like that (possibly has
> been
> >> > since before the DRM master concept even existed), and presumably it
> >> > actually works with simple setups (which might be the majority). So
> >> > there might be some backlash if it suddenly stops working.
> >>
> >> Fully agreed. Hence just RFC, and yes we need to get the EGL extension
> >> in place first, and at least kick most of the popular apps to have
> >> their code ready, and wait a bit, and wait some more, before we can
> >> nuke the ioctl from the kernel for non-master. It'll probably take 5
> >> years if we're fast :( I do think that we should be ok with breaking
> >> the last few hold-outs, but we definitely need to have an alternate
> >> solution for EGL ready. Hence why I want to know whether there's
> >> anyone who's using this outside of EGL.
> >>
> >> Really this was just drive-by that I spotted while looking around at
> >> stuff for our other discussion around vblanks.
> >> -Daniel
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 7496 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Thu, Jun 16, 2016 at 3:02 PM, Rainer Hochecker <fernetmenta@kodi.tv> wrote:
> Daniel,
>
> Peter posted me some snippets about your discussion around vblank that
> confused me. Our use case is as follows:
> We synchronise our video player clock to the pace of the display. Assume we
> play some 23.976 content and the actual refresh rate is 24hz. We increment
> the clock every vblank, that makes the clock run faster. Audio is resampled
> to the faster running clock.
> For this we just need an event fired for vblank.
>
> Another requirement not directly related to 1)
> We would like to know when a frame in actually presented on screen. Now this
> is only guessing.


If you do vblank support right (which means GLX_OML_sync_control, it
should work on mesa/glx but not anywhere else unfortunately) you get
both. That's what I mean with calling the vblank ioctl directly is a
hack: It's only really one half of correctly done a/v sync, you get
the clock source but still have no idea when the frames actually show
up. GLX_OML_sync_control (and a port to EGL/wayland, if someone
bothers with that) gives you that control, plus ofc still a clock
source which is as accurate as the current one (since it's the same).

Note that DRI2/DRI3 and wayland all support this already, it's just a
matter of wiring it up as an EGL extension. And of course it won't
work on nvidia blobs because they just suck at this and don't want to
let apps control all the double/triple buffering and whatever else
they have in their driver.

Note that the only reason this works on mesa is because Mario Kleiner
really cares about it, and fixed up all the bugs across the entire
stack (meas, X, kernel drivers). Mario might be interested in an EGL
version of all this, not sure. Adding him.

Oh and: Please feel free to add other video players and folks
interested in this, I don't know them really at all.

Cheers, Daniel

>
> Rainer
>
> On Wed, Jun 15, 2016 at 10:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch>
> wrote:
>>
>> On Wed, Jun 15, 2016 at 9:29 AM, Michel Dänzer <michel@daenzer.net> wrote:
>> > On 14.06.2016 18:35, Daniel Vetter wrote:
>> >> On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel@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?
>> >
>> > Off the top of my head, I know specifically about compton and xfwm4.
>> > There's certainly more.
>>
>> But do they anything more fancy than what could be achieved with
>> OML_sync too? Or is the issue that they don't want to use EGL/GLX? In
>> that case I think it's reasonable to ask them to use bare-metal
>> Present, since that's not any less portable than using the vblank
>> ioctl.
>>
>> > Note that as I mentioned before in the other thread, I absolutely agree
>> > that DRM_IOCTL_WAIT_VBLANK isn't supposed to be used directly. But the
>> > fact of the matter is that it is being used like that (possibly has been
>> > since before the DRM master concept even existed), and presumably it
>> > actually works with simple setups (which might be the majority). So
>> > there might be some backlash if it suddenly stops working.
>>
>> Fully agreed. Hence just RFC, and yes we need to get the EGL extension
>> in place first, and at least kick most of the popular apps to have
>> their code ready, and wait a bit, and wait some more, before we can
>> nuke the ioctl from the kernel for non-master. It'll probably take 5
>> years if we're fast :( I do think that we should be ok with breaking
>> the last few hold-outs, but we definitely need to have an alternate
>> solution for EGL ready. Hence why I want to know whether there's
>> anyone who's using this outside of EGL.
>>
>> Really this was just drive-by that I spotted while looking around at
>> stuff for our other discussion around vblanks.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Rainer Hochecker]

[-- Attachment #1.1: Type: text/plain, Size: 3380 bytes --]

Daniel,

Peter posted me some snippets about your discussion around vblank that
confused me. Our use case is as follows:
We synchronise our video player clock to the pace of the display. Assume we
play some 23.976 content and the actual refresh rate is 24hz. We increment
the clock every vblank, that makes the clock run faster. Audio is resampled
to the faster running clock.
For this we just need an event fired for vblank.

Another requirement not directly related to 1)
We would like to know when a frame in actually presented on screen. Now
this is only guessing.

Rainer

On Wed, Jun 15, 2016 at 10:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch>
wrote:

> On Wed, Jun 15, 2016 at 9:29 AM, Michel Dänzer <michel@daenzer.net> wrote:
> > On 14.06.2016 18:35, Daniel Vetter wrote:
> >> On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel@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?
> >
> > Off the top of my head, I know specifically about compton and xfwm4.
> > There's certainly more.
>
> But do they anything more fancy than what could be achieved with
> OML_sync too? Or is the issue that they don't want to use EGL/GLX? In
> that case I think it's reasonable to ask them to use bare-metal
> Present, since that's not any less portable than using the vblank
> ioctl.
>
> > Note that as I mentioned before in the other thread, I absolutely agree
> > that DRM_IOCTL_WAIT_VBLANK isn't supposed to be used directly. But the
> > fact of the matter is that it is being used like that (possibly has been
> > since before the DRM master concept even existed), and presumably it
> > actually works with simple setups (which might be the majority). So
> > there might be some backlash if it suddenly stops working.
>
> Fully agreed. Hence just RFC, and yes we need to get the EGL extension
> in place first, and at least kick most of the popular apps to have
> their code ready, and wait a bit, and wait some more, before we can
> nuke the ioctl from the kernel for non-master. It'll probably take 5
> years if we're fast :( I do think that we should be ok with breaking
> the last few hold-outs, but we definitely need to have an alternate
> solution for EGL ready. Hence why I want to know whether there's
> anyone who's using this outside of EGL.
>
> Really this was just drive-by that I spotted while looking around at
> stuff for our other discussion around vblanks.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 4457 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Wed, Jun 15, 2016 at 9:29 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On 14.06.2016 18:35, Daniel Vetter wrote:
>> On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel@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?
>
> Off the top of my head, I know specifically about compton and xfwm4.
> There's certainly more.

But do they anything more fancy than what could be achieved with
OML_sync too? Or is the issue that they don't want to use EGL/GLX? In
that case I think it's reasonable to ask them to use bare-metal
Present, since that's not any less portable than using the vblank
ioctl.

> Note that as I mentioned before in the other thread, I absolutely agree
> that DRM_IOCTL_WAIT_VBLANK isn't supposed to be used directly. But the
> fact of the matter is that it is being used like that (possibly has been
> since before the DRM master concept even existed), and presumably it
> actually works with simple setups (which might be the majority). So
> there might be some backlash if it suddenly stops working.

Fully agreed. Hence just RFC, and yes we need to get the EGL extension
in place first, and at least kick most of the popular apps to have
their code ready, and wait a bit, and wait some more, before we can
nuke the ioctl from the kernel for non-master. It'll probably take 5
years if we're fast :( I do think that we should be ok with breaking
the last few hold-outs, but we definitely need to have an alternate
solution for EGL ready. Hence why I want to know whether there's
anyone who's using this outside of EGL.

Really this was just drive-by that I spotted while looking around at
stuff for our other discussion around vblanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

On 14.06.2016 18:35, Daniel Vetter wrote:
> On Tue, Jun 14, 2016 at 11:09 AM, Michel Dänzer <michel@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?

Off the top of my head, I know specifically about compton and xfwm4.
There's certainly more.

Note that as I mentioned before in the other thread, I absolutely agree
that DRM_IOCTL_WAIT_VBLANK isn't supposed to be used directly. But the
fact of the matter is that it is being used like that (possibly has been
since before the DRM master concept even existed), and presumably it
actually works with simple setups (which might be the majority). So
there might be some backlash if it suddenly stops working.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

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.

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


> -	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.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

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. This
patch is meant to essentially start kicking that can from the back
end.

Cc: fritsch@kodi.tv
Cc: fernetmenta@kodi.tv
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 0510675eec5d..6cc78d648393 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -529,9 +529,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_MASTER|DRM_UNLOCKED),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel