[PATCH 1/3] drm/amdgpu: make all per vm BOs list

[PATCH 1/3] drm/amdgpu: make all per vm BOs list

[Chunming Zhou]

the list decides the lru order.

Change-Id: I8baf85aefd5781501599ff672949a9b71099a30e
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e9a41dd05345..5e35e23511cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1866,6 +1866,7 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	}
 	bo_va->base.vm = vm;
 	bo_va->base.bo = bo;
+	INIT_LIST_HEAD(&bo_va->base.vm_bo);
 	INIT_LIST_HEAD(&bo_va->base.bo_list);
 	INIT_LIST_HEAD(&bo_va->base.vm_status);
 
@@ -1881,6 +1882,10 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
 		return bo_va;
 
+	spin_lock(&vm->status_lock);
+	list_add_tail(&bo_va->base.vm_bo, &vm->vm_bo_list);
+	spin_unlock(&vm->status_lock);
+
 	if (bo->preferred_domains &
 	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
 		return bo_va;
@@ -2237,6 +2242,7 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
 
 	spin_lock(&vm->status_lock);
 	list_del(&bo_va->base.vm_status);
+	list_del(&bo_va->base.vm_bo);
 	spin_unlock(&vm->status_lock);
 
 	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
@@ -2409,6 +2415,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
 		vm->reserved_vmid[i] = NULL;
 	spin_lock_init(&vm->status_lock);
+	INIT_LIST_HEAD(&vm->vm_bo_list);
 	INIT_LIST_HEAD(&vm->evicted);
 	INIT_LIST_HEAD(&vm->relocated);
 	INIT_LIST_HEAD(&vm->moved);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index cf2c667ee538..1886a561c84e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -144,6 +144,9 @@ struct amdgpu_vm_bo_base {
 	struct amdgpu_vm		*vm;
 	struct amdgpu_bo		*bo;
 
+	/* protected by vm status lock */
+	struct list_head		vm_bo;
+
 	/* protected by bo being reserved */
 	struct list_head		bo_list;
 
@@ -177,6 +180,9 @@ struct amdgpu_vm {
 	/* protecting invalidated */
 	spinlock_t		status_lock;
 
+	/* protected by status lock */
+	struct list_head	vm_bo_list;
+
 	/* BOs who needs a validation */
 	struct list_head	evicted;
 
-- 
2.14.1

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

[PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Chunming Zhou]

Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 383bf2d31c92..414e61799236 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
 		}
 	}
 
+	amdgpu_vm_refresh_lru(adev, vm);
+
 	return r;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 5e35e23511cf..8ad2bb705765 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1902,6 +1902,21 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
 	return bo_va;
 }
 
+void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm)
+{
+	struct ttm_bo_global *glob = adev->mman.bdev.glob;
+	struct amdgpu_vm_bo_base *bo_base;
+
+	spin_lock(&vm->status_lock);
+	list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
+		spin_lock(&glob->lru_lock);
+		ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
+		if (bo_base->bo->shadow)
+			ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
+		spin_unlock(&glob->lru_lock);
+	}
+	spin_unlock(&vm->status_lock);
+}
 
 /**
  * amdgpu_vm_bo_insert_mapping - insert a new mapping
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 1886a561c84e..e01895581489 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			  struct dma_fence **fence);
 int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
 			   struct amdgpu_vm *vm);
+void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			struct amdgpu_bo_va *bo_va,
 			bool clear);
-- 
2.14.1

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

[PATCH 3/3] drm/amdgpu: re-validate per VM BOs if required v2

[Chunming Zhou]

From: Christian König <ckoenig.leichtzumerken@gmail.com>

If a per VM BO ends up in a allowed domain it never moves back into the
prefered domain.
v2: move to refresh function, the validation order also keeps same as lru

Change-Id: Ifb3e561785d3b464da28c439b271c26825224c5e
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-and-Tested-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8ad2bb705765..f28a94f3d687 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1914,6 +1914,13 @@ void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 		if (bo_base->bo->shadow)
 			ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
 		spin_unlock(&glob->lru_lock);
+		/* If the BO prefers to be in VRAM, but currently isn't add it
+		 * back to the evicted list so that it gets validated again on
+		 * the next command submission.
+		 */
+		if (bo_base->bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM &&
+		    bo_base->bo->tbo.mem.mem_type != TTM_PL_VRAM)
+			list_move_tail(&bo_base->vm_status, &vm->evicted);
 	}
 	spin_unlock(&vm->status_lock);
 }
