On Thu, 8 Aug 2019 06:08:05 -0700
Alyssa Rosenzweig <alyssa.rosenzw...@collabora.com> wrote:
> @Boris I don't see why this patch particularly needs to be backported to
> 19.1?
Nope, no reason to backport it to 19.1. It's just a (bad?) kernel dev
habit to put "Fixes:" tag the patch fixes a problem introduced by a
previous commit. But it does not necessarily means "backport to stable"
in mind.
>
> On Thu, Aug 08, 2019 at 11:46:43AM +0200, Juan A. Suarez Romero wrote:
> > 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
