[Mesa-dev] [PATCH 2/9] panfrost: Remove job from ctx->jobs at submission time

[Boris Brezillon]

On Fri, 2 Aug 2019 08:00:28 -0700
Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com> wrote:

> What happens if the CPU creates jobs significiantly faster than the GPU
> processes them? Could you have four jobs for the same framebuffer in
> flight at once?

That's probably what will happen, yes.

> 
> At present, we do some heavy flushing so "no" but in the future we'll
> want to lax up for performance.

Limiting the number of in-flight jobs per FB would require some kind of
per-FB jobs tracking. But let's assume we want to limit ourselves to
one in-flight job per FB at first and keep in-flight jobs in the
ctx->jobs set. We'd need to add some kind of job->in_flight state that
we set to true when submitting, and then wait for the job fence to be
signaled before allocating a new job.

Note that before this patch, someone trying to get a new job for an FB
that already has a job running would actually get the old job object
and might start messing up with it. Not sure that's what we want.

> 
> On Fri, Aug 02, 2019 at 12:12:50PM +0200, Boris Brezillon wrote:
> > This guarantees that new draws targetting the same framebuffer will
> > get a new job instance.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> > ---
> >  src/gallium/drivers/panfrost/pan_job.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
> > index 960c8556e2f0..d2a4c8c3c600 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.c
> > +++ b/src/gallium/drivers/panfrost/pan_job.c
> > @@ -173,6 +173,14 @@ panfrost_job_submit(struct panfrost_context *ctx, struct panfrost_job *job)
> >  
> >          if (ret)
> >                  fprintf(stderr, "panfrost_job_submit failed: %d\n", ret);
> > +
> > +        /* Remove the job from the ctx->jobs set so that future
> > +         * panfrost_get_job() calls don't see it.
> > +         * We must reset the job key to avoid removing another valid entry when
> > +         * the job is freed.
> > +         */
> > +        _mesa_hash_table_remove_key(ctx->jobs, &job->key);
> > +        memset(&job->key, 0, sizeof(job->key));
> >  }
> >  
> >  void
> > -- 
> > 2.21.0
> >