On Thu, Mar 09, 2017 at 07:57:18PM +0200, Mikko Perttunen wrote:
Add support for sync file-based prefences and postfences to job submission. Fences are passed to the Host1x implementation.
Signed-off-by: Mikko Perttunen mperttunen@nvidia.com
drivers/gpu/drm/tegra/drm.c | 69 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 64dff8530403..bf4a2a13c17d 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -10,6 +10,7 @@ #include <linux/bitops.h> #include <linux/host1x.h> #include <linux/iommu.h> +#include <linux/sync_file.h>
#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> @@ -344,6 +345,7 @@ int tegra_drm_submit(struct tegra_drm_context *context, struct drm_tegra_submit *args, struct drm_device *drm, struct drm_file *file) {
- struct host1x *host1x = dev_get_drvdata(drm->dev->parent); unsigned int num_cmdbufs = args->num_cmdbufs; unsigned int num_relocs = args->num_relocs; unsigned int num_waitchks = args->num_waitchks;
@@ -361,6 +363,11 @@ int tegra_drm_submit(struct tegra_drm_context *context, if (args->num_syncpts != 1) return -EINVAL;
- /* Check for unrecognized flags */
- if (args->flags & ~(DRM_TEGRA_SUBMIT_WAIT_FENCE_FD |
DRM_TEGRA_SUBMIT_CREATE_FENCE_FD))
return -EINVAL;
- job = host1x_job_alloc(context->channel, args->num_cmdbufs, args->num_relocs, args->num_waitchks); if (!job)
@@ -372,19 +379,27 @@ int tegra_drm_submit(struct tegra_drm_context *context, job->class = context->client->base.class; job->serialize = true;
if (args->flags & DRM_TEGRA_SUBMIT_WAIT_FENCE_FD) {
job->prefence = sync_file_get_fence(args->fence);
if (!job->prefence) {
err = -ENOENT;
goto put_job;
}
}
while (num_cmdbufs) { struct drm_tegra_cmdbuf cmdbuf; struct host1x_bo *bo;
if (copy_from_user(&cmdbuf, cmdbufs, sizeof(cmdbuf))) { err = -EFAULT;
goto fail;
goto put_fence;
}
bo = host1x_bo_lookup(file, cmdbuf.handle); if (!bo) { err = -ENOENT;
goto fail;
goto put_fence;
}
host1x_job_add_gather(job, bo, cmdbuf.words, cmdbuf.offset);
@@ -398,19 +413,19 @@ int tegra_drm_submit(struct tegra_drm_context *context, &relocs[num_relocs], drm, file); if (err < 0)
goto fail;
goto put_fence;
}
if (copy_from_user(job->waitchk, waitchks, sizeof(*waitchks) * num_waitchks)) { err = -EFAULT;
goto fail;
goto put_fence;
}
if (copy_from_user(&syncpt, (void __user *)(uintptr_t)args->syncpts, sizeof(syncpt))) { err = -EFAULT;
goto fail;
goto put_fence;
}
job->is_addr_reg = context->client->ops->is_addr_reg;
@@ -423,20 +438,54 @@ int tegra_drm_submit(struct tegra_drm_context *context,
err = host1x_job_pin(job, context->client->base.dev); if (err)
goto fail;
goto put_fence;
err = host1x_job_submit(job); if (err)
goto fail_submit;
goto unpin_job;
Shouldn't all error-unwinding gotos after this jump to the unpin_job label as well? Seems like they all jump to put_fence instead, which I think would leave the job pinned on failure.
- args->fence = job->syncpt_end;
if (args->flags & DRM_TEGRA_SUBMIT_CREATE_FENCE_FD) {
struct dma_fence *fence;
struct sync_file *file;
fence = host1x_fence_create(
host1x, host1x_syncpt_get(host1x, job->syncpt_id),
job->syncpt_end);
if (!fence) {
err = -ENOMEM;
goto put_fence;
}
file = sync_file_create(fence);
if (!file) {
dma_fence_put(fence);
err = -ENOMEM;
goto put_fence;
}
err = get_unused_fd_flags(O_CLOEXEC);
if (err < 0) {
dma_fence_put(fence);
goto put_fence;
}
fd_install(err, file->file);
args->fence = err;
} else {
args->fence = job->syncpt_end;
}
if (job->prefence)
dma_fence_put(job->prefence);
host1x_job_put(job); return 0;
-fail_submit: +unpin_job: host1x_job_unpin(job); -fail: +put_fence:
- if (job->prefence)
dma_fence_put(job->prefence);
Since we already have a conditional to check for usage of fence, I'm wondering if we can simplify this a little and leave out the put_fence label altogether, like so:
unpin_job: host1x_job_unpin(job); put_job: if (job->prefence) dma_fence_put(job->prefence);
host1x_job_put(job);
Thierry