* [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move
@ 2018-09-11 3:41 Huang Rui
[not found] ` <1536637283-6625-1-git-send-email-ray.huang-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Huang Rui @ 2018-09-11 3:41 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Tom StDenis, Huang Rui, Christian König
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="yes", Size: 3108 bytes --]
The bulk moving mechanism still has bug on some corner cases. So disable it by
default till it is fixed. We can use the module parameter to enable it for
debugging.
Signed-off-by: Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>
Cc: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
Cc: Tom StDenis <Tom.StDenis-5C7GfCeVMHo@public.gmane.org>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++--
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e992e0f..75fd7be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -134,6 +134,7 @@ extern int amdgpu_compute_multipipe;
extern int amdgpu_gpu_recovery;
extern int amdgpu_emu_mode;
extern uint amdgpu_smu_memory_pool_size;
+extern int amdgpu_bulk_move_enabled;
#ifdef CONFIG_DRM_AMDGPU_SI
extern int amdgpu_si_support;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b5c2ccb..d57d16e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -127,6 +127,7 @@ int amdgpu_compute_multipipe = -1;
int amdgpu_gpu_recovery = -1; /* auto */
int amdgpu_emu_mode = 0;
uint amdgpu_smu_memory_pool_size = 0;
+int amdgpu_bulk_move_enabled = 0;
/**
* DOC: vramlimit (int)
@@ -532,6 +533,16 @@ MODULE_PARM_DESC(smu_memory_pool_size,
"0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
+/**
+ * DOC: bulk_move_enabled (int)
+ * It is used to enable/disable bulk moving on LRU list mechanism for debugging
+ * and testing. The default is false (disabled). It will be enabled by default
+ * till all corner cases are hammered out.
+ */
+MODULE_PARM_DESC(bulk_move_enabled,
+ "Enable bulk moving on LRU list mechanism (1 = enabled, 0 = disabled (default)");
+module_param_named(bulk_move_enabled, amdgpu_bulk_move_enabled, uint, 0444);
+
#ifdef CONFIG_HSA_AMD
/**
* DOC: sched_policy (int)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 9a5b1bb..ab95a9c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,7 +397,8 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
}
spin_unlock(&glob->lru_lock);
- vm->bulk_moveable = true;
+ if (amdgpu_bulk_move_enabled)
+ vm->bulk_moveable = true;
}
/**
@@ -1213,7 +1214,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
bo_base = list_first_entry(&vm->relocated,
struct amdgpu_vm_bo_base,
vm_status);
- amdgpu_vm_bo_idle(bo_base);
+ if (amdgpu_bulk_move_enabled)
+ amdgpu_vm_bo_idle(bo_base);
+ else {
+ /* workaround: don't move PD/PT bos on LRU */
+ list_del_init(&bo_base->vm_status);
+ bo_base->moved = false;
+ }
bo = bo_base->bo->parent;
if (!bo)
--
2.7.4
[-- 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move
[not found] ` <1536637283-6625-1-git-send-email-ray.huang-5C7GfCeVMHo@public.gmane.org>
@ 2018-09-11 10:05 ` Christian König
[not found] ` <506eed80-ab22-81a4-ef28-609c61701ef9-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2018-09-11 10:05 UTC (permalink / raw)
To: Huang Rui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Tom StDenis, Christian König
[-- Attachment #1.1: Type: text/plain, Size: 3683 bytes --]
Am 11.09.2018 um 05:41 schrieb Huang Rui:
> The bulk moving mechanism still has bug on some corner cases. So disable it by
> default till it is fixed. We can use the module parameter to enable it for
> debugging.
Please no. Module parameters are for end users, not developers.
I really don't want a bug report that this option can crash the system.
Christian.
>
> Signed-off-by: Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>
> Cc: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> Cc: Tom StDenis <Tom.StDenis-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++--
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e992e0f..75fd7be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -134,6 +134,7 @@ extern int amdgpu_compute_multipipe;
> extern int amdgpu_gpu_recovery;
> extern int amdgpu_emu_mode;
> extern uint amdgpu_smu_memory_pool_size;
> +extern int amdgpu_bulk_move_enabled;
>
> #ifdef CONFIG_DRM_AMDGPU_SI
> extern int amdgpu_si_support;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index b5c2ccb..d57d16e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -127,6 +127,7 @@ int amdgpu_compute_multipipe = -1;
> int amdgpu_gpu_recovery = -1; /* auto */
> int amdgpu_emu_mode = 0;
> uint amdgpu_smu_memory_pool_size = 0;
> +int amdgpu_bulk_move_enabled = 0;
>
> /**
> * DOC: vramlimit (int)
> @@ -532,6 +533,16 @@ MODULE_PARM_DESC(smu_memory_pool_size,
> "0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
> module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
>
> +/**
> + * DOC: bulk_move_enabled (int)
> + * It is used to enable/disable bulk moving on LRU list mechanism for debugging
> + * and testing. The default is false (disabled). It will be enabled by default
> + * till all corner cases are hammered out.
> + */
> +MODULE_PARM_DESC(bulk_move_enabled,
> + "Enable bulk moving on LRU list mechanism (1 = enabled, 0 = disabled (default)");
> +module_param_named(bulk_move_enabled, amdgpu_bulk_move_enabled, uint, 0444);
> +
> #ifdef CONFIG_HSA_AMD
> /**
> * DOC: sched_policy (int)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 9a5b1bb..ab95a9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -397,7 +397,8 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
> }
> spin_unlock(&glob->lru_lock);
>
> - vm->bulk_moveable = true;
> + if (amdgpu_bulk_move_enabled)
> + vm->bulk_moveable = true;
> }
>
> /**
> @@ -1213,7 +1214,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> bo_base = list_first_entry(&vm->relocated,
> struct amdgpu_vm_bo_base,
> vm_status);
> - amdgpu_vm_bo_idle(bo_base);
> + if (amdgpu_bulk_move_enabled)
> + amdgpu_vm_bo_idle(bo_base);
> + else {
> + /* workaround: don't move PD/PT bos on LRU */
> + list_del_init(&bo_base->vm_status);
> + bo_base->moved = false;
> + }
>
> bo = bo_base->bo->parent;
> if (!bo)
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 4726 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move
[not found] ` <506eed80-ab22-81a4-ef28-609c61701ef9-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-09-11 10:24 ` Huang Rui
2018-09-11 10:49 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Huang Rui @ 2018-09-11 10:24 UTC (permalink / raw)
To: Koenig, Christian
Cc: StDenis, Tom,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Tue, Sep 11, 2018 at 06:05:57PM +0800, Christian König wrote:
> Am 11.09.2018 um 05:41 schrieb Huang Rui:
>
> The bulk moving mechanism still has bug on some corner cases. So disable it by
> default till it is fixed. We can use the module parameter to enable it for
> debugging.
>
>
> Please no. Module parameters are for end users, not developers.
>
> I really don't want a bug report that this option can crash the system.
>
How about use a macro such as BULK_ENABLED? Actually, I just want to have a
simple way to enable it to continue debugging.
#if BULK_ENABLED
#else
#endif
Thanks,
Ray
> Christian.
>
>
>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Tom StDenis <Tom.StDenis@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++--
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e992e0f..75fd7be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -134,6 +134,7 @@ extern int amdgpu_compute_multipipe;
> extern int amdgpu_gpu_recovery;
> extern int amdgpu_emu_mode;
> extern uint amdgpu_smu_memory_pool_size;
> +extern int amdgpu_bulk_move_enabled;
>
> #ifdef CONFIG_DRM_AMDGPU_SI
> extern int amdgpu_si_support;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index b5c2ccb..d57d16e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -127,6 +127,7 @@ int amdgpu_compute_multipipe = -1;
> int amdgpu_gpu_recovery = -1; /* auto */
> int amdgpu_emu_mode = 0;
> uint amdgpu_smu_memory_pool_size = 0;
> +int amdgpu_bulk_move_enabled = 0;
>
> /**
> * DOC: vramlimit (int)
> @@ -532,6 +533,16 @@ MODULE_PARM_DESC(smu_memory_pool_size,
> "0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
> module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
>
> +/**
> + * DOC: bulk_move_enabled (int)
> + * It is used to enable/disable bulk moving on LRU list mechanism for debugging
> + * and testing. The default is false (disabled). It will be enabled by default
> + * till all corner cases are hammered out.
> + */
> +MODULE_PARM_DESC(bulk_move_enabled,
> + "Enable bulk moving on LRU list mechanism (1 = enabled, 0 = disabled (default)");
> +module_param_named(bulk_move_enabled, amdgpu_bulk_move_enabled, uint, 0444);
> +
> #ifdef CONFIG_HSA_AMD
> /**
> * DOC: sched_policy (int)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 9a5b1bb..ab95a9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -397,7 +397,8 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
> }
> spin_unlock(&glob->lru_lock);
>
> - vm->bulk_moveable = true;
> + if (amdgpu_bulk_move_enabled)
> + vm->bulk_moveable = true;
> }
>
> /**
> @@ -1213,7 +1214,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> bo_base = list_first_entry(&vm->relocated,
> struct amdgpu_vm_bo_base,
> vm_status);
> - amdgpu_vm_bo_idle(bo_base);
> + if (amdgpu_bulk_move_enabled)
> + amdgpu_vm_bo_idle(bo_base);
> + else {
> + /* workaround: don't move PD/PT bos on LRU */
> + list_del_init(&bo_base->vm_status);
> + bo_base->moved = false;
> + }
>
> bo = bo_base->bo->parent;
> if (!bo)
>
>
>
> _______________________________________________
> 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] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move
2018-09-11 10:24 ` Huang Rui
@ 2018-09-11 10:49 ` Christian König
[not found] ` <8ac85f96-43f9-eab2-fefe-1e557b4a43ee-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2018-09-11 10:49 UTC (permalink / raw)
To: Huang Rui
Cc: StDenis, Tom,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Am 11.09.2018 um 12:24 schrieb Huang Rui:
> On Tue, Sep 11, 2018 at 06:05:57PM +0800, Christian König wrote:
>> Am 11.09.2018 um 05:41 schrieb Huang Rui:
>>
>> The bulk moving mechanism still has bug on some corner cases. So disable it by
>> default till it is fixed. We can use the module parameter to enable it for
>> debugging.
>>
>>
>> Please no. Module parameters are for end users, not developers.
>>
>> I really don't want a bug report that this option can crash the system.
>>
> How about use a macro such as BULK_ENABLED? Actually, I just want to have a
> simple way to enable it to continue debugging.
Yeah, sure that should work.
But why do we actually need to modifiy amdgpu_vm_update_directories() as
well? Because of the performance?
Thanks,
Christian.
>
> #if BULK_ENABLED
> #else
> #endif
>
> Thanks,
> Ray
>
>> Christian.
>>
>>
>>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Tom StDenis <Tom.StDenis@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++--
>> 3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index e992e0f..75fd7be 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -134,6 +134,7 @@ extern int amdgpu_compute_multipipe;
>> extern int amdgpu_gpu_recovery;
>> extern int amdgpu_emu_mode;
>> extern uint amdgpu_smu_memory_pool_size;
>> +extern int amdgpu_bulk_move_enabled;
>>
>> #ifdef CONFIG_DRM_AMDGPU_SI
>> extern int amdgpu_si_support;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index b5c2ccb..d57d16e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -127,6 +127,7 @@ int amdgpu_compute_multipipe = -1;
>> int amdgpu_gpu_recovery = -1; /* auto */
>> int amdgpu_emu_mode = 0;
>> uint amdgpu_smu_memory_pool_size = 0;
>> +int amdgpu_bulk_move_enabled = 0;
>>
>> /**
>> * DOC: vramlimit (int)
>> @@ -532,6 +533,16 @@ MODULE_PARM_DESC(smu_memory_pool_size,
>> "0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
>> module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
>>
>> +/**
>> + * DOC: bulk_move_enabled (int)
>> + * It is used to enable/disable bulk moving on LRU list mechanism for debugging
>> + * and testing. The default is false (disabled). It will be enabled by default
>> + * till all corner cases are hammered out.
>> + */
>> +MODULE_PARM_DESC(bulk_move_enabled,
>> + "Enable bulk moving on LRU list mechanism (1 = enabled, 0 = disabled (default)");
>> +module_param_named(bulk_move_enabled, amdgpu_bulk_move_enabled, uint, 0444);
>> +
>> #ifdef CONFIG_HSA_AMD
>> /**
>> * DOC: sched_policy (int)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 9a5b1bb..ab95a9c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -397,7 +397,8 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
>> }
>> spin_unlock(&glob->lru_lock);
>>
>> - vm->bulk_moveable = true;
>> + if (amdgpu_bulk_move_enabled)
>> + vm->bulk_moveable = true;
>> }
>>
>> /**
>> @@ -1213,7 +1214,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
>> bo_base = list_first_entry(&vm->relocated,
>> struct amdgpu_vm_bo_base,
>> vm_status);
>> - amdgpu_vm_bo_idle(bo_base);
>> + if (amdgpu_bulk_move_enabled)
>> + amdgpu_vm_bo_idle(bo_base);
>> + else {
>> + /* workaround: don't move PD/PT bos on LRU */
>> + list_del_init(&bo_base->vm_status);
>> + bo_base->moved = false;
>> + }
>>
>> bo = bo_base->bo->parent;
>> if (!bo)
>>
>>
>>
>> _______________________________________________
>> 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] 5+ messages in thread
* Re: [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move
[not found] ` <8ac85f96-43f9-eab2-fefe-1e557b4a43ee-5C7GfCeVMHo@public.gmane.org>
@ 2018-09-11 11:42 ` Huang Rui
0 siblings, 0 replies; 5+ messages in thread
From: Huang Rui @ 2018-09-11 11:42 UTC (permalink / raw)
To: Christian König
Cc: StDenis, Tom,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
On Tue, Sep 11, 2018 at 12:49:59PM +0200, Christian König wrote:
> Am 11.09.2018 um 12:24 schrieb Huang Rui:
> >On Tue, Sep 11, 2018 at 06:05:57PM +0800, Christian König wrote:
> >>Am 11.09.2018 um 05:41 schrieb Huang Rui:
> >>
> >> The bulk moving mechanism still has bug on some corner cases. So disable it by
> >> default till it is fixed. We can use the module parameter to enable it for
> >> debugging.
> >>
> >>
> >>Please no. Module parameters are for end users, not developers.
> >>
> >>I really don't want a bug report that this option can crash the system.
> >>
> >How about use a macro such as BULK_ENABLED? Actually, I just want to have a
> >simple way to enable it to continue debugging.
>
> Yeah, sure that should work.
>
> But why do we actually need to modifiy
> amdgpu_vm_update_directories() as well? Because of the performance?
Yes, I'm afraid that it will make the performance drop again in some
specific cases.
Thanks,
Ray
>
> Thanks,
> Christian.
>
> >
> >#if BULK_ENABLED
> >#else
> >#endif
> >
> >Thanks,
> >Ray
> >
> >>Christian.
> >>
> >>
> >>
> >> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> Cc: Christian König <christian.koenig@amd.com>
> >> Cc: Tom StDenis <Tom.StDenis@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +++++++++--
> >> 3 files changed, 21 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index e992e0f..75fd7be 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -134,6 +134,7 @@ extern int amdgpu_compute_multipipe;
> >> extern int amdgpu_gpu_recovery;
> >> extern int amdgpu_emu_mode;
> >> extern uint amdgpu_smu_memory_pool_size;
> >> +extern int amdgpu_bulk_move_enabled;
> >>
> >> #ifdef CONFIG_DRM_AMDGPU_SI
> >> extern int amdgpu_si_support;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> index b5c2ccb..d57d16e 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> >> @@ -127,6 +127,7 @@ int amdgpu_compute_multipipe = -1;
> >> int amdgpu_gpu_recovery = -1; /* auto */
> >> int amdgpu_emu_mode = 0;
> >> uint amdgpu_smu_memory_pool_size = 0;
> >> +int amdgpu_bulk_move_enabled = 0;
> >>
> >> /**
> >> * DOC: vramlimit (int)
> >> @@ -532,6 +533,16 @@ MODULE_PARM_DESC(smu_memory_pool_size,
> >> "0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte");
> >> module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444);
> >>
> >> +/**
> >> + * DOC: bulk_move_enabled (int)
> >> + * It is used to enable/disable bulk moving on LRU list mechanism for debugging
> >> + * and testing. The default is false (disabled). It will be enabled by default
> >> + * till all corner cases are hammered out.
> >> + */
> >> +MODULE_PARM_DESC(bulk_move_enabled,
> >> + "Enable bulk moving on LRU list mechanism (1 = enabled, 0 = disabled (default)");
> >> +module_param_named(bulk_move_enabled, amdgpu_bulk_move_enabled, uint, 0444);
> >> +
> >> #ifdef CONFIG_HSA_AMD
> >> /**
> >> * DOC: sched_policy (int)
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> index 9a5b1bb..ab95a9c 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> @@ -397,7 +397,8 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device *adev,
> >> }
> >> spin_unlock(&glob->lru_lock);
> >>
> >> - vm->bulk_moveable = true;
> >> + if (amdgpu_bulk_move_enabled)
> >> + vm->bulk_moveable = true;
> >> }
> >>
> >> /**
> >> @@ -1213,7 +1214,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device *adev,
> >> bo_base = list_first_entry(&vm->relocated,
> >> struct amdgpu_vm_bo_base,
> >> vm_status);
> >> - amdgpu_vm_bo_idle(bo_base);
> >> + if (amdgpu_bulk_move_enabled)
> >> + amdgpu_vm_bo_idle(bo_base);
> >> + else {
> >> + /* workaround: don't move PD/PT bos on LRU */
> >> + list_del_init(&bo_base->vm_status);
> >> + bo_base->moved = false;
> >> + }
> >>
> >> bo = bo_base->bo->parent;
> >> if (!bo)
> >>
> >>
> >> _______________________________________________
> >> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-11 11:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-11 3:41 [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move Huang Rui
[not found] ` <1536637283-6625-1-git-send-email-ray.huang-5C7GfCeVMHo@public.gmane.org>
2018-09-11 10:05 ` Christian König
[not found] ` <506eed80-ab22-81a4-ef28-609c61701ef9-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-09-11 10:24 ` Huang Rui
2018-09-11 10:49 ` Christian König
[not found] ` <8ac85f96-43f9-eab2-fefe-1e557b4a43ee-5C7GfCeVMHo@public.gmane.org>
2018-09-11 11:42 ` Huang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox