mbox series

[v3,0/3] drm/amdgpu: Explicit sync for GEM VA operations

Message ID 20240819072110.108715-1-friedrich.vock@gmx.de (mailing list archive)
Headers show
Series drm/amdgpu: Explicit sync for GEM VA operations | expand

Message

Friedrich Vock Aug. 19, 2024, 7:21 a.m. UTC
In Vulkan, it is the application's responsibility to perform adequate
synchronization before a sparse unmap, replace or BO destroy operation.
This adds an option to AMDGPU_VA_OPs to disable redundant implicit sync
that happens on sparse unmap or replace operations.

This has seen a significant improvement in stutter in Forza Horizon 5
and Forza Horizon 4. (As games that had significant issues in sparse
binding related stutter).

Userspace changes for this new version can be found at [1][2], and a kernel
branch containing these patches can be found at [3].

[1] https://gitlab.freedesktop.org/pixelcluster/drm/-/commits/vm-explicit-sync
[2] https://gitlab.freedesktop.org/pixelcluster/mesa/-/commits/vm-explicit-sync
[3] https://gitlab.freedesktop.org/pixelcluster/linux/-/commits/amdgpu-vm-explicit-sync

v3 changes:
- Rebased onto current amd-staging-drm-next
- Added option to wait for drm_syncobjs instead of executing immediately

Tatsuyuki Ishi (3):
  drm/amdgpu: Don't implicit sync PRT maps.
  drm/amdgpu: Add optional explicit sync fences for GEM operations.
  drm/amdgpu: Bump amdgpu driver version.

 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 76 ++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    | 23 +++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h     |  6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 68 +++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        | 30 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c    | 12 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  9 +++
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 18 ++---
 include/uapi/drm/amdgpu_drm.h                 |  7 ++
 14 files changed, 194 insertions(+), 66 deletions(-)

--
2.46.0

Comments

Christian König Aug. 19, 2024, 11:21 a.m. UTC | #1
Am 19.08.24 um 09:21 schrieb Friedrich Vock:
> In Vulkan, it is the application's responsibility to perform adequate
> synchronization before a sparse unmap, replace or BO destroy operation.
> This adds an option to AMDGPU_VA_OPs to disable redundant implicit sync
> that happens on sparse unmap or replace operations.
>
> This has seen a significant improvement in stutter in Forza Horizon 5
> and Forza Horizon 4. (As games that had significant issues in sparse
> binding related stutter).

Looks pretty good, I have Shashank and his team working on the similar 
patches ever since Bas initially came up with it since we need it for 
user queues as well.

Shashank can you take a look at the UAPI? Of hand looks pretty similar 
to what we have done as well, doesn't it?

For the internal implementation in the VM I'm currently working on a bug 
fix for the KFD team, so this is subject to change anyway. Going to keep 
this requirement here in mind while doing that, whatever implementation 
we end up with we probably need to re-base it anyway.

Regards,
Christian.

>
> Userspace changes for this new version can be found at [1][2], and a kernel
> branch containing these patches can be found at [3].
>
> [1] https://gitlab.freedesktop.org/pixelcluster/drm/-/commits/vm-explicit-sync
> [2] https://gitlab.freedesktop.org/pixelcluster/mesa/-/commits/vm-explicit-sync
> [3] https://gitlab.freedesktop.org/pixelcluster/linux/-/commits/amdgpu-vm-explicit-sync
>
> v3 changes:
> - Rebased onto current amd-staging-drm-next
> - Added option to wait for drm_syncobjs instead of executing immediately
>
> Tatsuyuki Ishi (3):
>    drm/amdgpu: Don't implicit sync PRT maps.
>    drm/amdgpu: Add optional explicit sync fences for GEM operations.
>    drm/amdgpu: Bump amdgpu driver version.
>
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c       |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  3 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 76 ++++++++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    | 23 +++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h     |  6 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 68 +++++++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        | 30 ++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c    | 12 ++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  9 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 18 ++---
>   include/uapi/drm/amdgpu_drm.h                 |  7 ++
>   14 files changed, 194 insertions(+), 66 deletions(-)
>
> --
> 2.46.0
>
Sharma, Shashank Aug. 20, 2024, 1:36 p.m. UTC | #2
Hey Christian,

On 19/08/2024 13:21, Christian König wrote:
> Am 19.08.24 um 09:21 schrieb Friedrich Vock:
>> In Vulkan, it is the application's responsibility to perform adequate
>> synchronization before a sparse unmap, replace or BO destroy operation.
>> This adds an option to AMDGPU_VA_OPs to disable redundant implicit sync
>> that happens on sparse unmap or replace operations.
>>
>> This has seen a significant improvement in stutter in Forza Horizon 5
>> and Forza Horizon 4. (As games that had significant issues in sparse
>> binding related stutter).
>
> Looks pretty good, I have Shashank and his team working on the similar 
> patches ever since Bas initially came up with it since we need it for 
> user queues as well.
>
> Shashank can you take a look at the UAPI? Of hand looks pretty similar 
> to what we have done as well, doesn't it?
>
I had a quick look and yes it does look very similar to what we have 
done in VM_IOCTL for UQ BO sync.

- Shashank

> For the internal implementation in the VM I'm currently working on a 
> bug fix for the KFD team, so this is subject to change anyway. Going 
> to keep this requirement here in mind while doing that, whatever 
> implementation we end up with we probably need to re-base it anyway.
>
> Regards,
> Christian.
>
>>
>> Userspace changes for this new version can be found at [1][2], and a 
>> kernel
>> branch containing these patches can be found at [3].
>>
>> [1] 
>> https://gitlab.freedesktop.org/pixelcluster/drm/-/commits/vm-explicit-sync
>> [2] 
>> https://gitlab.freedesktop.org/pixelcluster/mesa/-/commits/vm-explicit-sync
>> [3] 
>> https://gitlab.freedesktop.org/pixelcluster/linux/-/commits/amdgpu-vm-explicit-sync
>>
>> v3 changes:
>> - Rebased onto current amd-staging-drm-next
>> - Added option to wait for drm_syncobjs instead of executing immediately
>>
>> Tatsuyuki Ishi (3):
>>    drm/amdgpu: Don't implicit sync PRT maps.
>>    drm/amdgpu: Add optional explicit sync fences for GEM operations.
>>    drm/amdgpu: Bump amdgpu driver version.
>>
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c       |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  3 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 76 ++++++++++++++++---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    | 23 +++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h     |  6 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c  |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 68 +++++++++++------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        | 30 ++++----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c    | 12 ++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  9 +++
>>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 18 ++---
>>   include/uapi/drm/amdgpu_drm.h                 |  7 ++
>>   14 files changed, 194 insertions(+), 66 deletions(-)
>>
>> -- 
>> 2.46.0
>>
>