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

Submitted by Boris Brezillon on Aug. 2, 2019, 10:12 a.m.

Details

Message ID 20190802101257.10495-3-boris.brezillon@collabora.com
State New
Headers show
Series "panfrost: Allocate the polygon lists on-demand" ( rev: 1 ) in Mesa

Commit Message

Boris Brezillon Aug. 2, 2019, 10:12 a.m.
This guarantees that new draws targetting the same framebuffer will
get a new job instance.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 src/gallium/drivers/panfrost/pan_job.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Patch hide | download patch | download mbox

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

Comments


On Fri, 2 Aug 2019 08:00:28 -0700
Alyssa Rosenzweig <alyssa.rosenzweig@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@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
> >