* [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
@ 2017-09-19 12:59 Andres Rodriguez
[not found] ` <20170919125918.4132-1-andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Andres Rodriguez @ 2017-09-19 12:59 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q, andresx7-Re5JQEeQqe8AvxtiuMwx3w
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@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()
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->tbo.resv, owner);
+ amdgpu_sync_resv(adev, &sync, vm->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->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->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 */
--
2.9.3
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <20170919125918.4132-1-andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-19 13:24 ` Christian König
[not found] ` <fa69f268-541d-8729-48cf-93ae50d44035-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2017-09-19 13:24 UTC (permalink / raw)
To: Andres Rodriguez, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q
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@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?
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->tbo.resv, owner);
> + amdgpu_sync_resv(adev, &sync, vm->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->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->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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <fa69f268-541d-8729-48cf-93ae50d44035-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-19 13:57 ` Andres Rodriguez
[not found] ` <e1ba5ad3-72b7-3e3a-3588-d52a46a667f7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-21 14:38 ` Andres Rodriguez
1 sibling, 1 reply; 10+ messages in thread
From: Andres Rodriguez @ 2017-09-19 13:57 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q
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@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->tbo.resv, owner);
>> + amdgpu_sync_resv(adev, &sync, vm->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->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->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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <e1ba5ad3-72b7-3e3a-3588-d52a46a667f7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-19 14:10 ` Mao, David
[not found] ` <C3B2D770-2630-497A-B73F-231993566023-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Mao, David @ 2017-09-19 14:10 UTC (permalink / raw)
To: Andres Rodriguez
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q, Koenig, Christian,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 9514 bytes --]
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@gmail.com<mailto:andresx7@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@gmail.com<mailto:andresx7@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@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 34061 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <C3B2D770-2630-497A-B73F-231993566023-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-19 14:11 ` Andres Rodriguez
0 siblings, 0 replies; 10+ messages in thread
From: Andres Rodriguez @ 2017-09-19 14:11 UTC (permalink / raw)
To: Mao, David
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q, Koenig, Christian,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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@gmail.com
> <mailto:andresx7@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@gmail.com
>>>> <mailto:andresx7@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@lists.freedesktop.org <mailto: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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <fa69f268-541d-8729-48cf-93ae50d44035-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-19 13:57 ` Andres Rodriguez
@ 2017-09-21 14:38 ` Andres Rodriguez
[not found] ` <23e71499-cd99-7016-fb8e-a7029dadee51-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 10+ messages in thread
From: Andres Rodriguez @ 2017-09-21 14:38 UTC (permalink / raw)
To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q
Hi Christian,
The reference radv patches are on the list. The basic idea is to only
set the explicit sync flag for buffers allocated for dri usage.
Regards,
Andres
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@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?
>
> 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->tbo.resv, owner);
>> + amdgpu_sync_resv(adev, &sync, vm->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->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->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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <23e71499-cd99-7016-fb8e-a7029dadee51-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-21 18:14 ` Christian König
2017-10-12 18:27 ` Marek Olšák
1 sibling, 0 replies; 10+ messages in thread
From: Christian König @ 2017-09-21 18:14 UTC (permalink / raw)
To: Andres Rodriguez, christian.koenig-5C7GfCeVMHo,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: deathsimple-ANTagKRnAhcb1SvskN2V4Q
Patch is Reviewed-by: Christian König <christian.koenig@amd.com>.
Regards,
Christian.
Am 21.09.2017 um 16:38 schrieb Andres Rodriguez:
> Hi Christian,
>
> The reference radv patches are on the list. The basic idea is to only
> set the explicit sync flag for buffers allocated for dri usage.
>
> Regards,
> Andres
>
> 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@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?
>>
>> 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->tbo.resv, owner);
>>> + amdgpu_sync_resv(adev, &sync, vm->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->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->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@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <23e71499-cd99-7016-fb8e-a7029dadee51-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-21 18:14 ` Christian König
@ 2017-10-12 18:27 ` Marek Olšák
[not found] ` <CAAxE2A4MjBnDweD3kKZuSkxRGY9RZ1Z335Hm3XgZemZy0wz_qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 1 reply; 10+ messages in thread
From: Marek Olšák @ 2017-10-12 18:27 UTC (permalink / raw)
To: Andres Rodriguez
Cc: Christian König, Christian König, amd-gfx mailing list
On Thu, Sep 21, 2017 at 4:38 PM, Andres Rodriguez <andresx7@gmail.com> wrote:
> Hi Christian,
>
> The reference radv patches are on the list. The basic idea is to only set
> the explicit sync flag for buffers allocated for dri usage.
Did you mean "only set the explicit sync flag for buffers NOT
allocated for dri usage"?
Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <CAAxE2A4MjBnDweD3kKZuSkxRGY9RZ1Z335Hm3XgZemZy0wz_qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-10-12 18:30 ` Christian König
2017-10-12 18:31 ` Andres Rodriguez
1 sibling, 0 replies; 10+ messages in thread
From: Christian König @ 2017-10-12 18:30 UTC (permalink / raw)
To: Marek Olšák, Andres Rodriguez
Cc: Christian König, amd-gfx mailing list
Am 12.10.2017 um 20:27 schrieb Marek Olšák:
> On Thu, Sep 21, 2017 at 4:38 PM, Andres Rodriguez <andresx7@gmail.com> wrote:
>> Hi Christian,
>>
>> The reference radv patches are on the list. The basic idea is to only set
>> the explicit sync flag for buffers allocated for dri usage.
> Did you mean "only set the explicit sync flag for buffers NOT
> allocated for dri usage"?
I really hope so, cause otherwise that wouldn't be backward compatible.
But the name indicates that no implicit sync is requested any more, so I
think he got that right.
Christian.
>
> Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2
[not found] ` <CAAxE2A4MjBnDweD3kKZuSkxRGY9RZ1Z335Hm3XgZemZy0wz_qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-12 18:30 ` Christian König
@ 2017-10-12 18:31 ` Andres Rodriguez
1 sibling, 0 replies; 10+ messages in thread
From: Andres Rodriguez @ 2017-10-12 18:31 UTC (permalink / raw)
To: Marek Olšák
Cc: Christian König, Christian König, amd-gfx mailing list
On 2017-10-12 02:27 PM, Marek Olšák wrote:
> On Thu, Sep 21, 2017 at 4:38 PM, Andres Rodriguez <andresx7@gmail.com> wrote:
>> Hi Christian,
>>
>> The reference radv patches are on the list. The basic idea is to only set
>> the explicit sync flag for buffers allocated for dri usage.
>
> Did you mean "only set the explicit sync flag for buffers NOT
> allocated for dri usage"?
>
Yes, thanks for the correction. The dri buffers use implicit sync, the
rest of the buffers use explicit sync.
Regards,
Andres
> Marek
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-12 18:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 12:59 [PATCH] drm/amdgpu: introduce AMDGPU_GEM_CREATE_EXPLICIT_SYNC v2 Andres Rodriguez
[not found] ` <20170919125918.4132-1-andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-19 13:24 ` Christian König
[not found] ` <fa69f268-541d-8729-48cf-93ae50d44035-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-19 13:57 ` Andres Rodriguez
[not found] ` <e1ba5ad3-72b7-3e3a-3588-d52a46a667f7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-19 14:10 ` Mao, David
[not found] ` <C3B2D770-2630-497A-B73F-231993566023-5C7GfCeVMHo@public.gmane.org>
2017-09-19 14:11 ` Andres Rodriguez
2017-09-21 14:38 ` Andres Rodriguez
[not found] ` <23e71499-cd99-7016-fb8e-a7029dadee51-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-21 18:14 ` Christian König
2017-10-12 18:27 ` Marek Olšák
[not found] ` <CAAxE2A4MjBnDweD3kKZuSkxRGY9RZ1Z335Hm3XgZemZy0wz_qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-12 18:30 ` Christian König
2017-10-12 18:31 ` Andres Rodriguez
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.