[Mesa-dev] [RFC 7/7] radeon: remove screen ref counting

Rob Herring robh at kernel.org
Fri Jun 17 17:45:36 UTC 2016


Now that the pipe-loader is reference counting the screen creation, it
is unnecessary to do in it the winsys/driver.

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/drivers/r300/r300_screen.c            |  3 -
 src/gallium/drivers/r600/r600_pipe.c              |  6 --
 src/gallium/drivers/radeon/radeon_winsys.h        |  8 ---
 src/gallium/drivers/radeonsi/si_pipe.c            |  6 --
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c     | 66 +------------------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h     |  1 -
 src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 80 +----------------------
 7 files changed, 3 insertions(+), 167 deletions(-)

diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index 681681b..5d2d955 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -674,9 +674,6 @@ static void r300_destroy_screen(struct pipe_screen* pscreen)
     struct r300_screen* r300screen = r300_screen(pscreen);
     struct radeon_winsys *rws = radeon_winsys(pscreen);
 
-    if (rws && !rws->unref(rws))
-      return;
-
     pipe_mutex_destroy(r300screen->cmask_mutex);
 
     if (rws)
diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
index a49b00f..66cb78c 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -566,12 +566,6 @@ static void r600_destroy_screen(struct pipe_screen* pscreen)
 {
 	struct r600_screen *rscreen = (struct r600_screen *)pscreen;
 
-	if (!rscreen)
-		return;
-
-	if (!rscreen->b.ws->unref(rscreen->b.ws))
-		return;
-
 	if (rscreen->global_pool) {
 		compute_memory_pool_delete(rscreen->global_pool);
 	}
diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h
index c2d1f9e..ffe0d83 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -416,14 +416,6 @@ struct radeon_winsys {
     struct pipe_screen *screen;
 
     /**
-     * Decrement the winsys reference count.
-     *
-     * \param ws  The winsys this function is called for.
-     * \return    True if the winsys and screen should be destroyed.
-     */
-    bool (*unref)(struct radeon_winsys *ws);
-
-    /**
      * Destroy this winsys.
      *
      * \param ws        The winsys this function is called from.
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
index 0c601da..f3256fc 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -628,12 +628,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
 	};
 	unsigned i;
 
-	if (!sscreen)
-		return;
-
-	if (!sscreen->b.ws->unref(sscreen->b.ws))
-		return;
-
 	/* Free shader parts. */
 	for (i = 0; i < ARRAY_SIZE(parts); i++) {
 		while (parts[i]) {
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 7016221..39b4a11 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -34,7 +34,6 @@
 #include "amdgpu_cs.h"
 #include "amdgpu_public.h"
 
-#include "util/u_hash_table.h"
 #include <amdgpu_drm.h>
 #include <xf86drm.h>
 #include <stdio.h>
@@ -59,9 +58,6 @@
 #define     CIK__PIPE_CONFIG__ADDR_SURF_P16_32X32_8X16   16
 #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];
@@ -386,20 +382,6 @@ static bool amdgpu_read_registers(struct radeon_winsys *rws,
                                    0xffffffff, 0, out) == 0;
 }
 
-static unsigned hash_dev(void *key)
-{
-#if defined(PIPE_ARCH_X86_64)
-   return pointer_to_intptr(key) ^ (pointer_to_intptr(key) >> 32);
-#else
-   return pointer_to_intptr(key);
-#endif
-}
-
-static int compare_dev(void *key1, void *key2)
-{
-   return key1 != key2;
-}
-
 void amdgpu_ws_queue_cs(struct amdgpu_winsys *ws, struct amdgpu_cs *cs)
 {
    pipe_semaphore_wait(&ws->cs_queue_has_space);
@@ -448,26 +430,6 @@ static PIPE_THREAD_ROUTINE(amdgpu_cs_thread_func, param)
 DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", TRUE)
 static PIPE_THREAD_ROUTINE(amdgpu_cs_thread_func, param);
 
-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)
 {
@@ -483,34 +445,18 @@ 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);
-
    /* Initialize the amdgpu device. This should always return the same pointer
     * 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;
    }
 
-   /* 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);
-      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;
@@ -528,7 +474,6 @@ amdgpu_winsys_create(int fd, radeon_screen_create_t screen_create)
    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;
@@ -558,21 +503,12 @@ 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);
-
    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 d6734f7..61fc21d 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;
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 5c85c8f..7ef00e5 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -42,7 +42,6 @@
 #include <xf86drm.h>
 #include <stdio.h>
 #include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <radeon_surface.h>
 
@@ -63,9 +62,6 @@
 #define RADEON_INFO_GPU_RESET_COUNTER   0x26
 #endif
 
-static struct util_hash_table *fd_tab = NULL;
-pipe_static_mutex(fd_tab_mutex);
-
 /* Enable/disable feature access for one command stream.
  * If enable == TRUE, return TRUE on success.
  * Otherwise, return FALSE.
@@ -557,9 +553,6 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws)
     pipe_mutex_destroy(ws->bo_handles_mutex);
     pipe_mutex_destroy(ws->bo_va_mutex);
 
-    if (ws->fd >= 0)
-        close(ws->fd);
-
     FREE(rws);
 }
 
@@ -664,28 +657,6 @@ static bool radeon_read_registers(struct radeon_winsys *rws,
     return true;
 }
 
-static unsigned hash_fd(void *key)
-{
-    int fd = pointer_to_intptr(key);
-    struct stat stat;
-    fstat(fd, &stat);
-
-    return stat.st_dev ^ stat.st_ino ^ stat.st_rdev;
-}
-
-static int compare_fd(void *key1, void *key2)
-{
-    int fd1 = pointer_to_intptr(key1);
-    int fd2 = pointer_to_intptr(key2);
-    struct stat stat1, stat2;
-    fstat(fd1, &stat1);
-    fstat(fd2, &stat2);
-
-    return stat1.st_dev != stat2.st_dev ||
-           stat1.st_ino != stat2.st_ino ||
-           stat1.st_rdev != stat2.st_rdev;
-}
-
 void radeon_drm_ws_queue_cs(struct radeon_drm_winsys *ws, struct radeon_drm_cs *cs)
 {
 retry:
@@ -736,25 +707,6 @@ static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param)
 DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", TRUE)
 static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param);
 
-static bool radeon_winsys_unref(struct radeon_winsys *ws)
-{
-    struct radeon_drm_winsys *rws = (struct radeon_drm_winsys*)ws;
-    bool destroy;
-
-    /* When the reference counter drops to zero, remove the fd from the table.
-     * This must happen while the mutex is locked, so that
-     * radeon_drm_winsys_create in another thread doesn't get the winsys
-     * from the table when the counter drops to 0. */
-    pipe_mutex_lock(fd_tab_mutex);
-
-    destroy = pipe_reference(&rws->reference, NULL);
-    if (destroy && fd_tab)
-        util_hash_table_remove(fd_tab, intptr_to_pointer(rws->fd));
-
-    pipe_mutex_unlock(fd_tab_mutex);
-    return destroy;
-}
-
 #define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
 
 static unsigned handle_hash(void *key)
@@ -772,25 +724,11 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
 {
     struct radeon_drm_winsys *ws;
 
-    pipe_mutex_lock(fd_tab_mutex);
-    if (!fd_tab) {
-        fd_tab = util_hash_table_create(hash_fd, compare_fd);
-    }
-
-    ws = util_hash_table_get(fd_tab, intptr_to_pointer(fd));
-    if (ws) {
-        pipe_reference(NULL, &ws->reference);
-        pipe_mutex_unlock(fd_tab_mutex);
-        return &ws->base;
-    }
-
     ws = CALLOC_STRUCT(radeon_drm_winsys);
-    if (!ws) {
-        pipe_mutex_unlock(fd_tab_mutex);
+    if (!ws)
         return NULL;
-    }
 
-    ws->fd = dup(fd);
+    ws->fd = fd;
 
     if (!do_winsys_init(ws))
         goto fail1;
@@ -806,11 +744,8 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
             goto fail;
     }
 
-    /* init reference */
-    pipe_reference_init(&ws->reference, 1);
 
     /* Set functions. */
-    ws->base.unref = radeon_winsys_unref;
     ws->base.destroy = radeon_winsys_destroy;
     ws->base.query_info = radeon_query_info;
     ws->base.cs_request_feature = radeon_cs_request_feature;
@@ -849,27 +784,16 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
     ws->base.screen = screen_create(&ws->base);
     if (!ws->base.screen) {
         radeon_winsys_destroy(&ws->base);
-        pipe_mutex_unlock(fd_tab_mutex);
         return NULL;
     }
 
-    util_hash_table_set(fd_tab, intptr_to_pointer(ws->fd), 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(fd_tab_mutex);
-
     return &ws->base;
 
 fail:
     pb_cache_deinit(&ws->bo_cache);
 fail1:
-    pipe_mutex_unlock(fd_tab_mutex);
     if (ws->surf_man)
         radeon_surface_manager_free(ws->surf_man);
-    if (ws->fd >= 0)
-        close(ws->fd);
 
     FREE(ws);
     return NULL;
-- 
2.7.4



More information about the mesa-dev mailing list