[PATCH 4/4] amdgpu: use sync file for shared semaphores
From: Dave Airlie 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. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 203 include/uapi/drm/amdgpu_drm.h | 28 + 6 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 2814aad..404bcba 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index c1b9135..4ed8811 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -702,6 +702,8 @@ struct amdgpu_fpriv { struct mutexbo_list_lock; struct idr bo_list_handles; struct amdgpu_ctx_mgr ctx_mgr; + spinlock_t sem_handles_lock; + struct idr sem_handles; }; /* @@ -1814,5 +1816,15 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, uint64_t addr, struct amdgpu_bo **bo); int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser); +int amdgpu_sem_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +void amdgpu_sem_destroy(struct amdgpu_fpriv *fpriv, u32 handle); +int amdgpu_sem_lookup_and_signal(struct amdgpu_fpriv *fpriv, +uint32_t handle, +struct dma_fence *fence); +int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev, + struct amdgpu_fpriv *fpriv, + struct amdgpu_sync *sync, + uint32_t handle); #include "amdgpu_object.h" #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 4671432..80fc94b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -217,6 +217,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 +1011,28 @@ static int amdgpu_process_fence_dep(struct amdgpu_device *adev, return 0; } +static int amdgpu_process_sem_wait_dep(struct amdgpu_device *adev, + struct amdgpu_cs_parser *p, + struct amdgpu_cs_chunk *chunk) +{ + struct amdgpu_fpriv *fpriv = p->filp->driver_priv; + unsigned num_deps; + int i, r; + struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_sem); + + for (i = 0; i < num_deps; ++i) { + r = amdgpu_sem_lookup_and_sync(adev, fpriv, &p->job->sync, + deps[i].handle); + if (r) + return r; + } + return 0; +} + static int amdgpu_cs_dependencies(struct amdgpu_device *adev, struct amdgpu_cs_parser *p) { @@ -1023,12 +1047,56 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r = amdgpu_process_fence_dep(adev, p, chunk); if (r)
Re: [PATCH 4/4] amdgpu: use sync file for shared semaphores
On Wed, Mar 15, 2017 at 10:43:01AM +0100, Christian König wrote: > Am 15.03.2017 um 10:01 schrieb Daniel Vetter: > > On Tue, Mar 14, 2017 at 10:50:54AM +1000, Dave Airlie wrote: > > > From: Dave Airlie > > > > > > 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. > > > > > > Signed-off-by: Dave Airlie > > Another uapi corner case: Since you allow semaphores to be created before > > they have a first fence attached, that essentially creates future fences. > > I.e. sync_file fd with no fence yet attached. We want that anyway, but > > maybe not together with semaphores (since there's some more implications). > > > > I think it'd be good to attach a dummy fence which already starts out as > > signalled to sync_file->fence for semaphores. That way userspace can't go > > evil, create a semaphore, then pass it to a driver which then promptly > > oopses or worse because it assumes then when it has a sync_file, it also > > has a fence. And the dummy fence could be globally shared, so not really > > overhead. > > Sounds fishy to me, what happens in case of a race condition and the > receiver sometimes waits on the dummy and sometimes on the real fence? > > I would rather teach the users of sync_file to return a proper error code > when you want to wait on a sync_file which doesn't have a fence yet. Races in userspace are races in userspace :-) And it's only a problem initially when you use a semaphore for the first time. After that you still have the same race, but because there's always a fence attached, your userspace won't ever notice the fail. It will simply complete immediately (because most likely the old fence has completed already). And it also doesn't mesh with the rough future fence plan, where the idea is that sync_file_get_fence blocks until the fence is there, and only drivers who can handle the risk of a fence loop (through hangcheck and hang recovery) will use a __get_fence function to peek at the future fence. I think the assumption that a sync_file always comes with a fence is a good principle for code robustness in the kernel. If you really want the userspace debugging, we can make it a module option, or even better, sprinkle a pile of tracepoints all over fences to make it easier to watch. But when the tradeoff is between userspace debugging and kernel robustness, I think we need to go with kernel robustness, for security reasons and all that. -Daniel > > Christian. > > > -Daniel > > > > > --- > > > drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ > > > drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 204 > > > > > > include/uapi/drm/amdgpu_drm.h | 28 + > > > 6 files changed, 322 insertions(+), 2 deletions(-) > > > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > > > b/drivers/gpu/drm/amd/amdgpu/Makefile > > > index 2814aad..404bcba 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/Makefile > > > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > > > @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ > > > atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ > > > amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ > > > amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ > > > - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o > > > + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o > > > # add asic specific block > > > amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > index c1b9135..4ed8811 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > @@ -702,6 +702,8 @@ struct amdgpu_fpriv { > > > struct mutexbo_list_lock; > > > struct idr bo_list_handles; > > > struct amdgpu_ctx_mgr ctx_mgr; > > > + spinlock_t sem_handles_lock; > > > + struct idr sem_handl
Re: [PATCH 4/4] amdgpu: use sync file for shared semaphores
Am 15.03.2017 um 10:01 schrieb Daniel Vetter: On Tue, Mar 14, 2017 at 10:50:54AM +1000, Dave Airlie wrote: From: Dave Airlie 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. Signed-off-by: Dave Airlie Another uapi corner case: Since you allow semaphores to be created before they have a first fence attached, that essentially creates future fences. I.e. sync_file fd with no fence yet attached. We want that anyway, but maybe not together with semaphores (since there's some more implications). I think it'd be good to attach a dummy fence which already starts out as signalled to sync_file->fence for semaphores. That way userspace can't go evil, create a semaphore, then pass it to a driver which then promptly oopses or worse because it assumes then when it has a sync_file, it also has a fence. And the dummy fence could be globally shared, so not really overhead. Sounds fishy to me, what happens in case of a race condition and the receiver sometimes waits on the dummy and sometimes on the real fence? I would rather teach the users of sync_file to return a proper error code when you want to wait on a sync_file which doesn't have a fence yet. Christian. -Daniel --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 204 include/uapi/drm/amdgpu_drm.h | 28 + 6 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 2814aad..404bcba 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index c1b9135..4ed8811 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -702,6 +702,8 @@ struct amdgpu_fpriv { struct mutexbo_list_lock; struct idr bo_list_handles; struct amdgpu_ctx_mgr ctx_mgr; + spinlock_t sem_handles_lock; + struct idr sem_handles; }; /* @@ -1814,5 +1816,15 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, uint64_t addr, struct amdgpu_bo **bo); int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser); +int amdgpu_sem_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +void amdgpu_sem_destroy(struct amdgpu_fpriv *fpriv, u32 handle); +int amdgpu_sem_lookup_and_signal(struct amdgpu_fpriv *fpriv, +uint32_t handle, +struct dma_fence *fence); +int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev, + struct amdgpu_fpriv *fpriv, + struct amdgpu_sync *sync, + uint32_t handle); #include "amdgpu_object.h" #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 4671432..80fc94b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -217,6 +217,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 +1011,28 @@ static int amdgpu_process_fence_dep(struct amdgpu_device *adev, return 0; } +static int amdgpu_process_sem_wait_dep(struct amdgpu_device *adev, +
Re: [PATCH 4/4] amdgpu: use sync file for shared semaphores
On Tue, Mar 14, 2017 at 10:50:54AM +1000, Dave Airlie wrote: > From: Dave Airlie > > 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. > > Signed-off-by: Dave Airlie Another uapi corner case: Since you allow semaphores to be created before they have a first fence attached, that essentially creates future fences. I.e. sync_file fd with no fence yet attached. We want that anyway, but maybe not together with semaphores (since there's some more implications). I think it'd be good to attach a dummy fence which already starts out as signalled to sync_file->fence for semaphores. That way userspace can't go evil, create a semaphore, then pass it to a driver which then promptly oopses or worse because it assumes then when it has a sync_file, it also has a fence. And the dummy fence could be globally shared, so not really overhead. -Daniel > --- > drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 204 > > include/uapi/drm/amdgpu_drm.h | 28 + > 6 files changed, 322 insertions(+), 2 deletions(-) > create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c > > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > b/drivers/gpu/drm/amd/amdgpu/Makefile > index 2814aad..404bcba 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Makefile > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ > atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ > amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ > amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ > - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o > + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o > > # add asic specific block > amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index c1b9135..4ed8811 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -702,6 +702,8 @@ struct amdgpu_fpriv { > struct mutexbo_list_lock; > struct idr bo_list_handles; > struct amdgpu_ctx_mgr ctx_mgr; > + spinlock_t sem_handles_lock; > + struct idr sem_handles; > }; > > /* > @@ -1814,5 +1816,15 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, > uint64_t addr, struct amdgpu_bo **bo); > int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser); > > +int amdgpu_sem_ioctl(struct drm_device *dev, void *data, > + struct drm_file *filp); > +void amdgpu_sem_destroy(struct amdgpu_fpriv *fpriv, u32 handle); > +int amdgpu_sem_lookup_and_signal(struct amdgpu_fpriv *fpriv, > + uint32_t handle, > + struct dma_fence *fence); > +int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev, > +struct amdgpu_fpriv *fpriv, > +struct amdgpu_sync *sync, > +uint32_t handle); > #include "amdgpu_object.h" > #endif > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 4671432..80fc94b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -217,6 +217,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 +1011,28 @@ static int amdgpu_process_fence_dep(struct > amdgpu_device *adev, > return 0; > } > > +static int amdgpu_process_sem_wait_dep(struct amdgpu_device *adev, > +struct amdgpu_cs_parser *p, > +struct amdgpu_cs_chunk *chunk) > +{ > + struct amdgpu_fpriv *fpriv = p->filp->driver_priv; > + unsigne
[PATCH 4/4] amdgpu: use sync file for shared semaphores
From: Dave Airlie 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. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 203 include/uapi/drm/amdgpu_drm.h | 28 + 6 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 2814aad..404bcba 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index c1b9135..4ed8811 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -702,6 +702,8 @@ struct amdgpu_fpriv { struct mutexbo_list_lock; struct idr bo_list_handles; struct amdgpu_ctx_mgr ctx_mgr; + spinlock_t sem_handles_lock; + struct idr sem_handles; }; /* @@ -1814,5 +1816,15 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, uint64_t addr, struct amdgpu_bo **bo); int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser); +int amdgpu_sem_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +void amdgpu_sem_destroy(struct amdgpu_fpriv *fpriv, u32 handle); +int amdgpu_sem_lookup_and_signal(struct amdgpu_fpriv *fpriv, +uint32_t handle, +struct dma_fence *fence); +int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev, + struct amdgpu_fpriv *fpriv, + struct amdgpu_sync *sync, + uint32_t handle); #include "amdgpu_object.h" #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 4671432..80fc94b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -217,6 +217,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 +1011,28 @@ static int amdgpu_process_fence_dep(struct amdgpu_device *adev, return 0; } +static int amdgpu_process_sem_wait_dep(struct amdgpu_device *adev, + struct amdgpu_cs_parser *p, + struct amdgpu_cs_chunk *chunk) +{ + struct amdgpu_fpriv *fpriv = p->filp->driver_priv; + unsigned num_deps; + int i, r; + struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_sem); + + for (i = 0; i < num_deps; ++i) { + r = amdgpu_sem_lookup_and_sync(adev, fpriv, &p->job->sync, + deps[i].handle); + if (r) + return r; + } + return 0; +} + static int amdgpu_cs_dependencies(struct amdgpu_device *adev, struct amdgpu_cs_parser *p) { @@ -1023,12 +1047,56 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r = amdgpu_process_fence_dep(adev, p, chunk); if (r)
[PATCH 4/4] amdgpu: use sync file for shared semaphores
From: Dave Airlie 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. Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 12 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 70 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c | 204 include/uapi/drm/amdgpu_drm.h | 28 + 6 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sem.c diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 2814aad..404bcba 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -24,7 +24,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ atombios_encoders.o amdgpu_sa.o atombios_i2c.o \ amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \ amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \ - amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o + amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_sem.o # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index c1b9135..4ed8811 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -702,6 +702,8 @@ struct amdgpu_fpriv { struct mutexbo_list_lock; struct idr bo_list_handles; struct amdgpu_ctx_mgr ctx_mgr; + spinlock_t sem_handles_lock; + struct idr sem_handles; }; /* @@ -1814,5 +1816,15 @@ amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, uint64_t addr, struct amdgpu_bo **bo); int amdgpu_cs_sysvm_access_required(struct amdgpu_cs_parser *parser); +int amdgpu_sem_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +void amdgpu_sem_destroy(struct amdgpu_fpriv *fpriv, u32 handle); +int amdgpu_sem_lookup_and_signal(struct amdgpu_fpriv *fpriv, +uint32_t handle, +struct dma_fence *fence); +int amdgpu_sem_lookup_and_sync(struct amdgpu_device *adev, + struct amdgpu_fpriv *fpriv, + struct amdgpu_sync *sync, + uint32_t handle); #include "amdgpu_object.h" #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 4671432..80fc94b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -217,6 +217,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 +1011,28 @@ static int amdgpu_process_fence_dep(struct amdgpu_device *adev, return 0; } +static int amdgpu_process_sem_wait_dep(struct amdgpu_device *adev, + struct amdgpu_cs_parser *p, + struct amdgpu_cs_chunk *chunk) +{ + struct amdgpu_fpriv *fpriv = p->filp->driver_priv; + unsigned num_deps; + int i, r; + struct drm_amdgpu_cs_chunk_sem *deps; + + deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata; + num_deps = chunk->length_dw * 4 / + sizeof(struct drm_amdgpu_cs_chunk_sem); + + for (i = 0; i < num_deps; ++i) { + r = amdgpu_sem_lookup_and_sync(adev, fpriv, &p->job->sync, + deps[i].handle); + if (r) + return r; + } + return 0; +} + static int amdgpu_cs_dependencies(struct amdgpu_device *adev, struct amdgpu_cs_parser *p) { @@ -1023,12 +1047,56 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev, r = amdgpu_process_fence_dep(adev, p, chunk); if (r) return r; + }