[Mesa-dev] [PATCH v4 07/11] amdgpu: use common screen ref counting
Marek Olšák
maraeo at gmail.com
Fri Jul 29 17:51:43 UTC 2016
The fd table and reference counting in the winsys is required by the
GL-VDPAU interop.
radeon_drm_winsys_create and amdgpu_winsys_create are publicly
exported by both *_dri.so and libvdpau_*.so, and whichever is loaded
first will effectively provide the gallium driver implementation for
both of them. The second loaded lib can't create its own winsys &
screen & contexts because of the public *_winsys_create functions
always invoking the first loaded lib.
Given that, I'm rejecting patches 7 & 8, because they obviously break
the GL-VDPAU interop by ignoring this ld-based inter-lib interaction
that's very important to us.
It looks like Nouveau relies on the same interaction too.
Marek
On Fri, Jul 22, 2016 at 6:22 PM, Rob Herring <robh at kernel.org> wrote:
> Use the common pipe_screen ref count. amdgpu is unique in its hashing
> the dev pointer rather than the fd, so the common fd hashing cannot be
> used. However, the same reference count can be used instead of the
> private one. The mutex can be dropped as the pipe loader protects the
> create_screen() calls.
>
> Signed-off-by: Rob Herring <robh at kernel.org>
> Cc: "Marek Olšák" <marek.olsak at amd.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 45 ++++-----------------------
> src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 1 -
> 2 files changed, 6 insertions(+), 40 deletions(-)
>
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> index 9a04cbe..27293ac 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
> @@ -60,8 +60,6 @@
> #define CIK__PIPE_CONFIG__ADDR_SURF_P16_32X32_16X16 17
>
> static struct util_hash_table *dev_tab = NULL;
> -pipe_static_mutex(dev_tab_mutex);
> -
> static unsigned cik_get_num_tile_pipes(struct amdgpu_gpu_info *info)
> {
> unsigned mode2d = info->gb_tile_mode[CIK_TILE_MODE_COLOR_2D];
> @@ -329,6 +327,7 @@ static void amdgpu_winsys_destroy(struct radeon_winsys *rws)
> pipe_mutex_destroy(ws->global_bo_list_lock);
> AddrDestroy(ws->addrlib);
> amdgpu_device_deinitialize(ws->dev);
> + util_hash_table_remove(dev_tab, ws->dev);
> FREE(rws);
> }
>
> @@ -410,26 +409,6 @@ static int compare_dev(void *key1, void *key2)
>
> DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)
>
> -static bool amdgpu_winsys_unref(struct radeon_winsys *rws)
> -{
> - struct amdgpu_winsys *ws = (struct amdgpu_winsys*)rws;
> - bool destroy;
> -
> - /* When the reference counter drops to zero, remove the device pointer
> - * from the table.
> - * This must happen while the mutex is locked, so that
> - * amdgpu_winsys_create in another thread doesn't get the winsys
> - * from the table when the counter drops to 0. */
> - pipe_mutex_lock(dev_tab_mutex);
> -
> - destroy = pipe_reference(&ws->reference, NULL);
> - if (destroy && dev_tab)
> - util_hash_table_remove(dev_tab, ws->dev);
> -
> - pipe_mutex_unlock(dev_tab_mutex);
> - return destroy;
> -}
> -
> PUBLIC struct radeon_winsys *
> amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> {
> @@ -446,7 +425,6 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> drmFreeVersion(version);
>
> /* Look up the winsys from the dev table. */
> - pipe_mutex_lock(dev_tab_mutex);
> if (!dev_tab)
> dev_tab = util_hash_table_create(hash_dev, compare_dev);
>
> @@ -454,7 +432,6 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> * for the same fd. */
> r = amdgpu_device_initialize(fd, &drm_major, &drm_minor, &dev);
> if (r) {
> - pipe_mutex_unlock(dev_tab_mutex);
> fprintf(stderr, "amdgpu: amdgpu_device_initialize failed.\n");
> return NULL;
> }
> @@ -462,17 +439,14 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> /* Lookup a winsys if we have already created one for this device. */
> ws = util_hash_table_get(dev_tab, dev);
> if (ws) {
> - pipe_reference(NULL, &ws->reference);
> - pipe_mutex_unlock(dev_tab_mutex);
> + pipe_reference(NULL, &ws->base.screen->reference);
> return &ws->base;
> }
>
> /* Create a new winsys. */
> ws = CALLOC_STRUCT(amdgpu_winsys);
> - if (!ws) {
> - pipe_mutex_unlock(dev_tab_mutex);
> + if (!ws)
> return NULL;
> - }
>
> ws->dev = dev;
> ws->info.drm_major = drm_major;
> @@ -486,11 +460,7 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> (ws->info.vram_size + ws->info.gart_size) / 8,
> amdgpu_bo_destroy, amdgpu_bo_can_reclaim);
>
> - /* init reference */
> - pipe_reference_init(&ws->reference, 1);
> -
> /* Set functions. */
> - ws->base.unref = amdgpu_winsys_unref;
> ws->base.destroy = amdgpu_winsys_destroy;
> ws->base.query_info = amdgpu_winsys_query_info;
> ws->base.cs_request_feature = amdgpu_cs_request_feature;
> @@ -516,21 +486,18 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
> ws->base.screen = screen_create(&ws->base);
> if (!ws->base.screen) {
> amdgpu_winsys_destroy(&ws->base);
> - pipe_mutex_unlock(dev_tab_mutex);
> return NULL;
> }
>
> util_hash_table_set(dev_tab, dev, ws);
>
> - /* We must unlock the mutex once the winsys is fully initialized, so that
> - * other threads attempting to create the winsys from the same fd will
> - * get a fully initialized winsys and not just half-way initialized. */
> - pipe_mutex_unlock(dev_tab_mutex);
> + /* init reference */
> + pipe_reference_init(&ws->base.screen->reference, 1);
> + ws->base.screen->fd = -1;
>
> return &ws->base;
>
> fail:
> - pipe_mutex_unlock(dev_tab_mutex);
> pb_cache_deinit(&ws->bo_cache);
> FREE(ws);
> return NULL;
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
> index 8489530..b45c2d7 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
> @@ -42,7 +42,6 @@ struct amdgpu_cs;
>
> struct amdgpu_winsys {
> struct radeon_winsys base;
> - struct pipe_reference reference;
> struct pb_cache bo_cache;
>
> amdgpu_device_handle dev;
> --
> 2.9.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list