Re: [Mesa-dev] [PATCH v3 04/17] panfrost: Use the per-batch fences to wait on the last submitted batch
R-b
On Wed, Sep 18, 2019 at 03:24:26PM +0200, Boris Brezillon wrote:
> We just replace the per-context out_sync object by a pointer to the
> the fence of the last last submitted batch. Pipelining of batches will
> come later.
>
> Signed-off-by: Boris Brezillon
> ---
> Alyssa, I dropped your R-b since the other changes you asked me to do
> in "panfrost: Add a batch fence" had some impact on this patch.
>
> Changes in v3:
> * Make sure we don't try to wait on dummy batches (those with no
> vertex/tiler/fragment jobs)
> ---
> src/gallium/drivers/panfrost/pan_context.c | 6
> src/gallium/drivers/panfrost/pan_context.h | 3 +-
> src/gallium/drivers/panfrost/pan_job.c | 35 ++
> src/gallium/drivers/panfrost/pan_screen.c | 18 +--
> 4 files changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c
> b/src/gallium/drivers/panfrost/pan_context.c
> index 65a6c7f8c5ae..312a9e93e455 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -2702,12 +2702,6 @@ panfrost_create_context(struct pipe_screen *screen,
> void *priv, unsigned flags)
> panfrost_blend_context_init(gallium);
> panfrost_compute_context_init(gallium);
>
> -ASSERTED int ret;
> -
> -ret = drmSyncobjCreate(pscreen->fd, DRM_SYNCOBJ_CREATE_SIGNALED,
> - >out_sync);
> -assert(!ret);
> -
> /* XXX: leaks */
> gallium->stream_uploader = u_upload_create_default(gallium);
> gallium->const_uploader = gallium->stream_uploader;
> diff --git a/src/gallium/drivers/panfrost/pan_context.h
> b/src/gallium/drivers/panfrost/pan_context.h
> index c145d589757e..ce3e0c899a4f 100644
> --- a/src/gallium/drivers/panfrost/pan_context.h
> +++ b/src/gallium/drivers/panfrost/pan_context.h
> @@ -191,7 +191,8 @@ struct panfrost_context {
> /* True for t6XX, false for t8xx. */
> bool is_t6xx;
>
> -uint32_t out_sync;
> +/* The out sync fence of the last submitted batch. */
> +struct panfrost_batch_fence *last_out_sync;
> };
>
> /* Corresponds to the CSO */
> diff --git a/src/gallium/drivers/panfrost/pan_job.c
> b/src/gallium/drivers/panfrost/pan_job.c
> index b6763da66a97..55780dd3d9d6 100644
> --- a/src/gallium/drivers/panfrost/pan_job.c
> +++ b/src/gallium/drivers/panfrost/pan_job.c
> @@ -425,11 +425,13 @@ panfrost_batch_submit_ioctl(struct panfrost_batch
> *batch,
> uint32_t *bo_handles;
> int ret;
>
> -submit.in_syncs = (u64) (uintptr_t) >out_sync;
> -submit.in_sync_count = 1;
>
> -submit.out_sync = ctx->out_sync;
> +if (ctx->last_out_sync) {
> +submit.in_sync_count = 1;
> +submit.in_syncs = (uintptr_t)>last_out_sync->syncobj;
> +}
>
> +submit.out_sync = batch->out_sync->syncobj;
> submit.jc = first_job_desc;
> submit.requirements = reqs;
>
> @@ -445,6 +447,14 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
> submit.bo_handles = (u64) (uintptr_t) bo_handles;
> ret = drmIoctl(screen->fd, DRM_IOCTL_PANFROST_SUBMIT, );
> free(bo_handles);
> +
> +/* Release the last batch fence if any, and retain the new one */
> +if (ctx->last_out_sync)
> +panfrost_batch_fence_unreference(ctx->last_out_sync);
> +
> +panfrost_batch_fence_reference(batch->out_sync);
> +ctx->last_out_sync = batch->out_sync;
> +
> if (ret) {
> fprintf(stderr, "Error submitting: %m\n");
> return errno;
> @@ -453,7 +463,8 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
> /* Trace the job if we're doing that */
> if (pan_debug & PAN_DBG_TRACE) {
> /* Wait so we can get errors reported back */
> -drmSyncobjWait(screen->fd, >out_sync, 1, INT64_MAX, 0,
> NULL);
> +drmSyncobjWait(screen->fd, >out_sync->syncobj, 1,
> + INT64_MAX, 0, NULL);
> pandecode_jc(submit.jc, FALSE);
> }
>
> @@ -495,6 +506,15 @@ panfrost_batch_submit(struct panfrost_batch *batch)
> * to wait on it.
> */
> batch->out_sync->signaled = true;
> +
> +/* Release the last batch fence if any, and set
> ->last_out_sync
> + * to NULL
> + */
> +if (ctx->last_out_sync) {
> +panfrost_batch_fence_unreference(ctx->last_out_sync);
> +ctx->last_out_sync = NULL;
> +}
> +
> goto out;
> }
>
> @@ -527,8 +547,11 @@ out:
> * rendering is quite broken right now (to be fixed by the
> panfrost_job
> * refactor, just take the perf hit for correctness)
> */
> -
[Mesa-dev] [PATCH v3 04/17] panfrost: Use the per-batch fences to wait on the last submitted batch
We just replace the per-context out_sync object by a pointer to the
the fence of the last last submitted batch. Pipelining of batches will
come later.
Signed-off-by: Boris Brezillon
---
Alyssa, I dropped your R-b since the other changes you asked me to do
in "panfrost: Add a batch fence" had some impact on this patch.
Changes in v3:
* Make sure we don't try to wait on dummy batches (those with no
vertex/tiler/fragment jobs)
---
src/gallium/drivers/panfrost/pan_context.c | 6
src/gallium/drivers/panfrost/pan_context.h | 3 +-
src/gallium/drivers/panfrost/pan_job.c | 35 ++
src/gallium/drivers/panfrost/pan_screen.c | 18 +--
4 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/src/gallium/drivers/panfrost/pan_context.c
b/src/gallium/drivers/panfrost/pan_context.c
index 65a6c7f8c5ae..312a9e93e455 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -2702,12 +2702,6 @@ panfrost_create_context(struct pipe_screen *screen, void
*priv, unsigned flags)
panfrost_blend_context_init(gallium);
panfrost_compute_context_init(gallium);
-ASSERTED int ret;
-
-ret = drmSyncobjCreate(pscreen->fd, DRM_SYNCOBJ_CREATE_SIGNALED,
- >out_sync);
-assert(!ret);
-
/* XXX: leaks */
gallium->stream_uploader = u_upload_create_default(gallium);
gallium->const_uploader = gallium->stream_uploader;
diff --git a/src/gallium/drivers/panfrost/pan_context.h
b/src/gallium/drivers/panfrost/pan_context.h
index c145d589757e..ce3e0c899a4f 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -191,7 +191,8 @@ struct panfrost_context {
/* True for t6XX, false for t8xx. */
bool is_t6xx;
-uint32_t out_sync;
+/* The out sync fence of the last submitted batch. */
+struct panfrost_batch_fence *last_out_sync;
};
/* Corresponds to the CSO */
diff --git a/src/gallium/drivers/panfrost/pan_job.c
b/src/gallium/drivers/panfrost/pan_job.c
index b6763da66a97..55780dd3d9d6 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -425,11 +425,13 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
uint32_t *bo_handles;
int ret;
-submit.in_syncs = (u64) (uintptr_t) >out_sync;
-submit.in_sync_count = 1;
-submit.out_sync = ctx->out_sync;
+if (ctx->last_out_sync) {
+submit.in_sync_count = 1;
+submit.in_syncs = (uintptr_t)>last_out_sync->syncobj;
+}
+submit.out_sync = batch->out_sync->syncobj;
submit.jc = first_job_desc;
submit.requirements = reqs;
@@ -445,6 +447,14 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
submit.bo_handles = (u64) (uintptr_t) bo_handles;
ret = drmIoctl(screen->fd, DRM_IOCTL_PANFROST_SUBMIT, );
free(bo_handles);
+
+/* Release the last batch fence if any, and retain the new one */
+if (ctx->last_out_sync)
+panfrost_batch_fence_unreference(ctx->last_out_sync);
+
+panfrost_batch_fence_reference(batch->out_sync);
+ctx->last_out_sync = batch->out_sync;
+
if (ret) {
fprintf(stderr, "Error submitting: %m\n");
return errno;
@@ -453,7 +463,8 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
/* Trace the job if we're doing that */
if (pan_debug & PAN_DBG_TRACE) {
/* Wait so we can get errors reported back */
-drmSyncobjWait(screen->fd, >out_sync, 1, INT64_MAX, 0,
NULL);
+drmSyncobjWait(screen->fd, >out_sync->syncobj, 1,
+ INT64_MAX, 0, NULL);
pandecode_jc(submit.jc, FALSE);
}
@@ -495,6 +506,15 @@ panfrost_batch_submit(struct panfrost_batch *batch)
* to wait on it.
*/
batch->out_sync->signaled = true;
+
+/* Release the last batch fence if any, and set ->last_out_sync
+ * to NULL
+ */
+if (ctx->last_out_sync) {
+panfrost_batch_fence_unreference(ctx->last_out_sync);
+ctx->last_out_sync = NULL;
+}
+
goto out;
}
@@ -527,8 +547,11 @@ out:
* rendering is quite broken right now (to be fixed by the panfrost_job
* refactor, just take the perf hit for correctness)
*/
-drmSyncobjWait(pan_screen(ctx->base.screen)->fd, >out_sync, 1,
- INT64_MAX, 0, NULL);
+if (!batch->out_sync->signaled)
+drmSyncobjWait(pan_screen(ctx->base.screen)->fd,
+ >out_sync->syncobj, 1, INT64_MAX, 0,
+
