Re: [Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
Ah, perhaps, yes. My bad. On Tue, Sep 17, 2019 at 12:18:17AM +0200, Boris Brezillon wrote: > On Mon, 16 Sep 2019 15:26:12 -0400 > Alyssa Rosenzweig wrote: > > > Well, the hash tables strongly assume you're not using NULLs for things. > > > > See _mesa_hash_table_set_deleted_key for how to change that behaviour. > > Maybe I'm missing something, but AFAICT it's the key field that requires > special care, not the data one. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
On Mon, 16 Sep 2019 15:26:12 -0400 Alyssa Rosenzweig wrote: > Well, the hash tables strongly assume you're not using NULLs for things. > > See _mesa_hash_table_set_deleted_key for how to change that behaviour. Maybe I'm missing something, but AFAICT it's the key field that requires special care, not the data one. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
Well, the hash tables strongly assume you're not using NULLs for things.
See _mesa_hash_table_set_deleted_key for how to change that behaviour.
On Mon, Sep 16, 2019 at 04:17:37PM +0200, Boris Brezillon wrote:
> On Mon, 16 Sep 2019 10:00:13 -0400
> Alyssa Rosenzweig 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
> > > ---
> > > 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, >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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
On Mon, 16 Sep 2019 10:00:13 -0400
Alyssa Rosenzweig 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
> > ---
> > 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, >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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
What if flags = 0?
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
> ---
> 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, >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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 23/37] panfrost: Make panfrost_batch->bos a hash table
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
---
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, >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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
