Mesa (master): winsys/amdgpu: Re-use amdgpu_screen_winsys when possible

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 9f2bed49d476a15af0df270307f9051d810ea0ab
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9f2bed49d476a15af0df270307f9051d810ea0ab

Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Tue Jan 28 11:07:15 2020 +0100

winsys/amdgpu: Re-use amdgpu_screen_winsys when possible

Namely, if os_same_file_description determined that the DRM file
descriptor references the same file description.

v2:
* Adapt to amdgpu_winsys::sws_list_lock.
v3:
* Fix comparison of amdgpu_screen_winsys file descriptions, see
  https://gitlab.freedesktop.org/mesa/mesa/issues/2413 .
* Lock amdgpu_winsys::sws_list_lock for traversing the sws_list in
  amdgpu_winsys_create.

Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3582>

---

 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 71 ++++++++++++++++++---------
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h |  1 +
 2 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index 000abd00a14..89d3d93bb5f 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -30,6 +30,7 @@
 #include "amdgpu_cs.h"
 #include "amdgpu_public.h"
 
+#include "util/os_file.h"
 #include "util/u_cpu_detect.h"
 #include "util/u_hash_table.h"
 #include "util/hash_table.h"
@@ -172,26 +173,8 @@ static void amdgpu_winsys_destroy(struct radeon_winsys *rws)
 
    simple_mtx_unlock(&dev_tab_mutex);
 
-   if (destroy) {
+   if (destroy)
       do_winsys_deinit(ws);
-   } else {
-      struct amdgpu_screen_winsys **sws_iter;
-
-      /* Remove this amdgpu_screen_winsys from amdgpu_winsys' list */
-      simple_mtx_lock(&ws->sws_list_lock);
-      for (sws_iter = &ws->sws_list; *sws_iter; sws_iter = &(*sws_iter)->next) {
-         if (*sws_iter == sws) {
-            *sws_iter = sws->next;
-            break;
-         }
-      }
-      simple_mtx_unlock(&ws->sws_list_lock);
-   }
-
-   if (sws->kms_handles) {
-      assert(!destroy);
-      _mesa_hash_table_destroy(sws->kms_handles, NULL);
-   }
 
    close(sws->fd);
    FREE(rws);
@@ -297,11 +280,34 @@ static int compare_pointers(void *key1, void *key2)
 
 static bool amdgpu_winsys_unref(struct radeon_winsys *rws)
 {
-   /* radeon_winsys corresponds to amdgpu_screen_winsys, which is never
-    * referenced multiple times, so amdgpu_winsys_destroy always needs to be
-    * called. It handles reference counting for amdgpu_winsys.
-    */
-   return true;
+   struct amdgpu_screen_winsys *sws = amdgpu_screen_winsys(rws);
+   struct amdgpu_winsys *aws = sws->aws;
+   bool ret;
+
+   simple_mtx_lock(&aws->sws_list_lock);
+
+   ret = pipe_reference(&sws->reference, NULL);
+   if (ret) {
+      struct amdgpu_screen_winsys **sws_iter;
+      struct amdgpu_winsys *aws = sws->aws;
+
+      /* Remove this amdgpu_screen_winsys from amdgpu_winsys' list, so that
+       * amdgpu_winsys_create can't re-use it anymore
+       */
+      for (sws_iter = &aws->sws_list; *sws_iter; sws_iter = &(*sws_iter)->next) {
+         if (*sws_iter == sws) {
+            *sws_iter = sws->next;
+            break;
+         }
+      }
+   }
+
+   simple_mtx_unlock(&aws->sws_list_lock);
+
+   if (ret && sws->kms_handles)
+      _mesa_hash_table_destroy(sws->kms_handles, NULL);
+
+   return ret;
 }
 
 static void amdgpu_pin_threads_to_L3_cache(struct radeon_winsys *rws,
@@ -338,6 +344,7 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
    if (!ws)
       return NULL;
 
+   pipe_reference_init(&ws->reference, 1);
    ws->fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
 
    /* Look up the winsys from the dev table. */
@@ -356,12 +363,27 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
    /* Lookup a winsys if we have already created one for this device. */
    aws = util_hash_table_get(dev_tab, dev);
    if (aws) {
+      struct amdgpu_screen_winsys *sws_iter;
+
       /* Release the device handle, because we don't need it anymore.
        * This function is returning an existing winsys instance, which
        * has its own device handle.
        */
       amdgpu_device_deinitialize(dev);
 
+      simple_mtx_lock(&aws->sws_list_lock);
+      for (sws_iter = aws->sws_list; sws_iter; sws_iter = sws_iter->next) {
+         if (os_same_file_description(sws_iter->fd, ws->fd)) {
+            close(ws->fd);
+            FREE(ws);
+            ws = sws_iter;
+            pipe_reference(NULL, &ws->reference);
+            simple_mtx_unlock(&aws->sws_list_lock);
+            goto unlock;
+         }
+      }
+      simple_mtx_unlock(&aws->sws_list_lock);
+
       ws->kms_handles = _mesa_hash_table_create(NULL, kms_handle_hash,
                                                 kms_handle_equals);
       if (!ws->kms_handles)
@@ -478,6 +500,7 @@ amdgpu_winsys_create(int fd, const struct pipe_screen_config *config,
    aws->sws_list = ws;
    simple_mtx_unlock(&aws->sws_list_lock);
 
+unlock:
    /* 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. */
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
index c2edf4f46dc..a22be6086dc 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
@@ -107,6 +107,7 @@ struct amdgpu_screen_winsys {
    struct radeon_winsys base;
    struct amdgpu_winsys *aws;
    int fd;
+   struct pipe_reference reference;
    struct amdgpu_screen_winsys *next;
 
    /* Maps a BO to its KMS handle valid for this DRM file descriptor