Correct. The idea is to only set AMDGPU_GEM_CREATE_EXPLICIT_SYNC for buffers that are not associated with dri2/3 or PRIME. Regards, Andres On 2017-09-19 10:10 AM, Mao, David wrote: > Hi Andres, > The explicit sync should not be used for DrI3  and DRI2 but for cross > process memory sharing, right? > We still have to rely on implicit sync to guarantee the. Correct order > of rendering and present. > Could you confirm? > > Thanks. > > Sent from my iPhone > > On 19 Sep 2017, at 9:57 PM, Andres Rodriguez <andresx7 at gmail.com > <mailto:andresx7 at gmail.com>> wrote: > >> >> >> On 2017-09-19 09:24 AM, Christian König wrote: >>> Am 19.09.2017 um 14:59 schrieb Andres Rodriguez: >>>> Introduce a flag to signal that access to a BO will be synchronized >>>> through an external mechanism. >>>> >>>> Currently all buffers shared between contexts are subject to implicit >>>> synchronization. However, this is only required for protocols that >>>> currently don't support an explicit synchronization mechanism (DRI2/3). >>>> >>>> This patch introduces the AMDGPU_GEM_CREATE_EXPLICIT_SYNC, so that >>>> users can specify when it is safe to disable implicit sync. >>>> >>>> v2: only disable explicit sync in amdgpu_cs_ioctl >>>> >>>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com >>>> <mailto:andresx7 at gmail.com>> >>>> --- >>>> >>>> Hey Christian, >>>> >>>> I kept the amdgpu_bo_explicit_sync() function since it makes it easier >>>> to maintain an 80 line wrap in amdgpu_cs_sync_rings() >>> Looks good to me, but I would like to see the matching user space >>> code as well. >>> Especially I have no idea how you want to have DRI3 compatibility >>> with that? >> >> No problem. I'm fixing the radv patch atm and I'll re-send it for your >> reference. >> >> Regards, >> Andres >> >>> Regards, >>> Christian. >>>> >>>> Regards, >>>> Andres >>>> >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c    | 3 ++- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   | 4 +++- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 8 ++++++++ >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c  | 7 +++++-- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h  | 3 ++- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 5 +++-- >>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 8 ++++---- >>>>  include/uapi/drm/amdgpu_drm.h             | 2 ++ >>>>  8 files changed, 29 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> index db97e78..bc8a403 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >>>> @@ -704,7 +704,8 @@ static int amdgpu_cs_sync_rings(struct >>>> amdgpu_cs_parser *p) >>>>      list_for_each_entry(e, &p->validated, tv.head) { >>>>          struct reservation_object *resv = e->robj->tbo.resv; >>>> -       r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, p->filp); >>>> +       r = amdgpu_sync_resv(p->adev, &p->job->sync, resv, p->filp, >>>> +                    amdgpu_bo_explicit_sync(e->robj)); >>>>          if (r) >>>>              return r; >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>>> index b0d45c8..21e9936 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c >>>> @@ -212,7 +212,9 @@ int amdgpu_gem_create_ioctl(struct drm_device >>>> *dev, void *data, >>>>                AMDGPU_GEM_CREATE_NO_CPU_ACCESS | >>>>                AMDGPU_GEM_CREATE_CPU_GTT_USWC | >>>>                AMDGPU_GEM_CREATE_VRAM_CLEARED | >>>> -             AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)) >>>> +             AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | >>>> +             AMDGPU_GEM_CREATE_EXPLICIT_SYNC)) >>>> + >>>>          return -EINVAL; >>>>      /* reject invalid gem domains */ >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >>>> index c26ef53..428aae0 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h >>>> @@ -193,6 +193,14 @@ static inline bool >>>> amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo) >>>>      } >>>>  } >>>> +/** >>>> + * amdgpu_bo_explicit_sync - return whether the bo is explicitly synced >>>> + */ >>>> +static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo) >>>> +{ >>>> +   return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC; >>>> +} >>>> + >>>>  int amdgpu_bo_create(struct amdgpu_device *adev, >>>>                  unsigned long size, int byte_align, >>>>                  bool kernel, u32 domain, u64 flags, >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c >>>> index c586f44..a4bf21f 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c >>>> @@ -169,14 +169,14 @@ int amdgpu_sync_fence(struct amdgpu_device >>>> *adev, struct amdgpu_sync *sync, >>>>   * >>>>   * @sync: sync object to add fences from reservation object to >>>>   * @resv: reservation object with embedded fence >>>> - * @shared: true if we should only sync to the exclusive fence >>>> + * @explicit_sync: true if we should only sync to the exclusive fence >>>>   * >>>>   * Sync to the fence >>>>   */ >>>>  int amdgpu_sync_resv(struct amdgpu_device *adev, >>>>               struct amdgpu_sync *sync, >>>>               struct reservation_object *resv, >>>> -            void *owner) >>>> +            void *owner, bool explicit_sync) >>>>  { >>>>      struct reservation_object_list *flist; >>>>      struct dma_fence *f; >>>> @@ -191,6 +191,9 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, >>>>      f = reservation_object_get_excl(resv); >>>>      r = amdgpu_sync_fence(adev, sync, f); >>>> +   if (explicit_sync) >>>> +       return r; >>>> + >>>>      flist = reservation_object_get_list(resv); >>>>      if (!flist || r) >>>>          return r; >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h >>>> index dc76879..70d7e3a 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h >>>> @@ -45,7 +45,8 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, >>>> struct amdgpu_sync *sync, >>>>  int amdgpu_sync_resv(struct amdgpu_device *adev, >>>>               struct amdgpu_sync *sync, >>>>               struct reservation_object *resv, >>>> -            void *owner); >>>> +            void *owner, >>>> +            bool explicit_sync); >>>>  struct dma_fence *amdgpu_sync_peek_fence(struct amdgpu_sync *sync, >>>>                       struct amdgpu_ring *ring); >>>>  struct dma_fence *amdgpu_sync_get_fence(struct amdgpu_sync *sync); >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>>> index c9c059d..bd0b03a 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>>> @@ -1510,7 +1510,8 @@ int amdgpu_copy_buffer(struct amdgpu_ring >>>> *ring, uint64_t src_offset, >>>>      job->vm_needs_flush = vm_needs_flush; >>>>      if (resv) { >>>>          r = amdgpu_sync_resv(adev, &job->sync, resv, >>>> -                    AMDGPU_FENCE_OWNER_UNDEFINED); >>>> +                    AMDGPU_FENCE_OWNER_UNDEFINED, >>>> +                    false); >>>>          if (r) { >>>>              DRM_ERROR("sync failed (%d).\n", r); >>>>              goto error_free; >>>> @@ -1602,7 +1603,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo, >>>>      if (resv) { >>>>          r = amdgpu_sync_resv(adev, &job->sync, resv, >>>> -                    AMDGPU_FENCE_OWNER_UNDEFINED); >>>> +                    AMDGPU_FENCE_OWNER_UNDEFINED, false); >>>>          if (r) { >>>>              DRM_ERROR("sync failed (%d).\n", r); >>>>              goto error_free; >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> index 2df254c..9965d68 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> @@ -987,7 +987,7 @@ static int amdgpu_vm_wait_pd(struct >>>> amdgpu_device *adev, struct amdgpu_vm *vm, >>>>      int r; >>>>      amdgpu_sync_create(&sync); >>>> -   amdgpu_sync_resv(adev, &sync, vm->root.base.bo >>>> <http://root.base.bo>->tbo.resv, owner); >>>> +   amdgpu_sync_resv(adev, &sync, vm->root.base.bo >>>> <http://root.base.bo>->tbo.resv, owner, false); >>>>      r = amdgpu_sync_wait(&sync, true); >>>>      amdgpu_sync_free(&sync); >>>> @@ -1128,11 +1128,11 @@ static int amdgpu_vm_update_level(struct >>>> amdgpu_device *adev, >>>>              amdgpu_ring_pad_ib(ring, params.ib); >>>>              amdgpu_sync_resv(adev, &job->sync, >>>>                       parent->base.bo <http://base.bo>->tbo.resv, >>>> -                    AMDGPU_FENCE_OWNER_VM); >>>> +                    AMDGPU_FENCE_OWNER_VM, false); >>>>              if (shadow) >>>>                  amdgpu_sync_resv(adev, &job->sync, >>>>                           shadow->tbo.resv, >>>> -                        AMDGPU_FENCE_OWNER_VM); >>>> +                        AMDGPU_FENCE_OWNER_VM, false); >>>>              WARN_ON(params.ib->length_dw > ndw); >>>>              r = amdgpu_job_submit(job, ring, &vm->entity, >>>> @@ -1595,7 +1595,7 @@ static int amdgpu_vm_bo_update_mapping(struct >>>> amdgpu_device *adev, >>>>          goto error_free; >>>>      r = amdgpu_sync_resv(adev, &job->sync, vm->root.base.bo >>>> <http://root.base.bo>->tbo.resv, >>>> -                owner); >>>> +                owner, false); >>>>      if (r) >>>>          goto error_free; >>>> diff --git a/include/uapi/drm/amdgpu_drm.h >>>> b/include/uapi/drm/amdgpu_drm.h >>>> index 69d64e5..96fcde8 100644 >>>> --- a/include/uapi/drm/amdgpu_drm.h >>>> +++ b/include/uapi/drm/amdgpu_drm.h >>>> @@ -91,6 +91,8 @@ extern "C" { >>>>  #define AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS   (1 << 5) >>>>  /* Flag that BO is always valid in this VM */ >>>>  #define AMDGPU_GEM_CREATE_VM_ALWAYS_VALID   (1 << 6) >>>> +/* Flag that BO sharing will be explicitly synchronized */ >>>> +#define AMDGPU_GEM_CREATE_EXPLICIT_SYNC       (1 << 7) >>>>  struct drm_amdgpu_gem_create_in { >>>>      /** the requested memory size */ >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
