[PATCH] drm: turn syncobj timeline support off by default

[PATCH] drm: turn syncobj timeline support off by default

[Lionel Landwerlin]

Unfortunately userspace users of this API cannot be publicly disclosed
yet so disable this stuff by default until all is revealed.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/Kconfig       | 10 ++++++++++
 drivers/gpu/drm/drm_syncobj.c | 12 ++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 5e1bc630b885..5b002793f0e4 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -423,3 +423,13 @@ config DRM_PANEL_ORIENTATION_QUIRKS
 config DRM_LIB_RANDOM
 	bool
 	default n
+
+config DRM_SYNCOBJ_TIMELINE
+	bool "Enable syncobj timeline support"
+	depends on DRM
+	default n
+	help
+	  Choose this option if you want to enable preliminary support
+	  for timeline syncobjs.
+
+	  If in doubt, say "N".
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index ea22b79a7170..dd1bdd287225 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -754,6 +754,9 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
 	struct drm_syncobj_transfer *args = data;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
+		return -ENODEV;
+
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
 		return -ENODEV;
 
@@ -1105,6 +1108,9 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
 	struct drm_syncobj **syncobjs;
 	int ret = 0;
 
+	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
+		return -ENODEV;
+
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
 		return -ENODEV;
 
@@ -1209,6 +1215,9 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
 	uint32_t i, j;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
+		return -ENODEV;
+
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
 		return -EOPNOTSUPP;
 
@@ -1280,6 +1289,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 	uint32_t i;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
+		return -ENODEV;
+
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
 		return -ENODEV;
 
-- 
2.21.0.392.gf8f6787159e

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

Re: [PATCH] drm: turn syncobj timeline support off by default

[Dave Airlie]

On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
>
> Unfortunately userspace users of this API cannot be publicly disclosed
> yet so disable this stuff by default until all is revealed.

This begs the question how userspace is meant to know we support these?

Is there a CAP for it? if not why not?

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

Re: [PATCH] drm: turn syncobj timeline support off by default

[Lionel Landwerlin]

On 15/04/2019 20:52, Dave Airlie wrote:
> On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
>> Unfortunately userspace users of this API cannot be publicly disclosed
>> yet so disable this stuff by default until all is revealed.
> This begs the question how userspace is meant to know we support these?
>
> Is there a CAP for it? if not why not?
>
> Dave.
>
This comes with a submission path, so in i915 I've added a CAP.

AMD seems to have a versioned interface.


-Lionel

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

Re: [PATCH] drm: turn syncobj timeline support off by default

[Dave Airlie]

On Tue, 16 Apr 2019 at 06:05, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
>
> On 15/04/2019 20:52, Dave Airlie wrote:
> > On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin
> > <lionel.g.landwerlin@intel.com> wrote:
> >> Unfortunately userspace users of this API cannot be publicly disclosed
> >> yet so disable this stuff by default until all is revealed.
> > This begs the question how userspace is meant to know we support these?
> >
> > Is there a CAP for it? if not why not?
> >
> > Dave.
> >
> This comes with a submission path, so in i915 I've added a CAP.
>
> AMD seems to have a versioned interface.

I think we would do like we did for syncobjs, you have a generic cap
that the driver controls.

The versioned interface won't be useful if we decide to backport a fix for this.

Dave.

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

Re: [PATCH] drm: turn syncobj timeline support off by default

[Daniel Vetter]

On Mon, Apr 15, 2019 at 08:48:25PM +0100, Lionel Landwerlin wrote:
> Unfortunately userspace users of this API cannot be publicly disclosed
> yet so disable this stuff by default until all is revealed.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/Kconfig       | 10 ++++++++++
>  drivers/gpu/drm/drm_syncobj.c | 12 ++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 5e1bc630b885..5b002793f0e4 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -423,3 +423,13 @@ config DRM_PANEL_ORIENTATION_QUIRKS
>  config DRM_LIB_RANDOM
>  	bool
>  	default n
> +
> +config DRM_SYNCOBJ_TIMELINE
> +	bool "Enable syncobj timeline support"
> +	depends on DRM
> +	default n
> +	help
> +	  Choose this option if you want to enable preliminary support
> +	  for timeline syncobjs.
> +
> +	  If in doubt, say "N".
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index ea22b79a7170..dd1bdd287225 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -754,6 +754,9 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
>  	struct drm_syncobj_transfer *args = data;
>  	int ret;
>  
> +	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
> +		return -ENODEV;

I think the latest rage for this stuff is a module_option_unsafe. Much
easier for igts to set by default (we want to keep CI'ing this ofc). And
_unsafe taints the kernel when you set it, so about equally good.
-Daniel

> +
>  	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>  		return -ENODEV;
>  
> @@ -1105,6 +1108,9 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
>  	struct drm_syncobj **syncobjs;
>  	int ret = 0;
>  
> +	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
> +		return -ENODEV;
> +
>  	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>  		return -ENODEV;
>  
> @@ -1209,6 +1215,9 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
>  	uint32_t i, j;
>  	int ret;
>  
> +	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
> +		return -ENODEV;
> +
>  	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>  		return -EOPNOTSUPP;
>  
> @@ -1280,6 +1289,9 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>  	uint32_t i;
>  	int ret;
>  
> +	if (!IS_ENABLED(CONFIG_DRM_SYNCOBJ_TIMELINE))
> +		return -ENODEV;
> +
>  	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>  		return -ENODEV;
>  
> -- 
> 2.21.0.392.gf8f6787159e
> 

-- 
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] drm: turn syncobj timeline support off by default

[Lionel Landwerlin]

On 15/04/2019 21:23, Dave Airlie wrote:
> On Tue, 16 Apr 2019 at 06:05, Lionel Landwerlin
> <lionel.g.landwerlin@intel.com> wrote:
>> On 15/04/2019 20:52, Dave Airlie wrote:
>>> On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin
>>> <lionel.g.landwerlin@intel.com> wrote:
>>>> Unfortunately userspace users of this API cannot be publicly disclosed
>>>> yet so disable this stuff by default until all is revealed.
>>> This begs the question how userspace is meant to know we support these?
>>>
>>> Is there a CAP for it? if not why not?
>>>
>>> Dave.
>>>
>> This comes with a submission path, so in i915 I've added a CAP.
>>
>> AMD seems to have a versioned interface.
> I think we would do like we did for syncobjs, you have a generic cap
> that the driver controls.
>
> The versioned interface won't be useful if we decide to backport a fix for this.
>
> Dave.


Okay, but then it's not a global setting anymore :)

Which is what was suggested on IRC.


I'm fine with it regardless :)


-Lionel


>
>>
>> -Lionel
>>

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

Re: [PATCH] drm: turn syncobj timeline support off by default

[Daniel Vetter]

On Tue, Apr 16, 2019 at 11:51 AM Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
>
> On 15/04/2019 21:23, Dave Airlie wrote:
> > On Tue, 16 Apr 2019 at 06:05, Lionel Landwerlin
> > <lionel.g.landwerlin@intel.com> wrote:
> >> On 15/04/2019 20:52, Dave Airlie wrote:
> >>> On Tue, 16 Apr 2019 at 05:48, Lionel Landwerlin
> >>> <lionel.g.landwerlin@intel.com> wrote:
> >>>> Unfortunately userspace users of this API cannot be publicly disclosed
> >>>> yet so disable this stuff by default until all is revealed.
> >>> This begs the question how userspace is meant to know we support these?
> >>>
> >>> Is there a CAP for it? if not why not?
> >>>
> >>> Dave.
> >>>
> >> This comes with a submission path, so in i915 I've added a CAP.
> >>
> >> AMD seems to have a versioned interface.
> > I think we would do like we did for syncobjs, you have a generic cap
> > that the driver controls.
> >
> > The versioned interface won't be useful if we decide to backport a fix for this.

I guess amdgpu could then backport the version bump. Also, this is why
I don't like versioned uapi really. Either way we also need to hide
the amdgpu version bump until we've finalized the uapi and khr pushed
out the spec.

> Okay, but then it's not a global setting anymore :)
>
> Which is what was suggested on IRC.
>
>
> I'm fine with it regardless :)

I think global is good enough. If either i915 or amdgpu screwed up the
uapi enough that we can't backport the enabling patch, then maybe not
a good idea to do for either. Ofc this assumes the uapi as merged is
forward compatible already, i.e. new userspace has a way to figure out
whether timeline sync are support or not for a given driver.
-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