panfrost_xwayland_hang.patch

   1 From 74e4a5018b85af139dca06f3a614667d3d16748d Mon Sep 17 00:00:00 2001
   2 From: Alyssa Rosenzweig <alyssa@collabora.com>
   3 Date: Tue, 22 Feb 2022 11:24:58 -0500
   4 Subject: [PATCH 1/2] panfrost: Fix FD resource_get_handle
   5
   6 When handle->type is WINSYS_HANDLE_TYPE_FD, the caller wants a file descriptor
   7 for the BO backing the resource. We previously had two paths for this:
   8
   9 1. If rsrc->scanout is available, we prime the GEM handle from the KMS device
  10    (rsrc->scanout->handle) to a file descriptor via the KMS device.
  11
  12 2. If rsrc->scanout is not available, we prime the GEM handle from the GPU
  13    (bo->gem_handle) to a file descriptor via the GPU device.
  14
  15 In both cases, the caller passes in a resource (with BO) and expects out a file
  16 descriptor. There are no direct GEM handles in the function signature; the
  17 caller doesn't care which GEM handle we prime to get the file descriptor. In
  18 principle, both paths produce the same file descriptor for the same BO, since
  19 both GEM handles represent the same underlying resource (viewed from different
  20 devices).
  21
  22 On grounds of redundancy alone, it makes sense to remove the rsrc->scanout path.
  23 Why have a path that only works sometimes, when we have another path that works
  24 always?
  25
  26 In fact, the issues with the rsrc->scanout path are deeper. rsrc->scanout is
  27 populated by renderonly_create_gpu_import_for_resource, which does the
  28 following:
  29
  30 1. Get a file descriptor for the resource by resource_get_handle with
  31    WINSYS_HANDLE_TYPE_FD
  32 2. Prime the file descriptor to a GEM handle via the KMS device.
  33
  34 Here comes strike number 2: in order to get a file descriptor via the KMS
  35 device, we had to /already/ get a file descriptor via the GPU device. If we go
  36 down the KMS device path, we effectively round trip:
  37
  38    GPU handle -> fd -> KMS handle -> fd
  39
  40 There is no good reason to do this; if everything works, the fd is the same in
  41 each case. If everything works. If.
  42
  43 The lifetimes of the GPU handle and the KMS handle are not necessarily bound. In
  44 principle, a resource can be created with scanout (constructing a KMS handle).
  45 Then the KMS view can be destroyed (invalidating the GEM handle for the KMS
  46 device), even though the underlying resource is still valid. Notice the GPU
  47 handle is still valid; its lifetime is tied to the resource itself. Then a
  48 caller can ask for the FD for the resource; as the resource is still valid, this
  49 is sensible. Under the scanout path, we try to get the FD by priming the GEM
  50 handle on the KMS device... but that GEM handle is no longer valid, causing the
  51 PRIME ioctl to fail with ENOENT. On the other hand, if we primed the GPU GEM
  52 handle, everything works as expected.
  53
  54 These edge cases are not theoretical; recent versions of Xwayland trigger this
  55 ENOENT, causing issue #5758 on all Panfrost devices. As far as I can tell, no
  56 other kmsro driver has this 'special' kmsro path; the only part of
  57 resource_get_handle that needs special handling for kmsro is getting a KMS
  58 handle.
  59
  60 Let's remove the broken, useless path, fix Xwayland, bring us in line with other
  61 drivers, and delete some code.
  62
  63 Thank you for coming to my ted talk.
  64
  65 Closes: #5758
  66 Fixes: 7da251fc721 ("panfrost: Check in sources for command stream")
  67 Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
  68 Reported-and-tested-by: Jan Palus <jpalus@fastmail.com>
  69 Reviewed-by: Simon Ser <contact@emersion.fr>
  70 Reviewed-by: James Jones <jajones@nvidia.com>
  71 Acked-by: Daniel Stone <daniels@collabora.com>
  72 ---
  73  src/gallium/drivers/panfrost/pan_resource.c | 30 +++++----------------
  74  1 file changed, 7 insertions(+), 23 deletions(-)
  75
  76 diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
  77 index f473f71099e..094332862ff 100644
  78 --- a/src/gallium/drivers/panfrost/pan_resource.c
  79 +++ b/src/gallium/drivers/panfrost/pan_resource.c
  80 @@ -165,31 +165,15 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
  81                          return true;
  82                  }
  83          } else if (handle->type == WINSYS_HANDLE_TYPE_FD) {
  84 -                if (scanout) {
  85 -                        struct drm_prime_handle args = {
  86 -                                .handle = scanout->handle,
  87 -                                .flags = DRM_CLOEXEC,
  88 -                        };
  89 +                int fd = panfrost_bo_export(rsrc->image.data.bo);
  90
  91 -                        int ret = drmIoctl(dev->ro->kms_fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
  92 -                        if (ret == -1)
  93 -                                return false;
  94 -
  95 -                        handle->stride = scanout->stride;
  96 -                        handle->handle = args.fd;
  97 -
  98 -                        return true;
  99 -                } else {
 100 -                        int fd = panfrost_bo_export(rsrc->image.data.bo);
 101 -
 102 -                        if (fd < 0)
 103 -                                return false;
 104 +                if (fd < 0)
 105 +                        return false;
 106
 107 -                        handle->handle = fd;
 108 -                        handle->stride = rsrc->image.layout.slices[0].line_stride;
 109 -                        handle->offset = rsrc->image.layout.slices[0].offset;
 110 -                        return true;
 111 -                }
 112 +                handle->handle = fd;
 113 +                handle->stride = rsrc->image.layout.slices[0].line_stride;
 114 +                handle->offset = rsrc->image.layout.slices[0].offset;
 115 +                return true;
 116          }
 117
 118          return false;
 119 --
 120 GitLab
 121
 122
 123 From 43b5b452e23243ee178e6fd3c3f5b02fa1dabd71 Mon Sep 17 00:00:00 2001
 124 From: Alyssa Rosenzweig <alyssa@collabora.com>
 125 Date: Tue, 22 Feb 2022 11:30:05 -0500
 126 Subject: [PATCH 2/2] panfrost: Simplify panfrost_resource_get_handle
 127
 128 Unify the exit paths to clean up the logic. There are logically three modes we
 129 support (KMS without renderonly, KMS with renderonly, and FD); these each
 130 correspond to a leg of a small if statement. Outside of the small if's,
 131 everything else should be identical.
 132
 133 Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
 134 Reviewed-by: Simon Ser <contact@emersion.fr>
 135 Reviewed-by: James Jones <jajones@nvidia.com>
 136 Acked-by: Daniel Stone <daniels@collabora.com>
 137 ---
 138  src/gallium/drivers/panfrost/pan_resource.c | 23 ++++++++-------------
 139  1 file changed, 9 insertions(+), 14 deletions(-)
 140
 141 diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
 142 index 094332862ff..477bf1d7ce1 100644
 143 --- a/src/gallium/drivers/panfrost/pan_resource.c
 144 +++ b/src/gallium/drivers/panfrost/pan_resource.c
 145 @@ -153,17 +153,10 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
 146          handle->modifier = rsrc->image.layout.modifier;
 147          rsrc->modifier_constant = true;
 148
 149 -        if (handle->type == WINSYS_HANDLE_TYPE_SHARED) {
 150 -                return false;
 151 +        if (handle->type == WINSYS_HANDLE_TYPE_KMS && dev->ro) {
 152 +                return renderonly_get_handle(scanout, handle);
 153          } else if (handle->type == WINSYS_HANDLE_TYPE_KMS) {
 154 -                if (dev->ro) {
 155 -                        return renderonly_get_handle(scanout, handle);
 156 -                } else {
 157 -                        handle->handle = rsrc->image.data.bo->gem_handle;
 158 -                        handle->stride = rsrc->image.layout.slices[0].line_stride;
 159 -                        handle->offset = rsrc->image.layout.slices[0].offset;
 160 -                        return true;
 161 -                }
 162 +                handle->handle = rsrc->image.data.bo->gem_handle;
 163          } else if (handle->type == WINSYS_HANDLE_TYPE_FD) {
 164                  int fd = panfrost_bo_export(rsrc->image.data.bo);
 165
 166 @@ -171,12 +164,14 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
 167                          return false;
 168
 169                  handle->handle = fd;
 170 -                handle->stride = rsrc->image.layout.slices[0].line_stride;
 171 -                handle->offset = rsrc->image.layout.slices[0].offset;
 172 -                return true;
 173 +        } else {
 174 +                /* Other handle types not supported */
 175 +                return false;
 176          }
 177
 178 -        return false;
 179 +        handle->stride = rsrc->image.layout.slices[0].line_stride;
 180 +        handle->offset = rsrc->image.layout.slices[0].offset;
 181 +        return true;
 182  }
 183
 184  static bool
 185 --
 186 GitLab
 187