[PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
Andres Rodriguez
andresx7 at gmail.com
Tue Sep 19 14:11:41 UTC 2017
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
More information about the amd-gfx
mailing list