Re: [PATCH v2] drm/xe/sysctrl: Add helper to query application status

["Nilawar, Badal" <badal.nilawar@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6166 bytes --]


On 21-04-2026 21:08, Anoop, Vijay wrote:
> From: Anoop Vijay<anoop.c.vijay@intel.com>
>
> Introduce xe_sysctrl_check_app_status() to query whether a sysctrl
> application is loaded and booted, returning error codes for not loaded
> or not booted states.
Fix the commit message.
>
> Signed-off-by: Anoop Vijay<anoop.c.vijay@intel.com>
> ---
> v2: (Badal)
> - Return SysCtrl firmware application states instead of errno for application lifecycle conditions
> ---
>   drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h | 44 +++++++++++++++++
>   drivers/gpu/drm/xe/xe_sysctrl.c         | 65 +++++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_sysctrl.h         |  1 +
>   3 files changed, 110 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h b/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
> index 4cbde267ac44..8ed9ba791f3f 100644
> --- a/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
> +++ b/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
> @@ -62,4 +62,48 @@ struct xe_sysctrl_app_msg_hdr {
>   #define APP_HDR_VERSION_MASK		GENMASK(23, 16)
>   #define APP_HDR_RESERVED_MASK		GENMASK(31, 24)
>   
> +/** System Controller Core command group */
> +#define XE_SYSCTRL_GROUP_CORE		0xFF
> +
> +/*
> + * XE_SYSCTRL_CMD_GET_APP_STATUS - Query application status
> + *
> + * Check if a System Controller application is loaded and its status.
> + *
> + * Group: XE_SYSCTRL_GROUP_CORE (0xFF)
> + * Command: 0x04
> + */
> +#define XE_SYSCTRL_CMD_GET_APP_STATUS	0x04
> +
> +/**
> + * struct xe_sysctrl_get_app_status_req - Get application status request
> + * @app_name: Application name (16 bytes, null-terminated)
> + */
> +struct xe_sysctrl_get_app_status_req {
> +	char app_name[16];
> +} __packed;
> +
> +/**
> + * struct xe_sysctrl_get_app_status_resp - Get application status response
> + * @handle: Identifier used to control the app
> + * @flags: Application status flags (see XE_SYSCTRL_APP_* definitions)
> + */
> +struct xe_sysctrl_get_app_status_resp {
> +	u16 handle;
> +	u16 flags;
> +} __packed;
> +
> +/**
> + * DOC: System Controller Application Status Flags
> + *
> + * Flags returned in xe_sysctrl_get_app_status_resp indicating the
> + * current state of a System Controller application.
> + */
> +
> +enum xe_sysctrl_fw_status {
> +	XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED = 0, /* Firmware application not loaded */
> +	XE_SYSCTRL_FIRMWARE_APP_LOADED,		/* Firmware application loaded, but not booted */
> +	XE_SYSCTRL_FIRMWARE_APP_BOOTED,		/* Firmware application booted */
> +};
> +
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c b/drivers/gpu/drm/xe/xe_sysctrl.c
> index 2bcef304eb9a..8d51f8cdaaa4 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.c
> @@ -14,8 +14,11 @@
>   #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"
>   
> +#define XE_SYSCTRL_APP_BOOTED	BIT(2)
In previous revision there were more bits. Any specific reason to drop 
them in this rev.
> +
>   /**
>    * DOC: System Controller (sysctrl)
>    *
> @@ -99,3 +102,65 @@ void xe_sysctrl_pm_resume(struct xe_device *xe)
>   
>   	xe_sysctrl_mailbox_init(sc);
>   }
> +
> +/**
> + * xe_sysctrl_check_app_status - Check if System Controller app is loaded
> + * @xe: xe device instance
> + * @app_name: Name of the application to check (max 15 chars + null terminator)
> + *
> + * Query System Controller firmware to verify if the specified application
> + * is loaded and booted. This is typically required before sending commands
> + * to application-specific command groups.
> + *
> + * Common application names:
> + * - "ocode"- oCode
> + * - "diag" - Dianostics
> + *
> + * Return: XE_SYSCTRL_FIRMWARE_APP_BOOTED if app is loaded and booted, error code otherwise:
> + *		-EOPNOTSUPP if System Controller is not available
> + *		-EINVAL if app_name is invalid
> + *		-EIO if communication with firmware failed
> + *		XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED if application is not loaded
> + *		XE_SYSCTRL_FIRMWARE_APP_LOADED if application is loaded but not booted yet
> + */
> +int xe_sysctrl_check_app_status(struct xe_device *xe, const char *app_name)
> +{
> +	struct xe_sysctrl_get_app_status_req req = {};
> +	struct xe_sysctrl_get_app_status_resp resp = {};
> +	struct xe_sysctrl_mailbox_command cmd = {};
> +	size_t out_len = 0;
> +	u16 flags;
> +	int ret;
> +
> +	if (!xe->info.has_sysctrl)
> +		return -EOPNOTSUPP;
> +
> +	if (!app_name || strlen(app_name) == 0 || strlen(app_name) >= sizeof(req.app_name))
> +		return -EINVAL;
Is this check necessary? IMO for invalid app name XE_SYSCTRL_APP_VALID 
bit will not be set. You can add enum as XE_SYSCTRL_FIRMWARE_APP_INVALID.
> +
> +	strscpy(req.app_name, app_name, sizeof(req.app_name));
> +
> +	cmd.header.data =
> +		cpu_to_le32(FIELD_PREP(APP_HDR_GROUP_ID_MASK, XE_SYSCTRL_GROUP_CORE) |
> +			    FIELD_PREP(APP_HDR_COMMAND_MASK, XE_SYSCTRL_CMD_GET_APP_STATUS) |
> +			    FIELD_PREP(APP_HDR_VERSION_MASK, 0));
> +
> +	cmd.data_in = &req;
> +	cmd.data_in_len = sizeof(req);
> +	cmd.data_out = &resp;
> +	cmd.data_out_len = sizeof(resp);
> +
> +	ret = xe_sysctrl_send_command(&xe->sc, &cmd, &out_len);
> +	if (ret)
> +		return XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED;

Is it appropriate to treat error as XE_SYSCTRL_FIRMWARE_APP_NOT_LOADED? 
As per my understanding when app is not loaded then only 
XE_SYSCTRL_APP_VALID should be returned.

Thanks, Badal

> +
> +	if (out_len != sizeof(resp))
> +		return -EIO;
> +
> +	flags = le16_to_cpu(resp.flags);
> +
> +	if (!(flags & XE_SYSCTRL_APP_BOOTED))
> +		return XE_SYSCTRL_FIRMWARE_APP_LOADED;
> +
> +	return XE_SYSCTRL_FIRMWARE_APP_BOOTED;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h b/drivers/gpu/drm/xe/xe_sysctrl.h
> index f3b0f3716b2f..db2743d54cea 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
> @@ -18,5 +18,6 @@ static inline struct xe_device *sc_to_xe(struct xe_sysctrl *sc)
>   
>   int xe_sysctrl_init(struct xe_device *xe);
>   void xe_sysctrl_pm_resume(struct xe_device *xe);
> +int xe_sysctrl_check_app_status(struct xe_device *xe, const char *app_name);
>   
>   #endif

[-- Attachment #2: Type: text/html, Size: 7527 bytes --]