[Mesa-dev] [PATCH v3 03/17] panfrost: Add a batch fence

[Alyssa Rosenzweig]

R-b

On Wed, Sep 18, 2019 at 03:24:25PM +0200, Boris Brezillon wrote:
> So we can implement fine-grained dependency tracking between batches.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
> Changes in v3:
> * Fix typos
> * Do not initialize the syncobj in a signaled state, and set
>   fence->signaled to true when submitting a dummy batch (one with no
>   draw/clear queued)
> ---
>  src/gallium/drivers/panfrost/pan_job.c | 56 +++++++++++++++++++++++++-
>  src/gallium/drivers/panfrost/pan_job.h | 39 ++++++++++++++++++
>  2 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
> index 785317dbd0b0..b6763da66a97 100644
> --- a/src/gallium/drivers/panfrost/pan_job.c
> +++ b/src/gallium/drivers/panfrost/pan_job.c
> @@ -36,6 +36,45 @@
>  #include "pan_util.h"
>  #include "pandecode/decode.h"
>  
> +static struct panfrost_batch_fence *
> +panfrost_create_batch_fence(struct panfrost_batch *batch)
> +{
> +        struct panfrost_batch_fence *fence;
> +        ASSERTED int ret;
> +
> +        fence = rzalloc(NULL, struct panfrost_batch_fence);
> +        assert(fence);
> +        pipe_reference_init(&fence->reference, 1);
> +        fence->ctx = batch->ctx;
> +        fence->batch = batch;
> +        ret = drmSyncobjCreate(pan_screen(batch->ctx->base.screen)->fd, 0,
> +                               &fence->syncobj);
> +        assert(!ret);
> +
> +        return fence;
> +}
> +
> +static void
> +panfrost_free_batch_fence(struct panfrost_batch_fence *fence)
> +{
> +        drmSyncobjDestroy(pan_screen(fence->ctx->base.screen)->fd,
> +                          fence->syncobj);
> +        ralloc_free(fence);
> +}
> +
> +void
> +panfrost_batch_fence_unreference(struct panfrost_batch_fence *fence)
> +{
> +        if (pipe_reference(&fence->reference, NULL))
> +                 panfrost_free_batch_fence(fence);
> +}
> +
> +void
> +panfrost_batch_fence_reference(struct panfrost_batch_fence *fence)
> +{
> +        pipe_reference(NULL, &fence->reference);
> +}
> +
>  static struct panfrost_batch *
>  panfrost_create_batch(struct panfrost_context *ctx,
>                        const struct pipe_framebuffer_state *key)
> @@ -53,6 +92,7 @@ panfrost_create_batch(struct panfrost_context *ctx,
>  
>          util_dynarray_init(&batch->headers, batch);
>          util_dynarray_init(&batch->gpu_headers, batch);
> +        batch->out_sync = panfrost_create_batch_fence(batch);
>          util_copy_framebuffer_state(&batch->key, key);
>  
>          return batch;
> @@ -74,6 +114,15 @@ panfrost_free_batch(struct panfrost_batch *batch)
>          if (ctx->batch == batch)
>                  ctx->batch = NULL;
>  
> +        /* The out_sync fence lifetime is different from the the batch one
> +         * since other batches might want to wait on a fence of already
> +         * submitted/signaled batch. All we need to do here is make sure the
> +         * fence does not point to an invalid batch, which the core will
> +         * interpret as 'batch is already submitted'.
> +         */
> +        batch->out_sync->batch = NULL;
> +        panfrost_batch_fence_unreference(batch->out_sync);
> +
>          util_unreference_framebuffer_state(&batch->key);
>          ralloc_free(batch);
>  }
> @@ -441,8 +490,13 @@ panfrost_batch_submit(struct panfrost_batch *batch)
>          int ret;
>  
>          /* Nothing to do! */
> -        if (!batch->last_job.gpu && !batch->clear)
> +        if (!batch->last_job.gpu && !batch->clear) {
> +                /* Mark the fence as signaled so the fence logic does not try
> +                 * to wait on it.
> +                 */
> +                batch->out_sync->signaled = true;
>                  goto out;
> +        }
>  
>          if (!batch->clear && batch->last_tiler.gpu)
>                  panfrost_batch_draw_wallpaper(batch);
> diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h
> index 3f2cf1a999f3..88f1e4620fd0 100644
> --- a/src/gallium/drivers/panfrost/pan_job.h
> +++ b/src/gallium/drivers/panfrost/pan_job.h
> @@ -31,6 +31,36 @@
>  #include "pan_allocate.h"
>  #include "pan_resource.h"
>  
> +/* panfrost_batch_fence is the out fence of a batch that users or other batches
> + * might want to wait on. The batch fence lifetime is different from the batch
> + * one as want will certainly want to wait upon the fence after the batch has
> + * been submitted (which is when panfrost_batch objects are freed).
> + */
> +struct panfrost_batch_fence {
> +        /* Refcounting object for the fence. */
> +        struct pipe_reference reference;
> +
> +        /* Batch that created this fence object. Will become NULL at batch
> +         * submission time. This field is mainly here to know whether the
> +         * batch has been flushed or not.
> +         */
> +        struct panfrost_batch *batch;
> +
> +        /* Context this fence is attached to. We need both ctx and batch, as
> +         * the batch will go away after it's been submitted, but the fence
> +         * will stay a bit longer.
> +         */
> +        struct panfrost_context *ctx;
> +
> +        /* Sync object backing this fence. */
> +        uint32_t syncobj;
> +
> +        /* Cached value of the signaled state to avoid calling WAIT_SYNCOBJs
> +         * when we know the fence has already been signaled.
> +         */
> +        bool signaled;
> +};
> +
>  #define PAN_REQ_MSAA            (1 << 0)
>  #define PAN_REQ_DEPTH_WRITE     (1 << 1)
>  
> @@ -120,10 +150,19 @@ struct panfrost_batch {
>  
>          /* Framebuffer descriptor. */
>          mali_ptr framebuffer;
> +
> +        /* Output sync object. Only valid when submitted is true. */
> +        struct panfrost_batch_fence *out_sync;
>  };
>  
>  /* Functions for managing the above */
>  
> +void
> +panfrost_batch_fence_unreference(struct panfrost_batch_fence *fence);
> +
> +void
> +panfrost_batch_fence_reference(struct panfrost_batch_fence *batch);
> +
>  struct panfrost_batch *
>  panfrost_get_batch_for_fbo(struct panfrost_context *ctx);
>  
> -- 
> 2.21.0