[Intel-xe] [PATCH 09/12] drm/xe/gsc: Add an interface for GSC packet submissions
Kandpal, Suraj
suraj.kandpal at intel.com
Wed Nov 8 08:25:27 UTC 2023
On 10/31/2023 1:08 AM, Kandpal, Suraj wrote:
-----Original Message-----
From: Ceraolo Spurio, Daniele <daniele.ceraolospurio at intel.com><mailto:daniele.ceraolospurio at intel.com>
Sent: Saturday, October 28, 2023 3:59 AM
To: intel-xe at lists.freedesktop.org<mailto:intel-xe at lists.freedesktop.org>
Cc: Ceraolo Spurio, Daniele <daniele.ceraolospurio at intel.com><mailto:daniele.ceraolospurio at intel.com>; Teres Alexis,
Alan Previn <alan.previn.teres.alexis at intel.com><mailto:alan.previn.teres.alexis at intel.com>; Kandpal, Suraj
<suraj.kandpal at intel.com><mailto:suraj.kandpal at intel.com>
Subject: [PATCH 09/12] drm/xe/gsc: Add an interface for GSC packet
submissions
Communication with the GSC FW is done via input/output buffers, whose
addresses are provided via a GSCCS command. The buffers contain a generic
header and a client-specific packet (e.g. PXP, HDCP); the clients don't care
about the header format and/or the GSCCS command in the batch, they only
care about their client-specific header. This patch therefore introduces helpers
that allow the callers to automatically fill in the input header, submit the GSCCS
job and decode the output header, to make it so that the caller only needs to
worry about their client-specific input and output messages.
NOTE: this patch by itself only adds the interface so it does nothing, I've kept it
separate for review but the plan is to squash it with the follow up patch before
merge, so that the interface and the user are introduced at the same time.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com><mailto:daniele.ceraolospurio at intel.com>
Cc: Alan Previn <alan.previn.teres.alexis at intel.com><mailto:alan.previn.teres.alexis at intel.com>
Cc: Suraj Kandpal <suraj.kandpal at intel.com><mailto:suraj.kandpal at intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
.../gpu/drm/xe/abi/gsc_command_header_abi.h | 46 +++++
.../gpu/drm/xe/instructions/xe_gsc_commands.h | 2 +
drivers/gpu/drm/xe/xe_gsc_submit.c | 170 ++++++++++++++++++
drivers/gpu/drm/xe/xe_gsc_submit.h | 30 ++++
5 files changed, 249 insertions(+)
create mode 100644 drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
create mode 100644 drivers/gpu/drm/xe/xe_gsc_submit.c
create mode 100644 drivers/gpu/drm/xe/xe_gsc_submit.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile index
4a442dcf4d79..876c122ad63c 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -58,6 +58,7 @@ xe-y += xe_bb.o \
xe_force_wake.o \
xe_ggtt.o \
xe_gsc.o \
+ xe_gsc_submit.o \
xe_gt.o \
xe_gt_clock.o \
xe_gt_debugfs.o \
diff --git a/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
b/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
new file mode 100644
index 000000000000..a4c2646803b5
--- /dev/null
+++ b/drivers/gpu/drm/xe/abi/gsc_command_header_abi.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _ABI_GSC_COMMAND_HEADER_ABI_H
+#define _ABI_GSC_COMMAND_HEADER_ABI_H
+
+#include <linux/types.h>
+
+struct intel_gsc_mtl_header {
+ u32 validity_marker;
+#define GSC_HECI_VALIDITY_MARKER 0xA578875A
+
+ u8 heci_client_id;
Also during adding this is i915 we had decided to add certain defines
As seen here below may be add them as well
u8 heci_client_id;
#define HECI_MEADDRESS_MKHI 7
#define HECI_MEADDRESS_PROXY 10
#define HECI_MEADDRESS_PXP 17
#define HECI_MEADDRESS_HDCP 18
Before answering the specific question here, let me explain why the design diverged compared to i915, as it will be relevant for further comments as well. One big difference in Xe is the way memory is accessed, which now requires the use of xe_map_<rd/wr>_field (instead of doing direct access via pointer dereference), for which I have created the mtl_gsc_header_<rd/wr> wrappers. This lead me to wanting to make sure that all accesses to the header are done in this file, so that we don't need to export the wrappers, as that would require including the memory paths as well and make things complicated; consequently, I don't want other files to have to include the gsc_command_header_abi.h file, as they should have no need to do any header manipulation.
If other files can't include the header, the client-specific defines need to be moved to the client-specific files. For example, HECI_MEADDRESS_MKHI is defined in gsc_mkhi_commands_abi.h in the next patch. I'd expect the other defines to also go in the respective _abi.h files.
+
+ u8 reserved1;
+
+ u16 header_version;
+#define MTL_GSC_HEADER_VERSION 1
+
+ /* FW allows host to decide host_session handle as it sees fit. */
+ u64 host_session_handle;
Also during adding this is i915 we had decided to add certain masks for PXP and HDCP
As seen here below may be add the as well
/*
* FW allows host to decide host_session handle
* as it sees fit.
* For intertracebility reserving select bits(60-63)
* to differentiate caller-target subsystem
* 0000 - HDCP
* 0001 - PXP Single Session
*/
u64 host_session_handle;
#define HOST_SESSION_MASK REG_GENMASK64(63, 60)
#define HOST_SESSION_PXP_SINGLE BIT_ULL(60)
Given the greater separation of layers (as explained above), I don't think we should special case the common code based on the caller. If we want to set part of the session id based on the heci client, then we can just encode the whole heci_client_id (see below).
+
+ /* handle generated by FW for messages that need to be re-submitted
*/
+ u64 gsc_message_handle;
+
+ u32 message_size; /* lower 20 bits only, upper 12 are reserved */
+
+ /*
+ * Flags mask:
+ * Bit 0: Pending
+ * Bit 1: Session Cleanup;
+ * Bits 2-15: Flags
+ * Bits 16-31: Extension Size
+ * According to internal spec flags are either input or output
+ * we distinguish the flags using OUTFLAG or INFLAG
+ */
+ u32 flags;
+#define GSC_OUTFLAG_MSG_PENDING BIT(0)
+#define GSC_INFLAG_MSG_CLEANUP BIT(1)
+
+ u32 status;
+} __packed;
+
+#endif
diff --git a/drivers/gpu/drm/xe/instructions/xe_gsc_commands.h
b/drivers/gpu/drm/xe/instructions/xe_gsc_commands.h
index c7a833d7f965..f8949cad9d0f 100644
--- a/drivers/gpu/drm/xe/instructions/xe_gsc_commands.h
+++ b/drivers/gpu/drm/xe/instructions/xe_gsc_commands.h
@@ -28,6 +28,8 @@
REG_FIELD_PREP(GSC_OPCODE, op) | \
REG_FIELD_PREP(GSC_CMD_DATA_AND_LEN, dl))
+#define GSC_HECI_CMD_PKT __GSC_INSTR(0, 6)
+
#define GSC_FW_LOAD __GSC_INSTR(1, 2)
#define GSC_FW_LOAD_LIMIT_VALID REG_BIT(31)
diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c
b/drivers/gpu/drm/xe/xe_gsc_submit.c
new file mode 100644
index 000000000000..e96336ac4728
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_gsc_submit.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#include "xe_gsc_submit.h"
+
+#include "abi/gsc_command_header_abi.h"
+#include "xe_bb.h"
+#include "xe_exec_queue.h"
+#include "xe_gt_printk.h"
+#include "xe_gt_types.h"
+#include "xe_map.h"
+#include "xe_sched_job.h"
+#include "instructions/xe_gsc_commands.h"
+#include "regs/xe_gsc_regs.h"
+
+#define GSC_HDR_SIZE (sizeof(struct intel_gsc_mtl_header)) /* shorthand
+define */
+
+#define mtl_gsc_header_wr(xe_, map_, offset_, field_, val_) \
+ xe_map_wr_field(xe_, map_, offset_, struct intel_gsc_mtl_header,
+field_, val_)
+
+#define mtl_gsc_header_rd(xe_, map_, offset_, field_) \
+ xe_map_rd_field(xe_, map_, offset_, struct intel_gsc_mtl_header,
+field_)
+
+static struct xe_gt *
+gsc_to_gt(struct xe_gsc *gsc)
+{
+ return container_of(gsc, struct xe_gt, uc.gsc); }
+
+/**
+ * xe_gsc_header_write - write the MTL GSC header in memory
You have define xe_gsc_header_write up in comments but the actual
Function is still called xe_gsc_emit_header one of these names need to be changed
will fix
+ * @xe: the Xe device
+ * @map: the iosys map to write to
+ * @offset: offset from the start of the map at which to write the
+header
+ * @heci_client_id: client id identifying the type of command (see abi
+for values)
+ * @host_session_id: host session ID of the caller
+ * @payload_size: size of the payload that follows the header
+ *
+ * Returns: offset memory location following the header */
+u32 xe_gsc_emit_header(struct xe_device *xe, struct iosys_map *map, u32
offset,
+ u8 heci_client_id, u64 host_session_id, u32 payload_size) {
+ xe_map_memset(xe, map, offset, 0, GSC_HDR_SIZE);
+
+ mtl_gsc_header_wr(xe, map, offset, validity_marker,
GSC_HECI_VALIDITY_MARKER);
+ mtl_gsc_header_wr(xe, map, offset, heci_client_id, heci_client_id);
+ mtl_gsc_header_wr(xe, map, offset, host_session_handle,
host_session_id);
Also We OR the required host_session_handle with the appropriate mask
Dedpending on client id as seen below:
host_session_id &= ~HOST_SESSION_MASK;
if (host_session_id && heci_client_id == HECI_MEADDRESS_PXP)
host_session_id |= HOST_SESSION_PXP_SINGLE;
any reason to move away from this as it was decided we will have it here in a common place
Continuing from the comment above, this could become:
#define HOST_SESSION_MASK REG_GENMASK64(63, 56)
xe_assert(xe, !(host_session_id & HOST_SESSION_MASK));
if (host_session_id)
host_session_id |= FIELD_PREP(HOST_SESSION_MASK, heci_client_id);
>That way we can identify the client that generated the session without having to >special-case. Does that work for you?
That should work and other things suggested sound fair too
Regards,
Suraj Kandpal
Daniele
Regards,
Suraj Kandpal
+ mtl_gsc_header_wr(xe, map, offset, header_version,
MTL_GSC_HEADER_VERSION);
+ mtl_gsc_header_wr(xe, map, offset, message_size, payload_size +
+GSC_HDR_SIZE);
+
+ return offset + GSC_HDR_SIZE;
+};
+
+/**
+ * xe_gsc_check_and_update_pending - check the pending bit and update
+the input
+ * header with the retry handle from the output header
+ * @xe: the Xe device
+ * @in: the iosys map containing the input buffer
+ * @offset_in: offset within the iosys at which the input buffer is
+located
+ * @out: the iosys map containing the output buffer
+ * @offset_out: offset within the iosys at which the output buffer is
+located
+ *
+ * Returns: true if the pending bit was set, false otherwise */ bool
+xe_gsc_check_and_update_pending(struct xe_device *xe,
+ struct iosys_map *in, u32 offset_in,
+ struct iosys_map *out, u32 offset_out) {
+ if (mtl_gsc_header_rd(xe, out, offset_out, flags) &
GSC_OUTFLAG_MSG_PENDING) {
+ u64 handle = mtl_gsc_header_rd(xe, out, offset_out,
+gsc_message_handle);
+
+ mtl_gsc_header_wr(xe, in, offset_in, gsc_message_handle,
handle);
+
+ return true;
+ }
+
+ return false;
+}
+
+/**
+ * xe_gsc_read_out_header - reads and validates the output header and
+returns
+ * the offset of the reply following the header
+ * @xe: the Xe device
+ * @map: the iosys map containing the output buffer
+ * @offset: offset within the iosys at which the output buffer is
+located
+ * @min_payload_size: minimum size of the message excluding the gsc
+header
+ * @payload_offset: optional pointer to be set to the payload offset
+ *
+ * Returns: -errno value on failure, 0 otherwise */ int
+xe_gsc_read_out_header(struct xe_device *xe,
+ struct iosys_map *map, u32 offset,
+ u32 min_payload_size,
+ u32 *payload_offset)
+{
+ u32 marker = mtl_gsc_header_rd(xe, map, offset, validity_marker);
+ u32 size = mtl_gsc_header_rd(xe, map, offset, message_size);
+ u32 payload_size = size - GSC_HDR_SIZE;
+
+ if (marker != GSC_HECI_VALIDITY_MARKER)
+ return -EPROTO;
+
+ if (size < GSC_HDR_SIZE || payload_size < min_payload_size)
+ return -ENODATA;
+
+ if (payload_offset)
+ *payload_offset = offset + GSC_HDR_SIZE;
+
+ return 0;
+}
+
+/**
+ * xe_gsc_pkt_submit_kernel - submit a kernel heci pkt to the GSC
+ * @xe: the GSC uC
+ * @addr_in: GGTT address of the message to send to the GSC
+ * @size_in: size of the message to send to the GSC
+ * @addr_out: GGTT address for the GSC to write the reply to
+ * @size_out: size of the memory reserved for the reply */ int
+xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in,
+ u64 addr_out, u32 size_out)
+{
+ struct xe_gt *gt = gsc_to_gt(gsc);
+ struct xe_bb *bb;
+ struct xe_sched_job *job;
+ struct dma_fence *fence;
+ long timeout;
+
+ if (size_in < GSC_HDR_SIZE)
+ return -ENODATA;
+
+ if (size_out < GSC_HDR_SIZE)
+ return -ENOMEM;
+
+ bb = xe_bb_new(gt, 8, false);
+ if (IS_ERR(bb))
+ return PTR_ERR(bb);
+
+ bb->cs[bb->len++] = GSC_HECI_CMD_PKT;
+ bb->cs[bb->len++] = lower_32_bits(addr_in);
+ bb->cs[bb->len++] = upper_32_bits(addr_in);
+ bb->cs[bb->len++] = size_in;
+ bb->cs[bb->len++] = lower_32_bits(addr_out);
+ bb->cs[bb->len++] = upper_32_bits(addr_out);
+ bb->cs[bb->len++] = size_out;
+ bb->cs[bb->len++] = 0;
+
+ job = xe_bb_create_job(gsc->q, bb);
+ if (IS_ERR(job)) {
+ xe_bb_free(bb, NULL);
+ return PTR_ERR(job);
+ }
+
+ xe_sched_job_arm(job);
+ fence = dma_fence_get(&job->drm.s_fence->finished);
+ xe_sched_job_push(job);
+
+ timeout = dma_fence_wait_timeout(fence, false, HZ);
+ dma_fence_put(fence);
+ xe_bb_free(bb, NULL);
+ if (timeout < 0)
+ return timeout;
+ else if (!timeout)
+ return -ETIME;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h
b/drivers/gpu/drm/xe/xe_gsc_submit.h
new file mode 100644
index 000000000000..0801da5d446a
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_gsc_submit.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef _XE_GSC_SUBMIT_H_
+#define _XE_GSC_SUBMIT_H_
+
+#include <linux/types.h>
+
+struct iosys_map;
+struct xe_device;
+struct xe_gsc;
+
+u32 xe_gsc_emit_header(struct xe_device *xe, struct iosys_map *map, u32
offset,
+ u8 heci_client_id, u64 host_session_id, u32 payload_size);
+
+bool xe_gsc_check_and_update_pending(struct xe_device *xe,
+ struct iosys_map *in, u32 offset_in,
+ struct iosys_map *out, u32 offset_out);
+
+int xe_gsc_read_out_header(struct xe_device *xe,
+ struct iosys_map *map, u32 offset,
+ u32 min_payload_size,
+ u32 *payload_offset);
+
+int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in,
+ u64 addr_out, u32 size_out);
+
+#endif
--
2.41.0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20231108/3bff4b14/attachment-0001.htm>
More information about the Intel-xe
mailing list