Re: [PATCH v6 1/2] drm/xe/sysctrl: Add helper to query application status

On Thu, Aug 06, 2026 at 05:09:31AM -0700, Anoop, Vijay wrote:
>From: Anoop Vijay <[email protected]>
>
>Add xe_sysctrl_check_app_status() to query the state of a System
>Controller application using get_app_status_by_id mailbox command.
>
>The helper maps xe_sysctrl_app_id values to firmware application
>IDs and returns the reported application state. Add a convenience
>wrapper to check diag firmware application readiness.
>
>Signed-off-by: Anoop Vijay <[email protected]>
>---
>v2: (Badal)
>- Return SysCtrl firmware application states instead of errno for
>  application lifecycle conditions
>
>v3: (Riana, Badal)
>- Replace string-based app identifiers with enum xe_sysctrl_app_id
>- Use get_app_status_by_id (opcode 0x05)
>- Move mailbox definitions to xe_sysctrl_mailbox_types.h
>- Return enum xe_sysctrl_fw_status consistently
>- Map communication failures to COMM_FAILURE
>- Add INITIALIZED state and status helpers
>- Fix Diagnostics typo
>
>v4: (Anshuman)
>- Make xe_sysctrl_check_app_status() internal
>- Remove unused app identifier
>
>v5: (Anshuman)
>- Rename OOBMSM status helper to use xe_sysctrl_* prefix
>- Remove xe_is_diag_fw_ready() helper
>
>v6: (Riana)
>- Use xe_sysctrl_create_command() helper instead of manual FIELD_PREP
>  header packing
>- Rename xe_sysctrl_get_app_status_by_id_{req,resp} to shorter names
>- Use hex constant for application ID
>- Move oCode application readiness helper to a separate patch
>- Add xe_sysctrl_is_diag_fw_ready() helper.
>---
> drivers/gpu/drm/xe/xe_sysctrl.c               | 63 +++++++++++++++++++
> drivers/gpu/drm/xe/xe_sysctrl.h               |  1 +
> drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h | 63 +++++++++++++++++++
> 3 files changed, 127 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
>index 1db20be8158b..35b8b8b85a48 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl.c
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.c
>@@ -13,9 +13,11 @@
> #include "xe_device.h"
> #include "xe_mmio.h"
> #include "xe_pm.h"
>+#include "xe_printk.h"
> #include "xe_soc_remapper.h"
> #include "xe_sysctrl.h"
> #include "xe_sysctrl_mailbox.h"
>+#include "xe_sysctrl_mailbox_types.h"
> #include "xe_sysctrl_types.h"
>
> /**
>@@ -130,3 +132,64 @@ void xe_sysctrl_pm_resume(struct xe_device *xe)
>
> 	xe_sysctrl_mailbox_init(sc);
> }
>+
>+static enum xe_sysctrl_fw_status
>+xe_sysctrl_check_app_status(struct xe_device *xe, enum xe_sysctrl_app_id app_id)
>+{
>+	struct xe_sysctrl_app_status_req req = {};
>+	struct xe_sysctrl_app_status_resp resp = {};
>+	struct xe_sysctrl_mailbox_command cmd = {};
>+	size_t out_len = 0;
>+	u32 flags;
>+	int ret;
>+
>+	if (!xe->info.has_sysctrl)
>+		return XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;
>+
>+	req.app_id = (u8)app_id;
>+
>+	xe_sysctrl_create_command(&cmd, XE_SYSCTRL_GROUP_CORE, XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID,
>+				  &req, sizeof(req), &resp, sizeof(resp));
>+
>+	ret = xe_sysctrl_send_command(&xe->sc, &cmd, &out_len);
>+	if (ret)
>+		return XE_SYSCTRL_FIRMWARE_COMM_FAILURE;
>+
>+	if (out_len != sizeof(resp)) {
>+		xe_err(xe, "sysctrl: unexpected get app status response length %zu (expected %zu)\n",
>+		       out_len, sizeof(resp));
>+		return XE_SYSCTRL_FIRMWARE_COMM_FAILURE;
>+	}
>+
>+	flags = resp.flags;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_VALID))
>+		return XE_SYSCTRL_FIRMWARE_APP_INVALID;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_BOOTED))
>+		return XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED;
>+
>+	if (!(flags & XE_SYSCTRL_APP_RESP_INITIALIZED))
>+		return XE_SYSCTRL_FIRMWARE_APP_BOOTED;
>+
>+	return XE_SYSCTRL_FIRMWARE_APP_INITIALIZED;
>+}
>+
>+/**
>+ * xe_sysctrl_is_diag_fw_ready() - Check if diag firmware is fully initialized
>+ * @xe: xe device instance
>+ *
>+ * Returns true if diag firmware has reached the initialized state, indicating
>+ * it is ready to handle requests. Returns true also on platforms without System
>+ * Controller support, as there is no firmware gate to wait on.

When there's no system controller, you still return true? What does that 
mean? On all such platforms Diag FW is already available/loaded? If so, 
please clarify in the comment. 

>+ *
>+ * Return: true if diag firmware is initialized or sysctrl is not present, false otherwise
>+ */
>+bool xe_sysctrl_is_diag_fw_ready(struct xe_device *xe)
>+{
>+	enum xe_sysctrl_fw_status status =
>+		xe_sysctrl_check_app_status(xe, XE_SYSCTRL_APP_DIAG);
>+
>+	return status == XE_SYSCTRL_FIRMWARE_APP_INITIALIZED ||
>+	       status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;
>+}
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
>index 090dffb6d55f..8dc576796890 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl.h
>+++ b/drivers/gpu/drm/xe/xe_sysctrl.h
>@@ -20,5 +20,6 @@ void xe_sysctrl_event(struct xe_sysctrl *sc);
> int xe_sysctrl_init(struct xe_device *xe);
> void xe_sysctrl_irq_handler(struct xe_device *xe, u32 master_ctl);
> void xe_sysctrl_pm_resume(struct xe_device *xe);
>+bool xe_sysctrl_is_diag_fw_ready(struct xe_device *xe);
>
> #endif
>diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>index d0341538ad05..5e39d2a9c2b0 100644
>--- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>+++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>@@ -14,9 +14,11 @@
>  * enum xe_sysctrl_group - System Controller command groups
>  *
>  * @XE_SYSCTRL_GROUP_GFSP: GFSP group
>+ * @XE_SYSCTRL_GROUP_CORE: Core group
>  */
> enum xe_sysctrl_group {
> 	XE_SYSCTRL_GROUP_GFSP			= 0x01,
>+	XE_SYSCTRL_GROUP_CORE			= 0xFF,
> };
>
> /**
>@@ -38,6 +40,67 @@ enum xe_sysctrl_gfsp_cmd {
> 	XE_SYSCTRL_CMD_SET_HEALTH		= 0x0C,
> };
>
>+/**
>+ * enum xe_sysctrl_core_cmd - Commands supported by Core group
>+ *
>+ * @XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID: Retrieve application status by ID
>+ */
>+enum xe_sysctrl_core_cmd {
>+	XE_SYSCTRL_CMD_GET_APP_STATUS_BY_ID		= 0x05,
>+};
>+
>+/**
>+ * struct xe_sysctrl_app_status_req - Get application status request
>+ *
>+ * @app_id: Application ID for which to retrieve status
>+ */
>+struct xe_sysctrl_app_status_req {
>+	u8 app_id;
>+} __packed;
>+
>+/**
>+ * struct xe_sysctrl_app_status_resp - Get application status response
>+ * @flags: Application status flags (see XE_SYSCTRL_APP_RESP_* definitions)
>+ */
>+struct xe_sysctrl_app_status_resp {
>+	u32 flags;
>+} __packed;
>+
>+/** XE_SYSCTRL_APP_RESP_VALID - app_id is recognized by System Controller firmware */
>+#define XE_SYSCTRL_APP_RESP_VALID		BIT(0)
>+/** XE_SYSCTRL_APP_RESP_BOOTED - application has completed its boot sequence */
>+#define XE_SYSCTRL_APP_RESP_BOOTED		BIT(1)
>+/** XE_SYSCTRL_APP_RESP_INITIALIZED - application has completed all post-boot initialization */
>+#define XE_SYSCTRL_APP_RESP_INITIALIZED		BIT(2)

I think caller doesn't care about the above bits, so these defines can 
just move to your c file.

>+
>+/**
>+ * enum xe_sysctrl_fw_status - System Controller firmware application lifecycle states
>+ *
>+ * @XE_SYSCTRL_FIRMWARE_APP_INVALID: app_id is not recognized by firmware
>+ * @XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED: application is known but has not yet booted
>+ * @XE_SYSCTRL_FIRMWARE_APP_BOOTED: boot sequence completed, post-boot init pending
>+ * @XE_SYSCTRL_FIRMWARE_APP_INITIALIZED: application fully operational
>+ * @XE_SYSCTRL_FIRMWARE_APP_NOTSUPP: System Controller not available on this device
>+ * @XE_SYSCTRL_FIRMWARE_COMM_FAILURE: communication with System Controller firmware failed
>+ */
>+enum xe_sysctrl_fw_status {
>+	XE_SYSCTRL_FIRMWARE_APP_INVALID,
>+	XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED,
>+	XE_SYSCTRL_FIRMWARE_APP_BOOTED,
>+	XE_SYSCTRL_FIRMWARE_APP_INITIALIZED,
>+	XE_SYSCTRL_FIRMWARE_APP_NOTSUPP,
>+	XE_SYSCTRL_FIRMWARE_COMM_FAILURE,
>+};
>+
>+/**
>+ * enum xe_sysctrl_app_id - Known System Controller application identifiers
>+ *
>+ * @XE_SYSCTRL_APP_DIAG: diag application (firmware ID 13)
>+ */
>+enum xe_sysctrl_app_id {
>+	XE_SYSCTRL_APP_DIAG	= 0x0D,
>+};

The helpers to check diag and oobmsm states do not take in APP ID, so 
the app ids should move to your C file.

Thanks,
Umesh

>+
> /**
>  * struct xe_sysctrl_mailbox_command - System Controller mailbox command
>  */
>-- 
>2.43.0
>