2016-11-07 Christian König <deathsimple at vodafone.de>: > Am 07.11.2016 um 02:10 schrieb Gustavo Padovan: > > Hi Alex, > > > > 2016-11-04 Alex Deucher <alexdeucher at gmail.com>: > > > > > From: Junwei Zhang <Jerry.Zhang at 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 at amd.com> > > > Reviewed-by: Monk Liu <monk.liu at amd.com> > > > Reviewed-by: Jammy Zhou <Jammy.Zhou at amd.com> > > > Signed-off-by: Alex Deucher <alexander.deucher at 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