On 6/27/19 5:09 PM, Wladimir J. van der Laan wrote: > On Tue, Jun 04, 2019 at 01:39:29AM +0200, Marek Vasut wrote: >> Use hash table instead of ad-hoc arrays. >> >> Signed-off-by: Marek Vasut <marex@xxxxxxx> >> Cc: Christian Gmeiner <christian.gmeiner@xxxxxxxxx> >> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx> >> --- >> etnaviv/etnaviv_bo.c | 6 +++--- >> etnaviv/etnaviv_cmd_stream.c | 31 ++++++++++++++++++++++--------- >> etnaviv/etnaviv_priv.h | 17 ++++++++++------- >> 3 files changed, 35 insertions(+), 19 deletions(-) > > I do not know how to quantify the performance difference here, is using a hash > table faster than an ad-hoc array for the typically small sized arrays here? > I guess so (but this doubt is why I've never done this change myself). > > Reviewed-by: Wladimir J. van der Laan <laanwj@xxxxxxxxx> Maybe you want to look at https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1190 I updated this patch against mesa master, apparently the libdrm-etnaviv bits were folded into mesa now. >> 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; > > Do we have to catch integer overflow here? > It's very unlikely for there to have been 2^32 streams, but if it happens it might be good to at least > warn. I don't think so. If you allocated (2^(machine word size))-1 streams, you would exhaust your memory long before that. And this can actually roll over, since if you were to allocate streams one after the other and then free them, their numbers would just increment without colliding. I guess what could happen here would be something like, you allocate stream #0 , then allocate/free (2^(machine word size)) - 2 streams and then allocating the (2^(machine word size)) - 1th stream would end up allocating new stream with the same stream count (?). So maybe we should redo this and track streams with a hashmap (?). But that's way out of scope of this patch ; and would have to be fixed in freedreno too. -- Best regards, Marek Vasut _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
