All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/amdgpu: track if a PD needs an update
@ 2017-07-13 13:25 Christian König
       [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-07-13 13:25 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

That should reduce our command submission overhead quite a bit.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 55d1c7f..3f5863e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -173,6 +173,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
 	if (!parent->entries)
 		return 0;
 
+	parent->needs_update = true;
 	for (i = 0; i <= parent->last_entry_used; ++i) {
 		struct amdgpu_vm_pt *entry = &parent->entries[i];
 
@@ -351,6 +352,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 
 			entry->bo = pt;
 			entry->addr = 0;
+			parent->needs_update = true;
 		}
 
 		if (level < adev->vm_manager.num_level) {
@@ -361,6 +363,8 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
 						   sub_eaddr, level);
 			if (r)
 				return r;
+
+			parent->needs_update |= entry->needs_update;
 		}
 	}
 
@@ -1050,7 +1054,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
 
 	int r;
 
-	if (!parent->entries)
+	if (!parent->entries || !parent->needs_update)
 		return 0;
 
 	memset(&params, 0, sizeof(params));
@@ -1196,6 +1200,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
 			return r;
 	}
 
+	parent->needs_update = false;
 	return 0;
 
 error_free:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 3441ec5..c3dfd3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -102,6 +102,7 @@ struct amdgpu_vm_pt {
 	/* array of page tables, one for each directory entry */
 	struct amdgpu_vm_pt	*entries;
 	unsigned		last_entry_used;
+	bool			needs_update;
 };
 
 struct amdgpu_vm {
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] drm/amdgpu: only move VM BOs in the LRU during validation
       [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-07-13 13:25   ` Christian König
       [not found]     ` <1499952313-2398-2-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-07-13 13:25   ` [PATCH 3/3] drm/amdgpu: only bind VM shadows after validation Christian König
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-07-13 13:25 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

This should save us a bunch of command submission overhead.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 15 +++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 ++++++----------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 --
 3 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 44ec11d..652425c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -669,10 +669,8 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	}
 
 error_validate:
-	if (r) {
-		amdgpu_vm_move_pt_bos_in_lru(p->adev, &fpriv->vm);
+	if (r)
 		ttm_eu_backoff_reservation(&p->ticket, &p->validated);
-	}
 
 error_free_pages:
 
@@ -720,21 +718,18 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
  * If error is set than unvalidate buffer, otherwise just free memory
  * used by parsing context.
  **/
-static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, bool backoff)
+static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
+				  bool backoff)
 {
-	struct amdgpu_fpriv *fpriv = parser->filp->driver_priv;
 	unsigned i;
 
-	if (!error) {
-		amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
-
+	if (!error)
 		ttm_eu_fence_buffer_objects(&parser->ticket,
 					    &parser->validated,
 					    parser->fence);
-	} else if (backoff) {
+	else if (backoff)
 		ttm_eu_backoff_reservation(&parser->ticket,
 					   &parser->validated);
-	}
 	dma_fence_put(parser->fence);
 
 	if (parser->ctx)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3f5863e..ea419ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -159,7 +159,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
  */
 static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
 				    int (*validate)(void *, struct amdgpu_bo *),
-				    void *param, bool use_cpu_for_update)
+				    void *param, bool use_cpu_for_update,
+				    struct ttm_bo_global *glob)
 {
 	unsigned i;
 	int r;
@@ -170,6 +171,10 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
 			return r;
 	}
 
+	spin_lock(&glob->lru_lock);
+	ttm_bo_move_to_lru_tail(&parent->bo->tbo);
+	spin_unlock(&glob->lru_lock);
+
 	if (!parent->entries)
 		return 0;
 
@@ -189,7 +194,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
 		 * have only a maximum of 5 layers.
 		 */
 		r = amdgpu_vm_validate_level(entry, validate, param,
-					     use_cpu_for_update);
+					     use_cpu_for_update, glob);
 		if (r)
 			return r;
 	}
@@ -221,54 +226,11 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		return 0;
 
 	return amdgpu_vm_validate_level(&vm->root, validate, param,
-					vm->use_cpu_for_update);
+					vm->use_cpu_for_update,
+					adev->mman.bdev.glob);
 }
 
 /**
- * amdgpu_vm_move_level_in_lru - move one level of PT BOs to the LRU tail
- *
- * @adev: amdgpu device instance
- * @vm: vm providing the BOs
- *
- * Move the PT BOs to the tail of the LRU.
- */
-static void amdgpu_vm_move_level_in_lru(struct amdgpu_vm_pt *parent)
-{
-	unsigned i;
-
-	if (!parent->entries)
-		return;
-
-	for (i = 0; i <= parent->last_entry_used; ++i) {
-		struct amdgpu_vm_pt *entry = &parent->entries[i];
-
-		if (!entry->bo)
-			continue;
-
-		ttm_bo_move_to_lru_tail(&entry->bo->tbo);
-		amdgpu_vm_move_level_in_lru(entry);
-	}
-}
-
-/**
- * amdgpu_vm_move_pt_bos_in_lru - move the PT BOs to the LRU tail
- *
- * @adev: amdgpu device instance
- * @vm: vm providing the BOs
- *
- * Move the PT BOs to the tail of the LRU.
- */
-void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
-				  struct amdgpu_vm *vm)
-{
-	struct ttm_bo_global *glob = adev->mman.bdev.glob;
-
-	spin_lock(&glob->lru_lock);
-	amdgpu_vm_move_level_in_lru(&vm->root);
-	spin_unlock(&glob->lru_lock);
-}
-
- /**
  * amdgpu_vm_alloc_levels - allocate the PD/PT levels
  *
  * @adev: amdgpu_device pointer
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index c3dfd3e..46e030c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -215,8 +215,6 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
 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_alloc_pts(struct amdgpu_device *adev,
 			struct amdgpu_vm *vm,
 			uint64_t saddr, uint64_t size);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] drm/amdgpu: only bind VM shadows after validation
       [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-07-13 13:25   ` [PATCH 2/3] drm/amdgpu: only move VM BOs in the LRU during validation Christian König
@ 2017-07-13 13:25   ` Christian König
       [not found]     ` <1499952313-2398-3-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-07-13 15:57   ` [PATCH 1/3] drm/amdgpu: track if a PD needs an update Felix Kuehling
  2017-07-14  2:13   ` zhoucm1
  3 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2017-07-13 13:25 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

From: Christian König <christian.koenig@amd.com>

No need to do this on every CS.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ea419ec..e1641f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -171,6 +171,18 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
 			return r;
 	}
 
+	if (parent->bo->shadow) {
+		struct amdgpu_bo *shadow = parent->bo->shadow;
+
+		r = amdgpu_ttm_bind(&shadow->tbo, &shadow->tbo.mem);
+		if (r)
+			return r;
+
+		spin_lock(&glob->lru_lock);
+		ttm_bo_move_to_lru_tail(&shadow->tbo);
+		spin_unlock(&glob->lru_lock);
+	}
+
 	spin_lock(&glob->lru_lock);
 	ttm_bo_move_to_lru_tail(&parent->bo->tbo);
 	spin_unlock(&glob->lru_lock);
@@ -1031,11 +1043,6 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
 
 		params.func = amdgpu_vm_cpu_set_ptes;
 	} else {
-		if (shadow) {
-			r = amdgpu_ttm_bind(&shadow->tbo, &shadow->tbo.mem);
-			if (r)
-				return r;
-		}
 		ring = container_of(vm->entity.sched, struct amdgpu_ring,
 				    sched);
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] drm/amdgpu: only bind VM shadows after validation
       [not found]     ` <1499952313-2398-3-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-07-13 15:56       ` Felix Kuehling
  0 siblings, 0 replies; 7+ messages in thread
From: Felix Kuehling @ 2017-07-13 15:56 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


On 17-07-13 09:25 AM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> No need to do this on every CS.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index ea419ec..e1641f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -171,6 +171,18 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>  			return r;
>  	}
>  
> +	if (parent->bo->shadow) {
> +		struct amdgpu_bo *shadow = parent->bo->shadow;
> +
> +		r = amdgpu_ttm_bind(&shadow->tbo, &shadow->tbo.mem);
> +		if (r)
> +			return r;
> +
> +		spin_lock(&glob->lru_lock);
> +		ttm_bo_move_to_lru_tail(&shadow->tbo);
> +		spin_unlock(&glob->lru_lock);
> +	}
> +
>  	spin_lock(&glob->lru_lock);
>  	ttm_bo_move_to_lru_tail(&parent->bo->tbo);
>  	spin_unlock(&glob->lru_lock);
> @@ -1031,11 +1043,6 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>  
>  		params.func = amdgpu_vm_cpu_set_ptes;
>  	} else {
> -		if (shadow) {
> -			r = amdgpu_ttm_bind(&shadow->tbo, &shadow->tbo.mem);
> -			if (r)
> -				return r;
> -		}

There is another bind of shadow BOs further down in the loop that does
the update.

Regards,
  Felix

>  		ring = container_of(vm->entity.sched, struct amdgpu_ring,
>  				    sched);
>  

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] drm/amdgpu: track if a PD needs an update
       [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2017-07-13 13:25   ` [PATCH 2/3] drm/amdgpu: only move VM BOs in the LRU during validation Christian König
  2017-07-13 13:25   ` [PATCH 3/3] drm/amdgpu: only bind VM shadows after validation Christian König
@ 2017-07-13 15:57   ` Felix Kuehling
  2017-07-14  2:13   ` zhoucm1
  3 siblings, 0 replies; 7+ messages in thread
From: Felix Kuehling @ 2017-07-13 15:57 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


On 17-07-13 09:25 AM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> That should reduce our command submission overhead quite a bit.
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 55d1c7f..3f5863e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -173,6 +173,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>  	if (!parent->entries)
>  		return 0;
>  
> +	parent->needs_update = true;

This means that after amdgpu_vm_validate_pt_bos, all vm_pts will have
needs_update set, even if the last eviction didn't evict any page
tables. That will create unnecessary overhead in amdgpu_vm_update_level.
You could probably add another check in the loop below for whether
validate actually moved any child page tables.

Regards,
  Felix

>  	for (i = 0; i <= parent->last_entry_used; ++i) {
>  		struct amdgpu_vm_pt *entry = &parent->entries[i];
>  
> @@ -351,6 +352,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>  
>  			entry->bo = pt;
>  			entry->addr = 0;
> +			parent->needs_update = true;
>  		}
>  
>  		if (level < adev->vm_manager.num_level) {
> @@ -361,6 +363,8 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>  						   sub_eaddr, level);
>  			if (r)
>  				return r;
> +
> +			parent->needs_update |= entry->needs_update;
>  		}
>  	}
>  
> @@ -1050,7 +1054,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>  
>  	int r;
>  
> -	if (!parent->entries)
> +	if (!parent->entries || !parent->needs_update)
>  		return 0;
>  
>  	memset(&params, 0, sizeof(params));
> @@ -1196,6 +1200,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>  			return r;
>  	}
>  
> +	parent->needs_update = false;
>  	return 0;
>  
>  error_free:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 3441ec5..c3dfd3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -102,6 +102,7 @@ struct amdgpu_vm_pt {
>  	/* array of page tables, one for each directory entry */
>  	struct amdgpu_vm_pt	*entries;
>  	unsigned		last_entry_used;
> +	bool			needs_update;
>  };
>  
>  struct amdgpu_vm {

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] drm/amdgpu: only move VM BOs in the LRU during validation
       [not found]     ` <1499952313-2398-2-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2017-07-13 16:01       ` Felix Kuehling
  0 siblings, 0 replies; 7+ messages in thread
From: Felix Kuehling @ 2017-07-13 16:01 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


On 17-07-13 09:25 AM, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> This should save us a bunch of command submission overhead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 15 +++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 ++++++----------------------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 --
>  3 files changed, 14 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 44ec11d..652425c8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -669,10 +669,8 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  	}
>  
>  error_validate:
> -	if (r) {
> -		amdgpu_vm_move_pt_bos_in_lru(p->adev, &fpriv->vm);
> +	if (r)
>  		ttm_eu_backoff_reservation(&p->ticket, &p->validated);
> -	}
>  
>  error_free_pages:
>  
> @@ -720,21 +718,18 @@ static int amdgpu_cs_sync_rings(struct amdgpu_cs_parser *p)
>   * If error is set than unvalidate buffer, otherwise just free memory
>   * used by parsing context.
>   **/
> -static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, bool backoff)
> +static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
> +				  bool backoff)
>  {
> -	struct amdgpu_fpriv *fpriv = parser->filp->driver_priv;
>  	unsigned i;
>  
> -	if (!error) {
> -		amdgpu_vm_move_pt_bos_in_lru(parser->adev, &fpriv->vm);
> -
> +	if (!error)
>  		ttm_eu_fence_buffer_objects(&parser->ticket,
>  					    &parser->validated,
>  					    parser->fence);
> -	} else if (backoff) {
> +	else if (backoff)
>  		ttm_eu_backoff_reservation(&parser->ticket,
>  					   &parser->validated);
> -	}
>  	dma_fence_put(parser->fence);
>  
>  	if (parser->ctx)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 3f5863e..ea419ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -159,7 +159,8 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
>   */
>  static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>  				    int (*validate)(void *, struct amdgpu_bo *),
> -				    void *param, bool use_cpu_for_update)
> +				    void *param, bool use_cpu_for_update,
> +				    struct ttm_bo_global *glob)
>  {
>  	unsigned i;
>  	int r;
> @@ -170,6 +171,10 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>  			return r;
>  	}
>  
> +	spin_lock(&glob->lru_lock);
> +	ttm_bo_move_to_lru_tail(&parent->bo->tbo);
> +	spin_unlock(&glob->lru_lock);
> +
>  	if (!parent->entries)
>  		return 0;
>  
> @@ -189,7 +194,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>  		 * have only a maximum of 5 layers.
>  		 */
>  		r = amdgpu_vm_validate_level(entry, validate, param,
> -					     use_cpu_for_update);
> +					     use_cpu_for_update, glob);
>  		if (r)
>  			return r;
>  	}
> @@ -221,54 +226,11 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  		return 0;
>  
>  	return amdgpu_vm_validate_level(&vm->root, validate, param,
> -					vm->use_cpu_for_update);
> +					vm->use_cpu_for_update,
> +					adev->mman.bdev.glob);
>  }
>  
>  /**
> - * amdgpu_vm_move_level_in_lru - move one level of PT BOs to the LRU tail
> - *
> - * @adev: amdgpu device instance
> - * @vm: vm providing the BOs
> - *
> - * Move the PT BOs to the tail of the LRU.
> - */
> -static void amdgpu_vm_move_level_in_lru(struct amdgpu_vm_pt *parent)
> -{
> -	unsigned i;
> -
> -	if (!parent->entries)
> -		return;
> -
> -	for (i = 0; i <= parent->last_entry_used; ++i) {
> -		struct amdgpu_vm_pt *entry = &parent->entries[i];
> -
> -		if (!entry->bo)
> -			continue;
> -
> -		ttm_bo_move_to_lru_tail(&entry->bo->tbo);
> -		amdgpu_vm_move_level_in_lru(entry);
> -	}
> -}
> -
> -/**
> - * amdgpu_vm_move_pt_bos_in_lru - move the PT BOs to the LRU tail
> - *
> - * @adev: amdgpu device instance
> - * @vm: vm providing the BOs
> - *
> - * Move the PT BOs to the tail of the LRU.
> - */
> -void amdgpu_vm_move_pt_bos_in_lru(struct amdgpu_device *adev,
> -				  struct amdgpu_vm *vm)
> -{
> -	struct ttm_bo_global *glob = adev->mman.bdev.glob;
> -
> -	spin_lock(&glob->lru_lock);
> -	amdgpu_vm_move_level_in_lru(&vm->root);
> -	spin_unlock(&glob->lru_lock);
> -}
> -
> - /**
>   * amdgpu_vm_alloc_levels - allocate the PD/PT levels
>   *
>   * @adev: amdgpu_device pointer
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index c3dfd3e..46e030c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -215,8 +215,6 @@ void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm,
>  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_alloc_pts(struct amdgpu_device *adev,
>  			struct amdgpu_vm *vm,
>  			uint64_t saddr, uint64_t size);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] drm/amdgpu: track if a PD needs an update
       [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
                     ` (2 preceding siblings ...)
  2017-07-13 15:57   ` [PATCH 1/3] drm/amdgpu: track if a PD needs an update Felix Kuehling
@ 2017-07-14  2:13   ` zhoucm1
  3 siblings, 0 replies; 7+ messages in thread
From: zhoucm1 @ 2017-07-14  2:13 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

'if (parent->entries[pt_idx].addr == pt)' is good enough for condition 
of need_update, adding previous Felix's optimization patch, then the 
whole picture looks better than this one, IIRC.

Regards,
David Zhou
On 2017年07月13日 21:25, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> That should reduce our command submission overhead quite a bit.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 55d1c7f..3f5863e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -173,6 +173,7 @@ static int amdgpu_vm_validate_level(struct amdgpu_vm_pt *parent,
>   	if (!parent->entries)
>   		return 0;
>   
> +	parent->needs_update = true;
>   	for (i = 0; i <= parent->last_entry_used; ++i) {
>   		struct amdgpu_vm_pt *entry = &parent->entries[i];
>   
> @@ -351,6 +352,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   
>   			entry->bo = pt;
>   			entry->addr = 0;
> +			parent->needs_update = true;
>   		}
>   
>   		if (level < adev->vm_manager.num_level) {
> @@ -361,6 +363,8 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device *adev,
>   						   sub_eaddr, level);
>   			if (r)
>   				return r;
> +
> +			parent->needs_update |= entry->needs_update;
>   		}
>   	}
>   
> @@ -1050,7 +1054,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>   
>   	int r;
>   
> -	if (!parent->entries)
> +	if (!parent->entries || !parent->needs_update)
>   		return 0;
>   
>   	memset(&params, 0, sizeof(params));
> @@ -1196,6 +1200,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device *adev,
>   			return r;
>   	}
>   
> +	parent->needs_update = false;
>   	return 0;
>   
>   error_free:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 3441ec5..c3dfd3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -102,6 +102,7 @@ struct amdgpu_vm_pt {
>   	/* array of page tables, one for each directory entry */
>   	struct amdgpu_vm_pt	*entries;
>   	unsigned		last_entry_used;
> +	bool			needs_update;
>   };
>   
>   struct amdgpu_vm {

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-07-14  2:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-13 13:25 [PATCH 1/3] drm/amdgpu: track if a PD needs an update Christian König
     [not found] ` <1499952313-2398-1-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-13 13:25   ` [PATCH 2/3] drm/amdgpu: only move VM BOs in the LRU during validation Christian König
     [not found]     ` <1499952313-2398-2-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-13 16:01       ` Felix Kuehling
2017-07-13 13:25   ` [PATCH 3/3] drm/amdgpu: only bind VM shadows after validation Christian König
     [not found]     ` <1499952313-2398-3-git-send-email-deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-13 15:56       ` Felix Kuehling
2017-07-13 15:57   ` [PATCH 1/3] drm/amdgpu: track if a PD needs an update Felix Kuehling
2017-07-14  2:13   ` zhoucm1

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.