kmsro: Fix renderonly_scanout BO aliasing
authorAsahi Lina <lina@asahilina.net>
Tue, 20 Dec 2022 15:37:31 +0000 (00:37 +0900)
committerEric Engestrom <eric@engestrom.ch>
Wed, 11 Jan 2023 17:44:21 +0000 (17:44 +0000)
BOs can only have one handle. If renderonly_create_gpu_import_for_resource
ends up importing a BO that was already mapped for scanout, it will get
the same handle. This leaves us with two renderonly_scanout objects for
one handle, and the first one to be destroyed will free it.

Import the BO map tracking logic from asahi, to avoid aliasing
renderonly_scanout objects. Each actual BO now is only represented by a
single object instance, which is reference counted.

Fixes KWin full-screen PipeWire capture breaking scanout entirely.

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Signed-off-by: Asahi Lina <lina@asahilina.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20397>
(cherry picked from commit ad4d7ca8332488be8a75aff001f00306a9f6402e)

.pick_status.json
src/gallium/auxiliary/renderonly/renderonly.c
src/gallium/auxiliary/renderonly/renderonly.h
src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c

index 3dd47a4..b36524c 100644 (file)
         "description": "kmsro: Fix renderonly_scanout BO aliasing",
         "nominated": false,
         "nomination_type": null,
-        "resolution": 4,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": null
     },
index 56dc06b..d8628a3 100644 (file)
@@ -43,11 +43,22 @@ renderonly_scanout_destroy(struct renderonly_scanout *scanout,
 {
    struct drm_mode_destroy_dumb destroy_dumb = {0};
 
-   if (ro->kms_fd != -1) {
+   if (p_atomic_dec_return(&scanout->refcnt))
+      return;
+
+   simple_mtx_lock(&ro->bo_map_lock);
+
+   /* Someone might have imported this BO while we were waiting for the
+    * lock, let's make sure it's still not referenced before freeing it.
+    */
+   if (p_atomic_read(&scanout->refcnt) == 0 && ro->kms_fd != -1) {
       destroy_dumb.handle = scanout->handle;
+      scanout->handle = 0;
+      scanout->stride = 0;
       drmIoctl(ro->kms_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_dumb);
    }
-   FREE(scanout);
+
+   simple_mtx_unlock(&ro->bo_map_lock);
 }
 
 struct renderonly_scanout *
@@ -64,21 +75,27 @@ renderonly_create_kms_dumb_buffer_for_resource(struct pipe_resource *rsc,
    };
    struct drm_mode_destroy_dumb destroy_dumb = {0};
 
-   scanout = CALLOC_STRUCT(renderonly_scanout);
-   if (!scanout)
-      return NULL;
-
    /* create dumb buffer at scanout GPU */
    err = drmIoctl(ro->kms_fd, DRM_IOCTL_MODE_CREATE_DUMB, &create_dumb);
    if (err < 0) {
       fprintf(stderr, "DRM_IOCTL_MODE_CREATE_DUMB failed: %s\n",
             strerror(errno));
-      goto free_scanout;
+      return NULL;
    }
 
+   simple_mtx_lock(&ro->bo_map_lock);
+   scanout = util_sparse_array_get(&ro->bo_map, create_dumb.handle);
+   simple_mtx_unlock(&ro->bo_map_lock);
+
+   if (!scanout)
+      goto free_dumb;
+
    scanout->handle = create_dumb.handle;
    scanout->stride = create_dumb.pitch;
 
+   assert(p_atomic_read(&scanout->refcnt) == 0);
+   p_atomic_set(&scanout->refcnt, 1);
+
    if (!out_handle)
       return scanout;
 
@@ -100,9 +117,6 @@ free_dumb:
    destroy_dumb.handle = scanout->handle;
    drmIoctl(ro->kms_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_dumb);
 
-free_scanout:
-   FREE(scanout);
-
    return NULL;
 }
 
@@ -112,36 +126,40 @@ renderonly_create_gpu_import_for_resource(struct pipe_resource *rsc,
                                           struct winsys_handle *out_handle)
 {
    struct pipe_screen *screen = rsc->screen;
-   struct renderonly_scanout *scanout;
+   struct renderonly_scanout *scanout = NULL;
    boolean status;
+   uint32_t scanout_handle;
    int fd, err;
    struct winsys_handle handle = {
       .type = WINSYS_HANDLE_TYPE_FD
    };
 
-   scanout = CALLOC_STRUCT(renderonly_scanout);
-   if (!scanout)
-      return NULL;
-
    status = screen->resource_get_handle(screen, NULL, rsc, &handle,
          PIPE_HANDLE_USAGE_FRAMEBUFFER_WRITE);
    if (!status)
-      goto free_scanout;
+      return NULL;
 
-   scanout->stride = handle.stride;
    fd = handle.handle;
 
-   err = drmPrimeFDToHandle(ro->kms_fd, fd, &scanout->handle);
+   simple_mtx_lock(&ro->bo_map_lock);
+   err = drmPrimeFDToHandle(ro->kms_fd, fd, &scanout_handle);
    close(fd);
 
    if (err < 0)
-      goto free_scanout;
+      goto err_unlock;
 
-   return scanout;
+   scanout = util_sparse_array_get(&ro->bo_map, scanout_handle);
+   if (!scanout)
+      goto err_unlock;
+
+   if (p_atomic_inc_return(&scanout->refcnt) == 1) {
+      scanout->handle = scanout_handle;
+      scanout->stride = handle.stride;
+   }
 
-free_scanout:
-   FREE(scanout);
+err_unlock:
+   simple_mtx_unlock(&ro->bo_map_lock);
 
-   return NULL;
+   return scanout;
 }
 
index 0eafbf5..911ad66 100644 (file)
 #include <stdint.h>
 #include "frontend/drm_driver.h"
 #include "pipe/p_state.h"
+#include "util/simple_mtx.h"
+#include "util/sparse_array.h"
 
 struct renderonly_scanout {
    uint32_t handle;
    uint32_t stride;
+   int32_t refcnt;
 };
 
 struct renderonly {
@@ -77,6 +80,9 @@ struct renderonly {
    void (*destroy)(struct renderonly *ro);
    int kms_fd;
    int gpu_fd;
+
+   simple_mtx_t bo_map_lock;
+   struct util_sparse_array bo_map;
 };
 
 static inline struct renderonly_scanout *
index 3c8a3c4..dd26120 100644 (file)
@@ -44,6 +44,8 @@ static void kmsro_ro_destroy(struct renderonly *ro)
    if (ro->gpu_fd >= 0)
       close(ro->gpu_fd);
 
+   util_sparse_array_finish(&ro->bo_map);
+
    FREE(ro);
 }
 
@@ -59,6 +61,8 @@ struct pipe_screen *kmsro_drm_screen_create(int fd,
    ro->kms_fd = fd;
    ro->gpu_fd = -1;
    ro->destroy = kmsro_ro_destroy;
+   util_sparse_array_init(&ro->bo_map, sizeof(struct renderonly_scanout), 64);
+   simple_mtx_init(&ro->bo_map_lock, mtx_plain);
 
 #if defined(GALLIUM_VC4)
    ro->gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER);