[PATCH libdrm] etnaviv: Use hash table to track BO indexes

[Christian Gmeiner]

Am Di., 4. Juni 2019 um 01:40 Uhr schrieb Marek Vasut <marex at denx.de>:
>
> Use hash table instead of ad-hoc arrays.
>

Please re-spin this patch in mesa on top of latest master. If you see
a real benefit for setups where
newest libdrm with older mesa gets used I will push this patch too.

> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Christian Gmeiner <christian.gmeiner at gmail.com>
> Cc: Lucas Stach <l.stach at pengutronix.de>
> ---
>  etnaviv/etnaviv_bo.c         |  6 +++---
>  etnaviv/etnaviv_cmd_stream.c | 31 ++++++++++++++++++++++---------
>  etnaviv/etnaviv_priv.h       | 17 ++++++++++-------
>  3 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/etnaviv/etnaviv_bo.c b/etnaviv/etnaviv_bo.c
> index 43ce6b4e..28ad3162 100644
> --- a/etnaviv/etnaviv_bo.c
> +++ b/etnaviv/etnaviv_bo.c
> @@ -44,14 +44,14 @@ drm_private void bo_del(struct etna_bo *bo)
>         if (bo->map)
>                 drm_munmap(bo->map, bo->size);
>
> -       if (bo->name)
> -               drmHashDelete(bo->dev->name_table, bo->name);
> -
>         if (bo->handle) {
>                 struct drm_gem_close req = {
>                         .handle = bo->handle,
>                 };
>
> +               if (bo->name)
> +                       drmHashDelete(bo->dev->name_table, bo->name);
> +
>                 drmHashDelete(bo->dev->handle_table, bo->handle);
>                 drmIoctl(bo->dev->fd, DRM_IOCTL_GEM_CLOSE, &req);
>         }
> diff --git a/etnaviv/etnaviv_cmd_stream.c b/etnaviv/etnaviv_cmd_stream.c
> index 261777b0..f550b2ff 100644
> --- a/etnaviv/etnaviv_cmd_stream.c
> +++ b/etnaviv/etnaviv_cmd_stream.c
> @@ -61,6 +61,7 @@ drm_public struct etna_cmd_stream *etna_cmd_stream_new(struct etna_pipe *pipe,
>                 void *priv)
>  {
>         struct etna_cmd_stream_priv *stream = NULL;
> +       struct etna_device *dev = pipe->gpu->dev;
>
>         if (size == 0) {
>                 ERROR_MSG("invalid size of 0");
> @@ -86,6 +87,7 @@ drm_public struct etna_cmd_stream *etna_cmd_stream_new(struct etna_pipe *pipe,
>         stream->pipe = pipe;
>         stream->reset_notify = reset_notify;
>         stream->reset_notify_priv = priv;
> +       stream->seqno = ++dev->stream_cnt;
>
>         return &stream->base;
>
> @@ -150,18 +152,24 @@ static uint32_t bo2idx(struct etna_cmd_stream *stream, struct etna_bo *bo,
>
>         pthread_mutex_lock(&idx_lock);
>
> -       if (bo->current_stream == stream) {
> +       if (bo->current_stream_seqno == priv->seqno) {
>                 idx = bo->idx;
>         } else {
> -               /* slow-path: */
> -               for (idx = 0; idx < priv->nr_bos; idx++)
> -                       if (priv->bos[idx] == bo)
> -                               break;
> -               if (idx == priv->nr_bos) {
> -                       /* not found */
> +               void *val;
> +
> +               if (!priv->bo_table)
> +                       priv->bo_table = drmHashCreate();

Would it make sense to move this to etna_cmd_stream_new(..)? I am not
sure if there is ever a stream
without any bo attached to it.

> +
> +               if (!drmHashLookup(priv->bo_table, bo->handle, &val)) {
> +                       /* found */
> +                       idx = (uint32_t)(uintptr_t)val;
> +               } else {
>                         idx = append_bo(stream, bo);
> +                       val = (void *)(uintptr_t)idx;
> +                       drmHashInsert(priv->bo_table, bo->handle, val);
>                 }
> -               bo->current_stream = stream;
> +
> +               bo->current_stream_seqno = priv->seqno;
>                 bo->idx = idx;
>         }
>         pthread_mutex_unlock(&idx_lock);
> @@ -213,10 +221,15 @@ static void flush(struct etna_cmd_stream *stream, int in_fence_fd,
>         for (uint32_t i = 0; i < priv->nr_bos; i++) {
>                 struct etna_bo *bo = priv->bos[i];
>
> -               bo->current_stream = NULL;
> +               bo->current_stream_seqno = 0;
>                 etna_bo_del(bo);
>         }
>
> +       if (priv->bo_table) {
> +               drmHashDestroy(priv->bo_table);
> +               priv->bo_table = NULL;
> +       }
> +
>         if (out_fence_fd)
>                 *out_fence_fd = req.fence_fd;
>  }
> diff --git a/etnaviv/etnaviv_priv.h b/etnaviv/etnaviv_priv.h
> index eef7f49c..bc82324e 100644
> --- a/etnaviv/etnaviv_priv.h
> +++ b/etnaviv/etnaviv_priv.h
> @@ -76,6 +76,8 @@ struct etna_device {
>         struct etna_bo_cache bo_cache;
>
>         int closefd;        /* call close(fd) upon destruction */
> +
> +       unsigned int stream_cnt;
>  };
>
>  drm_private void etna_bo_cache_init(struct etna_bo_cache *cache);
> @@ -98,14 +100,12 @@ struct etna_bo {
>         uint64_t        offset;         /* offset to mmap() */
>         atomic_t        refcnt;
>
> -       /* in the common case, a bo won't be referenced by more than a single
> -        * command stream.  So to avoid looping over all the bo's in the
> -        * reloc table to find the idx of a bo that might already be in the
> -        * table, we cache the idx in the bo.  But in order to detect the
> -        * slow-path where bo is ref'd in multiple streams, we also must track
> -        * the current_stream for which the idx is valid.  See bo2idx().
> +       /*
> +        * To avoid excess hashtable lookups, cache the stream this bo was
> +        * last emitted on (since that will probably also be the next ring
> +        * it is emitted on).
>          */
> -       struct etna_cmd_stream *current_stream;
> +       unsigned int current_stream_seqno;
>         uint32_t idx;
>
>         int reuse;
> @@ -153,6 +153,9 @@ struct etna_cmd_stream_priv {
>         /* notify callback if buffer reset happened */
>         void (*reset_notify)(struct etna_cmd_stream *stream, void *priv);
>         void *reset_notify_priv;
> +
> +       unsigned int seqno;
> +       void *bo_table;
>  };
>
>  struct etna_perfmon {
> --
> 2.20.1
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy