| Message ID | 1478290570-30982-2-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alex, 2016-11-04 Alex Deucher <alexdeucher@gmail.com>: > From: Junwei Zhang <Jerry.Zhang@amd.com> > > v2: agd: rebase and squash in all the previous optimizations and > changes so everything compiles. > v3: squash in Slava's 32bit build fix > v4: rebase on drm-next (fence -> dma_fence), > squash in Monk's ioctl update patch > > Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> > Reviewed-by: Monk Liu <monk.liu@amd.com> > Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- > include/uapi/drm/amdgpu_drm.h | 28 ++++++ > 5 files changed, 205 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index dc98ceb..7a94a3c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp); > int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); > int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); > +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, > + struct drm_file *filp); > > int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 2728805..2004836 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, > } > > /** > + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence > + * > + * @adev: amdgpu device > + * @filp: file private > + * @user: drm_amdgpu_fence copied from user space > + */ > +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev, > + struct drm_file *filp, > + struct drm_amdgpu_fence *user) > +{ > + struct amdgpu_ring *ring; > + struct amdgpu_ctx *ctx; > + struct dma_fence *fence; > + int r; > + > + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance, > + user->ring, &ring); > + if (r) > + return ERR_PTR(r); > + > + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id); > + if (ctx == NULL) > + return ERR_PTR(-EINVAL); > + > + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no); > + amdgpu_ctx_put(ctx); > + > + return fence; > +} > + > +/** > + * amdgpu_cs_wait_all_fence - wait on all fences to signal > + * > + * @adev: amdgpu device > + * @filp: file private > + * @wait: wait parameters > + * @fences: array of drm_amdgpu_fence > + */ > +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev, > + struct drm_file *filp, > + union drm_amdgpu_wait_fences *wait, > + struct drm_amdgpu_fence *fences) > +{ > + uint32_t fence_count = wait->in.fence_count; > + unsigned i; > + long r = 1; > + > + for (i = 0; i < fence_count; i++) { > + struct dma_fence *fence; > + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); > + > + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); > + if (IS_ERR(fence)) > + return PTR_ERR(fence); > + else if (!fence) > + continue; > + > + r = dma_fence_wait_timeout(fence, true, timeout); > + if (r < 0) > + return r; > + > + if (r == 0) > + break; > + } > + > + memset(wait, 0, sizeof(*wait)); > + wait->out.status = (r > 0); > + > + return 0; > +} > + > +/** > + * amdgpu_cs_wait_any_fence - wait on any fence to signal > + * > + * @adev: amdgpu device > + * @filp: file private > + * @wait: wait parameters > + * @fences: array of drm_amdgpu_fence > + */ > +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, > + struct drm_file *filp, > + union drm_amdgpu_wait_fences *wait, > + struct drm_amdgpu_fence *fences) > +{ > + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); > + uint32_t fence_count = wait->in.fence_count; > + uint32_t first = ~0; > + struct dma_fence **array; > + unsigned i; > + long r; > + > + /* Prepare the fence array */ > + array = (struct dma_fence **)kcalloc(fence_count, sizeof(struct dma_fence *), > + GFP_KERNEL); > + if (array == NULL) > + return -ENOMEM; > + > + for (i = 0; i < fence_count; i++) { > + struct dma_fence *fence; > + > + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); > + if (IS_ERR(fence)) { > + r = PTR_ERR(fence); > + goto err_free_fence_array; > + } else if (fence) { > + array[i] = fence; > + } else { /* NULL, the fence has been already signaled */ > + r = 1; > + goto out; > + } > + } > + > + r = dma_fence_wait_any_timeout(array, fence_count, true, timeout, &first); > + if (r < 0) > + goto err_free_fence_array; > + > +out: > + memset(wait, 0, sizeof(*wait)); > + wait->out.status = (r > 0); > + wait->out.first_signaled = first; > + /* set return value 0 to indicate success */ > + r = 0; > + > +err_free_fence_array: > + for (i = 0; i < fence_count; i++) > + dma_fence_put(array[i]); > + kfree(array); > + > + return r; > +} > + > +/** > + * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish > + * > + * @dev: drm device > + * @data: data from userspace > + * @filp: file private > + */ > +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, > + struct drm_file *filp) > +{ > + struct amdgpu_device *adev = dev->dev_private; > + union drm_amdgpu_wait_fences *wait = data; > + uint32_t fence_count = wait->in.fence_count; > + struct drm_amdgpu_fence *fences_user; > + struct drm_amdgpu_fence *fences; > + int r; > + > + /* Get the fences from userspace */ > + fences = kmalloc_array(fence_count, sizeof(struct drm_amdgpu_fence), > + GFP_KERNEL); > + if (fences == NULL) > + return -ENOMEM; > + > + fences_user = (void __user *)(unsigned long)(wait->in.fences); > + if (copy_from_user(fences, fences_user, > + sizeof(struct drm_amdgpu_fence) * fence_count)) { > + r = -EFAULT; > + goto err_free_fences; > + } > + > + if (wait->in.wait_all) > + r = amdgpu_cs_wait_all_fences(adev, filp, wait, fences); > + else > + r = amdgpu_cs_wait_any_fence(adev, filp, wait, fences); I wonder if it wouldn't be better if we use fence_array here and register callbacks to get notfied of the first signaled fence the "any" case. It seems to me that we could simplify this code by using a fence_array. Gustavo
Am 07.11.2016 um 02:10 schrieb Gustavo Padovan: > Hi Alex, > > 2016-11-04 Alex Deucher <alexdeucher@gmail.com>: > >> From: Junwei Zhang <Jerry.Zhang@amd.com> >> >> v2: agd: rebase and squash in all the previous optimizations and >> changes so everything compiles. >> v3: squash in Slava's 32bit build fix >> v4: rebase on drm-next (fence -> dma_fence), >> squash in Monk's ioctl update patch >> >> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> >> Reviewed-by: Monk Liu <monk.liu@amd.com> >> Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173 ++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- >> include/uapi/drm/amdgpu_drm.h | 28 ++++++ >> 5 files changed, 205 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> index dc98ceb..7a94a3c 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, >> struct drm_file *filp); >> int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); >> int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); >> +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *filp); >> >> int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, >> struct drm_file *filp); >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> index 2728805..2004836 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c >> @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, >> } >> >> /** >> + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence >> + * >> + * @adev: amdgpu device >> + * @filp: file private >> + * @user: drm_amdgpu_fence copied from user space >> + */ >> +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev, >> + struct drm_file *filp, >> + struct drm_amdgpu_fence *user) >> +{ >> + struct amdgpu_ring *ring; >> + struct amdgpu_ctx *ctx; >> + struct dma_fence *fence; >> + int r; >> + >> + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance, >> + user->ring, &ring); >> + if (r) >> + return ERR_PTR(r); >> + >> + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id); >> + if (ctx == NULL) >> + return ERR_PTR(-EINVAL); >> + >> + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no); >> + amdgpu_ctx_put(ctx); >> + >> + return fence; >> +} >> + >> +/** >> + * amdgpu_cs_wait_all_fence - wait on all fences to signal >> + * >> + * @adev: amdgpu device >> + * @filp: file private >> + * @wait: wait parameters >> + * @fences: array of drm_amdgpu_fence >> + */ >> +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev, >> + struct drm_file *filp, >> + union drm_amdgpu_wait_fences *wait, >> + struct drm_amdgpu_fence *fences) >> +{ >> + uint32_t fence_count = wait->in.fence_count; >> + unsigned i; >> + long r = 1; >> + >> + for (i = 0; i < fence_count; i++) { >> + struct dma_fence *fence; >> + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); >> + >> + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); >> + if (IS_ERR(fence)) >> + return PTR_ERR(fence); >> + else if (!fence) >> + continue; >> + >> + r = dma_fence_wait_timeout(fence, true, timeout); >> + if (r < 0) >> + return r; >> + >> + if (r == 0) >> + break; >> + } >> + >> + memset(wait, 0, sizeof(*wait)); >> + wait->out.status = (r > 0); >> + >> + return 0; >> +} >> + >> +/** >> + * amdgpu_cs_wait_any_fence - wait on any fence to signal >> + * >> + * @adev: amdgpu device >> + * @filp: file private >> + * @wait: wait parameters >> + * @fences: array of drm_amdgpu_fence >> + */ >> +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, >> + struct drm_file *filp, >> + union drm_amdgpu_wait_fences *wait, >> + struct drm_amdgpu_fence *fences) >> +{ >> + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); >> + uint32_t fence_count = wait->in.fence_count; >> + uint32_t first = ~0; >> + struct dma_fence **array; >> + unsigned i; >> + long r; >> + >> + /* Prepare the fence array */ >> + array = (struct dma_fence **)kcalloc(fence_count, sizeof(struct dma_fence *), >> + GFP_KERNEL); >> + if (array == NULL) >> + return -ENOMEM; >> + >> + for (i = 0; i < fence_count; i++) { >> + struct dma_fence *fence; >> + >> + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); >> + if (IS_ERR(fence)) { >> + r = PTR_ERR(fence); >> + goto err_free_fence_array; >> + } else if (fence) { >> + array[i] = fence; >> + } else { /* NULL, the fence has been already signaled */ >> + r = 1; >> + goto out; >> + } >> + } >> + >> + r = dma_fence_wait_any_timeout(array, fence_count, true, timeout, &first); >> + if (r < 0) >> + goto err_free_fence_array; >> + >> +out: >> + memset(wait, 0, sizeof(*wait)); >> + wait->out.status = (r > 0); >> + wait->out.first_signaled = first; >> + /* set return value 0 to indicate success */ >> + r = 0; >> + >> +err_free_fence_array: >> + for (i = 0; i < fence_count; i++) >> + dma_fence_put(array[i]); >> + kfree(array); >> + >> + return r; >> +} >> + >> +/** >> + * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish >> + * >> + * @dev: drm device >> + * @data: data from userspace >> + * @filp: file private >> + */ >> +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *filp) >> +{ >> + struct amdgpu_device *adev = dev->dev_private; >> + union drm_amdgpu_wait_fences *wait = data; >> + uint32_t fence_count = wait->in.fence_count; >> + struct drm_amdgpu_fence *fences_user; >> + struct drm_amdgpu_fence *fences; >> + int r; >> + >> + /* Get the fences from userspace */ >> + fences = kmalloc_array(fence_count, sizeof(struct drm_amdgpu_fence), >> + GFP_KERNEL); >> + if (fences == NULL) >> + return -ENOMEM; >> + >> + fences_user = (void __user *)(unsigned long)(wait->in.fences); >> + if (copy_from_user(fences, fences_user, >> + sizeof(struct drm_amdgpu_fence) * fence_count)) { >> + r = -EFAULT; >> + goto err_free_fences; >> + } >> + >> + if (wait->in.wait_all) >> + r = amdgpu_cs_wait_all_fences(adev, filp, wait, fences); >> + else >> + r = amdgpu_cs_wait_any_fence(adev, filp, wait, fences); > I wonder if it wouldn't be better if we use fence_array here and > register callbacks to get notfied of the first signaled fence the "any" case. > It seems to me that we could simplify this code by using a fence_array. I had this code in mind as well when working on the fence_array. But this code actually precedes the fence_array implementation, so I would like to push it upstream unchanged and then clean it up to use the fence array. That would make our backporting efforts a bit easier and shouldn't affect upstream to much in any way. Regards, Christian. > > Gustavo > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
2016-11-07 Christian König <deathsimple@vodafone.de>: > Am 07.11.2016 um 02:10 schrieb Gustavo Padovan: > > Hi Alex, > > > > 2016-11-04 Alex Deucher <alexdeucher@gmail.com>: > > > > > From: Junwei Zhang <Jerry.Zhang@amd.com> > > > > > > v2: agd: rebase and squash in all the previous optimizations and > > > changes so everything compiles. > > > v3: squash in Slava's 32bit build fix > > > v4: rebase on drm-next (fence -> dma_fence), > > > squash in Monk's ioctl update patch > > > > > > Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> > > > Reviewed-by: Monk Liu <monk.liu@amd.com> > > > Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com> > > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > > --- > > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + > > > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 173 ++++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 + > > > drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +- > > > include/uapi/drm/amdgpu_drm.h | 28 ++++++ > > > 5 files changed, 205 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > index dc98ceb..7a94a3c 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > > @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, > > > struct drm_file *filp); > > > int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); > > > int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); > > > +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, > > > + struct drm_file *filp); > > > int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, > > > struct drm_file *filp); > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > index 2728805..2004836 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > > > @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, > > > } > > > /** > > > + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence > > > + * > > > + * @adev: amdgpu device > > > + * @filp: file private > > > + * @user: drm_amdgpu_fence copied from user space > > > + */ > > > +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev, > > > + struct drm_file *filp, > > > + struct drm_amdgpu_fence *user) > > > +{ > > > + struct amdgpu_ring *ring; > > > + struct amdgpu_ctx *ctx; > > > + struct dma_fence *fence; > > > + int r; > > > + > > > + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance, > > > + user->ring, &ring); > > > + if (r) > > > + return ERR_PTR(r); > > > + > > > + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id); > > > + if (ctx == NULL) > > > + return ERR_PTR(-EINVAL); > > > + > > > + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no); > > > + amdgpu_ctx_put(ctx); > > > + > > > + return fence; > > > +} > > > + > > > +/** > > > + * amdgpu_cs_wait_all_fence - wait on all fences to signal > > > + * > > > + * @adev: amdgpu device > > > + * @filp: file private > > > + * @wait: wait parameters > > > + * @fences: array of drm_amdgpu_fence > > > + */ > > > +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev, > > > + struct drm_file *filp, > > > + union drm_amdgpu_wait_fences *wait, > > > + struct drm_amdgpu_fence *fences) > > > +{ > > > + uint32_t fence_count = wait->in.fence_count; > > > + unsigned i; > > > + long r = 1; > > > + > > > + for (i = 0; i < fence_count; i++) { > > > + struct dma_fence *fence; > > > + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); > > > + > > > + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); > > > + if (IS_ERR(fence)) > > > + return PTR_ERR(fence); > > > + else if (!fence) > > > + continue; > > > + > > > + r = dma_fence_wait_timeout(fence, true, timeout); > > > + if (r < 0) > > > + return r; > > > + > > > + if (r == 0) > > > + break; > > > + } > > > + > > > + memset(wait, 0, sizeof(*wait)); > > > + wait->out.status = (r > 0); > > > + > > > + return 0; > > > +} > > > + > > > +/** > > > + * amdgpu_cs_wait_any_fence - wait on any fence to signal > > > + * > > > + * @adev: amdgpu device > > > + * @filp: file private > > > + * @wait: wait parameters > > > + * @fences: array of drm_amdgpu_fence > > > + */ > > > +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, > > > + struct drm_file *filp, > > > + union drm_amdgpu_wait_fences *wait, > > > + struct drm_amdgpu_fence *fences) > > > +{ > > > + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); > > > + uint32_t fence_count = wait->in.fence_count; > > > + uint32_t first = ~0; > > > + struct dma_fence **array; > > > + unsigned i; > > > + long r; > > > + > > > + /* Prepare the fence array */ > > > + array = (struct dma_fence **)kcalloc(fence_count, sizeof(struct dma_fence *), > > > + GFP_KERNEL); > > > + if (array == NULL) > > > + return -ENOMEM; > > > + > > > + for (i = 0; i < fence_count; i++) { > > > + struct dma_fence *fence; > > > + > > > + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); > > > + if (IS_ERR(fence)) { > > > + r = PTR_ERR(fence); > > > + goto err_free_fence_array; > > > + } else if (fence) { > > > + array[i] = fence; > > > + } else { /* NULL, the fence has been already signaled */ > > > + r = 1; > > > + goto out; > > > + } > > > + } > > > + > > > + r = dma_fence_wait_any_timeout(array, fence_count, true, timeout, &first); > > > + if (r < 0) > > > + goto err_free_fence_array; > > > + > > > +out: > > > + memset(wait, 0, sizeof(*wait)); > > > + wait->out.status = (r > 0); > > > + wait->out.first_signaled = first; > > > + /* set return value 0 to indicate success */ > > > + r = 0; > > > + > > > +err_free_fence_array: > > > + for (i = 0; i < fence_count; i++) > > > + dma_fence_put(array[i]); > > > + kfree(array); > > > + > > > + return r; > > > +} > > > + > > > +/** > > > + * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish > > > + * > > > + * @dev: drm device > > > + * @data: data from userspace > > > + * @filp: file private > > > + */ > > > +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, > > > + struct drm_file *filp) > > > +{ > > > + struct amdgpu_device *adev = dev->dev_private; > > > + union drm_amdgpu_wait_fences *wait = data; > > > + uint32_t fence_count = wait->in.fence_count; > > > + struct drm_amdgpu_fence *fences_user; > > > + struct drm_amdgpu_fence *fences; > > > + int r; > > > + > > > + /* Get the fences from userspace */ > > > + fences = kmalloc_array(fence_count, sizeof(struct drm_amdgpu_fence), > > > + GFP_KERNEL); > > > + if (fences == NULL) > > > + return -ENOMEM; > > > + > > > + fences_user = (void __user *)(unsigned long)(wait->in.fences); > > > + if (copy_from_user(fences, fences_user, > > > + sizeof(struct drm_amdgpu_fence) * fence_count)) { > > > + r = -EFAULT; > > > + goto err_free_fences; > > > + } > > > + > > > + if (wait->in.wait_all) > > > + r = amdgpu_cs_wait_all_fences(adev, filp, wait, fences); > > > + else > > > + r = amdgpu_cs_wait_any_fence(adev, filp, wait, fences); > > I wonder if it wouldn't be better if we use fence_array here and > > register callbacks to get notfied of the first signaled fence the "any" case. > > It seems to me that we could simplify this code by using a fence_array. > > I had this code in mind as well when working on the fence_array. > > But this code actually precedes the fence_array implementation, so I would > like to push it upstream unchanged and then clean it up to use the fence > array. > > That would make our backporting efforts a bit easier and shouldn't affect > upstream to much in any way. That sounds good to me. Should add an extra patch to this patchset to do the conversion right away? Gustavo
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index dc98ceb..7a94a3c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1212,6 +1212,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp); int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 2728805..2004836 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1130,6 +1130,179 @@ int amdgpu_cs_wait_ioctl(struct drm_device *dev, void *data, } /** + * amdgpu_cs_get_fence - helper to get fence from drm_amdgpu_fence + * + * @adev: amdgpu device + * @filp: file private + * @user: drm_amdgpu_fence copied from user space + */ +static struct dma_fence *amdgpu_cs_get_fence(struct amdgpu_device *adev, + struct drm_file *filp, + struct drm_amdgpu_fence *user) +{ + struct amdgpu_ring *ring; + struct amdgpu_ctx *ctx; + struct dma_fence *fence; + int r; + + r = amdgpu_cs_get_ring(adev, user->ip_type, user->ip_instance, + user->ring, &ring); + if (r) + return ERR_PTR(r); + + ctx = amdgpu_ctx_get(filp->driver_priv, user->ctx_id); + if (ctx == NULL) + return ERR_PTR(-EINVAL); + + fence = amdgpu_ctx_get_fence(ctx, ring, user->seq_no); + amdgpu_ctx_put(ctx); + + return fence; +} + +/** + * amdgpu_cs_wait_all_fence - wait on all fences to signal + * + * @adev: amdgpu device + * @filp: file private + * @wait: wait parameters + * @fences: array of drm_amdgpu_fence + */ +static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev, + struct drm_file *filp, + union drm_amdgpu_wait_fences *wait, + struct drm_amdgpu_fence *fences) +{ + uint32_t fence_count = wait->in.fence_count; + unsigned i; + long r = 1; + + for (i = 0; i < fence_count; i++) { + struct dma_fence *fence; + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); + + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); + if (IS_ERR(fence)) + return PTR_ERR(fence); + else if (!fence) + continue; + + r = dma_fence_wait_timeout(fence, true, timeout); + if (r < 0) + return r; + + if (r == 0) + break; + } + + memset(wait, 0, sizeof(*wait)); + wait->out.status = (r > 0); + + return 0; +} + +/** + * amdgpu_cs_wait_any_fence - wait on any fence to signal + * + * @adev: amdgpu device + * @filp: file private + * @wait: wait parameters + * @fences: array of drm_amdgpu_fence + */ +static int amdgpu_cs_wait_any_fence(struct amdgpu_device *adev, + struct drm_file *filp, + union drm_amdgpu_wait_fences *wait, + struct drm_amdgpu_fence *fences) +{ + unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns); + uint32_t fence_count = wait->in.fence_count; + uint32_t first = ~0; + struct dma_fence **array; + unsigned i; + long r; + + /* Prepare the fence array */ + array = (struct dma_fence **)kcalloc(fence_count, sizeof(struct dma_fence *), + GFP_KERNEL); + if (array == NULL) + return -ENOMEM; + + for (i = 0; i < fence_count; i++) { + struct dma_fence *fence; + + fence = amdgpu_cs_get_fence(adev, filp, &fences[i]); + if (IS_ERR(fence)) { + r = PTR_ERR(fence); + goto err_free_fence_array; + } else if (fence) { + array[i] = fence; + } else { /* NULL, the fence has been already signaled */ + r = 1; + goto out; + } + } + + r = dma_fence_wait_any_timeout(array, fence_count, true, timeout, &first); + if (r < 0) + goto err_free_fence_array; + +out: + memset(wait, 0, sizeof(*wait)); + wait->out.status = (r > 0); + wait->out.first_signaled = first; + /* set return value 0 to indicate success */ + r = 0; + +err_free_fence_array: + for (i = 0; i < fence_count; i++) + dma_fence_put(array[i]); + kfree(array); + + return r; +} + +/** + * amdgpu_cs_wait_fences_ioctl - wait for multiple command submissions to finish + * + * @dev: drm device + * @data: data from userspace + * @filp: file private + */ +int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct amdgpu_device *adev = dev->dev_private; + union drm_amdgpu_wait_fences *wait = data; + uint32_t fence_count = wait->in.fence_count; + struct drm_amdgpu_fence *fences_user; + struct drm_amdgpu_fence *fences; + int r; + + /* Get the fences from userspace */ + fences = kmalloc_array(fence_count, sizeof(struct drm_amdgpu_fence), + GFP_KERNEL); + if (fences == NULL) + return -ENOMEM; + + fences_user = (void __user *)(unsigned long)(wait->in.fences); + if (copy_from_user(fences, fences_user, + sizeof(struct drm_amdgpu_fence) * fence_count)) { + r = -EFAULT; + goto err_free_fences; + } + + if (wait->in.wait_all) + r = amdgpu_cs_wait_all_fences(adev, filp, wait, fences); + else + r = amdgpu_cs_wait_any_fence(adev, filp, wait, fences); + +err_free_fences: + kfree(fences); + + return r; +} + +/** * amdgpu_cs_find_bo_va - find bo_va for VM address * * @parser: command submission parser context diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 8f48bed..d1cf9ac 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -825,6 +825,7 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_VA, amdgpu_gem_va_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c index fd26c4b..035f714 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c @@ -361,7 +361,7 @@ int amdgpu_sa_bo_new(struct amdgpu_sa_manager *sa_manager, if (count) { spin_unlock(&sa_manager->wq.lock); t = dma_fence_wait_any_timeout(fences, count, false, - MAX_SCHEDULE_TIMEOUT); + MAX_SCHEDULE_TIMEOUT, NULL); for (i = 0; i < count; ++i) dma_fence_put(fences[i]); diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index 4684f37..2191a9e 100644 --- a/include/uapi/drm/amdgpu_drm.h +++ b/include/uapi/drm/amdgpu_drm.h @@ -50,6 +50,7 @@ extern "C" { #define DRM_AMDGPU_WAIT_CS 0x09 #define DRM_AMDGPU_GEM_OP 0x10 #define DRM_AMDGPU_GEM_USERPTR 0x11 +#define DRM_AMDGPU_WAIT_FENCES 0x12 #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create) #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap) @@ -63,6 +64,7 @@ extern "C" { #define DRM_IOCTL_AMDGPU_WAIT_CS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_CS, union drm_amdgpu_wait_cs) #define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op) #define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr) +#define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences) #define AMDGPU_GEM_DOMAIN_CPU 0x1 #define AMDGPU_GEM_DOMAIN_GTT 0x2 @@ -307,6 +309,32 @@ union drm_amdgpu_wait_cs { struct drm_amdgpu_wait_cs_out out; }; +struct drm_amdgpu_fence { + __u32 ctx_id; + __u32 ip_type; + __u32 ip_instance; + __u32 ring; + __u64 seq_no; +}; + +struct drm_amdgpu_wait_fences_in { + /** This points to uint64_t * which points to fences */ + __u64 fences; + __u32 fence_count; + __u32 wait_all; + __u64 timeout_ns; +}; + +struct drm_amdgpu_wait_fences_out { + __u32 status; + __u32 first_signaled; +}; + +union drm_amdgpu_wait_fences { + struct drm_amdgpu_wait_fences_in in; + struct drm_amdgpu_wait_fences_out out; +}; + #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0 #define AMDGPU_GEM_OP_SET_PLACEMENT 1