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

[Deng, Emily]

Yes, I think we should add this rule to tell user mode driver, just like they will pass bo_list if they share bo with others.

Best Wishes,
Emily Deng

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of zhoucm1
> Sent: Wednesday, May 30, 2018 11:36 AM
> To: Deng, Emily <Emily.Deng at amd.com>; Zhou, David(ChunMing)
> <David1.Zhou at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: To get gds, gws and oa from adev->gds
> 
> 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) {
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx