dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
@ 2023-06-15  9:31 Thomas Zimmermann
  2023-06-15  9:31 ` [PATCH 1/3] drm: Enable PRIME import/export " Thomas Zimmermann
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-15  9:31 UTC (permalink / raw)
  To: daniel, airlied, mripard, maarten.lankhorst, zackr, contact,
	linux-graphics-maintainer
  Cc: Thomas Zimmermann, dri-devel

Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
for all DRM drivers. Even drivers that do not support PRIME import
or export of dma-bufs can now import their own buffer objects. This
is required by some userspace, such as wlroots-based compositors, to
share buffers among processes.

The only driver that does not use the drm_gem_prime_*() helpers is
vmwgfx. Once it has been converted, the callbacks in struct drm_driver
can be removed.

Simon Ser implemented the feature for drivers based on GEM VRAM helpers
in [1]. This patchset generalizes the code for all drivers that do not
otherwise support PRIME. Tested by running sway with gma500 hardware.

[1] https://lore.kernel.org/dri-devel/20230302143502.500661-1-contact@emersion.fr/

Thomas Zimmermann (3):
  drm: Enable PRIME import/export for all drivers
  drm: Clear fd/handle callbacks in struct drm_driver
  drm/prime: Unexport helpers for fd/handle conversion

 Documentation/gpu/todo.rst                    | 14 +++++
 drivers/accel/ivpu/ivpu_drv.c                 |  2 -
 drivers/accel/qaic/qaic_drv.c                 |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  2 -
 drivers/gpu/drm/armada/armada_drv.c           |  2 -
 drivers/gpu/drm/drm_ioctl.c                   |  4 +-
 drivers/gpu/drm/drm_prime.c                   | 62 ++++++++++---------
 drivers/gpu/drm/etnaviv/etnaviv_drv.c         |  2 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c       |  2 -
 drivers/gpu/drm/i915/i915_driver.c            |  2 -
 drivers/gpu/drm/lima/lima_drv.c               |  2 -
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  2 -
 drivers/gpu/drm/msm/msm_drv.c                 |  2 -
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  2 -
 drivers/gpu/drm/omapdrm/omap_drv.c            |  2 -
 drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 -
 drivers/gpu/drm/pl111/pl111_drv.c             |  2 -
 drivers/gpu/drm/qxl/qxl_drv.c                 |  2 -
 drivers/gpu/drm/radeon/radeon_drv.c           |  2 -
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c |  2 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  2 -
 drivers/gpu/drm/tegra/drm.c                   |  2 -
 drivers/gpu/drm/v3d/v3d_drv.c                 |  2 -
 drivers/gpu/drm/virtio/virtgpu_drv.c          |  2 -
 drivers/gpu/drm/xen/xen_drm_front.c           |  2 -
 include/drm/drm_drv.h                         |  6 ++
 include/drm/drm_gem_dma_helper.h              | 12 ++--
 include/drm/drm_gem_shmem_helper.h            |  6 +-
 include/drm/drm_gem_vram_helper.h             |  4 +-
 include/drm/drm_prime.h                       |  7 ---
 30 files changed, 62 insertions(+), 96 deletions(-)

-- 
2.41.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] drm: Enable PRIME import/export for all drivers
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
@ 2023-06-15  9:31 ` Thomas Zimmermann
  2023-06-15 14:47   ` Simon Ser
  2023-06-15  9:31 ` [PATCH 2/3] drm: Clear fd/handle callbacks in struct drm_driver Thomas Zimmermann
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-15  9:31 UTC (permalink / raw)
  To: daniel, airlied, mripard, maarten.lankhorst, zackr, contact,
	linux-graphics-maintainer
  Cc: Thomas Zimmermann, dri-devel

Call drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle() by
default if no PRIME import/export helpers have been set. Both functions
are the default for almost all drivers.

DRM drivers implement struct drm_driver.gem_prime_import_sg_table
to import dma-buf objects from other drivers. Having the function
drm_gem_prime_fd_to_handle() functions set by default allows each
driver to import dma-buf objects to itself, even without support for
other drivers.

For drm_gem_prime_handle_to_fd() it is similar: using it by default
allows each driver to export to itself, even without support for other
drivers.

This functionality enables userspace to share per-driver buffers
across process boundaries via PRIME (e.g., wlroots requires this
functionality). The patch generalizes a pattern that has previously
been implemented by GEM VRAM helpers [1] to work with any driver.
For example, gma500 can now run the wlroots-based sway compositor.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://lore.kernel.org/dri-devel/20230302143502.500661-1-contact@emersion.fr/
---
 drivers/gpu/drm/drm_ioctl.c |  4 ++--
 drivers/gpu/drm/drm_prime.c | 29 ++++++++++++++++++++---------
 include/drm/drm_drv.h       |  6 ++++++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 7c9d66ee917de..faa7018bda0a3 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -245,8 +245,8 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
 		req->value = 1;
 		return 0;
 	case DRM_CAP_PRIME:
-		req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0;
-		req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0;
+		req->value |= DRM_PRIME_CAP_IMPORT;
+		req->value |= DRM_PRIME_CAP_EXPORT;
 		return 0;
 	case DRM_CAP_SYNCOBJ:
 		req->value = drm_core_check_feature(dev, DRIVER_SYNCOBJ);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d29dafce9bb0a..36cecfc7c947b 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -372,11 +372,16 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_prime_handle *args = data;
 
-	if (!dev->driver->prime_fd_to_handle)
-		return -ENOSYS;
+	/*
+	 * TODO: Convert remaining drivers to drm_gem_prime_fd_to_handle()
+	 *       and remove this callback.
+	 */
+	if (dev->driver->prime_fd_to_handle) {
+		return dev->driver->prime_fd_to_handle(dev, file_priv, args->fd,
+						       &args->handle);
+	}
 
