Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
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
---
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 = >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,
>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,
+ >bo_list);
+ if (r)
+ return r;
}
- if (p->bo_list) {
- amdgpu_bo_list_get_list(p->bo_list, >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, >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();
amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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,
+
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
> >>>
> >>> 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.
Thanks,
Ray
> Regards,
> Christian.
>
> >
> > Thanks,
> > Ray
> >
> >> Christian.
> >>
> >>> Thanks,
> >>> Ray
> >>>
> Signed-off-by: Christian König
> ---
> 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 = >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,
> >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,
> + >bo_list);
> +if (r)
> +return r;
> }
> -if (p->bo_list) {
> -amdgpu_bo_list_get_list(p->bo_list, >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, >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();
> amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
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.
Regards,
Christian.
Thanks,
Ray
Christian.
Thanks,
Ray
Signed-off-by: Christian König
---
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 = >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,
>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,
+ >bo_list);
+ if (r)
+ return r;
}
- if (p->bo_list) {
- amdgpu_bo_list_get_list(p->bo_list, >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, >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();
amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
On 07/31/2018 04:52 PM, 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
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 will not use bo list either?
any perform consideration?
Regards,
Jerry
Christian.
Thanks,
Ray
Signed-off-by: Christian König
---
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 = >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,
>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,
+ >bo_list);
+if (r)
+return r;
}
-if (p->bo_list) {
-amdgpu_bo_list_get_list(p->bo_list, >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, >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();
amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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 = >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
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
> >
> >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?
Thanks,
Ray
> Christian.
>
> >
> >Thanks,
> >Ray
> >
> >>Signed-off-by: Christian König
> >>---
> >> 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 = >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,
> >> >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,
> >>+ >bo_list);
> >>+ if (r)
> >>+ return r;
> >>}
> >>- if (p->bo_list) {
> >>- amdgpu_bo_list_get_list(p->bo_list, >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, >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();
> >>amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
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...
Christian.
Thanks,
Ray
Signed-off-by: Christian König
---
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 = >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,
>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,
+ >bo_list);
+ if (r)
+ return r;
}
- if (p->bo_list) {
- amdgpu_bo_list_get_list(p->bo_list, >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, >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();
amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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 = >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;
}
Re: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
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
In which case, when the command is being submitted, there is no bo list
handle? All BOs are per-VM, no shared BO?
Thanks,
Ray
>
> Signed-off-by: Christian König
> ---
> 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 = >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,
> >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,
> + >bo_list);
> + if (r)
> + return r;
> }
>
> - if (p->bo_list) {
> - amdgpu_bo_list_get_list(p->bo_list, >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, >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();
> amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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 = >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)
>
RE: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is provided
Series is Reviewed-by: Chunming Zhou
-Original Message-
From: amd-gfx On Behalf Of Christian
K?nig
Sent: Monday, July 30, 2018 10:52 PM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH 9/9] drm/amdgpu: create an empty bo_list if no handle is
provided
Instead of having extra handling just create an empty bo_list when no handle is
provided.
Signed-off-by: Christian König
---
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 = >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,
>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,
+ >bo_list);
+ if (r)
+ return r;
}
- if (p->bo_list) {
- amdgpu_bo_list_get_list(p->bo_list, >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, >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();
amdgpu_vm_get_pd_bo(>vm, >validated, >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(_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 = >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 = >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;
}
-