-- 
2.14.1

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Christian König]

NAK, we already tried that and it is really not a good idea because it 
massively increases the per submission overhead.

Christian.

Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
> Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>   3 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 383bf2d31c92..414e61799236 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>   		}
>   	}
>   
> +	amdgpu_vm_refresh_lru(adev, vm);
> +
>   	return r;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5e35e23511cf..8ad2bb705765 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	return bo_va;
>   }
>   
> +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +{
> +	struct ttm_bo_global *glob = adev->mman.bdev.glob;
> +	struct amdgpu_vm_bo_base *bo_base;
> +
> +	spin_lock(&vm->status_lock);
> +	list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
> +		spin_lock(&glob->lru_lock);
> +		ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
> +		if (bo_base->bo->shadow)
> +			ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
> +		spin_unlock(&glob->lru_lock);
> +	}
> +	spin_unlock(&vm->status_lock);
> +}
>   
>   /**
>    * amdgpu_vm_bo_insert_mapping - insert a new mapping
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 1886a561c84e..e01895581489 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>   			  struct dma_fence **fence);
>   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>   			   struct amdgpu_vm *vm);
> +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);

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

Re: [PATCH 1/3] drm/amdgpu: make all per vm BOs list

[Christian König]

NAK, that increases the memory footprint way to much and there is no 
description on what that should be good for.

Christian.

Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
> the list decides the lru order.
>
> Change-Id: I8baf85aefd5781501599ff672949a9b71099a30e
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 +++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 6 ++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index e9a41dd05345..5e35e23511cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1866,6 +1866,7 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	}
>   	bo_va->base.vm = vm;
>   	bo_va->base.bo = bo;
> +	INIT_LIST_HEAD(&bo_va->base.vm_bo);
>   	INIT_LIST_HEAD(&bo_va->base.bo_list);
>   	INIT_LIST_HEAD(&bo_va->base.vm_status);
>   
> @@ -1881,6 +1882,10 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>   	if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>   		return bo_va;
>   
> +	spin_lock(&vm->status_lock);
> +	list_add_tail(&bo_va->base.vm_bo, &vm->vm_bo_list);
> +	spin_unlock(&vm->status_lock);
> +
>   	if (bo->preferred_domains &
>   	    amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
>   		return bo_va;
> @@ -2237,6 +2242,7 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>   
>   	spin_lock(&vm->status_lock);
>   	list_del(&bo_va->base.vm_status);
> +	list_del(&bo_va->base.vm_bo);
>   	spin_unlock(&vm->status_lock);
>   
>   	list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
> @@ -2409,6 +2415,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
>   		vm->reserved_vmid[i] = NULL;
>   	spin_lock_init(&vm->status_lock);
> +	INIT_LIST_HEAD(&vm->vm_bo_list);
>   	INIT_LIST_HEAD(&vm->evicted);
>   	INIT_LIST_HEAD(&vm->relocated);
>   	INIT_LIST_HEAD(&vm->moved);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index cf2c667ee538..1886a561c84e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -144,6 +144,9 @@ struct amdgpu_vm_bo_base {
>   	struct amdgpu_vm		*vm;
>   	struct amdgpu_bo		*bo;
>   
> +	/* protected by vm status lock */
> +	struct list_head		vm_bo;
> +
>   	/* protected by bo being reserved */
>   	struct list_head		bo_list;
>   
> @@ -177,6 +180,9 @@ struct amdgpu_vm {
>   	/* protecting invalidated */
>   	spinlock_t		status_lock;
>   
> +	/* protected by status lock */
> +	struct list_head	vm_bo_list;
> +
>   	/* BOs who needs a validation */
>   	struct list_head	evicted;
>   

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Zhou, David(ChunMing)]

