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

[Alyssa Rosenzweig]

@Boris I don't see why this patch particularly needs to be backported to
19.1?

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.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
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190808/c8969311/attachment.sig>