-	return dev->driver->prime_fd_to_handle(dev, file_priv,
-			args->fd, &args->handle);
+	return drm_gem_prime_fd_to_handle(dev, file_priv, args->fd, &args->handle);
 }
 
 static struct dma_buf *export_and_register_object(struct drm_device *dev,
@@ -518,15 +523,21 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_prime_handle *args = data;
 
-	if (!dev->driver->prime_handle_to_fd)
-		return -ENOSYS;
-
 	/* check flags are valid */
 	if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
 		return -EINVAL;
 
-	return dev->driver->prime_handle_to_fd(dev, file_priv,
-			args->handle, args->flags, &args->fd);
+	/*
+	 * TODO: Convert remaining drivers to drm_gem_prime_handle_to_fd()
+	 *       and remove this callback.
+	 */
+	if (dev->driver->prime_handle_to_fd) {
+		return dev->driver->prime_handle_to_fd(dev, file_priv,
+						       args->handle, args->flags,
+						       &args->fd);
+	}
+	return drm_gem_prime_handle_to_fd(dev, file_priv, args->handle,
+					  args->flags, &args->fd);
 }
 
 /**
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 89e2706cac561..10af1899236a0 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -309,6 +309,9 @@ struct drm_driver {
 	 *
 	 * For an in-depth discussion see :ref:`PRIME buffer sharing
 	 * documentation <prime_buffer_sharing>`.
+	 *
+	 * TODO: Convert remaining drivers to drm_gem_prime_handle_to_fd()
+	 *       and remove this callback.
 	 */
 	int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv,
 				uint32_t handle, uint32_t flags, int *prime_fd);
@@ -320,6 +323,9 @@ struct drm_driver {
 	 *
 	 * For an in-depth discussion see :ref:`PRIME buffer sharing
 	 * documentation <prime_buffer_sharing>`.
+	 *
+	 * TODO: Convert remaining drivers to drm_gem_prime_fd_to_handle()
+	 *       and remove this callback.
 	 */
 	int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv,
 				int prime_fd, uint32_t *handle);
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] drm: Clear fd/handle callbacks in struct drm_driver
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
  2023-06-15  9:31 ` [PATCH 1/3] drm: Enable PRIME import/export " Thomas Zimmermann
@ 2023-06-15  9:31 ` Thomas Zimmermann
  2023-06-15  9:31 ` [PATCH 3/3] drm/prime: Unexport helpers for fd/handle conversion Thomas Zimmermann
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-15  9:31 UTC (permalink / raw)
  To: daniel, airlied, mripard, maarten.lankhorst, zackr, contact,
	linux-graphics-maintainer
  Cc: Thomas Zimmermann, dri-devel

Clear all assignments of struct drm_driver's fd/handle callbacks to
drm_gem_prime_fd_to_handle() and drm_gem_prime_handle_to_fd(). These
functions are called by default. Add a TODO item to convert vmwgfx
to the defaults as well.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 Documentation/gpu/todo.rst                    | 14 ++++++++++++++
 drivers/accel/ivpu/ivpu_drv.c                 |  2 --
 drivers/accel/qaic/qaic_drv.c                 |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  2 --
 drivers/gpu/drm/armada/armada_drv.c           |  2 --
 drivers/gpu/drm/drm_prime.c                   | 13 ++++---------
 drivers/gpu/drm/etnaviv/etnaviv_drv.c         |  2 --
 drivers/gpu/drm/exynos/exynos_drm_drv.c       |  2 --
 drivers/gpu/drm/i915/i915_driver.c            |  2 --
 drivers/gpu/drm/lima/lima_drv.c               |  2 --
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  2 --
 drivers/gpu/drm/msm/msm_drv.c                 |  2 --
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  2 --
 drivers/gpu/drm/omapdrm/omap_drv.c            |  2 --
 drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 --
 drivers/gpu/drm/pl111/pl111_drv.c             |  2 --
 drivers/gpu/drm/qxl/qxl_drv.c                 |  2 --
 drivers/gpu/drm/radeon/radeon_drv.c           |  2 --
 drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c |  2 --
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  2 --
 drivers/gpu/drm/tegra/drm.c                   |  2 --
 drivers/gpu/drm/v3d/v3d_drv.c                 |  2 --
 drivers/gpu/drm/virtio/virtgpu_drv.c          |  2 --
 drivers/gpu/drm/xen/xen_drm_front.c           |  2 --
 include/drm/drm_gem_dma_helper.h              | 12 ++++--------
 include/drm/drm_gem_shmem_helper.h            |  6 ++----
 include/drm/drm_gem_vram_helper.h             |  4 +---
 27 files changed, 25 insertions(+), 67 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 68bdafa0284f5..a6971aa7c164c 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -647,6 +647,20 @@ See drivers/gpu/drm/amd/display/TODO for tasks.
 
 Contact: Harry Wentland, Alex Deucher
 
+vmwgfx: Clean up PRIME callbacks
+--------------------------------
+
+Vmwgfx is the only driver that sets struct drm_driver.prime_handle_to_fd and
+struct drm_driver.prime_fd_to_handle. Convert it to use struct
+drm_driver.gem_prime_import and struct drm_gem_object_funcs.export, plus the
+respective helpers.
+
+Afterwards remove struct drm_driver.prime_handle_to_fd and struct
+drm_driver.prime_fd_to_handle.
+
+Level: Advanced
+
+
 Bootsplash
 ==========
 
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 2df7643b843d5..50c9efcc04cd6 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -373,8 +373,6 @@ static const struct drm_driver driver = {
 
 	.open = ivpu_open,
 	.postclose = ivpu_postclose,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = ivpu_gem_prime_import,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index b5ba550a0c040..b5de82e6eb4d5 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -165,7 +165,6 @@ static const struct drm_driver qaic_accel_driver = {
 
 	.ioctls			= qaic_drm_ioctls,
 	.num_ioctls		= ARRAY_SIZE(qaic_drm_ioctls),
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import	= qaic_gem_prime_import,
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c9a41c997c6c7..5bac46a49eb77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2806,8 +2806,6 @@ static const struct drm_driver amdgpu_kms_driver = {
 	.show_fdinfo = amdgpu_show_fdinfo,
 #endif
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = amdgpu_gem_prime_import,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index e120144d4b470..e8d2fe955909a 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -37,8 +37,6 @@ static const struct drm_ioctl_desc armada_ioctls[] = {
 DEFINE_DRM_GEM_FOPS(armada_drm_fops);
 
 static const struct drm_driver armada_drm_driver = {
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import	= armada_gem_prime_import,
 	.dumb_create		= armada_gem_dumb_create,
 	.major			= 1,
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 36cecfc7c947b..b857a45a29553 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -51,15 +51,10 @@ MODULE_IMPORT_NS(DMA_BUF);
  * between applications, they can't be guessed like the globally unique GEM
  * names.
  *
- * Drivers that support the PRIME API implement the
- * &drm_driver.prime_handle_to_fd and &drm_driver.prime_fd_to_handle operations.
- * GEM based drivers must use drm_gem_prime_handle_to_fd() and
- * drm_gem_prime_fd_to_handle() to implement these. For GEM based drivers the
- * actual driver interfaces is provided through the &drm_gem_object_funcs.export
- * and &drm_driver.gem_prime_import hooks.
- *
- * &dma_buf_ops implementations for GEM drivers are all individually exported
- * for drivers which need to overwrite or reimplement some of them.
+ * Drivers that support the PRIME API implement the drm_gem_object_funcs.export
+ * and &drm_driver.gem_prime_import hooks. &dma_buf_ops implementations for
+ * drivers are all individually exported for drivers which need to overwrite
+ * or reimplement some of them.
  *
  * Reference Counting for GEM Drivers
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 31a7f59ccb49e..13188b826d73b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -481,8 +481,6 @@ static const struct drm_driver etnaviv_drm_driver = {
 	.driver_features    = DRIVER_GEM | DRIVER_RENDER,
 	.open               = etnaviv_open,
 	.postclose           = etnaviv_postclose,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = etnaviv_gem_prime_import_sg_table,
 	.gem_prime_mmap     = drm_gem_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 6b73fb7a83c3c..89841369bc237 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -109,8 +109,6 @@ static const struct drm_driver exynos_drm_driver = {
 	.open			= exynos_drm_open,
 	.postclose		= exynos_drm_postclose,
 	.dumb_create		= exynos_drm_gem_dumb_create,
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import	= exynos_drm_gem_prime_import,
 	.gem_prime_import_sg_table	= exynos_drm_gem_prime_import_sg_table,
 	.gem_prime_mmap		= drm_gem_prime_mmap,
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 75cbc43b326dd..171f4db9e5e31 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1818,8 +1818,6 @@ static const struct drm_driver i915_drm_driver = {
 	.postclose = i915_driver_postclose,
 	.show_fdinfo = i915_drm_client_fdinfo,
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = i915_gem_prime_import,
 
 	.dumb_create = i915_gem_dumb_create,
diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
index e8566211e9fa1..c8fca3f0f2122 100644
--- a/drivers/gpu/drm/lima/lima_drv.c
+++ b/drivers/gpu/drm/lima/lima_drv.c
@@ -276,9 +276,7 @@ static const struct drm_driver lima_drm_driver = {
 	.patchlevel         = 0,
 
 	.gem_create_object  = lima_gem_create_object,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 };
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 6dcb4ba2466c0..672eacde9cd19 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -556,8 +556,6 @@ static const struct drm_driver mtk_drm_driver = {
 
 	.dumb_create = mtk_drm_gem_dumb_create,
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = mtk_drm_gem_prime_import,
 	.gem_prime_import_sg_table = mtk_gem_prime_import_sg_table,
 	.gem_prime_mmap = drm_gem_prime_mmap,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 891eff8433a9c..b0bc80a11b0af 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1086,8 +1086,6 @@ static const struct drm_driver msm_driver = {
 	.postclose          = msm_postclose,
 	.dumb_create        = msm_gem_dumb_create,
 	.dumb_map_offset    = msm_gem_dumb_map_offset,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
 	.gem_prime_mmap     = msm_gem_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index cc7c5b4a05fd8..f956c3e416b4c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1234,8 +1234,6 @@ driver_stub = {
 	.num_ioctls = ARRAY_SIZE(nouveau_ioctls),
 	.fops = &nouveau_driver_fops,
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 671d26b9d339e..e2697fe80e62b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -655,8 +655,6 @@ static const struct drm_driver omap_drm_driver = {
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = omap_debugfs_init,
 #endif
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = omap_gem_prime_import,
 	.dumb_create = omap_gem_dumb_create,
 	.dumb_map_offset = omap_gem_dumb_map_offset,
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 19f8cff52e533..d3659fc87d832 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -539,8 +539,6 @@ static const struct drm_driver panfrost_drm_driver = {
 	.minor			= 2,
 
 	.gem_create_object	= panfrost_gem_create_object,
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
 	.gem_prime_mmap		= drm_gem_prime_mmap,
 };
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 43049c8028b21..6da89e1094c29 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -224,8 +224,6 @@ static const struct drm_driver pl111_drm_driver = {
 	.minor = 0,
 	.patchlevel = 0,
 	.dumb_create = drm_gem_dma_dumb_create,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = pl111_gem_import_sg_table,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index a3b83f89e0616..b30ede1cf62d3 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -290,8 +290,6 @@ static struct drm_driver qxl_driver = {
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = qxl_debugfs_init,
 #endif
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
 	.fops = &qxl_fops,
 	.ioctls = qxl_ioctls,
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index e4374814f0ef6..fcfd279a13e26 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -604,8 +604,6 @@ static const struct drm_driver kms_driver = {
 	.dumb_map_offset = radeon_mode_dumb_mmap,
 	.fops = &radeon_driver_kms_fops,
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
index ed3ee3d15baec..76a3352bdfd68 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
@@ -605,8 +605,6 @@ DEFINE_DRM_GEM_DMA_FOPS(rcar_du_fops);
 static const struct drm_driver rcar_du_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 	.dumb_create		= rcar_du_dumb_create,
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
 	.gem_prime_mmap		= drm_gem_prime_mmap,
 	.fops			= &rcar_du_fops,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index c0ebfdf56a711..ab21366b96438 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -224,8 +224,6 @@ DEFINE_DRM_GEM_FOPS(rockchip_drm_driver_fops);
 static const struct drm_driver rockchip_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 	.dumb_create		= rockchip_gem_dumb_create,
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table	= rockchip_gem_prime_import_sg_table,
 	.gem_prime_mmap		= drm_gem_prime_mmap,
 	.fops			= &rockchip_drm_driver_fops,
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 35ff303c6674f..ff36171c8fb70 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -887,8 +887,6 @@ static const struct drm_driver tegra_drm_driver = {
 	.debugfs_init = tegra_debugfs_init,
 #endif
 
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = tegra_gem_prime_import,
 
 	.dumb_create = tegra_bo_dumb_create,
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 71f9fdde24b41..89536e8115ca4 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -171,8 +171,6 @@ static const struct drm_driver v3d_drm_driver = {
 #endif
 
 	.gem_create_object = v3d_create_object,
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = v3d_prime_import_sg_table,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index add075681e18f..2798940c3b97c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -186,8 +186,6 @@ static const struct drm_driver driver = {
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = virtio_gpu_debugfs_init,
 #endif
-	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_mmap = drm_gem_prime_mmap,
 	.gem_prime_import = virtgpu_gem_prime_import,
 	.gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table,
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c
index 90996c108146d..eb6c399adc0be 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -474,8 +474,6 @@ DEFINE_DRM_GEM_FOPS(xen_drm_dev_fops);
 static const struct drm_driver xen_drm_driver = {
 	.driver_features           = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 	.release                   = xen_drm_drv_release,
-	.prime_handle_to_fd        = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle        = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = xen_drm_front_gem_import_sg_table,
 	.gem_prime_mmap            = drm_gem_prime_mmap,
 	.dumb_create               = xen_drm_drv_dumb_create,
diff --git a/include/drm/drm_gem_dma_helper.h b/include/drm/drm_gem_dma_helper.h
index 8a043235dad81..e2c40c6d496e3 100644
--- a/include/drm/drm_gem_dma_helper.h
+++ b/include/drm/drm_gem_dma_helper.h
@@ -166,11 +166,9 @@ drm_gem_dma_prime_import_sg_table(struct drm_device *dev,
  * DRM_GEM_DMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE() instead.
  */
 #define DRM_GEM_DMA_DRIVER_OPS_WITH_DUMB_CREATE(dumb_create_func) \
-	.dumb_create		= (dumb_create_func), \
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
+	.dumb_create		   = (dumb_create_func), \
 	.gem_prime_import_sg_table = drm_gem_dma_prime_import_sg_table, \
-	.gem_prime_mmap		= drm_gem_prime_mmap
+	.gem_prime_mmap		   = drm_gem_prime_mmap
 
 /**
  * DRM_GEM_DMA_DRIVER_OPS - DMA GEM driver operations
@@ -204,11 +202,9 @@ drm_gem_dma_prime_import_sg_table(struct drm_device *dev,
  * DRM_GEM_DMA_DRIVER_OPS_WITH_DUMB_CREATE() instead.
  */
 #define DRM_GEM_DMA_DRIVER_OPS_VMAP_WITH_DUMB_CREATE(dumb_create_func) \
-	.dumb_create		= dumb_create_func, \
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
+	.dumb_create		   = (dumb_create_func), \
 	.gem_prime_import_sg_table = drm_gem_dma_prime_import_sg_table_vmap, \
-	.gem_prime_mmap		= drm_gem_prime_mmap
+	.gem_prime_mmap		   = drm_gem_prime_mmap
 
 /**
  * DRM_GEM_DMA_DRIVER_OPS_VMAP - DMA GEM driver operations ensuring a virtual
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index 5994fed5e3278..7b40d7bff517e 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -290,10 +290,8 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
  * the &drm_driver structure.
  */
 #define DRM_GEM_SHMEM_DRIVER_OPS \
-	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
-	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
 	.gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
-	.gem_prime_mmap		= drm_gem_prime_mmap, \
-	.dumb_create		= drm_gem_shmem_dumb_create
+	.gem_prime_mmap		   = drm_gem_prime_mmap, \
+	.dumb_create		   = drm_gem_shmem_dumb_create
 
 #endif /* __DRM_GEM_SHMEM_HELPER_H__ */
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index f4aab64411d82..d3e8920c0b643 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -160,9 +160,7 @@ void drm_gem_vram_simple_display_pipe_cleanup_fb(
 	.debugfs_init             = drm_vram_mm_debugfs_init, \
 	.dumb_create		  = drm_gem_vram_driver_dumb_create, \
 	.dumb_map_offset	  = drm_gem_ttm_dumb_map_offset, \
-	.gem_prime_mmap		  = drm_gem_prime_mmap, \
-	.prime_handle_to_fd	  = drm_gem_prime_handle_to_fd, \
-	.prime_fd_to_handle	  = drm_gem_prime_fd_to_handle
+	.gem_prime_mmap		  = drm_gem_prime_mmap
 
 /*
  *  VRAM memory manager
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] drm/prime: Unexport helpers for fd/handle conversion
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
  2023-06-15  9:31 ` [PATCH 1/3] drm: Enable PRIME import/export " Thomas Zimmermann
  2023-06-15  9:31 ` [PATCH 2/3] drm: Clear fd/handle callbacks in struct drm_driver Thomas Zimmermann
@ 2023-06-15  9:31 ` Thomas Zimmermann
  2023-06-15 14:50 ` [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Simon Ser
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-15  9:31 UTC (permalink / raw)
  To: daniel, airlied, mripard, maarten.lankhorst, zackr, contact,
	linux-graphics-maintainer
  Cc: Thomas Zimmermann, dri-devel

Unexport drm_gem_prime_fd_to_handle() and drm_gem_prime_handle_to_fd().
Both are only used internally within the PRIME code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_prime.c | 20 +++++++++-----------
 include/drm/drm_prime.h     |  7 -------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index b857a45a29553..2b11fc2444cff 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -278,7 +278,7 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 }
 EXPORT_SYMBOL(drm_gem_dmabuf_release);
 
-/**
+/*
  * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
  * @dev: drm_device to import into
  * @file_priv: drm file-private structure
@@ -292,9 +292,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
  *
  * Returns 0 on success or a negative error code on failure.
  */
-int drm_gem_prime_fd_to_handle(struct drm_device *dev,
-			       struct drm_file *file_priv, int prime_fd,
-			       uint32_t *handle)
+static int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+				      struct drm_file *file_priv, int prime_fd,
+				      uint32_t *handle)
 {
 	struct dma_buf *dma_buf;
 	struct drm_gem_object *obj;
@@ -360,7 +360,6 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	dma_buf_put(dma_buf);
 	return ret;
 }
-EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
@@ -413,7 +412,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
 	return dmabuf;
 }
 
-/**
+/*
  * drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
  * @dev: dev to export the buffer from
  * @file_priv: drm file-private structure
@@ -426,10 +425,10 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
  * The actual exporting from GEM object to a dma-buf is done through the
  * &drm_gem_object_funcs.export callback.
  */
-int drm_gem_prime_handle_to_fd(struct drm_device *dev,
-			       struct drm_file *file_priv, uint32_t handle,
-			       uint32_t flags,
-			       int *prime_fd)
+static int drm_gem_prime_handle_to_fd(struct drm_device *dev,
+				      struct drm_file *file_priv, uint32_t handle,
+				      uint32_t flags,
+				      int *prime_fd)
 {
 	struct drm_gem_object *obj;
 	int ret = 0;
@@ -511,7 +510,6 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
 
 int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
 				 struct drm_file *file_priv)
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 2a1d01e5b56b8..a7abf9f3e6972 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -60,19 +60,12 @@ enum dma_data_direction;
 
 struct drm_device;
 struct drm_gem_object;
-struct drm_file;
 
 /* core prime functions */
 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
 				      struct dma_buf_export_info *exp_info);
 void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
 
-int drm_gem_prime_fd_to_handle(struct drm_device *dev,
-			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
-int drm_gem_prime_handle_to_fd(struct drm_device *dev,
-			       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
-			       int *prime_fd);
-
 /* helper functions for exporting */
 int drm_gem_map_attach(struct dma_buf *dma_buf,
 		       struct dma_buf_attachment *attach);
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] drm: Enable PRIME import/export for all drivers
  2023-06-15  9:31 ` [PATCH 1/3] drm: Enable PRIME import/export " Thomas Zimmermann
@ 2023-06-15 14:47   ` Simon Ser
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Ser @ 2023-06-15 14:47 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: mripard, linux-graphics-maintainer, dri-devel

On Thursday, June 15th, 2023 at 11:31, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 89e2706cac561..10af1899236a0 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -309,6 +309,9 @@ struct drm_driver {
>  	 *
>  	 * For an in-depth discussion see :ref:`PRIME buffer sharing
>  	 * documentation <prime_buffer_sharing>`.
> +	 *
> +	 * TODO: Convert remaining drivers to drm_gem_prime_handle_to_fd()
> +	 *       and remove this callback.
>  	 */

The docs right above still state: "Should be implemented with
drm_gem_prime_handle_to_fd() for GEM based drivers". Maybe we can replace that
and state that leaving this NULL will use a default implementation?

>  	int (*prime_handle_to_fd)(struct drm_device *dev, struct drm_file *file_priv,
>  				uint32_t handle, uint32_t flags, int *prime_fd);
> @@ -320,6 +323,9 @@ struct drm_driver {
>  	 *
>  	 * For an in-depth discussion see :ref:`PRIME buffer sharing
>  	 * documentation <prime_buffer_sharing>`.
> +	 *
> +	 * TODO: Convert remaining drivers to drm_gem_prime_fd_to_handle()
> +	 *       and remove this callback.
>  	 */

Ditto.

>  	int (*prime_fd_to_handle)(struct drm_device *dev, struct drm_file *file_priv,
>  				int prime_fd, uint32_t *handle);
> --
> 2.41.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2023-06-15  9:31 ` [PATCH 3/3] drm/prime: Unexport helpers for fd/handle conversion Thomas Zimmermann
@ 2023-06-15 14:50 ` Simon Ser
  2023-06-15 14:56   ` Thomas Zimmermann
  2023-06-16  3:59 ` Zack Rusin
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Simon Ser @ 2023-06-15 14:50 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: mripard, linux-graphics-maintainer, dri-devel

On Thursday, June 15th, 2023 at 11:31, Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
> for all DRM drivers. Even drivers that do not support PRIME import
> or export of dma-bufs can now import their own buffer objects. This
> is required by some userspace, such as wlroots-based compositors, to
> share buffers among processes.
> 
> The only driver that does not use the drm_gem_prime_*() helpers is
> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
> can be removed.
> 
> Simon Ser implemented the feature for drivers based on GEM VRAM helpers
> in [1]. This patchset generalizes the code for all drivers that do not
> otherwise support PRIME. Tested by running sway with gma500 hardware.

Very nice! Thanks a lot for doing this!

Just one minor comment about docs. I think there are also some remaining
references to drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
in the drm_prime.c overview. These become stale since this series unexports
these functions.

With that fixed:

Reviewed-by: Simon Ser <contact@emersion.fr>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-15 14:50 ` [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Simon Ser
@ 2023-06-15 14:56   ` Thomas Zimmermann
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-15 14:56 UTC (permalink / raw)
  To: Simon Ser; +Cc: mripard, linux-graphics-maintainer, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

Hi

Am 15.06.23 um 16:50 schrieb Simon Ser:
> On Thursday, June 15th, 2023 at 11:31, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
>> for all DRM drivers. Even drivers that do not support PRIME import
>> or export of dma-bufs can now import their own buffer objects. This
>> is required by some userspace, such as wlroots-based compositors, to
>> share buffers among processes.
>>
>> The only driver that does not use the drm_gem_prime_*() helpers is
>> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
>> can be removed.
>>
>> Simon Ser implemented the feature for drivers based on GEM VRAM helpers
>> in [1]. This patchset generalizes the code for all drivers that do not
>> otherwise support PRIME. Tested by running sway with gma500 hardware.
> 
> Very nice! Thanks a lot for doing this!
> 
> Just one minor comment about docs. I think there are also some remaining
> references to drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
> in the drm_prime.c overview. These become stale since this series unexports
> these functions.

I'll address the documentation issue.

> 
> With that fixed:
> 
> Reviewed-by: Simon Ser <contact@emersion.fr>

Thanks a lot.

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2023-06-15 14:50 ` [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Simon Ser
@ 2023-06-16  3:59 ` Zack Rusin
  2023-06-16  7:11   ` Thomas Zimmermann
  2023-06-16 17:00 ` Jeffrey Hugo
  2023-06-16 17:22 ` Alex Deucher
  6 siblings, 1 reply; 12+ messages in thread
From: Zack Rusin @ 2023-06-16  3:59 UTC (permalink / raw)
  To: daniel, maarten.lankhorst, mripard, airlied,
	Linux-graphics-maintainer, tzimmermann, contact
  Cc: dri-devel

On Thu, 2023-06-15 at 11:31 +0200, Thomas Zimmermann wrote:
> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
> for all DRM drivers. Even drivers that do not support PRIME import
> or export of dma-bufs can now import their own buffer objects. This
> is required by some userspace, such as wlroots-based compositors, to
> share buffers among processes.
> 
> The only driver that does not use the drm_gem_prime_*() helpers is
> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
> can be removed.

Hmm, I'm not sure that's ever going to be possible on vmwgfx. Or at least not until
Xorg is used by anything. Some things in vmwgfx stack create "surfaces" which are
not GEM's (they can be backed by a GEM object, but don't have to) and the prime
implementation on vmwgfx has to be able to export and import those. 

In an ideal world I'd just delete the ioctl's that allow creating those "surfaces",
but of course that's a no-no, so possibility of introducing vmwgfx2 with a
saner/modern ioctl model has been floating around internally. Which at least would
open a path to removing vmwgfx at some point in the future, but there's not much
that can be done about vmwgfx having to be able to prime import/export GEM's and
wonky non-GEM objects.

z


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-16  3:59 ` Zack Rusin
@ 2023-06-16  7:11   ` Thomas Zimmermann
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-16  7:11 UTC (permalink / raw)
  To: Zack Rusin, daniel, maarten.lankhorst, mripard, airlied,
	Linux-graphics-maintainer, contact
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2251 bytes --]

Hi

Am 16.06.23 um 05:59 schrieb Zack Rusin:
> On Thu, 2023-06-15 at 11:31 +0200, Thomas Zimmermann wrote:
>> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
>> for all DRM drivers. Even drivers that do not support PRIME import
>> or export of dma-bufs can now import their own buffer objects. This
>> is required by some userspace, such as wlroots-based compositors, to
>> share buffers among processes.
>>
>> The only driver that does not use the drm_gem_prime_*() helpers is
>> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
>> can be removed.
> 
> Hmm, I'm not sure that's ever going to be possible on vmwgfx. Or at least not until
> Xorg is used by anything. Some things in vmwgfx stack create "surfaces" which are
> not GEM's (they can be backed by a GEM object, but don't have to) and the prime
> implementation on vmwgfx has to be able to export and import those.

I see that's not easily fixable, if at all. I'd remove the TODO item and 
change the callback comments to say that they are reserved for vmwgfx.

> 
> In an ideal world I'd just delete the ioctl's that allow creating those "surfaces",
> but of course that's a no-no, so possibility of introducing vmwgfx2 with a
> saner/modern ioctl model has been floating around internally. Which at least would
> open a path to removing vmwgfx at some point in the future, but there's not much
> that can be done about vmwgfx having to be able to prime import/export GEM's and
> wonky non-GEM objects.

