-From 74e4a5018b85af139dca06f3a614667d3d16748d Mon Sep 17 00:00:00 2001
-From: Alyssa Rosenzweig <alyssa@collabora.com>
-Date: Tue, 22 Feb 2022 11:24:58 -0500
-Subject: [PATCH 1/2] panfrost: Fix FD resource_get_handle
-
-When handle->type is WINSYS_HANDLE_TYPE_FD, the caller wants a file descriptor
-for the BO backing the resource. We previously had two paths for this:
-
-1. If rsrc->scanout is available, we prime the GEM handle from the KMS device
- (rsrc->scanout->handle) to a file descriptor via the KMS device.
-
-2. If rsrc->scanout is not available, we prime the GEM handle from the GPU
- (bo->gem_handle) to a file descriptor via the GPU device.
-
-In both cases, the caller passes in a resource (with BO) and expects out a file
-descriptor. There are no direct GEM handles in the function signature; the
-caller doesn't care which GEM handle we prime to get the file descriptor. In
-principle, both paths produce the same file descriptor for the same BO, since
-both GEM handles represent the same underlying resource (viewed from different
-devices).
-
-On grounds of redundancy alone, it makes sense to remove the rsrc->scanout path.
-Why have a path that only works sometimes, when we have another path that works
-always?
-
-In fact, the issues with the rsrc->scanout path are deeper. rsrc->scanout is
-populated by renderonly_create_gpu_import_for_resource, which does the
-following:
-
-1. Get a file descriptor for the resource by resource_get_handle with
- WINSYS_HANDLE_TYPE_FD
-2. Prime the file descriptor to a GEM handle via the KMS device.
-
-Here comes strike number 2: in order to get a file descriptor via the KMS
-device, we had to /already/ get a file descriptor via the GPU device. If we go
-down the KMS device path, we effectively round trip:
-
- GPU handle -> fd -> KMS handle -> fd
-
-There is no good reason to do this; if everything works, the fd is the same in
-each case. If everything works. If.
-
-The lifetimes of the GPU handle and the KMS handle are not necessarily bound. In
-principle, a resource can be created with scanout (constructing a KMS handle).
-Then the KMS view can be destroyed (invalidating the GEM handle for the KMS
-device), even though the underlying resource is still valid. Notice the GPU
-handle is still valid; its lifetime is tied to the resource itself. Then a
-caller can ask for the FD for the resource; as the resource is still valid, this
-is sensible. Under the scanout path, we try to get the FD by priming the GEM
-handle on the KMS device... but that GEM handle is no longer valid, causing the
-PRIME ioctl to fail with ENOENT. On the other hand, if we primed the GPU GEM
-handle, everything works as expected.
-
-These edge cases are not theoretical; recent versions of Xwayland trigger this
-ENOENT, causing issue #5758 on all Panfrost devices. As far as I can tell, no
-other kmsro driver has this 'special' kmsro path; the only part of
-resource_get_handle that needs special handling for kmsro is getting a KMS
-handle.
-
-Let's remove the broken, useless path, fix Xwayland, bring us in line with other
-drivers, and delete some code.
-
-Thank you for coming to my ted talk.
-
-Closes: #5758
-Fixes: 7da251fc721 ("panfrost: Check in sources for command stream")
-Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
-Reported-and-tested-by: Jan Palus <jpalus@fastmail.com>
-Reviewed-by: Simon Ser <contact@emersion.fr>
-Reviewed-by: James Jones <jajones@nvidia.com>
-Acked-by: Daniel Stone <daniels@collabora.com>
----
- src/gallium/drivers/panfrost/pan_resource.c | 30 +++++----------------
- 1 file changed, 7 insertions(+), 23 deletions(-)
-
-diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
-index f473f71099e..094332862ff 100644
---- a/src/gallium/drivers/panfrost/pan_resource.c
-+++ b/src/gallium/drivers/panfrost/pan_resource.c
-@@ -165,31 +165,15 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
- return true;
- }
- } else if (handle->type == WINSYS_HANDLE_TYPE_FD) {
-- if (scanout) {
-- struct drm_prime_handle args = {
-- .handle = scanout->handle,
-- .flags = DRM_CLOEXEC,
-- };
-+ int fd = panfrost_bo_export(rsrc->image.data.bo);
-
-- int ret = drmIoctl(dev->ro->kms_fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
-- if (ret == -1)
-- return false;
--
-- handle->stride = scanout->stride;
-- handle->handle = args.fd;
--
-- return true;
-- } else {
-- int fd = panfrost_bo_export(rsrc->image.data.bo);
--
-- if (fd < 0)
-- return false;
-+ if (fd < 0)
-+ return false;
-
-- handle->handle = fd;
-- handle->stride = rsrc->image.layout.slices[0].line_stride;
-- handle->offset = rsrc->image.layout.slices[0].offset;
-- return true;
-- }
-+ handle->handle = fd;
-+ handle->stride = rsrc->image.layout.slices[0].line_stride;
-+ handle->offset = rsrc->image.layout.slices[0].offset;
-+ return true;
- }
-
- return false;
---
-GitLab
-
-
-From 43b5b452e23243ee178e6fd3c3f5b02fa1dabd71 Mon Sep 17 00:00:00 2001
-From: Alyssa Rosenzweig <alyssa@collabora.com>
-Date: Tue, 22 Feb 2022 11:30:05 -0500
-Subject: [PATCH 2/2] panfrost: Simplify panfrost_resource_get_handle
-
-Unify the exit paths to clean up the logic. There are logically three modes we
-support (KMS without renderonly, KMS with renderonly, and FD); these each
-correspond to a leg of a small if statement. Outside of the small if's,
-everything else should be identical.
-
-Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
-Reviewed-by: Simon Ser <contact@emersion.fr>
-Reviewed-by: James Jones <jajones@nvidia.com>
-Acked-by: Daniel Stone <daniels@collabora.com>
----
- src/gallium/drivers/panfrost/pan_resource.c | 23 ++++++++-------------
- 1 file changed, 9 insertions(+), 14 deletions(-)
-
-diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
-index 094332862ff..477bf1d7ce1 100644
---- a/src/gallium/drivers/panfrost/pan_resource.c
-+++ b/src/gallium/drivers/panfrost/pan_resource.c
-@@ -153,17 +153,10 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
- handle->modifier = rsrc->image.layout.modifier;
- rsrc->modifier_constant = true;
-
-- if (handle->type == WINSYS_HANDLE_TYPE_SHARED) {
-- return false;
-+ if (handle->type == WINSYS_HANDLE_TYPE_KMS && dev->ro) {
-+ return renderonly_get_handle(scanout, handle);
- } else if (handle->type == WINSYS_HANDLE_TYPE_KMS) {
-- if (dev->ro) {
-- return renderonly_get_handle(scanout, handle);
-- } else {
-- handle->handle = rsrc->image.data.bo->gem_handle;
-- handle->stride = rsrc->image.layout.slices[0].line_stride;
-- handle->offset = rsrc->image.layout.slices[0].offset;
-- return true;
-- }
-+ handle->handle = rsrc->image.data.bo->gem_handle;
- } else if (handle->type == WINSYS_HANDLE_TYPE_FD) {
- int fd = panfrost_bo_export(rsrc->image.data.bo);
-
-@@ -171,12 +164,14 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
- return false;
-
- handle->handle = fd;
-- handle->stride = rsrc->image.layout.slices[0].line_stride;
-- handle->offset = rsrc->image.layout.slices[0].offset;
-- return true;
-+ } else {
-+ /* Other handle types not supported */
-+ return false;
- }
-
-- return false;
-+ handle->stride = rsrc->image.layout.slices[0].line_stride;
-+ handle->offset = rsrc->image.layout.slices[0].offset;
-+ return true;
- }
-
- static bool
---
-GitLab
-
projects / packages / Mesa.git / commitdiff