[Mesa-dev] [PATCH v2 1/9] panfrost: Make ctx->job useful

[Juan A. Suarez Romero]

On Fri, 2019-08-02 at 19:18 +0200, Boris Brezillon wrote:
> ctx->job is supposed to serve as a cache to avoid an hash table lookup
> everytime we access the job attached to the currently bound FB, except
> it was never assigned to anything but NULL.
> 
> Fix that by adding the missing assignment in panfrost_get_job_for_fbo().
> Also add a missing NULL assignment in the ->set_framebuffer_state()
> path.
> 
> While at it, add extra assert()s to make sure ctx->job is consistent.
> 
> Fixes: 59c9623d0a75 ("panfrost: Import job data structures from v3d")
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> Changes in v2:
> * New patch (suggested by Alyssa)


This patch is a candidate for 19.1, but unfortunately it does not apply cleanly.
It seems it depends on a bunch of other commits not in 19.1

At this point, I will ignore/reject this patch for 19.1, unless you can provide
me a specific version for 19.1 branch (use staging/19.1 as reference).

Thanks.

	J.A.

> ---
>  src/gallium/drivers/panfrost/pan_context.c |  5 +++++
>  src/gallium/drivers/panfrost/pan_job.c     | 19 ++++++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 014f8f6a9d07..ba2df2ce66ae 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -2372,6 +2372,11 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
>                  panfrost_flush(pctx, NULL, PIPE_FLUSH_END_OF_FRAME);
>          }
>  
> +        /* Invalidate the FBO job cache since we've just been assigned a new
> +         * FB state.
> +         */
> +        ctx->job = NULL;
> +
>          util_copy_framebuffer_state(&ctx->pipe_framebuffer, fb);
>  
>          /* Given that we're rendering, we'd love to have compression */
> diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
> index 6339b39d29a0..cc81d475165e 100644
> --- a/src/gallium/drivers/panfrost/pan_job.c
> +++ b/src/gallium/drivers/panfrost/pan_job.c
> @@ -23,6 +23,8 @@
>   *
>   */
>  
> +#include <assert.h>
> +
>  #include "pan_context.h"
>  #include "util/hash_table.h"
>  #include "util/ralloc.h"
> @@ -124,8 +126,13 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
>  
>          /* If we already began rendering, use that */
>  
> -        if (ctx->job)
> +        if (ctx->job) {
> +                assert(ctx->job->key.zsbuf == ctx->pipe_framebuffer.zsbuf &&
> +                       !memcmp(ctx->job->key.cbufs,
> +                               ctx->pipe_framebuffer.cbufs,
> +                               sizeof(ctx->job->key.cbufs)));
>                  return ctx->job;
> +        }
>  
>          /* If not, look up the job */
>  
> @@ -133,6 +140,10 @@ panfrost_get_job_for_fbo(struct panfrost_context *ctx)
>          struct pipe_surface *zsbuf = ctx->pipe_framebuffer.zsbuf;
>          struct panfrost_job *job = panfrost_get_job(ctx, cbufs, zsbuf);
>  
> +        /* Set this job as the current FBO job. Will be reset when updating the
> +         * FB state and when submitting or releasing a job.
> +         */
> +        ctx->job = job;
>          return job;
>  }
>  
> @@ -181,6 +192,12 @@ panfrost_job_submit(struct panfrost_context *ctx, struct panfrost_job *job)
>  
>          if (ret)
>                  fprintf(stderr, "panfrost_job_submit failed: %d\n", ret);
> +
> +        /* The job has been submitted, let's invalidate the current FBO job
> +         * cache.
> +	 */
> +        assert(!ctx->job || job == ctx->job);
> +        ctx->job = NULL;
>  }
>  
>  void