Rob Herring <r...@kernel.org> writes:
> It is necessary to reuse existing BOs when dmabufs are imported. There
> are 2 cases that need to be handled. dmabufs can be created/exported and
> imported by the same process and can be imported multiple times.
> Copying other drivers, add a hash table to track exported BOs so the
> BOs get reused.
>
> Cc: Eric Anholt <e...@anholt.net>
> Signed-off-by: Rob Herring <r...@kernel.org>
> ---
> With this and the fd hashing to get a single screen, the flickery screen
> is gone and Android is somewhat working. Several apps though hang, don't
> render, and then exit. I also see CMA allocation errors, but not
> correlating to the app problems.
>
> Also, flink names need similar hash table look-up as well. Maybe that's
> a don't care for vc4? In any case, I don't have the setup to test that.
>
> Rob
>
> src/gallium/drivers/vc4/vc4_bufmgr.c | 20 +++++++++++++++++++-
> src/gallium/drivers/vc4/vc4_bufmgr.h | 12 +++++++++++-
> src/gallium/drivers/vc4/vc4_screen.c | 15 +++++++++++++++
> src/gallium/drivers/vc4/vc4_screen.h | 3 +++
> 4 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/vc4/vc4_bufmgr.c
> b/src/gallium/drivers/vc4/vc4_bufmgr.c
> index 21e3bde..d91157b 100644
> --- a/src/gallium/drivers/vc4/vc4_bufmgr.c
> +++ b/src/gallium/drivers/vc4/vc4_bufmgr.c
> @@ -28,6 +28,7 @@
> #include <xf86drm.h>
> #include <xf86drmMode.h>
>
> +#include "util/u_hash_table.h"
> #include "util/u_memory.h"
> #include "util/ralloc.h"
>
> @@ -329,10 +330,19 @@ vc4_bo_open_handle(struct vc4_screen *screen,
> uint32_t winsys_stride,
> uint32_t handle, uint32_t size)
> {
> - struct vc4_bo *bo = CALLOC_STRUCT(vc4_bo);
> + struct vc4_bo *bo;
>
> assert(size);
>
> + pipe_mutex_lock(screen->bo_handles_mutex);
> +
> + bo = util_hash_table_get(screen->bo_handles,
> (void*)(uintptr_t)handle);
> + if (bo) {
> + pipe_reference(NULL, &bo->reference);
> + goto done;
> + }
> +
> + bo = CALLOC_STRUCT(vc4_bo);
> pipe_reference_init(&bo->reference, 1);
> bo->screen = screen;
> bo->handle = handle;
> @@ -347,6 +357,10 @@ vc4_bo_open_handle(struct vc4_screen *screen,
> bo->map = malloc(bo->size);
> #endif
>
> + util_hash_table_set(screen->bo_handles, (void *)(uintptr_t)handle,
> bo);
> +
> +done:
> + pipe_mutex_unlock(screen->bo_handles_mutex);
> return bo;
> }
>
> @@ -401,6 +415,10 @@ vc4_bo_get_dmabuf(struct vc4_bo *bo)
> }
> bo->private = false;
>
> + pipe_mutex_lock(bo->screen->bo_handles_mutex);
> + util_hash_table_set(bo->screen->bo_handles, (void
> *)(uintptr_t)bo->handle, bo);
> + pipe_mutex_unlock(bo->screen->bo_handles_mutex);
> +
> return fd;
> }
>
> diff --git a/src/gallium/drivers/vc4/vc4_bufmgr.h
> b/src/gallium/drivers/vc4/vc4_bufmgr.h
> index b77506e..0896b30 100644
> --- a/src/gallium/drivers/vc4/vc4_bufmgr.h
> +++ b/src/gallium/drivers/vc4/vc4_bufmgr.h
> @@ -25,6 +25,7 @@
> #define VC4_BUFMGR_H
>
> #include <stdint.h>
> +#include "util/u_hash_table.h"
> #include "util/u_inlines.h"
> #include "vc4_qir.h"
>
> @@ -87,11 +88,20 @@ vc4_bo_reference(struct vc4_bo *bo)
> static inline void
> vc4_bo_unreference(struct vc4_bo **bo)
> {
> + struct vc4_screen *screen;
> if (!*bo)
> return;
>
> - if (pipe_reference(&(*bo)->reference, NULL))
> + screen = (*bo)->screen;
> + pipe_mutex_lock(screen->bo_handles_mutex);
> +
> + if (pipe_reference(&(*bo)->reference, NULL)) {
> vc4_bo_last_unreference(*bo);
> + util_hash_table_remove(screen->bo_handles,
> + (void *)(uintptr_t)(*bo)->handle);I think you're use-after-freeing bo here. Just stick it before last_unreference()? > + } > + > + pipe_mutex_unlock(screen->bo_handles_mutex); > *bo = NULL; > } Taking a mutex on every unref sucks -- it's a *really* hot path, and it kind of defeats the point of doing these pipe_reference atomics. We should be able to skip the mutex in the bo->private case, since any flinked/dmabuf BO will be !private. Think you could give that a shot? Thanks for debugging! Note, I'm still on vacation, so I'll be slow at replying.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
