Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 07/31/2018 05:26 PM, Huang Rui wrote:
> On Tue, Jul 31, 2018 at 05:00:46PM +0800, Koenig, Christian wrote:
>> Am 31.07.2018 um 11:09 schrieb Huang Rui:
>>> On Tue, Jul 31, 2018 at 10:52:06AM +0200, Christian König wrote:
>>>> Am 31.07.2018 um 09:51 schrieb Huang Rui:
>>>>> On Mon, Jul 30, 2018 at 04:51:59PM +0200, Christian König wrote:
>>>>>> Instead of having extra handling just create an empty bo_list when no
>>>>>> handle is provided.
>>>>> Reviewed-by: Huang Rui <ray.huang@amd.com>
>>>>>
>>>>> In which case, when the command is being submitted, there is no bo list
>>>>> handle? All BOs are per-VM, no shared BO?
>>>> Yes, exactly. Or in the future just SVM etc...
>>>>
>>> "SVM" means single VM? Only one thread?
>>
>> Shared Virtual Memory, e.g. the GPU uses the virtual CPU address space.
>>
>> Can be implemented using the ATC or starting with Vega10 using HMM.
>>
>
> Oh, I remembered it should be a proposal for apu such as raven, you know,
> the vram is actually carved out from a part of system memory in raven.

That's a feature in OpenCL 2.0

Regards,
Jerry