We have removed ioctls before, but over years so that userspace had time 
to update. Removing the ioctls would require a change to the driver 
version, I think.

You could explore the option of implementing GEM-based objects with the 
GEM helpers and the non-GEM objects with the current code. If done 
correctly, the old non-GEM support could be hidden behind a Kconfig option.

Anyway, I'll change the patchset to reflect the current state.

Best regards
Thomas

> 
> z
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2023-06-16  3:59 ` Zack Rusin
@ 2023-06-16 17:00 ` Jeffrey Hugo
  2023-06-19  8:52   ` Thomas Zimmermann
  2023-06-16 17:22 ` Alex Deucher
  6 siblings, 1 reply; 12+ messages in thread
From: Jeffrey Hugo @ 2023-06-16 17:00 UTC (permalink / raw)
  To: Thomas Zimmermann, daniel, airlied, mripard, maarten.lankhorst,
	zackr, contact, linux-graphics-maintainer
  Cc: dri-devel

On 6/15/2023 3:31 AM, Thomas Zimmermann wrote:
> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
> for all DRM drivers. Even drivers that do not support PRIME import
> or export of dma-bufs can now import their own buffer objects. This
> is required by some userspace, such as wlroots-based compositors, to
> share buffers among processes.
> 
> The only driver that does not use the drm_gem_prime_*() helpers is
> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
> can be removed.
> 
> Simon Ser implemented the feature for drivers based on GEM VRAM helpers
> in [1]. This patchset generalizes the code for all drivers that do not
> otherwise support PRIME. Tested by running sway with gma500 hardware.
> 
> [1] https://lore.kernel.org/dri-devel/20230302143502.500661-1-contact@emersion.fr/
> 
> Thomas Zimmermann (3):
>    drm: Enable PRIME import/export for all drivers
>    drm: Clear fd/handle callbacks in struct drm_driver
>    drm/prime: Unexport helpers for fd/handle conversion
> 
>   Documentation/gpu/todo.rst                    | 14 +++++
>   drivers/accel/ivpu/ivpu_drv.c                 |  2 -
>   drivers/accel/qaic/qaic_drv.c                 |  1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  2 -
>   drivers/gpu/drm/armada/armada_drv.c           |  2 -
>   drivers/gpu/drm/drm_ioctl.c                   |  4 +-
>   drivers/gpu/drm/drm_prime.c                   | 62 ++++++++++---------
>   drivers/gpu/drm/etnaviv/etnaviv_drv.c         |  2 -
>   drivers/gpu/drm/exynos/exynos_drm_drv.c       |  2 -
>   drivers/gpu/drm/i915/i915_driver.c            |  2 -
>   drivers/gpu/drm/lima/lima_drv.c               |  2 -
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  2 -
>   drivers/gpu/drm/msm/msm_drv.c                 |  2 -
>   drivers/gpu/drm/nouveau/nouveau_drm.c         |  2 -
>   drivers/gpu/drm/omapdrm/omap_drv.c            |  2 -
>   drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 -
>   drivers/gpu/drm/pl111/pl111_drv.c             |  2 -
>   drivers/gpu/drm/qxl/qxl_drv.c                 |  2 -
>   drivers/gpu/drm/radeon/radeon_drv.c           |  2 -
>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c |  2 -
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  2 -
>   drivers/gpu/drm/tegra/drm.c                   |  2 -
>   drivers/gpu/drm/v3d/v3d_drv.c                 |  2 -
>   drivers/gpu/drm/virtio/virtgpu_drv.c          |  2 -
>   drivers/gpu/drm/xen/xen_drm_front.c           |  2 -
>   include/drm/drm_drv.h                         |  6 ++
>   include/drm/drm_gem_dma_helper.h              | 12 ++--
>   include/drm/drm_gem_shmem_helper.h            |  6 +-
>   include/drm/drm_gem_vram_helper.h             |  4 +-
>   include/drm/drm_prime.h                       |  7 ---
>   30 files changed, 62 insertions(+), 96 deletions(-)
> 

