[Mesa-dev] [PATCH v3 11/25] panfrost: Use a pipe_framebuffer_state as the batch key

[Alyssa Rosenzweig]

Hrm. I'm not sure I'm 100% comfortable using a Gallium object for this,
since many of these properties could be inferred, but this is still
probably the best compromise for now, so R-b.

On Thu, Sep 05, 2019 at 09:41:36PM +0200, Boris Brezillon wrote:
> This way we have all the fb_state information directly attached to a
> batch and can pass only the batch to functions emitting CMDs, which is
> needed if we want to be able to queue CMDs to a batch that's not
> currently bound to the context.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  src/gallium/drivers/panfrost/pan_job.c | 34 +++++++-------------------
>  src/gallium/drivers/panfrost/pan_job.h |  5 ++--
>  2 files changed, 11 insertions(+), 28 deletions(-)
> 
> diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
> index 7c40bcee0fca..6b0f612bb156 100644
> --- a/src/gallium/drivers/panfrost/pan_job.c
> +++ b/src/gallium/drivers/panfrost/pan_job.c
> @@ -79,21 +79,10 @@ panfrost_free_batch(struct panfrost_batch *batch)
>  
>  struct panfrost_batch *
>  panfrost_get_batch(struct panfrost_context *ctx,
> -                 struct pipe_surface **cbufs, struct pipe_surface *zsbuf)
> +                   const struct pipe_framebuffer_state *key)
>  {
>          /* Lookup the job first */
> -
> -        struct panfrost_batch_key key = {
> -                .cbufs = {
> -                        cbufs[0],
> -                        cbufs[1],
> -                        cbufs[2],
> -                        cbufs[3],
> -                },
> -                .zsbuf = zsbuf
> -        };
> -
> -        struct hash_entry *entry = _mesa_hash_table_search(ctx->batches, &key);
> +        struct hash_entry *entry = _mesa_hash_table_search(ctx->batches, key);
>  
>          if (entry)
>                  return entry->data;
> @@ -103,8 +92,7 @@ panfrost_get_batch(struct panfrost_context *ctx,
>          struct panfrost_batch *batch = panfrost_create_batch(ctx);
>  
>          /* Save the created job */
> -
> -        memcpy(&batch->key, &key, sizeof(key));
> +        util_copy_framebuffer_state(&batch->key, key);
>          _mesa_hash_table_insert(ctx->batches, &batch->key, batch);
>  
>          return batch;
> @@ -124,18 +112,14 @@ panfrost_get_batch_for_fbo(struct panfrost_context *ctx)
>          /* If we already began rendering, use that */
>  
>          if (ctx->batch) {
> -                assert(ctx->batch->key.zsbuf == ctx->pipe_framebuffer.zsbuf &&
> -                       !memcmp(ctx->batch->key.cbufs,
> -                               ctx->pipe_framebuffer.cbufs,
> -                               sizeof(ctx->batch->key.cbufs)));
> +                assert(util_framebuffer_state_equal(&ctx->batch->key,
> +                                                    &ctx->pipe_framebuffer));
>                  return ctx->batch;
>          }
>  
>          /* If not, look up the job */
> -
> -        struct pipe_surface **cbufs = ctx->pipe_framebuffer.cbufs;
> -        struct pipe_surface *zsbuf = ctx->pipe_framebuffer.zsbuf;
> -        struct panfrost_batch *batch = panfrost_get_batch(ctx, cbufs, zsbuf);
> +        struct panfrost_batch *batch = panfrost_get_batch(ctx,
> +                                                          &ctx->pipe_framebuffer);
>  
>          /* Set this job as the current FBO job. Will be reset when updating the
>           * FB state and when submitting or releasing a job.
> @@ -390,13 +374,13 @@ panfrost_flush_jobs_reading_resource(struct panfrost_context *panfrost,
>  static bool
>  panfrost_batch_compare(const void *a, const void *b)
>  {
> -        return memcmp(a, b, sizeof(struct panfrost_batch_key)) == 0;
> +        return util_framebuffer_state_equal(a, b);
>  }
>  
>  static uint32_t
>  panfrost_batch_hash(const void *key)
>  {
> -        return _mesa_hash_data(key, sizeof(struct panfrost_batch_key));
> +        return _mesa_hash_data(key, sizeof(struct pipe_framebuffer_state));
>  }
>  
>  /* Given a new bounding rectangle (scissor), let the job cover the union of the
> diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h
> index c9f487871216..6d89603f8798 100644
> --- a/src/gallium/drivers/panfrost/pan_job.h
> +++ b/src/gallium/drivers/panfrost/pan_job.h
> @@ -46,7 +46,7 @@ struct panfrost_batch_key {
>  
>  struct panfrost_batch {
>          struct panfrost_context *ctx;
> -        struct panfrost_batch_key key;
> +        struct pipe_framebuffer_state key;
>  
>          /* Buffers cleared (PIPE_CLEAR_* bitmask) */
>          unsigned clear;
> @@ -127,8 +127,7 @@ panfrost_free_batch(struct panfrost_batch *batch);
>  
>  struct panfrost_batch *
>  panfrost_get_batch(struct panfrost_context *ctx,
> -                   struct pipe_surface **cbufs,
> -                   struct pipe_surface *zsbuf);
> +                   const struct pipe_framebuffer_state *key);
>  
>  struct panfrost_batch *
>  panfrost_get_batch_for_fbo(struct panfrost_context *ctx);
> -- 
> 2.21.0