>
> Thanks,
> Ray
>
>> Regards,
>> Christian.
>>
>>>
>>> Thanks,
>>> Ray
>>>
>>>> Christian.
>>>>
>>>>> Thanks,
>>>>> Ray
>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 111 ++++++++++++++-------------------
>>>>>>    1 file changed, 46 insertions(+), 65 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> index 1d7292ab2b62..502b94fb116a 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>>> @@ -561,6 +561,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>>>>>>    				union drm_amdgpu_cs *cs)
>>>>>>    {
>>>>>>    	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>>>>>> +	struct amdgpu_vm *vm = &fpriv->vm;
>>>>>>    	struct amdgpu_bo_list_entry *e;
>>>>>>    	struct list_head duplicates;
>>>>>>    	struct amdgpu_bo *gds;
>>>>>> @@ -580,13 +581,17 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>>>>>>    				       &p->bo_list);
>>>>>>    		if (r)
>>>>>>    			return r;
>>>>>> +	} else if (!p->bo_list) {
>>>>>> +		/* Create a empty bo_list when no handle is provided */
>>>>>> +		r = amdgpu_bo_list_create(p->adev, p->filp, NULL, 0,
>>>>>> +					  &p->bo_list);
>>>>>> +		if (r)
>>>>>> +			return r;
>>>>>>    	}
>>>>>> -	if (p->bo_list) {
>>>>>> -		amdgpu_bo_list_get_list(p->bo_list, &p->validated);
>>>>>> -		if (p->bo_list->first_userptr != p->bo_list->num_entries)
>>>>>> -			p->mn = amdgpu_mn_get(p->adev, AMDGPU_MN_TYPE_GFX);
>>>>>> -	}
>>>>>> +	amdgpu_bo_list_get_list(p->bo_list, &p->validated);
>>>>>> +	if (p->bo_list->first_userptr != p->bo_list->num_entries)
>>>>>> +		p->mn = amdgpu_mn_get(p->adev, AMDGPU_MN_TYPE_GFX);
>>>>>>    	INIT_LIST_HEAD(&duplicates);
>>>>>>    	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
>>>>>> @@ -605,10 +610,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>>>>>>    			goto error_free_pages;
>>>>>>    		}
>>>>>> -		/* Without a BO list we don't have userptr BOs */
>>>>>> -		if (!p->bo_list)
>>>>>> -			break;
>>>>>> -
>>>>>>    		INIT_LIST_HEAD(&need_pages);
>>>>>>    		amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>>>>>    			struct amdgpu_bo *bo = e->robj;
>>>>>> @@ -703,21 +704,12 @@ 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_vm *vm = &fpriv->vm;
>>>>>> -		struct amdgpu_bo_list_entry *e;
>>>>>> +	gds = p->bo_list->gds_obj;
>>>>>> +	gws = p->bo_list->gws_obj;
>>>>>> +	oa = p->bo_list->oa_obj;
>>>>>> -		gds = p->bo_list->gds_obj;
>>>>>> -		gws = p->bo_list->gws_obj;
>>>>>> -		oa = p->bo_list->oa_obj;
>>>>>> -
>>>>>> -		amdgpu_bo_list_for_each_entry(e, p->bo_list)
>>>>>> -			e->bo_va = amdgpu_vm_bo_find(vm, e->robj);
>>>>>> -	} else {
>>>>>> -		gds = p->adev->gds.gds_gfx_bo;
>>>>>> -		gws = p->adev->gds.gws_gfx_bo;
>>>>>> -		oa = p->adev->gds.oa_gfx_bo;
>>>>>> -	}
>>>>>> +	amdgpu_bo_list_for_each_entry(e, p->bo_list)
>>>>>> +		e->bo_va = amdgpu_vm_bo_find(vm, e->robj);
>>>>>>    	if (gds) {
>>>>>>    		p->job->gds_base = amdgpu_bo_gpu_offset(gds);
>>>>>> @@ -745,15 +737,13 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>>>>>>    error_free_pages:
>>>>>> -	if (p->bo_list) {
>>>>>> -		amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>>>>> -			if (!e->user_pages)
>>>>>> -				continue;
>>>>>> +	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>>>>> +		if (!e->user_pages)
>>>>>> +			continue;
>>>>>> -			release_pages(e->user_pages,
>>>>>> -				      e->robj->tbo.ttm->num_pages);
>>>>>> -			kvfree(e->user_pages);
>>>>>> -		}
>>>>>> +		release_pages(e->user_pages,
>>>>>> +			      e->robj->tbo.ttm->num_pages);
>>>>>> +		kvfree(e->user_pages);
>>>>>>    	}
>>>>>>    	return r;
>>>>>> @@ -815,9 +805,10 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
>>>>>>    static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>>>>>>    {
>>>>>> -	struct amdgpu_device *adev = p->adev;
>>>>>>    	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>>>>>> +	struct amdgpu_device *adev = p->adev;
>>>>>>    	struct amdgpu_vm *vm = &fpriv->vm;
>>>>>> +	struct amdgpu_bo_list_entry *e;
>>>>>>    	struct amdgpu_bo_va *bo_va;
>>>>>>    	struct amdgpu_bo *bo;
>>>>>>    	int r;
>>>>>> @@ -850,31 +841,26 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>>>>>>    			return r;
>>>>>>    	}
>>>>>> -	if (p->bo_list) {
>>>>>> -		struct amdgpu_bo_list_entry *e;
>>>>>> -
>>>>>> -		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>>>>>> -			struct dma_fence *f;
>>>>>> -
>>>>>> -			/* ignore duplicates */
>>>>>> -			bo = e->robj;
>>>>>> -			if (!bo)
>>>>>> -				continue;
>>>>>> +	amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>>>>>> +		struct dma_fence *f;
>>>>>> -			bo_va = e->bo_va;
>>>>>> -			if (bo_va == NULL)
>>>>>> -				continue;
>>>>>> +		/* ignore duplicates */
>>>>>> +		bo = e->robj;
>>>>>> +		if (!bo)
>>>>>> +			continue;
>>>>>> -			r = amdgpu_vm_bo_update(adev, bo_va, false);
>>>>>> -			if (r)
>>>>>> -				return r;
>>>>>> +		bo_va = e->bo_va;
>>>>>> +		if (bo_va == NULL)
>>>>>> +			continue;
>>>>>> -			f = bo_va->last_pt_update;
>>>>>> -			r = amdgpu_sync_fence(adev, &p->job->sync, f, false);
>>>>>> -			if (r)
>>>>>> -				return r;
>>>>>> -		}
>>>>>> +		r = amdgpu_vm_bo_update(adev, bo_va, false);
>>>>>> +		if (r)
>>>>>> +			return r;
>>>>>> +		f = bo_va->last_pt_update;
>>>>>> +		r = amdgpu_sync_fence(adev, &p->job->sync, f, false);
>>>>>> +		if (r)
>>>>>> +			return r;
>>>>>>    	}
>>>>>>    	r = amdgpu_vm_handle_moved(adev, vm);
>>>>>> @@ -889,9 +875,7 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>>>>>>    	if (r)
>>>>>>    		return r;
>>>>>> -	if (amdgpu_vm_debug && p->bo_list) {
>>>>>> -		struct amdgpu_bo_list_entry *e;
>>>>>> -
>>>>>> +	if (amdgpu_vm_debug) {
>>>>>>    		/* Invalidate all BOs to test for userspace bugs */
>>>>>>    		amdgpu_bo_list_for_each_entry(e, p->bo_list) {
>>>>>>    			/* ignore duplicates */
>>>>>> @@ -1217,22 +1201,19 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
>>>>>>    	struct amdgpu_ring *ring = p->ring;
>>>>>>    	struct drm_sched_entity *entity = &p->ctx->rings[ring->idx].entity;
>>>>>>    	enum drm_sched_priority priority;
>>>>>> +	struct amdgpu_bo_list_entry *e;
>>>>>>    	struct amdgpu_job *job;
>>>>>>    	uint64_t seq;
>>>>>>    	int r;
>>>>>>    	amdgpu_mn_lock(p->mn);
>>>>>> -	if (p->bo_list) {
>>>>>> -		struct amdgpu_bo_list_entry *e;
>>>>>> +	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>>>>> +		struct amdgpu_bo *bo = e->robj;
>>>>>> -		amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
>>>>>> -			struct amdgpu_bo *bo = e->robj;
>>>>>> -
>>>>>> -			if (amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {
>>>>>> -				amdgpu_mn_unlock(p->mn);
>>>>>> -				return -ERESTARTSYS;
>>>>>> -			}
>>>>>> +		if (amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {
>>>>>> +			amdgpu_mn_unlock(p->mn);
>>>>>> +			return -ERESTARTSYS;
>>>>>>    		}
>>>>>>    	}
>>>>>> --
>>>>>> 2.14.1
>>>>>>
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx