* [PATCH] drm/amdgpu: move PT validation back into VM code v2
@ 2016-10-04 12:21 Christian König
[not found] ` <1475583718-16860-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
0 siblings, 1 reply; 2+ messages in thread
From: Christian König @ 2016-10-04 12:21 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Christian König <christian.koenig@amd.com>
Saves a bunch of CPU cycles when swapping things back in and
allows us to split the VM headers into a separate file.
v2: rename parameters
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 42 ++++++++++++++++++++++-----------
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 27 ++++++++++++---------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 ++++++++++-------
4 files changed, 60 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0b54b5b..3bcf538 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -917,8 +917,9 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
struct list_head *validated,
struct amdgpu_bo_list_entry *entry);
-void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
- struct list_head *duplicates);
+int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+ int (*callback)(void *p, struct amdgpu_bo *bo),
+ void *param);
void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
struct amdgpu_vm *vm);
int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 504ae09..a13e551 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -388,9 +388,9 @@ retry:
/* Last resort, try to evict something from the current working set */
static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
- struct amdgpu_bo_list_entry *lobj)
+ struct amdgpu_bo *validated)
{
- uint32_t domain = lobj->robj->allowed_domains;
+ uint32_t domain = validated->allowed_domains;
int r;
if (!p->evictable)
@@ -406,7 +406,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
uint32_t other;
/* If we reached our current BO we can forget it */
- if (candidate == lobj)
+ if (candidate->robj == validated)
break;
other = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
@@ -439,6 +439,23 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
return false;
}
+static int amdgpu_cs_validate(void *param, struct amdgpu_bo *bo)
+{
+ struct amdgpu_cs_parser *p = param;
+ int r;
+
+ do {
+ r = amdgpu_cs_bo_validate(p, bo);
+ } while (r == -ENOMEM && amdgpu_cs_try_evict(p, bo));
+ if (r)
+ return r;
+
+ if (bo->shadow)
+ r = amdgpu_cs_bo_validate(p, bo);
+
+ return r;
+}
+
static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
struct list_head *validated)
{
@@ -466,18 +483,10 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
if (p->evictable == lobj)
p->evictable = NULL;
- do {
- r = amdgpu_cs_bo_validate(p, bo);
- } while (r == -ENOMEM && amdgpu_cs_try_evict(p, lobj));
+ r = amdgpu_cs_validate(p, bo);
if (r)
return r;
- if (bo->shadow) {
- r = amdgpu_cs_bo_validate(p, bo);
- if (r)
- return r;
- }
-
if (binding_userptr) {
drm_free_large(lobj->user_pages);
lobj->user_pages = NULL;
@@ -595,14 +604,19 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
list_splice(&need_pages, &p->validated);
}
- amdgpu_vm_get_pt_bos(p->adev, &fpriv->vm, &duplicates);
-
p->bytes_moved_threshold = amdgpu_cs_get_threshold_for_moves(p->adev);
p->bytes_moved = 0;
p->evictable = list_last_entry(&p->validated,
struct amdgpu_bo_list_entry,
tv.head);
+ r = amdgpu_vm_validate_pt_bos(p->adev, &fpriv->vm,
+ amdgpu_cs_validate, p);
+ if (r) {
+ DRM_ERROR("amdgpu_vm_validate_pt_bos() failed.\n");
+ goto error_validate;
+ }
+
r = amdgpu_cs_list_validate(p, &duplicates);
if (r) {
DRM_ERROR("amdgpu_cs_list_validate(duplicates) failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index c14b853..a4f5733 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -471,6 +471,16 @@ out:
return r;
}
+static int amdgpu_gem_va_check(void *param, struct amdgpu_bo *bo)
+{
+ unsigned domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
+
+ /* if anything is swapped out don't swap it in here,
+ just abort and wait for the next CS */
+
+ return domain == AMDGPU_GEM_DOMAIN_CPU ? -ERESTARTSYS : 0;
+}
+
/**
* amdgpu_gem_va_update_vm -update the bo_va in its VM
*
@@ -481,7 +491,8 @@ out:
* vital here, so they are not reported back to userspace.
*/
static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
- struct amdgpu_bo_va *bo_va, uint32_t operation)
+ struct amdgpu_bo_va *bo_va,
+ uint32_t operation)
{
struct ttm_validate_buffer tv, *entry;
struct amdgpu_bo_list_entry vm_pd;
@@ -504,7 +515,6 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
if (r)
goto error_print;
- amdgpu_vm_get_pt_bos(adev, bo_va->vm, &duplicates);
list_for_each_entry(entry, &list, head) {
domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type);
/* if anything is swapped out don't swap it in here,
@@ -512,13 +522,10 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
if (domain == AMDGPU_GEM_DOMAIN_CPU)
goto error_unreserve;
}
- list_for_each_entry(entry, &duplicates, head) {
- domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type);
- /* if anything is swapped out don't swap it in here,
- just abort and wait for the next CS */
- if (domain == AMDGPU_GEM_DOMAIN_CPU)
- goto error_unreserve;
- }
+ r = amdgpu_vm_validate_pt_bos(adev, bo_va->vm, amdgpu_gem_va_check,
+ NULL);
+ if (r)
+ goto error_unreserve;
r = amdgpu_vm_update_page_directory(adev, bo_va->vm);
if (r)
@@ -539,8 +546,6 @@ error_print:
DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
}
-
-
int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f4b78b6..c171b16 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -116,27 +116,29 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
}
/**
- * amdgpu_vm_get_bos - add the vm BOs to a duplicates list
+ * amdgpu_vm_validate_pt_bos - validate the page table BOs
*
* @adev: amdgpu device pointer
* @vm: vm providing the BOs
- * @duplicates: head of duplicates list
+ * @validate: callback to do the validation
+ * @param: parameter for the validation callback
*
- * Add the page directory to the BO duplicates list
- * for command submission.
+ * Validate the page table BOs on command submission if neccessary.
*/
-void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
- struct list_head *duplicates)
+int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+ int (*validate)(void *p, struct amdgpu_bo *bo),
+ void *param)
{
uint64_t num_evictions;
unsigned i;
+ int r;
/* We only need to validate the page tables
* if they aren't already valid.
*/
num_evictions = atomic64_read(&adev->num_evictions);
if (num_evictions == vm->last_eviction_counter)
- return;
+ return 0;
/* add the vm page table to the list */
for (i = 0; i <= vm->max_pde_used; ++i) {
@@ -145,9 +147,12 @@ void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
if (!entry->robj)
continue;
- list_add(&entry->tv.head, duplicates);
+ r = validate(param, entry->robj);
+ if (r)
+ return r;
}
+ return 0;
}
/**
--
2.5.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amdgpu: move PT validation back into VM code v2
[not found] ` <1475583718-16860-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2016-10-04 14:35 ` Alex Deucher
0 siblings, 0 replies; 2+ messages in thread
From: Alex Deucher @ 2016-10-04 14:35 UTC (permalink / raw)
To: Christian König; +Cc: amd-gfx list
On Tue, Oct 4, 2016 at 8:21 AM, Christian König <deathsimple@vodafone.de> wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Saves a bunch of CPU cycles when swapping things back in and
> allows us to split the VM headers into a separate file.
>
> v2: rename parameters
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 42 ++++++++++++++++++++++-----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 27 ++++++++++++---------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 21 ++++++++++-------
> 4 files changed, 60 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 0b54b5b..3bcf538 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -917,8 +917,9 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
> struct list_head *validated,
> struct amdgpu_bo_list_entry *entry);
> -void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> - struct list_head *duplicates);
> +int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + int (*callback)(void *p, struct amdgpu_bo *bo),
> + void *param);
> void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
> struct amdgpu_vm *vm);
> int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 504ae09..a13e551 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -388,9 +388,9 @@ retry:
>
> /* Last resort, try to evict something from the current working set */
> static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
> - struct amdgpu_bo_list_entry *lobj)
> + struct amdgpu_bo *validated)
> {
> - uint32_t domain = lobj->robj->allowed_domains;
> + uint32_t domain = validated->allowed_domains;
> int r;
>
> if (!p->evictable)
> @@ -406,7 +406,7 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
> uint32_t other;
>
> /* If we reached our current BO we can forget it */
> - if (candidate == lobj)
> + if (candidate->robj == validated)
> break;
>
> other = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
> @@ -439,6 +439,23 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
> return false;
> }
>
> +static int amdgpu_cs_validate(void *param, struct amdgpu_bo *bo)
> +{
> + struct amdgpu_cs_parser *p = param;
> + int r;
> +
> + do {
> + r = amdgpu_cs_bo_validate(p, bo);
> + } while (r == -ENOMEM && amdgpu_cs_try_evict(p, bo));
> + if (r)
> + return r;
> +
> + if (bo->shadow)
> + r = amdgpu_cs_bo_validate(p, bo);
> +
> + return r;
> +}
> +
> static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
> struct list_head *validated)
> {
> @@ -466,18 +483,10 @@ static int amdgpu_cs_list_validate(struct amdgpu_cs_parser *p,
> if (p->evictable == lobj)
> p->evictable = NULL;
>
> - do {
> - r = amdgpu_cs_bo_validate(p, bo);
> - } while (r == -ENOMEM && amdgpu_cs_try_evict(p, lobj));
> + r = amdgpu_cs_validate(p, bo);
> if (r)
> return r;
>
> - if (bo->shadow) {
> - r = amdgpu_cs_bo_validate(p, bo);
> - if (r)
> - return r;
> - }
> -
> if (binding_userptr) {
> drm_free_large(lobj->user_pages);
> lobj->user_pages = NULL;
> @@ -595,14 +604,19 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
> list_splice(&need_pages, &p->validated);
> }
>
> - amdgpu_vm_get_pt_bos(p->adev, &fpriv->vm, &duplicates);
> -
> p->bytes_moved_threshold = amdgpu_cs_get_threshold_for_moves(p->adev);
> p->bytes_moved = 0;
> p->evictable = list_last_entry(&p->validated,
> struct amdgpu_bo_list_entry,
> tv.head);
>
> + r = amdgpu_vm_validate_pt_bos(p->adev, &fpriv->vm,
> + amdgpu_cs_validate, p);
> + if (r) {
> + DRM_ERROR("amdgpu_vm_validate_pt_bos() failed.\n");
> + goto error_validate;
> + }
> +
> r = amdgpu_cs_list_validate(p, &duplicates);
> if (r) {
> DRM_ERROR("amdgpu_cs_list_validate(duplicates) failed.\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index c14b853..a4f5733 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -471,6 +471,16 @@ out:
> return r;
> }
>
> +static int amdgpu_gem_va_check(void *param, struct amdgpu_bo *bo)
> +{
> + unsigned domain = amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type);
> +
> + /* if anything is swapped out don't swap it in here,
> + just abort and wait for the next CS */
> +
> + return domain == AMDGPU_GEM_DOMAIN_CPU ? -ERESTARTSYS : 0;
> +}
> +
> /**
> * amdgpu_gem_va_update_vm -update the bo_va in its VM
> *
> @@ -481,7 +491,8 @@ out:
> * vital here, so they are not reported back to userspace.
> */
> static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
> - struct amdgpu_bo_va *bo_va, uint32_t operation)
> + struct amdgpu_bo_va *bo_va,
> + uint32_t operation)
> {
> struct ttm_validate_buffer tv, *entry;
> struct amdgpu_bo_list_entry vm_pd;
> @@ -504,7 +515,6 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
> if (r)
> goto error_print;
>
> - amdgpu_vm_get_pt_bos(adev, bo_va->vm, &duplicates);
> list_for_each_entry(entry, &list, head) {
> domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type);
> /* if anything is swapped out don't swap it in here,
> @@ -512,13 +522,10 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
> if (domain == AMDGPU_GEM_DOMAIN_CPU)
> goto error_unreserve;
> }
> - list_for_each_entry(entry, &duplicates, head) {
> - domain = amdgpu_mem_type_to_domain(entry->bo->mem.mem_type);
> - /* if anything is swapped out don't swap it in here,
> - just abort and wait for the next CS */
> - if (domain == AMDGPU_GEM_DOMAIN_CPU)
> - goto error_unreserve;
> - }
> + r = amdgpu_vm_validate_pt_bos(adev, bo_va->vm, amdgpu_gem_va_check,
> + NULL);
> + if (r)
> + goto error_unreserve;
>
> r = amdgpu_vm_update_page_directory(adev, bo_va->vm);
> if (r)
> @@ -539,8 +546,6 @@ error_print:
> DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
> }
>
> -
> -
> int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f4b78b6..c171b16 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -116,27 +116,29 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
> }
>
> /**
> - * amdgpu_vm_get_bos - add the vm BOs to a duplicates list
> + * amdgpu_vm_validate_pt_bos - validate the page table BOs
> *
> * @adev: amdgpu device pointer
> * @vm: vm providing the BOs
> - * @duplicates: head of duplicates list
> + * @validate: callback to do the validation
> + * @param: parameter for the validation callback
> *
> - * Add the page directory to the BO duplicates list
> - * for command submission.
> + * Validate the page table BOs on command submission if neccessary.
> */
> -void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> - struct list_head *duplicates)
> +int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> + int (*validate)(void *p, struct amdgpu_bo *bo),
> + void *param)
> {
> uint64_t num_evictions;
> unsigned i;
> + int r;
>
> /* We only need to validate the page tables
> * if they aren't already valid.
> */
> num_evictions = atomic64_read(&adev->num_evictions);
> if (num_evictions == vm->last_eviction_counter)
> - return;
> + return 0;
>
> /* add the vm page table to the list */
> for (i = 0; i <= vm->max_pde_used; ++i) {
> @@ -145,9 +147,12 @@ void amdgpu_vm_get_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> if (!entry->robj)
> continue;
>
> - list_add(&entry->tv.head, duplicates);
> + r = validate(param, entry->robj);
> + if (r)
> + return r;
> }
>
> + return 0;
> }
>
> /**
> --
> 2.5.0
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-10-04 14:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-04 12:21 [PATCH] drm/amdgpu: move PT validation back into VM code v2 Christian König
[not found] ` <1475583718-16860-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-10-04 14:35 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.