[PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo (v3)

[Christian König]

Am 31.07.2018 um 10:05 schrieb Zhang, Jerry (Junwei):
> On 07/31/2018 03:03 PM, Christian König wrote:
>> Am 31.07.2018 um 08:58 schrieb Zhang, Jerry (Junwei):
>>> On 07/30/2018 06:47 PM, Christian König wrote:
>>>> Am 30.07.2018 um 12:02 schrieb Junwei Zhang:
>>>>> From: Chunming Zhou <David1.Zhou at amd.com>
>>>>>
>>>>> v2: get original gem handle from gobj
>>>>> v3: update find bo data structure as union(in, out)
>>>>>      simply some code logic
>>>>
>>>> Do we now have an open source user for this, so that we can 
>>>> upstream it? One more point below.
>>>>
>>>>>
>>>>> Signed-off-by: Chunming Zhou <David1.Zhou at amd.com>
>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang at amd.com> (v3)
>>>>> Reviewed-by: Christian König <christian.koenig at amd.com>
>>>>> Reviewed-by: Jammy Zhou <Jammy.Zhou at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 63 
>>>>> +++++++++++++++++++++++++++++++++
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  3 +-
>>>>>   include/uapi/drm/amdgpu_drm.h           | 21 +++++++++++
>>>>>   4 files changed, 88 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index 4cd20e7..46c370b 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -1213,6 +1213,8 @@ int amdgpu_gem_info_ioctl(struct drm_device 
>>>>> *dev, void *data,
>>>>>                 struct drm_file *filp);
>>>>>   int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>>>>>               struct drm_file *filp);
>>>>> +int amdgpu_gem_find_bo_by_cpu_mapping_ioctl(struct drm_device 
>>>>> *dev, void *data,
>>>>> +                        struct drm_file *filp);
>>>>>   int amdgpu_gem_mmap_ioctl(struct drm_device *dev, void *data,
>>>>>                 struct drm_file *filp);
>>>>>   int amdgpu_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> index 71792d8..bae8417 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>>> @@ -288,6 +288,69 @@ int amdgpu_gem_create_ioctl(struct drm_device 
>>>>> *dev, void *data,
>>>>>       return 0;
>>>>>   }
>>>>> +static int amdgpu_gem_get_handle_from_object(struct drm_file *filp,
>>>>> +                         struct drm_gem_object *obj)
>>>>> +{
>>>>> +    int i;
>>>>> +    struct drm_gem_object *tmp;
>>>>> +
>>>>> +    spin_lock(&filp->table_lock);
>>>>> +    idr_for_each_entry(&filp->object_idr, tmp, i) {
>>>>> +        if (obj == tmp) {
>>>>> +            drm_gem_object_reference(obj);
>>>>> +            spin_unlock(&filp->table_lock);
>>>>> +            return i;
>>>>> +        }
>>>>> +    }
>>>>
>>>> Please double check if that is still up to date.
>>>
>>> We may have to replace drm_gem_object_reference() with 
>>> drm_gem_object_get().
>>>
>>> On 2nd thought, do we really need to do reference every time?
>>
>> Yes, that's a must have. Otherwise the handle could be freed and 
>> reused already when we return.
>>
>>> if UMD find the same gem object for 3 times, it also need to 
>>> explicitly free(put) that object for 3 times?
>>
>> Correct yes. Thinking more about this the real problem is to 
>> translate the handle into a structure in libdrm.
>>
>> Here we are back to the problem Marek and Michel has been working on 
>> for a while that we always need to be able to translate a handle into 
>> a bo structure.....
>>
>> So that needs to be solved before we can upstream the changes.
>
> Thanks for your info.
> It's better to fix that before upstream.

Thinking more about this the hash currently used in libdrm is not 
adequate any more.

E.g. we now need to be able to find all BOs based on their handle. Since 
the handles are dense either an r/b tree or a radix tree now sounds like 
the best approach to me.

Christian.

>
> Regards,
> Jerry