<div dir="ltr"><div dir="ltr"><div>I don't see anything wrong with it</div><div><br></div><div>Reviewed-by: Faith Ekstrand <<a href="mailto:faith.ekstrand@collabora.com">faith.ekstrand@collabora.com</a>></div><div><br></div><div>However, I'm not an AMDGPU developer so you probably want someone who knows the amdgpu sync stuff to review as well.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 7, 2024 at 8:09 PM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">For the rationale see the earlier RFC by Faith: <a href="https://lists.freedesktop.org/archives/amd-gfx/2024-August/112273.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/amd-gfx/2024-August/112273.html</a><br>
<br>
This mainly makes two changes:<br>
<br>
1. Uses a submission flag rather than a context creation flag.<br>
2. Uses DMA_RESV_USAGE_BOOKKEEP to avoid adding implicit fences still.<br>
<br>
Note that this doesn't disable implicit sync wrt VM ops (map/unmap), I know we have series for that going around,<br>
but I believe doing just submissions here is less involved and doesn't really complicate doing VM ops later.<br>
<br>
As of now this has received a limited set of testing, no full CTS runs etc yet.<br>
<br>
For Userspace see:<br>
<br>
libdrm: <a href="https://gitlab.freedesktop.org/bnieuwenhuizen/drm/-/commits/basic-explicit-sync" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/bnieuwenhuizen/drm/-/commits/basic-explicit-sync</a><br>
<br>
mesa: <a href="https://gitlab.freedesktop.org/bnieuwenhuizen/mesa/-/commits/basic-explicit-sync" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/bnieuwenhuizen/mesa/-/commits/basic-explicit-sync</a><br></blockquote><div><br></div><div>If you want to throws in MRs, I'll review them as well. That can be a "real" review.</div><div><br></div><div>Also, Michel is working on trying to reproduce the original Mesa issue so we can verify the end result.</div><div><br></div><div>~Faith</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
(Still missing a bunch of the version bumps & version checks, would like to postpone that till we know the actual version)<br>
<br>
Bas Nieuwenhuizen (6):<br>
  amdgpu: Add usage argument to amdgpu_sync_resv.<br>
  amdgpu: Ignore BOOKKEEP fences for submissions.<br>
  drm/amdgpu: Check cs flags.<br>
  drm/amdgpu: Add UAPI for disabling implicit sync per submission.<br>
  drm/amdgpu: Implement disabling implicit sync per submission.<br>
  drm/amdgpu: Bump the driver version for the new flag.<br>
<br>
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  1 +<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        | 21 ++++++++++++++++---<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h        |  1 +<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  3 ++-<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  3 ++-<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c      |  7 +++----<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h      |  4 ++--<br>
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |  3 ++-<br>
 include/uapi/drm/amdgpu_drm.h                 |  6 ++++++<br>
 9 files changed, 37 insertions(+), 12 deletions(-)<br>
<br>
-- <br>
2.45.2<br>
<br>
</blockquote></div></div>