[Mesa-dev,v4,08/11] radeon: use common screen ref counting

Submitted by Rob Herring on July 22, 2016, 4:22 p.m.

Details

Message ID 20160722162222.21871-9-robh@kernel.org
State New
Headers show
Series "Common pipe screen ref counting" ( rev: 4 ) in Mesa

Not browsing as part of any series.

Commit Message

Rob Herring July 22, 2016, 4:22 p.m.
Use the common pipe_screen ref counting and fd hashing functions. The
mutex can be dropped as the pipe loader protects the create_screen()
calls.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: "Marek Olšák" <marek.olsak@amd.com>
Cc: Ilia Mirkin <imirkin@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/radeon/drm/radeon_drm_winsys.c | 80 ++---------------------
 5 files changed, 6 insertions(+), 97 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
index d47b70d..1340009 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -676,9 +676,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 f23daf9..c645295 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -570,12 +570,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 a9c9b9e..2ca19f4 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -426,14 +426,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 ee97bcf..1c6920c 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -657,12 +657,6 @@  static void si_destroy_screen(struct pipe_screen* pscreen)
 	};
 	unsigned i;
 
-	if (!sscreen)
-		return;
-
-	if (!sscreen->b.ws->unref(sscreen->b.ws))
-		return;
-
 	if (util_queue_is_initialized(&sscreen->shader_compiler_queue))
 		util_queue_destroy(&sscreen->shader_compiler_queue);
 
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
index 1b32c37..355197c 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
@@ -38,11 +38,11 @@ 
 #include "pipebuffer/pb_bufmgr.h"
 #include "util/u_memory.h"
 #include "util/u_hash_table.h"
+#include "util/u_screen.h"
 
 #include <xf86drm.h>
 #include <stdio.h>
 #include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <radeon_surface.h>
 
@@ -63,9 +63,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.
@@ -558,9 +555,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);
 }
 
@@ -665,49 +659,8 @@  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;
-}
-
 DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)
 
-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)
@@ -724,24 +677,14 @@  PUBLIC struct radeon_winsys *
 radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
 {
     struct radeon_drm_winsys *ws;
+    struct pipe_screen *pscreen = pipe_screen_reference(fd);
 
-    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;
-    }
+    if (pscreen)
+        return pscreen;
 
     ws = CALLOC_STRUCT(radeon_drm_winsys);
-    if (!ws) {
-        pipe_mutex_unlock(fd_tab_mutex);
+    if (!ws)
         return NULL;
-    }
 
     ws->fd = dup(fd);
 
@@ -759,11 +702,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;
@@ -799,23 +739,15 @@  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);
-
+    pipe_screen_reference_init(&ws->base, ws->fd);
     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)

Comments

On 22 July 2016 at 17:22, Rob Herring <robh@kernel.org> wrote:
> Use the common pipe_screen ref counting and fd hashing functions. The
> mutex can be dropped as the pipe loader protects the create_screen()
> calls.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: "Marek Olšák" <marek.olsak@amd.com>
> Cc: Ilia Mirkin <imirkin@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/radeon/drm/radeon_drm_winsys.c | 80 ++---------------------
>  5 files changed, 6 insertions(+), 97 deletions(-)
>
> diff --git a/src/gallium/drivers/r300/r300_screen.c b/src/gallium/drivers/r300/r300_screen.c
> index d47b70d..1340009 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -676,9 +676,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 f23daf9..c645295 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -570,12 +570,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 a9c9b9e..2ca19f4 100644
> --- a/src/gallium/drivers/radeon/radeon_winsys.h
> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
> @@ -426,14 +426,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 ee97bcf..1c6920c 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -657,12 +657,6 @@ static void si_destroy_screen(struct pipe_screen* pscreen)
>         };
>         unsigned i;
>
> -       if (!sscreen)
> -               return;
> -
> -       if (!sscreen->b.ws->unref(sscreen->b.ws))
> -               return;
> -
>         if (util_queue_is_initialized(&sscreen->shader_compiler_queue))
>                 util_queue_destroy(&sscreen->shader_compiler_queue);
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index 1b32c37..355197c 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -38,11 +38,11 @@
>  #include "pipebuffer/pb_bufmgr.h"
>  #include "util/u_memory.h"
>  #include "util/u_hash_table.h"
> +#include "util/u_screen.h"
>
>  #include <xf86drm.h>
>  #include <stdio.h>
>  #include <sys/types.h>
> -#include <sys/stat.h>
>  #include <unistd.h>
>  #include <radeon_surface.h>
>
> @@ -63,9 +63,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.
> @@ -558,9 +555,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);
>  }
>
> @@ -665,49 +659,8 @@ 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;
> -}
> -
>  DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", true)
>
> -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)
> @@ -724,24 +677,14 @@ PUBLIC struct radeon_winsys *
>  radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
>  {
>      struct radeon_drm_winsys *ws;
> +    struct pipe_screen *pscreen = pipe_screen_reference(fd);
>
> -    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;
> -    }
> +    if (pscreen)
> +        return pscreen;
>
>      ws = CALLOC_STRUCT(radeon_drm_winsys);
> -    if (!ws) {
> -        pipe_mutex_unlock(fd_tab_mutex);
> +    if (!ws)
>          return NULL;
> -    }
>
>      ws->fd = dup(fd);
>
> @@ -759,11 +702,8 @@ radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
>              goto fail;
>      }
>
> -    /* init reference */
> -    pipe_reference_init(&ws->reference, 1);
>
Afaict with this radeon_drm_winsys::reference is no longer used.

Emil