Am 11.04.2017 um 05:18 schrieb Dave Airlie:
On 4 April 2017 at 21:05, Christian König <deathsimple@xxxxxxxxxxx> wrote:
Am 04.04.2017 um 10:10 schrieb Daniel Vetter:
On Tue, Apr 04, 2017 at 09:40:57AM +0200, Christian König wrote:
Am 04.04.2017 um 06:27 schrieb Dave Airlie:
From: Dave Airlie <airlied@xxxxxxxxxx>
This creates a new interface for amdgpu with ioctls to
create/destroy/import and export shared semaphores using
sem object backed by the sync_file code. The semaphores
are not installed as fd (except for export), but rather
like other driver internal objects in an idr. The idr
holds the initial reference on the sync file.
The command submission interface is enhanced with two new
chunks, one for semaphore waiting, one for semaphore signalling
and just takes a list of handles for each.
This is based on work originally done by David Zhou at AMD,
with input from Christian Konig on what things should look like.
NOTE: this interface addition needs a version bump to expose
it to userspace.
v1.1: keep file reference on import.
v2: move to using syncobjs
Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
Looks good to me in general, but one note below.
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 86
++++++++++++++++++++++++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
include/uapi/drm/amdgpu_drm.h | 6 +++
3 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 4671432..4dd210b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -27,6 +27,7 @@
#include <linux/pagemap.h>
#include <drm/drmP.h>
#include <drm/amdgpu_drm.h>
+#include <drm/drm_syncobj.h>
#include "amdgpu.h"
#include "amdgpu_trace.h"
@@ -217,6 +218,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser
*p, void *data)
break;
case AMDGPU_CHUNK_ID_DEPENDENCIES:
+ case AMDGPU_CHUNK_ID_SEM_WAIT:
+ case AMDGPU_CHUNK_ID_SEM_SIGNAL:
break;
default:
@@ -1009,6 +1012,44 @@ static int amdgpu_process_fence_dep(struct
amdgpu_device *adev,
return 0;
}
+static int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev,
+ struct drm_file *file_private,
+ struct amdgpu_sync *sync,
+ uint32_t handle)
+{
+ int r;
+ struct dma_fence *old_fence;
+ r = drm_syncobj_swap_fences(file_private, handle, NULL,
&old_fence);
What happens when we the CS fails later on? Would the semaphore then not
contain the fence any more?
Atm rules are that you're not allowed to publish a dma_fence before you're
committed to executing whatever it represents (i.e. guaranteed to get
signalled).
Yeah, I know. But this isn't the signaling case, but rather the waiting
case.
If I'm not completely mistaken what Dave does here is retrieving the fence
from a semaphore object we need to wait for and setting it to NULL.
If the command submission fails with -ERESTARTSYS after this is done the
retried IOCTL will behave incorrectly.
I don't think it will since the return ioctl will already have waited won't it?
No, it the IOCTL doesn't wait for anything. If you added a wait here
that would clearly be a NAK for the patch.
So once it's done the waiting, there is no point in waiting on resubmit.
We don't wait in the IOCTL, instead all the fences are put as
dependencies into the job and submit that to the GPU scheduler for
execution when all fences are done.
Christian.
Dave.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
