| Message ID | 1533902159-14232-1-git-send-email-ray.huang@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/ttm,amdgpu: Introduce LRU bulk move functionality | expand |
Hi I've been testing these patches over the weekend on my Tonga and Raven systems Tested-by: Mike Lothian <mike@fireburn.co.uk> Cheers Mike On Fri, 10 Aug 2018 at 12:56 Huang Rui <ray.huang@amd.com> wrote: > 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: > "drm/amdgpu: band aid validating VM PTs" > Commit 0bbf32026cf5ba41e9922b30e26e1bed1ecd38ae > > 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. > > Thanks, > Rui > > 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 (v2) > drm/amdgpu: move PD/PT bos on LRU again > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 77 > +++++++++++++++++++++++++-------- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++ > drivers/gpu/drm/ttm/ttm_bo.c | 78 > +++++++++++++++++++++++++++++++++- > include/drm/ttm/ttm_bo_api.h | 16 ++++++- > include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++ > 5 files changed, 184 insertions(+), 19 deletions(-) > > -- > 2.7.4 > > _______________________________________________ > 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>I've been testing these patches over the weekend on my Tonga and Raven systems</div><div><br></div><div>Tested-by: Mike Lothian <<a href="mailto:mike@fireburn.co.uk">mike@fireburn.co.uk</a>></div><div><br></div><div>Cheers</div><div><br></div><div>Mike</div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, 10 Aug 2018 at 12:56 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">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> "drm/amdgpu: band aid validating VM PTs"<br> Commit 0bbf32026cf5ba41e9922b30e26e1bed1ecd38ae<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> Thanks,<br> Rui<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 (v2)<br> drm/amdgpu: move PD/PT bos on LRU again<br> <br> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 77 +++++++++++++++++++++++++--------<br> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++<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> 5 files changed, 184 insertions(+), 19 deletions(-)<br> <br> -- <br> 2.7.4<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>
On Sun, Aug 12, 2018 at 10:12:56PM +0100, Mike Lothian wrote: > Hi > > I've been testing these patches over the weekend on my Tonga and Raven > systems > > Tested-by: Mike Lothian <mike@fireburn.co.uk> > Thanks so much, Mike. I will add your tested-by in my next version. Thanks, Ray > Cheers > > Mike > > On Fri, 10 Aug 2018 at 12:56 Huang Rui <ray.huang@amd.com> wrote: > > > 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: > > "drm/amdgpu: band aid validating VM PTs" > > Commit 0bbf32026cf5ba41e9922b30e26e1bed1ecd38ae > > > > 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. > > > > Thanks, > > Rui > > > > 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 (v2) > > drm/amdgpu: move PD/PT bos on LRU again > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 77 > > +++++++++++++++++++++++++-------- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ++ > > drivers/gpu/drm/ttm/ttm_bo.c | 78 > > +++++++++++++++++++++++++++++++++- > > include/drm/ttm/ttm_bo_api.h | 16 ++++++- > > include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++ > > 5 files changed, 184 insertions(+), 19 deletions(-) > > > > -- > > 2.7.4 > > > > _______________________________________________ > > 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