Re: [PATCH v4 1/4] drm/xe/sysctrl: Add System Controller get application status

[Anoop Vijay <anoop.c.vijay@intel.com>]

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


On 04-09-2026 02:12, Ruhl, Michael J wrote:
>
> Anoop,
>
> The XE VSEC code will only use this API if the device is CRI, so yes, 
> removing the NOTSUPP
>
> will work.
>
> Mike
>
Thanks for confirming. I will drop NOTSUPP handling and the 
corresponding checks in the _ready() helpers.

Anoop

> *From:*Vijay, Anoop C <anoop.c.vijay@intel.com>
> *Sent:* Thursday, September 3, 2026 4:13 PM
> *To:* Nerlige Ramappa, Umesh <umesh.nerlige.ramappa@intel.com>
> *Cc:* intel-xe@lists.freedesktop.org; Nilawar, Badal 
> <badal.nilawar@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; 
> Iddamsetty, Aravind <aravind.iddamsetty@intel.com>; Tauro, Riana 
> <riana.tauro@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>; 
> Roper, Matthew D <matthew.d.roper@intel.com>; Ruhl, Michael J 
> <michael.j.ruhl@intel.com>; Luse, Paul E <paul.e.luse@intel.com>; V, 
> Mohamed Mansoor <mohamed.mansoor.v@intel.com>; Nasim, Kam 
> <kam.nasim@intel.com>
> *Subject:* Re: [PATCH v4 1/4] drm/xe/sysctrl: Add System Controller 
> get application status
>
> On 04-09-2026 00:47, Umesh Nerlige Ramappa wrote:
>
>     On Thu, Sep 03, 2026 at 07:03:41AM -0700, Anoop, Vijay wrote:
>
>         From: Anoop Vijay <anoop.c.vijay@intel.com>
>         <mailto:anoop.c.vijay@intel.com>
>
>         Add support for querying System Controller firmware application
>         status. This allows callers to determine whether a given firmware
>         application has completed boot and post-boot initialization.
>
>         This patch is carried here only as a build dependency for this
>         series; it is authored and under review as part of a separate
>         get-app-status series and should be dropped/rebased away once
>         that
>         series lands.
>
>         Link:
>         https://lore.kernel.org/intel-xe/20260814095306.2482289-5-anoop.c.vijay@intel.com/
>         Link:
>         https://lore.kernel.org/intel-xe/20260814095306.2482289-6-anoop.c.vijay@intel.com/
>
>         Signed-off-by: Anoop Vijay <anoop.c.vijay@intel.com>
>         <mailto:anoop.c.vijay@intel.com>
>         ---
>         v4 (Rodrigo):
>         - Added lore links to the parent application-status series
>         ---
>         drivers/gpu/drm/xe/xe_sysctrl.c               | 82
>         +++++++++++++++++++
>         drivers/gpu/drm/xe/xe_sysctrl.h               |  2 +
>         drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h | 65
>         +++++++++++++++
>         3 files changed, 149 insertions(+)
>
>         diff --git a/drivers/gpu/drm/xe/xe_sysctrl.c
>         b/drivers/gpu/drm/xe/xe_sysctrl.c
>         index 62ccc9be71b4..5da591ade4e4 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"
>
>         /**
>         @@ -125,3 +127,83 @@ void xe_sysctrl_pm_resume(struct
>         xe_device *xe)
>
>             xe->soc_remapper.set_sysctrl_region(xe,
>         SYSCTRL_MAILBOX_INDEX);
>         }
>         +
>         +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;
>
>
>     Reiterating previous review comment - based on the users that you
>     had listed earlier who query app status, we will never get to the
>     above condition, so it can be safely dropped. Once you do that,
>     you can drop the status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP checks
>     in the below (_ready) helpers.
>
>     Instead, if you want to retain this check here, then each of the
>     below helpers must be split into _ready() and _supported(). The
>     split versions should check for the specific case. It's the
>     caller's responsibility to handle the cases as needed.
>
> APP_NOTSUPP was added based on Mike's earlier feedback to handle 
> non-SysCtrl platforms.
>
> @Ruhl, Michael J <mailto:michael.j.ruhl@intel.com>, can xe_vsec 
> perform platform check and only call _fw_ready() on SysCtrl-capable 
> platforms? If so, I'll remove APP_NOTSUPP. Otherwise, do you need 
> separate _supported() and _ready() helpers?
>
>
>
>         +
>         +    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_oobmsm_fw_ready() - Check if oCode firmware
>         is fully initialized
>         + * @xe: xe device instance
>         + *
>         + * Returns true if oCode 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.
>         + *
>         + * Return: true if oCode is initialized or sysctrl is not
>         present, false otherwise
>         + */
>         +bool xe_sysctrl_is_oobmsm_fw_ready(struct xe_device *xe)
>         +{
>         +    enum xe_sysctrl_fw_status status =
>         +        xe_sysctrl_check_app_status(xe, XE_SYSCTRL_APP_OCODE);
>         +
>         +    return status == XE_SYSCTRL_FIRMWARE_APP_INITIALIZED ||
>         +           status == XE_SYSCTRL_FIRMWARE_APP_NOTSUPP;
>
>
>     See above comment.
>
>
>         +}
>         +
>         +/**
>         + * 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.
>         + *
>         + * 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;
>
>
>     See above comment.
>
>     Thanks,
>     Umesh
>
>         +}
>         diff --git a/drivers/gpu/drm/xe/xe_sysctrl.h
>         b/drivers/gpu/drm/xe/xe_sysctrl.h
>         index 090dffb6d55f..b69a3f474236 100644
>         --- a/drivers/gpu/drm/xe/xe_sysctrl.h
>         +++ b/drivers/gpu/drm/xe/xe_sysctrl.h
>         @@ -20,5 +20,7 @@ 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_oobmsm_fw_ready(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 66e7cbcc3f91..d756b50cde6b 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,
>         };
>
>         /**
>         @@ -42,6 +44,69 @@ 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)
>         +
>         +/**
>         + * 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_OCODE: oCode application (firmware ID 12)
>         + * @XE_SYSCTRL_APP_DIAG: diag application (firmware ID 13)
>         + */
>         +enum xe_sysctrl_app_id {
>         +    XE_SYSCTRL_APP_OCODE    = 0x0C,
>         +    XE_SYSCTRL_APP_DIAG    = 0x0D,
>         +};
>         +
>         /**
>          * struct xe_sysctrl_mailbox_command - System Controller
>         mailbox command
>          */
>         -- 
>         2.43.0
>

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