diff mbox

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

Message ID 1465894932-3759-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 14, 2016, 9:02 a.m. UTC
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(-)

Comments

Rainer Hochecker Aug. 8, 2018, 3:35 p.m. UTC | #1
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 Aug. 8, 2018, 4:59 p.m. UTC | #2
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
>>
Rainer Hochecker Aug. 8, 2018, 5:41 p.m. UTC | #3
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
diff mbox

Patch

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),