Hey Alex,
Thanks for the review comments, please find mine inline.
- Shashank
On 19/05/2023 23:03, Alex Deucher wrote:
On Mon, Apr 24, 2023 at 1:39 PM Shashank Sharma <shashank.sharma@xxxxxxx> wrote:
From: Alex Deucher <alexander.deucher@xxxxxxx>
This patch intorduces new UAPI/IOCTL for usermode graphics
queue. The userspace app will fill this structure and request
the graphics driver to add a graphics work queue for it. The
output of this UAPI is a queue id.
This UAPI maps the queue into GPU, so the graphics app can start
submitting work to the queue as soon as the call returns.
V2: Addressed review comments from Alex and Christian
- Make the doorbell offset's comment clearer
- Change the output parameter name to queue_id
V3: Integration with doorbell manager
V4:
- Updated the UAPI doc (Pierre-Eric)
- Created a Union for engine specific MQDs (Alex)
- Added Christian's R-B
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Christian Koenig <christian.koenig@xxxxxxx>
Reviewed-by: Christian König <christian.koenig@xxxxxxx>
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
---
include/uapi/drm/amdgpu_drm.h | 95 +++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 4038abe8505a..083a1df8e8ef 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -54,6 +54,7 @@ extern "C" {
#define DRM_AMDGPU_VM 0x13
#define DRM_AMDGPU_FENCE_TO_HANDLE 0x14
#define DRM_AMDGPU_SCHED 0x15
+#define DRM_AMDGPU_USERQ 0x16
#define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
#define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -71,6 +72,7 @@ extern "C" {
#define DRM_IOCTL_AMDGPU_VM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_VM, union drm_amdgpu_vm)
#define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
#define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
+#define DRM_IOCTL_AMDGPU_USERQ DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ, union drm_amdgpu_userq)
/**
* DOC: memory domains
@@ -302,6 +304,99 @@ union drm_amdgpu_ctx {
union drm_amdgpu_ctx_out out;
};
+/* user queue IOCTL */
+#define AMDGPU_USERQ_OP_CREATE 1
+#define AMDGPU_USERQ_OP_FREE 2
+
+#define AMDGPU_USERQ_MQD_FLAGS_SECURE (1 << 0)
+#define AMDGPU_USERQ_MQD_FLAGS_AQL (1 << 1)
+
+/*
+ * MQD (memory queue descriptor) is a set of parameters which allow
+ * the GPU to uniquely define and identify a usermode queue.
+ */
+struct drm_amdgpu_userq_mqd_gfx {
+ /**
+ * @flags: flags to indicate special function for queue like secure
+ * buffer (TMZ). Unused for now.
+ */
+ __u32 flags;
+ /**
+ * @doorbell_handle: the handle of doorbell GEM object
+ * associated to this client.
+ */
+ __u32 doorbell_handle;
+ /**
+ * @doorbell_offset: 32-bit offset of the doorbell in the doorbell bo.
+ * Kernel will generate absolute doorbell offset using doorbell_handle
+ * and doorbell_offset in the doorbell bo.
+ */
+ __u32 doorbell_offset;
+ /**
+ * @queue_va: Virtual address of the GPU memory which holds the queue
+ * object. The queue holds the workload packets.
+ */
+ __u64 queue_va;
+ /**
+ * @queue_size: Size of the queue in bytes, this needs to be 256-byte
+ * aligned.
+ */
+ __u64 queue_size;
+ /**
+ * @rptr_va : Virtual address of the GPU memory which holds the ring RPTR.
+ * This object must be at least 8 byte in size and aligned to 8-byte offset.
+ */
+ __u64 rptr_va;
+ /**
+ * @wptr_va : Virtual address of the GPU memory which holds the ring WPTR.
+ * This object must be at least 8 byte in size and aligned to 8-byte offset.
+ *
+ * Queue, RPTR and WPTR can come from the same object, as long as the size
+ * and alignment related requirements are met.
+ */
+ __u64 wptr_va;
+ /**
+ * @shadow_va: Virtual address of the GPU memory to hold the shadow buffer.
+ * This must be a from a separate GPU object, and must be at least 4-page
+ * sized.
+ */
+ __u64 shadow_va;
You were planning to add the other pointers here as well for gds and csa right?
That was decided in this patch set only, from a different review
comment, so we are yet to work on it :)
+};
+
+ /*
+ * MQD can be different for different GPU IP/engine. This union will
+ * contain structures of MQDs corresponding to different engines.
+ */
+union drm_amdgpu_userq_mqd {
+ struct drm_amdgpu_userq_mqd_gfx gfx;
+};
+
+struct drm_amdgpu_userq_in {
+ /** AMDGPU_USERQ_OP_* */
+ __u32 op;
+ /** Flags */
+ __u32 flags;
+ /** Queue handle for USERQ_OP_FREE */
+ __u32 queue_id;
+ /** the target GPU engine to execute workload (AMDGPU_HW_IP_*) */
+ __u32 ip_type;
+
+ /** Queue descriptor for USERQ_OP_CREATE */
+ union drm_amdgpu_userq_mqd mqd;
Just thinking out loud here, I wonder if we should make this a
pointer/size element so that we could make this variable sized in case
some future MQD ends up being bigger. I guess we could always add a
AMDGPU_USERQ_OP_CREATE2 if we ever need to expand it.
I think it is a good idea and sounds scalable as well. Then we might not
even need a union, we can just define MQD size and a ptr.
- Shashank
Alex
+};
+
+struct drm_amdgpu_userq_out {
+ /** Queue handle */
+ __u32 queue_id;
+ /** Flags */
+ __u32 flags;
+};
+
+union drm_amdgpu_userq {
+ struct drm_amdgpu_userq_in in;
+ struct drm_amdgpu_userq_out out;
+};
+
/* vm ioctl */
#define AMDGPU_VM_OP_RESERVE_VMID 1
#define AMDGPU_VM_OP_UNRESERVE_VMID 2
--
2.40.0
