drm/amdgpu: To get gds, gws and oa from adev->gds.

Submitted by Deng, Emily on May 29, 2018, 8:06 a.m.

Details

Message ID 1527581199-31786-1-git-send-email-Emily.Deng@amd.com
State New
Headers show
Series "drm/amdgpu: To get gds, gws and oa from adev->gds." ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Deng, Emily May 29, 2018, 8:06 a.m.
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@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 44 ++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 18 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index e1756b6..49be8b65 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -515,14 +515,17 @@  static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
 	return 0;
 }
 
-static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
-				union drm_amdgpu_cs *cs)
+static int amdgpu_cs_parser_bos(struct amdgpu_device *adev,
+				struct amdgpu_cs_parser *p, union drm_amdgpu_cs *cs)
 {
 	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
 	struct amdgpu_bo_list_entry *e;
 	struct list_head duplicates;
 	unsigned i, tries = 10;
 	int r;
+	struct amdgpu_bo *gds;
+	struct amdgpu_bo *gws;
+	struct amdgpu_bo *oa;
 
 	INIT_LIST_HEAD(&p->validated);
 
@@ -652,10 +655,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;
 		struct amdgpu_vm *vm = &fpriv->vm;
 		unsigned i;
 
@@ -664,19 +668,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 = adev->gds.gds_gfx_bo;
+		gws = adev->gds.gws_gfx_bo;
+		oa = 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) {
@@ -1233,7 +1241,7 @@  int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	if (r)
 		goto out;
 
-	r = amdgpu_cs_parser_bos(&parser, data);
+	r = amdgpu_cs_parser_bos(adev, &parser, data);
 	if (r) {
 		if (r == -ENOMEM)
 			DRM_ERROR("Not enough memory for command submission!\n");

Comments

Am 29.05.2018 um 10:06 schrieb Emily Deng:
> 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.

Good catch, a few minor notes below.

>
> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 44 ++++++++++++++++++++--------------
>   1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index e1756b6..49be8b65 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -515,14 +515,17 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
>   	return 0;
>   }
>   
> -static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> -				union drm_amdgpu_cs *cs)
> +static int amdgpu_cs_parser_bos(struct amdgpu_device *adev,
> +				struct amdgpu_cs_parser *p, union drm_amdgpu_cs *cs)

No need for this change, adev should be available as p->adev as well.

>   {
>   	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
>   	struct amdgpu_bo_list_entry *e;
>   	struct list_head duplicates;
>   	unsigned i, tries = 10;
>   	int r;
> +	struct amdgpu_bo *gds;
> +	struct amdgpu_bo *gws;
> +	struct amdgpu_bo *oa;

Reverse xmas tree order please, e.g. "int r" should be the last one.

Apart from that it looks good to me,
Christian.

>   
>   	INIT_LIST_HEAD(&p->validated);
>   
> @@ -652,10 +655,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;
>   		struct amdgpu_vm *vm = &fpriv->vm;
>   		unsigned i;
>   
> @@ -664,19 +668,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 = adev->gds.gds_gfx_bo;
> +		gws = adev->gds.gws_gfx_bo;
> +		oa = 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) {
> @@ -1233,7 +1241,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>   	if (r)
>   		goto out;
>   
> -	r = amdgpu_cs_parser_bos(&parser, data);
> +	r = amdgpu_cs_parser_bos(adev, &parser, data);
>   	if (r) {
>   		if (r == -ENOMEM)
>   			DRM_ERROR("Not enough memory for command submission!\n");
Thanks Christian, will modify the patch as you suggest.

Best Wishes,
Emily Deng

> -----Original Message-----

> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]

> Sent: Tuesday, May 29, 2018 5:01 PM

> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org

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

> 

> Am 29.05.2018 um 10:06 schrieb Emily Deng:

> > 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.

> 

> Good catch, a few minor notes below.

> 

> >

> > Signed-off-by: Emily Deng <Emily.Deng@amd.com>

> > ---

> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 44

> ++++++++++++++++++++--------------

> >   1 file changed, 26 insertions(+), 18 deletions(-)

> >

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

> > index e1756b6..49be8b65 100644

> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

> > @@ -515,14 +515,17 @@ static int amdgpu_cs_list_validate(struct

> amdgpu_cs_parser *p,

> >   	return 0;

> >   }

> >

> > -static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,

> > -				union drm_amdgpu_cs *cs)

> > +static int amdgpu_cs_parser_bos(struct amdgpu_device *adev,

> > +				struct amdgpu_cs_parser *p, union

> drm_amdgpu_cs *cs)

> 

> No need for this change, adev should be available as p->adev as well.

> 

> >   {

> >   	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;

> >   	struct amdgpu_bo_list_entry *e;

> >   	struct list_head duplicates;

> >   	unsigned i, tries = 10;

> >   	int r;

> > +	struct amdgpu_bo *gds;

> > +	struct amdgpu_bo *gws;

> > +	struct amdgpu_bo *oa;

> 

> Reverse xmas tree order please, e.g. "int r" should be the last one.

> 

> Apart from that it looks good to me,

> Christian.

> 

> >

> >   	INIT_LIST_HEAD(&p->validated);

> >

> > @@ -652,10 +655,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;

> >   		struct amdgpu_vm *vm = &fpriv->vm;

> >   		unsigned i;

> >

> > @@ -664,19 +668,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 = adev->gds.gds_gfx_bo;

> > +		gws = adev->gds.gws_gfx_bo;

> > +		oa = 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) {

> > @@ -1233,7 +1241,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev,

> void *data, struct drm_file *filp)

> >   	if (r)

> >   		goto out;

> >

> > -	r = amdgpu_cs_parser_bos(&parser, data);

> > +	r = amdgpu_cs_parser_bos(adev, &parser, data);

> >   	if (r) {

> >   		if (r == -ENOMEM)

> >   			DRM_ERROR("Not enough memory for command

> submission!\n");