[-- Attachment #1.1: Type: text/plain, Size: 3208 bytes --]

then how to keep unique lru order? any ideas?

To stable performance, we have to keep unique lru order, otherwise like the issue I look into, sometimes F1game is 40fps, sometimes 28fps...even re-validate allowed domains BO.

The left root cause is the moved BOs are not same.


send from Smartisan Pro

Christian K鰊ig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 于 2018年3月27日 下午6:50写道：

NAK, we already tried that and it is really not a good idea because it
massively increases the per submission overhead.

Christian.

Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
> Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
> Signed-off-by: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>   3 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 383bf2d31c92..414e61799236 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
>                }
>        }
>
> +     amdgpu_vm_refresh_lru(adev, vm);
> +
>        return r;
>   }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5e35e23511cf..8ad2bb705765 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>        return bo_va;
>   }
>
> +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +{
> +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
> +     struct amdgpu_vm_bo_base *bo_base;
> +
> +     spin_lock(&vm->status_lock);
> +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
> +             spin_lock(&glob->lru_lock);
> +             ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
> +             if (bo_base->bo->shadow)
> +                     ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
> +             spin_unlock(&glob->lru_lock);
> +     }
> +     spin_unlock(&vm->status_lock);
> +}
>
>   /**
>    * amdgpu_vm_bo_insert_mapping - insert a new mapping
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 1886a561c84e..e01895581489 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>                          struct dma_fence **fence);
>   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>                           struct amdgpu_vm *vm);
> +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>                        struct amdgpu_bo_va *bo_va,
>                        bool clear);


[-- Attachment #1.2: Type: text/html, Size: 6559 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 3731 bytes --]

How about we update the LRU only when we need to re-validate at least 
one BO?

BTW: We can easily walk all BOs which belong to a VM, skipping over the 
few which aren't per VM BOs should be trivial.

Christian.

Am 27.03.2018 um 13:56 schrieb Zhou, David(ChunMing):
> then how to keep unique lru order? any ideas?
>
> To stable performance, we have to keep unique lru order, otherwise 
> like the issue I look into, sometimes F1game is 40fps, sometimes 
> 28fps...even re-validate allowed domains BO.
>
> The left root cause is the moved BOs are not same.
>
> send from Smartisan Pro
>
> Christian K鰊ig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 于 2018年3月27日 
> 下午6:50写道：
>
> NAK, we already tried that and it is really not a good idea because it
> massively increases the per submission overhead.
>
> Christian.
>
> Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
> > Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
> > Signed-off-by: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
> >   3 files changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 383bf2d31c92..414e61799236 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct 
> amdgpu_cs_parser *p)
> >                }
> >        }
> >
> > +     amdgpu_vm_refresh_lru(adev, vm);
> > +
> >        return r;
> >   }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 5e35e23511cf..8ad2bb705765 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
> >        return bo_va;
> >   }
> >
> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm)
> > +{
> > +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
> > +     struct amdgpu_vm_bo_base *bo_base;
> > +
> > +     spin_lock(&vm->status_lock);
> > +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
> > +             spin_lock(&glob->lru_lock);
> > + ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
> > +             if (bo_base->bo->shadow)
> > + ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
> > +             spin_unlock(&glob->lru_lock);
> > +     }
> > +     spin_unlock(&vm->status_lock);
> > +}
> >
> >   /**
> >    * amdgpu_vm_bo_insert_mapping - insert a new mapping
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > index 1886a561c84e..e01895581489 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device 
> *adev,
> >                          struct dma_fence **fence);
> >   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> >                           struct amdgpu_vm *vm);
> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
> amdgpu_vm *vm);
> >   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> >                        struct amdgpu_bo_va *bo_va,
> >                        bool clear);
>


[-- Attachment #1.2: Type: text/html, Size: 7144 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 4302 bytes --]



On 2018年03月27日 21:44, Christian König wrote:

> How about we update the LRU only when we need to re-validate at least 
> one BO?
I tried this just now, performance still isn't stable, sometime drop to 
28fps by accident.

I also tried to check num_evictions, if eviction happens, then update 
LRU, also sometime drop to 28fps by accident.

When BOs change, we not only need keep LRU order, but also validation 
order in vm->evicted list. Any other ideas which can keep these order 
but not increase submission overhead?

Regards,
David Zhou
>
> BTW: We can easily walk all BOs which belong to a VM, skipping over 
> the few which aren't per VM BOs should be trivial.
>
> Christian.
>
> Am 27.03.2018 um 13:56 schrieb Zhou, David(ChunMing):
>> then how to keep unique lru order? any ideas?
>>
>> To stable performance, we have to keep unique lru order, otherwise 
>> like the issue I look into, sometimes F1game is 40fps, sometimes 
>> 28fps...even re-validate allowed domains BO.
>>
>> The left root cause is the moved BOs are not same.
>>
>> send from Smartisan Pro
>>
>> Christian K鰊ig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 于 2018年3月27日 
>> 下午6:50写道：
>>
>> NAK, we already tried that and it is really not a good idea because it
>> massively increases the per submission overhead.
>>
>> Christian.
>>
>> Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
>> > Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
>> > Signed-off-by: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>
>> > ---
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>> >   3 files changed, 18 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > index 383bf2d31c92..414e61799236 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct 
>> amdgpu_cs_parser *p)
>> >                }
>> >        }
>> >
>> > +     amdgpu_vm_refresh_lru(adev, vm);
>> > +
>> >        return r;
>> >   }
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> > index 5e35e23511cf..8ad2bb705765 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> > @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
>> amdgpu_device *adev,
>> >        return bo_va;
>> >   }
>> >
>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>> amdgpu_vm *vm)
>> > +{
>> > +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
>> > +     struct amdgpu_vm_bo_base *bo_base;
>> > +
>> > +     spin_lock(&vm->status_lock);
>> > +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
>> > +             spin_lock(&glob->lru_lock);
>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
>> > +             if (bo_base->bo->shadow)
>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
>> > +             spin_unlock(&glob->lru_lock);
>> > +     }
>> > +     spin_unlock(&vm->status_lock);
>> > +}
>> >
>> >   /**
>> >    * amdgpu_vm_bo_insert_mapping - insert a new mapping
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> > index 1886a561c84e..e01895581489 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> > @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device 
>> *adev,
>> >                          struct dma_fence **fence);
>> >   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>> >                           struct amdgpu_vm *vm);
>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>> amdgpu_vm *vm);
>> >   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>> >                        struct amdgpu_bo_va *bo_va,
>> >                        bool clear);
>>
>


[-- Attachment #1.2: Type: text/html, Size: 8497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[zhoucm1]

[-- Attachment #1.1: Type: text/plain, Size: 5151 bytes --]



On 2018年03月28日 16:13, zhoucm1 wrote:
>
>
>
> On 2018年03月27日 21:44, Christian König wrote:
>
>> How about we update the LRU only when we need to re-validate at least 
>> one BO?
> I tried this just now, performance still isn't stable, sometime drop 
> to 28fps by accident.
>
> I also tried to check num_evictions, if eviction happens, then update 
> LRU, also sometime drop to 28fps by accident.
>
> When BOs change, we not only need keep LRU order, but also validation 
> order in vm->evicted list. Any other ideas which can keep these order 
> but not increase submission overhead?

With more thinking, we need to add new LRU design for per vm bo, we need 
to make sure the order when adding to LRU. How about the below idea:
0. separate traditional bo list lru and per-vm-bo lru. Traditional lru 
keeps old way, per-vm-lru follows below design.
1. TTM bdev maintains a vm/process list.
2. Every vm_list node contains its own per-vm-bo LRU[priority]
3. To manage the vm_list lru in specific driver, we will need add 
callback for it.
4. We will add an order for every per-vm-bo in that vm/process.
5. To speed up per-vm-lru sort, we will introduce RB tree for it in 
callback. The RB tree key is order.

This way, we will be able to keep the per-vm-bo LRU order.

What do you think of it?

Regards,
David Zhou
>
> Regards,
> David Zhou
>>
>> BTW: We can easily walk all BOs which belong to a VM, skipping over 
>> the few which aren't per VM BOs should be trivial.
>>
>> Christian.
>>
>> Am 27.03.2018 um 13:56 schrieb Zhou, David(ChunMing):
>>> then how to keep unique lru order? any ideas?
>>>
>>> To stable performance, we have to keep unique lru order, otherwise 
>>> like the issue I look into, sometimes F1game is 40fps, sometimes 
>>> 28fps...even re-validate allowed domains BO.
>>>
>>> The left root cause is the moved BOs are not same.
>>>
>>> send from Smartisan Pro
>>>
>>> Christian K鰊ig <ckoenig.leichtzumerken@gmail.com> 于 2018年3月27日 
>>> 下午6:50写道：
>>>
>>> NAK, we already tried that and it is really not a good idea because it
>>> massively increases the per submission overhead.
>>>
>>> Christian.
>>>
>>> Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
>>> > Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
>>> > Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>> > ---
>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>>> >   3 files changed, 18 insertions(+)
>>> >
>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> > index 383bf2d31c92..414e61799236 100644
>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> > @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct 
>>> amdgpu_cs_parser *p)
>>> >                }
>>> >        }
>>> >
>>> > +     amdgpu_vm_refresh_lru(adev, vm);
>>> > +
>>> >        return r;
>>> >   }
>>> >
>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> > index 5e35e23511cf..8ad2bb705765 100644
>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> > @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va 
>>> *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>> >        return bo_va;
>>> >   }
>>> >
>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>> amdgpu_vm *vm)
>>> > +{
>>> > +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
>>> > +     struct amdgpu_vm_bo_base *bo_base;
>>> > +
>>> > +     spin_lock(&vm->status_lock);
>>> > +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
>>> > +             spin_lock(&glob->lru_lock);
>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
>>> > +             if (bo_base->bo->shadow)
>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
>>> > +             spin_unlock(&glob->lru_lock);
>>> > +     }
>>> > +     spin_unlock(&vm->status_lock);
>>> > +}
>>> >
>>> >   /**
>>> >    * amdgpu_vm_bo_insert_mapping - insert a new mapping
>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> > index 1886a561c84e..e01895581489 100644
>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> > @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device 
>>> *adev,
>>> >                          struct dma_fence **fence);
>>> >   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>> >                           struct amdgpu_vm *vm);
>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>> amdgpu_vm *vm);
>>> >   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>> >                        struct amdgpu_bo_va *bo_va,
>>> >                        bool clear);
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 10013 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 5640 bytes --]

Am 29.03.2018 um 10:37 schrieb zhoucm1:
>
>
>
> On 2018年03月28日 16:13, zhoucm1 wrote:
>>
>>
>>
>> On 2018年03月27日 21:44, Christian König wrote:
>>
>>> How about we update the LRU only when we need to re-validate at 
>>> least one BO?
>> I tried this just now, performance still isn't stable, sometime drop 
>> to 28fps by accident.

Can you give me the code for that? I probably can't work this week on 
that, but I can take a look next week.

>>
>> I also tried to check num_evictions, if eviction happens, then update 
>> LRU, also sometime drop to 28fps by accident.
>>
>> When BOs change, we not only need keep LRU order, but also validation 
>> order in vm->evicted list. Any other ideas which can keep these order 
>> but not increase submission overhead?
>
> With more thinking, we need to add new LRU design for per vm bo, we 
> need to make sure the order when adding to LRU. How about the below idea:
> 0. separate traditional bo list lru and per-vm-bo lru. Traditional lru 
> keeps old way, per-vm-lru follows below design.
> 1. TTM bdev maintains a vm/process list.
> 2. Every vm_list node contains its own per-vm-bo LRU[priority]
> 3. To manage the vm_list lru in specific driver, we will need add 
> callback for it.
> 4. We will add an order for every per-vm-bo in that vm/process.
> 5. To speed up per-vm-lru sort, we will introduce RB tree for it in 
> callback. The RB tree key is order.
>
> This way, we will be able to keep the per-vm-bo LRU order.
>
> What do you think of it?

No, we need a single LRU for per VM and not per VM BOs to maintain 
eviction fairness, so we don't really win anything with that.

Regards,
Christian.

>
> Regards,
> David Zhou
>>
>> Regards,
>> David Zhou
>>>
>>> BTW: We can easily walk all BOs which belong to a VM, skipping over 
>>> the few which aren't per VM BOs should be trivial.
>>>
>>> Christian.
>>>
>>> Am 27.03.2018 um 13:56 schrieb Zhou, David(ChunMing):
>>>> then how to keep unique lru order? any ideas?
>>>>
>>>> To stable performance, we have to keep unique lru order, otherwise 
>>>> like the issue I look into, sometimes F1game is 40fps, sometimes 
>>>> 28fps...even re-validate allowed domains BO.
>>>>
>>>> The left root cause is the moved BOs are not same.
>>>>
>>>> send from Smartisan Pro
>>>>
>>>> Christian K鰊ig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 于 2018年3月27日 
>>>> 下午6:50写道：
>>>>
>>>> NAK, we already tried that and it is really not a good idea because it
>>>> massively increases the per submission overhead.
>>>>
>>>> Christian.
>>>>
>>>> Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
>>>> > Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
>>>> > Signed-off-by: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>
>>>> > ---
>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>>>> >   3 files changed, 18 insertions(+)
>>>> >
>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> > index 383bf2d31c92..414e61799236 100644
>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> > @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct 
>>>> amdgpu_cs_parser *p)
>>>> >                }
>>>> >        }
>>>> >
>>>> > +     amdgpu_vm_refresh_lru(adev, vm);
>>>> > +
>>>> >        return r;
>>>> >   }
>>>> >
>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> > index 5e35e23511cf..8ad2bb705765 100644
>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> > @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va 
>>>> *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>>> >        return bo_va;
>>>> >   }
>>>> >
>>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>>> amdgpu_vm *vm)
>>>> > +{
>>>> > +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
>>>> > +     struct amdgpu_vm_bo_base *bo_base;
>>>> > +
>>>> > +     spin_lock(&vm->status_lock);
>>>> > +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
>>>> > +             spin_lock(&glob->lru_lock);
>>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
>>>> > +             if (bo_base->bo->shadow)
>>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
>>>> > + spin_unlock(&glob->lru_lock);
>>>> > +     }
>>>> > +     spin_unlock(&vm->status_lock);
>>>> > +}
>>>> >
>>>> >   /**
>>>> >    * amdgpu_vm_bo_insert_mapping - insert a new mapping
>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> > index 1886a561c84e..e01895581489 100644
>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>> > @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct 
>>>> amdgpu_device *adev,
>>>> >                          struct dma_fence **fence);
>>>> >   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>>> >                           struct amdgpu_vm *vm);
>>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>>> amdgpu_vm *vm);
>>>> >   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>>> >                        struct amdgpu_bo_va *bo_va,
>>>> >                        bool clear);
>>>>
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 11561 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [PATCH 2/3] drm/amdgpu: refresh per vm bo lru

[Chunming Zhou]

[-- Attachment #1.1: Type: text/plain, Size: 6498 bytes --]



在 2018/3/29 16:59, Christian König 写道:
> Am 29.03.2018 um 10:37 schrieb zhoucm1:
>>
>>
>>
>> On 2018年03月28日 16:13, zhoucm1 wrote:
>>>
>>>
>>>
>>> On 2018年03月27日 21:44, Christian König wrote:
>>>
>>>> How about we update the LRU only when we need to re-validate at 
>>>> least one BO?
>>> I tried this just now, performance still isn't stable, sometime drop 
>>> to 28fps by accident.
>
> Can you give me the code for that? I probably can't work this week on 
> that, but I can take a look next week.
just git send-mail to your amd mail.

>
>>>
>>> I also tried to check num_evictions, if eviction happens, then 
>>> update LRU, also sometime drop to 28fps by accident.
>>>
>>> When BOs change, we not only need keep LRU order, but also 
>>> validation order in vm->evicted list. Any other ideas which can keep 
>>> these order but not increase submission overhead?
>>
>> With more thinking, we need to add new LRU design for per vm bo, we 
>> need to make sure the order when adding to LRU. How about the below idea:
>> 0. separate traditional bo list lru and per-vm-bo lru. Traditional 
>> lru keeps old way, per-vm-lru follows below design.
>> 1. TTM bdev maintains a vm/process list.
>> 2. Every vm_list node contains its own per-vm-bo LRU[priority]
>> 3. To manage the vm_list lru in specific driver, we will need add 
>> callback for it.
>> 4. We will add an order for every per-vm-bo in that vm/process.
>> 5. To speed up per-vm-lru sort, we will introduce RB tree for it in 
>> callback. The RB tree key is order.
>>
>> This way, we will be able to keep the per-vm-bo LRU order.
>>
>> What do you think of it?
>
> No, we need a single LRU for per VM and not per VM BOs to maintain 
> eviction fairness, so we don't really win anything with that.
If following original LRU design, the bo should be moved to lru tail 
when bo is used, so that keep the last used bo is in lru tail.
All per vm BOs are used for every command submission, then after every 
CS, we should refresh the lru, that is required by original LRU design, 
but as your NAK on it, it will introduce much CPU overhead for CS, they 
are inconsistencies.

For per vm case, if we don't want to introduce extra overhead, the 
per-vm-bo order shoud fixed in lru to avoid refresh LRU for every CS.
so my thinking for lru is:
VM1-BO1---->BO2--->BO3--->BOn--->VM2-BO1--->BO2--->BO3--->BOn--->VM3-BO...

Regards,
David
>
> Regards,
> Christian.
>
>>
>> Regards,
>> David Zhou
>>>
>>> Regards,
>>> David Zhou
>>>>
>>>> BTW: We can easily walk all BOs which belong to a VM, skipping over 
>>>> the few which aren't per VM BOs should be trivial.
>>>>
>>>> Christian.
>>>>
>>>> Am 27.03.2018 um 13:56 schrieb Zhou, David(ChunMing):
>>>>> then how to keep unique lru order? any ideas?
>>>>>
>>>>> To stable performance, we have to keep unique lru order, otherwise 
>>>>> like the issue I look into, sometimes F1game is 40fps, sometimes 
>>>>> 28fps...even re-validate allowed domains BO.
>>>>>
>>>>> The left root cause is the moved BOs are not same.
>>>>>
>>>>> send from Smartisan Pro
>>>>>
>>>>> Christian K鰊ig <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 于 2018年3月27日 
>>>>> 下午6:50写道：
>>>>>
>>>>> NAK, we already tried that and it is really not a good idea 
>>>>> because it
>>>>> massively increases the per submission overhead.
>>>>>
>>>>> Christian.
>>>>>
>>>>> Am 27.03.2018 um 12:16 schrieb Chunming Zhou:
>>>>> > Change-Id: Ibad84ed585b0746867a5f4cd1eadc2273e7cf596
>>>>> > Signed-off-by: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>
>>>>> > ---
>>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 ++
>>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 15 +++++++++++++++
>>>>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  1 +
>>>>> >   3 files changed, 18 insertions(+)
>>>>> >
>>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>> > index 383bf2d31c92..414e61799236 100644
>>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>> > @@ -919,6 +919,8 @@ static int amdgpu_bo_vm_update_pte(struct 
>>>>> amdgpu_cs_parser *p)
>>>>> >                }
>>>>> >        }
>>>>> >
>>>>> > +     amdgpu_vm_refresh_lru(adev, vm);
>>>>> > +
>>>>> >        return r;
>>>>> >   }
>>>>> >
>>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> > index 5e35e23511cf..8ad2bb705765 100644
>>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> > @@ -1902,6 +1902,21 @@ struct amdgpu_bo_va 
>>>>> *amdgpu_vm_bo_add(struct amdgpu_device *adev,
>>>>> >        return bo_va;
>>>>> >   }
>>>>> >
>>>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>>>> amdgpu_vm *vm)
>>>>> > +{
>>>>> > +     struct ttm_bo_global *glob = adev->mman.bdev.glob;
>>>>> > +     struct amdgpu_vm_bo_base *bo_base;
>>>>> > +
>>>>> > +     spin_lock(&vm->status_lock);
>>>>> > +     list_for_each_entry(bo_base, &vm->vm_bo_list, vm_bo) {
>>>>> > + spin_lock(&glob->lru_lock);
>>>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->tbo);
>>>>> > +             if (bo_base->bo->shadow)
>>>>> > + ttm_bo_move_to_lru_tail(&bo_base->bo->shadow->tbo);
>>>>> > + spin_unlock(&glob->lru_lock);
>>>>> > +     }
>>>>> > +     spin_unlock(&vm->status_lock);
>>>>> > +}
>>>>> >
>>>>> >   /**
>>>>> >    * amdgpu_vm_bo_insert_mapping - insert a new mapping
>>>>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>> > index 1886a561c84e..e01895581489 100644
>>>>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>>>> > @@ -285,6 +285,7 @@ int amdgpu_vm_clear_freed(struct 
>>>>> amdgpu_device *adev,
>>>>> >                          struct dma_fence **fence);
>>>>> >   int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>>>> >                           struct amdgpu_vm *vm);
>>>>> > +void amdgpu_vm_refresh_lru(struct amdgpu_device *adev, struct 
>>>>> amdgpu_vm *vm);
>>>>> >   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>>>> >                        struct amdgpu_bo_va *bo_va,
>>>>> >                        bool clear);
>>>>>
>>>>
>>>
>>
>


[-- Attachment #1.2: Type: text/html, Size: 13382 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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