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 at amd.com>
> >> Cc: Christian König <christian.koenig at amd.com>
> >> Cc: Tom StDenis <Tom.StDenis at 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 at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >>
> >>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
