diff mbox series

[1/2] drm: report consistent errors when checking syncobj capibility

Message ID 20190416123048.2913-1-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm: report consistent errors when checking syncobj capibility | expand

Commit Message

Lionel Landwerlin April 16, 2019, 12:30 p.m. UTC 
We've been somewhat inconsistent when adding the new ioctl and
returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
capibility.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and timeline v2")
Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/drm_syncobj.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christian König April 16, 2019, 12:40 p.m. UTC | #1 
Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
> We've been somewhat inconsistent when adding the new ioctl and
> returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
> capibility.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and timeline v2")
> Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Chunming Zhou <david1.zhou@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for the series.

How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so 
that userspace can note that as well?

Thanks,
Christian.

> ---
>   drivers/gpu/drm/drm_syncobj.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index c534c5d46f1e..fb65f13d25cf 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
>   	int ret;
>   
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> -		return -ENODEV;
> +		return -EOPNOTSUPP;
>   
>   	if (args->pad)
>   		return -EINVAL;
> @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
>   	int ret = 0;
>   
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> -		return -ENODEV;
> +		return -EOPNOTSUPP;
>   
>   	if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>   			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
Daniel Vetter April 16, 2019, 12:43 p.m. UTC | #2 
On Tue, Apr 16, 2019 at 02:40:37PM +0200, Christian König wrote:
> Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
> > We've been somewhat inconsistent when adding the new ioctl and
> > returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
> > capibility.
> > 
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and timeline v2")
> > Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Chunming Zhou <david1.zhou@amd.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com> for the series.
> 
> How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so that
> userspace can note that as well?

Attempting one of the ioctls and getting a EOPNOTSUPP should be good
enough. In case that "Daniel" meant me ...
-Daniel

> 
> Thanks,
> Christian.
> 
> > ---
> >   drivers/gpu/drm/drm_syncobj.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index c534c5d46f1e..fb65f13d25cf 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
> >   	int ret;
> >   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> > -		return -ENODEV;
> > +		return -EOPNOTSUPP;
> >   	if (args->pad)
> >   		return -EINVAL;
> > @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
> >   	int ret = 0;
> >   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> > -		return -ENODEV;
> > +		return -EOPNOTSUPP;
> >   	if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
> >   			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Lionel Landwerlin April 16, 2019, 12:43 p.m. UTC | #3 
On 16/04/2019 13:40, Christian König wrote:
> Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
>> We've been somewhat inconsistent when adding the new ioctl and
>> returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
>> capibility.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between 
>> binary and timeline v2")
>> Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point 
>> wait v8")
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Chunming Zhou <david1.zhou@amd.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com> for the series.
>
> How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested 
> so that userspace can note that as well?
>
> Thanks,
> Christian.


Thanks Christian, I forgot about that...


>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index c534c5d46f1e..fb65f13d25cf 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device 
>> *dev, void *data,
>>       int ret;
>>         if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> -        return -ENODEV;
>> +        return -EOPNOTSUPP;
>>         if (args->pad)
>>           return -EINVAL;
>> @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct 
>> drm_device *dev, void *data,
>>       int ret = 0;
>>         if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> -        return -ENODEV;
>> +        return -EOPNOTSUPP;
>>         if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>>                   DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>
>
Christian König April 16, 2019, 12:44 p.m. UTC | #4 
Am 16.04.19 um 14:43 schrieb Daniel Vetter:
> On Tue, Apr 16, 2019 at 02:40:37PM +0200, Christian König wrote:
>> Am 16.04.19 um 14:30 schrieb Lionel Landwerlin:
>>> We've been somewhat inconsistent when adding the new ioctl and
>>> returned ENODEV instead of EOPNOTSUPPORTED upon failing the syncobj
>>> capibility.
>>>
>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Fixes: ea569910cbab98 ("drm/syncobj: add transition iotcls between binary and timeline v2")
>>> Fixes: 01d6c357837918 ("drm/syncobj: add support for timeline point wait v8")
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Chunming Zhou <david1.zhou@amd.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com> for the series.
>>
>> How about also adding a DRM_CAP_TIMELINE_SYNCOBJ as Daniel suggested so that
>> userspace can note that as well?
> Attempting one of the ioctls and getting a EOPNOTSUPP should be good
> enough. In case that "Daniel" meant me ...

Oh, sorry my fault. It was actually Dave who suggested that...

Christian.

> -Daniel
>
>> Thanks,
>> Christian.
>>
>>> ---
>>>    drivers/gpu/drm/drm_syncobj.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index c534c5d46f1e..fb65f13d25cf 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -756,7 +756,7 @@ drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
>>>    	int ret;
>>>    	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> -		return -ENODEV;
>>> +		return -EOPNOTSUPP;
>>>    	if (args->pad)
>>>    		return -EINVAL;
>>> @@ -1107,7 +1107,7 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
>>>    	int ret = 0;
>>>    	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>>> -		return -ENODEV;
>>> +		return -EOPNOTSUPP;
>>>    	if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
>>>    			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index c534c5d46f1e..fb65f13d25cf 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -756,7 +756,7 @@  drm_syncobj_transfer_ioctl(struct drm_device *dev, void *data,
 	int ret;
 
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
-		return -ENODEV;
+		return -EOPNOTSUPP;
 
 	if (args->pad)
 		return -EINVAL;
@@ -1107,7 +1107,7 @@  drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
 	int ret = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
-		return -ENODEV;
+		return -EOPNOTSUPP;
 
 	if (args->flags & ~(DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL |
 			    DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |