[Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table

[Boris Brezillon]

On Mon, 16 Sep 2019 10:00:13 -0400
Alyssa Rosenzweig <alyssa at rosenzweig.io> wrote:

> What if flags = 0?

Not sure what you have in mind. 0 would be a valid value (though not
really useful since that just means the BO is private and we don't give
any information on the type of access done on this BO). If you're
worried about having hentry->data = (uintptr_t)0 (IOW hentry->data =
NULL), I don't see the problem.

> 
> On Mon, Sep 16, 2019 at 11:37:01AM +0200, Boris Brezillon wrote:
> > So we can store the flags as data and keep the BO as a key. This way
> > we keep track of the type of access done on BOs.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> > ---
> >  src/gallium/drivers/panfrost/pan_job.c | 33 +++++++++++++++++---------
> >  src/gallium/drivers/panfrost/pan_job.h |  2 +-
> >  2 files changed, 23 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
> > index 6332529b2f9b..739f36a593f1 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.c
> > +++ b/src/gallium/drivers/panfrost/pan_job.c
> > @@ -44,9 +44,8 @@ panfrost_create_batch(struct panfrost_context *ctx,
> >  
> >          batch->ctx = ctx;
> >  
> > -        batch->bos = _mesa_set_create(batch,
> > -                                      _mesa_hash_pointer,
> > -                                      _mesa_key_pointer_equal);
> > +        batch->bos = _mesa_hash_table_create(batch, _mesa_hash_pointer,
> > +                                             _mesa_key_pointer_equal);
> >  
> >          batch->minx = batch->miny = ~0;
> >          batch->maxx = batch->maxy = 0;
> > @@ -67,10 +66,8 @@ panfrost_free_batch(struct panfrost_batch *batch)
> >  
> >          struct panfrost_context *ctx = batch->ctx;
> >  
> > -        set_foreach(batch->bos, entry) {
> > -                struct panfrost_bo *bo = (struct panfrost_bo *)entry->key;
> > -                panfrost_bo_unreference(bo);
> > -        }
> > +        hash_table_foreach(batch->bos, entry)
> > +                panfrost_bo_unreference((struct panfrost_bo *)entry->key);
> >  
> >          _mesa_hash_table_remove_key(ctx->batches, &batch->key);
> >  
> > @@ -138,11 +135,25 @@ panfrost_batch_add_bo(struct panfrost_batch *batch, struct panfrost_bo *bo,
> >          if (!bo)
> >                  return;
> >  
> > -        if (_mesa_set_search(batch->bos, bo))
> > +        struct hash_entry *entry;
> > +        uint32_t old_flags = 0;
> > +
> > +        entry = _mesa_hash_table_search(batch->bos, bo);
> > +        if (!entry) {
> > +                entry = _mesa_hash_table_insert(batch->bos, bo,
> > +                                                (void *)(uintptr_t)flags);
> > +                panfrost_bo_reference(bo);
> > +	} else {
> > +                old_flags = (uintptr_t)entry->data;
> > +        }
> > +
> > +        assert(entry);
> > +
> > +        if (old_flags == flags)
> >                  return;
> >  
> > -        panfrost_bo_reference(bo);
> > -        _mesa_set_add(batch->bos, bo);
> > +        flags |= old_flags;
> > +        entry->data = (void *)(uintptr_t)flags;
> >  }
> >  
> >  void panfrost_batch_add_fbo_bos(struct panfrost_batch *batch)
> > @@ -376,7 +387,7 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
> >          bo_handles = calloc(batch->bos->entries, sizeof(*bo_handles));
> >          assert(bo_handles);
> >  
> > -        set_foreach(batch->bos, entry) {
> > +        hash_table_foreach(batch->bos, entry) {
> >                  struct panfrost_bo *bo = (struct panfrost_bo *)entry->key;
> >                  assert(bo->gem_handle > 0);
> >                  bo_handles[submit.bo_handle_count++] = bo->gem_handle;
> > diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h
> > index 0b37a3131e86..3f2cf1a999f3 100644
> > --- a/src/gallium/drivers/panfrost/pan_job.h
> > +++ b/src/gallium/drivers/panfrost/pan_job.h
> > @@ -98,7 +98,7 @@ struct panfrost_batch {
> >          unsigned job_index;
> >  
> >          /* BOs referenced -- will be used for flushing logic */
> > -        struct set *bos;
> > +        struct hash_table *bos;
> >  
> >          /* Current transient BO */
> >  	struct panfrost_bo *transient_bo;
> > -- 
> > 2.21.0