On Wed, Aug 22, 2018 at 04:24:02PM +0800, Christian König wrote: > Please commit patches #1, #2 and #3, doesn't make much sense to send > them out even more often. > > Jerry's comments on patch #4 sound valid to me as well, but with those > minor issues fixes/commented I think we can commit it. > > Thanks for taking care of this, > Christian. OK. Thanks to your time. Thanks, Ray > > Am 22.08.2018 um 09:52 schrieb Huang Rui: > > The idea and proposal is originally from Christian, and I continue to work to > > deliver it. > > > > Background: > > amdgpu driver will move all PD/PT and PerVM BOs into idle list. Then move all of > > them on the end of LRU list one by one. Thus, that cause so many BOs moved to > > the end of the LRU, and impact performance seriously. > > > > Then Christian provided a workaround to not move PD/PT BOs on LRU with below > > patch: > > Commit 0bbf32026cf5ba41e9922b30e26e1bed1ecd38ae ("drm/amdgpu: band aid > > validating VM PTs") > > > > However, the final solution should bulk move all PD/PT and PerVM BOs on the LRU > > instead of one by one. > > > > Whenever amdgpu_vm_validate_pt_bos() is called and we have BOs which need to be > > validated we move all BOs together to the end of the LRU without dropping the > > lock for the LRU. > > > > While doing so we note the beginning and end of this block in the LRU list. > > > > Now when amdgpu_vm_validate_pt_bos() is called and we don't have anything to do, > > we don't move every BO one by one, but instead cut the LRU list into pieces so > > that we bulk move everything to the end in just one operation. > > > > Test data: > > +--------------+-----------------+-----------+---------------------------------------+ > > | |The Talos |Clpeak(OCL)|BusSpeedReadback(OCL) | > > | |Principle(Vulkan)| | | > > +------------------------------------------------------------------------------------+ > > | | | |0.319 ms(1k) 0.314 ms(2K) 0.308 ms(4K) | > > | Original | 147.7 FPS | 76.86 us |0.307 ms(8K) 0.310 ms(16K) | > > +------------------------------------------------------------------------------------+ > > | Orignial + WA| | |0.254 ms(1K) 0.241 ms(2K) | > > |(don't move | 162.1 FPS | 42.15 us |0.230 ms(4K) 0.223 ms(8K) 0.204 ms(16K)| > > |PT BOs on LRU)| | | | > > +------------------------------------------------------------------------------------+ > > | Bulk move | 163.1 FPS | 40.52 us |0.244 ms(1K) 0.252 ms(2K) 0.213 ms(4K) | > > | | | |0.214 ms(8K) 0.225 ms(16K) | > > +--------------+-----------------+-----------+---------------------------------------+ > > > > After test them with above three benchmarks include vulkan and opencl. We can > > see the visible improvement than original, and even better than original with > > workaround. > > > > Changes from V1 -> V2: > > - Fix to missed the BOs in relocated/moved that should be also moved to the end > > of LRU. > > > > Changes from V2 -> V3: > > - Remove unused parameter and use list_for_each_entry instead of the one with > > save entry. > > > > Changes from V3 -> V4: > > - Move the amdgpu_vm_move_to_lru_tail after command submission, at that time, > > all bo will be back on idle list. > > > > Changes from V4 -> V5: > > - Remove amdgpu_vm_move_to_lru_tail_by_list(), use bulk_moveable instread of > > validated, and move ttm_bo_bulk_move_lru_tail() also into > > amdgpu_vm_move_to_lru_tail(). > > > > Thanks, > > Ray > > > > Christian König (2): > > drm/ttm: add helper structures for bulk moves on lru list > > drm/ttm: revise ttm_bo_move_to_lru_tail to support bulk moves > > > > Huang Rui (3): > > drm/ttm: add bulk move function on LRU > > drm/amdgpu: use bulk moves for efficient VM LRU handling (v5) > > drm/amdgpu: move PD/PT bos on LRU again > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 +++++ > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 68 +++++++++++++++++++---------- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 11 ++++- > > drivers/gpu/drm/ttm/ttm_bo.c | 78 +++++++++++++++++++++++++++++++++- > > include/drm/ttm/ttm_bo_api.h | 16 ++++++- > > include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++ > > 6 files changed, 186 insertions(+), 25 deletions(-) > > >