| Message ID | 1534924375-5837-1-git-send-email-ray.huang@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/ttm,amdgpu: Introduce LRU bulk move functionality | expand |
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. 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(-) >
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(-) > > >
Hi Is there an updated series? These no longer apply for me Thanks Mike On Wed, 22 Aug 2018 at 09:42 Huang Rui <ray.huang@amd.com> wrote: > 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(-) > > > > > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > <div dir="ltr">Hi<div><br></div><div>Is there an updated series? These no longer apply for me </div><div><br></div><div>Thanks</div><div><br></div><div>Mike</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, 22 Aug 2018 at 09:42 Huang Rui <<a href="mailto:ray.huang@amd.com">ray.huang@amd.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Aug 22, 2018 at 04:24:02PM +0800, Christian König wrote:<br> > Please commit patches #1, #2 and #3, doesn't make much sense to send <br> > them out even more often.<br> > <br> > Jerry's comments on patch #4 sound valid to me as well, but with those <br> > minor issues fixes/commented I think we can commit it.<br> > <br> > Thanks for taking care of this,<br> > Christian.<br> <br> OK. Thanks to your time.<br> <br> Thanks,<br> Ray<br> <br> > <br> > Am 22.08.2018 um 09:52 schrieb Huang Rui:<br> > > The idea and proposal is originally from Christian, and I continue to work to<br> > > deliver it.<br> > ><br> > > Background:<br> > > amdgpu driver will move all PD/PT and PerVM BOs into idle list. Then move all of<br> > > them on the end of LRU list one by one. Thus, that cause so many BOs moved to<br> > > the end of the LRU, and impact performance seriously.<br> > ><br> > > Then Christian provided a workaround to not move PD/PT BOs on LRU with below<br> > > patch:<br> > > Commit 0bbf32026cf5ba41e9922b30e26e1bed1ecd38ae ("drm/amdgpu: band aid<br> > > validating VM PTs")<br> > ><br> > > However, the final solution should bulk move all PD/PT and PerVM BOs on the LRU<br> > > instead of one by one.<br> > ><br> > > Whenever amdgpu_vm_validate_pt_bos() is called and we have BOs which need to be<br> > > validated we move all BOs together to the end of the LRU without dropping the<br> > > lock for the LRU.<br> > ><br> > > While doing so we note the beginning and end of this block in the LRU list.<br> > ><br> > > Now when amdgpu_vm_validate_pt_bos() is called and we don't have anything to do,<br> > > we don't move every BO one by one, but instead cut the LRU list into pieces so<br> > > that we bulk move everything to the end in just one operation.<br> > ><br> > > Test data:<br> > > +--------------+-----------------+-----------+---------------------------------------+<br> > > | |The Talos |Clpeak(OCL)|BusSpeedReadback(OCL) |<br> > > | |Principle(Vulkan)| | |<br> > > +------------------------------------------------------------------------------------+<br> > > | | | |0.319 ms(1k) 0.314 ms(2K) 0.308 ms(4K) |<br> > > | Original | 147.7 FPS | 76.86 us |0.307 ms(8K) 0.310 ms(16K) |<br> > > +------------------------------------------------------------------------------------+<br> > > | Orignial + WA| | |0.254 ms(1K) 0.241 ms(2K) |<br> > > |(don't move | 162.1 FPS | 42.15 us |0.230 ms(4K) 0.223 ms(8K) 0.204 ms(16K)|<br> > > |PT BOs on LRU)| | | |<br> > > +------------------------------------------------------------------------------------+<br> > > | Bulk move | 163.1 FPS | 40.52 us |0.244 ms(1K) 0.252 ms(2K) 0.213 ms(4K) |<br> > > | | | |0.214 ms(8K) 0.225 ms(16K) |<br> > > +--------------+-----------------+-----------+---------------------------------------+<br> > ><br> > > After test them with above three benchmarks include vulkan and opencl. We can<br> > > see the visible improvement than original, and even better than original with<br> > > workaround.<br> > ><br> > > Changes from V1 -> V2:<br> > > - Fix to missed the BOs in relocated/moved that should be also moved to the end<br> > > of LRU.<br> > ><br> > > Changes from V2 -> V3:<br> > > - Remove unused parameter and use list_for_each_entry instead of the one with<br> > > save entry.<br> > ><br> > > Changes from V3 -> V4:<br> > > - Move the amdgpu_vm_move_to_lru_tail after command submission, at that time,<br> > > all bo will be back on idle list.<br> > ><br> > > Changes from V4 -> V5:<br> > > - Remove amdgpu_vm_move_to_lru_tail_by_list(), use bulk_moveable instread of<br> > > validated, and move ttm_bo_bulk_move_lru_tail() also into<br> > > amdgpu_vm_move_to_lru_tail().<br> > ><br> > > Thanks,<br> > > Ray<br> > ><br> > > Christian König (2):<br> > > drm/ttm: add helper structures for bulk moves on lru list<br> > > drm/ttm: revise ttm_bo_move_to_lru_tail to support bulk moves<br> > ><br> > > Huang Rui (3):<br> > > drm/ttm: add bulk move function on LRU<br> > > drm/amdgpu: use bulk moves for efficient VM LRU handling (v5)<br> > > drm/amdgpu: move PD/PT bos on LRU again<br> > ><br> > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 +++++<br> > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 68 +++++++++++++++++++----------<br> > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 11 ++++-<br> > > drivers/gpu/drm/ttm/ttm_bo.c | 78 +++++++++++++++++++++++++++++++++-<br> > > include/drm/ttm/ttm_bo_api.h | 16 ++++++-<br> > > include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++<br> > > 6 files changed, 186 insertions(+), 25 deletions(-)<br> > ><br> > <br> _______________________________________________<br> amd-gfx mailing list<br> <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a><br> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br> </blockquote></div>
That one is already committed to amd-staging-drm-next. But I've fixed a few bugs with that just yesterday, not sure if the public copy of amd-staging-drm-next is already up to date. Christian. Am 02.09.2018 10:12 schrieb Mike Lothian <mike@fireburn.co.uk>: Hi Is there an updated series? These no longer apply for me Thanks Mike On Wed, 22 Aug 2018 at 09:42 Huang Rui <ray.huang@amd.com<mailto:ray.huang@amd.com>> wrote: 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(-) > > >
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(-)