Surely the maintainers of the 22 drivers touched by this series should 
be in the to list or something?  Speaking for myself, threads that 
directly address me trigger my "priority" filters.

Series looks sane to me, but I'd like to see what you do for a v2 to 
address the existing comments.

For the QAIC part -

Acked-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

-Jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2023-06-16 17:00 ` Jeffrey Hugo
@ 2023-06-16 17:22 ` Alex Deucher
  6 siblings, 0 replies; 12+ messages in thread
From: Alex Deucher @ 2023-06-16 17:22 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: mripard, linux-graphics-maintainer, dri-devel

Series looks good to me:

Acked-by: Alex Deucher <alexander.deucher@amd.com>

On Thu, Jun 15, 2023 at 5:42 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
> for all DRM drivers. Even drivers that do not support PRIME import
> or export of dma-bufs can now import their own buffer objects. This
> is required by some userspace, such as wlroots-based compositors, to
> share buffers among processes.
>
> The only driver that does not use the drm_gem_prime_*() helpers is
> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
> can be removed.
>
> Simon Ser implemented the feature for drivers based on GEM VRAM helpers
> in [1]. This patchset generalizes the code for all drivers that do not
> otherwise support PRIME. Tested by running sway with gma500 hardware.
>
> [1] https://lore.kernel.org/dri-devel/20230302143502.500661-1-contact@emersion.fr/
>
> Thomas Zimmermann (3):
>   drm: Enable PRIME import/export for all drivers
>   drm: Clear fd/handle callbacks in struct drm_driver
>   drm/prime: Unexport helpers for fd/handle conversion
>
>  Documentation/gpu/todo.rst                    | 14 +++++
>  drivers/accel/ivpu/ivpu_drv.c                 |  2 -
>  drivers/accel/qaic/qaic_drv.c                 |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  2 -
>  drivers/gpu/drm/armada/armada_drv.c           |  2 -
>  drivers/gpu/drm/drm_ioctl.c                   |  4 +-
>  drivers/gpu/drm/drm_prime.c                   | 62 ++++++++++---------
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c         |  2 -
>  drivers/gpu/drm/exynos/exynos_drm_drv.c       |  2 -
>  drivers/gpu/drm/i915/i915_driver.c            |  2 -
>  drivers/gpu/drm/lima/lima_drv.c               |  2 -
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  2 -
>  drivers/gpu/drm/msm/msm_drv.c                 |  2 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c         |  2 -
>  drivers/gpu/drm/omapdrm/omap_drv.c            |  2 -
>  drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 -
>  drivers/gpu/drm/pl111/pl111_drv.c             |  2 -
>  drivers/gpu/drm/qxl/qxl_drv.c                 |  2 -
>  drivers/gpu/drm/radeon/radeon_drv.c           |  2 -
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c |  2 -
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  2 -
>  drivers/gpu/drm/tegra/drm.c                   |  2 -
>  drivers/gpu/drm/v3d/v3d_drv.c                 |  2 -
>  drivers/gpu/drm/virtio/virtgpu_drv.c          |  2 -
>  drivers/gpu/drm/xen/xen_drm_front.c           |  2 -
>  include/drm/drm_drv.h                         |  6 ++
>  include/drm/drm_gem_dma_helper.h              | 12 ++--
>  include/drm/drm_gem_shmem_helper.h            |  6 +-
>  include/drm/drm_gem_vram_helper.h             |  4 +-
>  include/drm/drm_prime.h                       |  7 ---
>  30 files changed, 62 insertions(+), 96 deletions(-)
>
> --
> 2.41.0
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers
  2023-06-16 17:00 ` Jeffrey Hugo
@ 2023-06-19  8:52   ` Thomas Zimmermann
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2023-06-19  8:52 UTC (permalink / raw)
  To: Jeffrey Hugo, daniel, airlied, mripard, maarten.lankhorst, zackr,
	contact, linux-graphics-maintainer
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 4522 bytes --]

