Re: [PATCH] drm/amdgpu: add module parameter to enable/disable bulk move

[Huang Rui <ray.huang-5C7GfCeVMHo@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