[PATCH] drm/syncobj: return meaningful value to user space

[Chunming Zhou]

在 2019/7/18 22:08, Lionel Landwerlin 写道:
> On 18/07/2019 16:02, Chunming Zhou wrote:
>> 在 2019/7/18 19:31, Lionel Landwerlin 写道:
>>> On 18/07/2019 14:13, Chunming Zhou wrote:
>>>> if WAIT_FOR_SUBMIT isn't set and in the meanwhile no underlying fence
>>>> on syncobj,
>>>> then return non-block error code to user sapce.
>>>>
>>>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>>>> ---
>>>>    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 361a01a08c18..929f7c64f9a2 100644
>>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>> @@ -252,7 +252,7 @@ int drm_syncobj_find_fence(struct drm_file
>>>> *file_private,
>>>>                return 0;
>>>>            dma_fence_put(*fence);
>>>>        } else {
>>>> -        ret = -EINVAL;
>>>> +        ret = -ENOTBLK;
>>>>        }
>>>>          if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
>>>> @@ -832,7 +832,7 @@ static signed long
>>>> drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>>>>                if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
>>>>                    continue;
>>>>                } else {
>>>> -                timeout = -EINVAL;
>>>> +                timeout = -ENOTBLK;
>>>>                    goto cleanup_entries;
>>>>                }
>>>>            }
>>>
>>> This would break existing tests for binary syncobjs.
>> How does this breaks binary syncobj?
>
>
> This is used in the submission path of several drivers.
>
> Changing the error code will change what the drivers are reporting to 
> userspace and could break tests.
>
>
> i915 doesn't use that function so it's not affected but 
> lima/panfrost/vc4 seem to be.


anyone from vc4 can confirm this? There are many place in wait_ioctl 
being able to return previous EINVAL, not sure what they use to.


>
>
>>
>>
>>> Is this really what we want?
>> I want to use this meaningful return value to judge if WaitBeforeSignal
>> happens.
>>
>> I think this is the cheapest change for that.
>
>
> I thought the plan was to add a new ioctl to query the last submitted 
> value.

Yes, that is optional way too.  I just thought changing return value is 
very cheap, isn't it?


-David

>
> Did I misunderstand?
>
>
> Thanks,
>
>
> -Lionel
>
>
>>
>> -David
>>
>>
>>>
>>> -Lionel
>>>
>>>
>