[v3,11/25] panfrost: Use a pipe_framebuffer_state as the batch key

Submitted by Boris Brezillon on Sept. 5, 2019, 7:41 p.m.

Details

Message ID 20190905194150.17608-12-boris.brezillon@collabora.com
State New
Headers show
Series "panfrost: Rework the batch pipelining logic" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon Sept. 5, 2019, 7:41 p.m.
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@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(-)

Patch hide | download patch | download mbox

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);

Comments

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@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