Hi Simon,
Thanks for working on this feature!
On 11/9/23 04:45, Simon Ser wrote:
User-space sometimes needs to allocate scanout-capable memory for
GPU rendering purposes. On a vc4/v3d split render/display SoC, this
is achieved via DRM dumb buffers: the v3d user-space driver opens
the primary vc4 node, allocates a DRM dumb buffer there, exports it
as a DMA-BUF, imports it into the v3d render node, and renders to it.
However, DRM dumb buffers are only meant for CPU rendering, they are
not intended to be used for GPU rendering. Primary nodes should only
be used for mode-setting purposes, other programs should not attempt
to open it. Moreover, opening the primary node is already broken on
some setups: systemd grants permission to open primary nodes to
physically logged in users, but this breaks when the user is not
physically logged in (e.g. headless setup) and when the distribution
is using a different init (e.g. Alpine Linux uses openrc).
We need an alternate way for v3d to allocate scanout-capable memory.
For me, it is a bit unclear how we import the vc4 DMA-BUF heap into v3d.
Should we create an IOCTL for it on v3d?
Also, if you need some help testing with the RPi 5, you can ping on IRC
and I can try to help by testing.
Best Regards,
- Maíra
Leverage DMA heaps for this purpose: expose a CMA heap to user-space.
Preliminary testing has been done with wlroots [1].
This is still an RFC. Open questions:
- Does this approach make sense to y'all in general?
- What would be a good name for the heap? "vc4" is maybe a bit naive and
not precise enough. Something with "cma"? Do we need to plan a naming
scheme to accomodate for multiple vc4 devices?
- Right now root privileges are necessary to open the heap. Should we
allow everybody to open the heap by default (after all, user-space can
already allocate arbitrary amounts of GPU memory)? Should we leave it
up to user-space to solve this issue (via logind/seatd or a Wayland
protocol or something else)?
TODO:
- Need to add !vc5 support.
- Need to the extend DMA heaps API to allow vc4 to unregister the heap
on unload.
[1]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4414
Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
Cc: Maxime Ripard <mripard@xxxxxxxxxx>
Cc: Daniel Vetter <daniel@xxxxxxxx>
Cc: Daniel Stone <daniel@xxxxxxxxxxxxx>
Cc: Erico Nunes <nunes.erico@xxxxxxxxx>
Cc: Iago Toral Quiroga <itoral@xxxxxxxxxx>
Cc: Maíra Canal <mcanal@xxxxxxxxxx>
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
drivers/gpu/drm/vc4/Makefile | 2 ++
drivers/gpu/drm/vc4/vc4_dma_heap.c | 51 ++++++++++++++++++++++++++++++
drivers/gpu/drm/vc4/vc4_drv.c | 6 ++++
drivers/gpu/drm/vc4/vc4_drv.h | 5 +++
4 files changed, 64 insertions(+)
create mode 100644 drivers/gpu/drm/vc4/vc4_dma_heap.c
diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile
index c41f89a15a55..e4048870cec7 100644
--- a/drivers/gpu/drm/vc4/Makefile
+++ b/drivers/gpu/drm/vc4/Makefile
@@ -34,4 +34,6 @@ vc4-$(CONFIG_DRM_VC4_KUNIT_TEST) += \
vc4-$(CONFIG_DEBUG_FS) += vc4_debugfs.o
+vc4-$(CONFIG_DMABUF_HEAPS) += vc4_dma_heap.o
+
obj-$(CONFIG_DRM_VC4) += vc4.o
diff --git a/drivers/gpu/drm/vc4/vc4_dma_heap.c b/drivers/gpu/drm/vc4/vc4_dma_heap.c
new file mode 100644
index 000000000000..010d0a88f3fa
--- /dev/null
+++ b/drivers/gpu/drm/vc4/vc4_dma_heap.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright © 2023 Simon Ser
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+
+#include "vc4_drv.h"
+
+static struct dma_buf *vc4_dma_heap_allocate(struct dma_heap *heap,
+ unsigned long size,
+ unsigned long fd_flags,
+ unsigned long heap_flags)
+{
+ struct vc4_dev *vc4 = dma_heap_get_drvdata(heap);
+ //DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct drm_gem_dma_object *dma_obj;
+ struct dma_buf *dmabuf;
+
+ if (WARN_ON_ONCE(!vc4->is_vc5))
+ return ERR_PTR(-ENODEV);
+
+ dma_obj = drm_gem_dma_create(&vc4->base, size);
+ if (IS_ERR(dma_obj))
+ return ERR_CAST(dma_obj);
+
+ dmabuf = drm_gem_prime_export(&dma_obj->base, fd_flags);
+ drm_gem_object_put(&dma_obj->base);
+ return dmabuf;
+}
+
+static const struct dma_heap_ops vc4_dma_heap_ops = {
+ .allocate = vc4_dma_heap_allocate,
+};
+
+int vc4_dma_heap_create(struct vc4_dev *vc4)
+{
+ struct dma_heap_export_info exp_info;
+ struct dma_heap *heap;
+
+ exp_info.name = "vc4"; /* TODO: allow multiple? */
+ exp_info.ops = &vc4_dma_heap_ops;
+ exp_info.priv = vc4; /* TODO: unregister when unloading */
+
+ heap = dma_heap_add(&exp_info);
+ if (IS_ERR(heap))
+ return PTR_ERR(heap);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index c133e96b8aca..c7297dd7d9d5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -391,6 +391,12 @@ static int vc4_drm_bind(struct device *dev)
drm_fbdev_dma_setup(drm, 16);
+#if IS_ENABLED(CONFIG_DMABUF_HEAPS)
+ ret = vc4_dma_heap_create(vc4);
+ if (ret)
+ goto err;
+#endif
+
return 0;
err:
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index ab61e96e7e14..d5c5dd18815c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -1078,4 +1078,9 @@ int vc4_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
int vc4_perfmon_get_values_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+/* vc4_dma_heap.c */
+#if IS_ENABLED(CONFIG_DMABUF_HEAPS)
+int vc4_dma_heap_create(struct vc4_dev *vc4);
+#endif
+
#endif /* _VC4_DRV_H_ */
