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.brezil...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
