drm/amdgpu: use dep_sync for CS dependency/syncobj
Subsystems (2)
PATCH 1/1 drm/amdgpu: use dep_sync for CS dependency/syncobj
Otherwise, they could be optimized by scheduled fence.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +2 -2
@@ -1078,7 +1078,7 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,amdgpu_ctx_put (ctx);return r; }else if (fence) { - r =amdgpu_sync_fence (p->adev, &p->job->sync, + r =amdgpu_sync_fence (p->adev, &p->job->dep_sync, fence);dma_fence_put (fence);amdgpu_ctx_put (ctx);
@@ -1103,7 +1103,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,if (r)return r; - r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); + r =amdgpu_sync_fence (p->adev, &p->job->dep_sync, fence);dma_fence_put (fence);return r;
Signature · 6 lines
2.14.1 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): [email protected]
Am 13.11.2017 um 03:53 schrieb Chunming Zhou:
Quoted 4 lines
Otherwise, they could be optimized by scheduled fence.
Change-Id I6857eee20aebeaad793d9fe4e1b5222f1be7470eSigned-off-by Chunming Zhou <[email protected]>
First of all patch is Reviewed-by: Christian König <[email protected]>.
Second do you remember why we did this? I have some brief memory in my head that a certain CTS test failed because we didn't completely synchronized between dependencies explicit added by a semaphore.
Then can we narrow this down into a unit test for libdrm? Probably not so easy to reproduce otherwise.
Thanks,
Christian.
Quoted 18 lines
--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1078,7 +1078,7 @@ static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,amdgpu_ctx_put (ctx);return r; }else if (fence) { - r =amdgpu_sync_fence (p->adev, &p->job->sync, + r =amdgpu_sync_fence (p->adev, &p->job->dep_sync, fence);dma_fence_put (fence);amdgpu_ctx_put (ctx);
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, · line 1109
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1103,7 +1103,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,if (r)return r; - r =amdgpu_sync_fence (p->adev, &p->job->sync, fence); + r =amdgpu_sync_fence (p->adev, &p->job->dep_sync, fence);dma_fence_put (fence);return r;
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): a664c7c5-68d0-b668-3504-935c54fb4c53-5C7GfCeVMHo@public.gmane.org
Quoted 7 lines · Christian König
Am 13.11.2017 um 03:53 schrieb Chunming Zhou:
Quoted 4 lines
Otherwise, they could be optimized by scheduled fence.
Change-Id I6857eee20aebeaad793d9fe4e1b5222f1be7470eSigned-off-by Chunming Zhou <[email protected]>First of all patch is Reviewed-by: Christian König <[email protected]>.
Thanks.
Quoted 2 lines
Second do you remember why we did this? I have some brief memory in my head that a certain CTS test failed because we didn't completely synchronized between dependencies explicit added by a semaphore.
Yes, exactly, vulkan cts could fail for some semaphore case, which is to sync two job with different context but same process and same engine. Although two jobs are series in hw ring, their executions are in parallel, which could result in hang.
Quoted 2 lines
Then can we narrow this down into a unit test for libdrm? Probably not so easy to reproduce otherwise.
Also yes, this is occasional issue, it's not very easy to reproduce.
Regards,
David Zhou
Quoted 22 lines
Thanks,
Christian.Quoted 18 lines
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1078,7 +1078,7 @@ static int amdgpu_cs_process_fence_dep(structamdgpu_cs_parser *p, amdgpu_ctx_put(ctx); return r; } else if (fence) { - r = amdgpu_sync_fence(p->adev, &p->job->sync, + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence);
amdgpu_ctx_put(ctx);
Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c static int · line 1109
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-0@@ -1103,7 +1103,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, if (r) return r; - r = amdgpu_sync_fence(p->adev, &p->job->sync, fence); + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence); return r;
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): 0b92394d-e893-b79e-9b8b-c164168eb7a8-5C7GfCeVMHo@public.gmane.org
Am 13.11.2017 um 09:05 schrieb Chunming Zhou:
Quoted 9 lines · Christian König
Quoted 6 lines
Am 13.11.2017 um 03:53 schrieb Chunming Zhou:
Quoted 4 lines
Otherwise, they could be optimized by scheduled fence.
Change-Id I6857eee20aebeaad793d9fe4e1b5222f1be7470eSigned-off-by Chunming Zhou <[email protected]>
Quoted 6 lines
Quoted 1 line
First of all patch is Reviewed-by: Christian König <[email protected]>.
Thanks.
Quoted 2 lines
Second do you remember why we did this? I have some brief memory in my head that a certain CTS test failed because we didn't completely synchronized between dependencies explicit added by a semaphore.
Yes, exactly, vulkan cts could fail for some semaphore case, which is to sync two job with different context but same process and same engine. Although two jobs are series in hw ring, their executions are in parallel, which could result in hang.
Ah, now I remember. Yeah the problem is that the two job executions overlap and we need to insert an pipeline sync between them.
Quoted 4 lines
Quoted 3 lines
Then can we narrow this down into a unit test for libdrm? Probably not so easy to reproduce otherwise.
Also yes, this is occasional issue, it's not very easy to reproduce.
Yeah, we would need to do something like job 1 writes a value A to memory location X using shaders, then job 2 write to the same location value B using the CP.
Then send both with a semaphore dependency between the two. If everything works like expected we see value B, but if we don't wait for the shaders to finish before running job 2 we see value A.
Do you have time to put all this into a unit tests? I think that would be important to make sure we don't break it again in the future.
Otherwise Andrey can probably take a look.
Regards,
Christian.
Quoted 36 lines
Regards,
David ZhouQuoted 22 lines
Thanks,
Christian.Quoted 18 lines
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1078,7 +1078,7 @@ static int amdgpu_cs_process_fence_dep(structamdgpu_cs_parser *p, amdgpu_ctx_put(ctx); return r; } else if (fence) { - r = amdgpu_sync_fence(p->adev, &p->job->sync, + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence);
amdgpu_ctx_put(ctx);Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c static int · line 1109
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-0@@ -1103,7 +1103,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, if (r) return r; - r = amdgpu_sync_fence(p->adev, &p->job->sync, fence); + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence); return r;
Quoted 4 lines
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
A message that is not in the archive (sent off-list or not yet received): b7c6d075-5882-b1d8-88d6-0e351b200b4c-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Quoted 24 lines · Christian König
Am 13.11.2017 um 09:05 schrieb Chunming Zhou:
Quoted 9 lines · Christian König
Quoted 6 lines
Am 13.11.2017 um 03:53 schrieb Chunming Zhou:
Quoted 4 lines
Otherwise, they could be optimized by scheduled fence.
Change-Id I6857eee20aebeaad793d9fe4e1b5222f1be7470eSigned-off-by Chunming Zhou <[email protected]>Quoted 6 lines
Quoted 1 line
First of all patch is Reviewed-by: Christian König <[email protected]>.
Thanks.
Quoted 2 lines
Second do you remember why we did this? I have some brief memory in my head that a certain CTS test failed because we didn't completely synchronized between dependencies explicit added by a semaphore.
Yes, exactly, vulkan cts could fail for some semaphore case, which is to sync two job with different context but same process and same engine. Although two jobs are series in hw ring, their executions are in parallel, which could result in hang.
Ah, now I remember. Yeah the problem is that the two job executions overlap and we need to insert an pipeline sync between them.
Quoted 4 lines
Quoted 3 lines
Then can we narrow this down into a unit test for libdrm? Probably not so easy to reproduce otherwise.
Also yes, this is occasional issue, it's not very easy to reproduce.
Quoted 7 lines
Yeah, we would need to do something like job 1 writes a value A to memory location X using shaders, then job 2 write to the same location value B using the CP.
Then send both with a semaphore dependency between the two. If everything works like expected we see value B, but if we don't wait for the shaders to finish before running job 2 we see value A.
Do you have time to put all this into a unit tests? I think that would be important to make sure we don't break it again in the future.
Otherwise Andrey can probably take a look.
OK, feel free to assign.
Thanks,
David Zhou
Quoted 46 lines
Regards,
Christian.Quoted 40 lines
Regards,
David ZhouQuoted 22 lines
Thanks,
Christian.Quoted 18 lines
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-1@@ -1078,7 +1078,7 @@ static int amdgpu_cs_process_fence_dep(structamdgpu_cs_parser *p, amdgpu_ctx_put(ctx); return r; } else if (fence) { - r = amdgpu_sync_fence(p->adev, &p->job->sync, + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence);
amdgpu_ctx_put(ctx);Quoted 10 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c static int · line 1109
Quoted 8 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +1
-0@@ -1103,7 +1103,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p, if (r) return r; - r = amdgpu_sync_fence(p->adev, &p->job->sync, fence); + r = amdgpu_sync_fence(p->adev, &p->job->dep_sync, fence); dma_fence_put(fence); return r;_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx