On 1/11/2023 1:42 PM, Alan Previn wrote:
Add MTL's function for ARB session creation using PXP firmware
version 4.3 ABI structure format.
Before checking the return status, look at the GSC-CS-Mem-Header's
pending-bit which means the GSC firmware is busy and we should
resubmit.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
---
.../drm/i915/pxp/intel_pxp_cmd_interface_43.h | 21 +++++++
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c | 56 ++++++++++++++++++-
2 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
index 52b9a61bcdd4..ee78c0817ba1 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
@@ -11,6 +11,7 @@
/* PXP-Cmd-Op definitions */
#define PXP43_CMDID_START_HUC_AUTH 0x0000003A
+#define PXP43_CMDID_INIT_SESSION 0x00000036
/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
#define PXP43_MAX_HECI_IN_SIZE (SZ_32K)
@@ -27,4 +28,24 @@ struct pxp43_start_huc_auth_out {
struct pxp_cmd_header header;
} __packed;
+/* PXP-Input-Packet: Init PXP session */
+struct pxp43_create_arb_in {
+ struct pxp_cmd_header header;
+ /* header.stream_id fields for vesion 4.3 of Init PXP session: */
+ #define PXP43_INIT_SESSION_VALID GENMASK(0, 0)
GENMASK(0, 0) -> BIT(0) ? same for (1, 1)
+ #define PXP43_INIT_SESSION_APPTYPE GENMASK(1, 1)
+ #define PXP43_INIT_SESSION_APPID GENMASK(17, 2)
+ u32 protection_mode;
+ #define PXP43_INIT_SESSION_PROTECTION_ARB 0x2
+ u32 sub_session_id;
+ u32 init_flags;
+ u32 rsvd[12];
+} __packed;
+
+/* PXP-Input-Packet: Init PXP session */
+struct pxp43_create_arb_out {
+ struct pxp_cmd_header header;
+ u32 rsvd[8];
+} __packed;
+
#endif /* __INTEL_PXP_FW_INTERFACE_43_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
index ff235822743e..1b06629ac16e 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
@@ -43,7 +43,8 @@ static inline struct gsccs_teelink_priv *pxp_to_gsccs_priv(struct intel_pxp *pxp
static int gsccs_send_message(struct intel_pxp *pxp,
void *msg_in, size_t msg_in_size,
void *msg_out, size_t msg_out_size_max,
- size_t *msg_out_len)
+ size_t *msg_out_len,
+ u64 *gsc_msg_handle_retry)
{
struct intel_gt *gt = pxp->ctrl_gt;
struct drm_i915_private *i915 = gt->i915;
@@ -75,6 +76,9 @@ static int gsccs_send_message(struct intel_pxp *pxp,
msg_in_size + sizeof(*header),
exec->host_session_handle);
+ /* copy caller provided gsc message handle if this is polling for a prior msg completion */
+ header->gsc_message_handle = *gsc_msg_handle_retry;
+
memcpy(exec->pkt_vaddr + sizeof(*header), msg_in, msg_in_size);
pkt.addr_in = i915_vma_offset(exec->pkt_vma);
@@ -91,7 +95,7 @@ static int gsccs_send_message(struct intel_pxp *pxp,
goto unlock;
}
- /* we keep separate location for reply, so get the response header loc first */
+ /* we keep separate location for reply, so go to the response header now */
Any reason to update the comment in this patch and not directly in the
original one?
header = exec->pkt_vaddr + PXP43_MAX_HECI_IN_SIZE;
/* Response validity marker, status and busyness */
@@ -108,6 +112,13 @@ static int gsccs_send_message(struct intel_pxp *pxp,
}
if (header->flags & GSC_HECI_FLAG_MSG_PENDING) {
drm_dbg(&i915->drm, "gsc PXP reply is busy\n");
+ /*
+ * When the GSC firmware replies with pending bit, it means that the requested
+ * operation has begun but the completion is pending and the caller needs
+ * to re-request with the gsc_message_handle that was returned by the firmware.
+ * until the pending bit is turned off.
+ */
+ *gsc_msg_handle_retry = header->gsc_message_handle;
ret = -EAGAIN;
goto unlock;
}
@@ -135,7 +146,46 @@ static int gsccs_send_message(struct intel_pxp *pxp,
int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
int arb_session_id)
{
- return -ENODEV;
+ struct gsccs_session_resources *exec = &pxp_to_gsccs_priv(pxp)->arb_exec_res;
+ struct pxp43_create_arb_in msg_in = {0};
+ struct pxp43_create_arb_out msg_out = {0};
+ u64 gsc_session_retry = 0;
+ int insize, outsize, ret, tries = 0;
+ void *inptr, *outptr;
+
+ /* get a unique host-session-handle (used later in HW cmds) at time of session creation */
+ get_random_bytes(&exec->host_session_handle, sizeof(exec->host_session_handle));
+
+ msg_in.header.api_version = PXP_APIVER(4, 3);
+ msg_in.header.command_id = PXP43_CMDID_INIT_SESSION;
+ msg_in.header.stream_id = (FIELD_PREP(PXP43_INIT_SESSION_APPID, arb_session_id) |
+ FIELD_PREP(PXP43_INIT_SESSION_VALID, 1) |
+ FIELD_PREP(PXP43_INIT_SESSION_APPTYPE, 0));
+ msg_in.header.buffer_len = sizeof(msg_in) - sizeof(msg_in.header);
+ msg_in.protection_mode = PXP43_INIT_SESSION_PROTECTION_ARB;
+
+ inptr = &msg_in;
+ outptr = &msg_out;
+ insize = sizeof(msg_in);
+ outsize = sizeof(msg_out);
Are those local vars required? Can't you just pass the values directly?
it doesn't look like you're saving many characters.
+
+ /*
+ * Keep sending request if GSC firmware was busy.
+ * Based on test data, we expects a worst case delay of 250 milisecs.
+ */
+ do {
+ ret = gsccs_send_message(pxp,
+ inptr, insize,
+ outptr, outsize, NULL,
+ &gsc_session_retry);
+ /* Only try again if gsc says so */
+ if (ret != -EAGAIN)
+ break;
+
+ msleep(20);
I seem to remember that the recommended sleep time was 50ms, but can't
find that in the specs now.
+ } while (++tries < 12);
Found a note in the specs that says we should give up retrying after 2
secs, so should probably adjust the retry count accordingly.
Daniele
+
+ return ret;
}
static void
