[PATCH] drm/amdgpu: To get gds, gws and oa from adev->gds

[zhoucm1]

Yes, but how to make sure there is bo list if all BOs are per-vm-bo? 
Maybe we say we should give this rule to UMD, but who can make sure it? 
we can imagine a situation when UMD forgot this rule, they create gds bo 
with per-vm-bo, there is no bo list for CS, then our kernel give a 
kernel gds as backup, everything runs ok, but UMD never get right 
result, it will be very difficult to debug, right?

That is why I created patch #2 to make sure gds BOs created from user 
space are in bo list.


Regards,
David Zhou
On 2018年05月30日 11:27, Deng, Emily wrote:
> Hi David,
>       For the use case that the user mode creates the gds, gws, and oa buffer object itself, it need to pass bo_list to kernel driver, and we need
> to get those objects from bo_list.
>
> Best Wishes,
> Emily Deng
>
>> -----Original Message-----
>> From: Zhou, David(ChunMing)
>> Sent: Tuesday, May 29, 2018 7:23 PM
>> To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu: To get gds, gws and oa from adev->gds
>>
>>
>>
>> On 2018年05月29日 17:17, Emily Deng wrote:
>>> As now enabled per vm bo feature, the user mode driver won't supply
>>> the bo_list generally, for this case, the gdb_base, gds_size,
>>> gws_base, gws_size and oa_base, oa_size won't be set.
>>>
>>> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 38
>> ++++++++++++++++++++--------------
>>>    1 file changed, 23 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index d296c95..ff94435 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -546,6 +546,9 @@ static int amdgpu_cs_parser_bos(struct
>> amdgpu_cs_parser *p,
>>>    	struct amdgpu_bo_list_entry *e;
>>>    	struct list_head duplicates;
>>>    	unsigned i, tries = 10;
>>> +	struct amdgpu_bo *gds;
>>> +	struct amdgpu_bo *gws;
>>> +	struct amdgpu_bo *oa;
>>>    	int r;
>>>
>>>    	INIT_LIST_HEAD(&p->validated);
>>> @@ -698,10 +701,11 @@ static int amdgpu_cs_parser_bos(struct
>>> amdgpu_cs_parser *p,
>>>
>>>    	amdgpu_cs_report_moved_bytes(p->adev, p->bytes_moved,
>>>    				     p->bytes_moved_vis);
>>> +
>>>    	if (p->bo_list) {
>>> -		struct amdgpu_bo *gds = p->bo_list->gds_obj;
>>> -		struct amdgpu_bo *gws = p->bo_list->gws_obj;
>>> -		struct amdgpu_bo *oa = p->bo_list->oa_obj;
>>> +		gds = p->bo_list->gds_obj;
>>> +		gws = p->bo_list->gws_obj;
>>> +		oa = p->bo_list->oa_obj;
>> out of curious, why not directly use p->adev->gds however per-vm-bo is
>> enabled?
>>
>> Regards,
>> David Zhou
>>>    		struct amdgpu_vm *vm = &fpriv->vm;
>>>    		unsigned i;
>>>
>>> @@ -710,19 +714,23 @@ static int amdgpu_cs_parser_bos(struct
>>> amdgpu_cs_parser *p,
>>>
>>>    			p->bo_list->array[i].bo_va =
>> amdgpu_vm_bo_find(vm, bo);
>>>    		}
>>> +	} else {
>>> +		gds = p->adev->gds.gds_gfx_bo;
>>> +		gws = p->adev->gds.gws_gfx_bo;
>>> +		oa = p->adev->gds.oa_gfx_bo;
>>> +	}
>>>
>>> -		if (gds) {
>>> -			p->job->gds_base = amdgpu_bo_gpu_offset(gds);
>>> -			p->job->gds_size = amdgpu_bo_size(gds);
>>> -		}
>>> -		if (gws) {
>>> -			p->job->gws_base = amdgpu_bo_gpu_offset(gws);
>>> -			p->job->gws_size = amdgpu_bo_size(gws);
>>> -		}
>>> -		if (oa) {
>>> -			p->job->oa_base = amdgpu_bo_gpu_offset(oa);
>>> -			p->job->oa_size = amdgpu_bo_size(oa);
>>> -		}
>>> +	if (gds) {
>>> +		p->job->gds_base = amdgpu_bo_gpu_offset(gds);
>>> +		p->job->gds_size = amdgpu_bo_size(gds);
>>> +	}
>>> +	if (gws) {
>>> +		p->job->gws_base = amdgpu_bo_gpu_offset(gws);
>>> +		p->job->gws_size = amdgpu_bo_size(gws);
>>> +	}
>>> +	if (oa) {
>>> +		p->job->oa_base = amdgpu_bo_gpu_offset(oa);
>>> +		p->job->oa_size = amdgpu_bo_size(oa);
>>>    	}
>>>
>>>    	if (!r && p->uf_entry.robj) {