Hi

Am 16.06.23 um 19:00 schrieb Jeffrey Hugo:
> On 6/15/2023 3:31 AM, Thomas Zimmermann wrote:
>> Set drm_gem_prime_handle_to_fd() and drm_gem_prime_fd_to_handle()
>> for all DRM drivers. Even drivers that do not support PRIME import
>> or export of dma-bufs can now import their own buffer objects. This
>> is required by some userspace, such as wlroots-based compositors, to
>> share buffers among processes.
>>
>> The only driver that does not use the drm_gem_prime_*() helpers is
>> vmwgfx. Once it has been converted, the callbacks in struct drm_driver
>> can be removed.
>>
>> Simon Ser implemented the feature for drivers based on GEM VRAM helpers
>> in [1]. This patchset generalizes the code for all drivers that do not
>> otherwise support PRIME. Tested by running sway with gma500 hardware.
>>
>> [1] 
>> https://lore.kernel.org/dri-devel/20230302143502.500661-1-contact@emersion.fr/
>>
>> Thomas Zimmermann (3):
>>    drm: Enable PRIME import/export for all drivers
>>    drm: Clear fd/handle callbacks in struct drm_driver
>>    drm/prime: Unexport helpers for fd/handle conversion
>>
>>   Documentation/gpu/todo.rst                    | 14 +++++
>>   drivers/accel/ivpu/ivpu_drv.c                 |  2 -
>>   drivers/accel/qaic/qaic_drv.c                 |  1 -
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  2 -
>>   drivers/gpu/drm/armada/armada_drv.c           |  2 -
>>   drivers/gpu/drm/drm_ioctl.c                   |  4 +-
>>   drivers/gpu/drm/drm_prime.c                   | 62 ++++++++++---------
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.c         |  2 -
>>   drivers/gpu/drm/exynos/exynos_drm_drv.c       |  2 -
>>   drivers/gpu/drm/i915/i915_driver.c            |  2 -
>>   drivers/gpu/drm/lima/lima_drv.c               |  2 -
>>   drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  2 -
>>   drivers/gpu/drm/msm/msm_drv.c                 |  2 -
>>   drivers/gpu/drm/nouveau/nouveau_drm.c         |  2 -
>>   drivers/gpu/drm/omapdrm/omap_drv.c            |  2 -
>>   drivers/gpu/drm/panfrost/panfrost_drv.c       |  2 -
>>   drivers/gpu/drm/pl111/pl111_drv.c             |  2 -
>>   drivers/gpu/drm/qxl/qxl_drv.c                 |  2 -
>>   drivers/gpu/drm/radeon/radeon_drv.c           |  2 -
>>   drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c |  2 -
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c   |  2 -
>>   drivers/gpu/drm/tegra/drm.c                   |  2 -
>>   drivers/gpu/drm/v3d/v3d_drv.c                 |  2 -
>>   drivers/gpu/drm/virtio/virtgpu_drv.c          |  2 -
>>   drivers/gpu/drm/xen/xen_drm_front.c           |  2 -
>>   include/drm/drm_drv.h                         |  6 ++
>>   include/drm/drm_gem_dma_helper.h              | 12 ++--
>>   include/drm/drm_gem_shmem_helper.h            |  6 +-
>>   include/drm/drm_gem_vram_helper.h             |  4 +-
>>   include/drm/drm_prime.h                       |  7 ---
>>   30 files changed, 62 insertions(+), 96 deletions(-)
>>
> 
> Surely the maintainers of the 22 drivers touched by this series should 
> be in the to list or something?  Speaking for myself, threads that 
> directly address me trigger my "priority" filters.

My apologies. I didn't want to snub you or anyone else. This change 
should not have an effect on *any* of these drivers. It's just clearing 
a pointer assignment that is now the default. And adding the maintainers 
of all affected drivers would add all maintainers of DRM code, hence I 
decided to not do that.  Please note that the one maintainer that 
actually is affected, Zack, is in CC.

> 
> Series looks sane to me, but I'd like to see what you do for a v2 to 
> address the existing comments.

You'll be cc'ed.

> 
> For the QAIC part -
> 
> Acked-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Thanks a lot.

Best regards
Thomas

> 
> -Jeff

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-19  8:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-15  9:31 [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Thomas Zimmermann
2023-06-15  9:31 ` [PATCH 1/3] drm: Enable PRIME import/export " Thomas Zimmermann
2023-06-15 14:47   ` Simon Ser
2023-06-15  9:31 ` [PATCH 2/3] drm: Clear fd/handle callbacks in struct drm_driver Thomas Zimmermann
2023-06-15  9:31 ` [PATCH 3/3] drm/prime: Unexport helpers for fd/handle conversion Thomas Zimmermann
2023-06-15 14:50 ` [PATCH 0/3] drm: Allow PRIME 'self-import' for all drivers Simon Ser
2023-06-15 14:56   ` Thomas Zimmermann
2023-06-16  3:59 ` Zack Rusin
2023-06-16  7:11   ` Thomas Zimmermann
2023-06-16 17:00 ` Jeffrey Hugo
2023-06-19  8:52   ` Thomas Zimmermann
2023-06-16 17:22 ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).